From a5e55c787b0689c991a11ec936ce609fefca68f5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 16 Sep 2024 13:02:59 -0700 Subject: [PATCH 01/17] Add Process code to get the fs_base region and the .tdata sections. Plus a test --- lldb/source/Target/Process.cpp | 67 ++++++++++++++++++- .../TestProcessSaveCoreMinidump.py | 37 +++++++++- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index ff6a2f59eba35..1f573fc687798 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6539,6 +6539,63 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } +static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileMemoryRanges &ranges, lldb::addr_t range_end) { + 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 reg_value; + bool success = reg_ctx->ReadRegister(reg_info, reg_value); + if (!success) + return; + + const uint64_t fail_value = UINT64_MAX; + bool readSuccess = true; + const lldb::addr_t reg_value_addr = reg_value.GetAsUInt64(fail_value, &readSuccess); + if (!readSuccess || reg_value_addr == fail_value) + return; + + MemoryRegionInfo register_region; + Status err = process.GetMemoryRegionInfo(reg_value_addr, register_region); + if (err.Fail()) + return; + + // We already saved off this truncated stack range. + if (register_region.GetRange().GetRangeEnd() == range_end) + return; + + // We don't need to worry about duplication because the CoreFileMemoryRanges + // will unique them + AddRegion(register_region, true, ranges); +} + +static void AddModuleThreadLocalSections(Process &process, ThreadSP &thread_sp, CoreFileMemoryRanges &ranges, lldb::addr_t range_end) { + ModuleList &module_list = process.GetTarget().GetImages(); + for (size_t idx = 0; idx < module_list.GetSize(); idx++) { + ModuleSP module_sp = module_list.GetModuleAtIndex(idx); + if (!module_sp) + continue; + // We want the entire section, so the offset is 0. + const lldb::addr_t tls_storage_addr = thread_sp->GetThreadLocalData(module_sp, 0); + if (tls_storage_addr == LLDB_INVALID_ADDRESS) + continue; + MemoryRegionInfo tls_storage_region; + Status err = process.GetMemoryRegionInfo(tls_storage_addr, tls_storage_region); + if (err.Fail()) + continue; + + // We already saved off this truncated stack range. + if (tls_storage_region.GetRange().GetRangeEnd() == range_end) + continue; + + AddRegion(tls_storage_region, true, ranges); + } +} + static void SaveOffRegionsWithStackPointers(Process &process, const SaveCoreOptions &core_options, const MemoryRegionInfos ®ions, @@ -6570,11 +6627,17 @@ 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); + // Add the register section if x86_64 and add the module tls data + // only if the range isn't the same as this truncated stack range. + AddRegisterSections(process, thread_sp, ranges, range_end); + AddModuleThreadLocalSections(process, thread_sp, ranges, range_end); + } } } } 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 03cc415924e0b..170ed3da8b2c2 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_difference(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,37 @@ def minidump_deterministic_difference(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() + 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_difference(self): + """Test that verifies that two minidumps produced are identical.""" core_styles = [ lldb.eSaveCoreStackOnly, From f9c834d953a4617b1964e642c220db7fa4250a64 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 16 Sep 2024 13:46:49 -0700 Subject: [PATCH 02/17] Add POSIX dyld to processminidump --- lldb/source/Expression/DWARFExpression.cpp | 2 ++ .../Plugins/Process/minidump/ProcessMinidump.cpp | 15 +++++++++++++++ .../Plugins/Process/minidump/ProcessMinidump.h | 3 ++- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 97bcd4f7eec26..e4fdcab415826 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2166,6 +2166,8 @@ llvm::Expected DWARFExpression::Evaluate( // pushes it on the stack. case DW_OP_form_tls_address: case DW_OP_GNU_push_tls_address: { + bool debug = true; + while (debug) {} if (stack.size() < 1) { if (op == DW_OP_form_tls_address) return llvm::createStringError( diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 32ffba763c08e..42cc4abefa3e6 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -21,6 +21,7 @@ #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" @@ -35,6 +36,7 @@ #include "llvm/Support/Threading.h" #include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h" +#include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h" #include "Plugins/Process/Utility/StopInfoMachException.h" #include @@ -333,6 +335,19 @@ ArchSpec ProcessMinidump::GetArchitecture() { return ArchSpec(triple); } +DynamicLoader* ProcessMinidump::GetDynamicLoader() { + if (m_dyld_up && m_dyld_up.get()) + return m_dyld_up.get(); + + ArchSpec arch = GetArchitecture(); + if (arch.GetTriple().getOS() == llvm::Triple::Linux) { + m_dyld_up.reset(DynamicLoader::FindPlugin( + this, DynamicLoaderPOSIXDYLD::GetPluginNameStatic())); + } + + return m_dyld_up.get(); +} + void ProcessMinidump::BuildMemoryRegions() { if (m_memory_regions) return; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index f2ea0a2b61d14..4e7ce0da5e081 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -50,10 +50,11 @@ class ProcessMinidump : public PostMortemProcess { bool plugin_specified_by_name) override; CommandObject *GetPluginCommandObject() override; + Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override { return nullptr; } + DynamicLoader *GetDynamicLoader() override; llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } From 9ec6e74b69dfd1e55e64ab57f10c1c83641601fe Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 19 Sep 2024 14:45:33 -0700 Subject: [PATCH 03/17] Change DYLD To support TLS for minidump --- lldb/source/Expression/DWARFExpression.cpp | 2 -- .../Process/minidump/ProcessMinidump.cpp | 16 ++++++----- .../Process/minidump/ProcessMinidump.h | 5 ++-- lldb/source/Target/Process.cpp | 28 ++++++++++++------- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index e4fdcab415826..97bcd4f7eec26 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2166,8 +2166,6 @@ llvm::Expected DWARFExpression::Evaluate( // pushes it on the stack. case DW_OP_form_tls_address: case DW_OP_GNU_push_tls_address: { - bool debug = true; - while (debug) {} if (stack.size() < 1) { if (op == DW_OP_form_tls_address) return llvm::createStringError( diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 42cc4abefa3e6..4a1d95bb21d79 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -336,18 +336,20 @@ ArchSpec ProcessMinidump::GetArchitecture() { } DynamicLoader* ProcessMinidump::GetDynamicLoader() { - if (m_dyld_up && m_dyld_up.get()) - return m_dyld_up.get(); - - ArchSpec arch = GetArchitecture(); - if (arch.GetTriple().getOS() == llvm::Triple::Linux) { + if (m_dyld_up.get() == nullptr) m_dyld_up.reset(DynamicLoader::FindPlugin( this, DynamicLoaderPOSIXDYLD::GetPluginNameStatic())); - } - return m_dyld_up.get(); } +DataExtractor ProcessMinidump::GetAuxvData() { + std::optional> auxv = m_minidump_parser->GetStream(StreamType::LinuxAuxv); + if (!auxv) + return DataExtractor(); + + return DataExtractor(auxv->data(), auxv->size(), ByteOrder::eByteOrderLittle, GetAddressByteSize(), GetAddressByteSize()); +} + void ProcessMinidump::BuildMemoryRegions() { if (m_memory_regions) return; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index 4e7ce0da5e081..9e1b6f7dcd12d 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -54,12 +54,13 @@ class ProcessMinidump : public PostMortemProcess { Status DoLoadCore() override; + // Returns AUXV structure found in the core file + lldb_private::DataExtractor GetAuxvData() override; + DynamicLoader *GetDynamicLoader() override; llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } - SystemRuntime *GetSystemRuntime() override { return nullptr; } - Status DoDestroy() override; void RefreshStateAfterStop() override; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 1f573fc687798..acb7e18364fec 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6554,7 +6554,7 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileM return; const uint64_t fail_value = UINT64_MAX; - bool readSuccess = true; + bool readSuccess = false; const lldb::addr_t reg_value_addr = reg_value.GetAsUInt64(fail_value, &readSuccess); if (!readSuccess || reg_value_addr == fail_value) return; @@ -6573,23 +6573,29 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileM AddRegion(register_region, true, ranges); } -static void AddModuleThreadLocalSections(Process &process, ThreadSP &thread_sp, CoreFileMemoryRanges &ranges, lldb::addr_t range_end) { +static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, std::set &stack_ends) { 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; - // We want the entire section, so the offset is 0. - const lldb::addr_t tls_storage_addr = thread_sp->GetThreadLocalData(module_sp, 0); - if (tls_storage_addr == LLDB_INVALID_ADDRESS) + + 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 tls_storage_region; - Status err = process.GetMemoryRegionInfo(tls_storage_addr, tls_storage_region); + Status err = process.GetMemoryRegionInfo(load_addr, tls_storage_region); if (err.Fail()) continue; // We already saved off this truncated stack range. - if (tls_storage_region.GetRange().GetRangeEnd() == range_end) + if (stack_ends.count(tls_storage_region.GetRange().GetRangeEnd()) > 0) continue; AddRegion(tls_storage_region, true, ranges); @@ -6636,7 +6642,6 @@ static void SaveOffRegionsWithStackPointers(Process &process, // Add the register section if x86_64 and add the module tls data // only if the range isn't the same as this truncated stack range. AddRegisterSections(process, thread_sp, ranges, range_end); - AddModuleThreadLocalSections(process, thread_sp, ranges, range_end); } } } @@ -6746,9 +6751,12 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, std::set 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); + stack_ends); + AddModuleSections(*this, ranges, stack_ends); + } + switch (core_style) { case eSaveCoreUnspecified: From 7861f9f9456e6deaf875270f40ad2847b7cfb5d7 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Fri, 20 Sep 2024 13:49:27 -0700 Subject: [PATCH 04/17] Get TLS variables to work for minidump, add a test that the value is accessable correctly --- lldb/source/Core/DynamicLoader.cpp | 3 +- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 22 ++++++++++++- .../RegisterContextMinidump_x86_64.cpp | 16 ++++++++-- .../TestProcessSaveCoreMinidump.py | 32 +++++++++++++++++++ .../process_save_core_minidump/main.cpp | 2 ++ 5 files changed, 70 insertions(+), 5 deletions(-) diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 7758a87403b5a..de981f1679765 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -355,7 +355,7 @@ int64_t DynamicLoader::ReadUnsignedIntWithSizeInBytes(addr_t addr, return (int64_t)value; } -addr_t DynamicLoader::ReadPointer(addr_t addr) { +addr_t DynamicLoader:: ReadPointer(addr_t addr) { Status error; addr_t value = m_process->ReadPointerFromMemory(addr, error); if (error.Fail()) @@ -369,4 +369,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 b9c0e174c3be6..8185e0b56e626 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -108,8 +108,28 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { // if we dont have a load address we cant re-base bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS; +<<<<<<< HEAD +======= + // if we have a valid executable + if (executable_sp.get()) { + lldb_private::ObjectFile *obj = executable_sp->GetObjectFile(); + if (obj) { + // don't rebase if the module already has a load address + Target &target = m_process->GetTarget(); + Address addr = obj->GetImageInfoAddress(&target); + addr_t load_addr = addr.GetLoadAddress(&target); + if (load_addr != LLDB_INVALID_ADDRESS) { + rebase_exec = false; + } + } + } else { + // no executable, nothing to re-base + rebase_exec = false; + } + +>>>>>>> 7aae643a0970 (Get TLS variables to work for minidump, add a test that the value is accessable correctly) // if the target executable should be re-based - if (rebase_exec) { + if (rebase_exec || IsCoreFile()) { ModuleList module_list; module_list.Append(executable_sp); diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp index e879c49315659..03acd71f0349e 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 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 source_data, RegisterInfoInterface *target_reg_interface) { @@ -105,10 +116,11 @@ 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, + writeBaseRegister(&context->fs_base, result_base, reg_info[x86_64_with_base::lldb_fs_base]); - writeRegister(&context->gs_base, result_base, + writeBaseRegister(&context->gs_base, result_base, reg_info[x86_64_with_base::lldb_gs_base]); } 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 170ed3da8b2c2..0c1402571c922 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -548,6 +548,7 @@ def minidump_saves_fs_base_region(self): 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() @@ -598,3 +599,34 @@ def minidump_deterministic_difference(self): 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 fa34a371f2064..11804f1f50c52 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 #include #include +thread_local size_t lf = 42; void g() { assert(false); } @@ -16,6 +17,7 @@ size_t h() { return sum; } + int main() { std::thread t1(f); From c76f7b91beb8e17d92ca46d210bc025e46dfd3aa Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Fri, 20 Sep 2024 13:59:47 -0700 Subject: [PATCH 05/17] Cleanup and run formatters --- lldb/source/Core/DynamicLoader.cpp | 3 ++- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 3 +-- .../Process/minidump/ProcessMinidump.cpp | 10 ++++--- .../Process/minidump/ProcessMinidump.h | 1 - .../RegisterContextMinidump_x86_64.cpp | 6 ++--- lldb/source/Target/Process.cpp | 26 +++++++++++-------- .../TestProcessSaveCoreMinidump.py | 6 ++--- .../process_save_core_minidump/main.cpp | 1 - 8 files changed, 30 insertions(+), 26 deletions(-) diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index de981f1679765..7758a87403b5a 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -355,7 +355,7 @@ int64_t DynamicLoader::ReadUnsignedIntWithSizeInBytes(addr_t addr, return (int64_t)value; } -addr_t DynamicLoader:: ReadPointer(addr_t addr) { +addr_t DynamicLoader::ReadPointer(addr_t addr) { Status error; addr_t value = m_process->ReadPointerFromMemory(addr, error); if (error.Fail()) @@ -369,3 +369,4 @@ 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 8185e0b56e626..4ee62275c72d6 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -118,9 +118,8 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { Target &target = m_process->GetTarget(); Address addr = obj->GetImageInfoAddress(&target); addr_t load_addr = addr.GetLoadAddress(&target); - if (load_addr != LLDB_INVALID_ADDRESS) { + if (load_addr != LLDB_INVALID_ADDRESS) rebase_exec = false; - } } } else { // no executable, nothing to re-base diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 4a1d95bb21d79..2a8f9fe9b9d93 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -35,8 +35,8 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Threading.h" -#include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h" #include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h" +#include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h" #include "Plugins/Process/Utility/StopInfoMachException.h" #include @@ -335,7 +335,7 @@ ArchSpec ProcessMinidump::GetArchitecture() { return ArchSpec(triple); } -DynamicLoader* ProcessMinidump::GetDynamicLoader() { +DynamicLoader *ProcessMinidump::GetDynamicLoader() { if (m_dyld_up.get() == nullptr) m_dyld_up.reset(DynamicLoader::FindPlugin( this, DynamicLoaderPOSIXDYLD::GetPluginNameStatic())); @@ -343,11 +343,13 @@ DynamicLoader* ProcessMinidump::GetDynamicLoader() { } DataExtractor ProcessMinidump::GetAuxvData() { - std::optional> auxv = m_minidump_parser->GetStream(StreamType::LinuxAuxv); + std::optional> auxv = + m_minidump_parser->GetStream(StreamType::LinuxAuxv); if (!auxv) return DataExtractor(); - return DataExtractor(auxv->data(), auxv->size(), ByteOrder::eByteOrderLittle, GetAddressByteSize(), GetAddressByteSize()); + return DataExtractor(auxv->data(), auxv->size(), ByteOrder::eByteOrderLittle, + GetAddressByteSize(), GetAddressByteSize()); } void ProcessMinidump::BuildMemoryRegions() { diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index 9e1b6f7dcd12d..332e2637cc7ee 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -50,7 +50,6 @@ class ProcessMinidump : public PostMortemProcess { bool plugin_specified_by_name) override; CommandObject *GetPluginCommandObject() override; - Status DoLoadCore() override; diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp index 03acd71f0349e..f305d1b7031d8 100644 --- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp +++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp @@ -46,7 +46,7 @@ static void writeRegister(const void *reg_src, uint8_t *context, // 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. +// 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 ®) { @@ -119,9 +119,9 @@ lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64( // See comment on base regsiter if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) { writeBaseRegister(&context->fs_base, result_base, - reg_info[x86_64_with_base::lldb_fs_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]); + 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 acb7e18364fec..73a62b52a1da1 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6539,12 +6539,15 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } -static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileMemoryRanges &ranges, lldb::addr_t range_end) { +static void AddRegisterSections(Process &process, ThreadSP &thread_sp, + CoreFileMemoryRanges &ranges, + lldb::addr_t range_end) { 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); + const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo( + lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_TP); if (!reg_info) return; @@ -6555,10 +6558,11 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileM const uint64_t fail_value = UINT64_MAX; bool readSuccess = false; - const lldb::addr_t reg_value_addr = reg_value.GetAsUInt64(fail_value, &readSuccess); + const lldb::addr_t reg_value_addr = + reg_value.GetAsUInt64(fail_value, &readSuccess); if (!readSuccess || reg_value_addr == fail_value) return; - + MemoryRegionInfo register_region; Status err = process.GetMemoryRegionInfo(reg_value_addr, register_region); if (err.Fail()) @@ -6573,9 +6577,10 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileM AddRegion(register_region, true, ranges); } -static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, std::set &stack_ends) { +static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, + std::set &stack_ends) { ModuleList &module_list = process.GetTarget().GetImages(); - Target* target = &process.GetTarget(); + Target *target = &process.GetTarget(); for (size_t idx = 0; idx < module_list.GetSize(); idx++) { ModuleSP module_sp = module_list.GetModuleAtIndex(idx); if (!module_sp) @@ -6639,8 +6644,6 @@ static void SaveOffRegionsWithStackPointers(Process &process, // or contains the thread id from thread_sp. if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) { AddRegion(sp_region, try_dirty_pages, ranges); - // Add the register section if x86_64 and add the module tls data - // only if the range isn't the same as this truncated stack range. AddRegisterSections(process, thread_sp, ranges, range_end); } } @@ -6751,13 +6754,14 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, std::set 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); + stack_ends); + // Save off the load sections for the TLS data. AddModuleSections(*this, ranges, stack_ends); } - switch (core_style) { case eSaveCoreUnspecified: case eSaveCoreCustomOnly: 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 0c1402571c922..9fc41141a97ef 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -557,9 +557,9 @@ def minidump_saves_fs_base_region(self): 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.assertTrue(error.Success()) self.assertEqual(live_region, core_region) - + finally: self.assertTrue(self.dbg.DeleteTarget(target)) self.assertTrue(self.dbg.DeleteTarget(core_target)) @@ -623,7 +623,7 @@ def minidump_saves_fs_base_region(self): 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') + tls_val = frame.FindValue("lf") self.assertEqual(tls_val.GetValueAsUnsigned(), 42) except: 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 11804f1f50c52..15daa68e9a648 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/main.cpp +++ b/lldb/test/API/functionalities/process_save_core_minidump/main.cpp @@ -17,7 +17,6 @@ size_t h() { return sum; } - int main() { std::thread t1(f); From fb792f8615f5fc05ceef54980a739c647b3bf148 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 1 Oct 2024 14:21:39 -0700 Subject: [PATCH 06/17] remove path that sets rebase_exec to false in posixDYLD --- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 4ee62275c72d6..b56fbfb1faf88 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -108,25 +108,6 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { // if we dont have a load address we cant re-base bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS; -<<<<<<< HEAD -======= - // if we have a valid executable - if (executable_sp.get()) { - lldb_private::ObjectFile *obj = executable_sp->GetObjectFile(); - if (obj) { - // don't rebase if the module already has a load address - Target &target = m_process->GetTarget(); - Address addr = obj->GetImageInfoAddress(&target); - addr_t load_addr = addr.GetLoadAddress(&target); - if (load_addr != LLDB_INVALID_ADDRESS) - rebase_exec = false; - } - } else { - // no executable, nothing to re-base - rebase_exec = false; - } - ->>>>>>> 7aae643a0970 (Get TLS variables to work for minidump, add a test that the value is accessable correctly) // if the target executable should be re-based if (rebase_exec || IsCoreFile()) { ModuleList module_list; From 49c85242ce9ca5a9c1edccf07f9b469a4b91b5b3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 1 Oct 2024 15:33:24 -0700 Subject: [PATCH 07/17] Readd linkmap explicitly because we need it for the dynamic loader to figure out the TLS sections --- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 2 +- .../Plugins/Process/minidump/ProcessMinidump.cpp | 3 ++- lldb/source/Target/Process.cpp | 15 +++++---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index b56fbfb1faf88..b9c0e174c3be6 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -109,7 +109,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS; // if the target executable should be re-based - if (rebase_exec || IsCoreFile()) { + if (rebase_exec) { ModuleList module_list; module_list.Append(executable_sp); diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 2a8f9fe9b9d93..f2f87a8ab2b76 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -336,7 +336,8 @@ ArchSpec ProcessMinidump::GetArchitecture() { } DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_up.get() == nullptr) + if (m_dyld_up.get() == nullptr + && GetArchitecture().GetTriple().isOSLinux()) m_dyld_up.reset(DynamicLoader::FindPlugin( this, DynamicLoaderPOSIXDYLD::GetPluginNameStatic())); return m_dyld_up.get(); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 73a62b52a1da1..7421f1c3a7d9f 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6539,7 +6539,7 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } -static void AddRegisterSections(Process &process, ThreadSP &thread_sp, +static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileMemoryRanges &ranges, lldb::addr_t range_end) { lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); @@ -6577,8 +6577,7 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, AddRegion(register_region, true, ranges); } -static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, - std::set &stack_ends) { +static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) { ModuleList &module_list = process.GetTarget().GetImages(); Target *target = &process.GetTarget(); for (size_t idx = 0; idx < module_list.GetSize(); idx++) { @@ -6599,10 +6598,6 @@ static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, if (err.Fail()) continue; - // We already saved off this truncated stack range. - if (stack_ends.count(tls_storage_region.GetRange().GetRangeEnd()) > 0) - continue; - AddRegion(tls_storage_region, true, ranges); } } @@ -6644,7 +6639,7 @@ static void SaveOffRegionsWithStackPointers(Process &process, // or contains the thread id from thread_sp. if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) { AddRegion(sp_region, try_dirty_pages, ranges); - AddRegisterSections(process, thread_sp, ranges, range_end); + AddSegmentRegisterSections(process, thread_sp, ranges, range_end); } } } @@ -6758,8 +6753,8 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, options.HasSpecifiedThreads()) { SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends); - // Save off the load sections for the TLS data. - AddModuleSections(*this, ranges, stack_ends); + // We need the link map for TLS data. + AddLinkMapSections(*this, ranges); } switch (core_style) { From 3678237477a50990e62dc63289db3c94cb5d61cc Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 2 Oct 2024 11:26:58 -0700 Subject: [PATCH 08/17] Fix tests --- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 1 - .../TestProcessSaveCoreMinidump.py | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index b9c0e174c3be6..d07469ef9cfe5 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -298,7 +298,6 @@ bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() { "Rendezvous breakpoint breakpoint id {0} for pid {1}" "is already set.", m_dyld_bid, - m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID); return true; } 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 9fc41141a97ef..4818dde4f3b83 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -567,7 +567,14 @@ def minidump_saves_fs_base_region(self): os.unlink(custom_file) def minidump_deterministic_difference(self): - """Test that verifies that two minidumps produced are identical.""" + """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, @@ -596,7 +603,6 @@ def minidump_deterministic_difference(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)) From 433851a99225a5a9bd8b832a3362f7faea379fd3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 3 Oct 2024 15:13:43 -0700 Subject: [PATCH 09/17] Fix test case when we have a processminidump created placeholder main executable --- lldb/source/Core/DynamicLoader.cpp | 7 +++++-- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 1 + .../Plugins/Process/minidump/ProcessMinidump.cpp | 16 ++++++---------- .../Plugins/Process/minidump/ProcessMinidump.h | 2 -- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 7758a87403b5a..47162da040376 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_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 differ. 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 d07469ef9cfe5..b9c0e174c3be6 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -298,6 +298,7 @@ bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() { "Rendezvous breakpoint breakpoint id {0} for pid {1}" "is already set.", m_dyld_bid, + m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID); return true; } diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index f2f87a8ab2b76..e66fd9f75d39a 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -27,6 +27,7 @@ #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" @@ -335,14 +336,6 @@ ArchSpec ProcessMinidump::GetArchitecture() { return ArchSpec(triple); } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_up.get() == nullptr - && GetArchitecture().GetTriple().isOSLinux()) - m_dyld_up.reset(DynamicLoader::FindPlugin( - this, DynamicLoaderPOSIXDYLD::GetPluginNameStatic())); - return m_dyld_up.get(); -} - DataExtractor ProcessMinidump::GetAuxvData() { std::optional> auxv = m_minidump_parser->GetStream(StreamType::LinuxAuxv); @@ -482,7 +475,6 @@ ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid, void ProcessMinidump::ReadModuleList() { std::vector filtered_modules = m_minidump_parser->GetFilteredModuleList(); - Log *log = GetLog(LLDBLog::DynamicLoader); for (auto module : filtered_modules) { @@ -551,9 +543,13 @@ void ProcessMinidump::ReadModuleList() { "Unable to locate the matching object file, creating a " "placeholder module for: {0}", name); - module_sp = Module::CreateModuleFromObjectFile( module_spec, load_addr, load_size); + // 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 */); } diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index 332e2637cc7ee..3d235670a33ab 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -56,8 +56,6 @@ class ProcessMinidump : public PostMortemProcess { // Returns AUXV structure found in the core file lldb_private::DataExtractor GetAuxvData() override; - DynamicLoader *GetDynamicLoader() override; - llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } Status DoDestroy() override; From 48c2703729c9c303f5693abc314734a24c1626f7 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 3 Oct 2024 15:14:01 -0700 Subject: [PATCH 10/17] Run GCF --- lldb/source/Core/DynamicLoader.cpp | 2 +- .../Plugins/Process/minidump/ProcessMinidump.cpp | 12 ++++++------ lldb/source/Target/Process.cpp | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 47162da040376..68d6ab0850853 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -83,7 +83,7 @@ ModuleSP DynamicLoader::GetTargetExecutable() { ModuleSpec module_spec(executable->GetFileSpec(), executable->GetArchitecture()); auto module_sp = std::make_shared(module_spec); - // If we're a coredump and we already have a main executable, we don't + // 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; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index e66fd9f75d39a..690362b6f26b1 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -545,12 +545,12 @@ void ProcessMinidump::ReadModuleList() { name); module_sp = Module::CreateModuleFromObjectFile( module_spec, load_addr, load_size); - // 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 */); + // 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/Target/Process.cpp b/lldb/source/Target/Process.cpp index 7421f1c3a7d9f..e421e5b6d0b98 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6540,8 +6540,8 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, } static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, - CoreFileMemoryRanges &ranges, - lldb::addr_t range_end) { + CoreFileMemoryRanges &ranges, + lldb::addr_t range_end) { lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); if (!reg_ctx) return; From 6134c0bf844e559d3ca049512239f0cc5a9da114 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 3 Oct 2024 15:24:18 -0700 Subject: [PATCH 11/17] Clean-up variable names in process.cpp --- lldb/source/Target/Process.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index e421e5b6d0b98..c4fa2dd4752cb 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6551,30 +6551,30 @@ static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, if (!reg_info) return; - lldb_private::RegisterValue reg_value; - bool success = reg_ctx->ReadRegister(reg_info, reg_value); + 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 = - reg_value.GetAsUInt64(fail_value, &readSuccess); + thread_local_register_value.GetAsUInt64(fail_value, &readSuccess); if (!readSuccess || reg_value_addr == fail_value) return; - MemoryRegionInfo register_region; - Status err = process.GetMemoryRegionInfo(reg_value_addr, register_region); + MemoryRegionInfo thread_local_region; + Status err = process.GetMemoryRegionInfo(reg_value_addr, thread_local_region); if (err.Fail()) return; // We already saved off this truncated stack range. - if (register_region.GetRange().GetRangeEnd() == range_end) + if (thread_local_region.GetRange().GetRangeEnd() == range_end) return; // We don't need to worry about duplication because the CoreFileMemoryRanges // will unique them - AddRegion(register_region, true, ranges); + AddRegion(thread_local_region, true, ranges); } static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) { @@ -6593,12 +6593,12 @@ static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) { if (load_addr == LLDB_INVALID_ADDRESS) continue; - MemoryRegionInfo tls_storage_region; - Status err = process.GetMemoryRegionInfo(load_addr, tls_storage_region); + MemoryRegionInfo link_map_section; + Status err = process.GetMemoryRegionInfo(load_addr, link_map_section); if (err.Fail()) continue; - AddRegion(tls_storage_region, true, ranges); + AddRegion(link_map_section, true, ranges); } } From 1d285630706422daf3a02384f457a7db1159b769 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 7 Oct 2024 10:49:38 -0700 Subject: [PATCH 12/17] Check for stack truncations in the link map code, surprisingly. --- lldb/source/Target/Process.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index c4fa2dd4752cb..33650f59a9a39 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6577,7 +6577,8 @@ static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, AddRegion(thread_local_region, true, ranges); } -static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) { +static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges, + std::set &stack_ends) { ModuleList &module_list = process.GetTarget().GetImages(); Target *target = &process.GetTarget(); for (size_t idx = 0; idx < module_list.GetSize(); idx++) { @@ -6598,6 +6599,11 @@ static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) { if (err.Fail()) continue; + // Sometimes, the link map section is included in one of the stack memory + // ranges. In that case, we already saved a truncated version of that range + if (stack_ends.count(link_map_section.GetRange().GetRangeEnd()) == 0) + continue; + AddRegion(link_map_section, true, ranges); } } @@ -6754,7 +6760,7 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends); // We need the link map for TLS data. - AddLinkMapSections(*this, ranges); + AddLinkMapSections(*this, ranges, stack_ends); } switch (core_style) { From c075079e78e0847baad6ea8a5add7e20abd6ddd0 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 8 Oct 2024 13:17:57 -0700 Subject: [PATCH 13/17] Move the TLS code to the DYLD --- lldb/include/lldb/Target/DynamicLoader.h | 8 ++ .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 75 +++++++++++++++++ .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 2 + lldb/source/Target/Process.cpp | 80 +++---------------- 4 files changed, 98 insertions(+), 67 deletions(-) diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 0629e2faae7e9..93bc606e9164f 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -18,6 +18,7 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" +#include "lldb/Target/CoreFileMemoryRanges.h" #include #include @@ -337,6 +338,13 @@ class DynamicLoader : public PluginInterface { return std::nullopt; } + /// Returns a list of memory ranges that should be saved in the core file, + /// specific for this dßynamic loader. + /// + /// By default, this returns an empty list, but for POSIX/ELF it will return + /// the link map, and the TLS data. + virtual void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector &ranges, std::function save_thread_predicate) {}; + protected: // Utility methods for derived classes diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index b9c0e174c3be6..9499fd595190a 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,77 @@ 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 AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, + std::vector &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 &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 &ranges, std::function 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.get())) + continue; + + AddSegmentRegisterSections(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 4c92335602cdf..23ec0b99d4f7b 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -60,6 +60,8 @@ 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 &ranges, std::function save_thread_predicate) override; + protected: /// Runtime linker rendezvous structure. DYLDRendezvous m_rendezvous; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 33650f59a9a39..f16c805c06d5f 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6539,72 +6539,18 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } -static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, - CoreFileMemoryRanges &ranges, - lldb::addr_t range_end) { - lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); - if (!reg_ctx) +static void SaveDynamicLoaderSections(Process &process, const SaveCoreOptions &options, CoreFileMemoryRanges &ranges, std::set &stack_ends) { + DynamicLoader *dyld = process.GetDynamicLoader(); + if (!dyld) 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; - - // We already saved off this truncated stack range. - if (thread_local_region.GetRange().GetRangeEnd() == range_end) - return; - - // We don't need to worry about duplication because the CoreFileMemoryRanges - // will unique them - AddRegion(thread_local_region, true, ranges); -} - -static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges, - std::set &stack_ends) { - 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; - - // Sometimes, the link map section is included in one of the stack memory - // ranges. In that case, we already saved a truncated version of that range - if (stack_ends.count(link_map_section.GetRange().GetRangeEnd()) == 0) - continue; - - AddRegion(link_map_section, true, ranges); + std::vector dynamic_loader_mem_regions; + std::function 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); } } @@ -6645,7 +6591,6 @@ static void SaveOffRegionsWithStackPointers(Process &process, // or contains the thread id from thread_sp. if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) { AddRegion(sp_region, try_dirty_pages, ranges); - AddSegmentRegisterSections(process, thread_sp, ranges, range_end); } } } @@ -6759,8 +6704,9 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, options.HasSpecifiedThreads()) { SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends); - // We need the link map for TLS data. - AddLinkMapSections(*this, 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) { From da84d9c0d10981950a0c6185fec0614a0521d7e4 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 8 Oct 2024 13:24:46 -0700 Subject: [PATCH 14/17] Move the TLS code to the DYLD --- lldb/include/lldb/Target/DynamicLoader.h | 10 +++++++--- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 8 ++++++-- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 6 +++++- lldb/source/Target/Process.cpp | 20 +++++++++++++------ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 93bc606e9164f..1ab8b1f2643fb 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" @@ -18,7 +19,6 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" -#include "lldb/Target/CoreFileMemoryRanges.h" #include #include @@ -338,12 +338,16 @@ class DynamicLoader : public PluginInterface { return std::nullopt; } - /// Returns a list of memory ranges that should be saved in the core file, + /// Returns a list of memory ranges that should be saved in the core file, /// specific for this dßynamic loader. /// /// By default, this returns an empty list, but for POSIX/ELF it will return /// the link map, and the TLS data. - virtual void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector &ranges, std::function save_thread_predicate) {}; + virtual void CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector &ranges, + std::function save_thread_predicate) { + }; protected: // Utility methods for derived classes diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 9499fd595190a..5a1dedd4b60ad 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -901,7 +901,8 @@ static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, } // Save off the link map for core files. -static void AddLinkMapSections(Process &process, std::vector &ranges) { +static void AddLinkMapSections(Process &process, + std::vector &ranges) { ModuleList &module_list = process.GetTarget().GetImages(); Target *target = &process.GetTarget(); for (size_t idx = 0; idx < module_list.GetSize(); idx++) { @@ -926,7 +927,10 @@ static void AddLinkMapSections(Process &process, std::vector & } } -void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector &ranges, std::function save_thread_predicate) { +void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector &ranges, + std::function 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); diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h index 23ec0b99d4f7b..2e7d0fdaa9df4 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -60,7 +60,11 @@ 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 &ranges, std::function save_thread_predicate) override; + void CalculateDynamicSaveCoreRanges( + lldb_private::Process &process, + std::vector &ranges, + std::function save_thread_predicate) + override; protected: /// Runtime linker rendezvous structure. diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index f16c805c06d5f..fd68372838821 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6539,16 +6539,24 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, CreateCoreFileMemoryRange(region)); } -static void SaveDynamicLoaderSections(Process &process, const SaveCoreOptions &options, CoreFileMemoryRanges &ranges, std::set &stack_ends) { +static void SaveDynamicLoaderSections(Process &process, + const SaveCoreOptions &options, + CoreFileMemoryRanges &ranges, + std::set &stack_ends) { DynamicLoader *dyld = process.GetDynamicLoader(); if (!dyld) return; std::vector dynamic_loader_mem_regions; - std::function save_thread_predicate = [&](const lldb_private::Thread &t) -> bool { return options.ShouldThreadBeSaved(t.GetID()); }; - dyld->CalculateDynamicSaveCoreRanges(process, dynamic_loader_mem_regions, save_thread_predicate); + std::function 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 + // 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); } @@ -6704,8 +6712,8 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, 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. + // 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); } From 83a8f44a74e0839f75ba182475bc10319ac74996 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 8 Oct 2024 17:09:57 -0700 Subject: [PATCH 15/17] Code review cleanup --- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 690362b6f26b1..5ea3db23f114c 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -342,7 +342,7 @@ DataExtractor ProcessMinidump::GetAuxvData() { if (!auxv) return DataExtractor(); - return DataExtractor(auxv->data(), auxv->size(), ByteOrder::eByteOrderLittle, + return DataExtractor(auxv->data(), auxv->size(), GetByteOrder(), GetAddressByteSize(), GetAddressByteSize()); } @@ -475,6 +475,7 @@ ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid, void ProcessMinidump::ReadModuleList() { std::vector filtered_modules = m_minidump_parser->GetFilteredModuleList(); + Log *log = GetLog(LLDBLog::DynamicLoader); for (auto module : filtered_modules) { @@ -543,6 +544,7 @@ void ProcessMinidump::ReadModuleList() { "Unable to locate the matching object file, creating a " "placeholder module for: {0}", name); + module_sp = Module::CreateModuleFromObjectFile( module_spec, load_addr, load_size); // If we haven't loaded a main executable yet, set the first module to be From 00471d2a0fd19224daa32bb6d7c8fbb6a37a933c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 10 Oct 2024 13:22:11 -0700 Subject: [PATCH 16/17] Code review feedback on function names and comment phrasing --- lldb/include/lldb/Target/DynamicLoader.h | 8 ++++---- .../DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 8 ++++---- .../DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 1ab8b1f2643fb..7065f615a755b 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -339,14 +339,14 @@ class DynamicLoader : public PluginInterface { } /// Returns a list of memory ranges that should be saved in the core file, - /// specific for this dßynamic loader. + /// specific for this dynamic loader. /// - /// By default, this returns an empty list, but for POSIX/ELF it will return - /// the link map, and the TLS data. + /// 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 &ranges, - std::function save_thread_predicate) { + llvm::function_ref save_thread_predicate) { }; protected: diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 5a1dedd4b60ad..d9d1afe5b31cf 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -869,7 +869,7 @@ bool DynamicLoaderPOSIXDYLD::IsCoreFile() const { } // For our ELF/POSIX builds save off the fs_base/gs_base regions -static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp, +static void AddThreadLocalMemoryRegions(Process &process, ThreadSP &thread_sp, std::vector &ranges) { lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); if (!reg_ctx) @@ -930,17 +930,17 @@ static void AddLinkMapSections(Process &process, void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges( lldb_private::Process &process, std::vector &ranges, - std::function save_thread_predicate) { + llvm::function_ref 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.get())) + if (!save_thread_predicate(*thread_sp)) continue; - AddSegmentRegisterSections(process, thread_sp, ranges); + 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 2e7d0fdaa9df4..086e94171826b 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -63,7 +63,7 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader { void CalculateDynamicSaveCoreRanges( lldb_private::Process &process, std::vector &ranges, - std::function save_thread_predicate) + llvm::function_ref save_thread_predicate) override; protected: From 77f48e640d4e0f1a06713d9f92a0f171f8f9d108 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 10 Oct 2024 13:30:51 -0700 Subject: [PATCH 17/17] Rebase and format --- lldb/include/lldb/Target/DynamicLoader.h | 4 ++-- .../DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 5 +++-- .../DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h index 7065f615a755b..75bb6cb6bb907 100644 --- a/lldb/include/lldb/Target/DynamicLoader.h +++ b/lldb/include/lldb/Target/DynamicLoader.h @@ -346,8 +346,8 @@ class DynamicLoader : public PluginInterface { virtual void CalculateDynamicSaveCoreRanges( lldb_private::Process &process, std::vector &ranges, - llvm::function_ref save_thread_predicate) { - }; + llvm::function_ref + save_thread_predicate) {}; protected: // Utility methods for derived classes diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index d9d1afe5b31cf..34aca50df0ac4 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -870,7 +870,7 @@ bool DynamicLoaderPOSIXDYLD::IsCoreFile() const { // For our ELF/POSIX builds save off the fs_base/gs_base regions static void AddThreadLocalMemoryRegions(Process &process, ThreadSP &thread_sp, - std::vector &ranges) { + std::vector &ranges) { lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext(); if (!reg_ctx) return; @@ -930,7 +930,8 @@ static void AddLinkMapSections(Process &process, void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges( lldb_private::Process &process, std::vector &ranges, - llvm::function_ref save_thread_predicate) { + llvm::function_ref + 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); diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h index 086e94171826b..bde334aaca40b 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h @@ -63,8 +63,8 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader { void CalculateDynamicSaveCoreRanges( lldb_private::Process &process, std::vector &ranges, - llvm::function_ref save_thread_predicate) - override; + llvm::function_ref + save_thread_predicate) override; protected: /// Runtime linker rendezvous structure.