[Lldb-commits] [lldb] f9f3316 - Replace ArchSpec::PiecewiseCompare() with Triple::operator==()

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2024-02-26T09:57:07-08:00
New Revision: f9f331652d4f0aff9ece3570abe8c686cdfefff4

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

LOG: Replace ArchSpec::PiecewiseCompare() with Triple::operator==()

Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).

This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.

rdar://123338218

Added: 
    lldb/test/API/macosx/arm64e-attach/Makefile
    lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
    lldb/test/API/macosx/arm64e-attach/main.c

Modified: 
    lldb/include/lldb/Utility/ArchSpec.h
    lldb/source/Target/Target.cpp
    lldb/source/Utility/ArchSpec.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/ArchSpec.h b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
 
   bool IsFullySpecifiedTriple() const;
 
-  void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_
diff erent,
-                              bool &vendor_
diff erent, bool &os_
diff erent,
-                              bool &os_version_
diff erent,
-                              bool &env_
diff erent) const;
-
   /// Detect whether this architecture uses thumb code exclusively
   ///
   /// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform,
 
       if (m_arch.GetSpec().IsCompatibleMatch(other)) {
         compatible_local_arch = true;
-        bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-            env_changed;
 
-        m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-                                                vendor_changed, os_changed,
-                                                os_ver_changed, env_changed);
-
-        if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+        if (m_arch.GetSpec().GetTriple() == other.GetTriple())
           replace_local_arch = false;
       }
     }

diff  --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
   return true;
 }
 
-void ArchSpec::PiecewiseTripleCompare(
-    const ArchSpec &other, bool &arch_
diff erent, bool &vendor_
diff erent,
-    bool &os_
diff erent, bool &os_version_
diff erent, bool &env_
diff erent) const {
-  const llvm::Triple &me(GetTriple());
-  const llvm::Triple &them(other.GetTriple());
-
-  arch_
diff erent = (me.getArch() != them.getArch());
-
-  vendor_
diff erent = (me.getVendor() != them.getVendor());
-
-  os_
diff erent = (me.getOS() != them.getOS());
-
-  os_version_
diff erent = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
-  env_
diff erent = (me.getEnvironment() != them.getEnvironment());
-}
-
 bool ArchSpec::IsAlwaysThumbInstructions() const {
   std::string Status;
   if (GetTriple().getArch() == llvm::Triple::arm ||

diff  --git a/lldb/test/API/macosx/arm64e-attach/Makefile b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00000000000000..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules

diff  --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00000000000000..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+    @skipIf(archs=no_match(["arm64e"]))
+    def test(self):
+        # Skip this test if not running on AArch64 target that supports PAC
+        if not self.isAArch64PAuth():
+            self.skipTest("Target must support pointer authentication.")
+        self.build()
+        popen = self.spawnSubprocess(self.getBuildArtifact(), [])
+        error = lldb.SBError()
+        # This simulates how Xcode attaches to a process by pid/name.
+        target = self.dbg.CreateTarget("", "arm64", "", True, error)
+        listener = lldb.SBListener("my.attach.listener")
+        process = target.AttachToProcessWithID(listener, popen.pid, error)
+        self.assertSuccess(error)
+        self.assertTrue(process, PROCESS_IS_VALID)
+        self.assertEqual(target.GetTriple().split('-')[0], "arm64e",
+                         "target triple is updated correctly")
+        error = process.Kill()
+        self.assertSuccess(error)

diff  --git a/lldb/test/API/macosx/arm64e-attach/main.c b/lldb/test/API/macosx/arm64e-attach/main.c
new file mode 100644
index 00000000000000..7baf2ffd8f2899
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/main.c
@@ -0,0 +1,2 @@
+int getchar();
+int main() { return getchar(); }