Fix subprocess helper exec on macOS - #4994
marketchedai wants to merge 1 commit into
Conversation
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>
|
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. |
|
We are already reviewing #4972 for this. |
|
Thanks @allight — agreed, #4972 supersedes this, and it is the better fix: a For anyone finding this later: #4972 merged as 2257034 and removes the Note that #4993 is a separate problem and is still outstanding on current |
Fixes #4991.
xls/common/subprocess.cccannot 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 anexec'able path for the embedded helper:
returns
/proc/self/fd/<fd>, holding the descriptor open for the lifetimeof the process.
mkstemp(),marks it executable with
fchmod(0700), and unlinks it throughstd::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 withEACCES, and there is nofexecve(). 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_helperis now a reference to a function-localstatic const,which has thread-safe initialization, is never mutated, and outlives the
c_str()pointers handed toposix_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 otherbranch 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
#elsebranch.On macOS arm64, with Bazel 8.7.0:
//xls/common:subprocess_testpasses (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-sonamepassed to the Mach-O linker #4992 to be fixed first, otherwise the testcannot be built on macOS at all.
//xls/common/...passes 36/36.interpreter_main,ir_converter_main,opt_mainandcodegen_mainallbuild, and a DSLX -> IR -> opt -> Verilog run produces correct output.
🤖 Generation assisted with Claude Code