Fix debugserver translation check

[aperez]

Summary

Currently, debugserver has a test to check if it was launched in
translation. The intent was to cover the case where an x86_64
debugserver attempts to control an arm64/arm64e process, returning
an error. However, this check also covers the case where users
are attaching to an x86_64 process, exiting out before attempting
to hand off control to the translated debugserver at
/Library/Apple/usr/libexec/oah/debugserver.

This diff delays the debugserver translation check until after
determining whether to hand off control to
/Library/Apple/usr/libexec/oah/debugserver. Only when the
process is not translated and thus has not been handed off do we
check if the debugserver is translated, erroring out in that case.

[aperez]

I did not add unit tests because I would need to debug an x86_64 binary using an x86_64 debugserver on arm64, and was not sure if the test infrastructure allowed for that.

I did run a few manual tests which I'm including in this comment. I compiled and used a Darwin-x86_64 debugserver with the patch from this diff applied. x86_64 processes are now handed off to the translated debugserver:

$ lldb main_x86_64
(lldb) target create "main_x86_64"
Current executable set to '/Users/alexandreperez/temp/main_x86_64' (x86_64).
(lldb) b main
Breakpoint 1: where = main_x86_64`main + 11 at main.cpp:2:3, address = 0x0000000100003fab
(lldb) r
Process 80635 launched: '/Users/alexandreperez/temp/main_x86_64' (x86_64)
Process 80635 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x0000000100003fab main_x86_64`main at main.cpp:2:3
   1   	int main() {
-> 2   	  return 0;
   3   	}
(lldb)

Verified that the arm64 binary fail to attach using the patched Darwin-x86_64 debugserver, as expected:

$ lldb main_arm64
(lldb) target create "main_arm64"
Current executable set to '/Users/alexandreperez/temp/main_arm64' (arm64).
(lldb) r
error: process exited with status -1 (debugserver is x86_64 binary running in translation, attach failed.)
(lldb)

Using the LLDB_DEBUGSERVER_PATH environment variable prevents from handing off to the translated debugserver, as expected:

$ LLDB_DEBUGSERVER_PATH="/path/to/patched/debugserver" lldb main_x86_64
(lldb) target create "main_x86_64"
Current executable set to '/Users/alexandreperez/temp/main_x86_64' (x86_64).
(lldb) r
error: process exited with status -1 (debugserver is x86_64 binary running in translation, attach failed.)
(lldb)

[jasonmolenda]

Ah, I see what you're doing. You've got a shell running x86_64 or something (maybe started lldb as x86_64) so everything that is spawned from that is x86_64 -- debugserver and the inferior process -- and so you hit this. let me look more closely later tonight, but I didn't take that scenario into account. Normally when people are running debugserver as x86_64 it's because they unintentionally ran a parent as x86_64 and are paying an unintended perf hit across the debug session and part of this error reporting was to say "hey, you probably didn't mean to do this".

[jasonmolenda]

(tbh every case I've seen, it's someone who checked "Run in Rosetta" on the Xcode.app bundle with Finder, and are running their entire IDE as x86_64....)

[aperez]

In D124814#3487179, @jasonmolenda wrote:

Ah, I see what you're doing. You've got a shell running x86_64 or something (maybe started lldb as x86_64) so everything that is spawned from that is x86_64 -- debugserver and the inferior process -- and so you hit this. let me look more closely later tonight, but I didn't take that scenario into account. Normally when people are running debugserver as x86_64 it's because they unintentionally ran a parent as x86_64 and are paying an unintended perf hit across the debug session and part of this error reporting was to say "hey, you probably didn't mean to do this".

In our case we are currently distributing an x86_64-only version of the toolchain (including lldb and debugserver) across many machines. Some of these machines are m1 laptops, and we started observing failures to attach on these machines once 0c443e92d3b9 was included.

[jasonmolenda]

