[lldb] Fix platform selection on Apple Silicon (again)

[JDevlieghere]

Summary

This patch is another attempt to fix platform selection on Apple Silicon. It partially undoes D117340 which tried to fix the issue by always instantiating a remote-ios platform for "iPhone and iPad Apps on Apple Silicon Macs". While the previous patch worked for attaching, it broke launching and everything else that expects the remote platform to be connected. I made an attempt to work around that, but quickly found out that there were just too may places that had this assumption baked in.

This patch takes a different approach and reverts back to marking the host platform compatible with iOS triples. This brings us back to the original situation where platform selection was broken for remote iOS debugging on Apple Silicon. To fix that, I now look at the process' system architecture to differentiate between iOS binaries running remotely and iOS binaries running locally. I wish I could make this distinction in the platform, but you need a connected process to do this.

I tested the following scenarios, which now all uses the desired platform:

• Launching an iOS binary on macOS: uses the host platform
• Attaching to an iOS binary on macOS: uses the host platform
• Attaching to a remote iOS binary: uses the remote-ios platform

rdar://89840215

[JDevlieghere]

Include TargetConditionals.h

[labath]

I wish I could make this distinction in the platform, but you need a connected process to do this.

Basically, what you're saying is that the ArchSpec alone is not sufficient to select the right platform. Instead of punching right through the layers, could we just give more information (which I guess would be the system architecture) to the functions responsible for selecting the right plugin? Then they could do the regular ask-each-plugin-if-it-wants-to-handle-it dance. The host plugin would say "I don't handle processes with system-arch=*-ios" and then we would go around until we find the remote-ios plugin saying "I live for debugging ios systems"?

[I must admit that I have a bit of an ulterior motive for suggesting this -- I also have a use case where the ArchSpec is not sufficient to select the best platform, but I've struggled to formulate it in a way that makes sense upstream. The system architecture is not relevant for my case (what I'd really need is to inspect the details of the executable being debugged -- mainly its path, but possibly also some other bits), but if we open the door to more elaborate platform selection, then I might be able to squeeze this in as well.

[JDevlieghere]

In D121444#3374854, @labath wrote:

I wish I could make this distinction in the platform, but you need a connected process to do this.

Basically, what you're saying is that the ArchSpec alone is not sufficient to select the right platform. Instead of punching right through the layers, could we just give more information (which I guess would be the system architecture) to the functions responsible for selecting the right plugin? Then they could do the regular ask-each-plugin-if-it-wants-to-handle-it dance. The host plugin would say "I don't handle processes with system-arch=*-ios" and then we would go around until we find the remote-ios plugin saying "I live for debugging ios systems"?

[I must admit that I have a bit of an ulterior motive for suggesting this -- I also have a use case where the ArchSpec is not sufficient to select the best platform, but I've struggled to formulate it in a way that makes sense upstream. The system architecture is not relevant for my case (what I'd really need is to inspect the details of the executable being debugged -- mainly its path, but possibly also some other bits), but if we open the door to more elaborate platform selection, then I might be able to squeeze this in as well.

Yup, that could totally work, it didn't seem worth like changing all that for just the one use case, but happy to do it if you can benefit from that as well. Did you have any particular design in mind? For me, passing an extra ArchSpec for the host arch would do the trick, but that doesn't get you much further. I could pass in a little context object which would hold an ArchSpec for the host arch and a FileSpec for your use case. Or maybe we should just pass in the process if we have one and get all the information from that.

[labath]

In D121444#3375445, @JDevlieghere wrote:

In D121444#3374854, @labath wrote:

I wish I could make this distinction in the platform, but you need a connected process to do this.

Basically, what you're saying is that the ArchSpec alone is not sufficient to select the right platform. Instead of punching right through the layers, could we just give more information (which I guess would be the system architecture) to the functions responsible for selecting the right plugin? Then they could do the regular ask-each-plugin-if-it-wants-to-handle-it dance. The host plugin would say "I don't handle processes with system-arch=*-ios" and then we would go around until we find the remote-ios plugin saying "I live for debugging ios systems"?

[I must admit that I have a bit of an ulterior motive for suggesting this -- I also have a use case where the ArchSpec is not sufficient to select the best platform, but I've struggled to formulate it in a way that makes sense upstream. The system architecture is not relevant for my case (what I'd really need is to inspect the details of the executable being debugged -- mainly its path, but possibly also some other bits), but if we open the door to more elaborate platform selection, then I might be able to squeeze this in as well.

