Make the embedded subprocess_helper work on non-Linux hosts by zarubaf · Pull Request #4864 · google/xls

zarubaf · 2026-08-27T10:17:36Z

xls/common/subprocess.cc does not compile on macOS:

xls/common/subprocess.cc:18:10: fatal error: 'linux/memfd.h' file not found

2c355ce53 ("Embed subprocess_helper in subprocess.cc") replaced the runfiles lookup with an embedded copy executed from a memfd. The mechanism is Linux specific — neither memfd_create() nor /proc/self/fd exists on macOS — and the include is unconditional.

The blast radius is larger than one file. //xls/common:subprocess is reachable from all four binaries that nightly-macos.yml builds (interpreter_main, ir_converter_main, opt_main, codegen_main), so the macOS nightly build cannot succeed while this is present.

Change

Helper creation moves behind a single CreateSubprocessHelper() returning a path to spawn:

Linux keeps the existing memfd behavior. The only difference is that the fd is now released rather than held in a StatusOr — it has to stay open for the process lifetime for /proc/self/fd/N to resolve, which the previous function-local static did implicitly.
Elsewhere the embedded helper is written to a private mkstemp() file under $TMPDIR, marked executable, closed, and unlinked via atexit. Closing before exec matters: macOS returns ETXTBSY for a file still open for writing.

The tradeoff, and a question

The temporary file gives up the "no dependency on build artifacts continuing to exist" property that motivated the memfd approach. I could not find a way to preserve it on macOS: there is no fexecve() and no /proc, so exec needs a real path.

If you would rather not have a temp file on macOS at all, the alternative is falling back to the previous GetXlsRunfilePath() lookup on non-Linux and keeping memfd on Linux. That keeps nothing on disk that was not already on disk, at the cost of reintroducing the runfiles dependency for that platform. Happy to switch if that is preferred — I picked the temp file because it keeps behavior uniform across platforms.

Verification

macOS arm64, -c opt, on top of the minimum-OS and hermetic LLVM SDK changes needed to build there at all:

//xls/common:subprocess fails to compile before, builds after
the four nightly-macos.yml targets fail before, build after
//xls/common:subprocess_test passes — though only with one additional local workaround, noted below

Since main is currently red on an unrelated lockfile issue, this was checked on cf36eb5a3, the last green ancestor.

The subprocess_test caveat: linking any Mach-O shared library currently fails because rules_cc appends a GNU-style -Wl,-soname= that ld64 rejects (ld64.lld: error: unknown argument '-soname=...'). I worked around it locally with --nointerface_shared_objects to run the test. That is a separate problem, not addressed here.

Context: #1434

google-cla · 2026-08-27T10:17:53Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

allight · 2026-09-01T17:48:23Z

+  }
+
+  static std::string* const cleanup_path = new std::string(helper_path);
+  std::atexit([] { unlink(cleanup_path->c_str()); });


I'd much prefer to have static destructors handle this instead of an atexit handler.

Just add a FileName function to the CleanableFD, make it virtual and have the mac version return an override with the unlink in its destructor (and the linux one does the close it currently has). This means the normal static destructors deal with closing the FD on linux and removing the file in mac.

allight · 2026-09-01T17:52:44Z

        "Failed to seek subprocess helper in memfd: ", Strerror(errno)));
  }
-  return std::move(fd);
+  // Every spawn names this fd through /proc/self/fd, so it has to stay open for


We need to make sure that the FD is closed by the static destructors or some other mechanism. Just leaking it is not ok. Especially because we care about the linux build in a way that we (unfortunately) don't for the mac build.

You are right. The memfd is owned by MemfdSubprocessHelper and closed by the inherited ~CleanableFd() when the function-local static is destroyed.

allight · 2026-09-01T18:05:09Z

