[Lldb-commits] [lldb] fix parallel module loading deadlock for Linux DYLD (PR #166480)
Tom Yang via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 4 23:47:48 PST 2025
https://github.com/zhyty updated https://github.com/llvm/llvm-project/pull/166480
>From ffd2e0c0cfcbe9c20deda9e95a08ebcc30dcb14e Mon Sep 17 00:00:00 2001
From: Tom Yang <toyang at fb.com>
Date: Tue, 14 Oct 2025 00:13:03 -0700
Subject: [PATCH 1/2] move preload out of GetOrCreateModules and parallelize
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D86004440
---
lldb/include/lldb/Core/ModuleList.h | 3 ++
lldb/include/lldb/Target/DynamicLoader.h | 17 +++++++-
lldb/include/lldb/Target/Target.h | 10 ++++-
lldb/source/Core/DynamicLoader.cpp | 12 ++++--
lldb/source/Core/ModuleList.cpp | 13 ++++++
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 40 ++++++++++++-------
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 9 +++--
.../wasm-DYLD/DynamicLoaderWasmDYLD.cpp | 6 ++-
.../wasm-DYLD/DynamicLoaderWasmDYLD.h | 9 +++--
lldb/source/Target/Target.cpp | 8 ++--
10 files changed, 92 insertions(+), 35 deletions(-)
diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index e71f3b2bad6b4..8e87a2cb01df6 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -511,6 +511,9 @@ 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.
+ void PreloadSymbols() const;
+
protected:
// Class typedefs.
typedef std::vector<lldb::ModuleSP>
diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 75bb6cb6bb907..271d5f761696c 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -207,10 +207,17 @@ class DynamicLoader : public PluginInterface {
/// 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.
+ ///
+ /// \param[in] defer_module_preload
+ /// If the module needs to be loaded, prevent the module from being
+ /// preloaded even if the user sets the preload symbols option. This is
+ /// used as a performance optimization should the caller preload the
+ /// modules in parallel after calling this function.
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 defer_module_preload = false);
/// Find/load a binary into lldb given a UUID and the address where it is
/// loaded in memory, or a slide to be applied to the file address.
@@ -352,7 +359,13 @@ class DynamicLoader : public PluginInterface {
protected:
// Utility methods for derived classes
- lldb::ModuleSP FindModuleViaTarget(const FileSpec &file);
+ /// Find a module in the target that matches the given file.
+ ///
+ /// \param[in] defer_module_preload
+ /// If the module needs to be loaded, prevent the module from being
+ /// preloaded even if the user sets the preload symbols option.
+ lldb::ModuleSP FindModuleViaTarget(const FileSpec &file,
+ bool defer_module_preload = false);
/// Checks to see if the target module has changed, updates the target
/// accordingly and returns the target executable module.
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index c375df248154f..af755508cbe74 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -649,12 +649,20 @@ class Target : public std::enable_shared_from_this<Target>,
/// will handle / summarize the failures in a custom way and
/// don't use these messages.
///
+ /// \param[in] defer_preload
+ /// If true, the module will not be preloaded even if
+ /// Target::GetPreloadSymbols() is true. This is useful when the caller
+ /// wishes to preload loaded modules in parallel after calling this
+ /// function for better performance. This is because it's currently not
+ /// thread-safe to do so during the execution of this function.
+ ///
/// \return
/// An empty ModuleSP will be returned if no matching file
/// was found. If error_ptr was non-nullptr, an error message
/// will likely be provided.
lldb::ModuleSP GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
- Status *error_ptr = nullptr);
+ Status *error_ptr = nullptr,
+ bool defer_preload = false);
// Settings accessors
diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 7580b15c02ce1..d1f2ae33af75e 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -154,7 +154,8 @@ DynamicLoader::GetSectionListFromModule(const ModuleSP module) const {
return sections;
}
-ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec &file) {
+ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec &file,
+ bool defer_module_preload) {
Target &target = m_process->GetTarget();
ModuleSpec module_spec(file, target.GetArchitecture());
if (UUID uuid = m_process->FindModuleUUID(file.GetPath())) {
@@ -165,7 +166,9 @@ 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, false /* notify */, nullptr /* error_ptr */,
+ defer_module_preload))
return module_sp;
return nullptr;
@@ -174,8 +177,9 @@ ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec &file) {
ModuleSP DynamicLoader::LoadModuleAtAddress(const FileSpec &file,
addr_t link_map_addr,
addr_t base_addr,
- bool base_addr_is_offset) {
- if (ModuleSP module_sp = FindModuleViaTarget(file)) {
+ bool base_addr_is_offset,
+ bool defer_module_preload) {
+ if (ModuleSP module_sp = FindModuleViaTarget(file, defer_module_preload)) {
UpdateLoadedSections(module_sp, link_map_addr, base_addr,
base_addr_is_offset);
return module_sp;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index c40612c1ced5e..c8e917bb0da4a 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
@@ -15,6 +16,7 @@
#include "lldb/Interpreter/OptionValueFileSpecList.h"
#include "lldb/Interpreter/OptionValueProperties.h"
#include "lldb/Interpreter/Property.h"
+#include "llvm/Support/ThreadPool.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/TypeList.h"
@@ -1357,3 +1359,14 @@ void ModuleList::Swap(ModuleList &other) {
m_modules_mutex, other.m_modules_mutex);
m_modules.swap(other.m_modules);
}
+
+void ModuleList::PreloadSymbols() const {
+ std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+ 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 destructor waits for all tasks to complete
+}
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 326b6910b5267..2ca3f1eb6e27a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -453,7 +453,8 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
// exists for the duration of this call in `m_rendezvous`.
auto load_module_fn =
[this, &loaded_modules, &new_modules,
- &interpreter_module_mutex](const DYLDRendezvous::SOEntry &so_entry) {
+ &interpreter_module_mutex](const DYLDRendezvous::SOEntry &so_entry,
+ bool defer_module_preload) {
// Don't load a duplicate copy of ld.so if we have already loaded it
// earlier in LoadInterpreterModule. If we instead loaded then
// unloaded it later, the section information for ld.so would be
@@ -469,7 +470,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,
+ true /* base_addr_is_offset */, defer_module_preload);
if (!module_sp.get())
return;
@@ -503,11 +505,14 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
if (m_process->GetTarget().GetParallelModuleLoad()) {
llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
for (; I != E; ++I)
- task_group.async(load_module_fn, *I);
+ task_group.async(load_module_fn, *I, true /* defer_module_preload */);
task_group.wait();
+
+ if (m_process->GetTarget().GetPreloadSymbols())
+ new_modules.PreloadSymbols();
} else {
for (; I != E; ++I)
- load_module_fn(*I);
+ load_module_fn(*I, false /* defer_module_preload */);
}
m_process->GetTarget().ModulesDidLoad(new_modules);
@@ -644,12 +649,12 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
return nullptr;
}
-ModuleSP DynamicLoaderPOSIXDYLD::LoadModuleAtAddress(const FileSpec &file,
- addr_t link_map_addr,
- addr_t base_addr,
- bool base_addr_is_offset) {
+ModuleSP DynamicLoaderPOSIXDYLD::LoadModuleAtAddress(
+ const FileSpec &file, addr_t link_map_addr, addr_t base_addr,
+ bool base_addr_is_offset, bool defer_module_preload) {
if (ModuleSP module_sp = DynamicLoader::LoadModuleAtAddress(
- file, link_map_addr, base_addr, base_addr_is_offset))
+ file, link_map_addr, base_addr, base_addr_is_offset,
+ defer_module_preload))
return module_sp;
// This works around an dynamic linker "bug" on android <= 23, where the
@@ -668,7 +673,7 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadModuleAtAddress(const FileSpec &file,
!(memory_info.GetName().IsEmpty())) {
if (ModuleSP module_sp = DynamicLoader::LoadModuleAtAddress(
FileSpec(memory_info.GetName().GetStringRef()), link_map_addr,
- base_addr, base_addr_is_offset))
+ base_addr, base_addr_is_offset, defer_module_preload))
return module_sp;
}
}
@@ -704,9 +709,11 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
module_names, m_process->GetTarget().GetArchitecture().GetTriple());
auto load_module_fn = [this, &module_list,
- &log](const DYLDRendezvous::SOEntry &so_entry) {
- ModuleSP module_sp = LoadModuleAtAddress(
- so_entry.file_spec, so_entry.link_addr, so_entry.base_addr, true);
+ &log](const DYLDRendezvous::SOEntry &so_entry,
+ bool defer_module_preload) {
+ ModuleSP module_sp =
+ LoadModuleAtAddress(so_entry.file_spec, so_entry.link_addr,
+ so_entry.base_addr, true, defer_module_preload);
if (module_sp.get()) {
LLDB_LOG(log, "LoadAllCurrentModules loading module: {0}",
so_entry.file_spec.GetFilename());
@@ -723,11 +730,14 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
if (m_process->GetTarget().GetParallelModuleLoad()) {
llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
- task_group.async(load_module_fn, *I);
+ task_group.async(load_module_fn, *I, true /* defer_module_preload */);
task_group.wait();
+
+ if (m_process->GetTarget().GetPreloadSymbols())
+ module_list.PreloadSymbols();
} else {
for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
- load_module_fn(*I);
+ load_module_fn(*I, false /* defer_module_preload */);
}
}
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 6efb92673a13c..2d9dba9bca9fa 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -55,10 +55,11 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
// PluginInterface protocol
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
- lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec &file,
- lldb::addr_t link_map_addr,
- lldb::addr_t base_addr,
- bool base_addr_is_offset) override;
+ 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 defer_module_preload = false) override;
void CalculateDynamicSaveCoreRanges(
lldb_private::Process &process,
diff --git a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
index d019415cb67a6..09cde464ccdc8 100644
--- a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp
@@ -67,9 +67,11 @@ ThreadPlanSP DynamicLoaderWasmDYLD::GetStepThroughTrampolinePlan(Thread &thread,
lldb::ModuleSP DynamicLoaderWasmDYLD::LoadModuleAtAddress(
const lldb_private::FileSpec &file, lldb::addr_t link_map_addr,
- lldb::addr_t base_addr, bool base_addr_is_offset) {
+ lldb::addr_t base_addr, bool base_addr_is_offset,
+ bool defer_module_preload) {
if (ModuleSP module_sp = DynamicLoader::LoadModuleAtAddress(
- file, link_map_addr, base_addr, base_addr_is_offset))
+ file, link_map_addr, base_addr, base_addr_is_offset,
+ defer_module_preload))
return module_sp;
if (ModuleSP module_sp = m_process->ReadModuleFromMemory(file, base_addr)) {
diff --git a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
index 5ed855395cca7..9c13bdff0279a 100644
--- a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
@@ -33,10 +33,11 @@ class DynamicLoaderWasmDYLD : public DynamicLoader {
Status CanLoadImage() override { return Status(); }
lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
bool stop) override;
- lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec &file,
- lldb::addr_t link_map_addr,
- lldb::addr_t base_addr,
- bool base_addr_is_offset) override;
+ 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 defer_module_preload = false) override;
/// \}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index d070c3d953d4a..ea685992043e7 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2342,7 +2342,8 @@ bool Target::ReadPointerFromMemory(const Address &addr, Status &error,
}
ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
- bool notify, Status *error_ptr) {
+ bool notify, Status *error_ptr,
+ bool defer_preload) {
ModuleSP module_sp;
Status error;
@@ -2406,7 +2407,7 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
module_spec.GetFileSpec().GetDirectory(), transformed_dir)) {
transformed_spec.GetFileSpec().SetDirectory(transformed_dir);
transformed_spec.GetFileSpec().SetFilename(
- module_spec.GetFileSpec().GetFilename());
+ module_spec.GetFileSpec().GetFilename());
error = ModuleList::GetSharedModule(transformed_spec, module_sp,
&search_paths, &old_modules,
&did_create_module);
@@ -2510,8 +2511,9 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
// Preload symbols outside of any lock, so hopefully we can do this for
// each library in parallel.
- if (GetPreloadSymbols())
+ if (GetPreloadSymbols() && !defer_preload)
module_sp->PreloadSymbols();
+
llvm::SmallVector<ModuleSP, 1> replaced_modules;
for (ModuleSP &old_module_sp : old_modules) {
if (m_images.GetIndexForModule(old_module_sp.get()) !=
>From 51d6bc6b06e4295cca5ba974b20fa541584b7214 Mon Sep 17 00:00:00 2001
From: Tom Yang <toyang at fb.com>
Date: Tue, 4 Nov 2025 23:47:31 -0800
Subject: [PATCH 2/2] formatting fix
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
---
lldb/source/Core/ModuleList.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index c8e917bb0da4a..c8594b4aa803a 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#include "lldb/Core/Debugger.h"
#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"
@@ -16,7 +16,6 @@
#include "lldb/Interpreter/OptionValueFileSpecList.h"
#include "lldb/Interpreter/OptionValueProperties.h"
#include "lldb/Interpreter/Property.h"
-#include "llvm/Support/ThreadPool.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/TypeList.h"
@@ -28,6 +27,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"
More information about the lldb-commits
mailing list