[Lldb-commits] [lldb] [LLDB][Minidump] Have Minidumps save off and properly read TLS data (PR #109477)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 1 15:33:39 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/109477
>From 424b200b296df80071adad2c5ed52c93eaa05807 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 16 Sep 2024 13:02:59 -0700
Subject: [PATCH 1/7] Add Process code to get the fs_base region and the .tdata
sections. Plus a test
---
lldb/source/Target/Process.cpp | 67 ++++++++++++++++++-
.../TestProcessSaveCoreMinidump.py | 42 ++++++++++++
2 files changed, 107 insertions(+), 2 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 ccdb6653cf16f8..454ce8bf387cc5 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -522,3 +522,45 @@ def minidump_deleted_on_save_failure(self):
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
+
+ @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")
+ 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)
+ 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)
>From e6740ece9e58f9e290c98df91679485d0c1faaf1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 16 Sep 2024 13:46:49 -0700
Subject: [PATCH 2/7] 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 7921b3ea3ae8789eba1ff38bd18542a7d86af471 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 19 Sep 2024 14:45:33 -0700
Subject: [PATCH 3/7] 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 bfd70762bbd720cd50dcdbb30eb377b95b6bf19b Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 20 Sep 2024 13:49:27 -0700
Subject: [PATCH 4/7] 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 454ce8bf387cc5..fdd541a66fe0db 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()
@@ -564,3 +565,34 @@ def minidump_saves_fs_base_region(self):
self.assertTrue(self.dbg.DeleteTarget(core_target))
if os.path.isfile(custom_file):
os.unlink(custom_file)
+
+ @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 83580c6b978a36d0be8de52e55c37020c461e033 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 20 Sep 2024 13:59:47 -0700
Subject: [PATCH 5/7] 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 fdd541a66fe0db..80ebc884d3dbf5 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))
@@ -589,7 +589,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 86172332af31d84f23fba5c94287f59f05b6c901 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 1 Oct 2024 14:21:39 -0700
Subject: [PATCH 6/7] 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 5181a042a05db364677933359e1df0c12999570a Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 1 Oct 2024 15:33:24 -0700
Subject: [PATCH 7/7] 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) {
More information about the lldb-commits
mailing list