https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/148735
>From c021e054277a590901012e6c9fcdcff8fedf9867 Mon Sep 17 00:00:00 2001 From: Jeffrey Tan <jeffrey...@meta.com> Date: Wed, 21 Jul 2021 13:32:56 -0700 Subject: [PATCH] Add new "target module replace" command Summary: For dump debugging (both minidump and coredump), the dump most likely only contains the process memory without real modules so placeholder object files are created for the modules. LLDB lacks a feature to hydrate/upgrade these dummy modules into real modules. This diff bridges the gap with a new `target module replace` command. The new command would replace the place holder object file in target module with input new/real object file and refresh the symbol table/stack. The target module is located with file name matching, users can also specify the target module via "-s" option if the module names differ. Another change is placeholder modules will have `(*)` added at the end as hint. The workflow would be: * lldb -c <path_to_dump> * `image list` to see place holder modules * download fbpkg and unzip it * `target module replace <downloaded_fbpkg_module>` Test Plan: A new unit test is added. c4crasher end-to-end debugging is tested. c4crasher testing workflow is documented here: https://www.internalfb.com/intern/wiki/Coredumper/Developer_Guide/Development_Process/ Reviewers: wanyi, #lldb_team Reviewed By: wanyi Subscribers: gclayton, #lldb_team Differential Revision: https://phabricator.intern.facebook.com/D43756670 --- lldb/include/lldb/Core/Module.h | 21 +++ lldb/source/Commands/CommandObjectTarget.cpp | 155 +++++++++++++++++- lldb/source/Core/Module.cpp | 89 ++++++---- .../postmortem/elf-core/TestLinuxCore.py | 35 ++++ 4 files changed, 267 insertions(+), 33 deletions(-) diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index 8bb55c95773bc..04e1427a6a984 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/lldb/include/lldb/Core/Module.h @@ -547,6 +547,22 @@ class Module : public std::enable_shared_from_this<Module>, /// remains valid as long as the object is around. virtual ObjectFile *GetObjectFile(); + /// Replace existing backing object file with new \param object_file. + /// + /// The old object file being replaced will be kept alive to prevent from + /// dangling symbol references. + /// UUID and underlying symbol file will be reparsed during further access. + /// A common use case is to replace an Placeholder object file with a real + /// one during dump debugging. + /// + /// \param[in] target + /// The target to update object file load address. + /// + /// \param[in] object_file + /// The new object file spec to replace the existing one. + virtual void ReplaceObjectFile(Target &target, FileSpec object_file, + uint64_t object_offset); + /// Get the unified section list for the module. This is the section list /// created by the module's object file and any debug info and symbol files /// created by the symbol vendor. @@ -1032,6 +1048,9 @@ class Module : public std::enable_shared_from_this<Module>, lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file /// parser for this module as it may or may /// not be shared with the SymbolFile + lldb::ObjectFileSP m_old_objfile_sp; /// Strong reference to keep the old + /// object file being replaced alive. + UnwindTable m_unwind_table; ///< Table of FuncUnwinders /// objects created for this /// Module's functions @@ -1093,6 +1112,8 @@ class Module : public std::enable_shared_from_this<Module>, private: Module(); // Only used internally by CreateJITModule () + void LoadObjectFile(); + Module(const Module &) = delete; const Module &operator=(const Module &) = delete; diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index dbebbbd38093e..9d7207d032058 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -2866,6 +2866,150 @@ class CommandObjectTargetModulesAdd : public CommandObjectParsed { } }; +class CommandObjectTargetModulesReplace : public CommandObjectParsed { +public: + CommandObjectTargetModulesReplace(CommandInterpreter &interpreter) + : CommandObjectParsed( + interpreter, "target modules replace", + "Replace module's existing object file with a new object file.", + "target modules replace [<module>]", eCommandRequiresTarget), + m_file_to_replace(LLDB_OPT_SET_1, false, "shlib", 's', + lldb::eModuleCompletion, eArgTypeShlibName, + "File name of the shared library to replace.") { + m_option_group.Append(&m_uuid_option_group, LLDB_OPT_SET_ALL, + LLDB_OPT_SET_1); + m_option_group.Append(&m_file_to_replace, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1); + m_option_group.Finalize(); + CommandArgumentData module_arg{eArgTypePath, eArgRepeatStar}; + m_arguments.push_back({module_arg}); + } + + ~CommandObjectTargetModulesReplace() override = default; + + Options *GetOptions() override { return &m_option_group; } + + void + HandleArgumentCompletion(CompletionRequest &request, + OptionElementVector &opt_element_vector) override { + CommandCompletions::InvokeCommonCompletionCallbacks( + GetCommandInterpreter(), lldb::eDiskFileCompletion, request, nullptr); + } + +protected: + OptionGroupOptions m_option_group; + OptionGroupUUID m_uuid_option_group; + OptionGroupFile m_file_to_replace; + + void DoExecute(Args &args, CommandReturnObject &result) override { + if (args.GetArgumentCount() == 0) { + result.AppendError( + "one or more executable image paths must be specified"); + return; + } + + Target &target = GetTarget(); + bool flush = false; + // TODO: investigate if we should only allow one module. Similar for + // CommandObjectTargetModulesAdd and CommandObjectTargetSymbolsAdd. + for (auto &entry : args.entries()) { + if (entry.ref().empty()) + continue; + + FileSpec file_spec(entry.ref()); + if (FileSystem::Instance().Exists(file_spec)) { + ModuleSpec module_spec(file_spec); + if (m_uuid_option_group.GetOptionValue().OptionWasSet()) + module_spec.GetUUID() = + m_uuid_option_group.GetOptionValue().GetCurrentValue(); + if (!module_spec.GetArchitecture().IsValid()) + module_spec.GetArchitecture() = target.GetArchitecture(); + if (m_file_to_replace.GetOptionValue().OptionWasSet()) + module_spec.GetFileSpec().SetFilename( + m_file_to_replace.GetOptionValue() + .GetCurrentValue() + .GetFilename()); + + ModuleList matching_modules = findMatchingModules(module_spec); + if (matching_modules.IsEmpty()) { + result.AppendErrorWithFormat("can't find matching modules for '%s'", + entry.ref().str().c_str()); + return; + } + + if (matching_modules.GetSize() > 1) { + result.AppendErrorWithFormat( + "multiple modules match symbol file '%s', " + "use the --uuid option to resolve the " + "ambiguity.\n", + entry.ref().str().c_str()); + return; + } + + assert(matching_modules.GetSize() == 1); + auto module_sp = matching_modules.GetModuleAtIndex(0); + module_sp->ReplaceObjectFile(target, file_spec, /*object_offset=*/0); + + if (target.GetPreloadSymbols()) + module_sp->PreloadSymbols(); + + flush = true; + result.SetStatus(eReturnStatusSuccessFinishResult); + } else { + std::string resolved_path = file_spec.GetPath(); + if (resolved_path != entry.ref()) { + result.AppendErrorWithFormat( + "invalid module path '%s' with resolved path '%s'\n", + entry.ref().str().c_str(), resolved_path.c_str()); + break; + } + result.AppendErrorWithFormat("invalid module path '%s'\n", + entry.c_str()); + break; + } + } + + if (flush) { + ProcessSP process = target.GetProcessSP(); + if (process) + process->Flush(); + } + return; + } + + ModuleList findMatchingModules(const ModuleSpec &module_spec) { + Target &target = GetTarget(); + ModuleList matching_modules; + lldb_private::ModuleSpecList module_specs; + if (ObjectFile::GetModuleSpecifications(module_spec.GetFileSpec(), 0, 0, + module_specs)) { + // Now extract the module spec that matches the target architecture + ModuleSpec target_arch_module_spec; + ModuleSpec arch_matched_module_spec; + target_arch_module_spec.GetArchitecture() = target.GetArchitecture(); + if (module_specs.FindMatchingModuleSpec(target_arch_module_spec, + arch_matched_module_spec)) { + if (arch_matched_module_spec.GetUUID().IsValid()) { + // It has a UUID, look for this UUID in the target modules + ModuleSpec uuid_module_spec; + uuid_module_spec.GetUUID() = arch_matched_module_spec.GetUUID(); + target.GetImages().FindModules(uuid_module_spec, matching_modules); + } + } + } + + // Just try to match up the file by basename if we have no matches at + // this point. + if (matching_modules.IsEmpty()) { + ModuleSpec filename_only_spec; + filename_only_spec.GetFileSpec().SetFilename( + module_spec.GetFileSpec().GetFilename()); + target.GetImages().FindModules(filename_only_spec, matching_modules); + } + + return matching_modules; + } +}; + class CommandObjectTargetModulesLoad : public CommandObjectTargetModulesModuleAutoComplete { public: @@ -3333,10 +3477,14 @@ class CommandObjectTargetModulesList : public CommandObjectParsed { DumpModuleArchitecture(strm, module, true, width); break; - case 'f': + case 'f': { DumpFullpath(strm, &module->GetFileSpec(), width); dump_object_name = true; - break; + + ObjectFile *objfile = module->GetObjectFile(); + if (objfile && objfile->GetPluginName() == "placeholder") + strm.Printf("(*)"); + } break; case 'd': DumpDirectory(strm, &module->GetFileSpec(), width); @@ -4205,6 +4353,9 @@ class CommandObjectTargetModules : public CommandObjectMultiword { "target modules <sub-command> ...") { LoadSubCommand( "add", CommandObjectSP(new CommandObjectTargetModulesAdd(interpreter))); + LoadSubCommand( + "replace", + CommandObjectSP(new CommandObjectTargetModulesReplace(interpreter))); LoadSubCommand("load", CommandObjectSP(new CommandObjectTargetModulesLoad( interpreter))); LoadSubCommand("dump", CommandObjectSP(new CommandObjectTargetModulesDump( diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 90997dada3666..4140ec6c26930 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1177,42 +1177,69 @@ ObjectFile *Module::GetObjectFile() { if (!m_did_load_objfile.load()) { LLDB_SCOPED_TIMERF("Module::GetObjectFile () module = %s", GetFileSpec().GetFilename().AsCString("")); - lldb::offset_t data_offset = 0; - lldb::offset_t file_size = 0; - - if (m_data_sp) - file_size = m_data_sp->GetByteSize(); - else if (m_file) - file_size = FileSystem::Instance().GetByteSize(m_file); - - if (file_size > m_object_offset) { - m_did_load_objfile = true; - // FindPlugin will modify its data_sp argument. Do not let it - // modify our m_data_sp member. - auto data_sp = m_data_sp; - m_objfile_sp = ObjectFile::FindPlugin( - shared_from_this(), &m_file, m_object_offset, - file_size - m_object_offset, data_sp, data_offset); - if (m_objfile_sp) { - // Once we get the object file, update our module with the object - // file's architecture since it might differ in vendor/os if some - // parts were unknown. But since the matching arch might already be - // more specific than the generic COFF architecture, only merge in - // those values that overwrite unspecified unknown values. - m_arch.MergeFrom(m_objfile_sp->GetArchitecture()); - - m_unwind_table.ModuleWasUpdated(); - } else { - ReportError("failed to load objfile for {0}\nDebugging will be " - "degraded for this module.", - GetFileSpec().GetPath().c_str()); - } - } + LoadObjectFile(); } } return m_objfile_sp.get(); } +void Module::LoadObjectFile() { + lldb::offset_t data_offset = 0; + lldb::offset_t file_size = 0; + + if (m_data_sp) + file_size = m_data_sp->GetByteSize(); + else if (m_file) + file_size = FileSystem::Instance().GetByteSize(m_file); + + if (file_size <= m_object_offset) + return; + + m_did_load_objfile = true; + // FindPlugin will modify its data_sp argument. Do not let it + // modify our m_data_sp member. + auto data_sp = m_data_sp; + m_objfile_sp = + ObjectFile::FindPlugin(shared_from_this(), &m_file, m_object_offset, + file_size - m_object_offset, data_sp, data_offset); + if (m_objfile_sp) { + // Once we get the object file, update our module with the object + // file's architecture since it might differ in vendor/os if some + // parts were unknown. But since the matching arch might already be + // more specific than the generic COFF architecture, only merge in + // those values that overwrite unspecified unknown values. + m_arch.MergeFrom(m_objfile_sp->GetArchitecture()); + m_unwind_table.ModuleWasUpdated(); + } else { + ReportError("failed to load objfile for {0}", + GetFileSpec().GetPath().c_str()); + } +} + +void Module::ReplaceObjectFile(Target &target, FileSpec object_file, + uint64_t object_offset) { + m_old_objfile_sp = m_objfile_sp; + m_file = object_file; + m_object_offset = object_offset; + lldb::addr_t load_address = + GetObjectFile()->GetBaseAddress().GetLoadAddress(&target); + + // Scope locking. + { + std::lock_guard<std::recursive_mutex> guard(m_mutex); + LLDB_SCOPED_TIMERF("Module::ReplaceObjectFile () module = %s", + GetFileSpec().GetFilename().AsCString("")); + LoadObjectFile(); + } + + bool changed = false; + SetLoadAddress(target, load_address, false, changed); + + // Force reparsing UUID and symbol files. + m_did_set_uuid = false; + m_did_load_symfile = false; +} + SectionList *Module::GetSectionList() { // Populate m_sections_up with sections from objfile. if (!m_sections_up) { diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index a68175dc4e4d7..04bb418798dfa 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -1005,6 +1005,41 @@ def test_read_only_cstring(self): cstr = var.GetSummary() self.assertEqual(cstr, '"_start"') + @skipIfLLVMTargetMissing("X86") + @skipIfWindows + def test_module_list_dyld(self): + """ + Test module list based dyld can successfully get images + list from NT_FILE without main executable + """ + self.runCmd("settings set use-module-list-dyld true") + target = self.dbg.CreateTarget(None) + process = target.LoadCore("linux-x86_64.core") + self.assertTrue(process, PROCESS_IS_VALID) + + self.assertEqual(process.GetTarget().GetNumModules(), 1) + exe_module = process.GetTarget().GetModuleAtIndex(0) + # Module load address is got from coredump NT_FILE. + self.assertEqual( + exe_module.GetObjectFileHeaderAddress().GetLoadAddress(target), 0x400000 + ) + self.dbg.DeleteTarget(target) + + def test_replace_placeholder_module(self): + """ + Test module list based dyld can successfully get images list from + NT_FILE without main executable. And `target module replace` + command can replace the Placeholder object file with real one. + """ + self.runCmd("settings set use-module-list-dyld true") + target = self.dbg.CreateTarget(None) + process = target.LoadCore("linux-x86_64.core") + self.assertTrue(process, PROCESS_IS_VALID) + + self.runCmd("target module replace linux-x86_64.out -s a.out") + self.check_all(process, self._x86_64_pid, self._x86_64_regions, "a.out") + self.dbg.DeleteTarget(target) + def check_memory_regions(self, process, region_count): region_list = process.GetMemoryRegions() self.assertEqual(region_list.GetSize(), region_count) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
