[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 8 17:10:20 PDT 2024


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

>From 73880b94c09f1a9a9233f80f7eb70b4f68cbf6aa 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 01/15] 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 &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 = 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 &regions,
@@ -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 83d8b4a260876712e8137995a1f3ad155508d7ff 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 02/15] 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 292f01a25a9189332e5d9286855da8612eacbcee 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 03/15] Change DYLD To support TLS for minidump

---
 lldb/source/Expression/DWARFExpression.cpp    |  2 --
 .../Process/minidump/ProcessMinidump.cpp      | 16 ++++++-----
 .../Process/minidump/ProcessMinidump.h        |  5 ++--
 lldb/source/Target/Process.cpp                | 28 ++++++++++++-------
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 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/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 ecd629e28e7f7ead3d85967d9ea3d04170800512 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 04/15] Get TLS variables to work for minidump, add a test that
 the value is accessable correctly

---
 lldb/source/Core/DynamicLoader.cpp            |  3 +-
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp     | 22 ++++++++++++-
 .../RegisterContextMinidump_x86_64.cpp        | 16 ++++++++--
 .../TestProcessSaveCoreMinidump.py            | 32 +++++++++++++++++++
 .../process_save_core_minidump/main.cpp       |  2 ++
 5 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 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 b9c0e174c3be68..8185e0b56e6265 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -108,8 +108,28 @@ void DynamicLoaderPOSIXDYLD::DidAttach() {
   // if we dont have a load address we cant re-base
   bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS;
 
+<<<<<<< HEAD
+=======
+  // if we have a valid executable
+  if (executable_sp.get()) {
+    lldb_private::ObjectFile *obj = executable_sp->GetObjectFile();
+    if (obj) {
+      // don't rebase if the module already has a load address
+      Target &target = m_process->GetTarget();
+      Address addr = obj->GetImageInfoAddress(&target);
+      addr_t load_addr = addr.GetLoadAddress(&target);
+      if (load_addr != LLDB_INVALID_ADDRESS) {
+        rebase_exec = false;
+      } 
+    }
+  } else {
+    // no executable, nothing to re-base
+    rebase_exec = false;
+  }
+
+>>>>>>> 7aae643a0970 (Get TLS variables to work for minidump, add a test that the value is accessable correctly)
   // if the target executable should be re-based
-  if (rebase_exec) {
+  if (rebase_exec || IsCoreFile()) {
     ModuleList module_list;
 
     module_list.Append(executable_sp);
diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index 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 &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,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 b82f6402af37cd032b008cd8560a88c74b306f55 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 05/15] 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 8185e0b56e6265..4ee62275c72d6e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -118,9 +118,8 @@ void DynamicLoaderPOSIXDYLD::DidAttach() {
       Target &target = m_process->GetTarget();
       Address addr = obj->GetImageInfoAddress(&target);
       addr_t load_addr = addr.GetLoadAddress(&target);
-      if (load_addr != LLDB_INVALID_ADDRESS) {
+      if (load_addr != LLDB_INVALID_ADDRESS)
         rebase_exec = false;
-      } 
     }
   } else {
     // no executable, nothing to re-base
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 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 &reg) {
@@ -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 &region, 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 559515af6c1275880e03c4047ec7a0ced7953275 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 06/15] remove path that sets rebase_exec to false in posixDYLD

---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp     | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 4ee62275c72d6e..b56fbfb1faf886 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -108,25 +108,6 @@ void DynamicLoaderPOSIXDYLD::DidAttach() {
   // if we dont have a load address we cant re-base
   bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS;
 
-<<<<<<< HEAD
-=======
-  // if we have a valid executable
-  if (executable_sp.get()) {
-    lldb_private::ObjectFile *obj = executable_sp->GetObjectFile();
-    if (obj) {
-      // don't rebase if the module already has a load address
-      Target &target = m_process->GetTarget();
-      Address addr = obj->GetImageInfoAddress(&target);
-      addr_t load_addr = addr.GetLoadAddress(&target);
-      if (load_addr != LLDB_INVALID_ADDRESS)
-        rebase_exec = false;
-    }
-  } else {
-    // no executable, nothing to re-base
-    rebase_exec = false;
-  }
-
->>>>>>> 7aae643a0970 (Get TLS variables to work for minidump, add a test that the value is accessable correctly)
   // if the target executable should be re-based
   if (rebase_exec || IsCoreFile()) {
     ModuleList module_list;

>From 7e7585e3c638157193f41f187764fa9a0180f2c3 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 07/15] Readd linkmap explicitly because we need it for the
 dynamic loader to figure out the TLS sections

---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp         |  2 +-
 .../Plugins/Process/minidump/ProcessMinidump.cpp  |  3 ++-
 lldb/source/Target/Process.cpp                    | 15 +++++----------
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index b56fbfb1faf886..b9c0e174c3be68 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -109,7 +109,7 @@ void DynamicLoaderPOSIXDYLD::DidAttach() {
   bool rebase_exec = load_offset != LLDB_INVALID_ADDRESS;
 
   // if the target executable should be re-based
-  if (rebase_exec || IsCoreFile()) {
+  if (rebase_exec) {
     ModuleList module_list;
 
     module_list.Append(executable_sp);
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 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 &region, 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 f3b01d72129b0d2a58cf02adda7180fd1711f2f8 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 2 Oct 2024 11:26:58 -0700
Subject: [PATCH 08/15] 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 6feab484388fde240646c7b3ce4775be3a7fb0e3 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 3 Oct 2024 15:13:43 -0700
Subject: [PATCH 09/15] 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 68cde7717b9b77d05bcd6516b0215357285c8115 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 3 Oct 2024 15:14:01 -0700
Subject: [PATCH 10/15] 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 &region, 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 f448ad29771a204496d5a15016eae1b919c3d38e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 3 Oct 2024 15:24:18 -0700
Subject: [PATCH 11/15] 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);
   }
 }
 

>From 36a91b116df15901c02af5045ecb384698b8f14d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 7 Oct 2024 10:49:38 -0700
Subject: [PATCH 12/15] Check for stack truncations in the link map code,
 surprisingly.

---
 lldb/source/Target/Process.cpp | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index c05f547d31cb13..91c3ff0404028d 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6566,7 +6566,8 @@ static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp,
   AddRegion(thread_local_region, true, ranges);
 }
 
-static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) {
+static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges,
+                               std::set<addr_t> &stack_ends) {
   ModuleList &module_list = process.GetTarget().GetImages();
   Target *target = &process.GetTarget();
   for (size_t idx = 0; idx < module_list.GetSize(); idx++) {
@@ -6587,6 +6588,11 @@ static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges) {
     if (err.Fail())
       continue;
 
+    // Sometimes, the link map section is included in one of the stack memory
+    // ranges. In that case, we already saved a truncated version of that range
+    if (stack_ends.count(link_map_section.GetRange().GetRangeEnd()) == 0)
+      continue;
+
     AddRegion(link_map_section, true, ranges);
   }
 }