Yup, that could totally work, it didn't seem worth like changing all that for just the one use case, but happy to do it if you can benefit from that as well. Did you have any particular design in mind? For me, passing an extra ArchSpec for the host arch would do the trick, but that doesn't get you much further. I could pass in a little context object which would hold an ArchSpec for the host arch and a FileSpec for your use case. Or maybe we should just pass in the process if we have one and get all the information from that.

A process wouldn't help me, since I am primarily interested in the launch scenario, where the platform is selected long before the process is launched. I am contemplating passing in a ModuleSpec instead of a plain ArchSpec. That would be sufficient for my use case, but I don't know if I really like it (it seems somewhat strange because the we would really like to pass in the process architecture, but a ModuleSpec would normally describe the module architecture (which may be less precise)). So, it may be best to have a specialized context object, but at the end of the day, I don't think that's something that you need to solve. I think it would be sufficient to just add an additional ArchSpec argument for now. If that idea proves itself (and I decide to go ahead with this), I can upgrade that to a context object.

[JDevlieghere]

In D121444#3375701, @labath wrote:

In D121444#3375445, @JDevlieghere wrote:

In D121444#3374854, @labath wrote:

I wish I could make this distinction in the platform, but you need a connected process to do this.

Basically, what you're saying is that the ArchSpec alone is not sufficient to select the right platform. Instead of punching right through the layers, could we just give more information (which I guess would be the system architecture) to the functions responsible for selecting the right plugin? Then they could do the regular ask-each-plugin-if-it-wants-to-handle-it dance. The host plugin would say "I don't handle processes with system-arch=*-ios" and then we would go around until we find the remote-ios plugin saying "I live for debugging ios systems"?

[I must admit that I have a bit of an ulterior motive for suggesting this -- I also have a use case where the ArchSpec is not sufficient to select the best platform, but I've struggled to formulate it in a way that makes sense upstream. The system architecture is not relevant for my case (what I'd really need is to inspect the details of the executable being debugged -- mainly its path, but possibly also some other bits), but if we open the door to more elaborate platform selection, then I might be able to squeeze this in as well.

Yup, that could totally work, it didn't seem worth like changing all that for just the one use case, but happy to do it if you can benefit from that as well. Did you have any particular design in mind? For me, passing an extra ArchSpec for the host arch would do the trick, but that doesn't get you much further. I could pass in a little context object which would hold an ArchSpec for the host arch and a FileSpec for your use case. Or maybe we should just pass in the process if we have one and get all the information from that.

A process wouldn't help me, since I am primarily interested in the launch scenario, where the platform is selected long before the process is launched. I am contemplating passing in a ModuleSpec instead of a plain ArchSpec. That would be sufficient for my use case, but I don't know if I really like it (it seems somewhat strange because the we would really like to pass in the process architecture, but a ModuleSpec would normally describe the module architecture (which may be less precise)). So, it may be best to have a specialized context object, but at the end of the day, I don't think that's something that you need to solve. I think it would be sufficient to just add an additional ArchSpec argument for now. If that idea proves itself (and I decide to go ahead with this), I can upgrade that to a context object.

Cool, that works for me

[JDevlieghere]

Check the host architecture in PlatformMacOSX

[labath]

I don't suppose we could have a (gdb-client?) test for some of this stuff?

lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp

158

if (!host_arch || host_arch.GetTriple().getOS() == llvm::Triple::MacOSX)

...

[JDevlieghere]

Add tests

[JDevlieghere]

Fix typo(s)

[labath]

Cool.

lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py

