diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp index a6f757173728b..3f8de8dd064a5 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp @@ -281,53 +281,43 @@ int internal_sysctlbyname(const char *sname, void *oldp, uptr *oldlenp, (size_t)newlen); } -static fd_t internal_spawn_impl(const char *argv[], const char *envp[], - pid_t *pid) { - fd_t primary_fd = kInvalidFd; - fd_t secondary_fd = kInvalidFd; - +bool internal_spawn(const char* argv[], const char* envp[], pid_t* pid, + fd_t fd_stdin, fd_t fd_stdout) { + // NOTE: Caller ensures that fd_stdin and fd_stdout are not 0, 1, or 2, since + // this can break communication. + // + // NOTE: Caller is responsible for closing fd_stdin after the process has + // died. + + int res; auto fd_closer = at_scope_exit([&] { - internal_close(primary_fd); - internal_close(secondary_fd); + // NOTE: We intentionally do not close fd_stdin since this can + // cause us to receive a fatal SIGPIPE if the process dies. + internal_close(fd_stdout); }); - // 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. - primary_fd = posix_openpt(O_RDWR); - if (primary_fd == kInvalidFd) - return kInvalidFd; - - int res = grantpt(primary_fd) || unlockpt(primary_fd); - if (res != 0) return kInvalidFd; - - // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. - char secondary_pty_name[128]; - res = ioctl(primary_fd, TIOCPTYGNAME, secondary_pty_name); - if (res == -1) return kInvalidFd; - - secondary_fd = internal_open(secondary_pty_name, O_RDWR); - if (secondary_fd == kInvalidFd) - return kInvalidFd; - // File descriptor actions posix_spawn_file_actions_t acts; res = posix_spawn_file_actions_init(&acts); - if (res != 0) return kInvalidFd; + if (res != 0) + return false; auto acts_cleanup = at_scope_exit([&] { posix_spawn_file_actions_destroy(&acts); }); - res = posix_spawn_file_actions_adddup2(&acts, secondary_fd, STDIN_FILENO) || - posix_spawn_file_actions_adddup2(&acts, secondary_fd, STDOUT_FILENO) || - posix_spawn_file_actions_addclose(&acts, secondary_fd); - if (res != 0) return kInvalidFd; + res = posix_spawn_file_actions_adddup2(&acts, fd_stdin, STDIN_FILENO) || + posix_spawn_file_actions_adddup2(&acts, fd_stdout, STDOUT_FILENO) || + posix_spawn_file_actions_addclose(&acts, fd_stdin) || + posix_spawn_file_actions_addclose(&acts, fd_stdout); + if (res != 0) + return false; // Spawn attributes posix_spawnattr_t attrs; res = posix_spawnattr_init(&attrs); - if (res != 0) return kInvalidFd; + if (res != 0) + return false; auto attrs_cleanup = at_scope_exit([&] { posix_spawnattr_destroy(&attrs); @@ -336,50 +326,17 @@ static fd_t internal_spawn_impl(const char *argv[], const char *envp[], // In the spawned process, close all file descriptors that are not explicitly // described by the file actions object. This is Darwin-specific extension. res = posix_spawnattr_setflags(&attrs, POSIX_SPAWN_CLOEXEC_DEFAULT); - if (res != 0) return kInvalidFd; + if (res != 0) + return false; // posix_spawn char **argv_casted = const_cast(argv); char **envp_casted = const_cast(envp); res = posix_spawn(pid, argv[0], &acts, &attrs, argv_casted, envp_casted); - if (res != 0) return kInvalidFd; - - // Disable echo in the new terminal, disable CR. - struct termios termflags; - tcgetattr(primary_fd, &termflags); - termflags.c_oflag &= ~ONLCR; - termflags.c_lflag &= ~ECHO; - tcsetattr(primary_fd, TCSANOW, &termflags); - - // On success, do not close primary_fd on scope exit. - fd_t fd = primary_fd; - primary_fd = kInvalidFd; - - return fd; -} - -fd_t internal_spawn(const char *argv[], const char *envp[], 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 = 0; - - for (; count < 3; count++) { - low_fds[count] = posix_openpt(O_RDWR); - if (low_fds[count] >= STDERR_FILENO) - break; - } - - fd_t fd = internal_spawn_impl(argv, envp, pid); - - for (; count > 0; count--) { - internal_close(low_fds[count]); - } + if (res != 0) + return false; - return fd; + return true; } 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 index b5491c540dc08..063408b8360c1 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h @@ -67,7 +67,8 @@ uptr internal_ptrace(int request, int pid, void *addr, void *data); uptr internal_waitpid(int pid, int *status, int options); int internal_fork(); -fd_t internal_spawn(const char *argv[], const char *envp[], pid_t *pid); +bool internal_spawn(const char* argv[], const char* envp[], pid_t* pid, + fd_t stdin, fd_t stdout); 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 index 2345aee985541..6442a2980bf2f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_internal.h @@ -83,7 +83,7 @@ class SymbolizerProcess { const char *SendCommand(const char *command); protected: - ~SymbolizerProcess() {} + ~SymbolizerProcess(); /// The maximum number of arguments required to invoke a tool process. static const unsigned kArgVMax = 16; @@ -114,6 +114,10 @@ class SymbolizerProcess { fd_t input_fd_; fd_t output_fd_; + // We hold on to the child's stdin fd (the read end of the pipe) + // so that when we write to it, we don't get a SIGPIPE + fd_t child_stdin_fd_; + InternalMmapVector buffer_; static const uptr kMaxTimesRestarted = 5; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp index 565701c85d978..cc31d3d8056f9 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp @@ -476,10 +476,11 @@ const char *LLVMSymbolizer::FormatAndSendCommand(const char *command_prefix, return symbolizer_process_->SendCommand(buffer_); } -SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn) +SymbolizerProcess::SymbolizerProcess(const char* path, bool use_posix_spawn) : path_(path), input_fd_(kInvalidFd), output_fd_(kInvalidFd), + child_stdin_fd_(kInvalidFd), times_restarted_(0), failed_to_start_(false), reported_invalid_path_(false), @@ -488,6 +489,11 @@ SymbolizerProcess::SymbolizerProcess(const char *path, bool use_posix_spawn) CHECK_NE(path_[0], '\0'); } +SymbolizerProcess::~SymbolizerProcess() { + if (child_stdin_fd_ != kInvalidFd) + CloseFile(child_stdin_fd_); +} + static bool IsSameModule(const char *path) { if (const char *ProcessName = GetProcessName()) { if (const char *SymbolizerName = StripModuleName(path)) { @@ -533,6 +539,10 @@ bool SymbolizerProcess::Restart() { CloseFile(input_fd_); if (output_fd_ != kInvalidFd) CloseFile(output_fd_); + if (child_stdin_fd_ != kInvalidFd) { + CloseFile(child_stdin_fd_); + child_stdin_fd_ = kInvalidFd; // Don't free in destructor + } return StartSymbolizerSubprocess(); } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp index 7eb0c9756d64a..29c73e3e1cac1 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp @@ -156,30 +156,30 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() { Printf("\n"); } + 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; + } + if (use_posix_spawn_) { # if SANITIZER_APPLE - fd_t fd = internal_spawn(argv, const_cast(GetEnvP()), &pid); - if (fd == kInvalidFd) { + bool success = internal_spawn(argv, const_cast(GetEnvP()), + &pid, outfd[0], infd[1]); + if (!success) { Report("WARNING: failed to spawn external symbolizer (errno: %d)\n", errno); + internal_close(infd[0]); + internal_close(outfd[1]); return false; } - - input_fd_ = fd; - output_fd_ = fd; # else // SANITIZER_APPLE UNIMPLEMENTED(); # endif // SANITIZER_APPLE } else { - 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, GetEnvP(), /* stdin */ outfd[0], /* stdout */ infd[1]); if (pid < 0) { @@ -187,11 +187,14 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() { internal_close(outfd[1]); return false; } - - input_fd_ = infd[0]; - output_fd_ = outfd[1]; } + input_fd_ = infd[0]; + output_fd_ = outfd[1]; + + // We intentionally hold on to the read-end so that we don't get a SIGPIPE + child_stdin_fd_ = outfd[0]; + CHECK_GT(pid, 0); // Check that symbolizer subprocess started successfully.