fix(run): exec target/ninja directly, scope LD_LIBRARY_PATH to child#133
Merged
Conversation
mcpp run/test/build injected the bundled-glibc LD_LIBRARY_PATH into mcpp's own process (setenv) before std::system/capture, so the host /bin/sh it spawned inherited it. On newer-glibc distros sh then loaded the bundled (older) libc, which could not satisfy host libtinfo's GLIBC_2.42 symbols, and aborted before the target ran (reported as a 'sh:' version error). Add platform::process::run_exec/capture_exec (no shell; child-scoped env) and route the run, test, fast-path-ninja, and full-build-ninja launches through them. The produced binary is already self-contained (bundled --dynamic-linker + RUNPATH via gcc-specs/clang-cfg patch), so the loader env was redundant for normal binaries anyway. - store BuildResult::ninjaProgram raw (execvp needs argv, not a shell string); quote only at the remaining shell site; defensive unquote for legacy caches. - tests/unit/test_process_run_exec.cpp: proves parent env is never mutated. - tests/e2e/74: hostile LD_LIBRARY_PATH + bundled-interp guard. - ci-fresh-install: linux-distro-matrix across fedora/arch/tumbleweed/ debian-testing (+ old-glibc ubuntu-2004/debian-11).
CI surfaced two cross-platform bugs in the first cut:
- macOS: fork()+setenv()-in-child mutated the test's parent-env expectation
(post-fork setenv is async-signal-unsafe on macOS). Switch POSIX run_exec/
capture_exec to posix_spawnp with an envp built in the PARENT — the child
env never touches the parent, and there is no post-fork setenv.
- Windows: capture_exec force-quoted argv[0], so popen's cmd.exe /c "..."
stripped the outer quotes and mangled the ninja path ('filename syntax
incorrect'). Keep argv[0] raw (per platform.shell guidance) and quote only
later args.
Unit tests gated to POSIX (they reference /bin/*); Windows path is covered by
the integration build that launches ninja via capture_exec.
Verified on Linux: unit 19/19, e2e 74, fast-path rebuild.
Both spawn-based attempts regressed non-Linux: posix_spawn returned an error on the macOS runner (run_exec rc!=0), and _spawnvpe segfaulted mcpp run on Windows (0xC0000005). The leak is Linux-only (macOS injects no runtime env; Windows has no glibc symbol versioning), so changing the launch primitive on every platform was the wrong call. Reimplement run_exec/capture_exec on top of the existing shell helpers: - run_exec: std::system with the env as a prefix via build_env_prefix (POSIX) / _putenv_s (Windows). The shell std::system spawns starts clean, so a bundled-glibc LD_LIBRARY_PATH reaches only the target child, never /bin/sh — which fixes the original crash. - capture_exec: command_from_argv + ' 2>&1' through the existing capture_with_env (same prefix semantics). No spawn primitives, no per-platform exec code. Linux: unit 19/19, e2e 74, hostile-LD_LIBRARY_PATH run, and fast-path rebuild all green.
The macOS CI failures across every launcher attempt were this test, not the implementation: /bin/true lives at /usr/bin/true on macOS, so run_exec returned 127 and the rc==0 assertion failed. Use /bin/sh -c 'exit 0' (present on Linux and macOS).
… shell Per review: use a direct exec (posix_spawn) on Linux, where the leak actually lives and where it can be iterated locally, and keep the proven std::system shell path on macOS/Windows (which inject no runtime env / have no glibc symbol versioning). This gives Linux the clean child-only env isolation with no shell quoting/signal/injection surface, while not risking the platforms that can't be reproduced here. A TODO(launcher-unify) marks unifying onto spawn later if macOS/Windows ever need the same isolation. Linux verified: unit 19/19, e2e 74, hostile-LD_LIBRARY_PATH run, fast-path, passthrough args.
Sunrisepeak
added a commit
that referenced
this pull request
Jun 18, 2026
…56 (#134) Pack distribution has two orthogonal axes: libc/static (a build-target property via --target gnu/musl) and bundling depth (the --mode axis). The old modes mixed those frames (Static named libc; Bundle* named depth) and had no 'depend on OS for everything' mode. - Rename --mode values to idiomatic, self-documenting names; keep old names as PERMANENT aliases: bundle-project=vendored (default), bundle-all=self-contained. static unchanged. - Add 'system' mode (Mode::None): bundle nothing, repoint PT_INTERP to the LSB loader — for .deb/.rpm packaging and same-distro fleet deploy. - Split mode_name() into mode_cli_name() (display) and mode_tarball_suffix() (FROZEN wire format) so renaming cannot change release artifact names (guarded by 30_pack_modes.sh tarball-name asserts). - Bump version to 0.0.56 (mcpp.toml + MCPP_VERSION) + CHANGELOG. This release also carries the run/test/build loader-env launch fix (#133). tests/unit/test_pack_modes.cpp + 30_pack_modes.sh system-mode coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a newer-glibc distro,
mcpp runcrashed with:The failing process is
sh, not the target.mcpp run/test/build inject the bundled-glibcLD_LIBRARY_PATHinto mcpp's own process (ScopedEnv/setenv) beforestd::system/capture, so the host/bin/shthey spawn inherits it. On a host whose glibc is newer than the bundled 2.39,shloads the bundled (older) libc, which can't satisfy hostlibtinfo'sGLIBC_2.42symbols →shaborts before the target ever runs.The produced binary is already self-contained (bundled
--dynamic-linker+ RUNPATH baked in via the gcc-specs / clang-cfg patch), so the injected loader env is redundant for normal binaries — it only ever served the narrowdlopenclosure.Fix
platform::process::run_exec/capture_exec: launch directly (no/bin/sh), applying extra env to the child only (never mutating mcpp's env).capture_execmerges stderr (replaces the old2>&1).mcpp run,mcpp test, fast-path ninja, and the full-build ninja invocation (a latent same-class leak).BuildResult::ninjaProgramraw (execvp takes argv, not a shell string); quote only at the remaining shell site; defensive unquote for legacy caches.Tests / CI
tests/unit/test_process_run_exec.cpp— proves the parent environment is never mutated (the exact leak), child sees injected env, exit-code/stderr semantics.tests/e2e/74_run_no_loader_env_leak.sh— hostileLD_LIBRARY_PATH+ bundled-PT_INTERPguard.ci-fresh-install.yml— newlinux-distro-matrixacross fedora / arch / tumbleweed / debian-testing (the reproduction surface) plus ubuntu-20.04 / debian-11 (the safe reverse direction). Runs post-release against the released binary.Verified locally
Unit suite 19/19 (incl. 6 new); e2e 74 green; fast-path rebuild restored;
mcpp runworks under a hostileLD_LIBRARY_PATH; produced binaryPT_INTERPresolves into the bundled loader. (e2e 65 fails identically on baseline 0.0.55 — known environmental LLVM issue, not a regression.)Design:
.agents/docs/2026-06-19-runtime-launch-and-multi-distro-plan.md.