llvm · Jlalond · Oct 10, 2024 · Sep 16, 2024 · Sep 16, 2024 · Sep 19, 2024
@@ -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"
@@ -337,6 +338,17 @@ class DynamicLoader : public PluginInterface {
    return std::nullopt;
  }

+  /// Returns a list of memory ranges that should be saved in the core file,
+  /// specific for this dynamic loader.
+  ///
+  /// For example, an implementation of this function can save the thread
+  /// local data of a given thread.
+  virtual void CalculateDynamicSaveCoreRanges(
+      lldb_private::Process &process,
+      std::vector<lldb_private::MemoryRegionInfo> &ranges,
+      llvm::function_ref<bool(const lldb_private::Thread &)>
+          save_thread_predicate) {};
+
 protected:
  // Utility methods for derived classes


@@ -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);
 }
-
@@ -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,82 @@ 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 AddThreadLocalMemoryRegions(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,
+    llvm::function_ref<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))
+      continue;
+
+    AddThreadLocalMemoryRegions(process, thread_sp, ranges);
+  }
+
+  AddLinkMapSections(process, ranges);
+}
@@ -60,6 +60,12 @@ 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,
+      llvm::function_ref<bool(const lldb_private::Thread &)>
+          save_thread_predicate) override;
+
 protected:
  /// Runtime linker rendezvous structure.
  DYLDRendezvous m_rendezvous;

@@ -21,11 +21,13 @@
 #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"
 #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"
@@ -34,6 +36,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 +336,16 @@ ArchSpec ProcessMinidump::GetArchitecture() {
  return ArchSpec(triple);
 }

+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(), GetByteOrder(),
+                       GetAddressByteSize(), GetAddressByteSize());
+}
+
 void ProcessMinidump::BuildMemoryRegions() {
  if (m_memory_regions)
    return;
@@ -534,7 +547,12 @@ void ProcessMinidump::ReadModuleList() {

      module_sp = Module::CreateModuleFromObjectFile<ObjectFilePlaceholder>(
          module_spec, load_addr, load_size);
-      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;

@@ -53,12 +53,11 @@ 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(); }

-  SystemRuntime *GetSystemRuntime() override { return nullptr; }
-
  Status DoDestroy() override;

  void RefreshStateAfterStop() override;

@@ -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

@@ -6539,6 +6539,29 @@ 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) {
+  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);
+  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);
+  }
+}
+
 static void SaveOffRegionsWithStackPointers(Process &process,
                                            const SaveCoreOptions &core_options,
                                            const MemoryRegionInfos &regions,
@@ -6570,11 +6593,13 @@ 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);
+      }
    }
  }
 }
@@ -6683,9 +6708,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);
+    // 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) {
  case eSaveCoreUnspecified:
-Original file line number
+Diff line change
@@ -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
             Copy link

  
      
    
  

  
      

  
    Collaborator


      

  

  
    
      

      
            labath
  

      

      

      


        Oct 8, 2024


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Why is that? What is this trying to achieve/prevent?
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  Author


  

  
    
      

      
            Jlalond
  

      

      

      


        Oct 8, 2024


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      In the Testcase in MinidumpUUID, the one module which has a matching filename that isn't an actual match needs this check in order to work.
The situation is the DYLD will itself create a stub depending on some conditions, but this is after the process has already made a stub. So we need some way for the CoreProcess to supercede the dynamic loader clearing all loaded modules again, and so I added this
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Collaborator


      

  

  
    
      

      
            labath
  

      

      

      


        Oct 10, 2024


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Ok, I see. I'm not entirely happy by this, but it sort of makes sense, so I'm willing to look the other way. :P
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
            Jlalond reacted with laugh emoji
      
        
            
    All reactions
  


          
              
                😄
                  1 reaction
+          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);
     }