https://github.com/clayborg created 
https://github.com/llvm/llvm-project/pull/71772

Prior to this patch, each core file plugin (ObjectFileMachO.cpp and 
ObjectFileMinindump.cpp) would calculate the address ranges to save in 
different ways. This patch adds a new function to Process.h/.cpp:

```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, 
CoreFileMemoryRanges &ranges);
```

The patch updates the ObjectFileMachO::SaveCore(...) and 
ObjectFileMinindump::SaveCore(...) to use same code. This will allow core files 
to be consistent with the lldb::SaveCoreStyle across different core file 
creators and will allow us to add new core file saving features that do more 
complex things in future patches.

>From 9ff1e92175f90613de6f07dfb4821b8fc0226939 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayb...@gmail.com>
Date: Wed, 8 Nov 2023 21:14:49 -0800
Subject: [PATCH] Centralize the code that figures out which memory ranges to
 save into core files.

Prior to this patch, each core file plugin (ObjectFileMachO.cpp and 
ObjectFileMinindump.cpp) would calculate the address ranges to save in 
different ways. This patch adds a new function to Process.h/.cpp:

```
Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, 
CoreFileMemoryRanges &ranges);
```

The patch updates the ObjectFileMachO::SaveCore(...) and 
ObjectFileMinindump::SaveCore(...) to use same code. This will allow core files 
to be consistent with the lldb::SaveCoreStyle across different core file 
creators and will allow us to add new core file saving features that do more 
complex things in future patches.
---
 lldb/include/lldb/Target/MemoryRegionInfo.h   |   8 +-
 lldb/include/lldb/Target/Process.h            |  43 +++-
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp     | 137 +++---------
 .../Minidump/MinidumpFileBuilder.cpp          |  37 +---
 .../ObjectFile/Minidump/MinidumpFileBuilder.h |   3 +-
 .../Minidump/ObjectFileMinidump.cpp           |   9 +-
 lldb/source/Target/Process.cpp                | 196 +++++++++++++++++-
 lldb/source/Target/TraceDumper.cpp            |   7 +-
 8 files changed, 274 insertions(+), 166 deletions(-)

diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index 47d4c9d6398728c..66a4b3ed1b42d5f 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -81,11 +81,11 @@ class MemoryRegionInfo {
   // lldb::Permissions
   uint32_t GetLLDBPermissions() const {
     uint32_t permissions = 0;
-    if (m_read)
+    if (m_read == eYes)
       permissions |= lldb::ePermissionsReadable;
-    if (m_write)
+    if (m_write == eYes)
       permissions |= lldb::ePermissionsWritable;
-    if (m_execute)
+    if (m_execute == eYes)
       permissions |= lldb::ePermissionsExecutable;
     return permissions;
   }
@@ -151,7 +151,7 @@ class MemoryRegionInfo {
   int m_pagesize = 0;
   std::optional<std::vector<lldb::addr_t>> m_dirty_pages;
 };
-  
+
 inline bool operator<(const MemoryRegionInfo &lhs,
                       const MemoryRegionInfo &rhs) {
   return lhs.GetRange() < rhs.GetRange();
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..335cb64768a682d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -54,6 +54,7 @@
 #include "lldb/Utility/UserIDResolver.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/ADT/AddressRanges.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/VersionTuple.h"
@@ -354,11 +355,11 @@ class Process : public 
std::enable_shared_from_this<Process>,
   };
   // This is all the event bits the public process broadcaster broadcasts.
   // The process shadow listener signs up for all these bits...
-  static constexpr int g_all_event_bits = eBroadcastBitStateChanged 
+  static constexpr int g_all_event_bits = eBroadcastBitStateChanged
                                         | eBroadcastBitInterrupt
-                                        | eBroadcastBitSTDOUT 
+                                        | eBroadcastBitSTDOUT
                                         | eBroadcastBitSTDERR
-                                        | eBroadcastBitProfileData 
+                                        | eBroadcastBitProfileData
                                         | eBroadcastBitStructuredData;
 
   enum {
@@ -390,7 +391,7 @@ class Process : public 
std::enable_shared_from_this<Process>,
   ConstString &GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
-  
+
 /// A notification structure that can be used by clients to listen
 /// for changes in a process's lifetime.
 ///
@@ -579,7 +580,7 @@ class Process : public 
std::enable_shared_from_this<Process>,
   ///     of CommandObject like CommandObjectRaw, CommandObjectParsed,
   ///     or CommandObjectMultiword.
   virtual CommandObject *GetPluginCommandObject() { return nullptr; }
-  
+
   /// The underlying plugin might store the low-level communication history for
   /// this session.  Dump it into the provided stream.
   virtual void DumpPluginHistory(Stream &s) { return; }
@@ -614,7 +615,7 @@ class Process : public 
std::enable_shared_from_this<Process>,
     return error;
   }
 
-  /// The "ShadowListener" for a process is just an ordinary Listener that 
+  /// The "ShadowListener" for a process is just an ordinary Listener that
   /// listens for all the Process event bits.  It's convenient because you can
   /// specify it in the LaunchInfo or AttachInfo, so it will get events from
   /// the very start of the process.
@@ -704,7 +705,37 @@ class Process : public 
std::enable_shared_from_this<Process>,
   ///     is not supported by the plugin, error otherwise.
   virtual llvm::Expected<bool> SaveCore(llvm::StringRef outfile);
 
+  struct CoreFileMemoryRange {
+    llvm::AddressRange range; /// The address range to save into the core file.
+    uint32_t lldb_permissions; /// A bit set of lldb::Permissions bits.
+
+    bool operator==(const CoreFileMemoryRange &rhs) const {
+      return range == rhs.range && lldb_permissions == rhs.lldb_permissions;
+    }
+
+    bool operator!=(const CoreFileMemoryRange &rhs) const {
+      return !(*this == rhs);
+    }
+
+    bool operator<(const CoreFileMemoryRange &rhs) const {
+      if (range < rhs.range)
+        return true;
+      if (range == rhs.range)
+        return lldb_permissions < rhs.lldb_permissions;
+      return false;
+    }
+  };
+
+  using CoreFileMemoryRanges = std::vector<CoreFileMemoryRange>;
+
+  /// Helper function for Process::SaveCore(...) that calculates the address
+  /// ranges that should be save. This allows all core file plug-ins to save
+  /// consistent memory ranges given a \a core_style.
+  Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+                                     CoreFileMemoryRanges &ranges);
+
 protected:
+
   virtual JITLoaderList &GetJITLoaders();
 
 public:
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3714e37ec5c57d0..73b338639b13163 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6474,9 +6474,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
&process_sp,
     return false;
 
   // Default on macOS is to create a dirty-memory-only corefile.
-  if (core_style == SaveCoreStyle::eSaveCoreUnspecified) {
+  if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
     core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
-  }
 
   Target &target = process_sp->GetTarget();
   const ArchSpec target_arch = target.GetArchitecture();
