llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Bar Soloveychik (barsolo2000) <details> <summary>Changes</summary> Adds a command that replaces a module in the target with a different file on disk: ``` target modules replace [--old-path <path>] [--force] <path> ``` The usual case is a core file whose binary could not be found, leaving a placeholder module with no real contents. Once the file is located this swaps it in at the address the placeholder had, so backtraces symbolicate and breakpoints resolve into it. The module to replace is worked out from the new file's UUID, or from its name when it has no UUID, and `--old-path` names it explicitly when neither is enough. A UUID mismatch is an error unless we add `--force`. It removes the old module through the regular unload path, which unloads its sections and deletes the breakpoint locations, then it adds the new module at the same load address, and notifies the dynamic loader. The old module is destroyed rather than having its object file swapped out underneath it. Replacing the object file of a live Module leaves stack frames, symbol blocks and breakpoint locations pointing into freed memory. --- Patch is 50.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/214576.diff 17 Files Affected: - (modified) lldb/include/lldb/Target/DynamicLoader.h (+25) - (modified) lldb/include/lldb/Target/Target.h (+23) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+233) - (modified) lldb/source/Commands/Options.td (+13) - (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+40-8) - (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h (+18-4) - (modified) lldb/source/Target/Target.cpp (+118) - (added) lldb/test/API/commands/target/modules/replace/Makefile (+18) - (added) lldb/test/API/commands/target/modules/replace/TestTargetModulesReplace.py (+404) - (added) lldb/test/API/commands/target/modules/replace/hidden/v.cpp (+11) - (added) lldb/test/API/commands/target/modules/replace/main.cpp (+24) - (added) lldb/test/API/commands/target/modules/replace/other_main.cpp (+4) - (added) lldb/test/API/commands/target/modules/replace/placeholder-no-uuid.yaml (+17) - (added) lldb/test/API/commands/target/modules/replace/replacement.yaml (+20) - (added) lldb/test/API/commands/target/modules/replace/unplaceable.yaml (+13) - (added) lldb/test/API/commands/target/modules/replace/v.cpp (+10) - (modified) lldb/test/API/functionalities/completion/TestCompletion.py (+9) ``````````diff diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 62307a9f9c2a2..7622fb0bb57ab 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -207,6 +207,31 @@ class DynamicLoader : public PluginInterface { return LLDB_INVALID_ADDRESS; } + /// Inform the dynamic loader that Target::ReplaceModule() has swapped one + /// module for another. + /// + /// If this function returns a Status that is a success, the dynamic loader + /// will have removed all old section mappings for \a old_module_sp, and + /// loaded all sections for \a new_module_sp. If an error is returned from + /// this function, the target will unload all sections from \a old_module_sp + /// and not do anything to load the \a new_module_sp's sections. + /// + /// \param[in] old_module_sp + /// The module that was removed from the target. + /// + /// \param[in] new_module_sp + /// The module that took its place. + /// + /// \return + /// An error if this loader cannot place the replacement correctly. + virtual Status ReplaceModule(const lldb::ModuleSP &old_module_sp, + const lldb::ModuleSP &new_module_sp) { + return Status::FromErrorStringWithFormatv( + "The {0} dynamic loader plug-in doesn't support replacing a module. " + "You can try starting your debug session again to use the new module.", + GetPluginName()); + } + /// Locates or creates a module given by \p file and updates/loads the /// resulting module at the virtual base address \p base_addr. /// Note that this calls Target::GetOrCreateModule with notify being false, diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 39602421cfd96..cfd80fd9d0ae1 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1158,6 +1158,29 @@ class Target : public std::enable_shared_from_this<Target>, void ModulesDidUnload(ModuleList &module_list, bool delete_locations); + /// Replace a module in this target with a different one. + /// + /// Removes \a old_module_sp, unloading its sections and deleting the + /// breakpoint locations that resolved into it, then adds \a new_module_sp at + /// the same load address and tells the dynamic loader about the swap. + /// + /// To attach debug info to a module that is otherwise fine, add a symbol file + /// to it instead. + /// + /// \param[in] old_module_sp + /// The module to remove. Passed by value because this drops the last + /// reference the target holds, letting the module be destroyed here once + /// nothing points into it. + /// + /// \param[in] new_module_sp + /// The module to put in its place. It may already have been added to the + /// target, as Target::GetOrCreateModule() does. + /// + /// \return + /// An error if the replacement could not be completed. + Status ReplaceModule(lldb::ModuleSP old_module_sp, + const lldb::ModuleSP &new_module_sp); + void SymbolsDidLoad(ModuleList &module_list); void ClearModules(bool delete_locations); diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index f77e87ad43aa2..bd463bc3f4cf9 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -41,6 +41,7 @@ #include "lldb/Symbol/UnwindPlan.h" #include "lldb/Symbol/VariableList.h" #include "lldb/Target/ABI.h" +#include "lldb/Target/DynamicLoader.h" #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/SectionLoadList.h" @@ -3135,6 +3136,235 @@ class CommandObjectTargetModulesLoad OptionGroupUInt64 m_slide_option; }; +#pragma mark CommandObjectTargetModulesReplace + +#define LLDB_OPTIONS_target_modules_replace +#include "CommandOptions.inc" + +// Replace a module in the target with a different file on disk. + +class CommandObjectTargetModulesReplace : public CommandObjectParsed { +public: + CommandObjectTargetModulesReplace(CommandInterpreter &interpreter) + : CommandObjectParsed( + interpreter, "target modules replace", + "Replace a module in the current target with a file that has more " + "complete contents, usually to resolve a placeholder module from a " + "core file once the real binary has been located. To attach debug " + "info to a module that is otherwise fine, use 'target symbols " + "add'.\n" + "The module to replace is found by the new file's UUID, or by its " + "basename if it has no UUID. Use --old-path when neither picks a " + "single module, and --force to replace a module whose UUID does " + "not match the new file's.", + "target modules replace [--old-path <path>] [--force] <path>", + eCommandRequiresTarget | eCommandTryTargetAPILock | + eCommandProcessMustBePaused) { + AddSimpleArgumentList(eArgTypePath, eArgRepeatPlain); + } + + ~CommandObjectTargetModulesReplace() override = default; + + Options *GetOptions() override { return &m_options; } + + class CommandOptions : public Options { + public: + CommandOptions() = default; + + ~CommandOptions() override = default; + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, + ExecutionContext *execution_context) override { + const int short_option = m_getopt_table[option_idx].val; + + switch (short_option) { + case 'o': + m_old_path.assign(std::string(option_arg)); + break; + case 'f': + m_force = true; + break; + default: + llvm_unreachable("Unimplemented option"); + } + return Status(); + } + + void OptionParsingStarting(ExecutionContext *execution_context) override { + m_old_path.clear(); + m_force = false; + } + + llvm::ArrayRef<OptionDefinition> GetDefinitions() override { + return llvm::ArrayRef(g_target_modules_replace_options); + } + + std::string m_old_path; + bool m_force = false; + }; + +protected: + CommandOptions m_options; + + // Find the module the new file is meant to stand in for. Preferred order is + // the path the user gave, then the UUID read out of the new file, then its + // basename. + ModuleSP FindModuleToReplace(Target &target, + const ModuleSpec &new_module_spec, + CommandReturnObject &result) { + ModuleSpec search_spec; + llvm::StringRef description; + + if (!m_options.m_old_path.empty()) { + search_spec.GetFileSpec().SetPath(m_options.m_old_path); + description = "the specified path"; + } else if (new_module_spec.GetUUID().IsValid()) { + search_spec.GetUUID() = new_module_spec.GetUUID(); + description = "a matching UUID"; + } else { + search_spec.GetFileSpec().SetFilename( + new_module_spec.GetFileSpec().GetFilename()); + description = "a matching file basename"; + } + + ModuleList matches; + target.GetImages().FindModules(search_spec, matches); + + if (matches.IsEmpty()) { + // A file with a UUID that names nothing in the target is still worth + // trying by name, the placeholder it should replace may have been built + // without one. + if (m_options.m_old_path.empty() && new_module_spec.GetUUID().IsValid()) { + ModuleSpec by_name; + by_name.GetFileSpec().SetFilename( + new_module_spec.GetFileSpec().GetFilename()); + target.GetImages().FindModules(by_name, matches); + description = matches.IsEmpty() ? "a matching UUID or file basename" + : "a matching file basename"; + } + if (matches.IsEmpty()) { + result.AppendErrorWithFormatv( + "no module in the target was found by {0}, use the --old-path " + "option to name the module to replace", + description); + return ModuleSP(); + } + } + + if (matches.GetSize() > 1) { + StreamString paths; + for (size_t i = 0; i < matches.GetSize(); ++i) + paths.Format("\n {0}", matches.GetModuleAtIndex(i)->GetFileSpec()); + result.AppendErrorWithFormatv( + "{0} modules in the target were found by {1}, use the --old-path " + "option to name one of:{2}", + matches.GetSize(), description, paths.GetString()); + return ModuleSP(); + } + + return matches.GetModuleAtIndex(0); + } + + void DoExecute(Args &args, CommandReturnObject &result) override { + Target *target = GetTarget(); + assert(target && "target guaranteed by eCommandRequiresTarget"); + + if (args.GetArgumentCount() != 1) { + result.AppendError( + "'target modules replace' takes one argument: the path of the file " + "to replace a module with"); + return; + } + llvm::StringRef new_module_path = args.GetArgumentAtIndex(0); + + // Nothing below may change the target until Target::ReplaceModule() is + // called, so a failure can never leave the target half way through a + // replacement. + FileSpec new_file_spec(new_module_path); + FileSystem::Instance().Resolve(new_file_spec); + if (!FileSystem::Instance().Exists(new_file_spec)) { + std::string resolved_path = new_file_spec.GetPath(); + if (resolved_path != new_module_path) + result.AppendErrorWithFormatv( + "invalid module path '{0}' with resolved path '{1}'", + new_module_path, resolved_path); + else + result.AppendErrorWithFormatv("invalid module path '{0}'", + new_module_path); + return; + } + + // Read the file rather than build a Module from it, so the module it + // should replace can be found before anything is added to the target. + ModuleSpec new_module_spec(new_file_spec); + ModuleSpecList new_module_specs = + ObjectFile::GetModuleSpecifications(new_file_spec, 0, 0); + if (new_module_specs.GetSize() > 0) { + ModuleSpec arch_spec; + arch_spec.GetArchitecture() = target->GetArchitecture(); + ModuleSpec matching_spec; + if (!new_module_specs.FindMatchingModuleSpec(arch_spec, matching_spec)) { + result.AppendErrorWithFormatv( + "'{0}' does not contain the target architecture {1}", + new_file_spec.GetPath(), + target->GetArchitecture().GetTriple().str()); + return; + } + new_module_spec = matching_spec; + } + ModuleSP old_module_sp = + FindModuleToReplace(*target, new_module_spec, result); + // FindModuleToReplace() has already put the error into result. + if (!old_module_sp) + return; + + // Different UUIDs mean the new file is not the binary that ran, so the + // symbols would not describe the memory the target has. + const UUID &old_uuid = old_module_sp->GetUUID(); + const UUID &new_uuid = new_module_spec.GetUUID(); + if (!m_options.m_force && old_uuid.IsValid() && new_uuid.IsValid() && + old_uuid != new_uuid) { + result.AppendErrorWithFormatv( + "'{0}' has UUID {1}, which does not match UUID {2} of the module it " + "would replace, '{3}'. Use the --force option to replace it anyway", + new_file_spec.GetPath(), new_uuid.GetAsString(), + old_uuid.GetAsString(), old_module_sp->GetFileSpec().GetPath()); + return; + } + + if (!new_module_spec.GetArchitecture().IsValid()) + new_module_spec.GetArchitecture() = target->GetArchitecture(); + + // Look the file up by path alone. Asking for the UUID as well would find + // the module already in the target, which is the one being replaced. + new_module_spec.GetUUID().Clear(); + + Status error; + ModuleSP new_module_sp = + target->GetOrCreateModule(new_module_spec, /*notify=*/false, &error); + if (!new_module_sp) { + if (error.Fail()) + result.SetError(error.takeError()); + else + result.AppendErrorWithFormatv("unsupported module: {0}", + new_file_spec.GetPath()); + return; + } + + const std::string old_module_desc = old_module_sp->GetFileSpec().GetPath(); + Status replace_error = + target->ReplaceModule(std::move(old_module_sp), new_module_sp); + if (replace_error.Fail()) { + result.SetError(replace_error.takeError()); + return; + } + + result.AppendMessageWithFormatv("replaced '{0}' with '{1}'", + old_module_desc, new_file_spec.GetPath()); + result.SetStatus(eReturnStatusSuccessFinishResult); + } +}; + #pragma mark CommandObjectTargetModulesList // List images with associated information #define LLDB_OPTIONS_target_modules_list @@ -4230,6 +4460,9 @@ class CommandObjectTargetModules : public CommandObjectMultiword { LoadSubCommand( "lookup", CommandObjectSP(new CommandObjectTargetModulesLookup(interpreter))); + LoadSubCommand( + "replace", + CommandObjectSP(new CommandObjectTargetModulesReplace(interpreter))); LoadSubCommand( "search-paths", CommandObjectSP( diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index ab851725979ef..055adf1222e8a 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1820,6 +1820,19 @@ let Command = "target modules show unwind" in { Desc<"Show cached unwind information">; } +let Command = "target modules replace" in { + def target_modules_replace_old_path + : Option<"old-path", "o">, + Arg<"Path">, + Desc<"Path of the module in the target to replace. This is only needed " + "if the new module doesn't have a valid UUID or if the basename " + "doesn't uniquely match any existing target modules.">; + def target_modules_replace_force + : Option<"force", "f">, + Desc<"Replace the module even when its UUID does not match the UUID of " + "the new file.">; +} + let Command = "target modules lookup" in { def target_modules_lookup_address : Option<"address", "a">, Group<1>, diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 70d24c45552dd..9bc40aed2bd17 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -192,9 +192,11 @@ void DynamicLoaderPOSIXDYLD::DidLaunch() { Status DynamicLoaderPOSIXDYLD::CanLoadImage() { return Status(); } void DynamicLoaderPOSIXDYLD::SetLoadedModule(const ModuleSP &module_sp, - addr_t link_map_addr) { + addr_t link_map_addr, + addr_t base_addr, + bool base_addr_is_offset) { llvm::sys::ScopedWriter lock(m_loaded_modules_rw_mutex); - m_loaded_modules[module_sp] = link_map_addr; + m_loaded_modules[module_sp] = {link_map_addr, base_addr, base_addr_is_offset}; } void DynamicLoaderPOSIXDYLD::UnloadModule(const ModuleSP &module_sp) { @@ -202,8 +204,8 @@ void DynamicLoaderPOSIXDYLD::UnloadModule(const ModuleSP &module_sp) { m_loaded_modules.erase(module_sp); } -std::optional<lldb::addr_t> -DynamicLoaderPOSIXDYLD::GetLoadedModuleLinkAddr(const ModuleSP &module_sp) { +std::optional<DynamicLoaderPOSIXDYLD::LoadedModuleInfo> +DynamicLoaderPOSIXDYLD::GetLoadedModuleInfo(const ModuleSP &module_sp) { llvm::sys::ScopedReader lock(m_loaded_modules_rw_mutex); auto it = m_loaded_modules.find(module_sp); if (it != m_loaded_modules.end()) @@ -211,11 +213,41 @@ DynamicLoaderPOSIXDYLD::GetLoadedModuleLinkAddr(const ModuleSP &module_sp) { return std::nullopt; } +Status DynamicLoaderPOSIXDYLD::ReplaceModule(const ModuleSP &old_module_sp, + const ModuleSP &new_module_sp) { + // Load the replacement the same way the old module was loaded, which also + // carries the link map address across so its thread locals resolve. + LoadedModuleInfo info; + if (std::optional<LoadedModuleInfo> loaded = + GetLoadedModuleInfo(old_module_sp); + loaded && loaded->base_addr != LLDB_INVALID_ADDRESS) { + info = *loaded; + } else if (ObjectFile *object_file = old_module_sp->GetObjectFile()) { + // Post mortem processes build their module list from the core file rather + // than by walking the rendezvous, so there is nothing recorded here for + // them. Fall back to where the module says it is. + Address base = object_file->GetBaseAddress(); + if (base.IsValid()) + info.base_addr = base.GetLoadAddress(&m_process->GetTarget()); + if (loaded) + info.link_map_addr = loaded->link_map_addr; + } + + if (info.base_addr == LLDB_INVALID_ADDRESS) + return Status::FromErrorStringWithFormatv( + "'{0}' is not loaded at a known address", old_module_sp->GetFileSpec()); + + UnloadSections(old_module_sp); + UpdateLoadedSections(new_module_sp, info.link_map_addr, info.base_addr, + info.base_addr_is_offset); + return Status(); +} + void DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module, addr_t link_map_addr, addr_t base_addr, bool base_addr_is_offset) { - SetLoadedModule(module, link_map_addr); + SetLoadedModule(module, link_map_addr, base_addr, base_addr_is_offset); UpdateLoadedSectionsCommon(module, base_addr, base_addr_is_offset); } @@ -841,8 +873,8 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp, const lldb::ThreadSP thread, lldb::addr_t tls_file_addr) { Log *log = GetLog(LLDBLog::DynamicLoader); - std::optional<addr_t> link_map_addr_opt = GetLoadedModuleLinkAddr(module_sp); - if (!link_map_addr_opt.has_value()) { + std::optional<LoadedModuleInfo> info = GetLoadedModuleInfo(module_sp); + if (!info.has_value()) { LLDB_LOG( log, "GetThreadLocalData error: module({0}) not found in loaded modules", @@ -850,7 +882,7 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp, return LLDB_INVALID_ADDRESS; } - addr_t link_map = link_map_addr_opt.value(); + addr_t link_map = info->link_map_addr; 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 6efb92673a13c..559a551cd5f73 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -66,6 +66,10 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader { llvm::function_ref<bool(const lldb_private::Thread &)> save_thread_predicate) override; + lldb_private::Status + ReplaceModule(const lldb::ModuleSP &old_module_sp, + const lldb::ModuleSP &new_module_sp) override; + protected: /// Runtime linker rendezvous structure. DYLDRendezvous m_rendezvous; @@ -177,18 +181,28 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader { const DynamicLoaderPOSIXDYLD & operator=(const DynamicLoaderPOSIXDYLD &) = delete; + /// What each module was loaded with, enough to load another module the same + /// way. + struct LoadedModuleInfo { + lldb::addr_t link_map_addr = LLDB_INVALID_ADDRESS; + lldb::addr_t base_addr = LLDB_INVALID_ADDRESS; + bool base_addr_is_offset = false; + }; + /// 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::a... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/214576 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
