[Lldb-commits] [lldb] 195a8b9 - don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

[Luboš Luňák via lldb-commits]

Author: Luboš Luňák
Date: 2022-04-12T00:36:33+02:00
New Revision: 195a8b977efe7a087709692f98821cdab1826346

URL: https://github.com/llvm/llvm-project/commit/195a8b977efe7a087709692f98821cdab1826346
DIFF: https://github.com/llvm/llvm-project/commit/195a8b977efe7a087709692f98821cdab1826346.diff

LOG: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

Places calling LoadModuleAtAddress() 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.

Differential Revision: https://reviews.llvm.org/D123128

Added: 
    

Modified: 
    lldb/include/lldb/Target/DynamicLoader.h
    lldb/source/Core/DynamicLoader.cpp
    lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 4f07045f86457..397b4aa303448 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -203,6 +203,8 @@ class DynamicLoader : public PluginInterface {
 
   /// Locates or creates a module given by \p file and updates/loads the
   /// resulting module at the virtual base address \p base_addr.
+  /// Note that this calls Target::GetOrCreateModule with notify being false,
+  /// so it is necessary to call Target::ModulesDidLoad afterwards.
   virtual lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec &file,
                                              lldb::addr_t link_map_addr,
                                              lldb::addr_t base_addr,

diff  --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 36443228ecd1d..96e0d4ec6555d 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -152,8 +152,7 @@ ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec &file) {
   if (ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec))
     return module_sp;
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-                                                    /*notify=*/true))
+  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec, false))
     return module_sp;
 
   return nullptr;

diff  --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
index fc8596db54606..925fbfff22baa 100644
--- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
+++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
@@ -16,10 +16,10 @@ class ModuleLoadedNotifysTestCase(TestBase):
     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().
@@ -72,20 +72,24 @@ def test_launch_notifications(self):
         total_solibs_removed = 0
         total_modules_added_events = 0
         total_modules_removed_events = 0
+        already_loaded_modules = []
         while listener.GetNextEvent(event):
             if lldb.SBTarget.EventIsTargetEvent(event):
                 if event.GetType() == lldb.SBTarget.eBroadcastBitModulesLoaded:
                     solib_count = lldb.SBTarget.GetNumModulesFromEvent(event)
                     total_modules_added_events += 1
                     total_solibs_added += solib_count
+                    added_files = []
+                    i = 0
+                    while i < solib_count:
+                        module = lldb.SBTarget.GetModuleAtIndexFromEvent(i, event)
+                        self.assertTrue(module not in already_loaded_modules)
+                        already_loaded_modules.append(module)
+                        if self.TraceOn():
+                            added_files.append(module.GetFileSpec().GetFilename())
+                        i = i + 1
                     if self.TraceOn():
                         # print all of the binaries that have been added
-                        added_files = []
-                        i = 0
-                        while i < solib_count:
-                            module = lldb.SBTarget.GetModuleAtIndexFromEvent(i, event)
-                            added_files.append(module.GetFileSpec().GetFilename())
-                            i = i + 1
                         print("Loaded files: %s" % (', '.join(added_files)))
 
                 if event.GetType() == lldb.SBTarget.eBroadcastBitModulesUnloaded:
@@ -107,6 +111,7 @@ def test_launch_notifications(self):
         # 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)