[compiler-rt] [sanitizer_common] [Darwin] Replace pty with pipe on posix_spawn path for spawning symbolizer (PR #170809)
Andrew Haberlandt via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 5 00:40:08 PST 2025
https://github.com/ndrewh updated https://github.com/llvm/llvm-project/pull/170809
>From 5bf0fa0ab5fa29e33f19c2144435a6c4e964dc09 Mon Sep 17 00:00:00 2001
From: Andrew Haberlandt <ahaberlandt at apple.com>
Date: Fri, 5 Dec 2025 00:02:25 -0800
Subject: [PATCH 1/2] Replace pty with pipe in macOS posix_spawn path
---
.../lib/sanitizer_common/sanitizer_mac.cpp | 91 +++++--------------
.../lib/sanitizer_common/sanitizer_posix.h | 3 +-
.../sanitizer_symbolizer_posix_libcdep.cpp | 34 +++----
3 files changed, 41 insertions(+), 87 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index a6f757173728b..7e9262c23bdeb 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -281,53 +281,39 @@ 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.
+ int res;
auto fd_closer = at_scope_exit([&] {
- internal_close(primary_fd);
- internal_close(secondary_fd);
+ internal_close(fd_stdin);
+ 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 +322,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<char **>(argv);
char **envp_casted = const_cast<char **>(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_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
index 7eb0c9756d64a..ee11f406c1923 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<const char **>(GetEnvP()), &pid);
- if (fd == kInvalidFd) {
+ bool success = internal_spawn(argv, const_cast<const char**>(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,11 @@ 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];
+
CHECK_GT(pid, 0);
// Check that symbolizer subprocess started successfully.
>From 782245c689729aa61ebb80e7eb00094c84e79c1d Mon Sep 17 00:00:00 2001
From: Andrew Haberlandt <ahaberlandt at apple.com>
Date: Fri, 5 Dec 2025 00:03:02 -0800
Subject: [PATCH 2/2] SymbolizerProcess should hold on to the read end of the
stdin pipe so we don't get SIGPIPE
---
compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp | 6 +++++-
.../sanitizer_common/sanitizer_symbolizer_internal.h | 6 +++++-
.../sanitizer_common/sanitizer_symbolizer_libcdep.cpp | 10 +++++++++-
.../sanitizer_symbolizer_posix_libcdep.cpp | 3 +++
4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 7e9262c23bdeb..3f8de8dd064a5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -285,10 +285,14 @@ 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(fd_stdin);
+ // 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);
});
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<char> 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..8ee0e23e8b575 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,8 @@ bool SymbolizerProcess::Restart() {
CloseFile(input_fd_);
if (output_fd_ != kInvalidFd)
CloseFile(output_fd_);
+ if (child_stdin_fd_ != kInvalidFd)
+ CloseFile(output_fd_);
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 ee11f406c1923..29c73e3e1cac1 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -192,6 +192,9 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {
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.
More information about the llvm-commits
mailing list