[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

[Alexandre Perez via Phabricator via lldb-commits]

aperez created this revision.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a project: All.
aperez edited the summary of this revision.
aperez added a project: LLDB.
Herald added a subscriber: JDevlieghere.
aperez added reviewers: clayborg, jasonmolenda.
aperez published this revision for review.
aperez added a comment.
Herald added a subscriber: lldb-commits.

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)


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124814

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/RNBRemote.cpp


Index: lldb/tools/debugserver/source/RNBRemote.cpp
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3753,17 +3753,6 @@
     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)) {
@@ -3823,6 +3812,17 @@
       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);
Index: lldb/tools/debugserver/source/DNBDefs.h
===================================================================
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -54,6 +54,7 @@
 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
Index: lldb/tools/debugserver/source/DNB.cpp
===================================================================
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -477,6 +477,10 @@
     }
   }
 
+  if (DNBDebugserverIsTranslated()) {
+    return INVALID_NUB_PROCESS_ARCH;
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124814.426544.patch
Type: text/x-patch
Size: 2463 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220503/00930ab3/attachment-0001.bin>