[Lldb-commits] [lldb] [lldb][Module] Only call LoadScriptingResourceInTarget via ModuleList (PR #190136)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 2 02:11:49 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
<details>
<summary>Changes</summary>
This patch is motivated by https://github.com/llvm/llvm-project/pull/189943, where we would like to print the "these module scripts weren't loaded" warning for *all* modules batched together. I.e., we want to print the warning *after* all the script loading attempts, not from within each attempt.
To do so we want to hoist the `ReportWarning` calls in `Module::LoadScriptingResourceInTarget` out into the callsites. But if we do that, the callers have to remember to print the warnings. To avoid this, we redirect all callsites to use `ModuleList::LoadScriptingResourceInTarget`, which will be responsible for printing any warnings manually.
To avoid future accidental uses of `Module::LoadScriptingResourceInTarget` I moved the API into `ModuleList` and made it `private`.
---
Full diff: https://github.com/llvm/llvm-project/pull/190136.diff
6 Files Affected:
- (modified) lldb/include/lldb/Core/Module.h (-2)
- (modified) lldb/include/lldb/Core/ModuleList.h (+5)
- (modified) lldb/source/Commands/CommandObjectTarget.cpp (+4-10)
- (modified) lldb/source/Core/Module.cpp (-87)
- (modified) lldb/source/Core/ModuleList.cpp (+85-1)
- (modified) lldb/source/Target/Target.cpp (+5-14)
``````````diff
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index cfe006629ae99..d33d1b1938ef4 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -510,8 +510,6 @@ class Module : public std::enable_shared_from_this<Module>,
/// \b true if it is, \b false otherwise.
bool IsLoadedInTarget(Target *target);
- bool LoadScriptingResourceInTarget(Target *target, Status &error);
-
/// Get the number of compile units for this module.
///
/// \return
diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index e147eeac61952..bea88a6cf1f51 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -559,6 +559,11 @@ class ModuleList {
/// An orphaned module that lives only in the ModuleList has a count of 1.
static constexpr long kUseCountModuleListOrphaned = 1;
+private:
+ static bool LoadScriptingResourceInTargetForModule(Module &module,
+ Target &target,
+ Status &error);
+
public:
typedef LockingAdaptedIterable<std::recursive_mutex, collection>
ModuleIterable;
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index dccb7256b1cb7..288655f79f442 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4400,16 +4400,10 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
// Make sure we load any scripting resources that may be embedded
// in the debug info files in case the platform supports that.
- Status error;
- module_sp->LoadScriptingResourceInTarget(target, error);
- if (error.Fail() && error.AsCString())
- result.AppendWarningWithFormat(
- "unable to load scripting data for module %s - error "
- "reported was %s",
- module_sp->GetFileSpec()
- .GetFileNameStrippingExtension()
- .GetCString(),
- error.AsCString());
+ std::list<Status> errors;
+ module_list.LoadScriptingResourcesInTarget(target, errors);
+ for (const auto &err : errors)
+ result.AppendWarning(err.AsCString());
flush = true;
result.SetStatus(eReturnStatusSuccessFinishResult);
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 808c8a347e9b2..439d53e4be4d2 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1419,93 +1419,6 @@ bool Module::IsLoadedInTarget(Target *target) {
return false;
}
-static bool LoadScriptingModule(const FileSpec &scripting_fspec,
- ScriptInterpreter &script_interpreter,
- Target &target, Status &error) {
- assert(scripting_fspec);
-
- StreamString scripting_stream;
- scripting_fspec.Dump(scripting_stream.AsRawOstream());
- LoadScriptOptions options;
- return script_interpreter.LoadScriptingModule(
- scripting_stream.GetData(), options, error,
- /*module_sp*/ nullptr, /*extra_path*/ {}, target.shared_from_this());
-}
-
-bool Module::LoadScriptingResourceInTarget(Target *target, Status &error) {
- Log *log = GetLog(LLDBLog::Modules);
-
- if (!target) {
- error = Status::FromErrorString("invalid destination Target");
- return false;
- }
-
- Debugger &debugger = target->GetDebugger();
- const ScriptLanguage script_language = debugger.GetScriptLanguage();
- if (script_language == eScriptLanguageNone)
- return true;
-
- ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
- if (!script_interpreter) {
- error = Status::FromErrorString("invalid ScriptInterpreter");
- return false;
- }
-
- PlatformSP platform_sp(target->GetPlatform());
-
- if (!platform_sp) {
- error = Status::FromErrorString("invalid Platform");
- return false;
- }
-
- StreamString feedback_stream;
- llvm::SmallDenseMap<FileSpec, LoadScriptFromSymFile> file_specs =
- platform_sp->LocateExecutableScriptingResources(target, *this,
- feedback_stream);
-
- if (!feedback_stream.Empty())
- debugger.ReportWarning(feedback_stream.GetString().str(), debugger.GetID());
-
- for (const auto &[scripting_fspec, load_style] : file_specs) {
- if (load_style == eLoadScriptFromSymFileFalse)
- continue;
-
- if (!FileSystem::Instance().Exists(scripting_fspec))
- continue;
-
- if (load_style == eLoadScriptFromSymFileWarn) {
- // clang-format off
- debugger.ReportWarning(
- llvm::formatv(
-R"('{0}' contains a debug script. To run this script in this debug session:
-
- command script import "{1}"
-
-To run all discovered debug scripts in this session:
-
- settings set target.load-script-from-symbol-file true
-)",
- GetFileSpec().GetFileNameStrippingExtension(),
- scripting_fspec.GetPath()),
- debugger.GetID());
- // clang-format on
-
- return false;
- }
-
- LLDB_LOG(log, "Auto-loading {0}", scripting_fspec.GetPath());
-
- if (!LoadScriptingModule(scripting_fspec, *script_interpreter, *target,
- error)) {
- LLDB_LOG(log, "Failed to load '{0}'. Remaining scripts won't be loaded.",
- scripting_fspec.GetPath());
- return false;
- }
- }
-
- return true;
-}
-
bool Module::SetArchitecture(const ArchSpec &new_arch) {
if (!m_arch.IsValid()) {
m_arch = new_arch;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index b54d787de3512..b8980f6a02c50 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -1332,6 +1332,90 @@ bool ModuleList::RemoveSharedModuleIfOrphaned(const ModuleWP module_wp) {
return GetSharedModuleList().RemoveIfOrphaned(module_wp);
}
+static bool LoadScriptingModule(const FileSpec &scripting_fspec,
+ ScriptInterpreter &script_interpreter,
+ Target &target, Status &error) {
+ assert(scripting_fspec);
+
+ StreamString scripting_stream;
+ scripting_fspec.Dump(scripting_stream.AsRawOstream());
+ LoadScriptOptions options;
+ return script_interpreter.LoadScriptingModule(
+ scripting_stream.GetData(), options, error,
+ /*module_sp*/ nullptr, /*extra_path*/ {}, target.shared_from_this());
+}
+
+bool ModuleList::LoadScriptingResourceInTargetForModule(Module &module,
+ Target &target,
+ Status &error) {
+ Log *log = GetLog(LLDBLog::Modules);
+
+ Debugger &debugger = target.GetDebugger();
+ const ScriptLanguage script_language = debugger.GetScriptLanguage();
+ if (script_language == eScriptLanguageNone)
+ return true;
+
+ ScriptInterpreter *script_interpreter = debugger.GetScriptInterpreter();
+ if (!script_interpreter) {
+ error = Status::FromErrorString("invalid ScriptInterpreter");
+ return false;
+ }
+
+ PlatformSP platform_sp(target.GetPlatform());
+
+ if (!platform_sp) {
+ error = Status::FromErrorString("invalid Platform");
+ return false;
+ }
+
+ StreamString feedback_stream;
+ llvm::SmallDenseMap<FileSpec, LoadScriptFromSymFile> file_specs =
+ platform_sp->LocateExecutableScriptingResources(&target, module,
+ feedback_stream);
+
+ if (!feedback_stream.Empty())
+ debugger.ReportWarning(feedback_stream.GetString().str(), debugger.GetID());
+
+ for (const auto &[scripting_fspec, load_style] : file_specs) {
+ if (load_style == eLoadScriptFromSymFileFalse)
+ continue;
+
+ if (!FileSystem::Instance().Exists(scripting_fspec))
+ continue;
+
+ if (load_style == eLoadScriptFromSymFileWarn) {
+ // clang-format off
+ debugger.ReportWarning(
+ llvm::formatv(
+R"('{0}' contains a debug script. To run this script in this debug session:
+
+ command script import "{1}"
+
+To run all discovered debug scripts in this session:
+
+ settings set target.load-script-from-symbol-file true
+)",
+ module.GetFileSpec().GetFileNameStrippingExtension(),
+ scripting_fspec.GetPath()),
+ debugger.GetID());
+ // clang-format on
+
+ return false;
+ }
+
+ LLDB_LOG(log, "Auto-loading {0}", scripting_fspec.GetPath());
+
+ if (!LoadScriptingModule(scripting_fspec, *script_interpreter, target,
+ error)) {
+ LLDB_LOG(log, "Failed to load '{0}'. Remaining scripts won't be loaded.",
+ scripting_fspec.GetPath());
+ return false;
+ }
+ }
+
+ return true;
+}
+
bool ModuleList::LoadScriptingResourcesInTarget(Target *target,
std::list<Status> &errors,
bool continue_on_error) {
@@ -1347,7 +1431,7 @@ bool ModuleList::LoadScriptingResourcesInTarget(Target *target,
for (auto module : tmp_module_list.ModulesNoLocking()) {
if (module) {
Status error;
- if (!module->LoadScriptingResourceInTarget(target, error)) {
+ if (!LoadScriptingResourceInTargetForModule(*module, *target, error)) {
if (error.Fail() && error.AsCString()) {
error = Status::FromErrorStringWithFormat(
"unable to load scripting data for "
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 2a3832225f9b7..896425e42464d 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1542,19 +1542,6 @@ Module *Target::GetExecutableModulePointer() {
return GetExecutableModule().get();
}
-static void LoadScriptingResourceForModule(const ModuleSP &module_sp,
- Target *target) {
- Status error;
- if (module_sp && !module_sp->LoadScriptingResourceInTarget(target, error)) {
- if (error.AsCString())
- target->GetDebugger().GetAsyncErrorStream()->Printf(
- "unable to load scripting data for module %s - error reported was "
- "%s\n",
- module_sp->GetFileSpec().GetFileNameStrippingExtension().GetCString(),
- error.AsCString());
- }
-}
-
void Target::ClearModules(bool delete_locations) {
ModulesDidUnload(m_images, delete_locations);
m_section_load_history.Clear();
@@ -1857,9 +1844,13 @@ void Target::ModulesDidLoad(ModuleList &module_list) {
const size_t num_images = module_list.GetSize();
if (m_valid && num_images) {
+ std::list<Status> errors;
+ module_list.LoadScriptingResourcesInTarget(this, errors);
+ for (const auto &err : errors)
+ GetDebugger().GetAsyncErrorStream()->PutCString(err.AsCString());
+
for (size_t idx = 0; idx < num_images; ++idx) {
ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
- LoadScriptingResourceForModule(module_sp, this);
LoadTypeSummariesForModule(module_sp);
LoadFormattersForModule(module_sp);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/190136
More information about the lldb-commits
mailing list