llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Greg Clayton (clayborg) <details> <summary>Changes</summary> This patch adds some new key/value pairs for the "statistics dump" command: - "platform_path" if the platform path of a module differs from the path - "memoryAddress" if the module is loaded in memory - "coreHasUUID" for core file modules to tell if we have a UUID in the core file and know that we have the right file. --- Full diff: https://github.com/llvm/llvm-project/pull/206853.diff 7 Files Affected: - (modified) lldb/include/lldb/Core/Module.h (+7) - (modified) lldb/include/lldb/Target/Statistics.h (+4) - (modified) lldb/source/Core/Module.cpp (+23) - (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+4) - (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+44-6) - (modified) lldb/source/Target/Statistics.cpp (+26-1) - (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py (+29) ``````````diff diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index 33904ef7be5d8..bb07234661c2b 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/lldb/include/lldb/Core/Module.h @@ -160,6 +160,13 @@ class Module : public std::enable_shared_from_this<Module>, bool MatchesModuleSpec(const ModuleSpec &module_ref); + // Get the module spec from this module. + // + // Get a module spec for this module. If the process is specified valid + // the load address of the module can be filled in as well. + lldb_private::ModuleSpec + GetModuleSpec(const lldb::ProcessSP &process_sp); + /// Set the load address for all sections in a module to be the file address /// plus \a slide. /// diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 24fe036d91a2d..ba307e8334a0c 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -144,6 +144,7 @@ struct ModuleStats { llvm::json::Value ToJSON() const; intptr_t identifier; std::string path; + std::optional<std::string> platform_path; std::string uuid; std::string triple; // Path separate debug info file, or empty if none. @@ -169,6 +170,9 @@ struct ModuleStats { bool symtab_stripped = false; bool debug_info_had_variable_errors = false; bool debug_info_had_incomplete_types = false; + /// If the module is loaded from memory, this will be a valid address. + std::optional<lldb::addr_t> memory_addr; + std::optional<bool> core_has_uuid; DWOStats dwo_stats; }; diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index 2bc8fd138427d..51d3a6a87cd73 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1673,3 +1673,26 @@ lldb_private::ModuleSpecList Module::GetSeparateDebugInfoFiles() { return symfile->GetSeparateDebugInfoFiles(); } + +ModuleSpec Module::GetModuleSpec(const lldb::ProcessSP &process_sp) { + ModuleSpec spec; + spec.GetFileSpec() = m_file; + spec.GetPlatformFileSpec() = m_platform_file; + spec.GetObjectName() = m_object_name; + spec.SetObjectOffset(m_object_offset); + spec.GetUUID() = m_uuid; + spec.GetArchitecture() = m_arch; + // Set the load address of the module if possible. + if (GetMemoryModuleAddress().has_value()) { + spec.SetLoadAddress(GetMemoryModuleAddress().value()); + } else if (process_sp) { + ObjectFile *objfile_ptr = GetObjectFile(); + if (objfile_ptr) { + Address base_addr = objfile_ptr->GetBaseAddress(); + addr_t load_addr = base_addr.GetLoadAddress(&process_sp->GetTarget()); + if (load_addr != LLDB_INVALID_ADDRESS) + spec.SetLoadAddress(load_addr); + } + } + return spec; +} diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 6db4d99ccbdba..5ccdd7606db73 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -997,6 +997,10 @@ void DynamicLoaderPOSIXDYLD::ResolveExecutableModule( ModuleSpec module_spec(process_info.GetExecutableFile(), process_info.GetArchitecture()); + // See if the process has UUID info for the executable. If this is a core + // file we really want the UUID in the module spec so we don't load a + // random executable with the same name and ignore the required UUID. + m_process->FindModuleUUID(module_spec); if (module_sp && module_sp->MatchesModuleSpec(module_spec)) return; diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index ced84ab829772..724e65ca0cd59 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -288,10 +288,10 @@ Status ProcessElfCore::DoLoadCore() { exe_module_sp = Module::CreateModuleFromObjectFile<ObjectFilePlaceholder>( exe_module_spec, load_addr, size); - if (exe_module_spec.GetPlatformFileSpec()) - exe_module_sp->SetPlatformFileSpec( - exe_module_spec.GetPlatformFileSpec()); } + if (exe_module_spec.GetPlatformFileSpec()) + exe_module_sp->SetPlatformFileSpec( + exe_module_spec.GetPlatformFileSpec()); } if (exe_module_sp) GetTarget().SetExecutableModule(exe_module_sp, eLoadDependentsNo); @@ -402,8 +402,24 @@ bool ProcessElfCore::FindModuleUUID(ModuleSpec &spec) { } // If we didn't find a file spec from the load address, fall back to using // the file spec. - if (path.empty()) + if (path.empty()) { + // No path found in NT_FILE info. path = spec.GetFileSpec().GetPath(); + } else { + // We got a path from the NT_FILE info. + if (spec.GetFileSpec()) { + // The module spec had a path, see if the NT_FILE path differs from the + // spec path and if so set the platform file spec. This is typically + // the resolved path where the spec path can be a symlink. Having both + // paths in the module spec and eventually in the module will help + // clients match files up more effectively. + if (spec.GetFileSpec().GetPath() != path) + spec.GetPlatformFileSpec().SetPath(path); + } else { + // The module spec doesn't have a path, fill it in in the spec. + spec.GetFileSpec().SetPath(path); + } + } auto it = m_uuids.find(path); if (it != m_uuids.end()) { @@ -1238,13 +1254,35 @@ bool ProcessElfCore::GetProcessInfo(ProcessInstanceInfo &info) { info.Clear(); info.SetProcessID(GetID()); info.SetArchitecture(GetArchitecture()); + ModuleSpec exe_module_spec; + bool added_executable = false; lldb::ModuleSP module_sp = GetTarget().GetExecutableModule(); + const bool add_exe_file_as_first_arg = true; if (module_sp) { - const bool add_exe_file_as_first_arg = false; info.SetExecutableFile(GetTarget().GetExecutableModule()->GetFileSpec(), add_exe_file_as_first_arg); + added_executable = true; + } else { + ModuleSpec exe_module_spec; + if (GetMainExecutableModuleSpec(exe_module_spec)) { + if (exe_module_spec.GetFileSpec()) { + info.SetExecutableFile(exe_module_spec.GetFileSpec(), + add_exe_file_as_first_arg); + added_executable = true; + } + } + } + Args process_args = m_process_args.as_args(); + bool first_arg_is_executable = true; + if (added_executable) { + // Strip the executable name from the process args as it can be a symlink + // that doesn't match the executable we would have created from a call to + // GetMainExecutableModuleSpec(...). + first_arg_is_executable = false; + info.SetArg0(process_args.GetArgumentAtIndex(0)); + process_args.DeleteArgumentAtIndex(0); } - info.SetArguments(m_process_args.as_args(), /*first_arg_is_executable=*/true); + info.SetArguments(process_args, first_arg_is_executable); return true; } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 6227d099642f2..0f674dc2deda2 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -53,6 +53,8 @@ void TargetStats::CollectStats(Target &target) { json::Value ModuleStats::ToJSON() const { json::Object module; EmplaceSafeString(module, "path", path); + if (platform_path.has_value()) + EmplaceSafeString(module, "platformPath", *platform_path); EmplaceSafeString(module, "uuid", uuid); EmplaceSafeString(module, "triple", triple); module.try_emplace("identifier", identifier); @@ -77,7 +79,10 @@ json::Value ModuleStats::ToJSON() const { module.try_emplace("dwoFileCount", dwo_stats.dwo_file_count); module.try_emplace("loadedDwoFileCount", dwo_stats.loaded_dwo_file_count); module.try_emplace("dwoErrorCount", dwo_stats.dwo_error_count); - + if (memory_addr.has_value()) + module.try_emplace("memoryAddress", *memory_addr); + if (core_has_uuid.has_value()) + module.try_emplace("coreHasUUID", *core_has_uuid); if (!symbol_locator_time.map.empty()) { json::Object obj; for (const auto &entry : symbol_locator_time.map) @@ -328,6 +333,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( const uint64_t num_modules = target != nullptr ? target->GetImages().GetSize() : Module::GetNumberAllocatedModules(); + ProcessSP process_sp = target ? target->GetProcessSP() : ProcessSP(); uint32_t num_debug_info_enabled_modules = 0; uint32_t num_modules_has_debug_info = 0; uint32_t num_modules_with_variable_errors = 0; @@ -356,6 +362,23 @@ llvm::json::Value DebuggerStats::ReportStatistics( if (module_stat.symtab_saved_to_cache) ++symtabs_saved_to_cache; } + if (process_sp && !process_sp->IsLiveDebugSession()) { + // We have a core file, it is good to know if the module had UUID + // information in the core file. Some core files don't have UUIDs and + // we might end up loading the file from the current filesystem and it + // can be the wrong file. + ModuleSpec module_spec = module->GetModuleSpec(process_sp); + // If we have a UUID and the module is in memory, then the UUID is in + // the core file. + if (!module->GetMemoryModuleAddress().has_value()) { + // Clear the UUID and see if the process can resolve it. + module_spec.GetUUID().Clear(); + // See if the core file has the UUID for this module. + process_sp->FindModuleUUID(module_spec); + } + module_stat.core_has_uuid = module_spec.GetUUID().IsValid(); + } + module_stat.memory_addr = module->GetMemoryModuleAddress(); SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false); if (sym_file) { if (!summary_only) { @@ -413,6 +436,8 @@ llvm::json::Value DebuggerStats::ReportStatistics( if (include_modules) { module_stat.identifier = (intptr_t)module; module_stat.path = module->GetFileSpec().GetPath(); + if (module->GetFileSpec() != module->GetPlatformFileSpec()) + module_stat.platform_path = module->GetPlatformFileSpec().GetPath(); if (ConstString object_name = module->GetObjectName()) { module_stat.path.append(1, '('); module_stat.path.append(object_name.GetStringRef().str()); diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index fc79ae98e5b73..107ed5a428886 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -1568,6 +1568,35 @@ def cleanup(): + "/lib64/ld-linux-x86-64.so.2: ECBDF3F8-784D-7A13-EFF2-FDD4352ABBEE-93CCE02C", log_text, ) + + # Test that "statistics dump" has core file specific key/value pairs + debug_stats = self.get_stats() + modules = debug_stats["modules"] + def find_module_dict(modules, path): + for d in modules: + if d["path"] == path: + return d + return None + module_dict = find_module_dict(modules, "/data/users/gclayton/crash-for-core/a.out") + self.assertIsNotNone(module_dict) + self.assertNotIn("platformPath", module_dict) + self.assertEqual(module_dict["coreHasUUID"], True) + self.assertEqual(module_dict["memoryAddress"], 94267628855296) + self.assertEqual(module_dict["uuid"], "0DA654F5-BD3A-60F5-D3CF-A88CD0ABD565-AC497CEA") + + module_dict = find_module_dict(modules, "linux-vdso.so.1") + self.assertIsNotNone(module_dict) + self.assertNotIn("platformPath", module_dict) + self.assertEqual(module_dict["coreHasUUID"], True) + self.assertEqual(module_dict["memoryAddress"], 140248680775680) + self.assertEqual(module_dict["uuid"], "585A45BC-FF77-F58F-5F90-63149C609FC6-1BFEF054") + + module_dict = find_module_dict(modules, "/lib64/libstdc++.so.6") + self.assertIsNotNone(module_dict) + self.assertEqual(module_dict["platformPath"], "/usr/lib64/libstdc++.so.6.0.29") + self.assertEqual(module_dict["coreHasUUID"], True) + self.assertEqual(module_dict["uuid"], "0C8999CC-A62E-9B9F-0075-566ECB23D564-443ED68F") + self.dbg.DeleteTarget(target) `````````` </details> https://github.com/llvm/llvm-project/pull/206853 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
