llvm · Jlalond · Jul 18, 2025 · Jul 2, 2025 · Jul 2, 2025 · Jul 2, 2025
@@ -45,6 +45,10 @@ Note that currently ELF Core files are not supported."
     Resetting will result in the reset of all process specific options, such as Threads to save."
 ) lldb::SBSaveCoreOptions::SetProcess;
 
+%feature("docstring", "
+    Get the process to save. If a process is not defined, whether by calling clear or by not setting a process, an invalid process will be returned."
+) lldb::SBSaveCoreOptions::GetProcess;
+
 %feature("docstring", "
     Add an SBThread to be saved, an error will be returned if an SBThread from a different process is specified. 
     The process is set either by the first SBThread added to the options container, or explicitly by the SetProcess call."
@@ -63,6 +67,12 @@ Note that currently ELF Core files are not supported."
     Get an SBThreadCollection of all threads marked to be saved. This collection is not sorted according to insertion order."
 ) lldb::SBSaveCoreOptions::GetThreadsToSave;
 
+%feature("docstring", "
+    Get an SBMemoryRegionInfoList of all the Regions that LLDB will attempt to write into the Core. Note, reading from these
+    regions can fail, and it's not guaraunteed every region will be present in the resulting core. If called without a valid process or style set an empty
+    collection will be returned."
+) lldb::SBSaveCoreOptions::GetMemoryRegionsToSave;
+
 %feature("docstring", "
     Get the current total number of bytes the core is expected to have, excluding the overhead of the core file format.
     Requires both a Process and a Style to be specified. An error will be returned if the provided options would result in no data being saved."

@@ -45,6 +45,7 @@ class LLDB_API SBMemoryRegionInfoList {
 
 private:
   friend class SBProcess;
+  friend class SBSaveCoreOptions;
 
   lldb_private::MemoryRegionInfos &ref();
 

@@ -12,6 +12,7 @@
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBMemoryRegionInfoList.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBThreadCollection.h"
@@ -78,6 +79,13 @@ class LLDB_API SBSaveCoreOptions {
   ///   api, or implicitly from any function that requires a process.
   SBError SetProcess(lldb::SBProcess process);
 
+  /// Get the process to save, if the process is not set an invalid SBProcess
+  /// will be returned.
+  ///
+  /// \return
+  ///   The set process, or an invalid SBProcess if no process is set.
+  SBProcess GetProcess();
+
   /// Add a thread to save in the core file.
   ///
   /// \param thread
@@ -119,6 +127,13 @@ class LLDB_API SBSaveCoreOptions {
   ///   an empty collection will be returned.
   SBThreadCollection GetThreadsToSave() const;
 
+  /// Get an unsorted copy of all memory regions to save
+  ///
+  /// \returns
+  ///   An unsorted copy of all memory regions to save. If no process or style
+  ///   is specified an empty collection will be returned.
+  SBMemoryRegionInfoList GetMemoryRegionsToSave();
+
   /// Get the current total number of bytes the core is expected to have
   /// excluding the overhead of the core file format. Requires a Process and
   /// Style to be specified.

@@ -261,8 +261,7 @@ class PluginManager {
   static ObjectFileCreateMemoryInstance
   GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
 
-  static Status SaveCore(const lldb::ProcessSP &process_sp,
-                         lldb_private::SaveCoreOptions &core_options);
+  static Status SaveCore(lldb_private::SaveCoreOptions &core_options);
 
   static std::vector<llvm::StringRef> GetSaveCorePluginNames();
 

@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
 #define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
 
+#include "lldb/Target/CoreFileMemoryRanges.h"
 #include "lldb/Target/ThreadCollection.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/RangeMap.h"
@@ -23,7 +24,7 @@ namespace lldb_private {
 
 class SaveCoreOptions {
 public:
-  SaveCoreOptions(){};
+  SaveCoreOptions() = default;
   ~SaveCoreOptions() = default;
 
   lldb_private::Status SetPluginName(const char *name);
@@ -36,17 +37,19 @@ class SaveCoreOptions {
   const std::optional<lldb_private::FileSpec> GetOutputFile() const;
 
   Status SetProcess(lldb::ProcessSP process_sp);
+  lldb::ProcessSP GetProcess() { return m_process_sp; }
 
   Status AddThread(lldb::ThreadSP thread_sp);
   bool RemoveThread(lldb::ThreadSP thread_sp);
   bool ShouldThreadBeSaved(lldb::tid_t tid) const;
   bool HasSpecifiedThreads() const;
 
-  Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
+  Status EnsureValidConfiguration() const;
   const MemoryRanges &GetCoreFileMemoryRanges() const;
 
   void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);
 
+  llvm::Expected<lldb_private::CoreFileMemoryRanges> GetMemoryRegionsToSave();
   lldb_private::ThreadCollection::collection GetThreadsToSave() const;
 
   llvm::Expected<uint64_t> GetCurrentSizeInBytes();

@@ -1263,6 +1263,15 @@ lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
     return error;
   }
 
+  if (!options.GetProcess())
+    options.SetProcess(process_sp);
+
+  if (options.GetProcess().GetSP() != process_sp) {
+    error = Status::FromErrorString(
+        "Save Core Options configured for a different process.");
+    return error;
+  }
+
   std::lock_guard<std::recursive_mutex> guard(
       process_sp->GetTarget().GetAPIMutex());
 
@@ -1271,7 +1280,7 @@ lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) {
     return error;
   }
 
-  error.ref() = PluginManager::SaveCore(process_sp, options.ref());
+  error.ref() = PluginManager::SaveCore(options.ref());
 
   return error;
 }

@@ -81,6 +81,11 @@ SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) {
   return m_opaque_up->SetProcess(process.GetSP());
 }
 
+SBProcess SBSaveCoreOptions::GetProcess() {
+  LLDB_INSTRUMENT_VA(this);
+  return SBProcess(m_opaque_up->GetProcess());
+}
+
 SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) {
   LLDB_INSTRUMENT_VA(this, thread);
   return m_opaque_up->AddThread(thread.GetSP());
@@ -128,6 +133,26 @@ uint64_t SBSaveCoreOptions::GetCurrentSizeInBytes(SBError &error) {
   return *expected_bytes;
 }
 
+lldb::SBMemoryRegionInfoList SBSaveCoreOptions::GetMemoryRegionsToSave() {
+  LLDB_INSTRUMENT_VA(this);
+  llvm::Expected<lldb_private::CoreFileMemoryRanges> memory_ranges =
+      m_opaque_up->GetMemoryRegionsToSave();
+  if (!memory_ranges) {
+    llvm::consumeError(memory_ranges.takeError());
+    return SBMemoryRegionInfoList();
+  }
+
+  SBMemoryRegionInfoList memory_region_infos;
+  for (const auto &range : *memory_ranges) {
+    SBMemoryRegionInfo region_info(
+        nullptr, range.GetRangeBase(), range.GetRangeEnd(),
+        range.data.lldb_permissions, /*mapped=*/true);
+    memory_region_infos.Append(region_info);
+  }
+
+  return memory_region_infos;
+}
+
 lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
   return *m_opaque_up;
 }
@@ -1354,7 +1354,8 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
         FileSystem::Instance().Resolve(output_file);
         auto &core_dump_options = m_options.m_core_dump_options;
         core_dump_options.SetOutputFile(output_file);
-        Status error = PluginManager::SaveCore(process_sp, core_dump_options);
+        core_dump_options.SetProcess(process_sp);
+        Status error = PluginManager::SaveCore(core_dump_options);
         if (error.Success()) {
           if (core_dump_options.GetStyle() ==
                   SaveCoreStyle::eSaveCoreDirtyOnly ||

@@ -952,27 +952,26 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
   return nullptr;
 }
 
-Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
-                               lldb_private::SaveCoreOptions &options) {
+Status PluginManager::SaveCore(lldb_private::SaveCoreOptions &options) {
   Status error;
   if (!options.GetOutputFile()) {
     error = Status::FromErrorString("No output file specified");
     return error;
   }
 
-  if (!process_sp) {
+  if (!options.GetProcess()) {
     error = Status::FromErrorString("Invalid process");
     return error;
   }
 
-  error = options.EnsureValidConfiguration(process_sp);
+  error = options.EnsureValidConfiguration();
   if (error.Fail())
     return error;
 
   if (!options.GetPluginName().has_value()) {
     // Try saving core directly from the process plugin first.
     llvm::Expected<bool> ret =
-        process_sp->SaveCore(options.GetOutputFile()->GetPath());
+        options.GetProcess()->SaveCore(options.GetOutputFile()->GetPath());
     if (!ret)
       return Status::FromError(ret.takeError());
     if (ret.get())
@@ -984,7 +983,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
   auto instances = GetObjectFileInstances().GetSnapshot();
   for (auto &instance : instances) {
     if (plugin_name.empty() || instance.name == plugin_name) {
-      if (instance.save_core && instance.save_core(process_sp, options, error))
+      // TODO: Refactor the instance.save_core() to not require a process and
+      // get it from options instead.
+      if (instance.save_core &&
+          instance.save_core(options.GetProcess(), options, error))
         return error;
     }
   }

@@ -836,13 +836,13 @@ Status MinidumpFileBuilder::AddMemoryList() {
   // 32 bit memory descriptiors, so we emit them first to ensure the memory is
   // in accessible with a 32 bit offset.
   std::vector<CoreFileMemoryRange> ranges_32;
-  std::vector<CoreFileMemoryRange> ranges_64;
-  CoreFileMemoryRanges all_core_memory_ranges;
-  error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
-                                                    all_core_memory_ranges);
+  llvm::Expected<CoreFileMemoryRanges> all_core_memory_ranges_maybe =
+      m_save_core_options.GetMemoryRegionsToSave();
+  if (!all_core_memory_ranges_maybe)
+    return Status::FromError(all_core_memory_ranges_maybe.takeError());
 
-  if (error.Fail())
-    return error;
+  const CoreFileMemoryRanges &all_core_memory_ranges =
+      *all_core_memory_ranges_maybe;
 
   lldb_private::Progress progress("Saving Minidump File", "",
                                   all_core_memory_ranges.GetSize());
@@ -868,6 +868,10 @@ Status MinidumpFileBuilder::AddMemoryList() {
     }
   }
 
+  // The header has to be in 32b memory, as it needs to be addressable by a 32b
+  // RVA. Everything else can be 64b.
+  total_size += sizeof(llvm::minidump::MemoryListHeader);
+
   if (total_size >= UINT32_MAX) {
     error = Status::FromErrorStringWithFormat(
         "Unable to write minidump. Stack memory "
@@ -876,35 +880,15 @@ Status MinidumpFileBuilder::AddMemoryList() {
     return error;
   }
 
-  // After saving the stacks, we start packing as much as we can into 32b.
-  // We apply a generous padding here so that the Directory, MemoryList and
-  // Memory64List sections all begin in 32b addressable space.
-  // Then anything overflow extends into 64b addressable space.
-  // all_core_memory_vec will either contain all stack regions at this point,
-  // or be empty if it's a stack only minidump.
-  if (!all_core_memory_vec.empty())
-    total_size += 256 + (all_core_memory_vec.size() *
-                         sizeof(llvm::minidump::MemoryDescriptor_64));
-
-  for (const auto &core_range : all_core_memory_vec) {
-    const addr_t range_size = core_range.range.size();
-    // We don't need to check for stacks here because we already removed them
-    // from all_core_memory_ranges.
-    if (total_size + range_size < UINT32_MAX) {
-      ranges_32.push_back(core_range);
-      total_size += range_size;
-    } else {
-      ranges_64.push_back(core_range);
-    }
-  }
-
+  // Save only the thread stacks to the 32b memory list. Everything else will
+  // get put in Memory64, this simplifies tracking
   error = AddMemoryList_32(ranges_32, progress);
   if (error.Fail())
     return error;
 
   // Add the remaining memory as a 64b range.
-  if (!ranges_64.empty()) {
-    error = AddMemoryList_64(ranges_64, progress);
+  if (!all_core_memory_ranges.IsEmpty()) {
+    error = AddMemoryList_64(all_core_memory_vec, progress);
     if (error.Fail())
       return error;
   }
@@ -977,14 +961,15 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
   const lldb::addr_t addr = range.range.start();
   const lldb::addr_t size = range.range.size();
   Log *log = GetLog(LLDBLog::Object);
+  uint64_t total_bytes_read = 0;
   Status addDataError;
   Process::ReadMemoryChunkCallback callback =
       [&](Status &error, lldb::addr_t current_addr, const void *buf,
           uint64_t bytes_read) -> lldb_private::IterationAction {
     if (error.Fail() || bytes_read == 0) {
       LLDB_LOGF(log,
                 "Failed to read memory region at: 0x%" PRIx64
-                ". Bytes read: %" PRIx64 ", error: %s",
+                ". Bytes read: 0x%" PRIx64 ", error: %s",
                 current_addr, bytes_read, error.AsCString());
 
       // If we failed in a memory read, we would normally want to skip
@@ -997,6 +982,21 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
       return lldb_private::IterationAction::Stop;
     }
 
+    if (current_addr != addr + total_bytes_read) {
+      LLDB_LOGF(log,
+                "Current addr is at unexpected address, 0x%" PRIx64
+                ", expected at 0x%" PRIx64,
+                current_addr, addr + total_bytes_read);
+
+      // Something went wrong and the address is not where it should be
+      // we'll error out of this Minidump generation.
+      addDataError = Status::FromErrorStringWithFormat(
+          "Unexpected address encounterd when reading memory in chunks "
+          "0x%" PRIx64 " expected 0x%" PRIx64,
+          current_addr, addr + total_bytes_read);
+      return lldb_private::IterationAction::Stop;
+    }
+
     // Write to the minidump file with the chunk potentially flushing to
     // disk.
     // This error will be captured by the outer scope and is considered fatal.
@@ -1006,13 +1006,13 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
     if (addDataError.Fail())
       return lldb_private::IterationAction::Stop;
 
+    total_bytes_read += bytes_read;
     // If we have a partial read, report it, but only if the partial read
     // didn't finish reading the entire region.
-    if (bytes_read != data_buffer.GetByteSize() &&
-        current_addr + bytes_read != size) {
+    if (bytes_read != data_buffer.GetByteSize() && total_bytes_read != size) {
       LLDB_LOGF(log,
-                "Memory region at: %" PRIx64 " partiall read 0x%" PRIx64
-                " bytes out of %" PRIx64 " bytes.",
+                "Memory region at: 0x%" PRIx64 " partial read 0x%" PRIx64
+                " bytes out of 0x%" PRIx64 " bytes.",
                 current_addr, bytes_read,
                 data_buffer.GetByteSize() - bytes_read);
 
@@ -1059,7 +1059,7 @@ MinidumpFileBuilder::AddMemoryList_32(std::vector<CoreFileMemoryRange> &ranges,
 
     LLDB_LOGF(log,
               "AddMemoryList %zu/%zu reading memory for region "
-              "(%" PRIx64 " bytes) [%" PRIx64 ", %" PRIx64 ")",
+              "(0x%" PRIx64 " bytes) [0x%" PRIx64 ", 0x%" PRIx64 ")",
               region_index, ranges.size(), size, addr, addr + size);
     ++region_index;
 
@@ -1117,7 +1117,7 @@ MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
     return error;
 
   error = AddDirectory(StreamType::Memory64List,
-                       (sizeof(llvm::support::ulittle64_t) * 2) +
+                       (sizeof(llvm::minidump::Memory64ListHeader)) +
                            ranges.size() *
                                sizeof(llvm::minidump::MemoryDescriptor_64));
   if (error.Fail())
@@ -1130,9 +1130,9 @@ MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
   // Capture the starting offset for all the descriptors so we can clean them up
   // if needed.
   offset_t starting_offset =
-      GetCurrentDataEndOffset() + sizeof(llvm::support::ulittle64_t);
+      GetCurrentDataEndOffset() + sizeof(llvm::minidump::Memory64ListHeader);
   // The base_rva needs to start after the directories, which is right after
-  // this 8 byte variable.
+  // the descriptors + the size of the header.
   offset_t base_rva =
       starting_offset +
       (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));