[Lldb-commits] [lldb] 52557bc - [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (#84998)

[via lldb-commits]

Author: Jason Molenda
Date: 2024-03-14T08:58:27-07:00
New Revision: 52557bce73f64df5da13d42dd97b57fbd4ab1b12

URL: https://github.com/llvm/llvm-project/commit/52557bce73f64df5da13d42dd97b57fbd4ab1b12
DIFF: https://github.com/llvm/llvm-project/commit/52557bce73f64df5da13d42dd97b57fbd4ab1b12.diff

LOG: [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (#84998)

Darwin AArch64 application processors are run with Top Byte Ignore mode
enabled so metadata may be stored in the top byte, it needs to be
ignored when reading/writing memory. David Spickett handled this already
in the base class Process::ReadMemory but ProcessMachCore overrides that
method (to avoid the memory cache) and did not pick up the same change.
I add a test case that creates a pointer with metadata in the top byte
and dereferences it with a live process and with a corefile.

rdar://123784501

Added: 
    lldb/test/API/macosx/tbi-honored/Makefile
    lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
    lldb/test/API/macosx/tbi-honored/main.c

Modified: 
    lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 3961dcf0fbcc0e..7b9938d4f02020 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -652,7 +652,7 @@ size_t ProcessMachCore::ReadMemory(addr_t addr, void *buf, size_t size,
                                    Status &error) {
   // Don't allow the caching that lldb_private::Process::ReadMemory does since
   // in core files we have it all cached our our core file anyway.
-  return DoReadMemory(addr, buf, size, error);
+  return DoReadMemory(FixAnyAddress(addr), buf, size, error);
 }
 
 size_t ProcessMachCore::DoReadMemory(addr_t addr, void *buf, size_t size,

diff  --git a/lldb/test/API/macosx/tbi-honored/Makefile b/lldb/test/API/macosx/tbi-honored/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
new file mode 100644
index 00000000000000..a5c0abd70e5a90
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -0,0 +1,57 @@
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses, both corefile and live."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTBIHonored(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def do_variable_access_tests(self, frame):
+        self.assertEqual(
+            frame.variables["pb"][0]
+            .GetChildMemberWithName("p")
+            .Dereference()
+            .GetValueAsUnsigned(),
+            15,
+        )
+        addr = frame.variables["pb"][0].GetChildMemberWithName("p").GetValueAsUnsigned()
+        # Confirm that there is metadata in the top byte of our pointer
+        self.assertEqual((addr >> 56) & 0xFF, 0xFE)
+        self.expect("expr -- *pb.p", substrs=["15"])
+        self.expect("frame variable *pb.p", substrs=["15"])
+        self.expect("expr -- *(int*)0x%x" % addr, substrs=["15"])
+
+    # This test is valid on AArch64 systems with TBI mode enabled,
+    # and an address mask that clears the top byte before reading
+    # from memory.
+    @skipUnlessDarwin
+    @skipIf(archs=no_match(["arm64", "arm64e"]))
+    @skipIfRemote
+    def test(self):
+        corefile = self.getBuildArtifact("process.core")
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.c")
+        )
+
+        # Test that we can dereference a pointer with TBI data
+        # in a live process.
+        self.do_variable_access_tests(thread.GetFrameAtIndex(0))
+
+        # Create a corefile, delete this process
+        self.runCmd("process save-core -s stack " + corefile)
+        process.Destroy()
+        self.dbg.DeleteTarget(target)
+
+        # Now load the corefile
+        target = self.dbg.CreateTarget("")
+        process = target.LoadCore(corefile)
+        thread = process.GetSelectedThread()
+        self.assertTrue(process.GetSelectedThread().IsValid())
+
+        # Test that we can dereference a pointer with TBI data
+        # in a corefile process.
+        self.do_variable_access_tests(thread.GetFrameAtIndex(0))

diff  --git a/lldb/test/API/macosx/tbi-honored/main.c b/lldb/test/API/macosx/tbi-honored/main.c
new file mode 100644
index 00000000000000..3d7ad0b04cd664
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/main.c
@@ -0,0 +1,13 @@
+#include <stdint.h>
+#include <stdio.h>
+union ptrbytes {
+  int *p;
+  uint8_t bytes[8];
+};
+int main() {
+  int c = 15;
+  union ptrbytes pb;
+  pb.p = &c;
+  pb.bytes[7] = 0xfe;
+  printf("%d\n", *pb.p); // break here
+}