jasonmolenda created this revision.
jasonmolenda added a reviewer: aprantl.
jasonmolenda added a project: LLDB.
Herald added subscribers: Michael137, JDevlieghere, arichardson.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139226

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/lang/c/high-mem-global/Makefile
  lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
  lldb/test/API/lang/c/high-mem-global/main.c

Index: lldb/test/API/lang/c/high-mem-global/main.c
===================================================================
--- lldb/test/API/lang/c/high-mem-global/main.c
+++ /dev/null
@@ -1,9 +0,0 @@
-
-struct mystruct {
-  int c, d, e;
-} global = {1, 2, 3};
-
-int main ()
-{
-  return global.c; // break here
-}
Index: lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
===================================================================
--- lldb/test/API/lang/c/high-mem-global/TestHighMemGlobal.py
+++ /dev/null
@@ -1,59 +0,0 @@
-"""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'])
Index: lldb/test/API/lang/c/high-mem-global/Makefile
===================================================================
--- lldb/test/API/lang/c/high-mem-global/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES := main.c
-
-include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===================================================================
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -925,10 +925,6 @@
         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;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to