llunak created this revision.
llunak added a reviewer: jasonmolenda.
llunak added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
llunak requested review of this revision.
Herald added a subscriber: lldb-commits.
Places calling LoadModuleAddress() already call ModulesDidLoad() after a loop
calling LoadModuleAtAddress(), so it's not necessary to call it from there, and
the batched ModulesDidLoad() may be more efficient than this place calling it
one after one.
This also makes the ModuleLoadedNotifys test pass on Linux now that the
duplicates no longer bring down the average of modules notified per call.
This change is a prerequisite for parallelizing PreloadSymbols() in D122975
<https://reviews.llvm.org/D122975>.
Note that this patch could have been shorter if I simply changed
LoadModuleAtAddress() to always pass false for `notify` (all callers currently
call ModulesDidLoad() afterwards) and added a note to LoadModuleAtAddress()
docs about it, but I don't know if that's a good design choice. I can change
the patch to be that way if you want.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D123128
Files:
lldb/include/lldb/Target/DynamicLoader.h
lldb/source/Core/DynamicLoader.cpp
lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
Index: lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
===================================================================
--- lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
+++ lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
@@ -16,10 +16,10 @@
mydir = TestBase.compute_mydir(__file__)
NO_DEBUG_INFO_TESTCASE = True
- # DynamicLoaderDarwin should batch up notifications about
- # newly added/removed libraries. Other DynamicLoaders may
+ # At least DynamicLoaderDarwin and DynamicLoaderPOSIXDYLD should batch up
+ # notifications about newly added/removed libraries. Other DynamicLoaders may
# not be written this way.
- @skipUnlessDarwin
+ @skipUnlessPlatform(["linux"]+lldbplatformutil.getDarwinOSTriples())
def setUp(self):
# Call super's setUp().
@@ -107,6 +107,7 @@
# binaries in batches. Check that we got back more than 1 solib per event.
# In practice on Darwin today, we get back two events for a do-nothing c
# program: a.out and dyld, and then all the rest of the system libraries.
+ # On Linux we get events for ld.so, [vdso], the binary and then all libraries.
- avg_solibs_added_per_event = int(float(total_solibs_added) / float(total_modules_added_events))
+ avg_solibs_added_per_event = round(float(total_solibs_added) / float(total_modules_added_events))
self.assertGreater(avg_solibs_added_per_event, 1)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -396,7 +396,7 @@
lldb::ModuleSP LoadModuleAtAddress(const FileSpec &file,
lldb::addr_t link_map,
lldb::addr_t base_addr,
- bool value_is_offset);
+ bool value_is_offset, bool notify);
Status UpdateAutomaticSignalFiltering() override;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4548,13 +4548,14 @@
lldb::ModuleSP ProcessGDBRemote::LoadModuleAtAddress(const FileSpec &file,
lldb::addr_t link_map,
lldb::addr_t base_addr,
- bool value_is_offset) {
+ bool value_is_offset,
+ bool notify) {
DynamicLoader *loader = GetDynamicLoader();
if (!loader)
return nullptr;
- return loader->LoadModuleAtAddress(file, link_map, base_addr,
- value_is_offset);
+ return loader->LoadModuleAtAddress(file, link_map, base_addr, value_is_offset,
+ notify);
}
llvm::Error ProcessGDBRemote::LoadModules() {
@@ -4586,8 +4587,8 @@
FileSpec file(mod_name);
FileSystem::Instance().Resolve(file);
- lldb::ModuleSP module_sp =
- LoadModuleAtAddress(file, link_map, mod_base, mod_base_is_offset);
+ lldb::ModuleSP module_sp = LoadModuleAtAddress(file, link_map, mod_base,
+ mod_base_is_offset, false);
if (module_sp.get())
new_modules.Append(module_sp);
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -431,8 +431,8 @@
m_initial_modules_added = true;
}
for (; I != E; ++I) {
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
+ ModuleSP module_sp = LoadModuleAtAddress(I->file_spec, I->link_addr,
+ I->base_addr, true, false);
if (module_sp.get()) {
if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
&m_process->GetTarget()) == m_interpreter_base &&
@@ -603,8 +603,8 @@
module_names, m_process->GetTarget().GetArchitecture().GetTriple());
for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
+ ModuleSP module_sp = LoadModuleAtAddress(I->file_spec, I->link_addr,
+ I->base_addr, true, false);
if (module_sp.get()) {
LLDB_LOG(log, "LoadAllCurrentModules loading module: {0}",
I->file_spec.GetFilename());
Index: lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
@@ -356,7 +356,7 @@
FileSpec file(I->path);
FileSystem::Instance().Resolve(file);
ModuleSP module_sp =
- LoadModuleAtAddress(file, I->link_addr, I->base_addr, true);
+ LoadModuleAtAddress(file, I->link_addr, I->base_addr, true, false);
if (module_sp.get()) {
loaded_modules.AppendIfNeeded(module_sp);
new_modules.Append(module_sp);
@@ -475,7 +475,7 @@
const char *module_path = I->path.c_str();
FileSpec file(module_path);
ModuleSP module_sp =
- LoadModuleAtAddress(file, I->link_addr, I->base_addr, true);
+ LoadModuleAtAddress(file, I->link_addr, I->base_addr, true, false);
if (module_sp.get()) {
module_list.Append(module_sp);
} else {
Index: lldb/source/Core/DynamicLoader.cpp
===================================================================
--- lldb/source/Core/DynamicLoader.cpp
+++ lldb/source/Core/DynamicLoader.cpp
@@ -148,7 +148,8 @@
ModuleSP DynamicLoader::LoadModuleAtAddress(const FileSpec &file,
addr_t link_map_addr,
addr_t base_addr,
- bool base_addr_is_offset) {
+ bool base_addr_is_offset,
+ bool notify) {
Target &target = m_process->GetTarget();
ModuleList &modules = target.GetImages();
ModuleSpec module_spec(file, target.GetArchitecture());
@@ -160,8 +161,7 @@
return module_sp;
}
- if ((module_sp = target.GetOrCreateModule(module_spec,
- true /* notify */))) {
+ if ((module_sp = target.GetOrCreateModule(module_spec, notify))) {
UpdateLoadedSections(module_sp, link_map_addr, base_addr,
base_addr_is_offset);
return module_sp;
@@ -197,8 +197,7 @@
return module_sp;
}
- if ((module_sp = target.GetOrCreateModule(new_module_spec,
- true /* notify */))) {
+ if ((module_sp = target.GetOrCreateModule(new_module_spec, notify))) {
UpdateLoadedSections(module_sp, link_map_addr, base_addr, false);
return module_sp;
}
Index: lldb/include/lldb/Target/DynamicLoader.h
===================================================================
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -202,11 +202,13 @@
}
/// Locates or creates a module given by \p file and updates/loads the
- /// resulting module at the virtual base address \p base_addr.
+ /// resulting module at the virtual base address \p base_addr,
+ /// \p notify is passed to Target::GetOrCreateModule().
virtual lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec &file,
lldb::addr_t link_map_addr,
lldb::addr_t base_addr,
- bool base_addr_is_offset);
+ bool base_addr_is_offset,
+ bool notify);
/// Get information about the shared cache for a process, if possible.
///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
