diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -64,7 +64,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -239,27 +241,84 @@ (size_t)newlen); } -int internal_forkpty(int *aparent) { - int parent, worker; - if (openpty(&parent, &worker, nullptr, nullptr, nullptr) == -1) return -1; - int pid = internal_fork(); - if (pid == -1) { - close(parent); - close(worker); - return -1; +static fd_t internal_spawn_impl(const char *argv[], pid_t *pid) { + fd_t master_fd = kInvalidFd; + fd_t slave_fd = kInvalidFd; + + auto fd_closer = at_scope_exit([&] { + internal_close(master_fd); + internal_close(slave_fd); + }); + + // We need a new pseudoterminal to avoid buffering problems. The 'atos' tool + // in particular detects when it's talking to a pipe and forgets to flush the + // output stream after sending a response. + master_fd = posix_openpt(O_RDWR); + if (master_fd == kInvalidFd) return kInvalidFd; + + int res = grantpt(master_fd) || unlockpt(master_fd); + if (res != 0) return kInvalidFd; + + // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. + char slave_pty_name[128]; + res = ioctl(master_fd, TIOCPTYGNAME, slave_pty_name); + if (res == -1) return kInvalidFd; + + slave_fd = internal_open(slave_pty_name, O_RDWR); + if (slave_fd == kInvalidFd) return kInvalidFd; + + posix_spawn_file_actions_t acts; + res = posix_spawn_file_actions_init(&acts); + if (res != 0) return kInvalidFd; + + auto fa_cleanup = at_scope_exit([&] { + posix_spawn_file_actions_destroy(&acts); + }); + + char **env = GetEnviron(); + res = posix_spawn_file_actions_adddup2(&acts, slave_fd, STDIN_FILENO) || + posix_spawn_file_actions_adddup2(&acts, slave_fd, STDOUT_FILENO) || + posix_spawn_file_actions_addclose(&acts, slave_fd) || + posix_spawn_file_actions_addclose(&acts, master_fd) || + posix_spawn(pid, argv[0], &acts, NULL, const_cast(argv), env); + if (res != 0) return kInvalidFd; + + // Disable echo in the new terminal, disable CR. + struct termios termflags; + tcgetattr(master_fd, &termflags); + termflags.c_oflag &= ~ONLCR; + termflags.c_lflag &= ~ECHO; + tcsetattr(master_fd, TCSANOW, &termflags); + + // On success, do not close master_fd on scope exit. + fd_t fd = master_fd; + master_fd = kInvalidFd; + + return fd; +} + +fd_t internal_spawn(const char *argv[], pid_t *pid) { + // The client program may close its stdin and/or stdout and/or stderr thus + // allowing open/posix_openpt to reuse file descriptors 0, 1 or 2. In this + // case the communication is broken if either the parent or the child tries to + // close or duplicate these descriptors. We temporarily reserve these + // descriptors here to prevent this. + fd_t low_fds[3]; + size_t count; + + for (count = 0; count < 3; count++) { + low_fds[count] = posix_openpt(O_RDWR); + if (low_fds[count] >= STDERR_FILENO) + break; } - if (pid == 0) { - close(parent); - if (login_tty(worker) != 0) { - // We already forked, there's not much we can do. Let's quit. - Report("login_tty failed (errno %d)\n", errno); - internal__exit(1); - } - } else { - *aparent = parent; - close(worker); + + fd_t fd = internal_spawn_impl(argv, pid); + + for (; count > 0; count--) { + internal_close(low_fds[count]); } - return pid; + + return fd; } uptr internal_rename(const char *oldpath, const char *newpath) { diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h @@ -63,7 +63,7 @@ uptr internal_waitpid(int pid, int *status, int options); int internal_fork(); -int internal_forkpty(int *amaster); +fd_t internal_spawn(const char *argv[], pid_t *pid); int internal_sysctl(const int *name, unsigned int namelen, void *oldp, uptr *oldlenp, const void *newp, uptr newlen); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h @@ -76,7 +76,7 @@ // SymbolizerProcess may not be used from two threads simultaneously. class SymbolizerProcess { public: - explicit SymbolizerProcess(const char *path, bool use_forkpty = false); + explicit SymbolizerProcess(const char *path, bool use_posix_spawn = false); const char *SendCommand(const char *command); protected: @@ -114,7 +114,7 @@ uptr times_restarted_; bool failed_to_start_; bool reported_invalid_path_; - bool use_forkpty_; + bool use_posix_spawn_; }; class LLVMSymbolizerProcess; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp @@ -452,14 +452,14 @@ return symbolizer_process_->SendCommand(buffer_); } -SymbolizerProcess::SymbolizerProcess(const char *path, bool use_forkpty) +SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn) : path_(path), input_fd_(kInvalidFd), output_fd_(kInvalidFd), times_restarted_(0), failed_to_start_(false), reported_invalid_path_(false), - use_forkpty_(use_forkpty) { + use_posix_spawn_(use_posix_spawn) { CHECK(path_); CHECK_NE(path_[0], '\0'); } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp @@ -50,7 +50,7 @@ class AtosSymbolizerProcess : public SymbolizerProcess { public: explicit AtosSymbolizerProcess(const char *path, pid_t parent_pid) - : SymbolizerProcess(path, /*use_forkpty*/ true) { + : SymbolizerProcess(path, /*use_posix_spawn*/ true) { // Put the string command line argument in the object so that it outlives // the call to GetArgV. internal_snprintf(pid_str_, sizeof(pid_str_), "%d", parent_pid); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp @@ -33,10 +33,6 @@ #include #include -#if SANITIZER_MAC -#include // for forkpty() -#endif // SANITIZER_MAC - // C++ demangling function, as required by Itanium C++ ABI. This is weak, // because we do not require a C++ ABI library to be linked to a program // using sanitizers; if it's not present, we'll just use the mangled name. @@ -151,80 +147,32 @@ return false; } - int pid = -1; - - int infd[2]; - internal_memset(&infd, 0, sizeof(infd)); - int outfd[2]; - internal_memset(&outfd, 0, sizeof(outfd)); - if (!CreateTwoHighNumberedPipes(infd, outfd)) { - Report("WARNING: Can't create a socket pair to start " - "external symbolizer (errno: %d)\n", errno); - return false; - } + const char *argv[kArgVMax]; + GetArgV(path_, argv); + pid_t pid; - if (use_forkpty_) { + if (use_posix_spawn_) { #if SANITIZER_MAC - fd_t fd = kInvalidFd; - - // forkpty redirects stdout and stderr into a single stream, so we would - // receive error messages as standard replies. To avoid that, let's dup - // stderr and restore it in the child. - int saved_stderr = dup(STDERR_FILENO); - CHECK_GE(saved_stderr, 0); - - // We only need one pipe, for stdin of the child. - close(outfd[0]); - close(outfd[1]); - - // Use forkpty to disable buffering in the new terminal. - pid = internal_forkpty(&fd); - if (pid == -1) { - // forkpty() failed. - Report("WARNING: failed to fork external symbolizer (errno: %d)\n", + fd_t fd = internal_spawn(argv, &pid); + if (fd == kInvalidFd) { + Report("WARNING: failed to spawn external symbolizer (errno: %d)\n", errno); return false; - } else if (pid == 0) { - // Child subprocess. - - // infd[0] is the child's reading end. - close(infd[1]); - - // Set up stdin to read from the pipe. - CHECK_GE(dup2(infd[0], STDIN_FILENO), 0); - close(infd[0]); - - // Restore stderr. - CHECK_GE(dup2(saved_stderr, STDERR_FILENO), 0); - close(saved_stderr); - - const char *argv[kArgVMax]; - GetArgV(path_, argv); - execv(path_, const_cast(&argv[0])); - internal__exit(1); } - // Input for the child, infd[1] is our writing end. - output_fd_ = infd[1]; - close(infd[0]); - - // Continue execution in parent process. input_fd_ = fd; - - close(saved_stderr); - - // Disable echo in the new terminal, disable CR. - struct termios termflags; - tcgetattr(fd, &termflags); - termflags.c_oflag &= ~ONLCR; - termflags.c_lflag &= ~ECHO; - tcsetattr(fd, TCSANOW, &termflags); + output_fd_ = fd; #else // SANITIZER_MAC UNIMPLEMENTED(); #endif // SANITIZER_MAC } else { - const char *argv[kArgVMax]; - GetArgV(path_, argv); + fd_t infd[2] = {}, outfd[2] = {}; + if (!CreateTwoHighNumberedPipes(infd, outfd)) { + Report("WARNING: Can't create a socket pair to start " + "external symbolizer (errno: %d)\n", errno); + return false; + } + pid = StartSubprocess(path_, argv, /* stdin */ outfd[0], /* stdout */ infd[1]); if (pid < 0) {