+
+// Runs the helper straight out of an anonymous in-memory file, so that nothing
+// on disk needs to outlive the build that produced it.
+absl::StatusOr<std::string> CreateSubprocessHelper() {


Instead maybe make CleanableFD a virtual class with a

virtual std::string file_name() const = 0;

and have this (in both versions) return absl::StatusOr<std::unique_ptr>

xls/common/subprocess.cc fails to compile on macOS: fatal error: 'linux/memfd.h' file not found Embedding subprocess_helper and running it from a memfd replaced the previous runfiles lookup, but the mechanism it uses is Linux specific: neither memfd_create() nor /proc/self/fd exists on macOS, and the include is unconditional. This breaks more than the compile. //xls/common:subprocess is reachable from all four binaries built by nightly-macos.yml (interpreter_main, ir_converter_main, opt_main, codegen_main), so the macOS nightly build cannot succeed while this is present. CleanableFd becomes the owner of whatever storage the helper lives in, with a virtual file_name() naming the path to spawn, and CreateSubprocessHelper() hands back a unique_ptr to one of two implementations: - MemfdSubprocessHelper keeps the existing memfd behavior and resolves through /proc/self/fd. The descriptor stays open for as long as the object lives and is closed by the inherited destructor. - TempFileSubprocessHelper writes the helper to a private mkstemp() file under $TMPDIR and unlinks it on destruction. Its descriptor is closed as soon as the file is written, because macOS refuses to exec a file that is still open for writing (ETXTBSY). Both are held in a function-local static, so the static destructor releases the storage on either platform: no leaked descriptor on Linux and no atexit handler on macOS. The temporary file gives up the "nothing on disk has to survive" property of the memfd path. macOS has no fexecve() and no /proc, so exec needs a real path. Verified on macOS arm64 (-c opt), on top of the minimum-OS and hermetic LLVM SDK changes needed to build there at all: - //xls/common:subprocess fails to compile before, builds after - //xls/common:subprocess_test passes - the four nightly-macos.yml targets fail before, build after Context: google#1434

zarubaf · 2026-09-04T10:17:00Z

Thanks for taking a look. Verified locally on my Mac.

allight

Just a few minor changes. Otherwise LGTM

allight · 2026-09-04T15:46:55Z

+  int fd() const { return fd_; }
+
 private:
  int fd_ = -1;


Just have the FD be only in the linux override.

allight · 2026-09-04T15:48:11Z

+    close(fd);
+    return written;
+  }
+  if (fchmod(fd, 0700) != 0) {


0500

No need to have write permission anymore.

allight · 2026-09-04T15:50:38Z

+  return helper;
 }

+#else


#else // for MACOS and other non-linux

zarubaf force-pushed the macos-subprocess-helper-portability branch from 72f703f to 8b5d836 Compare August 27, 2026 10:51

erinzmoore requested a review from allight September 1, 2026 16:43

allight requested changes Sep 1, 2026

View reviewed changes

zarubaf force-pushed the macos-subprocess-helper-portability branch from 8b5d836 to 231a413 Compare September 4, 2026 09:47

zarubaf requested a review from allight September 4, 2026 10:17

allight requested changes Sep 4, 2026

View reviewed changes

cdleary mentioned this pull request Sep 11, 2026

Split subprocess launching into Linux and macOS implementations #4972

Open

Navigation Menu

Uh oh!

Make the embedded subprocess_helper work on non-Linux hosts - #4864

Make the embedded subprocess_helper work on non-Linux hosts#4864
zarubaf wants to merge 1 commit into
google:mainfrom
zarubaf:macos-subprocess-helper-portability

zarubaf commented Aug 27, 2026

Uh oh!

google-cla Bot commented Aug 27, 2026

Uh oh!

allight Sep 1, 2026

Uh oh!

zarubaf Sep 4, 2026

Uh oh!

allight Sep 1, 2026

Uh oh!

zarubaf Sep 4, 2026

Uh oh!

allight Sep 1, 2026

Uh oh!

zarubaf Sep 4, 2026

Uh oh!

zarubaf commented Sep 4, 2026

Uh oh!

allight left a comment

Uh oh!

allight Sep 4, 2026

Uh oh!

allight Sep 4, 2026

Uh oh!

allight Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zarubaf commented Aug 27, 2026

Change

The tradeoff, and a question

Verification

Uh oh!

google-cla Bot commented Aug 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zarubaf commented Sep 4, 2026

Uh oh!

allight left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants