https://github.com/zhyty updated
https://github.com/llvm/llvm-project/pull/130912
>From fbf25b1cd4f6d527944fb85fc4d2d03498755a05 Mon Sep 17 00:00:00 2001
From: Tom Yang <toy...@fb.com>
Date: Mon, 10 Mar 2025 17:58:17 -0700
Subject: [PATCH 1/5] Add option for enabling/disabling parallel load
Add a setting to gate the new parallel dynamic library loading feature in the
next diff. I followed the example of
`lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp` and
`lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp` to
create a scoped setting for the new feature.
NOTE: this setting defaults to FALSE.
Users should be able to enable the new feature as easily as adding a line to
their .lldbinit file, or manually setting it in their session.
```
(lldb) apropos "POSIX dynamic"
No commands found pertaining to 'POSIX dynamic'. Try 'help' to see a complete
list of debugger commands.
The following settings variables may relate to 'POSIX dynamic':
plugin.dynamic-loader.posix-dyld.parallel-module-load -- Enable experimental
loading of modules in parallel for the POSIX dynamic loader.
(lldb) settings show plugin.dynamic-loader.posix-dyld.parallel-module-load
plugin.dynamic-loader.posix-dyld.parallel-module-load (boolean) = false
(lldb) settings set plugin.dynamic-loader.posix-dyld.parallel-module-load true
(lldb) settings show plugin.dynamic-loader.posix-dyld.parallel-module-load
plugin.dynamic-loader.posix-dyld.parallel-module-load (boolean) = true
```
---
.../DynamicLoader/POSIX-DYLD/CMakeLists.txt | 12 +++++
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 49 ++++++++++++++++++-
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 2 +
.../DynamicLoaderPOSIXDYLDProperties.td | 8 +++
4 files changed, 70 insertions(+), 1 deletion(-)
create mode 100644
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
index c1e00b2dd444f..532bfb20ea0f1 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
@@ -1,3 +1,11 @@
+lldb_tablegen(DynamicLoaderPOSIXDYLDProperties.inc -gen-lldb-property-defs
+ SOURCE DynamicLoaderPOSIXDYLDProperties.td
+ TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen)
+
+lldb_tablegen(DynamicLoaderPOSIXDYLDPropertiesEnum.inc
-gen-lldb-property-enum-defs
+ SOURCE DynamicLoaderPOSIXDYLDProperties.td
+ TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
+
add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
DYLDRendezvous.cpp
DynamicLoaderPOSIXDYLD.cpp
@@ -13,3 +21,7 @@ add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
LINK_COMPONENTS
Support
)
+
+add_dependencies(lldbPluginDynamicLoaderPosixDYLD
+ LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen
+ LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 53ba11ac21bd3..c89d16649922d 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -34,9 +34,56 @@ using namespace lldb_private;
LLDB_PLUGIN_DEFINE_ADV(DynamicLoaderPOSIXDYLD, DynamicLoaderPosixDYLD)
+namespace {
+
+#define LLDB_PROPERTIES_dynamicloaderposixdyld
+#include "DynamicLoaderPOSIXDYLDProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_dynamicloaderposixdyld
+#include "DynamicLoaderPOSIXDYLDPropertiesEnum.inc"
+};
+
+class PluginProperties : public Properties {
+public:
+ static llvm::StringRef GetSettingName() {
+ return DynamicLoaderPOSIXDYLD::GetPluginNameStatic();
+ }
+
+ PluginProperties() : Properties() {
+ m_collection_sp =
std::make_shared<OptionValueProperties>(GetSettingName());
+ m_collection_sp->Initialize(g_dynamicloaderposixdyld_properties);
+ }
+
+ ~PluginProperties() override = default;
+
+ bool GetParallelModuleLoad() const {
+ const uint32_t idx = ePropertyParallelModuleLoad;
+ return GetPropertyAtIndexAs<bool>(idx, true);
+ }
+};
+
+} // namespace
+
+static PluginProperties &GetGlobalPluginProperties() {
+ static PluginProperties g_settings;
+ return g_settings;
+}
+
void DynamicLoaderPOSIXDYLD::Initialize() {
PluginManager::RegisterPlugin(GetPluginNameStatic(),
- GetPluginDescriptionStatic(), CreateInstance);
+ GetPluginDescriptionStatic(), CreateInstance,
+ &DebuggerInitialize);
+}
+
+void DynamicLoaderPOSIXDYLD::DebuggerInitialize(Debugger &debugger) {
+ if (!PluginManager::GetSettingForDynamicLoaderPlugin(
+ debugger, PluginProperties::GetSettingName())) {
+ const bool is_global_setting = true;
+ PluginManager::CreateSettingForDynamicLoaderPlugin(
+ debugger, GetGlobalPluginProperties().GetValueProperties(),
+ "Properties for the POSIX dynamic loader plug-in.", is_global_setting);
+ }
}
void DynamicLoaderPOSIXDYLD::Terminate() {}
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index bde334aaca40b..5bb1e00dd7df0 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -28,6 +28,8 @@ class DynamicLoaderPOSIXDYLD : public
lldb_private::DynamicLoader {
static void Initialize();
+ static void DebuggerInitialize(lldb_private::Debugger &debugger);
+
static void Terminate();
static llvm::StringRef GetPluginNameStatic() { return "posix-dyld"; }
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
new file mode 100644
index 0000000000000..68e3448196cec
--- /dev/null
+++
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
@@ -0,0 +1,8 @@
+
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "dynamicloaderposixdyld" in {
+ def ParallelModuleLoad: Property<"parallel-module-load", "Boolean">,
+ DefaultFalse,
+ Desc<"Enable experimental loading of modules in parallel for the POSIX
dynamic loader.">;
+}
>From d0f9fadf068e9cc4e5146ef7be46b80bf8584f64 Mon Sep 17 00:00:00 2001
From: Tom Yang <toy...@fb.com>
Date: Tue, 11 Mar 2025 10:17:23 -0700
Subject: [PATCH 2/5] parallelize POSIX dyld RefreshModules
This diff parallelizes `DynamicLoaderPOSIXDYLD::RefreshModules`, which speeds
up module loading on Linux. The major benefit of this is we can speed up symbol
table indexing and parsing, which is the biggest bottleneck for targets which
dynamically link many shared libraries.
This speedup is only noticeable when **preloading** symbols. This is when
`target.preload-symbols` is `true`, which is the default Meta. The symbol
preload option tells the debugger to fully load all of the symbol tables when
modules are loaded, as opposed to lazily loading when symbols are requested.
Initially, I discovered the specific bottleneck by using the Linux `perf` tool.
I saw that ~93% of samples were in `RefreshModules`, and mainly in
`LoadModuleAtAddress` and `PreloadSymbols`.
`LoadModuleAtAddress` appears independent and parallelize-able at first. The
main issue is `DynamicLoaderPOSIXDYLD` maintains a map of loaded modules to
their link addresses via `m_loaded_modules`. Modifying and reading to this map
isn't thread-safe, so this diff also includes accessor methods that protect the
map in the multithreaded context. Luckily, the critical section of modifying or
reading from the map isn't super costly, so the contention doesn't appear to
negatively impact performance.
I tested with some larger projects with up to 15000 modules, and
found significant performance improvements. Typically, I was seeing 2-3X
launch speed increases, where "launch speed" is starting the binary and
reaching `main`.
I manually ran `ninja check-lldb` several times, and compared with the
baseline. At this point, we're not seeing any new failures or new unresolved
tests.
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 121 +++++++++++++-----
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 17 ++-
2 files changed, 101 insertions(+), 37 deletions(-)
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index c89d16649922d..b1e2ea974c77d 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -10,6 +10,7 @@
#include "DynamicLoaderPOSIXDYLD.h"
#include "lldb/Breakpoint/BreakpointLocation.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/PluginManager.h"
@@ -25,6 +26,7 @@
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/ProcessInfo.h"
+#include "llvm/Support/ThreadPool.h"
#include <memory>
#include <optional>
@@ -231,16 +233,37 @@ void DynamicLoaderPOSIXDYLD::DidLaunch() {
Status DynamicLoaderPOSIXDYLD::CanLoadImage() { return Status(); }
+void DynamicLoaderPOSIXDYLD::SetLoadedModule(const ModuleSP &module_sp,
+ addr_t link_map_addr) {
+ std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ m_loaded_modules[module_sp] = link_map_addr;
+}
+
+void DynamicLoaderPOSIXDYLD::UnloadModule(const ModuleSP &module_sp) {
+ std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ m_loaded_modules.erase(module_sp);
+}
+
+std::optional<lldb::addr_t>
+DynamicLoaderPOSIXDYLD::GetLoadedModuleLinkAddr(const ModuleSP &module_sp) {
+ std::shared_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ auto it = m_loaded_modules.find(module_sp);
+ if (it != m_loaded_modules.end())
+ return it->second;
+ return std::nullopt;
+}
+
void DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module,
addr_t link_map_addr,
addr_t base_addr,
bool base_addr_is_offset) {
- m_loaded_modules[module] = link_map_addr;
+ SetLoadedModule(module, link_map_addr);
+
UpdateLoadedSectionsCommon(module, base_addr, base_addr_is_offset);
}
void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module) {
- m_loaded_modules.erase(module);
+ UnloadModule(module);
UnloadSectionsCommon(module);
}
@@ -448,7 +471,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
// The rendezvous class doesn't enumerate the main module, so track that
// ourselves here.
ModuleSP executable = GetTargetExecutable();
- m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
+ SetLoadedModule(executable, m_rendezvous.GetLinkMapAddress());
DYLDRendezvous::iterator I;
DYLDRendezvous::iterator E;
@@ -470,34 +493,66 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
E = m_rendezvous.end();
m_initial_modules_added = true;
}
- for (; I != E; ++I) {
- // 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 removed. That
- // information is required for placing breakpoints on Arm/Thumb systems.
- if ((m_interpreter_module.lock() != nullptr) &&
- (I->base_addr == m_interpreter_base))
- continue;
-
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
- if (!module_sp.get())
- continue;
-
- if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
- &m_process->GetTarget()) == m_interpreter_base) {
- ModuleSP interpreter_sp = m_interpreter_module.lock();
- if (m_interpreter_module.lock() == nullptr) {
- m_interpreter_module = module_sp;
- } else if (module_sp == interpreter_sp) {
- // Module already loaded.
- continue;
- }
- }
- loaded_modules.AppendIfNeeded(module_sp);
- new_modules.Append(module_sp);
+ std::mutex interpreter_module_mutex;
+ // We should be able to take SOEntry as reference since the data
+ // 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) {
+ // 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
+ // removed. That information is required for placing breakpoints on
+ // Arm/Thumb systems.
+ {
+ // `m_interpreter_module` may be modified by another thread at the
+ // same time, so we guard the access here.
+ std::lock_guard<std::mutex> lock(interpreter_module_mutex);
+ if ((m_interpreter_module.lock() != nullptr) &&
+ (so_entry.base_addr == m_interpreter_base))
+ return;
+ }
+
+ ModuleSP module_sp = LoadModuleAtAddress(
+ so_entry.file_spec, so_entry.link_addr, so_entry.base_addr,
true);
+ if (!module_sp.get())
+ return;
+
+ {
+ // `m_interpreter_module` may be modified by another thread at the
+ // same time, so we guard the access here.
+ std::lock_guard<std::mutex> lock(interpreter_module_mutex);
+ // Set the interpreter module, if this is the interpreter.
+ if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
+ &m_process->GetTarget()) == m_interpreter_base) {
+ ModuleSP interpreter_sp = m_interpreter_module.lock();
+ if (m_interpreter_module.lock() == nullptr) {
+ m_interpreter_module = module_sp;
+ } else if (module_sp == interpreter_sp) {
+ // Module already loaded.
+ return;
+ }
+ }
+ }
+
+ loaded_modules.AppendIfNeeded(module_sp);
+ new_modules.Append(module_sp);
+ };
+
+ // Loading modules in parallel tends to be faster, but is still unstable.
+ // Once it's stable, we can remove this setting and remove the serial
+ // approach.
+ if (GetGlobalPluginProperties().GetParallelModuleLoad()) {
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (; I != E; ++I)
+ task_group.async(load_module_fn, *I);
+ task_group.wait();
+ } else {
+ for (; I != E; ++I)
+ load_module_fn(*I);
}
+
m_process->GetTarget().ModulesDidLoad(new_modules);
}
@@ -683,7 +738,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
// The rendezvous class doesn't enumerate the main module, so track that
// ourselves here.
ModuleSP executable = GetTargetExecutable();
- m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
+ SetLoadedModule(executable, m_rendezvous.GetLinkMapAddress());
std::vector<FileSpec> module_names;
for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
@@ -775,15 +830,15 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const
lldb::ModuleSP module_sp,
const lldb::ThreadSP thread,
lldb::addr_t tls_file_addr) {
Log *log = GetLog(LLDBLog::DynamicLoader);
- auto it = m_loaded_modules.find(module_sp);
- if (it == m_loaded_modules.end()) {
+ std::optional<addr_t> link_map_addr_opt = GetLoadedModuleLinkAddr(module_sp);
+ if (!link_map_addr_opt.has_value()) {
LLDB_LOGF(
log, "GetThreadLocalData error: module(%s) not found in loaded
modules",
module_sp->GetObjectName().AsCString());
return LLDB_INVALID_ADDRESS;
}
- addr_t link_map = it->second;
+ addr_t link_map = link_map_addr_opt.value();
if (link_map == LLDB_INVALID_ADDRESS || link_map == 0) {
LLDB_LOGF(log,
"GetThreadLocalData error: invalid link map address=0x%" PRIx64,
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 5bb1e00dd7df0..183a157654ba2 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -95,10 +95,6 @@ class DynamicLoaderPOSIXDYLD : public
lldb_private::DynamicLoader {
/// Contains the pointer to the interpret module, if loaded.
std::weak_ptr<lldb_private::Module> m_interpreter_module;
- /// Loaded module list. (link map for each module)
- std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
- m_loaded_modules;
-
/// Returns true if the process is for a core file.
bool IsCoreFile() const;
@@ -182,6 +178,19 @@ class DynamicLoaderPOSIXDYLD : public
lldb_private::DynamicLoader {
DynamicLoaderPOSIXDYLD(const DynamicLoaderPOSIXDYLD &) = delete;
const DynamicLoaderPOSIXDYLD &
operator=(const DynamicLoaderPOSIXDYLD &) = delete;
+
+ /// Loaded module list. (link map for each module)
+ /// This may be accessed in a multi-threaded context. Use the accessor
methods
+ /// to access `m_loaded_modules` safely.
+ std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
+ m_loaded_modules;
+ std::shared_mutex m_loaded_modules_rw_mutex;
+
+ void SetLoadedModule(const lldb::ModuleSP &module_sp,
+ lldb::addr_t link_map_addr);
+ void UnloadModule(const lldb::ModuleSP &module_sp);
+ std::optional<lldb::addr_t>
+ GetLoadedModuleLinkAddr(const lldb::ModuleSP &module_sp);
};
#endif // LLDB_SOURCE_PLUGINS_DYNAMICLOADER_POSIX_DYLD_DYNAMICLOADERPOSIXDYLD_H
>From 4a4a3550cd5bbb30beb9b86fc301da09e8266303 Mon Sep 17 00:00:00 2001
From: Tom Yang <toy...@fb.com>
Date: Tue, 11 Mar 2025 10:35:02 -0700
Subject: [PATCH 3/5] parallelize dyld for attach scenario
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 22 ++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index b1e2ea974c77d..f6f6057f830fe 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -746,19 +746,31 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
m_process->PrefetchModuleSpecs(
module_names, m_process->GetTarget().GetArchitecture().GetTriple());
- for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
- ModuleSP module_sp =
- LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
+ 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);
if (module_sp.get()) {
LLDB_LOG(log, "LoadAllCurrentModules loading module: {0}",
- I->file_spec.GetFilename());
+ so_entry.file_spec.GetFilename());
module_list.Append(module_sp);
} else {
Log *log = GetLog(LLDBLog::DynamicLoader);
LLDB_LOGF(
log,
"DynamicLoaderPOSIXDYLD::%s failed loading module %s at 0x%" PRIx64,
- __FUNCTION__, I->file_spec.GetPath().c_str(), I->base_addr);
+ __FUNCTION__, so_entry.file_spec.GetPath().c_str(),
+ so_entry.base_addr);
+ }
+ };
+ if (GetGlobalPluginProperties().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.wait();
+ } else {
+ for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I) {
+ load_module_fn(*I);
}
}
>From 6585ce1b3e4c625d4cd46d63a5cb1d7b167cd1b7 Mon Sep 17 00:00:00 2001
From: Tom Yang <toy...@fb.com>
Date: Wed, 19 Mar 2025 11:18:24 -0700
Subject: [PATCH 4/5] move
plugin.dynamic-loader.posix-dyld.parallel-module-load to
target.parallel-module-load
---
lldb/include/lldb/Target/Target.h | 2 +
.../DynamicLoader/POSIX-DYLD/CMakeLists.txt | 12 ----
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 56 +------------------
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 2 -
.../DynamicLoaderPOSIXDYLDProperties.td | 8 ---
lldb/source/Target/Target.cpp | 6 ++
lldb/source/Target/TargetProperties.td | 3 +
7 files changed, 14 insertions(+), 75 deletions(-)
delete mode 100644
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 98273fb7e1c97..01a928edc384d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -118,6 +118,8 @@ class TargetProperties : public Properties {
llvm::StringRef GetLaunchWorkingDirectory() const;
+ bool GetParallelModuleLoad() const;
+
const char *GetDisassemblyFlavor() const;
const char *GetDisassemblyCPU() const;
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
index 532bfb20ea0f1..c1e00b2dd444f 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/CMakeLists.txt
@@ -1,11 +1,3 @@
-lldb_tablegen(DynamicLoaderPOSIXDYLDProperties.inc -gen-lldb-property-defs
- SOURCE DynamicLoaderPOSIXDYLDProperties.td
- TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen)
-
-lldb_tablegen(DynamicLoaderPOSIXDYLDPropertiesEnum.inc
-gen-lldb-property-enum-defs
- SOURCE DynamicLoaderPOSIXDYLDProperties.td
- TARGET LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
-
add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
DYLDRendezvous.cpp
DynamicLoaderPOSIXDYLD.cpp
@@ -21,7 +13,3 @@ add_lldb_library(lldbPluginDynamicLoaderPosixDYLD PLUGIN
LINK_COMPONENTS
Support
)
-
-add_dependencies(lldbPluginDynamicLoaderPosixDYLD
- LLDBPluginDynamicLoaderPOSIXDYLDPropertiesGen
- LLDBPluginDynamicLoaderPOSIXDYLDPropertiesEnumGen)
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index f6f6057f830fe..8c7f523935a92 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -36,56 +36,9 @@ using namespace lldb_private;
LLDB_PLUGIN_DEFINE_ADV(DynamicLoaderPOSIXDYLD, DynamicLoaderPosixDYLD)
-namespace {
-
-#define LLDB_PROPERTIES_dynamicloaderposixdyld
-#include "DynamicLoaderPOSIXDYLDProperties.inc"
-
-enum {
-#define LLDB_PROPERTIES_dynamicloaderposixdyld
-#include "DynamicLoaderPOSIXDYLDPropertiesEnum.inc"
-};
-
-class PluginProperties : public Properties {
-public:
- static llvm::StringRef GetSettingName() {
- return DynamicLoaderPOSIXDYLD::GetPluginNameStatic();
- }
-
- PluginProperties() : Properties() {
- m_collection_sp =
std::make_shared<OptionValueProperties>(GetSettingName());
- m_collection_sp->Initialize(g_dynamicloaderposixdyld_properties);
- }
-
- ~PluginProperties() override = default;
-
- bool GetParallelModuleLoad() const {
- const uint32_t idx = ePropertyParallelModuleLoad;
- return GetPropertyAtIndexAs<bool>(idx, true);
- }
-};
-
-} // namespace
-
-static PluginProperties &GetGlobalPluginProperties() {
- static PluginProperties g_settings;
- return g_settings;
-}
-
void DynamicLoaderPOSIXDYLD::Initialize() {
PluginManager::RegisterPlugin(GetPluginNameStatic(),
- GetPluginDescriptionStatic(), CreateInstance,
- &DebuggerInitialize);
-}
-
-void DynamicLoaderPOSIXDYLD::DebuggerInitialize(Debugger &debugger) {
- if (!PluginManager::GetSettingForDynamicLoaderPlugin(
- debugger, PluginProperties::GetSettingName())) {
- const bool is_global_setting = true;
- PluginManager::CreateSettingForDynamicLoaderPlugin(
- debugger, GetGlobalPluginProperties().GetValueProperties(),
- "Properties for the POSIX dynamic loader plug-in.", is_global_setting);
- }
+ GetPluginDescriptionStatic(), CreateInstance);
}
void DynamicLoaderPOSIXDYLD::Terminate() {}
@@ -540,10 +493,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
new_modules.Append(module_sp);
};
- // Loading modules in parallel tends to be faster, but is still unstable.
- // Once it's stable, we can remove this setting and remove the serial
- // approach.
- if (GetGlobalPluginProperties().GetParallelModuleLoad()) {
+ if (m_process->GetTarget().GetParallelModuleLoad()) {
llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
for (; I != E; ++I)
task_group.async(load_module_fn, *I);
@@ -763,7 +713,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
so_entry.base_addr);
}
};
- if (GetGlobalPluginProperties().GetParallelModuleLoad()) {
+ 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);
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 183a157654ba2..a41cbfac86349 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -28,8 +28,6 @@ class DynamicLoaderPOSIXDYLD : public
lldb_private::DynamicLoader {
static void Initialize();
- static void DebuggerInitialize(lldb_private::Debugger &debugger);
-
static void Terminate();
static llvm::StringRef GetPluginNameStatic() { return "posix-dyld"; }
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
deleted file mode 100644
index 68e3448196cec..0000000000000
---
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLDProperties.td
+++ /dev/null
@@ -1,8 +0,0 @@
-
-include "../../../../include/lldb/Core/PropertiesBase.td"
-
-let Definition = "dynamicloaderposixdyld" in {
- def ParallelModuleLoad: Property<"parallel-module-load", "Boolean">,
- DefaultFalse,
- Desc<"Enable experimental loading of modules in parallel for the POSIX
dynamic loader.">;
-}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 550424720e095..da3dabb414b8d 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4502,6 +4502,12 @@ llvm::StringRef
TargetProperties::GetLaunchWorkingDirectory() const {
idx, g_target_properties[idx].default_cstr_value);
}
+bool TargetProperties::GetParallelModuleLoad() const {
+ const uint32_t idx = ePropertyParallelModuleLoad;
+ return GetPropertyAtIndexAs<bool>(
+ idx, g_target_properties[idx].default_uint_value != 0);
+}
+
const char *TargetProperties::GetDisassemblyFlavor() const {
const uint32_t idx = ePropertyDisassemblyFlavor;
const char *return_value;
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index 38a345dfd8849..3940ac00a2bd9 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -217,6 +217,9 @@ let Definition = "target" in {
"launched. If you change this setting, the new value will only apply
to "
"subsequent launches. Commands that take an explicit working
directory "
"will override this setting.">;
+ def ParallelModuleLoad: Property<"parallel-module-load", "Boolean">,
+ DefaultTrue,
+ Desc<"Enable loading of modules in parallel for the dynamic loader.">;
}
let Definition = "process_experimental" in {
>From 38a017e9e80ebf274b6af4bfe37e7abcbd5ebf2d Mon Sep 17 00:00:00 2001
From: Tom Yang <toy...@fb.com>
Date: Wed, 19 Mar 2025 11:32:39 -0700
Subject: [PATCH 5/5] switch usage of std shared_mutex with rwmutex, which is
more portable
---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 13 ++++++++++---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 2 +-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 8c7f523935a92..326b6910b5267 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -188,18 +188,18 @@ Status DynamicLoaderPOSIXDYLD::CanLoadImage() { return
Status(); }
void DynamicLoaderPOSIXDYLD::SetLoadedModule(const ModuleSP &module_sp,
addr_t link_map_addr) {
- std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ llvm::sys::ScopedWriter lock(m_loaded_modules_rw_mutex);
m_loaded_modules[module_sp] = link_map_addr;
}
void DynamicLoaderPOSIXDYLD::UnloadModule(const ModuleSP &module_sp) {
- std::unique_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ llvm::sys::ScopedWriter lock(m_loaded_modules_rw_mutex);
m_loaded_modules.erase(module_sp);
}
std::optional<lldb::addr_t>
DynamicLoaderPOSIXDYLD::GetLoadedModuleLinkAddr(const ModuleSP &module_sp) {
- std::shared_lock<std::shared_mutex> lock(m_loaded_modules_rw_mutex);
+ llvm::sys::ScopedReader lock(m_loaded_modules_rw_mutex);
auto it = m_loaded_modules.find(module_sp);
if (it != m_loaded_modules.end())
return it->second;
@@ -447,6 +447,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
m_initial_modules_added = true;
}
+ // Synchronize reading and writing of `m_interpreter_module`.
std::mutex interpreter_module_mutex;
// We should be able to take SOEntry as reference since the data
// exists for the duration of this call in `m_rendezvous`.
@@ -489,6 +490,12 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
}
}
+ // Note: in a multi-threaded environment, these module lists may be
+ // appended to out-of-order. This is fine, since there's no
+ // expectation for `loaded_modules` or `new_modules` to be in any
+ // particular order, and appending to each module list is
thread-safe.
+ // Also, `new_modules` is only used for the `ModulesDidLoad` call at
+ // the end of this function.
loaded_modules.AppendIfNeeded(module_sp);
new_modules.Append(module_sp);
};
diff --git
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index a41cbfac86349..6efb92673a13c 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -182,7 +182,7 @@ class DynamicLoaderPOSIXDYLD : public
lldb_private::DynamicLoader {
/// to access `m_loaded_modules` safely.
std::map<lldb::ModuleWP, lldb::addr_t, std::owner_less<lldb::ModuleWP>>
m_loaded_modules;
- std::shared_mutex m_loaded_modules_rw_mutex;
+ llvm::sys::RWMutex m_loaded_modules_rw_mutex;
void SetLoadedModule(const lldb::ModuleSP &module_sp,
lldb::addr_t link_map_addr);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