@@ -6743,7 +6749,7 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
     SaveOffRegionsWithStackPointers(*this, options, regions, ranges,
                                     stack_ends);
     // We need the link map for TLS data.
-    AddLinkMapSections(*this, ranges);
+    AddLinkMapSections(*this, ranges, stack_ends);
   }
 
   switch (core_style) {

>From dc2cd8feccd7881c3ef245bada7254c398d1f32e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 8 Oct 2024 13:17:57 -0700
Subject: [PATCH 13/15] Move the TLS code to the DYLD

---
 lldb/include/lldb/Target/DynamicLoader.h      |  8 ++
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp     | 75 +++++++++++++++++
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h       |  2 +
 lldb/source/Target/Process.cpp                | 80 +++----------------
 4 files changed, 98 insertions(+), 67 deletions(-)

diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 0629e2faae7e9e..93bc606e9164f7 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -18,6 +18,7 @@
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-types.h"
+#include "lldb/Target/CoreFileMemoryRanges.h"
 
 #include <cstddef>
 #include <cstdint>
@@ -337,6 +338,13 @@ class DynamicLoader : public PluginInterface {
     return std::nullopt;
   }
 
+  /// Returns a list of memory ranges that should be saved in the core file, 
+  /// specific for this dßynamic loader.
+  ///
+  /// By default, this returns an empty list, but for POSIX/ELF it will return
+  /// the link map, and the TLS data.
+  virtual void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) {};
+
 protected:
   // Utility methods for derived classes
 
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index b9c0e174c3be68..9499fd595190ab 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Platform.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
@@ -866,3 +867,77 @@ bool DynamicLoaderPOSIXDYLD::AlwaysRelyOnEHUnwindInfo(
 bool DynamicLoaderPOSIXDYLD::IsCoreFile() const {
   return !m_process->IsLiveDebugSession();
 }
+
+// For our ELF/POSIX builds save off the fs_base/gs_base regions
+static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp,
+                                       std::vector<MemoryRegionInfo> &ranges) {
+  lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext();
+  if (!reg_ctx)
+    return;
+
+  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo(
+      lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_TP);
+  if (!reg_info)
+    return;
+
+  lldb_private::RegisterValue thread_local_register_value;
+  bool success = reg_ctx->ReadRegister(reg_info, thread_local_register_value);
+  if (!success)
+    return;
+
+  const uint64_t fail_value = UINT64_MAX;
+  bool readSuccess = false;
+  const lldb::addr_t reg_value_addr =
+      thread_local_register_value.GetAsUInt64(fail_value, &readSuccess);
+  if (!readSuccess || reg_value_addr == fail_value)
+    return;
+
+  MemoryRegionInfo thread_local_region;
+  Status err = process.GetMemoryRegionInfo(reg_value_addr, thread_local_region);
+  if (err.Fail())
+    return;
+
+  ranges.push_back(thread_local_region);
+}
+
+// Save off the link map for core files.
+static void AddLinkMapSections(Process &process, std::vector<MemoryRegionInfo> &ranges) {
+  ModuleList &module_list = process.GetTarget().GetImages();
+  Target *target = &process.GetTarget();
+  for (size_t idx = 0; idx < module_list.GetSize(); idx++) {
+    ModuleSP module_sp = module_list.GetModuleAtIndex(idx);
+    if (!module_sp)
+      continue;
+
+    ObjectFile *obj = module_sp->GetObjectFile();
+    if (!obj)
+      continue;
+    Address addr = obj->GetImageInfoAddress(target);
+    addr_t load_addr = addr.GetLoadAddress(target);
+    if (load_addr == LLDB_INVALID_ADDRESS)
+      continue;
+
+    MemoryRegionInfo link_map_section;
+    Status err = process.GetMemoryRegionInfo(load_addr, link_map_section);
+    if (err.Fail())
+      continue;
+
+    ranges.push_back(link_map_section);
+  }
+}
+
+void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) {
+  ThreadList &thread_list = process.GetThreadList();
+  for (size_t idx = 0; idx < thread_list.GetSize(); idx++) {
+    ThreadSP thread_sp = thread_list.GetThreadAtIndex(idx);
+    if (!thread_sp)
+      continue;
+
+    if (!save_thread_predicate(*thread_sp.get()))
+      continue;
+
+    AddSegmentRegisterSections(process, thread_sp, ranges);
+  }
+
+  AddLinkMapSections(process, ranges);
+}
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 4c92335602cdf4..23ec0b99d4f7b0 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -60,6 +60,8 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
                                      lldb::addr_t base_addr,
                                      bool base_addr_is_offset) override;
 