@@ -6505,115 +6504,42 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
&process_sp,
     }
 
     if (make_core) {
-      std::vector<llvm::MachO::segment_command_64> segment_load_commands;
-      //                uint32_t range_info_idx = 0;
-      MemoryRegionInfo range_info;
-      Status range_error = process_sp->GetMemoryRegionInfo(0, range_info);
-      const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
-      const ByteOrder byte_order = target_arch.GetByteOrder();
-      std::vector<page_object> pages_to_copy;
-
-      if (range_error.Success()) {
-        while (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS) {
-          // Calculate correct protections
-          uint32_t prot = 0;
-          if (range_info.GetReadable() == MemoryRegionInfo::eYes)
-            prot |= VM_PROT_READ;
-          if (range_info.GetWritable() == MemoryRegionInfo::eYes)
-            prot |= VM_PROT_WRITE;
-          if (range_info.GetExecutable() == MemoryRegionInfo::eYes)
-            prot |= VM_PROT_EXECUTE;
-
-          const addr_t addr = range_info.GetRange().GetRangeBase();
-          const addr_t size = range_info.GetRange().GetByteSize();
-
-          if (size == 0)
-            break;
-
-          bool include_this_region = true;
-          bool dirty_pages_only = false;
-          if (core_style == SaveCoreStyle::eSaveCoreStackOnly) {
-            dirty_pages_only = true;
-            if (range_info.IsStackMemory() != MemoryRegionInfo::eYes) {
-              include_this_region = false;
-            }
-          }
-          if (core_style == SaveCoreStyle::eSaveCoreDirtyOnly) {
-            dirty_pages_only = true;
-          }
-
-          if (prot != 0 && include_this_region) {
-            addr_t pagesize = range_info.GetPageSize();
-            const std::optional<std::vector<addr_t>> &dirty_page_list =
-                range_info.GetDirtyPageList();
-            if (dirty_pages_only && dirty_page_list) {
-              for (addr_t dirtypage : *dirty_page_list) {
-                page_object obj;
-                obj.addr = dirtypage;
-                obj.size = pagesize;
-                obj.prot = prot;
-                pages_to_copy.push_back(obj);
-              }
-            } else {
-              page_object obj;
-              obj.addr = addr;
-              obj.size = size;
-              obj.prot = prot;
-              pages_to_copy.push_back(obj);
-            }
-          }
-
-          range_error = process_sp->GetMemoryRegionInfo(
-              range_info.GetRange().GetRangeEnd(), range_info);
-          if (range_error.Fail())
-            break;
-        }
-
-        // Combine contiguous entries that have the same
-        // protections so we don't have an excess of
-        // load commands.
-        std::vector<page_object> combined_page_objects;
-        page_object last_obj;
-        last_obj.addr = LLDB_INVALID_ADDRESS;
-        last_obj.size = 0;
-        for (page_object obj : pages_to_copy) {
-          if (last_obj.addr == LLDB_INVALID_ADDRESS) {
-            last_obj = obj;
-            continue;
-          }
-          if (last_obj.addr + last_obj.size == obj.addr &&
-              last_obj.prot == obj.prot) {
-            last_obj.size += obj.size;
-            continue;
-          }
-          combined_page_objects.push_back(last_obj);
-          last_obj = obj;
-        }
-        // Add the last entry we were looking to combine
-        // on to the array.
-        if (last_obj.addr != LLDB_INVALID_ADDRESS && last_obj.size != 0)
-          combined_page_objects.push_back(last_obj);
-
-        for (page_object obj : combined_page_objects) {
+      Process::CoreFileMemoryRanges core_ranges;
+      error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
+      if (error.Success()) {
+        const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
+        const ByteOrder byte_order = target_arch.GetByteOrder();
+        std::vector<llvm::MachO::segment_command_64> segment_load_commands;
+        for (const auto &core_range : core_ranges) {
           uint32_t cmd_type = LC_SEGMENT_64;
           uint32_t segment_size = sizeof(llvm::MachO::segment_command_64);
           if (addr_byte_size == 4) {
             cmd_type = LC_SEGMENT;
             segment_size = sizeof(llvm::MachO::segment_command);
           }
+          // Skip any ranges with no read/write/execute permissions and empty
+          // ranges.
+          if (core_range.lldb_permissions == 0 || core_range.range.size() == 0)
+            continue;
+          uint32_t vm_prot = 0;
+          if (core_range.lldb_permissions & ePermissionsReadable)
+            vm_prot |= VM_PROT_READ;
+          if (core_range.lldb_permissions & ePermissionsWritable)
+            vm_prot |= VM_PROT_WRITE;
+          if (core_range.lldb_permissions & ePermissionsExecutable)
+            vm_prot |= VM_PROT_EXECUTE;
+          const addr_t vm_addr = core_range.range.start();
+          const addr_t vm_size = core_range.range.size();
           llvm::MachO::segment_command_64 segment = {
               cmd_type,     // uint32_t cmd;
               segment_size, // uint32_t cmdsize;
               {0},          // char segname[16];
-              obj.addr,     // uint64_t vmaddr;    // uint32_t for 32-bit
-                            // Mach-O
-              obj.size,     // uint64_t vmsize;    // uint32_t for 32-bit
-                            // Mach-O
-              0,            // uint64_t fileoff;   // uint32_t for 32-bit 
Mach-O
-              obj.size,     // uint64_t filesize;  // uint32_t for 32-bit
-                            // Mach-O
-              obj.prot,     // uint32_t maxprot;
-              obj.prot,     // uint32_t initprot;
+              vm_addr,      // uint64_t vmaddr;   // uint32_t for 32-bit Mach-O
+              vm_size,      // uint64_t vmsize;   // uint32_t for 32-bit Mach-O
+              0,            // uint64_t fileoff;  // uint32_t for 32-bit Mach-O
+              vm_size,      // uint64_t filesize; // uint32_t for 32-bit Mach-O
+              vm_prot,      // uint32_t maxprot;
+              vm_prot,      // uint32_t initprot;
               0,            // uint32_t nsects;
               0};           // uint32_t flags;
           segment_load_commands.push_back(segment);
@@ -6622,11 +6548,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
&process_sp,
         StreamString buffer(Stream::eBinary, addr_byte_size, byte_order);
 
         llvm::MachO::mach_header_64 mach_header;
-        if (addr_byte_size == 8) {
-          mach_header.magic = MH_MAGIC_64;
-        } else {
-          mach_header.magic = MH_MAGIC;
-        }
+        mach_header.magic = addr_byte_size == 8 ? MH_MAGIC_64 : MH_MAGIC;
         mach_header.cputype = target_arch.GetMachOCPUType();
         mach_header.cpusubtype = target_arch.GetMachOCPUSubType();
         mach_header.filetype = MH_CORE;
@@ -6911,9 +6833,6 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP 
&process_sp,
             }
           }
         }
-      } else {
-        error.SetErrorString(
-            "process doesn't support getting memory region info");
       }
     }
     return true; // This is the right plug to handle saving core files for
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c396cb061c01776..e8e0d09b5324d0f 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -557,43 +557,24 @@ Status MinidumpFileBuilder::AddException(const 
lldb::ProcessSP &process_sp) {
 }
 
 lldb_private::Status
-MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp,
+                                   lldb::SaveCoreStyle core_style) {
   Status error;
-
+  Process::CoreFileMemoryRanges core_ranges;
+  error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges);
   if (error.Fail()) {
     error.SetErrorString("Process doesn't support getting memory region 
info.");
     return error;
   }
 
-  // Get interesting addresses
-  std::vector<size_t> interesting_addresses;
-  auto thread_list = process_sp->GetThreadList();
-  for (size_t i = 0; i < thread_list.GetSize(); ++i) {
-    ThreadSP thread_sp(thread_list.GetThreadAtIndex(i));
-    RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
-    RegisterContext *reg_ctx = reg_ctx_sp.get();
-
-    interesting_addresses.push_back(read_register_u64(reg_ctx, "rsp"));
-    interesting_addresses.push_back(read_register_u64(reg_ctx, "rip"));
-  }
-
   DataBufferHeap helper_data;
   std::vector<MemoryDescriptor> mem_descriptors;
-
-  std::set<addr_t> visited_region_base_addresses;
-  for (size_t interesting_address : interesting_addresses) {
-    MemoryRegionInfo range_info;
-    error = process_sp->GetMemoryRegionInfo(interesting_address, range_info);
-    // Skip failed memory region requests or any regions with no permissions.
-    if (error.Fail() || range_info.GetLLDBPermissions() == 0)
-      continue;
-    const addr_t addr = range_info.GetRange().GetRangeBase();
-    // Skip any regions we have already saved out.
-    if (visited_region_base_addresses.insert(addr).second == false)
-      continue;
-    const addr_t size = range_info.GetRange().GetByteSize();
-    if (size == 0)
+  for (const auto &core_range : core_ranges) {
+    // Skip empty memory regions or any regions with no permissions.
+    if (core_range.range.empty() || core_range.lldb_permissions == 0)
       continue;
+    const addr_t addr = core_range.range.start();
+    const addr_t size = core_range.range.size();
     auto data_up = std::make_unique<DataBufferHeap>(size, 0);
     const size_t bytes_read =
         process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index f4017fb663840ec..cae355799fa7247 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -64,7 +64,8 @@ class MinidumpFileBuilder {
   // failed status.
   lldb_private::Status AddException(const lldb::ProcessSP &process_sp);
   // Add MemoryList stream, containing dumps of important memory segments
-  lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp);
+  lldb_private::Status AddMemoryList(const lldb::ProcessSP &process_sp,
+                                     lldb::SaveCoreStyle core_style);
   // Add MiscInfo stream, mainly providing ProcessId
   void AddMiscInfo(const lldb::ProcessSP &process_sp);
   // Add informative files about a Linux process
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 17b37afe557d914..f5294b2f08c66e1 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -57,10 +57,9 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
&process_sp,
                                   const lldb_private::FileSpec &outfile,
                                   lldb::SaveCoreStyle &core_style,
                                   lldb_private::Status &error) {
-  if (core_style != SaveCoreStyle::eSaveCoreStackOnly) {
-    error.SetErrorString("Only stack minidumps supported yet.");
-    return false;
-  }
+  // Set default core style if it isn't set.
+  if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
+    core_style = SaveCoreStyle::eSaveCoreStackOnly;
 
   if (!process_sp)
     return false;
@@ -88,7 +87,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
&process_sp,
     if (error.Fail())
       return false;
 
-    error = builder.AddMemoryList(process_sp);
+    error = builder.AddMemoryList(process_sp, core_style);
     if (error.Fail())
       return false;
   }
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f82ab05362fbee9..4277555e52195e0 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2404,16 +2404,7 @@ bool Process::GetLoadAddressPermissions(lldb::addr_t 
load_addr,
       range_info.GetExecutable() == MemoryRegionInfo::eDontKnow) {
     return false;
   }
-
-  if (range_info.GetReadable() == MemoryRegionInfo::eYes)
-    permissions |= lldb::ePermissionsReadable;
-
-  if (range_info.GetWritable() == MemoryRegionInfo::eYes)
-    permissions |= lldb::ePermissionsWritable;
-
-  if (range_info.GetExecutable() == MemoryRegionInfo::eYes)
-    permissions |= lldb::ePermissionsExecutable;
-
+  permissions = range_info.GetLLDBPermissions();
   return true;
 }
 
@@ -6252,3 +6243,188 @@ Status Process::WriteMemoryTags(lldb::addr_t addr, 
size_t len,
   return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
                            *packed_tags);
 }
