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
on.
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.
rdar://problem/29760580
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72813
Files:
lldb/source/Host/macosx/objcxx/Host.mm
lldb/tools/darwin-debug/darwin-debug.cpp
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)
::chdir(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 @@
exit(1);
}
- // 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);
-
system("clear");
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);
- }
-
llvm::sys::fs::remove(unix_socket_name);
[applescript release];
if (pid != LLDB_INVALID_PROCESS_ID)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
