[LLDB][SBSaveCore] Sbsavecore subregions bug

[Jlalond]

Custom regions in Process::GetUserSpecifiedCoreFileSaveRanges originally used FindEntryThatContains. This made sense on my first attempt, but what we really want are intersecting regions. This is so the user can specify arbitrary memory, and if it's available we output it to the core (Minidump or MachO).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

Custom regions in Process::GetUserSpecifiedCoreFileSaveRanges originally used entry that contains. This made sense on my first attempt, but what we really want are intersecting regions. This is so the user can specify arbitrary memory, and if it's available we output it to the core (Minidump or MachO).

---

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

4 Files Affected:

• (modified) lldb/include/lldb/Utility/RangeMap.h (+19)
• (modified) lldb/source/Target/Process.cpp (+29-3)
• (added) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py (+142)
• (added) lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml (+35)

diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index 8af690e813c4a..2f7711c1eb11e 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -380,6 +380,25 @@ template <typename B, typename S, unsigned N = 0> class RangeVector {
     return nullptr;
   }
 
+  const Entry *FindEntryThatIntersects(const Entry &range) const {
+#ifdef ASSERT_RANGEMAP_ARE_SORTED
+    assert(IsSorted());
+#endif
+    if (!m_entries.empty()) {
+      typename Collection::const_iterator begin = m_entries.begin();
+      typename Collection::const_iterator end = m_entries.end();
+      typename Collection::const_iterator pos =
+          std::lower_bound(begin, end, range, BaseLessThan);
+
+      while (pos != begin && pos[-1].DoesIntersect(range))
+        --pos;
+
+      if (pos != end && pos->DoesIntersect(range))
+        return &(*pos);
+    }
+    return nullptr;
+  }
+
   using const_iterator = typename Collection::const_iterator;
   const_iterator begin() const { return m_entries.begin(); }
   const_iterator end() const { return m_entries.end(); }
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 73557eb767c72..7d6cded48b21a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6689,6 +6689,25 @@ static void GetCoreFileSaveRangesStackOnly(Process &process,
   }
 }
 
+// TODO: We should refactor CoreFileMemoryRanges to use the lldb range type, and
+// then add an intersect method on it, or MemoryRegionInfo.
+static MemoryRegionInfo
+Intersect(const MemoryRegionInfo &lhs,
+          const Range<lldb::addr_t, lldb::addr_t> &rhs) {
+  const lldb::addr_t lhs_base = lhs.GetRange().GetRangeBase();
+  const lldb::addr_t rhs_base = rhs.GetRangeBase();
+  const lldb::addr_t lhs_end = lhs.GetRange().GetRangeEnd();
+  const lldb::addr_t rhs_end = rhs.GetRangeEnd();
+
+  MemoryRegionInfo region_info;
+  region_info.SetLLDBPermissions(lhs.GetLLDBPermissions());
+  auto &range = region_info.GetRange();
+  range.SetRangeBase(std::max(lhs_base, rhs_base));
+  range.SetRangeEnd(std::min(lhs_end, rhs_end));
+
+  return region_info;
+}
+
 static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
                                                const MemoryRegionInfos &regions,
                                                const SaveCoreOptions &options,
