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

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

<details>
<summary>Changes</summary>

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

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


6 Files Affected:

- (modified) lldb/include/lldb/Utility/ArchSpec.h (-5) 
- (modified) lldb/source/Target/Target.cpp (+1-7) 
- (modified) lldb/source/Utility/ArchSpec.cpp (-17) 
- (added) lldb/test/API/macosx/arm64e-attach/Makefile (+2) 
- (added) lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py (+25) 
- (added) lldb/test/API/macosx/arm64e-attach/main.c (+2) 


``````````diff
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_different,
-                              bool &vendor_different, bool &os_different,
-                              bool &os_version_different,
-                              bool &env_different) 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_different, bool &vendor_different,
-    bool &os_different, bool &os_version_different, bool &env_different) const {
-  const llvm::Triple &me(GetTriple());
-  const llvm::Triple &them(other.GetTriple());
-
-  arch_different = (me.getArch() != them.getArch());
-
-  vendor_different = (me.getVendor() != them.getVendor());
-
-  os_different = (me.getOS() != them.getOS());
-
-  os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
-  env_different = (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..90daa54baa9d36
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,25 @@
+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):
+        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(); }

``````````

</details>


https://github.com/llvm/llvm-project/pull/82804