+   void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) override; 
+
 protected:
   /// Runtime linker rendezvous structure.
   DYLDRendezvous m_rendezvous;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 91c3ff0404028d..c3f6b28ce464c1 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6528,72 +6528,18 @@ static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
                 CreateCoreFileMemoryRange(region));
 }
 
-static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp,
-                                       CoreFileMemoryRanges &ranges,
-                                       lldb::addr_t range_end) {
-  lldb::RegisterContextSP reg_ctx = thread_sp->GetRegisterContext();
-  if (!reg_ctx)
+static void SaveDynamicLoaderSections(Process &process, const SaveCoreOptions &options, CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+  DynamicLoader *dyld = process.GetDynamicLoader();
+  if (!dyld)
     return;
 
-  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo(
-      lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_TP);
-  if (!reg_info)
-    return;
-
-  lldb_private::RegisterValue thread_local_register_value;
-  bool success = reg_ctx->ReadRegister(reg_info, thread_local_register_value);
-  if (!success)
-    return;
-
-  const uint64_t fail_value = UINT64_MAX;
-  bool readSuccess = false;
-  const lldb::addr_t reg_value_addr =
-      thread_local_register_value.GetAsUInt64(fail_value, &readSuccess);
-  if (!readSuccess || reg_value_addr == fail_value)
-    return;
-
-  MemoryRegionInfo thread_local_region;
-  Status err = process.GetMemoryRegionInfo(reg_value_addr, thread_local_region);
-  if (err.Fail())
-    return;
-
-  // We already saved off this truncated stack range.
-  if (thread_local_region.GetRange().GetRangeEnd() == range_end)
-    return;
-
-  // We don't need to worry about duplication because the CoreFileMemoryRanges
-  // will unique them
-  AddRegion(thread_local_region, true, ranges);
-}
-
-static void AddLinkMapSections(Process &process, CoreFileMemoryRanges &ranges,
-                               std::set<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 link_map_section;
-    Status err = process.GetMemoryRegionInfo(load_addr, link_map_section);
-    if (err.Fail())
-      continue;
-
-    // Sometimes, the link map section is included in one of the stack memory
-    // ranges. In that case, we already saved a truncated version of that range
-    if (stack_ends.count(link_map_section.GetRange().GetRangeEnd()) == 0)
-      continue;
-
-    AddRegion(link_map_section, true, ranges);
+  std::vector<MemoryRegionInfo> dynamic_loader_mem_regions;
+  std::function<bool(const lldb_private::Thread&)> save_thread_predicate = [&](const lldb_private::Thread &t) -> bool { return options.ShouldThreadBeSaved(t.GetID()); };
+  dyld->CalculateDynamicSaveCoreRanges(process, dynamic_loader_mem_regions, save_thread_predicate);
+  for (const auto &region : dynamic_loader_mem_regions) {
+    // The Dynamic Loader can give us regions that could include a truncated stack
+    if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
+      AddRegion(region, true, ranges);
   }
 }
 
@@ -6634,7 +6580,6 @@ static void SaveOffRegionsWithStackPointers(Process &process,
       // or contains the thread id from thread_sp.
       if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) {
         AddRegion(sp_region, try_dirty_pages, ranges);
-        AddSegmentRegisterSections(process, thread_sp, ranges, range_end);
       }
     }
   }
