[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 15:44:52 PST 2025
https://github.com/ndrewh updated https://github.com/llvm/llvm-project/pull/170809
>From f1678fc7c2a1612c5964acfb7a3b259062715e9b 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/4] Replace pty with pipe in macOS posix_spawn path
---
.../lib/sanitizer_common/sanitizer_mac.cpp | 86 ++++---------------
.../lib/sanitizer_common/sanitizer_posix.h | 2 +-
.../sanitizer_symbolizer_posix_libcdep.cpp | 33 ++++---
3 files changed, 34 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..9f01499f29b21 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -281,53 +281,36 @@ 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 +319,15 @@ 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);
+ if (res != 0) return false;
- for (; count > 0; count--) {
- internal_close(low_fds[count]);
- }
-
- 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..259388da3747e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
@@ -67,7 +67,7 @@ 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..d8506a506141f 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,29 @@ 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 +186,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 1dd09afc138567b0a66056e7c5fd8c80c892bb88 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/4] 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 | 5 ++++-
.../lib/sanitizer_common/sanitizer_symbolizer_internal.h | 6 +++++-
.../lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp | 8 ++++++++
.../sanitizer_symbolizer_posix_libcdep.cpp | 3 +++
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 9f01499f29b21..3810b94fe5fee 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -285,10 +285,13 @@ 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..eaedb8679bc88 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -480,6 +480,7 @@ 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 d8506a506141f..44cb1ae0ca4f2 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -191,6 +191,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.
>From 0bec80fdef6b40a0ed0e51a964edb5fba9f0fce5 Mon Sep 17 00:00:00 2001
From: Andrew Haberlandt <ahaberlandt at apple.com>
Date: Fri, 5 Dec 2025 00:41:31 -0800
Subject: [PATCH 3/4] clang-format
---
.../lib/sanitizer_common/sanitizer_mac.cpp | 26 ++++++++++++-------
.../lib/sanitizer_common/sanitizer_posix.h | 3 ++-
.../sanitizer_symbolizer_libcdep.cpp | 2 +-
.../sanitizer_symbolizer_posix_libcdep.cpp | 3 ++-
4 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 3810b94fe5fee..3f8de8dd064a5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -281,12 +281,13 @@ int internal_sysctlbyname(const char *sname, void *oldp, uptr *oldlenp,
(size_t)newlen);
}
-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.
+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.
+ // NOTE: Caller is responsible for closing fd_stdin after the process has
+ // died.
int res;
auto fd_closer = at_scope_exit([&] {
@@ -298,7 +299,8 @@ bool internal_spawn(const char *argv[], const char *envp[],
// File descriptor actions
posix_spawn_file_actions_t acts;
res = posix_spawn_file_actions_init(&acts);
- if (res != 0) return false;
+ if (res != 0)
+ return false;
auto acts_cleanup = at_scope_exit([&] {
posix_spawn_file_actions_destroy(&acts);
@@ -308,12 +310,14 @@ bool internal_spawn(const char *argv[], const char *envp[],
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;
+ if (res != 0)
+ return false;
// Spawn attributes
posix_spawnattr_t attrs;
res = posix_spawnattr_init(&attrs);
- if (res != 0) return false;
+ if (res != 0)
+ return false;
auto attrs_cleanup = at_scope_exit([&] {
posix_spawnattr_destroy(&attrs);
@@ -322,13 +326,15 @@ bool internal_spawn(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 false;
+ 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 false;
+ if (res != 0)
+ return false;
return true;
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
index 259388da3747e..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();
-bool internal_spawn(const char *argv[], const char *envp[], pid_t *pid, fd_t stdin, fd_t stdout);
+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_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
index eaedb8679bc88..8ee0e23e8b575 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -476,7 +476,7 @@ 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),
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 44cb1ae0ca4f2..29c73e3e1cac1 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -167,7 +167,8 @@ bool SymbolizerProcess::StartSymbolizerSubprocess() {
if (use_posix_spawn_) {
# if SANITIZER_APPLE
- bool success = internal_spawn(argv, const_cast<const char **>(GetEnvP()), &pid, outfd[0], infd[1]);
+ 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);
>From 3497a665e9abb31a9cedf798faac1b38efd77ea2 Mon Sep 17 00:00:00 2001
From: Andrew Haberlandt <ahaberlandt at apple.com>
Date: Fri, 5 Dec 2025 15:44:40 -0800
Subject: [PATCH 4/4] Fix typo in fd cleanup
---
.../lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
index 8ee0e23e8b575..11d399883a0b7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp
@@ -539,8 +539,10 @@ bool SymbolizerProcess::Restart() {
CloseFile(input_fd_);
if (output_fd_ != kInvalidFd)
CloseFile(output_fd_);
- if (child_stdin_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();
}
More information about the llvm-commits
mailing list