[Lldb-commits] [lldb] [LLDB][Minidump] Have Minidumps save off and properly read TLS data (PR #109477)

via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 20 14:05:33 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

<details>
<summary>Changes</summary>

This patch adds the support to `Process.cpp` to automatically save off TLS sections, either via loading the memory region for the module, or via reading `fs_base` via generic register. Then when Minidumps are loaded, we now specify we want the dynamic loader to be the `POSIXDYLD` so we can leverage the same TLS accessor code as `ProcessELFCore`. Being able to access TLS Data is an important step for LLDB generated minidumps to have feature parity with ELF Core dumps.

---
Full diff: https://github.com/llvm/llvm-project/pull/109477.diff


7 Files Affected:

- (modified) lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp (+3-2) 
- (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp (+19) 
- (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.h (+4-3) 
- (modified) lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp (+16-4) 
- (modified) lldb/source/Target/Process.cpp (+78-3) 
- (modified) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (+74) 
- (modified) lldb/test/API/functionalities/process_save_core_minidump/main.cpp (+1) 


``````````diff
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 51e4b3e6728f23..38e9a61817ab55 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -115,7 +115,8 @@ 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;
     }
   } else {
@@ -124,7 +125,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/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 32ffba763c08e3..2a8f9fe9b9d93d 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"
@@ -34,6 +35,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 
+#include "Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h"
 #include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h"
 #include "Plugins/Process/Utility/StopInfoMachException.h"
 
@@ -333,6 +335,23 @@ ArchSpec ProcessMinidump::GetArchitecture() {
   return ArchSpec(triple);
 }
 
+DynamicLoader *ProcessMinidump::GetDynamicLoader() {
+  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 f2ea0a2b61d14e..332e2637cc7ee7 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -53,11 +53,12 @@ class ProcessMinidump : public PostMortemProcess {
 
   Status DoLoadCore() override;
 
-  DynamicLoader *GetDynamicLoader() override { return nullptr; }
+  // Returns AUXV structure found in the core file
+  lldb_private::DataExtractor GetAuxvData() override;
 
-  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+  DynamicLoader *GetDynamicLoader() override;
 
-  SystemRuntime *GetSystemRuntime() override { return nullptr; }
+  llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
   Status DoDestroy() override;
 
diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index e879c493156593..f305d1b7031d82 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
@@ -44,6 +44,17 @@ static void writeRegister(const void *reg_src, uint8_t *context,
   memcpy(reg_dest.data(), reg_src, reg_dest.size());
 }
 
+// TODO: Fix the registers in this file!
+// writeRegister checks x86_64 registers without base registers. This causes
+// an overlap in the register enum values. So we were truncating fs_base.
+// We should standardize to the x86_64_with_base registers.
+static void writeBaseRegister(const void *reg_src, uint8_t *context,
+                              const RegisterInfo &reg) {
+  auto bytes = reg.mutable_data(context);
+  llvm::MutableArrayRef<uint8_t> reg_dest = bytes.take_front(8);
+  memcpy(reg_dest.data(), reg_src, reg_dest.size());
+}
+
 lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64(
     llvm::ArrayRef<uint8_t> source_data,
     RegisterInfoInterface *target_reg_interface) {
@@ -105,11 +116,12 @@ lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64(
     writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]);
   }
 
+  // See comment on base regsiter
   if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) {
-    writeRegister(&context->fs_base, result_base,
-                  reg_info[x86_64_with_base::lldb_fs_base]);
-    writeRegister(&context->gs_base, result_base,
-                  reg_info[x86_64_with_base::lldb_gs_base]);
+    writeBaseRegister(&context->fs_base, result_base,
+                      reg_info[x86_64_with_base::lldb_fs_base]);
+    writeBaseRegister(&context->gs_base, result_base,
+                      reg_info[x86_64_with_base::lldb_gs_base]);
   }
 
   // TODO parse the floating point registers
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index aca08972811470..8ddb2be409f855 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6528,6 +6528,74 @@ static void AddRegion(const MemoryRegionInfo &region, 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 = false;
+  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 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;
+
+    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(load_addr, tls_storage_region);
+    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);
+  }
+}
+
 static void SaveOffRegionsWithStackPointers(Process &process,
                                             const SaveCoreOptions &core_options,
                                             const MemoryRegionInfos &regions,
@@ -6559,11 +6627,14 @@ 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);
+        AddRegisterSections(process, thread_sp, ranges, range_end);
+      }
     }
   }
 }
