Don't mark DW_OP_addr addresses as file addresses while converting them to load addresses sometimes

[jasonmolenda]

Summary

I touched this code in DWARFExpression.cpp https://reviews.llvm.org/D137682 where it would convert a DW_OP_addr to a load address if the ExecutionContext had a StackFrame available. This was a proxy way of telling if there was a live process. I changed it to do this conversion if the ExecutionContext had a *Target*, to make it possible to write a test case that would move a section around to different load addresses without ever creating a process. This, it turns out, broke a use case on another platform that Ted Woodward relayed to me.

This code to convert the address to a load address was added by Adrian in 2018 via https://reviews.llvm.org/D46362 for swift reasons. I'm not sure it was entirely the correct way to solve this, but it has since become quite unnecessary for Swift -- in fact, on Darwin platforms, global addresses before execution are no longer even file addresses, they're part of the dynamic linker (dyld) fixup data and lldb does not know how to parse these. For instance, static char *f = "hi"; in a program, f will have the address of the string bytes. Before and after process execution, lldb must read this address out of the binary, and it is not in any format that lldb can correctly map to a section+offset in the binary. So this change is very definitely not necessary on Darwin today; we can't parse these addresses at all.

Given that this is breaking a use case on another platform, I'm removing Adrian's DW_OP_addr conversion, and removing my test case for the high memory special address testing. I can't find a way to fake up an address space well enough to make this work the way it needs to, for now.

[aprantl]

Yeah, the previous code was a hack that worked under *some* circumstances. That's not good enough to keep it.

lldb/source/Expression/DWARFExpression.cpp

case DW_OP_addr:
       stack.push_back(Scalar(opcodes.GetAddress(&offset)));
       if (target &&
           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 (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/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
-}