[lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs

[jasonmolenda]

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

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

Changes

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

---

Full diff: https://github.com/llvm/llvm-project/pull/84998.diff

4 Files Affected:

• (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+1-1)
• (added) lldb/test/API/macosx/tbi-honored/Makefile (+3)
• (added) lldb/test/API/macosx/tbi-honored/TestTBIHonored.py (+49)
• (added) lldb/test/API/macosx/tbi-honored/main.c (+13)

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..d38685359af6d1
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -0,0 +1,49 @@
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses."""
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTBIHonored(TestBase):
+    @no_debug_info_test
+    @skipUnlessDarwin
+    @skipIf(archs=no_match(["arm64", "arm64e"]))
+    @skipIfRemote
+    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()
+        self.expect("expr -- *pb.p", substrs=["15"])
+        self.expect("frame variable *pb.p", substrs=["15"])
+        self.expect("expr -- *(int*)0x%x" % addr, substrs=["15"])
+
+    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")
+        )
+
+        self.do_variable_access_tests(thread.GetFrameAtIndex(0))
+
+        self.runCmd("process save-core -s stack " + corefile)
+        self.dbg.DeleteTarget(target)
+
+        # Now load the corefile
+        target = self.dbg.CreateTarget("")
+        process = target.LoadCore(corefile)
+        thread = process.GetSelectedThread()
+        self.assertTrue(process.GetSelectedThread().IsValid())
+
+        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
+}

[DavidSpickett]

These seem to be unused.

[DavidSpickett]

addresses in core files.

Also the filename should have core somewhere in it.

[DavidSpickett]

Oh I get it, you're running a process first then making a core file and expecting to get the same output.

This needs to be made clear at the top of the file at least, with some extra comments later too.

[DavidSpickett]

This isn't a live process, do we need to be on Darwin arm64?

[jasonmolenda]

Thanks for all the helpful comments as always, David. I updated the test case to fix the issues you identified and added comments that I think make it clearer that it is testing both a live process and a corefile.

[DavidSpickett]

LGTM