LGTM. I see how your workflow is set up. Yeah, you return an error if someone tries to run debugserver in translation (and the inferior isn't x86_64) - that's what we need to do, a translated debugserver won't be able to debug arm64 processes.

lldb/tools/debugserver/source/DNB.cpp

if (sysctl(mib, (unsigned)(sizeof(mib) / sizeof(int)), &processInfo,
               (char *)0);
         DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
                          "translated process: ", errno, strerror(errno));
         __builtin_trap();
       }
     }
   }
 
+  if (DNBDebugserverIsTranslated()) {
+    return INVALID_NUB_PROCESS_ARCH;
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {
     DNBLogThreadedIf(LOG_PROCESS, "(DebugNub) attaching to pid %d...",
                      attach_pid);
     pid =
         processSP->AttachForDebug(attach_pid, unmask_signals, err_str, err_len);
 

lldb/tools/debugserver/source/DNBDefs.h

 typedef ssize_t nub_ssize_t;
 typedef uint32_t nub_index_t;
 typedef pid_t nub_process_t;
 typedef uint64_t nub_thread_t;
 typedef uint32_t nub_event_t;
 typedef uint32_t nub_bool_t;
 
 #define INVALID_NUB_PROCESS ((nub_process_t)0)
+#define INVALID_NUB_PROCESS_ARCH ((nub_process_t)-1)
 #define INVALID_NUB_THREAD ((nub_thread_t)0)
 #define INVALID_NUB_WATCH_ID ((nub_watch_t)0)
 #define INVALID_NUB_HW_INDEX UINT32_MAX
 #define INVALID_NUB_REGNUM UINT32_MAX
 #define NUB_GENERIC_ERROR UINT32_MAX
 
 // Watchpoint types
 #define WATCH_TYPE_READ (1u << 0)

lldb/tools/debugserver/source/RNBRemote.cpp

rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
   } else if (strstr(p, "vAttach") == p) {
     nub_process_t attach_pid =
         INVALID_NUB_PROCESS; // attach_pid will be set to 0 if the attach fails
     nub_process_t pid_attaching_to =
         INVALID_NUB_PROCESS; // pid_attaching_to is the original pid specified
     char err_str[1024] = {'\0'};
     std::string attach_name;
 
-    if (DNBDebugserverIsTranslated()) {
-      DNBLogError("debugserver is x86_64 binary running in translation, attach "
-                  "failed.");
-      std::string return_message = "E96;";
-      return_message +=
-          cstring_to_asciihex_string("debugserver is x86_64 binary running in "
-                                     "translation, attached failed.");
-      SendPacket(return_message);
-      return rnb_err;
-    }
-
     if (strstr(p, "vAttachWait;") == p) {
       p += strlen("vAttachWait;");
       if (!GetProcessNameFrom_vAttach(p, attach_name)) {
         return HandlePacket_ILLFORMED(
             __FILE__, __LINE__, p, "non-hex char in arg on 'vAttachWait' pkt");
       }
       DNBLog("[LaunchAttach] START %d vAttachWait for process name '%s'",
              getpid(), attach_name.c_str());
if (strstr(p, "vAttachWait;") == p) {
                pid_attaching_to);
         attach_pid = DNBProcessAttach(pid_attaching_to, &attach_timeout_abstime,
                                       false, err_str, sizeof(err_str));
       }
     } else {
       return HandlePacket_UNIMPLEMENTED(p);
     }
 
+    if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
+      DNBLogError("debugserver is x86_64 binary running in translation, attach "
+                  "failed.");
+      std::string return_message = "E96;";
+      return_message +=
+          cstring_to_asciihex_string("debugserver is x86_64 binary running in "
+                                     "translation, attach failed.");
+      SendPacket(return_message.c_str());
+      return rnb_err;
+    }
+
     if (attach_pid != INVALID_NUB_PROCESS) {
       if (m_ctx.ProcessID() != attach_pid)
         m_ctx.SetProcessID(attach_pid);
       DNBLog("Successfully attached to pid %d", attach_pid);
       // Send a stop reply packet to indicate we successfully attached!
       NotifyThatProcessStopped();
       return rnb_success;
     } else {