+
+// Create a CoreFileMemoryRange from a MemoryRegionInfo
+static Process::CoreFileMemoryRange
+CreateCoreFileMemoryRange(const MemoryRegionInfo &region) {
+  const addr_t addr = region.GetRange().GetRangeBase();
+  llvm::AddressRange range(addr, addr + region.GetRange().GetByteSize());
+  return {range, region.GetLLDBPermissions()};
+}
+
+// Add dirty pages to the core file ranges and return true if dirty pages
+// were added. Return false if the dirty page information is not valid or in
+// the region.
+static bool AddDirtyPages(const MemoryRegionInfo &region,
+                          Process::CoreFileMemoryRanges &ranges) {
+  const auto &dirty_page_list = region.GetDirtyPageList();
+  if (!dirty_page_list)
+    return false;
+  const uint32_t lldb_permissions = region.GetLLDBPermissions();
+  const addr_t page_size = region.GetPageSize();
+  if (page_size == 0)
+    return false;
+  llvm::AddressRange range(0, 0);
+  for (addr_t page_addr : *dirty_page_list) {
+    if (range.empty()) {
+      // No range yet, initialize the range with the current dirty page.
+      range = llvm::AddressRange(page_addr, page_addr + page_size);
+    } else {
+      if (range.end() == page_addr) {
+        // Combine consective ranges.
+        range = llvm::AddressRange(range.start(), page_addr + page_size);
+      } else {
+        // Add previous contiguous range and init the new range with the
+        // current dirty page.
+        ranges.push_back({range, lldb_permissions});
+        range = llvm::AddressRange(page_addr, page_addr + page_size);
+      }
+    }
+  }
+  // The last range
+  if (!range.empty())
+    ranges.push_back({range, lldb_permissions});
+  return true;
+}
+
+// Given a region, add the region to \a ranges.
+//
+// Only add the region if it isn't empty and if it has some permissions.
+// If \a try_dirty_pages is true, then try to add only the dirty pages for a
+// given region. If the region has dirty page information, only dirty pages
+// will be added to \a ranges, else the entire range will be added to \a
+// ranges.
+static void AddRegion(const MemoryRegionInfo &region, bool try_dirty_pages,
+                      Process::CoreFileMemoryRanges &ranges) {
+  // Don't add empty ranges or ranges with no permissions.
+  if (region.GetRange().GetByteSize() == 0 || region.GetLLDBPermissions() == 0)
+    return;
+  if (try_dirty_pages && AddDirtyPages(region, ranges))
+    return;
+  ranges.push_back(CreateCoreFileMemoryRange(region));
+}
+
+// Save all memory regions that are not empty or have at least some permissions
+// for a full core file style.
+static Status
+GetCoreFileSaveRangesFull(Process &process,
+                          const MemoryRegionInfos &regions,
+                          Process::CoreFileMemoryRanges &ranges) {
+
+  // Don't add only dirty pages, add full regions.
+  const bool try_dirty_pages = false;
+  for (const auto &region: regions)
+    AddRegion(region, try_dirty_pages, ranges);
+  return Status();
+}
+
+// Save only the dirty pages to the core file. Make sure the process has at
+// least some dirty pages, as some OS versions don't support reporting what
+// pages are dirty within an memory region. If no memory regions have dirty
+// page information, return an error.
+static Status
+GetCoreFileSaveRangesDirtyOnly(Process &process,
+                               const MemoryRegionInfos &regions,
+                               Process::CoreFileMemoryRanges &ranges) {
+  // Iterate over the regions and find all dirty pages.
+  bool have_dirty_page_info = false;
+  for (const auto &region: regions) {
+    if (AddDirtyPages(region, ranges))
+      have_dirty_page_info = true;
+  }
+
+  if (!have_dirty_page_info)
+    return Status("no process memory regions have dirty page information");
+
+  return Status();
+}
+
+// Save all thread stacks to the core file. Some OS versions support reporting
+// when a memory region is stack related. We check on this information, but we
+// also use the stack pointers of each thread and add those in case the OS
+// doesn't support reporting stack memory. This function does unique the stack
+// regions and won't add the same range twice. This function also attempts to
+// only emit dirty pages from the stack if the memory regions support reporting
+// dirty regions. If the process doesn't support dirty regions, then it will
+// fall back to adding the full stack region.
+static Status
+GetCoreFileSaveRangesStackOnly(Process &process,
+                               const MemoryRegionInfos &regions,
+                               Process::CoreFileMemoryRanges &ranges) {
+  // Some platforms support annotating the region information that tell us that
+  // it comes from a thread stack. So look for those regions first.
+
+  // Keep track of which stack regions we have added
+  std::set<addr_t> stack_bases;
+
+  const bool try_dirty_pages = true;
+  for (const auto &region: regions) {
+    if (region.IsStackMemory() == MemoryRegionInfo::eYes) {
+      stack_bases.insert(region.GetRange().GetRangeBase());
+      AddRegion(region, try_dirty_pages, ranges);
+    }
+  }
+
+  // Also check with our threads and get the regions for their stack pointers
+  // and add those regions if not already added above
+  ThreadList &thread_list = process.GetThreadList();
+  const size_t num_threads = thread_list.GetSize();
+  for (size_t idx=0; idx<num_threads; ++idx) {
+    lldb::ThreadSP thread_sp =
+        thread_list.GetThreadAtIndex(idx, /*can_update*/false);
+    if (!thread_sp)
+      continue;
+    StackFrameSP frame_sp = thread_sp->GetStackFrameAtIndex(0);
+    if (!frame_sp)
+      continue;
+    RegisterContextSP reg_ctx_sp = frame_sp->GetRegisterContext();
+    if (!reg_ctx_sp)
+      continue;
+    const addr_t sp = reg_ctx_sp->GetSP();
+    lldb_private::MemoryRegionInfo sp_region;
+    if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
+      // Only add this region if not already added above. If our stack pointer
+      // is pointing off in the weeds, we will want this range.
+      if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0)
+         AddRegion(sp_region, try_dirty_pages, ranges);
+    }
+  }
+  return Status();
+}
+
+Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+                                            CoreFileMemoryRanges &ranges) {
+  lldb_private::MemoryRegionInfos regions;
+  Status err = GetMemoryRegions(regions);
+  if (err.Fail())
+    return err;
+  if (regions.empty())
+    return Status("failed to get any valid memory regions from the process");
+
+  switch (core_style) {
+  case eSaveCoreUnspecified:
+    err = Status("callers must set the core_style to something other than "
+                 "eSaveCoreUnspecified");
+    break;
+
+  case eSaveCoreFull:
+    err = GetCoreFileSaveRangesFull(*this, regions, ranges);
+    break;
+
+  case eSaveCoreDirtyOnly:
+    err = GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges);
+    break;
+
+  case eSaveCoreStackOnly:
+    err = GetCoreFileSaveRangesStackOnly(*this, regions, ranges);
+    break;
+  }
+
+  if (err.Fail())
+    return err;
+
+  if (ranges.empty())
+    return Status("no valid address ranges found for core style");
+
+  return Status(); // Success!
+}
diff --git a/lldb/source/Target/TraceDumper.cpp 
b/lldb/source/Target/TraceDumper.cpp
index d059d443805c5af..e92419e70b32baa 100644
--- a/lldb/source/Target/TraceDumper.cpp
+++ b/lldb/source/Target/TraceDumper.cpp
@@ -472,7 +472,7 @@ TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() {
 static SymbolContext
 CalculateSymbolContext(const Address &address,
                        const SymbolContext &prev_symbol_context) {
-  AddressRange range;
+  lldb_private::AddressRange range;
   if (prev_symbol_context.GetAddressRange(eSymbolContextEverything, 0,
                                           /*inline_block_range*/ true, range) 
&&
       range.Contains(address))
@@ -508,7 +508,8 @@ CalculateDisass(const TraceDumper::SymbolInfo &symbol_info,
   // We fallback to a single instruction disassembler
   Target &target = exe_ctx.GetTargetRef();
   const ArchSpec arch = target.GetArchitecture();
-  AddressRange range(symbol_info.address, arch.GetMaximumOpcodeByteSize());
+  lldb_private::AddressRange range(symbol_info.address,
+                                   arch.GetMaximumOpcodeByteSize());
   DisassemblerSP disassembler =
       Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
                                      /*flavor*/ nullptr, target, range);
@@ -778,7 +779,7 @@ static TraceDumper::FunctionCall 
&AppendInstructionToFunctionCallForest(
     return *roots.back();
   }
 
-  AddressRange range;
+  lldb_private::AddressRange range;
   if (symbol_info.sc.GetAddressRange(
           eSymbolContextBlock | eSymbolContextFunction | eSymbolContextSymbol,
           0, /*inline_block_range*/ true, range)) {

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to