@@ -6698,9 +6717,16 @@ static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
     return;
 
   for (const auto &range : regions) {
-    auto entry = option_ranges.FindEntryThatContains(range.GetRange());
-    if (entry)
-      AddRegion(range, true, ranges);
+    auto *entry = option_ranges.FindEntryThatIntersects(range.GetRange());
+    if (entry) {
+      if (entry->GetRangeBase() != range.GetRange().GetRangeBase() ||
+          entry->GetRangeEnd() != range.GetRange().GetRangeEnd()) {
+        AddRegion(Intersect(range, *entry), true, ranges);
+      } else {
+        // If they match, add the range directly.
+        AddRegion(range, true, ranges);
+      }
+    }
   }
 }
 
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py
new file mode 100644
index 0000000000000..b8f9d697c8864
--- /dev/null
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py
@@ -0,0 +1,142 @@
+"""
+Test saving a mini dump, from yamilized examples.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ProcessSaveCoreMinidumpTestCaseYaml(TestBase):
+       def process_from_yaml(self, yaml_file):
+              minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
+              self.yaml2obj(yaml_file, minidump_path)
+              self.target = self.dbg.CreateTarget(None)
+              self.process = self.target.LoadCore(minidump_path)
+              return self.process
+
+       def test_saving_sub_memory_range(self):
+              """
+              Validate we can save a Minidump for a subsection of a memory range.
+              I.E.
+              If our memory range is 0x1000-0x2000 nd the user specifies 0x1200-0x1800 
+              we should still capture 0x1200 to 0x1800
+              """
+              yaml = "minidump_mem64.yaml"
+              proc = self.process_from_yaml(yaml)
+              new_minidump_path = self.getBuildArtifact(__name__ + ".dmp")
+              options = lldb.SBSaveCoreOptions()
+              options.SetOutputFile(lldb.SBFileSpec(new_minidump_path))
+              options.SetPluginName("minidump")
+              options.SetStyle(lldb.eSaveCoreCustomOnly)
+
+              size = 8
+              begin = 0x7FFF12A84030
+              end = begin + size
+              custom_range = lldb.SBMemoryRegionInfo("", begin, end, 3, True, False)
+              options.AddMemoryRegionToSave(custom_range)
+              
+              error = proc.SaveCore(options)
+              self.assertTrue(error.Success(), error.GetCString())
+              core_target = self.dbg.CreateTarget(None)
+              core_process = core_target.LoadCore(new_minidump_path)
+
+              error = lldb.SBError()
+              core_process.ReadMemory(begin, size, error)
+              self.assertTrue(error.Success(), error.GetCString())
+
+              # Try to read 1 byte past the end
+              core_process.ReadMemory(end + 1, 1, error)
+              self.assertTrue(error.Fail(), error.GetCString())
+
+       def test_saving_super_memory_range(self):
+              """
+              Validate we can save a Minidump for a subsection of a memory range.
+              I.E.
+              If our memory range is 0x1000-0x2000 nd the user specifies 0x0800-0x2800 
+              we should still capture 0x1000-0x2000
+              """
+              yaml = "minidump_mem64.yaml"
+              proc = self.process_from_yaml(yaml)
+              new_minidump_path = self.getBuildArtifact(__name__ + ".dmp")
+              options = lldb.SBSaveCoreOptions()
+              options.SetOutputFile(lldb.SBFileSpec(new_minidump_path))
+              options.SetPluginName("minidump")
+              options.SetStyle(lldb.eSaveCoreCustomOnly)
+
+              size = 0x2FD0
+              begin = 0x7FFF12A84030
+              end = begin + size
+              custom_range = lldb.SBMemoryRegionInfo("", begin - 16, end + 16, 3, True, False)
+              options.AddMemoryRegionToSave(custom_range)
+              
+              error = proc.SaveCore(options)
+              self.assertTrue(error.Success(), error.GetCString())
+              core_target = self.dbg.CreateTarget(None)
+              core_process = core_target.LoadCore(new_minidump_path)
+
+              error = lldb.SBError()
+              core_process.ReadMemory(begin, size, error)
+              self.assertTrue(error.Success(), error.GetCString())
+
+
+       def test_region_that_goes_out_of_bounds(self):
+              """
+              Validate we can save a Minidump for a custom region
+              that includes an end that enters an invalid (---) page.
+              """
+              yaml = "minidump_mem64.yaml"
+              proc = self.process_from_yaml(yaml)
+              new_minidump_path = self.getBuildArtifact(__name__ + ".dmp")
+              options = lldb.SBSaveCoreOptions()
+              options.SetOutputFile(lldb.SBFileSpec(new_minidump_path))
+              options.SetPluginName("minidump")
+              options.SetStyle(lldb.eSaveCoreCustomOnly)
+
+              size = 1024
+              begin = 0x00007fff12a8ffff
+              end = begin + size
+              custom_range = lldb.SBMemoryRegionInfo("", begin, end, 3, True, False)
+              options.AddMemoryRegionToSave(custom_range)
+              
+              error = proc.SaveCore(options)
+              self.assertTrue(error.Success(), error.GetCString())
+              core_target = self.dbg.CreateTarget(None)
+              core_process = core_target.LoadCore(new_minidump_path)
+
+              error = lldb.SBError()
+              core_process.ReadMemory(begin, 0x00000020, error)
+              self.assertTrue(error.Success(), error.GetCString())
+
+              # Whole region should be unavailable
+              core_process.ReadMemory(end, 1, error)
+              self.assertTrue(error.Fail(), error.GetCString())
+
+       def test_region_that_starts_out_of_bounds(self):
+              """
+              Validate we can save a Minidump for a custom region
+              that includes a start in a (---) page but ends in a valid page.
+              """
+              yaml = "minidump_mem64.yaml"
+              proc = self.process_from_yaml(yaml)
+              new_minidump_path = self.getBuildArtifact(__name__ + ".dmp")
+              options = lldb.SBSaveCoreOptions()
+              options.SetOutputFile(lldb.SBFileSpec(new_minidump_path))
+              options.SetPluginName("minidump")
+              options.SetStyle(lldb.eSaveCoreCustomOnly)
+
+              size = 0x00000020
+              begin = 0x00007fff12a8ffff
+              end = begin + size
+              custom_range = lldb.SBMemoryRegionInfo("", begin - 16, end, 3, True, False)
+              options.AddMemoryRegionToSave(custom_range)
+              
+              error = proc.SaveCore(options)
+              self.assertTrue(error.Success(), error.GetCString())
+              core_target = self.dbg.CreateTarget(None)
+              core_process = core_target.LoadCore(new_minidump_path)
+
+              error = lldb.SBError()
+              core_process.ReadMemory(begin, 0x00000020, error)
+              self.assertTrue(error.Success(), error.GetCString())
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml b/lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml
new file mode 100644
index 0000000000000..24fdcc8aed74a
--- /dev/null
+++ b/lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml
@@ -0,0 +1,35 @@
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Processor Level: 6
+    Processor Revision: 15876
+    Number of Processors: 40
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13.0-91-generic'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x2896BB
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000700100000000000FFFFFFFF0000FFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B040A812FF7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000050D0A75BBA7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x0
+          Content:         ''
+  - Type:            Memory64List
+    Memory Ranges:
+      - Start of Memory Range: 0x7FFF12A84030
+        Data Size:       0x2FD0
+        Content :        ''
+      - Start of Memory Range: 0x00007fff12a87000
+        Data Size:       0x00000018
+        Content :        ''
+      - Start of Memory Range: 0x00007fff12a87018
+        Data Size:       0x00000400
+        Content :        ''
+      - Start of Memory Range: 0x00007fff12a8ffff
+        Data Size:       0x00000020
+        Content :        ''
+...

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[dmpots]

LGTM!

[Jlalond]

Updated the tests to skip on Windows. It would otherwise fail there.