[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously we recorded AllocationBase as the base address of the region
we get from VirtualQueryEx. However, this is the base of the allocation,
which can later be split into more regions.

So you got stuff like:
[0x00007fff377c0000-0x00007fff377c1000) r-- PECOFF header
[0x00007fff377c0000-0x00007fff37840000) r-x .text
[0x00007fff377c0000-0x00007fff37870000) r-- .rdata

Where all the base addresses were the same.

Instead, use BaseAddress as the base of the region. So we get:
[0x00007fff377c0000-0x00007fff377c1000) r-- PECOFF header
[0x00007fff377c1000-0x00007fff37840000) r-x .text
[0x00007fff37840000-0x00007fff37870000) r-- .rdata

https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-memory_basic_information

The added test is looking for the Windows situation
but we should not see repeated base addresses anywhere
else either. So I've not restricted it to Windows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129272

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===================================================================
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@
             substrs=["memory region <address-expression>",
                      "memory region -a"])
 
-    def test(self):
+    def setup_program(self):
         self.build()
 
         # Set breakpoint in main and run
@@ -37,6 +37,9 @@
 
         self.runCmd("run", RUN_SUCCEEDED)
 
+    def test_command(self):
+        self.setup_program()
+
         interp = self.dbg.GetCommandInterpreter()
         result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,23 @@
         interp.HandleCommand("memory region --all", result)
         self.assertTrue(result.Succeeded())
         self.assertEqual(result.GetOutput(), all_regions)
+
+    def test_unique_base_addresses(self):
+        # In the past on Windows we were recording AllocationBase as the base address
+        # of the current region, not BaseAddress. So if a range of pages was split
+        # into regions you would see several regions with the same base address.
+        # This checks that this no longer happens (and it shouldn't happen on any
+        # other OS either).
+        self.setup_program()
+
+        process = self.dbg.GetTargetAtIndex(0).GetProcess()
+        regions = process.GetMemoryRegions()
+        base_addresses = set()
+
+        for idx in range(regions.GetSize()):
+            region = lldb.SBMemoryRegionInfo()
+            regions.GetMemoryRegionAtIndex(idx, region)
+            base_address = region.GetRegionBase()
+            self.assertFalse(base_address in base_addresses,
+                "Did not expect to see a repeated base region address.")
+            base_addresses.add(base_address)
\ No newline at end of file
Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@
   // AllocationBase is defined for MEM_COMMIT and MEM_RESERVE but not MEM_FREE.
   if (mem_info.State != MEM_FREE) {
     info.GetRange().SetRangeBase(
-        reinterpret_cast<addr_t>(mem_info.AllocationBase));
+        reinterpret_cast<addr_t>(mem_info.BaseAddress));
     info.GetRange().SetRangeEnd(reinterpret_cast<addr_t>(mem_info.BaseAddress) +
                                 mem_info.RegionSize);
     info.SetMapped(MemoryRegionInfo::eYes);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129272.442852.patch
Type: text/x-patch
Size: 2666 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220707/0423cd1b/attachment.bin>