Split subprocess launching into Linux and macOS implementations

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", "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")
@@ -288,17 +289,117 @@ xls_cc_embed_data(
     data = ":subprocess_helper",
 )
 
+# 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 = "subprocess_chdir_darwin",
+    constraint_values = ["@platforms//os:osx"],
+    flag_values = {":subprocess_chdir": "auto"},
+)
+
+config_setting(
+    name = "subprocess_chdir_posix",
+    flag_values = {":subprocess_chdir": "posix"},
+)
+
+config_setting(
+    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 = {":subprocess_chdir": "auto"},
+)
+
+cc_library(
+    name = "subprocess_for_os",
+    srcs = select({
+        ":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_chdir.h",
+        "subprocess_for_os.h",
+    ],
+    local_defines = select({
+        ":subprocess_chdir_glibc": ["_GNU_SOURCE"],
+        "//conditions:default": [],
+    }),
+    deps = [
+        ":strerror",
+        "@abseil-cpp//absl/status",
+        "@abseil-cpp//absl/status:statusor",
+        "@abseil-cpp//absl/strings",
+        "@abseil-cpp//absl/types:span",
+    ] + select({
+        ":subprocess_chdir_darwin": [],
+        ":subprocess_chdir_posix": [],
+        ":subprocess_chdir_glibc": [],
+        ":subprocess_wrapper_enabled": [
+            ":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",
@@ -318,6 +419,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",

xls/common/subprocess.cc

@@ -15,11 +15,9 @@
 #include "xls/common/subprocess.h"
 
 #include <fcntl.h>
-#include <linux/memfd.h>
 #include <signal.h>  // NOLINT
 #include <spawn.h>
 #include <stdlib.h>  // NOLINT for WIFEXITED, WEXITSTATUS; not in <cstdlib>
-#include <sys/mman.h>
 #include <sys/poll.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -38,6 +36,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/cleanup/cleanup.h"
 #include "absl/container/fixed_array.h"
 #include "absl/log/check.h"
 #include "absl/log/log.h"
@@ -53,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 {
@@ -107,14 +104,8 @@ absl::Status ReplaceFdWithPipe(posix_spawn_file_actions_t& actions, int fd,
   return absl::OkStatus();
 }
 
-absl::StatusOr<posix_spawn_file_actions_t> 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(
@@ -126,89 +117,29 @@ absl::StatusOr<posix_spawn_file_actions_t> CreateChildFileActions(
   XLS_RETURN_IF_ERROR(
       ReplaceFdWithPipe(actions, STDERR_FILENO, stderr_pipe, "stderr"));
 
-  return actions;
-}
-
-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<CleanableFd> 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);
+  return absl::OkStatus();
 }
 
 absl::StatusOr<pid_t> ExecInChildProcess(
     const std::vector<const char*>& argv_pointers,
     const std::optional<std::filesystem::path>& cwd, Pipe& stdout_pipe,
     Pipe& stderr_pipe,
     absl::Span<const EnvironmentVariable> 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.
-
-  // 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<CleanableFd> 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<const char*> 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());
-
-  XLS_ASSIGN_OR_RETURN(posix_spawn_file_actions_t file_actions,
-                       CreateChildFileActions(stdout_pipe, stderr_pipe));
-
-  // posix_spawnp takes a null-terminate array of char* for environment
+  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)));
+  }
+  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_spawn takes a null-terminated array of char* for environment
   // variables. Each element has the form "NAME=VALUE".
   std::vector<std::string> env_vars;
   std::vector<char*> env_var_ptrs;
@@ -231,19 +162,10 @@ absl::StatusOr<pid_t> 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<char* const*>(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;

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 <spawn.h>
+
+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_

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 <spawn.h>
+
+#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