[Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

[davide]

Summary

Nothing crazy here, just an organizational cleanup.

[davide]

lldb/include/lldb/Host/Host.h

231

Stale comment, I'll update

[aprantl]

lldb/include/lldb/Host/Host.h

231

Is this supposed to be a generic function call that, e.g, should fire for debugging a process in QEMU under Linux, or is it meant to specifically check for Rosetta on macOS? The comment makes it sound like it's the latter and the API name hints at the former. It would be nice to clarify this in the comment.

[JDevlieghere]

lldb/source/Host/macosx/objcxx/Host.mm

1471

nit: static_cast for consistency with line 1468?

1472

nit: nullptr

[davide]

lldb/include/lldb/Host/Host.h

231

It is meant specifically for Rosetta, but this leaves in Host, so I decided for a generic name.
Do you have a better suggestion for the name or the comment ? Happy to go with it

[labath]

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

3424

I don't think this is a particularly good abstraction, as the debugserver path is still left here, and the path is definitely os- and arch-specific. It would be better if the API returned the path to the debugserver-to-be-used instead.

Bonus points if you can also hide the DEBUGSERVER_BASENAME logic from GDBRemoteCommunication.cpp into the same API.

[davide]

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

3424

hmm, I wonder if this code should live in GDBRemoteCommunication.cpp

[aprantl]

lldb/include/lldb/Host/Host.h

231

Not awesome, but IsMacOSRosettaProcess or IsMacOSRosettaTranslated?

A more serious question: Is Host actually the right place for this function? What happens when I remote-debug a Rosetta process from another Mac or a Linux machine? Is there even a correct abstraction for this use-case? Platform maybe?

[labath]

lldb/include/lldb/Host/Host.h

231

This code is only used when debugging on the host. When debugging remotely, you either connect to at already running instance of debugserver, or you ask the lldb-server-platform process to launch one for you. The platform process then should use this function to find the right debugserver to launch, but using a Host function is appropriate there, because the platform process is running remotely, and so it will the the "remote Host" which is answering the question.

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

3424

I think it ended up there because the gdb-server launching code is used from both liblldb and lldb-server (the platform version), and GDBRemoteCommunication is the greatest common denominator. It's not ideal, but I don't think its the worst thing that we have. (At one point I'd like to create a library to house code which is shared between liblldb and lldb-server, and then it may be easier to organize things better.)

Speaking of that... I think this code should live in GDBRemoteCommunication as well. In the situation where one is remote-debugging a mac via lldb-server-platform, I assume one would want the platform process to automatically spawn the right debugserver for the rosetta thingies.

[aprantl]

lldb/include/lldb/Host/Host.h

231

When debugging remotely, you either connect to at already running instance of debugserver, or you ask the lldb-server-platform process to launch one for you.

Thanks, that makes perfect sense!

[aprantl]

From my side this is good to go once everybody else's concerns are addressed.

[davide]

Somebody will pick this up, probably in a different form.

lldb/include/lldb/Host/Host.h

   static bool OpenFileInExternalEditor(const FileSpec &file_spec,
                                        uint32_t line_no);
 
+  /// Check whether a process is translated (Rosetta).
+  /// \arg process_info The info structure for the process queried.
+  static bool IsProcessTranslated(const ProcessInfo &process_info);
+
   static Environment GetEnvironment();
 
   static std::unique_ptr<Connection>

lldb/source/Host/common/Host.cpp

 }
 
 bool Host::ResolveExecutableInBundle(FileSpec &file) { return false; }
+
+bool Host::IsProcessTranslated(const ProcessInfo &process_info) {
+  return false;
+}
 #endif
 
 #ifndef _WIN32

lldb/source/Host/macosx/objcxx/Host.mm

     ::asl_vlog(NULL, g_aslmsg, asl_level, format, args);
   }
 }
+
+bool Host::IsProcessTranslated(const ProcessInfo &process_info) {
+  int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
+                static_cast<int>(process_info.GetProcessID()) };
+  struct kinfo_proc processInfo;
+  size_t bufsize = sizeof(processInfo);
+  if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), &processInfo,
+             &bufsize, NULL, 0) == 0 && bufsize <= 0)
+    return false;
+  return processInfo.kp_proc.p_flag & P_TRANSLATED;
+}

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 #include <unistd.h>
 #endif
 #include <sys/stat.h>
-#if defined(__APPLE__)
-#include <sys/sysctl.h>
-#endif
 #include <sys/types.h>
 #include <time.h>
 

         std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3, _4), false);
     debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined(__APPLE__)
     // On macOS 11, we need to support x86_64 applications translated to
     // arm64. We check whether a binary is translated and spawn the correct
     // debugserver accordingly.
-    int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID,
-                  static_cast<int>(process_info.GetProcessID()) };
-    struct kinfo_proc processInfo;
-    size_t bufsize = sizeof(processInfo);
-    if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), &processInfo,
-               &bufsize, NULL, 0) == 0 && bufsize > 0) {
-      if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
-        FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
-        debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
-      }
+    if (Host::IsProcessTranslated(process_info)) {
+      FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver");
+      debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
     }
-#endif
 
     int communication_fd = -1;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION