[Lldb-commits] [lldb] 1ca8a97 - [lldb][Windows] Fix memory region base addresses when a range is split

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-07-07T13:55:48Z
New Revision: 1ca8a978023f1c1fe6018ac87ed06b7ce26f0b77

URL: https://github.com/llvm/llvm-project/commit/1ca8a978023f1c1fe6018ac87ed06b7ce26f0b77
DIFF: https://github.com/llvm/llvm-project/commit/1ca8a978023f1c1fe6018ac87ed06b7ce26f0b77.diff

LOG: [lldb][Windows] Fix memory region base addresses when a range is split

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 checks for any overlapping regions which means
if we get the base or size wrong it'll fail. This logic
applies to any OS so the test isn't restricted to Windows.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D129272

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
index 59f5f6dfe7716..2aa7de6c853a4 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -441,7 +441,7 @@ Status ProcessDebugger::GetMemoryRegionInfo(lldb::addr_t vm_addr,
   // 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);

diff  --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index d84e0bc814436..18118377a8b8b 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -27,7 +27,7 @@ def test_help(self):
             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 @@ def test(self):
 
         self.runCmd("run", RUN_SUCCEEDED)
 
+    def test_command(self):
+        self.setup_program()
+
         interp = self.dbg.GetCommandInterpreter()
         result = lldb.SBCommandReturnObject()
 
@@ -84,3 +87,37 @@ def test(self):
         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()
+
+        regions = self.process().GetMemoryRegions()
+        num_regions = regions.GetSize()
+
+        if num_regions:
+            region = lldb.SBMemoryRegionInfo()
+            regions.GetMemoryRegionAtIndex(0, region)
+            previous_base = region.GetRegionBase()
+            previous_end = region.GetRegionEnd()
+
+            for idx in range(1, regions.GetSize()):
+                regions.GetMemoryRegionAtIndex(idx, region)
+
+                # Check that it does not overlap the previous region.
+                # This could happen if we got the base addresses or size wrong.
+                # Also catches the base addresses being the same.
+                region_base = region.GetRegionBase()
+                region_end = region.GetRegionEnd()
+
+                if (region_base >= previous_base and region_base < previous_end) \
+                    or (region_end > previous_base and region_end <= previous_end):
+                    self.assertFalse(base_address in base_addresses,
+                        "Unexpected overlapping memory region found.")
+
+                previous_base = region_base
+                previous_end = region_end
\ No newline at end of file