diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index df473dff091f8..1ef90b469c70c 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/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 diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 75bb6cb6bb907..4131a57df7540 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/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 diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 40f9c9bea1c12..908094bfd888d 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -629,13 +629,20 @@ class Target : public std::enable_shared_from_this, /// 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, // 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); diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index b309e0f0a72fd..31d277bc19681 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/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; diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index d9f845681e701..5444c04e74522 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/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 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(); +} diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 470fc2a2fdbb9..3605e7b2c6960 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/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); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 3b51e17d1c4e0..6fc8053038d10 100644 --- a/lldb/source/Target/Target.cpp +++ b/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 replaced_modules; for (ModuleSP &old_module_sp : old_modules) { if (m_images.GetIndexForModule(old_module_sp.get()) !=