@@ -6748,8 +6693,9 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
       options.HasSpecifiedThreads()) {
     SaveOffRegionsWithStackPointers(*this, options, regions, ranges,
                                     stack_ends);
-    // We need the link map for TLS data.
-    AddLinkMapSections(*this, ranges, stack_ends);
+    // Save off the dynamic loader sections, so if we are on an architecture that supports
+    // Thread Locals, that we include those as well.
+    SaveDynamicLoaderSections(*this, options, ranges, stack_ends);
   }
 
   switch (core_style) {

>From d4b05756e90b6ea1c15545045597dccf55efc4c5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 8 Oct 2024 13:24:46 -0700
Subject: [PATCH 14/15] Move the TLS code to the DYLD

---
 lldb/include/lldb/Target/DynamicLoader.h      | 10 +++++++---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp     |  8 ++++++--
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.h       |  6 +++++-
 lldb/source/Target/Process.cpp                | 20 +++++++++++++------
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 93bc606e9164f7..1ab8b1f2643fbd 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -11,6 +11,7 @@
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Target/CoreFileMemoryRanges.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
@@ -18,7 +19,6 @@
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-types.h"
-#include "lldb/Target/CoreFileMemoryRanges.h"
 
 #include <cstddef>
 #include <cstdint>
@@ -338,12 +338,16 @@ class DynamicLoader : public PluginInterface {
     return std::nullopt;
   }
 
-  /// Returns a list of memory ranges that should be saved in the core file, 
+  /// Returns a list of memory ranges that should be saved in the core file,
   /// specific for this dßynamic loader.
   ///
   /// By default, this returns an empty list, but for POSIX/ELF it will return
   /// the link map, and the TLS data.
-  virtual void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) {};
+  virtual void CalculateDynamicSaveCoreRanges(
+      lldb_private::Process &process,
+      std::vector<lldb_private::MemoryRegionInfo> &ranges,
+      std::function<bool(const lldb_private::Thread &)> save_thread_predicate) {
+  };
 
 protected:
   // Utility methods for derived classes
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 9499fd595190ab..5a1dedd4b60ad0 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -901,7 +901,8 @@ static void AddSegmentRegisterSections(Process &process, ThreadSP &thread_sp,
 }
 
 // Save off the link map for core files.
