Change IRMemoryMap's last-resort magic address to an inaddressable address so it doesn't conflict

[jasonmolenda]

Summary

When the IRInterpreter runs, and needs to store things in inferior memory, and it cannot allocate a memory region in the inferior, IRMemoryMap::FindSpace will create a buffer in lldb memory with a target address, and that will be used. If the target address overlaps with the address of an actual memory region in the inferior process, attempts to access the genuine memory there in IRInterpreted expressions will instead access lldb's local scratch memory. The Process may not support any way to query memory regions, so the solution Sean has picked in years past was to pick an especially unlikely address.

Obviously, we have a program that puts things at this address.

Given that 64-bit systems do not genuinely have 64 bits of address space, we can pick a better address. This patch changes the current unlikely address, 0xffffffff00000000, to 0xdead0fff00000000. Still in high memory, but because bit 62 is now 0 with this address, it cannot overlap with an actual virtual address unless there was a system alling for 62 bits of addressable address space. This is unlikely to happen soon.

I wrote a test which loads a binary into lldb, and slides the DATA segment around to different addresses, testing that the global variable in that DATA segment can still be read, as well as testing that if the DATA segment is slid to this impossible address, we get the incorrect value for the global.

This patch also fixes an incorrect way of getting the Target in DWARFExpression::Evaluate which gets a StackFrame from the ExecutionContext passed in, and uses that to get the Target, instead of simply using the Target, when calculating the load address of the address just read. This had the effect of always returning the original file address, even when I loaded the segment at different addresses, in this test where there's no running process. I looked through DWARFExpression briefly and didn't see any other code doing this.

[jasonmolenda]

Probably the laziest bit is at the end of the test case where I want to test that the variable isn't '1', I didn't use a substring because it's uninitialized memory could come back as '10' or something.

[jasonmolenda]

Update the test to check all three members of the struct can be read at the different memory addresses, and CAN'T be read when it overlaps with IRMemoryMap's special address. This final test, where we test that the values can't be accessed at this inaddressable location, is really only there to keep the test and IRMemoryMap in sync - if anyone changes how IRMemoryMap::FindSpace works with this setup, the test result changing will flag that this test needs to be revisited.

[jingham]

Except that you explicitly want to use the expression parser here so you shouldn't use p in the test, this seems fine to me. It would be better to have a less fragile strategy for getting an unused region, but short of doing exhaustive searches which have their own problems we haven't come up with anything. So making a better hack seems worthy. Fix the p usage and this is good to go.

lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py

30

Unless you want to test the alias, it's better to use expr rather than p in tests. We might some day change what the p alias does, and for instance if we get clever about using more static data fetching for p then your test won't test what you think it does anymore.

63

If you are really worried about false positives here, you could just use funkier values for the three members. Not sure that's really necessary, however.

[jasonmolenda]

Update for Jim's feedback.

lldb/source/Expression/DWARFExpression.cpp

case DW_OP_addr:
           target->GetArchitecture().GetCore() == ArchSpec::eCore_wasm32) {
         // wasm file sections aren't mapped into memory, therefore addresses can
         // never point into a file section and are always LoadAddresses.
         stack.back().SetValueType(Value::ValueType::LoadAddress);
       } else {
         stack.back().SetValueType(Value::ValueType::FileAddress);
         // Convert the file address to a load address, so subsequent
         // DWARF operators can operate on it.
-        if (frame)
-          stack.back().ConvertToLoadAddress(module_sp.get(),
-                                            frame->CalculateTarget().get());
+        if (target)
+          stack.back().ConvertToLoadAddress(module_sp.get(), target);
       }
       break;
 
     // The DW_OP_addr_sect_offset4 is used for any location expressions in
     // shared libraries that have a location like:
     //  DW_OP_addr(0x1000)
     // If this address resides in a shared library, then this virtual address
     // won't make sense when it is evaluated in the context of a running

lldb/source/Expression/IRMemoryMap.cpp

lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
   // to the end of the allocations we've already reported, or use a 'sensible'
   // default if this is our first allocation.
 
   if (m_allocations.empty()) {
     uint32_t address_byte_size = GetAddressByteSize();
     if (address_byte_size != UINT32_MAX) {
       switch (address_byte_size) {
       case 8:
-        ret = 0xffffffff00000000ull;
+        ret = 0xdead0fff00000000ull;
         break;
       case 4:
         ret = 0xee000000ull;
         break;
       default:
         break;
       }
     }

lldb/test/API/lang/c/high-mem-global/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py

+"""Look that lldb can display a global loaded in high memory at an addressable address."""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+
+class TestHighMemGlobal(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipUnlessDarwin  # hardcoding of __DATA segment name
+    def test_command_line(self):
+        """Test that we can display a global variable loaded in high memory."""
+        self.build()
+
+        exe = self.getBuildArtifact("a.out")
+        err = lldb.SBError()
+
+        target = self.dbg.CreateTarget(exe, '', '', False, err)
+        self.assertTrue(target.IsValid())
+        module = target.GetModuleAtIndex(0)
+        self.assertTrue(module.IsValid())
+        data_segment = module.FindSection("__DATA")
+        self.assertTrue(data_segment.IsValid())
+        err.Clear()
+
+        self.expect("expr -- global.c", substrs=[' = 1'])
+        self.expect("expr -- global.d", substrs=[' = 2'])
+        self.expect("expr -- global.e", substrs=[' = 3'])
+
+        err = target.SetSectionLoadAddress(data_segment, 0xffffffff00000000)
+        self.assertTrue(err.Success())
+        self.expect("expr -- global.c", substrs=[' = 1'])
+        self.expect("expr -- global.d", substrs=[' = 2'])
+        self.expect("expr -- global.e", substrs=[' = 3'])
+
+        err = target.SetSectionLoadAddress(data_segment, 0x0000088100004000)
+        self.assertTrue(err.Success())
+        self.expect("expr -- global.c", substrs=[' = 1'])
+        self.expect("expr -- global.d", substrs=[' = 2'])
+        self.expect("expr -- global.e", substrs=[' = 3'])
+
+        # This is an address in IRMemoryMap::FindSpace where it has an 
+        # lldb-side buffer of memory that's used in IR interpreters when
+        # memory cannot be allocated in the inferior / functions cannot
+        # be jitted.
+        err = target.SetSectionLoadAddress(data_segment, 0xdead0fff00000000)
+        self.assertTrue(err.Success())
+
+        # The global variable `global` is now overlayed by this 
+        # IRMemoryMap special buffer, and now we cannot see the variable.
+        # Testing that we get the incorrect values at this address ensures 
+        # that IRMemoryMap::FindSpace and this test stay in sync.
+        self.runCmd("expr -- int $global_c = global.c")
+        self.runCmd("expr -- int $global_d = global.d")
+        self.runCmd("expr -- int $global_e = global.e")
+        self.expect("expr -- $global_c != 1 || $global_d != 2 || $global_e != 3", substrs=[' = true'])

lldb/test/API/lang/c/high-mem-global/main.c

+
+struct mystruct {
+  int c, d, e;
+} global = {1, 2, 3};
+
+int main ()
+{
+  return global.c; // break here
+}