[lldb] fix parallel module loading deadlock for Linux DYLD

lldb/include/lldb/Core/ModuleList.h

@@ -511,6 +511,12 @@ class ModuleList {
   /// Atomically swaps the contents of this module list with \a other.
   void Swap(ModuleList &other);
 
+  /// For each module in this ModuleList, preload its symbols.
+  ///
+  /// \param[in] parallelize
+  ///     If true, all modules will be preloaded in parallel.
+  void PreloadSymbols(bool parallelize) const;
+
 protected:
   // Class typedefs.
   typedef std::vector<lldb::ModuleSP>

lldb/include/lldb/Target/DynamicLoader.h

@@ -352,6 +352,7 @@ class DynamicLoader : public PluginInterface {
 protected:
   // Utility methods for derived classes
 
+  /// Find a module in the target that matches the given file.
   lldb::ModuleSP FindModuleViaTarget(const FileSpec &file);
 
   /// Checks to see if the target module has changed, updates the target

lldb/include/lldb/Target/Target.h

@@ -629,13 +629,20 @@ class Target : public std::enable_shared_from_this<Target>,
   /// or identify a matching Module already present in the Target,
   /// and return a shared pointer to it.
   ///
+  /// Note that this function previously also preloaded the module's symbols
+  /// depending on a setting. This function no longer does any module
+  /// preloading because that can potentially cause deadlocks when called in
+  /// parallel with this function.
+  ///
   /// \param[in] module_spec
   ///     The criteria that must be matched for the binary being loaded.
   ///     e.g. UUID, architecture, file path.
   ///
   /// \param[in] notify
   ///     If notify is true, and the Module is new to this Target,
-  ///     Target::ModulesDidLoad will be called.
+  ///     Target::ModulesDidLoad will be called. See note in
+  ///     Target::ModulesDidLoad about thread-safety with
+  ///     Target::GetOrCreateModule.
   ///     If notify is false, it is assumed that the caller is adding
   ///     multiple Modules and will call ModulesDidLoad with the
   ///     full list at the end.
@@ -931,6 +938,13 @@ class Target : public std::enable_shared_from_this<Target>,
   // the address of its previous instruction and return that address.
   lldb::addr_t GetBreakableLoadAddress(lldb::addr_t addr);
 
+  /// This call may preload module symbols, and may do so in parallel depending
+  /// on the following target settings:
+  ///   - TargetProperties::GetPreloadSymbols()
+  ///   - TargetProperties::GetParallelModuleLoad()
+  ///
+  /// Warning: if preloading is active and this is called in parallel with
+  /// Target::GetOrCreateModule, this may result in a ABBA deadlock situation.
   void ModulesDidLoad(ModuleList &module_list);
 
   void ModulesDidUnload(ModuleList &module_list, bool delete_locations);

lldb/source/Core/DynamicLoader.cpp

@@ -165,7 +165,8 @@ 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, false))
+  if (ModuleSP module_sp =
+          target.GetOrCreateModule(module_spec, /*notify=*/false))
     return module_sp;
 
   return nullptr;

lldb/source/Core/ModuleList.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
@@ -28,6 +29,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-defines.h"
+#include "llvm/Support/ThreadPool.h"
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/PosixApi.h"
@@ -1381,3 +1383,21 @@ void ModuleList::Swap(ModuleList &other) {
       m_modules_mutex, other.m_modules_mutex);
   m_modules.swap(other.m_modules);
 }
+
+void ModuleList::PreloadSymbols(bool parallelize) const {
+  std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+
+  if (!parallelize) {
+    for (const ModuleSP &module_sp : m_modules)
+      module_sp->PreloadSymbols();
+    return;
+  }
+
+  llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+  for (const ModuleSP &module_sp : m_modules)
+    task_group.async([module_sp] {
+      if (module_sp)
+        module_sp->PreloadSymbols();
+    });
+  task_group.wait();
+}

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

@@ -469,7 +469,8 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
           }
 
           ModuleSP module_sp = LoadModuleAtAddress(
-              so_entry.file_spec, so_entry.link_addr, so_entry.base_addr, true);
+              so_entry.file_spec, so_entry.link_addr, so_entry.base_addr,
+              /*base_addr_is_offset=*/true);
           if (!module_sp.get())
             return;
 
@@ -726,9 +727,8 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
       task_group.async(load_module_fn, *I);
     task_group.wait();
   } else {
-    for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
+    for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
       load_module_fn(*I);
-    }
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);

lldb/source/Target/Target.cpp

@@ -1855,6 +1855,9 @@ void Target::NotifyModulesRemoved(lldb_private::ModuleList &module_list) {
 }
 
 void Target::ModulesDidLoad(ModuleList &module_list) {
+  if (GetPreloadSymbols())
+    module_list.PreloadSymbols(GetParallelModuleLoad());
+
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
     for (size_t idx = 0; idx < num_images; ++idx) {
@@ -2509,10 +2512,6 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
         if (symbol_file_spec)
           module_sp->SetSymbolFileFileSpec(symbol_file_spec);
 
-        // Preload symbols outside of any lock, so hopefully we can do this for
-        // each library in parallel.
-        if (GetPreloadSymbols())
-          module_sp->PreloadSymbols();
         llvm::SmallVector<ModuleSP, 1> replaced_modules;
         for (ModuleSP &old_module_sp : old_modules) {
           if (m_images.GetIndexForModule(old_module_sp.get()) !=