[Lldb-commits] [lldb] [LLDB][Save Core Options] Custom ranges should follow the same safety checks as everyone else (PR #125323)

[Jacob Lalonde via lldb-commits]

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

>From 973117aaa1d1bffe5a10a8c87edafec91dcd51bd Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 31 Jan 2025 17:37:46 -0800
Subject: [PATCH 1/3] Have custom ranges go through the same safety checks as
 everything else, to avoid empty ranges, or ranges with no permissions

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

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 89731f798deda88..428f8519b72fd51 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6677,11 +6677,8 @@ static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
 
   for (const auto &range : regions) {
     auto entry = option_ranges.FindEntryThatContains(range.GetRange());
-    if (entry) {
-      ranges.Append(range.GetRange().GetRangeBase(),
-                    range.GetRange().GetByteSize(),
-                    CreateCoreFileMemoryRange(range));
-    }
+    if (entry)
+      AddRegion(range, true, ranges);
   }
 }
 

>From 8bbb150ad7c7ed044d7ca30c745734938263b2d3 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 3 Feb 2025 10:23:22 -0800
Subject: [PATCH 2/3] Add a new test case

---
 .../TestProcessSaveCoreMinidump.py            | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

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 808de687e6ea2e7..2ce8251da3d07cf 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -636,3 +636,36 @@ def minidump_saves_fs_base_region(self):
             self.assertTrue(self.dbg.DeleteTarget(target))
             if os.path.isfile(tls_file):
                 os.unlink(tls_file)
+
+    @skipUnlessPlatform(["linux"])
+    @skipUnlessArch("x86_64")
+    def test_invalid_custom_regions_not_included(self):
+        options = lldb.SBSaveCoreOptions()
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        output_file = self.getBuildArtifact("no_empty_regions.dmp")
+        try:
+            target = self.dbg.CreateTarget(exe)
+            process = target.LaunchSimple(
+                None, None, self.get_process_working_directory()
+            )
+            self.assertState(process.GetState(), lldb.eStateStopped)
+            options.SetPluginName("minidump")
+            options.SetOutputFile(lldb.SBFileSpec(output_file))
+            options.SetStyle(lldb.eSaveCoreCustomOnly)
+            region_one = lldb.SBMemoryRegionInfo()
+            process.GetMemoryRegions().GetMemoryRegionAtIndex(0, region_one)
+            options.AddMemoryRegionToSave(region_one)
+            empty_region = lldb.SBMemoryRegionInfo("empty region", 0x0, 0x0, 3, True, False)
+            options.AddMemoryRegionToSave(empty_region)
+            region_with_no_permissions = lldb.SBMemoryRegionInfo("no permissions", 0x2AAA, 0x2BBB, 0, True, False)
+            options.AddMemoryRegionToSave(region_with_no_permissions)
+            error = process.SaveCore(options)
+            self.assertTrue(error.Success(), error.GetCString())
+            core_target = self.dbg.CreateTarget(None)
+            core_process = core_target.LoadCore(output_file)
+            self.assertNotIn(region_with_no_permissions, core_process.GetMemoryRegions())
+            self.assertNotIn(empty_region, core_process.GetMemoryRegions())
+        finally:
+            if os.path.isfile(output_file):
+                os.unlink(output_file)

>From c9ae55ac562769b5301f1d54843b8a1f178a8e0c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 3 Feb 2025 10:56:36 -0800
Subject: [PATCH 3/3] Run python formatter

---
 .../TestProcessSaveCoreMinidump.py                   | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

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 2ce8251da3d07cf..c2152640c425cee 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -656,15 +656,21 @@ def test_invalid_custom_regions_not_included(self):
             region_one = lldb.SBMemoryRegionInfo()
             process.GetMemoryRegions().GetMemoryRegionAtIndex(0, region_one)
             options.AddMemoryRegionToSave(region_one)
-            empty_region = lldb.SBMemoryRegionInfo("empty region", 0x0, 0x0, 3, True, False)
+            empty_region = lldb.SBMemoryRegionInfo(
+                "empty region", 0x0, 0x0, 3, True, False
+            )
             options.AddMemoryRegionToSave(empty_region)
-            region_with_no_permissions = lldb.SBMemoryRegionInfo("no permissions", 0x2AAA, 0x2BBB, 0, True, False)
+            region_with_no_permissions = lldb.SBMemoryRegionInfo(
+                "no permissions", 0x2AAA, 0x2BBB, 0, True, False
+            )
             options.AddMemoryRegionToSave(region_with_no_permissions)
             error = process.SaveCore(options)
             self.assertTrue(error.Success(), error.GetCString())
             core_target = self.dbg.CreateTarget(None)
             core_process = core_target.LoadCore(output_file)
-            self.assertNotIn(region_with_no_permissions, core_process.GetMemoryRegions())
+            self.assertNotIn(
+                region_with_no_permissions, core_process.GetMemoryRegions()
+            )
             self.assertNotIn(empty_region, core_process.GetMemoryRegions())
         finally:
             if os.path.isfile(output_file):