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

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Wed Mar 13 15:40:56 PDT 2024


https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/84998

>From 4278537c262b01b1d6432391bd9d8017eb96c60a Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Tue, 12 Mar 2024 17:09:30 -0700
Subject: [PATCH 1/2] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data
 from addrs

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
---
 .../Process/mach-core/ProcessMachCore.cpp     |  2 +-
 lldb/test/API/macosx/tbi-honored/Makefile     |  3 ++
 .../API/macosx/tbi-honored/TestTBIHonored.py  | 49 +++++++++++++++++++
 lldb/test/API/macosx/tbi-honored/main.c       | 13 +++++
 4 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 lldb/test/API/macosx/tbi-honored/Makefile
 create mode 100644 lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
 create mode 100644 lldb/test/API/macosx/tbi-honored/main.c

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
+}

>From 145e4161a311956ee15c4631667f332d50ef4e0b Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 13 Mar 2024 15:40:08 -0700
Subject: [PATCH 2/2] Address David Spickett's feedback on the testcase

Lots of fixes to the test case for correctness and
to make it clearer what it is testing.
---
 .../API/macosx/tbi-honored/TestTBIHonored.py  | 26 ++++++++++++-------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
index d38685359af6d1..a5c0abd70e5a90 100644
--- a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -1,8 +1,4 @@
-"""Test that lldb on Darwin ignores metadata in the top byte of addresses."""
-
-import os
-import re
-import subprocess
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses, both corefile and live."""
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -11,10 +7,8 @@
 
 
 class TestTBIHonored(TestBase):
-    @no_debug_info_test
-    @skipUnlessDarwin
-    @skipIf(archs=no_match(["arm64", "arm64e"]))
-    @skipIfRemote
+    NO_DEBUG_INFO_TESTCASE = True
+
     def do_variable_access_tests(self, frame):
         self.assertEqual(
             frame.variables["pb"][0]
@@ -24,10 +18,18 @@ def do_variable_access_tests(self, frame):
             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()
@@ -35,9 +37,13 @@ def test(self):
             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
@@ -46,4 +52,6 @@ def test(self):
         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))



More information about the lldb-commits mailing list