-static void AddLinkMapSections(Process &process, std::vector<MemoryRegionInfo> &ranges) {
+static void AddLinkMapSections(Process &process,
+                               std::vector<MemoryRegionInfo> &ranges) {
   ModuleList &module_list = process.GetTarget().GetImages();
   Target *target = &process.GetTarget();
   for (size_t idx = 0; idx < module_list.GetSize(); idx++) {
@@ -926,7 +927,10 @@ static void AddLinkMapSections(Process &process, std::vector<MemoryRegionInfo> &
   }
 }
 
-void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) {
+void DynamicLoaderPOSIXDYLD::CalculateDynamicSaveCoreRanges(
+    lldb_private::Process &process,
+    std::vector<lldb_private::MemoryRegionInfo> &ranges,
+    std::function<bool(const lldb_private::Thread &)> save_thread_predicate) {
   ThreadList &thread_list = process.GetThreadList();
   for (size_t idx = 0; idx < thread_list.GetSize(); idx++) {
     ThreadSP thread_sp = thread_list.GetThreadAtIndex(idx);
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
index 23ec0b99d4f7b0..2e7d0fdaa9df4e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -60,7 +60,11 @@ class DynamicLoaderPOSIXDYLD : public lldb_private::DynamicLoader {
                                      lldb::addr_t base_addr,
                                      bool base_addr_is_offset) override;
 
-   void CalculateDynamicSaveCoreRanges(lldb_private::Process &process, std::vector<lldb_private::MemoryRegionInfo> &ranges, std::function<bool(const lldb_private::Thread&)> save_thread_predicate) override; 
+  void CalculateDynamicSaveCoreRanges(
+      lldb_private::Process &process,
+      std::vector<lldb_private::MemoryRegionInfo> &ranges,
+      std::function<bool(const lldb_private::Thread &)> save_thread_predicate)
+      override;
 
 protected:
   /// Runtime linker rendezvous structure.
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index c3f6b28ce464c1..c009d17d3ba507 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6528,16 +6528,24 @@ static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
                 CreateCoreFileMemoryRange(region));
 }
 
-static void SaveDynamicLoaderSections(Process &process, const SaveCoreOptions &options, CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+static void SaveDynamicLoaderSections(Process &process,
+                                      const SaveCoreOptions &options,
+                                      CoreFileMemoryRanges &ranges,
+                                      std::set<addr_t> &stack_ends) {
   DynamicLoader *dyld = process.GetDynamicLoader();
   if (!dyld)
     return;
 
   std::vector<MemoryRegionInfo> dynamic_loader_mem_regions;
-  std::function<bool(const lldb_private::Thread&)> save_thread_predicate = [&](const lldb_private::Thread &t) -> bool { return options.ShouldThreadBeSaved(t.GetID()); };
-  dyld->CalculateDynamicSaveCoreRanges(process, dynamic_loader_mem_regions, save_thread_predicate);
+  std::function<bool(const lldb_private::Thread &)> save_thread_predicate =
+      [&](const lldb_private::Thread &t) -> bool {
+    return options.ShouldThreadBeSaved(t.GetID());
+  };
+  dyld->CalculateDynamicSaveCoreRanges(process, dynamic_loader_mem_regions,
+                                       save_thread_predicate);
   for (const auto &region : dynamic_loader_mem_regions) {
-    // The Dynamic Loader can give us regions that could include a truncated stack
+    // The Dynamic Loader can give us regions that could include a truncated
+    // stack
     if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
       AddRegion(region, true, ranges);
   }
@@ -6693,8 +6701,8 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
       options.HasSpecifiedThreads()) {
     SaveOffRegionsWithStackPointers(*this, options, regions, ranges,
                                     stack_ends);
-    // Save off the dynamic loader sections, so if we are on an architecture that supports
-    // Thread Locals, that we include those as well.
+    // Save off the dynamic loader sections, so if we are on an architecture
+    // that supports Thread Locals, that we include those as well.
     SaveDynamicLoaderSections(*this, options, ranges, stack_ends);
   }
 

>From df7a3f65ea7e1276ee63227ed81ab752cb494597 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 8 Oct 2024 17:09:57 -0700
Subject: [PATCH 15/15] Code review cleanup

---
 lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index 690362b6f26b18..5ea3db23f114c4 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -342,7 +342,7 @@ DataExtractor ProcessMinidump::GetAuxvData() {
   if (!auxv)
     return DataExtractor();
 
-  return DataExtractor(auxv->data(), auxv->size(), ByteOrder::eByteOrderLittle,
+  return DataExtractor(auxv->data(), auxv->size(), GetByteOrder(),
                        GetAddressByteSize(), GetAddressByteSize());
 }
 
@@ -475,6 +475,7 @@ ModuleSP ProcessMinidump::GetOrCreateModule(UUID minidump_uuid,
 void ProcessMinidump::ReadModuleList() {
   std::vector<const minidump::Module *> filtered_modules =
       m_minidump_parser->GetFilteredModuleList();
+
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
   for (auto module : filtered_modules) {
@@ -543,6 +544,7 @@ void ProcessMinidump::ReadModuleList() {
                "Unable to locate the matching object file, creating a "
                "placeholder module for: {0}",
                name);
+
       module_sp = Module::CreateModuleFromObjectFile<ObjectFilePlaceholder>(
           module_spec, load_addr, load_size);
       // If we haven't loaded a main executable yet, set the first module to be



More information about the lldb-commits mailing list