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

[llunak]

Summary

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.

[labath]

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.

That would definitely be better.

[llunak]

Changed to always disable notify and added a comment about that to LoadModuleAtAddress().

[labath]

Cool. Thanks.

lldb/include/lldb/Target/DynamicLoader.h

public:
   virtual lldb::addr_t GetThreadLocalData(const lldb::ModuleSP module,
                                           const lldb::ThreadSP thread,
                                           lldb::addr_t tls_file_addr) {
     return LLDB_INVALID_ADDRESS;
   }
 
   /// 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,
                                              bool base_addr_is_offset);
 
   /// Get information about the shared cache for a process, if possible.
   ///
   /// On some systems (e.g. Darwin based systems), a set of libraries that are

lldb/source/Core/DynamicLoader.cpp

 
 ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec &file) {
   Target &target = m_process->GetTarget();
   ModuleSpec module_spec(file, target.GetArchitecture());
 
   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;
 }
 
 ModuleSP DynamicLoader::LoadModuleAtAddress(const FileSpec &file,
                                             addr_t link_map_addr,
                                             addr_t base_addr,

lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py

 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 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().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.cpp', '// breakpoint')
 
     def test_launch_notifications(self):
def test_launch_notifications(self):
         self.assertEqual(
             process.GetState(), lldb.eStateStopped,
             PROCESS_STOPPED)
 
         total_solibs_added = 0
         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
-                    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)
+                        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
                         print("Loaded files: %s" % (', '.join(added_files)))
 
                 if event.GetType() == lldb.SBTarget.eBroadcastBitModulesUnloaded:
                     solib_count = lldb.SBTarget.GetNumModulesFromEvent(event)
                     total_modules_removed_events += 1
                     total_solibs_removed += solib_count
                     if self.TraceOn():
                         # print all of the binaries that have been removed
                         removed_files = []
                         i = 0
                         while i < solib_count:
                             module = lldb.SBTarget.GetModuleAtIndexFromEvent(i, event)
                             removed_files.append(module.GetFileSpec().GetFilename())
                             i = i + 1
                         print("Unloaded files: %s" % (', '.join(removed_files)))
         
 
         # This is testing that we get back a small number of events with the loaded 
         # 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)