From d5cc747d4e858ea5f9d5285aebaa59433205b3d9 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Wed, 9 Sep 2026 18:29:34 -0700 Subject: [PATCH 1/4] Support Darwin subprocess working directories with posix_spawn --- xls/common/BUILD | 1 + xls/common/subprocess.cc | 35 +++++++++++++++++++++++++++++++++++ xls/common/subprocess_test.cc | 28 ++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/xls/common/BUILD b/xls/common/BUILD index c18a9711c9..1f391f48fb 100644 --- a/xls/common/BUILD +++ b/xls/common/BUILD @@ -318,6 +318,7 @@ cc_test( deps = [ ":subprocess", ":xls_gunit_main", + "//xls/common/file:temp_directory", "//xls/common/status:matchers", "@abseil-cpp//absl/status", "@abseil-cpp//absl/status:status_matchers", diff --git a/xls/common/subprocess.cc b/xls/common/subprocess.cc index 1c04f29d64..b941d05fa7 100644 --- a/xls/common/subprocess.cc +++ b/xls/common/subprocess.cc @@ -15,7 +15,9 @@ #include "xls/common/subprocess.h" #include +#if !defined(__APPLE__) #include +#endif #include // NOLINT #include #include // NOLINT for WIFEXITED, WEXITSTATUS; not in @@ -129,6 +131,7 @@ absl::StatusOr CreateChildFileActions( return actions; } +#if !defined(__APPLE__) class CleanableFd { public: explicit CleanableFd(int fd) : fd_(fd) {} @@ -173,6 +176,22 @@ absl::StatusOr GetSubprocessHelperFd() { return std::move(fd); } +#else +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd) { +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 + return posix_spawn_file_actions_addchdir(file_actions, cwd); +#else +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_26_0 + if (__builtin_available(macOS 26.0, *)) { + return posix_spawn_file_actions_addchdir(file_actions, cwd); + } +#endif + return posix_spawn_file_actions_addchdir_np(file_actions, cwd); +#endif +} +#endif + absl::StatusOr ExecInChildProcess( const std::vector& argv_pointers, const std::optional& cwd, Pipe& stdout_pipe, @@ -184,6 +203,11 @@ absl::StatusOr ExecInChildProcess( // better, but it's not fully clear what's safe between vfork() and exec() // either, so we just use posix_spawn for safety and convenience. +#if defined(__APPLE__) + // Darwin can change the child directory without a helper executable. + std::string subprocess_helper = argv_pointers.front(); + std::vector helper_argv_pointers = argv_pointers; +#else // Since we may need the child to have a different working directory (per // `cwd`), and posix_spawn does not (yet) have support for a chdir action, we // use a helper binary that chdir's to its first argument, then invokes @@ -205,8 +229,19 @@ absl::StatusOr ExecInChildProcess( helper_argv_pointers.insert(helper_argv_pointers.end(), argv_pointers.begin(), argv_pointers.end()); +#endif + XLS_ASSIGN_OR_RETURN(posix_spawn_file_actions_t file_actions, CreateChildFileActions(stdout_pipe, stderr_pipe)); +#if defined(__APPLE__) + if (cwd.has_value()) { + if (int err = AddChdirFileAction(&file_actions, cwd->c_str()); err != 0) { + posix_spawn_file_actions_destroy(&file_actions); + return absl::InternalError(absl::StrCat( + "Cannot add child working directory action: ", Strerror(err))); + } + } +#endif // posix_spawnp takes a null-terminate array of char* for environment // variables. Each element has the form "NAME=VALUE". diff --git a/xls/common/subprocess_test.cc b/xls/common/subprocess_test.cc index 4c2cc5adb7..a4f7b5224b 100644 --- a/xls/common/subprocess_test.cc +++ b/xls/common/subprocess_test.cc @@ -14,6 +14,7 @@ #include "xls/common/subprocess.h" +#include #include #include #include @@ -25,6 +26,7 @@ #include "absl/status/statusor.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "xls/common/file/temp_directory.h" #include "xls/common/status/matchers.h" namespace xls { @@ -43,6 +45,32 @@ TEST(SubprocessTest, EmptyArgvFails) { EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument)); } +TEST(SubprocessTest, WorkingDirectoryDoesNotChangeParent) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + const std::filesystem::path parent_directory = + std::filesystem::current_path(); + + XLS_ASSERT_OK_AND_ASSIGN(SubprocessResult result, + SubprocessErrorAsStatus(InvokeSubprocess( + {"pwd", "-P"}, directory.path()))); + + EXPECT_EQ(result.stdout_content, + std::filesystem::canonical(directory.path()).string() + "\n"); + EXPECT_EQ(std::filesystem::current_path(), parent_directory); +} + +TEST(SubprocessTest, RelativeExecutableUsesChildWorkingDirectory) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + std::filesystem::create_symlink("/bin/pwd", directory.path() / "command"); + + XLS_ASSERT_OK_AND_ASSIGN(SubprocessResult result, + SubprocessErrorAsStatus(InvokeSubprocess( + {"./command", "-P"}, directory.path()))); + + EXPECT_EQ(result.stdout_content, + std::filesystem::canonical(directory.path()).string() + "\n"); +} + TEST(SubprocessTest, NonZeroExitWorks) { auto result = InvokeSubprocess({"/usr/bin/env", "bash", "-c", From 969d4ff76e711e141e422bc2fd038ddadc428c14 Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Fri, 11 Sep 2026 14:13:15 -0700 Subject: [PATCH 2/4] Split subprocess launching into OS-specific implementations Move the Linux embedded helper and Darwin spawn code behind a common SpawnSubprocess interface selected by Bazel's target OS. Keep pipe capture, environment construction, timeouts, and waiting in subprocess.cc. The macOS target no longer depends on the embedded Linux helper. Search the child's PATH with posix_spawn on macOS to avoid the posix_spawnp/chdir failure with relative PATH entries. Preserve execvp's shell fallback and test relative and empty PATH entries, permission failures, slash-containing commands, and scripts without shebangs. Destroy spawn file actions on every return path without losing ownership of a live child when cleanup fails. --- xls/common/BUILD | 51 ++++++- xls/common/subprocess.cc | 155 +++------------------ xls/common/subprocess_for_os.h | 45 +++++++ xls/common/subprocess_for_os_linux.cc | 92 +++++++++++++ xls/common/subprocess_for_os_macos.cc | 168 +++++++++++++++++++++++ xls/common/subprocess_for_os_test.cc | 185 ++++++++++++++++++++++++++ 6 files changed, 561 insertions(+), 135 deletions(-) create mode 100644 xls/common/subprocess_for_os.h create mode 100644 xls/common/subprocess_for_os_linux.cc create mode 100644 xls/common/subprocess_for_os_macos.cc create mode 100644 xls/common/subprocess_for_os_test.cc diff --git a/xls/common/BUILD b/xls/common/BUILD index 1f391f48fb..909c0ea214 100644 --- a/xls/common/BUILD +++ b/xls/common/BUILD @@ -288,17 +288,66 @@ xls_cc_embed_data( data = ":subprocess_helper", ) +# Select the launcher using the target OS (including Bazel's native macOS +# platform), so macOS does not build or embed the Linux helper executable. +config_setting( + name = "macos", + constraint_values = ["@platforms//os:osx"], +) + +cc_library( + name = "subprocess_for_os", + srcs = select({ + ":macos": ["subprocess_for_os_macos.cc"], + "@platforms//os:linux": ["subprocess_for_os_linux.cc"], + }), + hdrs = ["subprocess_for_os.h"], + deps = [ + ":strerror", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", + "@abseil-cpp//absl/types:span", + ] + select({ + ":macos": [], + "@platforms//os:linux": [ + ":subprocess_helper_embedded", + "//xls/common/file:file_descriptor", + "//xls/common/status:status_macros", + ], + }), +) + +cc_test( + name = "subprocess_for_os_test", + srcs = ["subprocess_for_os_test.cc"], + deps = [ + ":subprocess_for_os", + ":xls_gunit_main", + "//xls/common/file:filesystem", + "//xls/common/file:temp_directory", + "//xls/common/status:matchers", + "@abseil-cpp//absl/cleanup", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", + "@abseil-cpp//absl/types:span", + "@googletest//:gtest", + ], +) + cc_library( name = "subprocess", srcs = ["subprocess.cc"], hdrs = ["subprocess.h"], deps = [ ":strerror", - ":subprocess_helper_embedded", + ":subprocess_for_os", ":thread", "//xls/common/file:file_descriptor", "//xls/common/logging:log_lines", "//xls/common/status:status_macros", + "@abseil-cpp//absl/cleanup", "@abseil-cpp//absl/container:fixed_array", "@abseil-cpp//absl/log", "@abseil-cpp//absl/log:check", diff --git a/xls/common/subprocess.cc b/xls/common/subprocess.cc index b941d05fa7..1f1284ff8b 100644 --- a/xls/common/subprocess.cc +++ b/xls/common/subprocess.cc @@ -15,13 +15,9 @@ #include "xls/common/subprocess.h" #include -#if !defined(__APPLE__) -#include -#endif #include // NOLINT #include #include // NOLINT for WIFEXITED, WEXITSTATUS; not in -#include #include #include #include @@ -40,6 +36,7 @@ #include #include +#include "absl/cleanup/cleanup.h" #include "absl/container/fixed_array.h" #include "absl/log/check.h" #include "absl/log/log.h" @@ -55,12 +52,10 @@ #include "xls/common/logging/log_lines.h" #include "xls/common/status/status_macros.h" #include "xls/common/strerror.h" -#include "xls/common/subprocess_helper_embedded_embedded.h" +#include "xls/common/subprocess_for_os.h" #include "xls/common/thread.h" -#if defined(__APPLE__) extern char** environ; -#endif namespace xls { namespace { @@ -109,14 +104,8 @@ absl::Status ReplaceFdWithPipe(posix_spawn_file_actions_t& actions, int fd, return absl::OkStatus(); } -absl::StatusOr CreateChildFileActions( - Pipe& stdout_pipe, Pipe& stderr_pipe) { - posix_spawn_file_actions_t actions; - - if (int err = posix_spawn_file_actions_init(&actions); err != 0) { - return absl::InternalError( - absl::StrCat("Cannot initialize file actions: ", Strerror(err))); - } +absl::Status CreateChildFileActions(posix_spawn_file_actions_t& actions, + Pipe& stdout_pipe, Pipe& stderr_pipe) { if (int err = posix_spawn_file_actions_addclose(&actions, STDIN_FILENO); err != 0) { return absl::InternalError( @@ -128,122 +117,29 @@ absl::StatusOr CreateChildFileActions( XLS_RETURN_IF_ERROR( ReplaceFdWithPipe(actions, STDERR_FILENO, stderr_pipe, "stderr")); - return actions; -} - -#if !defined(__APPLE__) -class CleanableFd { - public: - explicit CleanableFd(int fd) : fd_(fd) {} - CleanableFd(const CleanableFd&) = delete; - CleanableFd& operator=(const CleanableFd&) = delete; - CleanableFd(CleanableFd&& o) : fd_(o.fd_) { o.fd_ = -1; } - CleanableFd& operator=(CleanableFd&& o) { - if (this != &o) { - fd_ = o.fd_; - o.fd_ = -1; - } - return *this; - } - ~CleanableFd() { - if (fd_ != -1) { - close(fd_); - } - } - operator int() const { return fd_; } - - private: - int fd_ = -1; -}; - -absl::StatusOr GetSubprocessHelperFd() { - int raw_fd = memfd_create("subprocess_helper", MFD_CLOEXEC); - CleanableFd fd(raw_fd); - if (fd == -1) { - return absl::InternalError(absl::StrCat( - "Failed to create memfd for subprocess helper: ", Strerror(errno))); - } - if (write(fd, get_subprocess_helper_embedded().data(), - get_subprocess_helper_embedded().size()) != - get_subprocess_helper_embedded().size()) { - return absl::InternalError(absl::StrCat( - "Failed to write subprocess helper to memfd: ", Strerror(errno))); - } - if (lseek(fd, 0, SEEK_SET) != 0) { - return absl::InternalError(absl::StrCat( - "Failed to seek subprocess helper in memfd: ", Strerror(errno))); - } - return std::move(fd); -} - -#else -int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, - const char* cwd) { -#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 - return posix_spawn_file_actions_addchdir(file_actions, cwd); -#else -#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_26_0 - if (__builtin_available(macOS 26.0, *)) { - return posix_spawn_file_actions_addchdir(file_actions, cwd); - } -#endif - return posix_spawn_file_actions_addchdir_np(file_actions, cwd); -#endif + return absl::OkStatus(); } -#endif absl::StatusOr ExecInChildProcess( const std::vector& argv_pointers, const std::optional& cwd, Pipe& stdout_pipe, Pipe& stderr_pipe, absl::Span environment_variables) { - // We previously used fork() & exec() here, but that's prone to many subtle - // problems (e.g., allocating between fork() and exec() can cause arbitrary - // problems)... and it's also slow. vfork() might have made the performance - // better, but it's not fully clear what's safe between vfork() and exec() - // either, so we just use posix_spawn for safety and convenience. - -#if defined(__APPLE__) - // Darwin can change the child directory without a helper executable. - std::string subprocess_helper = argv_pointers.front(); - std::vector helper_argv_pointers = argv_pointers; -#else - // Since we may need the child to have a different working directory (per - // `cwd`), and posix_spawn does not (yet) have support for a chdir action, we - // use a helper binary that chdir's to its first argument, then invokes - // "execvp" with the remaining arguments to replace itself with the command we - // actually wanted to run. - - // To avoid having dependencies on the bazel build artifacts continuing to - // exist we run subprocess_helper out of a memfd. - static const absl::StatusOr subprocess_helper_fd = - GetSubprocessHelperFd(); - XLS_RETURN_IF_ERROR(subprocess_helper_fd.status()); - int fd = *subprocess_helper_fd; - - std::string subprocess_helper = absl::StrCat("/proc/self/fd/", fd); - std::vector helper_argv_pointers; - helper_argv_pointers.reserve(argv_pointers.size() + 2); - helper_argv_pointers.push_back(subprocess_helper.c_str()); - helper_argv_pointers.push_back(cwd.has_value() ? cwd->c_str() : ""); - helper_argv_pointers.insert(helper_argv_pointers.end(), argv_pointers.begin(), - argv_pointers.end()); - -#endif - - XLS_ASSIGN_OR_RETURN(posix_spawn_file_actions_t file_actions, - CreateChildFileActions(stdout_pipe, stderr_pipe)); -#if defined(__APPLE__) - if (cwd.has_value()) { - if (int err = AddChdirFileAction(&file_actions, cwd->c_str()); err != 0) { - posix_spawn_file_actions_destroy(&file_actions); - return absl::InternalError(absl::StrCat( - "Cannot add child working directory action: ", Strerror(err))); - } + posix_spawn_file_actions_t file_actions; + if (int err = posix_spawn_file_actions_init(&file_actions); err != 0) { + return absl::InternalError( + absl::StrCat("Cannot initialize file actions: ", Strerror(err))); } -#endif + absl::Cleanup destroy_file_actions = [&] { + if (int err = posix_spawn_file_actions_destroy(&file_actions); err != 0) { + // Once spawned, the caller must receive the PID so it can reap the child. + LOG(ERROR) << "Cannot destroy file actions: " << Strerror(err); + } + }; + XLS_RETURN_IF_ERROR( + CreateChildFileActions(file_actions, stdout_pipe, stderr_pipe)); - // posix_spawnp takes a null-terminate array of char* for environment + // posix_spawn takes a null-terminated array of char* for environment // variables. Each element has the form "NAME=VALUE". std::vector env_vars; std::vector env_var_ptrs; @@ -266,19 +162,10 @@ absl::StatusOr ExecInChildProcess( env_var_ptrs.push_back(nullptr); child_env = env_var_ptrs.data(); } - pid_t pid; - if (int err = posix_spawnp( - &pid, subprocess_helper.c_str(), &file_actions, nullptr, - const_cast(helper_argv_pointers.data()), child_env); - err != 0) { - return absl::InternalError( - absl::StrCat("Cannot spawn child process: ", Strerror(err))); - } + XLS_ASSIGN_OR_RETURN( + pid_t pid, + internal::SpawnSubprocess(argv_pointers, cwd, &file_actions, child_env)); - if (int err = posix_spawn_file_actions_destroy(&file_actions); err != 0) { - return absl::InternalError( - absl::StrCat("Cannot destroy file actions: ", Strerror(err))); - } stdout_pipe.entrance.Close(); stderr_pipe.entrance.Close(); return pid; diff --git a/xls/common/subprocess_for_os.h b/xls/common/subprocess_for_os.h new file mode 100644 index 0000000000..9cb51c1e81 --- /dev/null +++ b/xls/common/subprocess_for_os.h @@ -0,0 +1,45 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef XLS_COMMON_SUBPROCESS_FOR_OS_H_ +#define XLS_COMMON_SUBPROCESS_FOR_OS_H_ + +#include +#include + +#include +#include + +#include "absl/status/statusor.h" +#include "absl/types/span.h" + +namespace xls::internal { + +// Starts a child with the given arguments, environment, and optional working +// directory. argv must contain a non-null command followed by a null-terminated +// argument list; envp must also be null-terminated. Bare command names are +// looked up using PATH in envp, relative to the child's working directory. +// +// file_actions must be initialized by the caller, which retains ownership. The +// implementation may append actions. On success, the caller must reap the PID. +// Uses posix_spawn rather than fork/exec so the parent can safely be +// multithreaded. +absl::StatusOr SpawnSubprocess( + absl::Span argv, + const std::optional& cwd, + posix_spawn_file_actions_t* file_actions, char* const* envp); + +} // namespace xls::internal + +#endif // XLS_COMMON_SUBPROCESS_FOR_OS_H_ diff --git a/xls/common/subprocess_for_os_linux.cc b/xls/common/subprocess_for_os_linux.cc new file mode 100644 index 0000000000..910535433c --- /dev/null +++ b/xls/common/subprocess_for_os_linux.cc @@ -0,0 +1,92 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" +#include "absl/types/span.h" +#include "xls/common/file/file_descriptor.h" +#include "xls/common/status/status_macros.h" +#include "xls/common/strerror.h" +#include "xls/common/subprocess_for_os.h" +#include "xls/common/subprocess_helper_embedded_embedded.h" + +namespace xls::internal { +namespace { + +absl::StatusOr GetSubprocessHelperFd() { + FileDescriptor fd(memfd_create("subprocess_helper", MFD_CLOEXEC)); + if (fd.get() == -1) { + return absl::InternalError(absl::StrCat( + "Failed to create memfd for subprocess helper: ", Strerror(errno))); + } + if (write(fd.get(), get_subprocess_helper_embedded().data(), + get_subprocess_helper_embedded().size()) != + get_subprocess_helper_embedded().size()) { + return absl::InternalError(absl::StrCat( + "Failed to write subprocess helper to memfd: ", Strerror(errno))); + } + if (lseek(fd.get(), 0, SEEK_SET) != 0) { + return absl::InternalError(absl::StrCat( + "Failed to seek subprocess helper in memfd: ", Strerror(errno))); + } + return std::move(fd); +} + +} // namespace + +absl::StatusOr SpawnSubprocess( + absl::Span argv, + const std::optional& cwd, + posix_spawn_file_actions_t* file_actions, char* const* envp) { + // The helper changes directory and calls execvp, supporting systems whose + // libc lacks a posix_spawn chdir action. Run it out of a memfd so + // subprocesses do not depend on Bazel build artifacts remaining on disk. + static const absl::StatusOr subprocess_helper_fd = + GetSubprocessHelperFd(); + XLS_RETURN_IF_ERROR(subprocess_helper_fd.status()); + std::string subprocess_helper = + absl::StrCat("/proc/self/fd/", subprocess_helper_fd->get()); + std::vector helper_argv; + helper_argv.reserve(argv.size() + 2); + helper_argv.push_back(subprocess_helper.c_str()); + helper_argv.push_back(cwd.has_value() ? cwd->c_str() : ""); + helper_argv.insert(helper_argv.end(), argv.begin(), argv.end()); + + pid_t pid; + if (int err = + posix_spawn(&pid, subprocess_helper.c_str(), file_actions, nullptr, + const_cast(helper_argv.data()), envp); + err != 0) { + return absl::InternalError( + absl::StrCat("Cannot spawn child process: ", Strerror(err))); + } + return pid; +} + +} // namespace xls::internal diff --git a/xls/common/subprocess_for_os_macos.cc b/xls/common/subprocess_for_os_macos.cc new file mode 100644 index 0000000000..a175cc39e4 --- /dev/null +++ b/xls/common/subprocess_for_os_macos.cc @@ -0,0 +1,168 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/strip.h" +#include "absl/types/span.h" +#include "xls/common/strerror.h" +#include "xls/common/subprocess_for_os.h" + +namespace xls::internal { +namespace { + +// macOS 26 introduced the standard addchdir spelling and deprecated _np, which +// has been available since macOS 10.15. SDK availability and the deployment +// target are separate: a new SDK can build a binary for an older runtime. +// Both APIs return zero on success or an error number directly. +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd) { +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 + // The minimum deployment target guarantees that the new API exists. + return posix_spawn_file_actions_addchdir(file_actions, cwd); +#else +// An older SDK lacks the new declaration, even inside a runtime check. +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_26_0 + // __builtin_available is Clang's C/C++ runtime availability check. For older + // deployment targets, the SDK annotation makes the new function a weak + // import; this check prevents calling it on an OS that lacks it. The required + // '*' covers unlisted platforms; Bazel selects this file only for macOS. + // https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available + if (__builtin_available(macOS 26.0, *)) { + return posix_spawn_file_actions_addchdir(file_actions, cwd); + } +#endif + // Use the macOS 10.15 API with older SDKs or on pre-26 runtimes. + return posix_spawn_file_actions_addchdir_np(file_actions, cwd); +#endif +} + +// Returns zero with *pid set on success, or a POSIX error number on failure. +int SpawnExecutable(pid_t* pid, const char* executable, + absl::Span argv, + const posix_spawn_file_actions_t* file_actions, + char* const* envp) { + int err; + // posix_spawn reports the error number in its return value; errno need not + // match. Defensively retry EINTR (an interrupted operation). + do { + err = posix_spawn(pid, executable, file_actions, nullptr, + const_cast(argv.data()), envp); + } while (err == EINTR); + // ENOEXEC means the executable format was not recognized. Only that error + // selects the shell fallback; preserve success and all other errors for the + // caller, which may try another PATH candidate. + if (err != ENOEXEC) { + return err; + } + + // Match execvp in the Linux helper: executable text without a shebang is + // interpreted by the shell. posix_spawn does not provide this fallback. + std::vector shell_argv = {"/bin/sh", executable}; + shell_argv.insert(shell_argv.end(), argv.begin() + 1, argv.end()); + // Apply the same interrupted-operation retry when spawning the shell. + do { + err = posix_spawn(pid, "/bin/sh", file_actions, nullptr, + const_cast(shell_argv.data()), envp); + } while (err == EINTR); + return err; +} + +int SpawnWithPathSearch(pid_t* pid, absl::Span argv, + const posix_spawn_file_actions_t* file_actions, + char* const* envp) { + std::string_view executable = argv.front(); + if (executable.empty()) { + // An empty command cannot name an executable; report "not found" without + // constructing PATH candidates that would name directories instead. + return ENOENT; + } + if (absl::StrContains(executable, '/')) { + return SpawnExecutable(pid, argv.front(), argv, file_actions, envp); + } + + std::string_view path = _PATH_DEFPATH; + for (char* const* entry = envp; *entry != nullptr; ++entry) { + std::string_view variable = *entry; + if (absl::ConsumePrefix(&variable, "PATH=")) { + path = variable; + break; + } + } + + // Darwin's posix_spawnp searches the parent's PATH and can return ENOENT even + // after launching a child when a chdir action and relative PATH are combined. + // Search the child's PATH explicitly with posix_spawn instead. Relative and + // empty entries must resolve after the child's chdir, not in the parent. + bool saw_eacces = false; + // StrSplit preserves empty entries, including an entirely empty PATH. + for (std::string_view directory : absl::StrSplit(path, ':')) { + std::string candidate = directory.empty() + ? absl::StrCat("./", executable) + : absl::StrCat(directory, "/", executable); + int err = SpawnExecutable(pid, candidate.c_str(), argv, file_actions, envp); + if (err == EACCES) { + // Permission was denied for this candidate. A later PATH entry may still + // work; remember this error so it takes precedence if the search fails. + saw_eacces = true; + } else if (err != ENOENT && err != ENOTDIR) { + // Zero means a child was launched; other errors here end the search. + // ENOENT (a missing file or path component) and ENOTDIR (a non-directory + // path component) instead allow us to try the next PATH entry. + return err; + } + } + // Match execvp: report permission denied if any candidate was inaccessible, + // otherwise report that no executable was found. + return saw_eacces ? EACCES : ENOENT; +} + +} // namespace + +absl::StatusOr SpawnSubprocess( + absl::Span argv, + const std::optional& cwd, + posix_spawn_file_actions_t* file_actions, char* const* envp) { + // Darwin can change the child directory without a helper executable. + if (cwd.has_value()) { + // This records the action; the directory change happens when spawning. + // A nonzero result here means the action could not be added. + if (int err = AddChdirFileAction(file_actions, cwd->c_str()); err != 0) { + return absl::InternalError(absl::StrCat( + "Cannot add child working directory action: ", Strerror(err))); + } + } + pid_t pid; + if (int err = SpawnWithPathSearch(&pid, argv, file_actions, envp); err != 0) { + return absl::InternalError( + absl::StrCat("Cannot spawn child process: ", Strerror(err))); + } + return pid; +} + +} // namespace xls::internal diff --git a/xls/common/subprocess_for_os_test.cc b/xls/common/subprocess_for_os_test.cc new file mode 100644 index 0000000000..b7f9b19f6a --- /dev/null +++ b/xls/common/subprocess_for_os_test.cc @@ -0,0 +1,185 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "xls/common/subprocess_for_os.h" + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "absl/cleanup/cleanup.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" +#include "absl/types/span.h" +#include "gtest/gtest.h" +#include "xls/common/file/filesystem.h" +#include "xls/common/file/temp_directory.h" +#include "xls/common/status/matchers.h" + +namespace xls { +namespace { + +// Creates a command with controlled permissions. A non-executable script lets +// the PATH-search tests check that an inaccessible candidate is skipped. +absl::Status WriteScript(const std::filesystem::path& path, + std::string_view contents, bool executable = true) { + absl::Status status = SetFileContents(path, contents); + if (!status.ok()) { + return status; + } + std::error_code error; + std::filesystem::permissions(path, + executable + ? std::filesystem::perms::owner_all + : std::filesystem::perms::owner_read | + std::filesystem::perms::owner_write, + std::filesystem::perm_options::replace, error); + if (error) { + return absl::InternalError(error.message()); + } + return absl::OkStatus(); +} + +// Exercises the OS launcher directly with a chosen child PATH and returns its +// normal exit code, including nonzero codes. Scripts use distinctive exit codes +// so assertions can identify which PATH candidate ran. +absl::StatusOr SpawnAndWait(std::initializer_list arguments, + const std::filesystem::path& cwd, + std::string path) { + // posix_spawn requires null-terminated argv and envp arrays. Keep their + // backing storage alive until SpawnSubprocess returns. + std::vector argv(arguments); + argv.push_back(nullptr); + + // Build a minimal child environment with the requested PATH, independent of + // the test runner's PATH. Keeping it in envp also avoids changing the + // parent's process-wide environment, which other threads may use. + std::string path_variable = absl::StrCat("PATH=", path); + char* envp[] = {path_variable.data(), nullptr}; + + // These tests observe the exit code, so file actions start empty. The + // launcher may append a child chdir action. The caller owns and destroys the + // actions even if spawning fails. + posix_spawn_file_actions_t file_actions; + if (posix_spawn_file_actions_init(&file_actions) != 0) { + return absl::InternalError("Failed to initialize spawn file actions"); + } + auto destroy_actions = absl::MakeCleanup( + [&] { EXPECT_EQ(posix_spawn_file_actions_destroy(&file_actions), 0); }); + + absl::StatusOr pid = internal::SpawnSubprocess( + absl::MakeConstSpan(argv), cwd, &file_actions, envp); + if (!pid.ok()) { + return pid.status(); + } + + // A successful spawn transfers responsibility for reaping the child to us. + // Retry interrupted waits so a signal cannot leave the child unreaped. + int wait_status; + pid_t waited; + do { + waited = waitpid(*pid, &wait_status, 0); + } while (waited == -1 && errno == EINTR); + if (waited != *pid) { + return absl::InternalError("Failed to wait for spawned process"); + } + if (!WIFEXITED(wait_status)) { + return absl::InternalError("Spawned process did not exit normally"); + } + return WEXITSTATUS(wait_status); +} + +TEST(SubprocessForOsTest, DotAndEmptyPathEntriesUseChildWorkingDirectory) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + XLS_ASSERT_OK( + WriteScript(directory.path() / "command", "#!/bin/sh\nexit 23\n")); + + // Both "." and an empty PATH component mean the child's working directory. + // This exercises the relative-PATH case that breaks Darwin's posix_spawnp + // when combined with a chdir action. + for (const std::string& path : {".", ":missing"}) { + SCOPED_TRACE(path); + XLS_ASSERT_OK_AND_ASSIGN(int exit_status, + SpawnAndWait({"command"}, directory.path(), path)); + EXPECT_EQ(exit_status, 23); + } +} + +TEST(SubprocessForOsTest, SearchesRelativePathEntriesFromChildDirectory) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + XLS_ASSERT_OK(RecursivelyCreateDir(directory.path() / "tools")); + XLS_ASSERT_OK(WriteScript(directory.path() / "tools" / "command", + "#!/bin/sh\nexit 29\n")); + + XLS_ASSERT_OK_AND_ASSIGN( + int exit_status, + SpawnAndWait({"command"}, directory.path(), "missing:tools")); + EXPECT_EQ(exit_status, 29); +} + +TEST(SubprocessForOsTest, SearchesPastInaccessiblePathEntry) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + XLS_ASSERT_OK(RecursivelyCreateDir(directory.path() / "blocked")); + XLS_ASSERT_OK(RecursivelyCreateDir(directory.path() / "tools")); + XLS_ASSERT_OK(WriteScript(directory.path() / "blocked" / "command", + "#!/bin/sh\nexit 31\n", /*executable=*/false)); + XLS_ASSERT_OK(WriteScript(directory.path() / "tools" / "command", + "#!/bin/sh\nexit 37\n")); + + XLS_ASSERT_OK_AND_ASSIGN( + int exit_status, + SpawnAndWait({"command"}, directory.path(), "blocked:tools")); + EXPECT_EQ(exit_status, 37); +} + +TEST(SubprocessForOsTest, SlashInCommandBypassesPathSearch) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + XLS_ASSERT_OK(RecursivelyCreateDir(directory.path() / "tools")); + XLS_ASSERT_OK(RecursivelyCreateDir(directory.path() / "on_path")); + XLS_ASSERT_OK(WriteScript(directory.path() / "tools" / "command", + "#!/bin/sh\nexit 41\n")); + XLS_ASSERT_OK(WriteScript(directory.path() / "on_path" / "command", + "#!/bin/sh\nexit 43\n")); + + XLS_ASSERT_OK_AND_ASSIGN( + int exit_status, + SpawnAndWait({"./tools/command"}, directory.path(), "on_path")); + EXPECT_EQ(exit_status, 41); +} + +TEST(SubprocessForOsTest, ExecutableTextWithoutShebangUsesShell) { + XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); + // execvp in the Linux helper falls back to /bin/sh for executable text + // without a shebang. The macOS launcher must provide that fallback and + // preserve args. + XLS_ASSERT_OK(WriteScript(directory.path() / "command", + "test \"$1\" = expected || exit 47\nexit 53\n")); + + XLS_ASSERT_OK_AND_ASSIGN( + int exit_status, + SpawnAndWait({"command", "expected"}, directory.path(), ".")); + EXPECT_EQ(exit_status, 53); +} + +} // namespace +} // namespace xls From 4a8e33ef93ecb04fb1f724c8123913198f4b1c47 Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Fri, 11 Sep 2026 15:59:09 -0700 Subject: [PATCH 3/4] Generalize subprocess launchers by chdir support Rename the launchers to subprocess_posix and subprocess_with_wrapper. Let Linux toolchains opt into direct spawning with --//xls/common:use_posix_spawn_chdir, while retaining the wrapper default for older libc versions and omitting helper dependencies in direct builds. Use the POSIX.1-2024 addchdir API when _POSIX_VERSION is at least 202405L, and support glibc's extension from 2.29 onward. Keep Darwin's SDK and runtime availability adapter for older macOS deployment targets. --- xls/common/BUILD | 34 ++++++++++++++++--- xls/common/subprocess_for_os_test.cc | 4 +-- ...ss_for_os_macos.cc => subprocess_posix.cc} | 32 +++++++++++++---- ...os_linux.cc => subprocess_with_wrapper.cc} | 4 +-- 4 files changed, 59 insertions(+), 15 deletions(-) rename xls/common/{subprocess_for_os_macos.cc => subprocess_posix.cc} (82%) rename xls/common/{subprocess_for_os_linux.cc => subprocess_with_wrapper.cc} (95%) diff --git a/xls/common/BUILD b/xls/common/BUILD index 909c0ea214..30bb1da335 100644 --- a/xls/common/BUILD +++ b/xls/common/BUILD @@ -15,6 +15,7 @@ # Common utilities shared among XLA subfolders. load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") load("@protobuf//bazel:cc_proto_library.bzl", "cc_proto_library") load("@protobuf//bazel:proto_library.bzl", "proto_library") load("@rules_cc//cc:cc_binary.bzl", "cc_binary") @@ -288,18 +289,40 @@ xls_cc_embed_data( data = ":subprocess_helper", ) -# Select the launcher using the target OS (including Bazel's native macOS -# platform), so macOS does not build or embed the Linux helper executable. +# macOS always uses its native chdir action, including on older runtimes through +# the Darwin availability adapter in subprocess_posix.cc. config_setting( name = "macos", constraint_values = ["@platforms//os:osx"], ) +# Enable direct spawning for toolchains with a POSIX.1-2024 chdir action, or +# glibc >= 2.29 with _GNU_SOURCE enabled. Keep the wrapper as the Linux default +# until the minimum supported libc provides the action. This setting also keeps +# the embedded helper out of the dependency graph when direct spawning is used: +# --//xls/common:use_posix_spawn_chdir +bool_flag( + name = "use_posix_spawn_chdir", + build_setting_default = False, +) + +config_setting( + name = "posix_spawn_chdir_enabled", + flag_values = {":use_posix_spawn_chdir": "true"}, +) + +config_setting( + name = "subprocess_wrapper_enabled", + constraint_values = ["@platforms//os:linux"], + flag_values = {":use_posix_spawn_chdir": "false"}, +) + cc_library( name = "subprocess_for_os", srcs = select({ - ":macos": ["subprocess_for_os_macos.cc"], - "@platforms//os:linux": ["subprocess_for_os_linux.cc"], + ":macos": ["subprocess_posix.cc"], + ":posix_spawn_chdir_enabled": ["subprocess_posix.cc"], + ":subprocess_wrapper_enabled": ["subprocess_with_wrapper.cc"], }), hdrs = ["subprocess_for_os.h"], deps = [ @@ -310,7 +333,8 @@ cc_library( "@abseil-cpp//absl/types:span", ] + select({ ":macos": [], - "@platforms//os:linux": [ + ":posix_spawn_chdir_enabled": [], + ":subprocess_wrapper_enabled": [ ":subprocess_helper_embedded", "//xls/common/file:file_descriptor", "//xls/common/status:status_macros", diff --git a/xls/common/subprocess_for_os_test.cc b/xls/common/subprocess_for_os_test.cc index b7f9b19f6a..bd42878167 100644 --- a/xls/common/subprocess_for_os_test.cc +++ b/xls/common/subprocess_for_os_test.cc @@ -169,8 +169,8 @@ TEST(SubprocessForOsTest, SlashInCommandBypassesPathSearch) { TEST(SubprocessForOsTest, ExecutableTextWithoutShebangUsesShell) { XLS_ASSERT_OK_AND_ASSIGN(TempDirectory directory, TempDirectory::Create()); - // execvp in the Linux helper falls back to /bin/sh for executable text - // without a shebang. The macOS launcher must provide that fallback and + // execvp in the wrapper falls back to /bin/sh for executable text + // without a shebang. The POSIX launcher must provide that fallback and // preserve args. XLS_ASSERT_OK(WriteScript(directory.path() / "command", "test \"$1\" = expected || exit 47\nexit 53\n")); diff --git a/xls/common/subprocess_for_os_macos.cc b/xls/common/subprocess_posix.cc similarity index 82% rename from xls/common/subprocess_for_os_macos.cc rename to xls/common/subprocess_posix.cc index a175cc39e4..ec7b3b80b7 100644 --- a/xls/common/subprocess_for_os_macos.cc +++ b/xls/common/subprocess_posix.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -36,12 +37,16 @@ namespace xls::internal { namespace { -// macOS 26 introduced the standard addchdir spelling and deprecated _np, which -// has been available since macOS 10.15. SDK availability and the deployment -// target are separate: a new SDK can build a binary for an older runtime. +// Use the standard POSIX.1-2024 chdir action where available. Some libcs expose +// the action as an extension without advertising full POSIX.1-2024 support. // Both APIs return zero on success or an error number directly. int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, const char* cwd) { +#if defined(__APPLE__) + // Darwin still advertises POSIX.1-2001. macOS 26 introduced the standard + // spelling and deprecated _np, which has been available since macOS 10.15. + // SDK availability and the deployment target are separate: a new SDK can + // build a binary for an older runtime. #if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 // The minimum deployment target guarantees that the new API exists. return posix_spawn_file_actions_addchdir(file_actions, cwd); @@ -51,7 +56,7 @@ int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, // __builtin_available is Clang's C/C++ runtime availability check. For older // deployment targets, the SDK annotation makes the new function a weak // import; this check prevents calling it on an OS that lacks it. The required - // '*' covers unlisted platforms; Bazel selects this file only for macOS. + // '*' covers unlisted platforms; this adapter branch is Darwin-only. // https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available if (__builtin_available(macOS 26.0, *)) { return posix_spawn_file_actions_addchdir(file_actions, cwd); @@ -60,6 +65,21 @@ int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, // Use the macOS 10.15 API with older SDKs or on pre-26 runtimes. return posix_spawn_file_actions_addchdir_np(file_actions, cwd); #endif +#elif _POSIX_VERSION >= 202405L + // POSIX.1-2024 specifies 202405L for _POSIX_VERSION in . + // https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/unistd.h.html + return posix_spawn_file_actions_addchdir(file_actions, cwd); +#elif defined(__GLIBC__) +#if __GLIBC_PREREQ(2, 29) && defined(_GNU_SOURCE) + // glibc has provided the GNU extension since 2.29, even when _POSIX_VERSION + // still reports POSIX.1-2008. C++ toolchains normally enable _GNU_SOURCE. + return posix_spawn_file_actions_addchdir_np(file_actions, cwd); +#else +#error "Use the wrapper: direct spawning needs glibc >= 2.29 and _GNU_SOURCE." +#endif +#else +#error "Direct spawning needs POSIX.1-2024 or a supported chdir extension." +#endif } // Returns zero with *pid set on success, or a POSIX error number on failure. @@ -81,7 +101,7 @@ int SpawnExecutable(pid_t* pid, const char* executable, return err; } - // Match execvp in the Linux helper: executable text without a shebang is + // Match execvp in the wrapper: executable text without a shebang is // interpreted by the shell. posix_spawn does not provide this fallback. std::vector shell_argv = {"/bin/sh", executable}; shell_argv.insert(shell_argv.end(), argv.begin() + 1, argv.end()); @@ -148,7 +168,7 @@ absl::StatusOr SpawnSubprocess( absl::Span argv, const std::optional& cwd, posix_spawn_file_actions_t* file_actions, char* const* envp) { - // Darwin can change the child directory without a helper executable. + // Change the child directory without a helper executable. if (cwd.has_value()) { // This records the action; the directory change happens when spawning. // A nonzero result here means the action could not be added. diff --git a/xls/common/subprocess_for_os_linux.cc b/xls/common/subprocess_with_wrapper.cc similarity index 95% rename from xls/common/subprocess_for_os_linux.cc rename to xls/common/subprocess_with_wrapper.cc index 910535433c..720a25252c 100644 --- a/xls/common/subprocess_for_os_linux.cc +++ b/xls/common/subprocess_with_wrapper.cc @@ -64,8 +64,8 @@ absl::StatusOr SpawnSubprocess( absl::Span argv, const std::optional& cwd, posix_spawn_file_actions_t* file_actions, char* const* envp) { - // The helper changes directory and calls execvp, supporting systems whose - // libc lacks a posix_spawn chdir action. Run it out of a memfd so + // The helper changes directory and calls execvp, supporting Linux toolchains + // whose libc lacks a posix_spawn chdir action. Run it out of a memfd so // subprocesses do not depend on Bazel build artifacts remaining on disk. static const absl::StatusOr subprocess_helper_fd = GetSubprocessHelperFd(); From 7149d7ad6899bd53bf51efe997e36438ae07449b Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Fri, 11 Sep 2026 16:07:56 -0700 Subject: [PATCH 4/4] Select subprocess chdir adapters in Bazel Move chdir API selection out of subprocess_posix.cc into separate POSIX, Darwin, and glibc adapters. The shared spawning algorithm has no platform conditionals; only the Darwin adapter retains SDK/runtime availability checks. Replace the boolean setting with subprocess_chdir=auto|posix|glibc so the build selects one API explicitly. Auto preserves the macOS native launcher and the older-libc Linux wrapper. The glibc adapter enables _GNU_SOURCE locally, and direct builds still omit the embedded helper dependency. --- xls/common/BUILD | 68 +++++++++++++++++++-------- xls/common/subprocess_chdir.h | 30 ++++++++++++ xls/common/subprocess_chdir_darwin.cc | 47 ++++++++++++++++++ xls/common/subprocess_chdir_glibc.cc | 28 +++++++++++ xls/common/subprocess_chdir_posix.cc | 28 +++++++++++ xls/common/subprocess_posix.cc | 47 +----------------- 6 files changed, 182 insertions(+), 66 deletions(-) create mode 100644 xls/common/subprocess_chdir.h create mode 100644 xls/common/subprocess_chdir_darwin.cc create mode 100644 xls/common/subprocess_chdir_glibc.cc create mode 100644 xls/common/subprocess_chdir_posix.cc diff --git a/xls/common/BUILD b/xls/common/BUILD index 30bb1da335..0859ecf5f6 100644 --- a/xls/common/BUILD +++ b/xls/common/BUILD @@ -15,7 +15,7 @@ # Common utilities shared among XLA subfolders. load("@bazel_skylib//:bzl_library.bzl", "bzl_library") -load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") +load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("@protobuf//bazel:cc_proto_library.bzl", "cc_proto_library") load("@protobuf//bazel:proto_library.bzl", "proto_library") load("@rules_cc//cc:cc_binary.bzl", "cc_binary") @@ -289,42 +289,69 @@ xls_cc_embed_data( data = ":subprocess_helper", ) -# macOS always uses its native chdir action, including on older runtimes through -# the Darwin availability adapter in subprocess_posix.cc. +# Select a chdir API supported by the toolchain and deployment target. "auto" +# uses Darwin's availability adapter on macOS and the wrapper on Linux, keeping +# compatibility with older libc versions. Explicit choices require the standard +# POSIX.1-2024 action ("posix") or glibc >= 2.29 ("glibc"). For example: +# --//xls/common:subprocess_chdir=glibc +string_flag( + name = "subprocess_chdir", + build_setting_default = "auto", + values = [ + "auto", + "posix", + "glibc", + ], +) + config_setting( - name = "macos", + name = "subprocess_chdir_darwin", constraint_values = ["@platforms//os:osx"], + flag_values = {":subprocess_chdir": "auto"}, ) -# Enable direct spawning for toolchains with a POSIX.1-2024 chdir action, or -# glibc >= 2.29 with _GNU_SOURCE enabled. Keep the wrapper as the Linux default -# until the minimum supported libc provides the action. This setting also keeps -# the embedded helper out of the dependency graph when direct spawning is used: -# --//xls/common:use_posix_spawn_chdir -bool_flag( - name = "use_posix_spawn_chdir", - build_setting_default = False, +config_setting( + name = "subprocess_chdir_posix", + flag_values = {":subprocess_chdir": "posix"}, ) config_setting( - name = "posix_spawn_chdir_enabled", - flag_values = {":use_posix_spawn_chdir": "true"}, + name = "subprocess_chdir_glibc", + constraint_values = ["@platforms//os:linux"], + flag_values = {":subprocess_chdir": "glibc"}, ) config_setting( name = "subprocess_wrapper_enabled", constraint_values = ["@platforms//os:linux"], - flag_values = {":use_posix_spawn_chdir": "false"}, + flag_values = {":subprocess_chdir": "auto"}, ) cc_library( name = "subprocess_for_os", srcs = select({ - ":macos": ["subprocess_posix.cc"], - ":posix_spawn_chdir_enabled": ["subprocess_posix.cc"], + ":subprocess_chdir_darwin": [ + "subprocess_chdir_darwin.cc", + "subprocess_posix.cc", + ], + ":subprocess_chdir_posix": [ + "subprocess_chdir_posix.cc", + "subprocess_posix.cc", + ], + ":subprocess_chdir_glibc": [ + "subprocess_chdir_glibc.cc", + "subprocess_posix.cc", + ], ":subprocess_wrapper_enabled": ["subprocess_with_wrapper.cc"], }), - hdrs = ["subprocess_for_os.h"], + hdrs = [ + "subprocess_chdir.h", + "subprocess_for_os.h", + ], + local_defines = select({ + ":subprocess_chdir_glibc": ["_GNU_SOURCE"], + "//conditions:default": [], + }), deps = [ ":strerror", "@abseil-cpp//absl/status", @@ -332,8 +359,9 @@ cc_library( "@abseil-cpp//absl/strings", "@abseil-cpp//absl/types:span", ] + select({ - ":macos": [], - ":posix_spawn_chdir_enabled": [], + ":subprocess_chdir_darwin": [], + ":subprocess_chdir_posix": [], + ":subprocess_chdir_glibc": [], ":subprocess_wrapper_enabled": [ ":subprocess_helper_embedded", "//xls/common/file:file_descriptor", diff --git a/xls/common/subprocess_chdir.h b/xls/common/subprocess_chdir.h new file mode 100644 index 0000000000..8920a52a5f --- /dev/null +++ b/xls/common/subprocess_chdir.h @@ -0,0 +1,30 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef XLS_COMMON_SUBPROCESS_CHDIR_H_ +#define XLS_COMMON_SUBPROCESS_CHDIR_H_ + +#include + +namespace xls::internal { + +// Records a child working-directory action. Returns zero on success or a POSIX +// error number directly; errno need not match. Bazel selects the implementation +// for the configured libc API and deployment target. +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd); + +} // namespace xls::internal + +#endif // XLS_COMMON_SUBPROCESS_CHDIR_H_ diff --git a/xls/common/subprocess_chdir_darwin.cc b/xls/common/subprocess_chdir_darwin.cc new file mode 100644 index 0000000000..7f3a504181 --- /dev/null +++ b/xls/common/subprocess_chdir_darwin.cc @@ -0,0 +1,47 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "xls/common/subprocess_chdir.h" + +namespace xls::internal { + +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd) { + // Darwin still advertises POSIX.1-2001. macOS 26 introduced the standard + // spelling and deprecated _np, which has been available since macOS 10.15. + // SDK availability and the deployment target are separate: a new SDK can + // build a binary for an older runtime. +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 + // The minimum deployment target guarantees that the new API exists. + return posix_spawn_file_actions_addchdir(file_actions, cwd); +#else +// An older SDK lacks the new declaration, even inside a runtime check. +#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_26_0 + // __builtin_available is Clang's C/C++ runtime availability check. For older + // deployment targets, the SDK annotation makes the new function a weak + // import; this check prevents calling it on an OS that lacks it. The required + // '*' covers unlisted platforms; Bazel selects this file for Darwin. + // https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available + if (__builtin_available(macOS 26.0, *)) { + return posix_spawn_file_actions_addchdir(file_actions, cwd); + } +#endif + // Use the macOS 10.15 API with older SDKs or on pre-26 runtimes. + return posix_spawn_file_actions_addchdir_np(file_actions, cwd); +#endif +} + +} // namespace xls::internal diff --git a/xls/common/subprocess_chdir_glibc.cc b/xls/common/subprocess_chdir_glibc.cc new file mode 100644 index 0000000000..039851433d --- /dev/null +++ b/xls/common/subprocess_chdir_glibc.cc @@ -0,0 +1,28 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "xls/common/subprocess_chdir.h" + +namespace xls::internal { + +// glibc has provided this extension since 2.29. The build enables _GNU_SOURCE +// to expose its declaration, without requiring full POSIX.1-2024 conformance. +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd) { + return posix_spawn_file_actions_addchdir_np(file_actions, cwd); +} + +} // namespace xls::internal diff --git a/xls/common/subprocess_chdir_posix.cc b/xls/common/subprocess_chdir_posix.cc new file mode 100644 index 0000000000..6025421efb --- /dev/null +++ b/xls/common/subprocess_chdir_posix.cc @@ -0,0 +1,28 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "xls/common/subprocess_chdir.h" + +namespace xls::internal { + +// The build selects this adapter when the toolchain and deployment target +// provide the standard POSIX.1-2024 chdir action. +int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, + const char* cwd) { + return posix_spawn_file_actions_addchdir(file_actions, cwd); +} + +} // namespace xls::internal diff --git a/xls/common/subprocess_posix.cc b/xls/common/subprocess_posix.cc index ec7b3b80b7..bf2bd4074c 100644 --- a/xls/common/subprocess_posix.cc +++ b/xls/common/subprocess_posix.cc @@ -15,7 +15,6 @@ #include #include #include -#include #include #include @@ -32,56 +31,12 @@ #include "absl/strings/strip.h" #include "absl/types/span.h" #include "xls/common/strerror.h" +#include "xls/common/subprocess_chdir.h" #include "xls/common/subprocess_for_os.h" namespace xls::internal { namespace { -// Use the standard POSIX.1-2024 chdir action where available. Some libcs expose -// the action as an extension without advertising full POSIX.1-2024 support. -// Both APIs return zero on success or an error number directly. -int AddChdirFileAction(posix_spawn_file_actions_t* file_actions, - const char* cwd) { -#if defined(__APPLE__) - // Darwin still advertises POSIX.1-2001. macOS 26 introduced the standard - // spelling and deprecated _np, which has been available since macOS 10.15. - // SDK availability and the deployment target are separate: a new SDK can - // build a binary for an older runtime. -#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_26_0 - // The minimum deployment target guarantees that the new API exists. - return posix_spawn_file_actions_addchdir(file_actions, cwd); -#else -// An older SDK lacks the new declaration, even inside a runtime check. -#if defined(__MAC_26_0) && __MAC_OS_X_VERSION_MAX_ALLOWED >= __MAC_26_0 - // __builtin_available is Clang's C/C++ runtime availability check. For older - // deployment targets, the SDK annotation makes the new function a weak - // import; this check prevents calling it on an OS that lacks it. The required - // '*' covers unlisted platforms; this adapter branch is Darwin-only. - // https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available - if (__builtin_available(macOS 26.0, *)) { - return posix_spawn_file_actions_addchdir(file_actions, cwd); - } -#endif - // Use the macOS 10.15 API with older SDKs or on pre-26 runtimes. - return posix_spawn_file_actions_addchdir_np(file_actions, cwd); -#endif -#elif _POSIX_VERSION >= 202405L - // POSIX.1-2024 specifies 202405L for _POSIX_VERSION in . - // https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/unistd.h.html - return posix_spawn_file_actions_addchdir(file_actions, cwd); -#elif defined(__GLIBC__) -#if __GLIBC_PREREQ(2, 29) && defined(_GNU_SOURCE) - // glibc has provided the GNU extension since 2.29, even when _POSIX_VERSION - // still reports POSIX.1-2008. C++ toolchains normally enable _GNU_SOURCE. - return posix_spawn_file_actions_addchdir_np(file_actions, cwd); -#else -#error "Use the wrapper: direct spawning needs glibc >= 2.29 and _GNU_SOURCE." -#endif -#else -#error "Direct spawning needs POSIX.1-2024 or a supported chdir extension." -#endif -} - // Returns zero with *pid set on success, or a POSIX error number on failure. int SpawnExecutable(pid_t* pid, const char* executable, absl::Span argv,