Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
217 changes: 209 additions & 8 deletions subprocess.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,63 @@ subprocess_weak int subprocess_alive(struct subprocess_s *const process);
#include <unistd.h>
#endif

/* __NetBSD_Version__ lives in <sys/param.h>, not in the compiler's predefines,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is verbose and not interesting, just remove it.

and the probes below need it. Including it early keeps them honest; without
it the version folds to 0 and every NetBSD would take the fork path, which
works but is heavier than necessary on the releases that do not need it. */
#if defined(__NetBSD__)
#include <sys/param.h>
#endif

/* Which spelling of the chdir file action the platform provides, if any.
POSIX 2024 standardised posix_spawn_file_actions_addchdir; implementations
that shipped it earlier called it ..._np. macOS 26 and NetBSD 10 use the
standard name, glibc 2.29+, macOS 10.15+ and FreeBSD 13.1+ use the _np name,
and AIX, NetBSD 9 and older, and OpenBSD provide neither. */
#if !defined(SUBPROCESS_ADDCHDIR_IS_POSIX)
#if (defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED >= 260000) || \
(defined(__NetBSD__) && __NetBSD_Version__ >= 1000000000)
#define SUBPROCESS_ADDCHDIR_IS_POSIX 1
#else
#define SUBPROCESS_ADDCHDIR_IS_POSIX 0
#endif
#endif

/* Whether to launch the child with fork()+exec() instead of posix_spawn().

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this comment more succinct please.

Some platforms provide no posix_spawn_file_actions_addchdir under either
spelling, so process_cwd cannot be honoured through posix_spawn at all:
AIX has never had it, OpenBSD does not have it as of 7.9, and NetBSD gained
it only in 10.0. Forking lets the child chdir() before exec, and a
close-on-exec pipe carries any exec() errno back to the parent, so both
capabilities below become available on such platforms.
Define this yourself to force either implementation. */
#if !defined(SUBPROCESS_SPAWN_VIA_FORK)
#if defined(_AIX) || defined(__OpenBSD__) || \
(defined(__NetBSD__) && (__NetBSD_Version__ < 1000000000))
#define SUBPROCESS_SPAWN_VIA_FORK 1
#else
#define SUBPROCESS_SPAWN_VIA_FORK 0
#endif
#endif

/* Exit status used by the forked child when it cannot reach exec. No header
defines this: 127 is the shell convention for "command not found" (POSIX
XCU 2.8.2), and the value glibc's posix_spawn used before it learned to
report exec failures. The real reason travels over the error pipe; this is
only what a caller sees if that write is lost. */
#if !defined(SUBPROCESS_EXEC_FAILURE_STATUS)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we even need this define?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

#define SUBPROCESS_EXEC_FAILURE_STATUS 127
#endif

/* Whether subprocess_create_ex can honour process_cwd. glibc only gained
posix_spawn_file_actions_addchdir_np in 2.29, and macOS in 10.15; the SDKs
mark it unavailable on iOS, tvOS and watchOS, where the undefined version
macro folds to 0 and so answers correctly. Define this yourself to override
the detection, for instance on musl older than 1.1.24. */
#if !defined(SUBPROCESS_HAVE_CWD)
#if defined(__GLIBC__)
#if SUBPROCESS_SPAWN_VIA_FORK
#define SUBPROCESS_HAVE_CWD 1
#elif defined(__GLIBC__)
#if __GLIBC_PREREQ(2, 29)
#define SUBPROCESS_HAVE_CWD 1
#else
Expand All @@ -294,10 +344,13 @@ subprocess_weak int subprocess_alive(struct subprocess_s *const process);
#endif
#endif

