Skip to content

Fix subprocess helper exec on macOS - #4994

Closed
marketchedai wants to merge 1 commit into
google:mainfrom
marketchedai:fix/macos-subprocess-helper
Closed

marketchedai wants to merge 1 commit into
google:mainfrom
marketchedai:fix/macos-subprocess-helper

Conversation

@marketchedai

Copy link
Copy Markdown

Fixes #4991.

xls/common/subprocess.cc cannot compile on macOS: it unconditionally uses
<linux/memfd.h>, memfd_create()/MFD_CLOEXEC, and the /proc/self/fd/<fd>
path passed to posix_spawnp(). macOS has none of these.

This introduces GetSubprocessHelperPath(), which encapsulates producing an
exec'able path for the embedded helper:

  • Linux — behavior unchanged. Still writes the helper to a memfd and
    returns /proc/self/fd/<fd>, holding the descriptor open for the lifetime
    of the process.
  • Apple. Materializes the helper into a temporary file via mkstemp(),
    marks it executable with fchmod(0700), and unlinks it through
    std::atexit() (the error paths unlink it too).

A named file is unavoidable on macOS. Unlike a memfd it is not reclaimed
automatically, and it cannot be unlinked up front because exec resolves it by
path. I checked the alternatives rather than assuming: exec'ing an unlinked
file through /dev/fd/<fd> is rejected with EACCES, and there is no
fexecve(). A process killed by a signal will still leave one behind.

The call site in ExecInChildProcess() is deliberately not platform-guarded —
the whole point is that the platform difference lives in one function.
subprocess_helper is now a reference to a function-local static const,
which has thread-safe initialization, is never mutated, and outlives the
c_str() pointers handed to posix_spawnp().

Testing

Tested on macOS (arm64, macOS 26.6.2) only. The Linux path in this change
has not been compiled anywhere — I do not have a Linux machine available, and
I could not cross-check locally: undefining __APPLE__ to force the other
branch breaks macOS's own libc++ headers, which key off that macro. I am
relying on CI for Linux coverage, and would appreciate a careful look at the
#else branch.

On macOS arm64, with Bazel 8.7.0:

  • //xls/common:subprocess_test passes (12 tests, covering non-zero exits,
    crashing children, large stdout/stderr, and environment variable
    propagation). Note this requires xls_cc_embed_data: shared-library link fails on macOS — ELF-only -soname passed to the Mach-O linker #4992 to be fixed first, otherwise the test
    cannot be built on macOS at all.
  • //xls/common/... passes 36/36.
  • interpreter_main, ir_converter_main, opt_main and codegen_main all
    build, and a DSLX -> IR -> opt -> Verilog run produces correct output.
  • Confirmed no temporary files accumulate: repeated runs leave none behind.

🤖 Generation assisted with Claude Code

subprocess.cc unconditionally used three Linux-only facilities, so the
file has not compiled on macOS since embedding of the subprocess helper
was introduced:

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

  - <linux/memfd.h>
  - memfd_create() / MFD_CLOEXEC
  - the /proc/self/fd/<fd> path handed to posix_spawnp()

macOS has neither memfd_create() nor a /proc filesystem, so the helper
cannot be exec'd out of an anonymous in-memory file descriptor.

Introduce GetSubprocessHelperPath() to encapsulate producing an
exec'able path for the embedded helper. Linux behavior is unchanged:
it still writes the helper to a memfd and returns /proc/self/fd/<fd>,
holding the descriptor open for the lifetime of the process. On Apple
the helper is instead materialized into a temporary file via mkstemp()
and made executable with fchmod(0700); the file is left in place
because it cannot be unlinked before the exec that uses it.

Unlike a Linux memfd, the temporary file has a name and is not reclaimed
automatically, so it is unlinked via std::atexit(); the error paths unlink
it too. A process killed by a signal still leaves one behind. There is no
way to avoid a named file on macOS: exec'ing an unlinked file through
/dev/fd is rejected with EACCES, and fexecve() does not exist.

Verified with //xls/common:subprocess_test (12 tests, covering
non-zero exits, crashing children, large stdout/stderr, and environment
variable propagation), which passes on macOS arm64.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@google-cla

google-cla Bot commented Sep 16, 2026

Copy link
Copy Markdown

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

allight commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

We are already reviewing #4972 for this.

@marketchedai

Copy link
Copy Markdown
Author

Thanks @allight — agreed, #4972 supersedes this, and it is the better fix: a
proper per-OS split selected by Bazel rather than more #ifdefs inside one
file. Closing in favour of it.

For anyone finding this later: #4972 merged as 2257034 and removes the
memfd_create() / /proc/self/fd dependency from subprocess.cc entirely.

Note that #4993 is a separate problem and is still outstanding on current
main: xls_cc_embed_data still builds a shared library, which fails to link
on macOS because the toolchain passes the ELF-only -soname to the Mach-O
linker. That still blocks building //xls/common:subprocess_test on macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xls/common/subprocess.cc does not compile on macOS: uses <linux/memfd.h>, memfd_create() and /proc/self/fd

2 participants