Author: Jacob Lalonde Date: 2024-10-10T15:59:51-07:00 New Revision: e9c8f75d45ababe7f805078bbf7bda2e7425f1b7
URL: https://github.com/llvm/llvm-project/commit/e9c8f75d45ababe7f805078bbf7bda2e7425f1b7 DIFF: https://github.com/llvm/llvm-project/commit/e9c8f75d45ababe7f805078bbf7bda2e7425f1b7.diff LOG: [LLDB][Minidump] Have Minidumps save off and properly read TLS data (#109477) This patch adds the support to `Process.cpp` to automatically save off TLS sections, either via loading the memory region for the module, or via reading `fs_base` via generic register. Then when Minidumps are loaded, we now specify we want the dynamic loader to be the `POSIXDYLD` so we can leverage the same TLS accessor code as `ProcessELFCore`. Being able to access TLS Data is an important step for LLDB generated minidumps to have feature parity with ELF Core dumps. Added: Modified: lldb/include/lldb/Target/DynamicLoader.h lldb/source/Core/DynamicLoader.cpp lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp lldb/source/Plugins/Process/minidump/ProcessMinidump.h lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp lldb/source/Target/Process.cpp lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py lldb/test/API/functionalities/process_save_core_minidump/main.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 0629e2faae7e9e..75bb6cb6bb9074 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -11,6 +11,7 @@ #include "lldb/Core/Address.h" #include "lldb/Core/PluginInterface.h" +#include "lldb/Target/CoreFileMemoryRanges.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/UUID.h" @@ -337,6 +338,17 @@ class DynamicLoader : public PluginInterface { return std::nullopt; } + /// Returns a list of memory ranges that should be saved in the core file, + /// specific for this dynamic loader. + /// + /// For example, an implementation of this function can save the thread + /// local data of a given thread. + virtual void CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector<lldb_private::MemoryRegionInfo> &ranges, + llvm::function_ref<bool(const lldb_private::Thread &)> + save_thread_predicate) {}; + protected: // Utility methods for derived classes diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 7758a87403b5a3..68d6ab0850853f 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -83,7 +83,11 @@ ModuleSP DynamicLoader::GetTargetExecutable() { ModuleSpec module_spec(executable->GetFileSpec(), executable->GetArchitecture()); auto module_sp = std::make_shared<Module>(module_spec); - + // If we're a coredump and we already have a main executable, we don't + // need to reload the module list that target already has + if (!m_process->IsLiveDebugSession()) { + return executable; + } // Check if the executable has changed and set it to the target // executable if they diff er. if (module_sp && module_sp->GetUUID().IsValid() && @@ -369,4 +373,3 @@ void DynamicLoader::LoadOperatingSystemPlugin(bool flush) if (m_process) m_process->LoadOperatingSystemPlugin(flush); } - diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index b9c0e174c3be68..34aca50df0ac4b 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -18,6 +18,7 @@ #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/Platform.h" +#include "lldb/Target/RegisterContext.h" #include "lldb/Target/Target.h" #include "lldb/Target/Thread.h" #include "lldb/Target/ThreadPlanRunToAddress.h" @@ -866,3 +867,82 @@ bool DynamicLoaderPOSIXDYLD::AlwaysRelyOnEHUnwindInfo( bool DynamicLoaderPOSIXDYLD::IsCoreFile() const { return !m_process->IsLiveDebugSession(); } + +// For our ELF/POSIX builds save off the fs_base/gs_base regions +static void AddThreadLocalMemoryRegions(Process &process, ThreadSP &thread_sp, + std::vector<MemoryRegionInfo> &ranges) { + lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); + if (!reg_ctx) + return; + + const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo( + lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_TP); + if (!reg_info) + return; + + lldb_private::RegisterValue thread_local_register_value; + bool success = reg_ctx->ReadRegister(reg_info, thread_local_register_value); + if (!success) + return; + + const uint64_t fail_value = UINT64_MAX; + bool readSuccess = false; + const lldb::addr_t reg_value_addr = + thread_local_register_value.GetAsUInt64(fail_value, &readSuccess); + if (!readSuccess || reg_value_addr == fail_value) + return; + + MemoryRegionInfo thread_local_region; + Status err = process.GetMemoryRegionInfo(reg_value_addr, thread_local_region); + if (err.Fail()) + return; + + ranges.push_back(thread_local_region); +} + +// Save off the link map for core files. +static void AddLinkMapSections(Process &process, + std::vector<MemoryRegionInfo> &ranges) { + ModuleList &module_list = process.GetTarget().GetImages(); + Target *target = &process.GetTarget(); + for (size_t idx = 0; idx < module_list.GetSize(); idx++) { + ModuleSP module_sp = module_list.GetModuleAtIndex(idx); + if (!module_sp) + continue; + + ObjectFile *obj = module_sp->GetObjectFile(); + if (!obj) + continue; + Address addr = obj->GetImageInfoAddress(target); + addr_t load_addr = addr.GetLoadAddress(target); + if (load_addr == LLDB_INVALID_ADDRESS) + continue; + + MemoryRegionInfo link_map_section; + Status err = process.GetMemoryRegionInfo(load_addr, link_map_section); + if (err.Fail()) + continue; + + ranges.push_back(link_map_section); + } +} + +void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector<lldb_private::MemoryRegionInfo> &ranges, + llvm::function_ref<bool(const lldb_private::Thread &)> + save_thread_predicate) { + ThreadList &thread_list = process.GetThreadList(); + for (size_t idx = 0; idx < thread_list.GetSize(); idx++) { + ThreadSP thread_sp = thread_list.GetThreadAtIndex(idx); + if (!thread_sp) + continue; + + if (!save_thread_predicate(*thread_sp)) + continue; + + AddThreadLocalMemoryRegions(process, thread_sp, ranges); + } + + AddLinkMapSections(process, ranges); +} diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h index 4c92335602cdf4..bde334aaca40b4 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -60,6 +60,12 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader { lldb::addr_t base_addr, bool base_addr_is_offset) override; + void CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector<lldb_private::MemoryRegionInfo> &ranges, + llvm::function_ref<bool(const lldb_private::Thread &)> + save_thread_predicate) override; + protected: /// Runtime linker rendezvous structure. DYLDRendezvous m_rendezvous; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 32ffba763c08e3..5ea3db23f114c4 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -21,11 +21,13 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Interpreter/OptionArgParser.h" #include "lldb/Interpreter/OptionGroupBoolean.h" +#include "lldb/Target/DynamicLoader.h" #include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" #include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/DataBufferHeap.h" #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" @@ -34,6 +36,7 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Threading.h" +#include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h" #include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h" #include "Plugins/Process/Utility/StopInfoMachException.h" @@ -333,6 +336,16 @@ ArchSpec ProcessMinidump::GetArchitecture() { return ArchSpec(triple); } +DataExtractor ProcessMinidump::GetAuxvData() { + std::optional<llvm::ArrayRef<uint8_t>> auxv = + m_minidump_parser->GetStream(StreamType::LinuxAuxv); + if (!auxv) + return DataExtractor(); + + return DataExtractor(auxv->data(), auxv->size(), GetByteOrder(), + GetAddressByteSize(), GetAddressByteSize()); +} + void ProcessMinidump::BuildMemoryRegions() { if (m_memory_regions) return; @@ -534,7 +547,12 @@ void ProcessMinidump::ReadModuleList() { module_sp = Module::CreateModuleFromObjectFile<ObjectFilePlaceholder>( module_spec, load_addr, load_size); - GetTarget().GetImages().Append(module_sp, true /* notify */); + // If we haven't loaded a main executable yet, set the first module to be + // main executable + if (!GetTarget().GetExecutableModule()) + GetTarget().SetExecutableModule(module_sp); + else + GetTarget().GetImages().Append(module_sp, true /* notify */); } bool load_addr_changed = false; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index f2ea0a2b61d14e..3d235670a33abc 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -53,12 +53,11 @@ class ProcessMinidump : public PostMortemProcess { Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override { return nullptr; } + // Returns AUXV structure found in the core file + lldb_private::DataExtractor GetAuxvData() override; llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } - SystemRuntime *GetSystemRuntime() override { return nullptr; } - Status DoDestroy() override; void RefreshStateAfterStop() override; diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp index e879c493156593..f305d1b7031d82 100644 --- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp +++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp @@ -44,6 +44,17 @@ static void writeRegister(const void *reg_src, uint8_t *context, memcpy(reg_dest.data(), reg_src, reg_dest.size()); } +// TODO: Fix the registers in this file! +// writeRegister checks x86_64 registers without base registers. This causes +// an overlap in the register enum values. So we were truncating fs_base. +// We should standardize to the x86_64_with_base registers. +static void writeBaseRegister(const void *reg_src, uint8_t *context, + const RegisterInfo ®) { + auto bytes = reg.mutable_data(context); + llvm::MutableArrayRef<uint8_t> reg_dest = bytes.take_front(8); + memcpy(reg_dest.data(), reg_src, reg_dest.size()); +} + lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64( llvm::ArrayRef<uint8_t> source_data, RegisterInfoInterface *target_reg_interface) { @@ -105,11 +116,12 @@ lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64( writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]); } + // See comment on base regsiter if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) { - writeRegister(&context->fs_base, result_base, - reg_info[x86_64_with_base::lldb_fs_base]); - writeRegister(&context->gs_base, result_base, - reg_info[x86_64_with_base::lldb_gs_base]); + writeBaseRegister(&context->fs_base, result_base, + reg_info[x86_64_with_base::lldb_fs_base]); + writeBaseRegister(&context->gs_base, result_base, + reg_info[x86_64_with_base::lldb_gs_base]); } // TODO parse the floating point registers diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index aca08972811470..c009d17d3ba507 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6528,6 +6528,29 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } +static void SaveDynamicLoaderSections(Process &process, + const SaveCoreOptions &options, + CoreFileMemoryRanges &ranges, + std::set<addr_t> &stack_ends) { + DynamicLoader *dyld = process.GetDynamicLoader(); + if (!dyld) + return; + + std::vector<MemoryRegionInfo> dynamic_loader_mem_regions; + std::function<bool(const lldb_private::Thread &)> save_thread_predicate = + [&](const lldb_private::Thread &t) -> bool { + return options.ShouldThreadBeSaved(t.GetID()); + }; + dyld->CalculateDynamicSaveCoreRanges(process, dynamic_loader_mem_regions, + save_thread_predicate); + for (const auto ®ion : dynamic_loader_mem_regions) { + // The Dynamic Loader can give us regions that could include a truncated + // stack + if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0) + AddRegion(region, true, ranges); + } +} + static void SaveOffRegionsWithStackPointers(Process &process, const SaveCoreOptions &core_options, const MemoryRegionInfos ®ions, @@ -6559,11 +6582,13 @@ static void SaveOffRegionsWithStackPointers(Process &process, // off in other calls sp_region.GetRange().SetRangeBase(stack_head); sp_region.GetRange().SetByteSize(stack_size); - stack_ends.insert(sp_region.GetRange().GetRangeEnd()); + const addr_t range_end = sp_region.GetRange().GetRangeEnd(); + stack_ends.insert(range_end); // This will return true if the threadlist the user specified is empty, // or contains the thread id from thread_sp. - if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) + if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) { AddRegion(sp_region, try_dirty_pages, ranges); + } } } } @@ -6672,9 +6697,14 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, std::set<addr_t> stack_ends; // For fully custom set ups, we don't want to even look at threads if there // are no threads specified. - if (core_style != lldb::eSaveCoreCustomOnly || options.HasSpecifiedThreads()) + if (core_style != lldb::eSaveCoreCustomOnly || + options.HasSpecifiedThreads()) { SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends); + // Save off the dynamic loader sections, so if we are on an architecture + // that supports Thread Locals, that we include those as well. + SaveDynamicLoaderSections(*this, options, ranges, stack_ends); + } switch (core_style) { case eSaveCoreUnspecified: diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py index 03cc415924e0bb..4818dde4f3b838 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -523,8 +523,10 @@ def minidump_deleted_on_save_failure(self): finally: self.assertTrue(self.dbg.DeleteTarget(target)) - def minidump_deterministic_ diff erence(self): - """Test that verifies that two minidumps produced are identical.""" + @skipUnlessPlatform(["linux"]) + @skipUnlessArch("x86_64") + def minidump_saves_fs_base_region(self): + """Test that verifies the minidump file saves region for fs_base""" self.build() exe = self.getBuildArtifact("a.out") @@ -534,6 +536,45 @@ def minidump_deterministic_ diff erence(self): None, None, self.get_process_working_directory() ) self.assertState(process.GetState(), lldb.eStateStopped) + thread = process.GetThreadAtIndex(0) + custom_file = self.getBuildArtifact("core.reg_region.dmp") + options = lldb.SBSaveCoreOptions() + options.SetOutputFile(lldb.SBFileSpec(custom_file)) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreCustomOnly) + options.AddThread(thread) + error = process.SaveCore(options) + self.assertTrue(error.Success()) + + registers = thread.GetFrameAtIndex(0).GetRegisters() + fs_base = registers.GetFirstValueByName("fs_base").GetValueAsUnsigned() + self.assertTrue(fs_base != 0) + core_target = self.dbg.CreateTarget(None) + core_proc = core_target.LoadCore(one_region_file) + core_region_list = core_proc.GetMemoryRegions() + live_region_list = process.GetMemoryRegions() + live_region = lldb.SBMemoryRegionInfo() + live_region_list.GetMemoryRegionForAddress(fs_base, live_region) + core_region = lldb.SBMemoryRegionInfo() + error = core_region_list.GetMemoryRegionForAddress(fs_base, core_region) + self.assertTrue(error.Success()) + self.assertEqual(live_region, core_region) + + finally: + self.assertTrue(self.dbg.DeleteTarget(target)) + self.assertTrue(self.dbg.DeleteTarget(core_target)) + if os.path.isfile(custom_file): + os.unlink(custom_file) + + def minidump_deterministic_ diff erence(self): + """Test that verifies that two minidumps produced are identical.""" + self.build() + exe = self.getBuildArtifact("a.out") + try: + target = self.dbg.CreateTarget(exe) + process = target.LaunchSimple( + None, None, self.get_process_working_directory() + ) core_styles = [ lldb.eSaveCoreStackOnly, @@ -562,6 +603,36 @@ def minidump_deterministic_ diff erence(self): self.assertEqual(file_one, file_two) self.assertTrue(os.unlink(spec_one.GetFileName())) self.assertTrue(os.unlink(spec_two.GetFileName())) - finally: self.assertTrue(self.dbg.DeleteTarget(target)) + + @skipUnlessPlatform(["linux"]) + @skipUnlessArch("x86_64") + def minidump_saves_fs_base_region(self): + self.build() + exe = self.getBuildArtifact("a.out") + try: + target = self.dbg.CreateTarget(exe) + process = target.LaunchSimple( + None, None, self.get_process_working_directory() + ) + self.assertState(process.GetState(), lldb.eStateStopped) + thread = process.GetThreadAtIndex(0) + tls_file = self.getBuildArtifact("core.tls.dmp") + options = lldb.SBSaveCoreOptions() + options.SetOutputFile(lldb.SBFileSpec(tls_file)) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreCustomOnly) + options.AddThread(thread) + error = process.SaveCore(options) + self.assertTrue(error.Success()) + core_target = self.dbg.CreateTarget(None) + core_proc = core_target.LoadCore(tls_file) + frame = core_proc.GetThreadAtIndex(0).GetFrameAtIndex(0) + tls_val = frame.FindValue("lf") + self.assertEqual(tls_val.GetValueAsUnsigned(), 42) + + except: + self.assertTrue(self.dbg.DeleteTarget(target)) + if os.path.isfile(tls_file): + os.unlink(tls_file) diff --git a/lldb/test/API/functionalities/process_save_core_minidump/main.cpp b/lldb/test/API/functionalities/process_save_core_minidump/main.cpp index fa34a371f20647..15daa68e9a648c 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/main.cpp +++ b/lldb/test/API/functionalities/process_save_core_minidump/main.cpp @@ -1,6 +1,7 @@ #include <cassert> #include <iostream> #include <thread> +thread_local size_t lf = 42; void g() { assert(false); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
