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

[DavidSpickett]

Summary

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.

[DavidSpickett]

Follow up to https://discourse.llvm.org/t/unexplained-memory-regions-on-windows/62662.

[labath]

Looks good, thanks. I'd consider strengthening the test to look for overlapping address ranges instead of just identical base addresses.

lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

99

use self.process() instead

[DavidSpickett]

• Use self.process()
• Check for overlapping regions instead of just the base addresses.

[labath]

lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

91

rename to test_region_overlap or something along those lines?

119

I don't see base_addresses defined anywhere. I guess this should just be assertFalse, and the overlap condition right?

Btw, can check for overlap with region_base < previous_end && previous_base < region_end.

[DavidSpickett]

lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

119

Completely missed that yes, I'll fix it up.

[DavidSpickett]

Fixed up in https://reviews.llvm.org/rGf3d43eca34d4. Next time I will remember to check what a test failure looks like.

lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp

 //===-- ProcessDebugger.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
if (mem_info.State == MEM_COMMIT) {
     info.SetReadable(MemoryRegionInfo::eNo);
     info.SetExecutable(MemoryRegionInfo::eNo);
     info.SetWritable(MemoryRegionInfo::eNo);
   }
 
   // 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);
   } else {
     // In the unmapped case we need to return the distance to the next block of
     // memory. VirtualQueryEx nearly does that except that it gives the
     // distance from the start of the page containing vm_addr.
     SYSTEM_INFO data;

lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

def setUp(self):
             '// Run here before printing memory regions')
 
     def test_help(self):
         """ Test that help shows you must have one of address or --all, not both."""
         self.expect("help memory region",
             substrs=["memory region <address-expression>",
                      "memory region -a"])
 
-    def test(self):
+    def setup_program(self):
         self.build()
 
         # Set breakpoint in main and run
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=-1, loc_exact=True)
 
         self.runCmd("run", RUN_SUCCEEDED)
 
+    def test_command(self):
+        self.setup_program()
+
         interp = self.dbg.GetCommandInterpreter()
         result = lldb.SBCommandReturnObject()
 
         # Test that the first 'memory region' command prints the usage.
         interp.HandleCommand("memory region", result)
         self.assertFalse(result.Succeeded())
         self.assertEqual(result.GetError(),
                     "error: 'memory region' takes one argument or \"--all\" option:\n"
def test_command(self):
         interp.HandleCommand("memory region", result)
         self.assertFalse(result.Succeeded())
         self.assertRegexpMatches(result.GetError(), "Usage: memory region <address\-expression> \(or \-\-all\)")
 
         # --all should match what repeating the command gives you
         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