Author: Jacob Lalonde Date: 2025-05-05T11:04:55-07:00 New Revision: c50cba6275271fba69be661b9ec0665b2be88dbc
URL: https://github.com/llvm/llvm-project/commit/c50cba6275271fba69be661b9ec0665b2be88dbc DIFF: https://github.com/llvm/llvm-project/commit/c50cba6275271fba69be661b9ec0665b2be88dbc.diff LOG: [LLDB][SBSaveCore] Sbsavecore subregions bug (#138206) 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). Added: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml Modified: lldb/include/lldb/Utility/RangeMap.h lldb/source/Target/Process.cpp Removed: ################################################################################ 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 ce64b44846a5d..13ff12b4ff953 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6706,6 +6706,18 @@ 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 MemoryRegionInfo::RangeType &rhs) { + + MemoryRegionInfo region_info; + region_info.SetLLDBPermissions(lhs.GetLLDBPermissions()); + region_info.GetRange() = lhs.GetRange().Intersect(rhs); + + return region_info; +} + static void GetUserSpecifiedCoreFileSaveRanges(Process &process, const MemoryRegionInfos ®ions, const SaveCoreOptions &options, @@ -6715,9 +6727,15 @@ 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 != range.GetRange()) { + 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..c0572e34d7746 --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidumpYaml.py @@ -0,0 +1,247 @@ +""" +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 AddressRange: + begin: int + end: int + + def __init__(self, begin, end): + self.begin = begin + self.end = end + + +# We skip all these tests on Windows because on Windows Minidumps +# are not generated by LLDB. +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 + + @skipIfWindows + def validate_regions_saved_correctly( + self, core_process, expected_region, expected_invalid_region=None + ): + """Validate that the expected_region is saved in the core_proc, and that the expected invalid region is not saved, if not not none.""" + + # Validate we can read the entire expected_region + error = lldb.SBError() + core_process.ReadMemory( + expected_region.begin, expected_region.end - expected_region.begin, error + ) + self.assertTrue(error.Success(), error.GetCString()) + + # Validate we can't read before and after the expected_region + core_process.ReadMemory(expected_region.begin - 1, 1, error) + self.assertTrue(error.Fail(), error.GetCString()) + + core_process.ReadMemory(expected_region.end + 1, 1, error) + self.assertTrue(error.Fail(), error.GetCString()) + + if expected_invalid_region is None: + return + + # Validate we can't read the original_region + core_process.ReadMemory( + expected_invalid_region.begin, + expected_invalid_region.end - expected_invalid_region.begin, + error, + ) + self.assertTrue(error.Fail(), error.GetCString()) + + @skipIfWindows + 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 0x2000-0x2020 and the user specifies 0x2000-0x2008 + we should still capture 0x2000-0x2008 + """ + 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 = 0x2000 + 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) + + expected_address_range = AddressRange(begin, end) + expected_invalid_range = AddressRange(begin, 0x2020) + self.validate_regions_saved_correctly( + core_process, expected_address_range, expected_invalid_range + ) + + @skipIfWindows + 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-0x1100 and the user specifies 0x900-x1200 + we should still capture 0x1000-0x1100 + """ + 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 = 0x100 + begin = 0x1000 + 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) + + expected_address_range = AddressRange(begin, end) + expected_invalid_range = AddressRange(begin - 16, end + 16) + self.validate_regions_saved_correctly( + core_process, expected_address_range, expected_invalid_range + ) + + @skipIfWindows + 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 = 0x120 + begin = 0x1000 + 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) + + expected_address_range = AddressRange(begin, end) + expected_invalid_range = AddressRange(begin - 16, end + 16) + self.validate_regions_saved_correctly( + core_process, expected_address_range, expected_invalid_range + ) + + @skipIfWindows + 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 = 0x20 + begin = 0x2000 + 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) + + expected_address_range = AddressRange(begin, end) + expected_invalid_range = AddressRange(begin - 16, end) + self.validate_regions_saved_correctly( + core_process, expected_address_range, expected_invalid_range + ) + + @skipIfWindows + def test_region_spans_multiple_regions(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 = 0x1000 + begin = 0x5000 + 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) + + expected_address_range = AddressRange(begin, end) + self.validate_regions_saved_correctly(core_process, expected_address_range) + + @skipIfWindows + def test_region_spans_multiple_regions_with_one_subrange(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 = 0x800 + begin = 0x5000 + 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) + + expected_address_range = AddressRange(begin, end) + expected_invalid_range = AddressRange(begin, begin + 0x1000) + self.validate_regions_saved_correctly( + core_process, expected_address_range, expected_invalid_range + ) 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..d04ca1ae0dc12 --- /dev/null +++ b/lldb/test/API/functionalities/process_save_core_minidump/minidump_mem64.yaml @@ -0,0 +1,38 @@ +--- !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: 0x1000 + Data Size: 0x100 + Content : '' + - Start of Memory Range: 0x2000 + Data Size: 0x20 + Content : '' + - Start of Memory Range: 0x3000 + Data Size: 0x400 + Content : '' + - Start of Memory Range: 0x5000 + Data Size: 0x500 + Content : '' + - Start of Memory Range: 0x5500 + Data Size: 0x500 + Content : '' +... _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits