jasonmolenda created this revision.
jasonmolenda added reviewers: clayborg, jingham.
Herald added a project: LLDB.

On macOS lldb has an option that can be added to SBLaunchInfo to create a new 
Terminal window and run the inferior in that new window.  lldb uses some 
AppleScript to open the new window and run the process there.  It runs 
darwin-debug in the window to set up the architecture / 
current-working-directory / environment variables before launching the actual 
inferior process.  lldb opens a socket on the local filesystem and passes the 
name to darwin-debug; darwin-debug sends back its pid to lldb over the socket, 
so lldb can attach to the inferior process once it has been exec'ed.  
AppleScript sits between lldb and the inferior, so the normal way we control 
processes at the start does not work.  We want to attach to the inferior binary 
once it has been started, and is sitting at its entry point in dyld_start, 
stopped, waiting for lldb to connect.

Today, darwin-debug sends its pid over the socket, then parses / constructs the 
environment variables to be passed to the inferior, then calls posix_spawn to 
start the inferior.  lldb gets the pid, then calls WaitForProcessToSIGSTOP 
which is intended to poll repeatedly for 5 seconds to detect when the inferior 
process has stopped in dyld_start.  Unfortunately this function has a few bugs 
- the main loop never runs, the return value from proc_pidinfo is incorrectly 
handled so none of the intended code will ever run, and finally the condition 
that it's looking for -- pbi_status==SSTOP -- is not going to indicate that the 
inferior has been suspended.

The only way to detect that the inferior has been started, and is sitting at 
dyld_start suspended, is to use mach calls (task_info()) which requires that 
lldb task_for_pid the inferior, which lldb doesn't have permissions for.  I 
haven't found a kernel API that doesn't require the task port which I can query 
the suspend count with yet.

The bug being fixed is that lldb attaches before the inferior has started when 
we have a lot of environment variables (darwin-debug is still processing the 
env vars when lldb attaches to it) and the UI layer isn't clear what is going 

This patch changes darwin-debug so it sends its pid just before it calls 
posix_spawn.  It changes lldb to wait up to 5 seconds to receive the pid, then 
it adds an extra 0.1 seconds of sleep in lldb before it tries to attach to the 
inferior (plus the time it takes to construct the attach packet and send it to 
debugserver, and debugserver to decode that packet and try to attach).

Fred suggested using the closing of the socket between lldb and darwin-debug as 
a way of telling when the exec() is actually happening, but the Read methods in 
lldb don't detect that closing via close-on-exec in my testing and I didn't dig 
in much further on this - it's straightforward to get the pid before we call 
posix_spawn which is close enough.


  rG LLVM Github Monorepo



Index: lldb/tools/darwin-debug/darwin-debug.cpp
--- lldb/tools/darwin-debug/darwin-debug.cpp
+++ lldb/tools/darwin-debug/darwin-debug.cpp
@@ -23,6 +23,7 @@
 #include <crt_externs.h>
 #include <getopt.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <mach/machine.h>
 #include <signal.h>
@@ -30,6 +31,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/errno.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -97,7 +99,7 @@
 pid_t posix_spawn_for_debug(char *const *argv, char *const *envp,
                             const char *working_dir, cpu_type_t cpu_type,
-                            int disable_aslr) {
+                            int disable_aslr, int socket_fd) {
   pid_t pid = 0;
   const char *path = argv[0];
@@ -147,6 +149,29 @@
   if (working_dir)
+  // We were able to connect to the socket, now write our PID so whomever
+  // launched us will know this process's ID
+  char pid_str[64];
+  const int pid_str_len =
+      ::snprintf(pid_str, sizeof(pid_str), "%i", ::getpid());
+  const int bytes_sent = ::send(socket_fd, pid_str, pid_str_len, 0);
+  if (pid_str_len != bytes_sent) {
+    perror("error: send (socket_fd, pid_str, pid_str_len, 0)");
+    exit(1);
+  }
+  // Set the socket socket_fd marked as close-on-exec, leave
+  // it open for now.  lldb might use this to detect when the
+  // exec has happened, and we can attach to the inferior
+  // safely.
+  errno = 0;
+  int opts = fcntl (socket_fd, F_GETFL);
+  if (errno == 0) {
+    opts = opts | O_CLOEXEC;
+    fcntl (socket_fd, F_SETFL, opts);
+  }
   exit_with_errno(::posix_spawnp(&pid, path, NULL, &attr, (char *const *)argv,
                                  (char *const *)envp),
                   "posix_spawn() error: ");
@@ -290,21 +315,6 @@
-  // We were able to connect to the socket, now write our PID so whomever
-  // launched us will know this process's ID
-  char pid_str[64];
-  const int pid_str_len =
-      ::snprintf(pid_str, sizeof(pid_str), "%i", ::getpid());
-  const int bytes_sent = ::send(s, pid_str, pid_str_len, 0);
-  if (pid_str_len != bytes_sent) {
-    perror("error: send (s, pid_str, pid_str_len, 0)");
-    exit(1);
-  }
-  // We are done with the socket
-  close(s);
   printf("Launching: '%s'\n", argv[0]);
   if (working_dir.empty()) {
@@ -326,7 +336,8 @@
       pass_env ? *_NSGetEnviron() : NULL, // Pass current environment as we may
                                           // have modified it if "--env" options
                                           // was used, do NOT pass "envp" here
-      working_dir.empty() ? NULL : working_dir.c_str(), cpu_type, disable_aslr);
+      working_dir.empty() ? NULL : working_dir.c_str(), cpu_type, disable_aslr,
+      s);
   return 0;
Index: lldb/source/Host/macosx/objcxx/Host.mm
--- lldb/source/Host/macosx/objcxx/Host.mm
+++ lldb/source/Host/macosx/objcxx/Host.mm
@@ -144,42 +144,22 @@
     ::memset(pid_str, 0, sizeof(pid_str));
     ConnectionStatus status;
     const size_t pid_str_len = file_conn.Read(
-        pid_str, sizeof(pid_str), std::chrono::seconds(0), status, NULL);
+        pid_str, sizeof(pid_str), std::chrono::seconds(5), status, NULL);
     if (pid_str_len > 0) {
       int pid = atoi(pid_str);
+      // The inferior sends its pid right before it calls posix_spawn.
+      // We want to wait a tiny bit for the next process to get 
+      // started, and stop at dyld_start, before we attach.
+      const int short_sleep = 100000; // 0.1 seconds
+      ::usleep(short_sleep);
       return (void *)(intptr_t)pid;
   return NULL;
-static bool WaitForProcessToSIGSTOP(const lldb::pid_t pid,
-                                    const int timeout_in_seconds) {
-  const int time_delta_usecs = 100000;
-  const int num_retries = timeout_in_seconds / time_delta_usecs;
-  for (int i = 0; i < num_retries; i++) {
-    struct proc_bsdinfo bsd_info;
-    int error = ::proc_pidinfo(pid, PROC_PIDTBSDINFO, (uint64_t)0, &bsd_info,
-                               PROC_PIDTBSDINFO_SIZE);
-    switch (error) {
-    case EINVAL:
-    case ENOTSUP:
-    case ESRCH:
-    case EPERM:
-      return false;
-    default:
-      break;
-    case 0:
-      if (bsd_info.pbi_status == SSTOP)
-        return true;
-    }
-    ::usleep(time_delta_usecs);
-  }
-  return false;
 #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__)
 const char *applscript_in_new_tty = "tell application \"Terminal\"\n"
@@ -323,15 +303,9 @@
   thread_result_t accept_thread_result = NULL;
   lldb_error = accept_thread->Join(&accept_thread_result);
-  if (lldb_error.Success() && accept_thread_result) {
+  if (lldb_error.Success() && accept_thread_result)
     pid = (intptr_t)accept_thread_result;
-    // Wait for process to be stopped at the entry point by watching
-    // for the process status to be set to SSTOP which indicates it it
-    // SIGSTOP'ed at the entry point
-    WaitForProcessToSIGSTOP(pid, 5);
-  }
   [applescript release];
lldb-commits mailing list

Reply via email to