/* Whether posix_spawn reports a failed exec back to the caller. glibc only
started doing so in 2.24; before that the child silently exits with 127. */
/* Whether a failed exec is reported back to the caller. The fork() path always
reports it through its error pipe. glibc's posix_spawn only started doing so
in 2.24; before that the child silently exits with 127. */
#if !defined(SUBPROCESS_SPAWN_REPORTS_EXEC_ERRORS)
#if defined(__GLIBC__)
#if SUBPROCESS_SPAWN_VIA_FORK
#define SUBPROCESS_SPAWN_REPORTS_EXEC_ERRORS 1
#elif defined(__GLIBC__)
#if __GLIBC_PREREQ(2, 24)
#define SUBPROCESS_SPAWN_REPORTS_EXEC_ERRORS 1
#else
Expand Down Expand Up @@ -1173,15 +1226,20 @@ int subprocess_create_ex(const char *const commandLine[], int options,
int stderrfd[2] = {-1, -1};
int fd, fd_flags;
int async_no_wait;
int actions_created = 0;
int result = subprocess_error_unknown;
int saved_errno = 0;
int posix_error;
pid_t child = 0;
extern char **environ;
char *const empty_environment[1] = {SUBPROCESS_NULL};
posix_spawn_file_actions_t actions;
char *const *used_environment;
#if SUBPROCESS_SPAWN_VIA_FORK
/* Pipe used to relay the child's exec() errno back to the parent. */
int exec_errfd[2] = {-1, -1};
#else
int actions_created = 0;
int posix_error;
posix_spawn_file_actions_t actions;
#endif

async_no_wait = subprocess_option_enable_async_no_wait ==
(options & subprocess_option_enable_async_no_wait);
Expand Down Expand Up @@ -1240,6 +1298,136 @@ int subprocess_create_ex(const char *const commandLine[], int options,
used_environment = empty_environment;
}

#if SUBPROCESS_SPAWN_VIA_FORK
/* fork()+exec() instead of posix_spawn, so the child can chdir() first.
exec_errfd[1] is close-on-exec: a successful exec closes it and the parent
reads EOF; a failed exec writes errno through it before _exit. */
if (0 != pipe(exec_errfd)) {
saved_errno = errno;
result = subprocess_error_pipe;
goto cleanup;
}

if (-1 == fcntl(exec_errfd[1], F_SETFD, FD_CLOEXEC)) {
saved_errno = errno;
result = subprocess_error_spawn;
goto cleanup;
}

child = fork();

if (child < 0) {
saved_errno = errno;
result = subprocess_error_spawn;
goto cleanup;
}

if (0 == child) {
/* Child. Everything below must stay async-signal-safe: after fork() in a
threaded process only such functions may be called before exec. */
int child_errno;

close(exec_errfd[0]);

if ((-1 == dup2(stdinfd[0], STDIN_FILENO)) ||
(-1 == dup2(stdoutfd[1], STDOUT_FILENO))) {
goto child_failed;
}

if (subprocess_option_combined_stdout_stderr ==
(options & subprocess_option_combined_stdout_stderr)) {
if (-1 == dup2(STDOUT_FILENO, STDERR_FILENO)) {
goto child_failed;
}
} else {
if (-1 == dup2(stderrfd[1], STDERR_FILENO)) {
goto child_failed;
}
}

/* The originals are only closed once they have been duplicated, so that a
pipe end that already sits on 0, 1 or 2 is not closed out from under us. */
if (stdinfd[0] > STDERR_FILENO) {
close(stdinfd[0]);
}
if (stdinfd[1] > STDERR_FILENO) {
close(stdinfd[1]);
}
if (stdoutfd[0] > STDERR_FILENO) {
close(stdoutfd[0]);
}
if (stdoutfd[1] > STDERR_FILENO) {
close(stdoutfd[1]);
}
if (stderrfd[0] > STDERR_FILENO) {
close(stderrfd[0]);
}
if (stderrfd[1] > STDERR_FILENO) {
close(stderrfd[1]);
}

if (process_cwd && (0 != chdir(process_cwd))) {
goto child_failed;
}

#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wcast-qual"
#pragma clang diagnostic ignored "-Wold-style-cast"
#endif
if (subprocess_option_search_user_path ==
(options & subprocess_option_search_user_path)) {
/* AIX exports execvpe from libc but declares it in no header. */

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we only extern to this in a define block for AIX above the function instead then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it above the function, but guarded on SUBPROCESS_SPAWN_VIA_FORK
rather than _AIX.

Narrowing it to AIX breaks glibc: execvpe is a GNU extension there, so
<unistd.h> only declares it when _GNU_SOURCE is set, and anyone forcing
the fork path without that macro loses the declaration. Measured both ways
with -Werror=implicit-function-declaration — AIX-only fails without
_GNU_SOURCE and builds with it. The suite can't see either way, since
test/CMakeLists.txt defines _GNU_SOURCE on Linux. musl hides it the same
way.

The comment is no longer AIX-specific either: OpenBSD and NetBSD reach this
code too, and both do have execvpe.

extern int execvpe(const char *, char *const *, char *const *);
execvpe(commandLine[0],
SUBPROCESS_CONST_CAST(char *const *, commandLine),
SUBPROCESS_CONST_CAST(char *const *, used_environment));
} else {
execve(commandLine[0],
SUBPROCESS_CONST_CAST(char *const *, commandLine),
SUBPROCESS_CONST_CAST(char *const *, used_environment));
}
#ifdef __clang__
#pragma clang diagnostic pop
#endif

child_failed:
child_errno = errno;
/* Nothing useful can be done if this write fails; the parent then sees EOF
and reports success, exactly as posix_spawn would without exec reporting. */
(void)!write(exec_errfd[1], &child_errno, sizeof(child_errno));
_exit(SUBPROCESS_EXEC_FAILURE_STATUS);
}

/* Parent. */
close(exec_errfd[1]);
exec_errfd[1] = -1;

{
int child_errno = 0;
ssize_t bytes_read;

do {
bytes_read = read(exec_errfd[0], &child_errno, sizeof(child_errno));
} while ((-1 == bytes_read) && (EINTR == errno));

close(exec_errfd[0]);
exec_errfd[0] = -1;

if (bytes_read == (ssize_t)sizeof(child_errno)) {
/* exec failed in the child. Reap it and surface the reason. */
while ((-1 == waitpid(child, SUBPROCESS_NULL, 0)) && (EINTR == errno)) {
}
child = 0;
saved_errno = child_errno;
result = subprocess_error_from_errno(child_errno);
if (subprocess_error_unknown == result) {
result = subprocess_error_spawn;
}
goto cleanup;
}
}
#else
posix_error = posix_spawn_file_actions_init(&actions);
if (0 != posix_error) {
saved_errno = posix_error;
Expand All @@ -1253,7 +1441,7 @@ int subprocess_create_ex(const char *const commandLine[], int options,

// Set working directory
if (process_cwd) {
#if defined(__NetBSD__) || (defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED >= 260000)
#if SUBPROCESS_ADDCHDIR_IS_POSIX
posix_error = posix_spawn_file_actions_addchdir(&actions, process_cwd);
#elif !SUBPROCESS_HAVE_CWD
posix_error = ENOSYS;
Expand Down Expand Up @@ -1406,6 +1594,7 @@ int subprocess_create_ex(const char *const commandLine[], int options,
#ifdef __clang__
#pragma clang diagnostic pop
#endif
#endif /* SUBPROCESS_SPAWN_VIA_FORK */

// Close the stdin read end
close(stdinfd[0]);
Expand Down Expand Up @@ -1480,9 +1669,21 @@ int subprocess_create_ex(const char *const commandLine[], int options,
result = subprocess_error_from_errno(saved_errno);
}

#if SUBPROCESS_SPAWN_VIA_FORK
if (-1 != exec_errfd[0]) {
close(exec_errfd[0]);
exec_errfd[0] = -1;
}

if (-1 != exec_errfd[1]) {
close(exec_errfd[1]);
exec_errfd[1] = -1;
}
#else
if (actions_created) {
posix_spawn_file_actions_destroy(&actions);
}
#endif

if (0 != result) {
if (child) {
Expand Down
13 changes: 11 additions & 2 deletions test/test_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,17 @@ SUBPROCESS_TEST(create, subprocess_fail_divzero) {
ASSERT_EQ(0, subprocess_join(&process, &ret));
ASSERT_EQ(0, subprocess_destroy(&process));

// On AArch64 systems divide by zero does not cause a failure.
#if !((defined(__arm64__) && defined(__APPLE__)) || defined(__aarch64__))
// Not every architecture traps on integer division by zero, and the ones that
// do not are specified to return a value instead:
// AArch64 SDIV yields 0
// PowerPC divw/divd leave the result undefined, no exception unless OE
// RISC-V DIV yields -1, i.e. all bits set
// Measured with the same source and compiler on each: x86_64, armv7l and s390x
// die on SIGFPE, while aarch64 exits 0 printing 0, ppc64le exits 0, and riscv64
// exits 0 printing -1. Reproduced on AIX 7.2/POWER8 too. _ARCH_PPC is listed
// beside __powerpc__ because IBM XL C and OpenXL define only the former.
#if !((defined(__arm64__) && defined(__APPLE__)) || defined(__aarch64__) || \
defined(__powerpc__) || defined(_ARCH_PPC) || defined(__riscv))
ASSERT_NE(ret, 0);
#endif
}
Expand Down
Loading