+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestPlatformMacOSX(GDBRemoteTestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    class MyResponder(MockGDBServerResponder):
+
+        def __init__(self, host):
+            self.host_ostype = host
+            MockGDBServerResponder.__init__(self)
+
+        def respond(self, packet):
+            if packet == "qProcessInfo":
+                return self.qProcessInfo()
+            return MockGDBServerResponder.respond(self, packet)
+
+        def qHostInfo(self):
+            return "cputype:16777223;cpusubtype:2;ostype:%s;vendor:apple;os_version:10.15.4;maccatalyst_version:13.4;endian:little;ptrsize:8;" % self.host_ostype
+
+        def qProcessInfo(self):
+            return "pid:a860;parent-pid:d2a0;real-uid:1f5;real-gid:14;effective-uid:1f5;effective-gid:14;cputype:100000c;cpusubtype:2;ptrsize:8;ostype:ios;vendor:apple;endian:little;"
+
+        def vCont(self):
+            return "vCont;"
+
+    def platform_test(self, host, expected_triple, expected_platform):
+        self.server.responder = self.MyResponder(host)
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets")
+            self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets"))
+
+        target = self.dbg.CreateTargetWithFileAndArch(None, None)
+        process = self.connect(target)
+
+        triple = target.GetTriple()
+        self.assertEqual(triple, expected_triple)
+
+        platform = target.GetPlatform()
+        self.assertEqual(platform.GetName(), expected_platform)
+
+    @skipIfRemote
+    def test_ios(self):
+        self.platform_test(host="ios",
+                           expected_triple="arm64e-apple-ios-",
+                           expected_platform="remote-ios")
+
+    @skipIfRemote
+    @skipUnlessDarwin
+    @skipUnlessArch("arm64")
+    def test_macos(self):
+        self.platform_test(host="macosx",
+                           expected_triple="arm64e-apple-ios-",
+                           expected_platform="host")

lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp

+//===-- PlatformMacOSXTest.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class PlatformMacOSXTest : public ::testing::Test {
+  SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+};
+
+static bool containsArch(const std::vector<ArchSpec> &archs,
+                         const ArchSpec &arch) {
+  return std::find_if(archs.begin(), archs.end(), [&](const ArchSpec &other) {
+           return arch.IsExactMatch(other);
+         }) != archs.end();
+}
+
+TEST_F(PlatformMacOSXTest, TestGetSupportedArchitectures) {
+  PlatformMacOSX platform;
+
+  const ArchSpec x86_macosx_arch("x86_64-apple-macosx");
+
+  EXPECT_TRUE(containsArch(platform.GetSupportedArchitectures(x86_macosx_arch),
+                           x86_macosx_arch));
+  EXPECT_TRUE(
+      containsArch(platform.GetSupportedArchitectures({}), x86_macosx_arch));
+
+#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+  const ArchSpec arm64_macosx_arch("arm64-apple-macosx");
+  const ArchSpec arm64_ios_arch("arm64-apple-ios");
+
+  EXPECT_TRUE(containsArch(
+      platform.GetSupportedArchitectures(arm64_macosx_arch), arm64_ios_arch));
+  EXPECT_TRUE(
+      containsArch(platform.GetSupportedArchitectures({}), arm64_ios_arch));
+  EXPECT_FALSE(containsArch(platform.GetSupportedArchitectures(arm64_ios_arch),
+                            arm64_ios_arch));
+#endif
+}

lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp

ConstString PlatformMacOSX::GetSDKDirectory(lldb_private::Target &target) {
 
   return {};
 }
 
 std::vector<ArchSpec>
 PlatformMacOSX::GetSupportedArchitectures(const ArchSpec &process_host_arch) {
   std::vector<ArchSpec> result;
 #if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
-  // macOS for ARM64 support both native and translated x86_64 processes
-
   // When cmdline lldb is run on iOS, watchOS, etc, it is still
   // using "PlatformMacOSX".
   llvm::Triple::OSType host_os = GetHostOSType();
   ARMGetSupportedArchitectures(result, host_os);
 
   if (host_os == llvm::Triple::MacOSX) {
     // We can't use x86GetSupportedArchitectures() because it uses
     // the system architecture for some of its return values and also
     // has a 32bits variant.
     result.push_back(ArchSpec("x86_64-apple-macosx"));
     result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
     result.push_back(ArchSpec("arm64-apple-ios-macabi"));
     result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+
+    // On Apple Silicon, the host platform is compatible with iOS triples to
+    // support unmodified "iPhone and iPad Apps on Apple Silicon Macs". Because
+    // the binaries are identical, we must rely on the host architecture to
+    // tell them apart and mark the host platform as compatible or not.
+    if (!process_host_arch ||
+        process_host_arch.GetTriple().getOS() == llvm::Triple::MacOSX) {
+      result.push_back(ArchSpec("arm64-apple-ios"));
+      result.push_back(ArchSpec("arm64e-apple-ios"));
+    }
   }
 #else
   x86GetSupportedArchitectures(result);
   result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
 #endif
   return result;
 }
 

lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py

+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestPlatformMacOSX(GDBRemoteTestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    class MyResponder(MockGDBServerResponder):
+
+        def __init__(self, host):
+            self.host_ostype = host
+            MockGDBServerResponder.__init__(self)
+
+        def respond(self, packet):
+            if packet == "qProcessInfo":
+                return self.qProcessInfo()
+            return MockGDBServerResponder.respond(self, packet)
+
+        def qHostInfo(self):
+            return "cputype:16777223;cpusubtype:2;ostype:%s;vendor:apple;os_version:10.15.4;maccatalyst_version:13.4;endian:little;ptrsize:8;" % self.host_ostype
+
+        def qProcessInfo(self):
+            return "pid:a860;parent-pid:d2a0;real-uid:1f5;real-gid:14;effective-uid:1f5;effective-gid:14;cputype:100000c;cpusubtype:2;ptrsize:8;ostype:ios;vendor:apple;endian:little;"
+
+        def vCont(self):
+            return "vCont;"
+
+    def platform_test(self, host, expected_triple, expected_platform):
+        self.server.responder = self.MyResponder(host)
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets")
+            self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets"))
+
+        target = self.dbg.CreateTargetWithFileAndArch(None, None)
+        process = self.connect(target)
+
+        triple = target.GetTriple()
+        self.assertEqual(triple, expected_triple)
+
+        platform = target.GetPlatform()
+        self.assertEqual(platform.GetName(), expected_platform)
+
+    @skipIfRemote
+    def test_ios(self):
+        self.platform_test(host="ios",
+                           expected_triple="arm64e-apple-ios-",
+                           expected_platform="remote-ios")
+
+    @skipIfRemote
+    @skipUnlessDarwin
+    @skipUnlessArch("arm64")
+    def test_macos(self):
+        self.platform_test(host="macosx",
+                           expected_triple="arm64e-apple-ios-",
+                           expected_platform="host")

lldb/unittests/Platform/CMakeLists.txt

 add_lldb_unittest(LLDBPlatformTests
   PlatformAppleSimulatorTest.cpp
   PlatformDarwinTest.cpp
+  PlatformMacOSXTest.cpp
   PlatformSiginfoTest.cpp
 
   LINK_LIBS
     lldbPluginPlatformFreeBSD
     lldbPluginPlatformLinux
     lldbPluginPlatformMacOSX
     lldbPluginPlatformNetBSD
   LINK_COMPONENTS
     Support
   )
 
 add_subdirectory(Android)

lldb/unittests/Platform/PlatformMacOSXTest.cpp

+//===-- PlatformMacOSXTest.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class PlatformMacOSXTest : public ::testing::Test {
+  SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+};
+
+static bool containsArch(const std::vector<ArchSpec> &archs,
+                         const ArchSpec &arch) {
+  return std::find_if(archs.begin(), archs.end(), [&](const ArchSpec &other) {
+           return arch.IsExactMatch(other);
+         }) != archs.end();
+}
+
+TEST_F(PlatformMacOSXTest, TestGetSupportedArchitectures) {
+  PlatformMacOSX platform;
+
+  const ArchSpec x86_macosx_arch("x86_64-apple-macosx");
+
+  EXPECT_TRUE(containsArch(platform.GetSupportedArchitectures(x86_macosx_arch),
+                           x86_macosx_arch));
+  EXPECT_TRUE(
+      containsArch(platform.GetSupportedArchitectures({}), x86_macosx_arch));
+
+#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+  const ArchSpec arm64_macosx_arch("arm64-apple-macosx");
+  const ArchSpec arm64_ios_arch("arm64-apple-ios");
+
+  EXPECT_TRUE(containsArch(
+      platform.GetSupportedArchitectures(arm64_macosx_arch), arm64_ios_arch));
+  EXPECT_TRUE(
+      containsArch(platform.GetSupportedArchitectures({}), arm64_ios_arch));
+  EXPECT_FALSE(containsArch(platform.GetSupportedArchitectures(arm64_ios_arch),
+                            arm64_ios_arch));
+#endif
+}