From f91124a55b365284ad0e8fd8169bfa3b041bbbd8 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 3 Apr 2026 08:03:11 +0100 Subject: [PATCH] [lldb][Module] Only call LoadScriptingResourceInTarget via ModuleList (#190136) 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 the warnings. To avoid future accidental uses of `Module::LoadScriptingResourceInTarget` I moved the API into `ModuleList` and made it `private`. --- lldb/include/lldb/Core/Module.h | 2 - lldb/include/lldb/Core/ModuleList.h | 5 ++ lldb/source/Commands/CommandObjectTarget.cpp | 14 +--- lldb/source/Core/Module.cpp | 87 -------------------- lldb/source/Core/ModuleList.cpp | 85 ++++++++++++++++++- lldb/source/Target/Target.cpp | 19 ++--- 6 files changed, 98 insertions(+), 114 deletions(-) diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index cfe006629ae9..d33d1b1938ef 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/lldb/include/lldb/Core/Module.h @@ -510,8 +510,6 @@ public: /// \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 e147eeac6195..bea88a6cf1f5 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/lldb/include/lldb/Core/ModuleList.h @@ -559,6 +559,11 @@ protected: /// 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 ModuleIterable; diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index dccb7256b1cb..288655f79f44 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -4400,16 +4400,10 @@ protected: // 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 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 808c8a347e9b..439d53e4be4d 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 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 b54d787de351..67ebe58cdb87 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/lldb/source/Core/ModuleList.cpp @@ -1332,6 +1332,89 @@ 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 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 &errors, bool continue_on_error) { @@ -1347,7 +1430,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 2a3832225f9b..896425e42464 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 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); }