@@ -6672,9 +6743,13 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
   std::set<addr_t> stack_ends;
   // For fully custom set ups, we don't want to even look at threads if there
   // are no threads specified.
-  if (core_style != lldb::eSaveCoreCustomOnly || options.HasSpecifiedThreads())
+  if (core_style != lldb::eSaveCoreCustomOnly ||
+      options.HasSpecifiedThreads()) {
     SaveOffRegionsWithStackPointers(*this, options, regions, ranges,
                                     stack_ends);
+    // Save off the load sections for the TLS data.
+    AddModuleSections(*this, ranges, stack_ends);
+  }
 
   switch (core_style) {
   case eSaveCoreUnspecified:
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index ccdb6653cf16f8..80ebc884d3dbf5 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,77 @@ 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()
+            self.assertTrue(fs_base != 0)
+            core_target = self.dbg.CreateTarget(None)
+            core_proc = core_target.LoadCore(one_region_file)
+            core_region_list = core_proc.GetMemoryRegions()
+            live_region_list = process.GetMemoryRegions()
+            live_region = lldb.SBMemoryRegionInfo()
+            live_region_list.GetMemoryRegionForAddress(fs_base, live_region)
+            core_region = lldb.SBMemoryRegionInfo()
+            error = core_region_list.GetMemoryRegionForAddress(fs_base, core_region)
+            self.assertTrue(error.Success())
+            self.assertEqual(live_region, core_region)
+
+        finally:
+            self.assertTrue(self.dbg.DeleteTarget(target))
+            self.assertTrue(self.dbg.DeleteTarget(core_target))
+            if os.path.isfile(custom_file):
+                os.unlink(custom_file)
+
+    @skipUnlessPlatform(["linux"])
+    @skipUnlessArch("x86_64")
+    def minidump_saves_fs_base_region(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        try:
+            target = self.dbg.CreateTarget(exe)
+            process = target.LaunchSimple(
+                None, None, self.get_process_working_directory()
+            )
+            self.assertState(process.GetState(), lldb.eStateStopped)
+            thread = process.GetThreadAtIndex(0)
+            tls_file = self.getBuildArtifact("core.tls.dmp")
+            options = lldb.SBSaveCoreOptions()
+            options.SetOutputFile(lldb.SBFileSpec(tls_file))
+            options.SetPluginName("minidump")
+            options.SetStyle(lldb.eSaveCoreCustomOnly)
+            options.AddThread(thread)
+            error = process.SaveCore(options)
+            self.assertTrue(error.Success())
+            core_target = self.dbg.CreateTarget(None)
+            core_proc = core_target.LoadCore(tls_file)
+            frame = core_proc.GetThreadAtIndex(0).GetFrameAtIndex(0)
+            tls_val = frame.FindValue("lf")
+            self.assertEqual(tls_val.GetValueAsUnsigned(), 42)
+
+        except:
+            self.assertTrue(self.dbg.DeleteTarget(target))
+            if os.path.isfile(tls_file):
+                os.unlink(tls_file)
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/main.cpp b/lldb/test/API/functionalities/process_save_core_minidump/main.cpp
index fa34a371f20647..15daa68e9a648c 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/main.cpp
+++ b/lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -1,6 +1,7 @@
 #include <cassert>
 #include <iostream>
 #include <thread>
+thread_local size_t lf = 42;
 
 void g() { assert(false); }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/109477


More information about the lldb-commits mailing list