https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/109477
>From f432d21587c32ca5e6dc96fc77fbc53a3ae5eeb8 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 16 Sep 2024 13:02:59 -0700 Subject: [PATCH 01/11] 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 aca08972811470..c66103b9443444 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6528,6 +6528,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, @@ -6559,11 +6616,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 03cc415924e0bb..170ed3da8b2c2d 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 fe3c0eb8ba6b44ccfa178ed6d73abd47bb929907 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Mon, 16 Sep 2024 13:46:49 -0700 Subject: [PATCH 02/11] 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 22d899f799d0fd..a8c55fdca6ba4a 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2168,6 +2168,8 @@ llvm::Expected<Value> 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 32ffba763c08e3..42cc4abefa3e68 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 <memory> @@ -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 f2ea0a2b61d14e..4e7ce0da5e081e 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 3f8693432d5a7f46937cc17edd13f3cb29b82da6 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 19 Sep 2024 14:45:33 -0700 Subject: [PATCH 03/11] Change DYLD To support TLS for minidump --- lldb/source/Expression/DWARFExpression.cpp | 2 -- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 5 +++- .../Process/minidump/ProcessMinidump.cpp | 16 ++++++----- .../Process/minidump/ProcessMinidump.h | 5 ++-- lldb/source/Target/Process.cpp | 28 ++++++++++++------- 5 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index a8c55fdca6ba4a..22d899f799d0fd 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -2168,8 +2168,6 @@ llvm::Expected<Value> 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/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 51e4b3e6728f23..4079afec0e558b 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -115,8 +115,11 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { // don't rebase if the module already has a load address Target &target = m_process->GetTarget(); Address addr = obj->GetImageInfoAddress(&target); - if (addr.GetLoadAddress(&target) != LLDB_INVALID_ADDRESS) + addr_t load_addr = addr.GetLoadAddress(&target); + if (load_addr != LLDB_INVALID_ADDRESS) { rebase_exec = false; + m_loaded_modules[executable_sp] = load_addr; + } } } 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 42cc4abefa3e68..4a1d95bb21d799 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<llvm::ArrayRef<uint8_t>> 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 4e7ce0da5e081e..9e1b6f7dcd12dd 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 c66103b9443444..70f2c962afc91c 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6543,7 +6543,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; @@ -6562,23 +6562,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<addr_t> &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); @@ -6625,7 +6631,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); } } } @@ -6735,9 +6740,12 @@ 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); + stack_ends); + AddModuleSections(*this, ranges, stack_ends); + } + switch (core_style) { case eSaveCoreUnspecified: >From 675110846645ffd07b162aa7975fc48ccd2c19f5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 20 Sep 2024 13:49:27 -0700 Subject: [PATCH 04/11] 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 | 3 +- .../RegisterContextMinidump_x86_64.cpp | 16 ++++++++-- .../TestProcessSaveCoreMinidump.py | 32 +++++++++++++++++++ .../process_save_core_minidump/main.cpp | 2 ++ 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 7758a87403b5a3..de981f1679765e 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 4079afec0e558b..c149b87ac5bffe 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -118,7 +118,6 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { addr_t load_addr = addr.GetLoadAddress(&target); if (load_addr != LLDB_INVALID_ADDRESS) { rebase_exec = false; - m_loaded_modules[executable_sp] = load_addr; } } } else { @@ -127,7 +126,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { } // 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 e879c493156593..03acd71f0349e9 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,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 170ed3da8b2c2d..0c1402571c9226 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 fa34a371f20647..11804f1f50c520 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); } @@ -16,6 +17,7 @@ size_t h() { return sum; } + int main() { std::thread t1(f); >From 3da761c49d9971ed6db7ff2ceb22fd6763b7e93a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 20 Sep 2024 13:59:47 -0700 Subject: [PATCH 05/11] 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 de981f1679765e..7758a87403b5a3 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 c149b87ac5bffe..38e9a61817ab55 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -116,9 +116,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 4a1d95bb21d799..2a8f9fe9b9d93d 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 <memory> @@ -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<llvm::ArrayRef<uint8_t>> auxv = m_minidump_parser->GetStream(StreamType::LinuxAuxv); + std::optional<llvm::ArrayRef<uint8_t>> 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 9e1b6f7dcd12dd..332e2637cc7ee7 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 03acd71f0349e9..f305d1b7031d82 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 70f2c962afc91c..8ddb2be409f855 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6528,12 +6528,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; @@ -6544,10 +6547,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()) @@ -6562,9 +6566,10 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, CoreFileM AddRegion(register_region, true, ranges); } -static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) { +static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, + std::set<addr_t> &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) @@ -6628,8 +6633,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); } } @@ -6740,13 +6743,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); + 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 0c1402571c9226..9fc41141a97efa 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 11804f1f50c520..15daa68e9a648c 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 f4c25e164872a0aa04686ed72b87a7255ef4c7f0 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 14:21:39 -0700 Subject: [PATCH 06/11] remove path that sets rebase_exec to false in posixDYLD --- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 38e9a61817ab55..34972cdc569a0e 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -108,24 +108,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { // if we dont have a load address we cant re-base bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS; - // 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; - } - - // if the target executable should be re-based - if (rebase_exec || IsCoreFile()) { + if (rebase_exec) { ModuleList module_list; module_list.Append(executable_sp); >From 8c42dd38dce00ec0a0de025ca3430751eb470630 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 1 Oct 2024 15:33:24 -0700 Subject: [PATCH 07/11] Readd linkmap explicitly because we need it for the dynamic loader to figure out the TLS sections --- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 1 + .../Plugins/Process/minidump/ProcessMinidump.cpp | 3 ++- lldb/source/Target/Process.cpp | 15 +++++---------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 34972cdc569a0e..b9c0e174c3be68 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -108,6 +108,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() { // if we dont have a load address we cant re-base bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS; + // if the target executable should be re-based if (rebase_exec) { ModuleList module_list; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 2a8f9fe9b9d93d..f2f87a8ab2b764 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 8ddb2be409f855..6dfb0aedb7fac7 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6528,7 +6528,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(); @@ -6566,8 +6566,7 @@ static void AddRegisterSections(Process &process, ThreadSP &thread_sp, AddRegion(register_region, true, ranges); } -static void AddModuleSections(Process &process, CoreFileMemoryRanges &ranges, - std::set<addr_t> &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++) { @@ -6588,10 +6587,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); } } @@ -6633,7 +6628,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); } } } @@ -6747,8 +6742,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 391a5ae4b378d5ced975bc8752651eae118dc573 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 2 Oct 2024 11:26:58 -0700 Subject: [PATCH 08/11] 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 b9c0e174c3be68..d07469ef9cfe5f 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 9fc41141a97efa..4818dde4f3b838 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 18c5563e94bc5f82fefaa153aae932fbf5b1af50 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 3 Oct 2024 15:13:43 -0700 Subject: [PATCH 09/11] 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 7758a87403b5a3..47162da0403766 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 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 d07469ef9cfe5f..b9c0e174c3be68 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 f2f87a8ab2b764..e66fd9f75d39ab 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<llvm::ArrayRef<uint8_t>> auxv = m_minidump_parser->GetStream(StreamType::LinuxAuxv); @@ -482,7 +475,6 @@ ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid, void ProcessMinidump::ReadModuleList() { std::vector<const minidump::Module *> 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<ObjectFilePlaceholder>( 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 332e2637cc7ee7..3d235670a33abc 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 039c9c8e6a34a4dc4b9a870f2817ccd1c11b00bf Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 3 Oct 2024 15:14:01 -0700 Subject: [PATCH 10/11] 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 47162da0403766..68d6ab0850853f 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>(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 e66fd9f75d39ab..690362b6f26b18 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<ObjectFilePlaceholder>( 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 6dfb0aedb7fac7..ee1076b12d71dd 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6529,8 +6529,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 fd888e60a5413f3954786f8cf462762b0e464914 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 3 Oct 2024 15:24:18 -0700 Subject: [PATCH 11/11] 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 ee1076b12d71dd..c05f547d31cb13 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6540,30 +6540,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) { @@ -6582,12 +6582,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); } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits