Skip to content

Commit 62bc752

Browse files
committed
fix(platform,xlings): a program probe is an argument vector, not a /bin/sh string
The vendored-xlings version probe ran `<xlings> --version 2>/dev/null` as a command string. On Windows every command string reaches cmd.exe, which cannot open /dev/null: it printed "The system cannot find the path specified." in every command after the first, did not run xlings, and returned an empty version, so a Windows home never replaced a vendored xlings older than the pin. mcpp.platform.process gains capture_stdout: the program runs directly, its standard output is captured, its standard error is discarded and its standard input is empty; on Windows the redirect names cmd.exe's own null device. The xlings probe and the four other program probes that carried POSIX grammar on a path Windows reaches use it: the clean link specs, the libstdc++ fallback probe, the freestanding size report and the publish digest. Tests: capture_stdout unit tests (stdout only, exit code, empty stdin, environment, empty output for a missing program, the Windows command line); a version-probe unit test that runs a .bat through the real launcher on Windows; e2e 687 (no path error on the second command; an older vendored binary is replaced).
1 parent 4182fdf commit 62bc752

9 files changed

Lines changed: 299 additions & 18 deletions

File tree

modules/platform/src/process.cppm

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ module;
3232
// Linux and macOS launchers do a direct exec (see run_exec / capture_exec
3333
// below); only Windows keeps the std::system shell path (#248).
3434
#include <unistd.h> // pipe, dup2, close, read
35+
#include <fcntl.h> // O_RDONLY, O_WRONLY for capture_stdout's /dev/null
3536
#include <sys/wait.h> // waitpid
3637
#include <spawn.h> // posix_spawnp, posix_spawn_file_actions_* (incl. addchdir_np)
3738
// The deadline runners' headers (signal.h, errno, poll.h, fcntl.h, time.h)
@@ -110,6 +111,37 @@ RunResult capture_exec(
110111
std::string_view cwd = {},
111112
int* spawn_error = nullptr);
112113

114+
// A PROGRAM PROBE: run `argv` directly, capture its standard output only, give
115+
// it an empty standard input, and discard its standard error.
116+
//
117+
// This is the argument-vector form of `<program> <args> 2>/dev/null`, and it
118+
// exists because that string form is written in one shell's grammar. On
119+
// Windows every command string reaches cmd.exe, which resolves `/dev/null` to
120+
// `\dev\null` on the current drive, cannot open it, prints "The system cannot
121+
// find the path specified." on its own stderr and does not run the program.
122+
// Measured on windows-2022 with mcpp 2026.9.14.3: the vendored-xlings version
123+
// probe printed that line in every command after the first, and returned an
124+
// empty version, which silently disabled the pin check it existed for. The
125+
// null device and the redirection belong to this layer; a caller states only
126+
// the program and its arguments.
127+
//
128+
// The output is the program's standard output and nothing else. A program
129+
// that cannot be started yields exit code 127 and EMPTY output: a probe parses
130+
// what it captured, so a launcher error message in `output` would be read as
131+
// the program's answer. `spawn_error` has run_exec's contract.
132+
RunResult capture_stdout(
133+
const std::vector<std::string>& argv,
134+
const std::vector<std::pair<std::string, std::string>>& extraEnv = {},
135+
int* spawn_error = nullptr);
136+
137+
// capture_stdout for a host tool: the target runtime library search variable is
138+
// cleared for the child, as capture_host_tool does for a command string.
139+
RunResult capture_host_tool_stdout(const std::vector<std::string>& argv);
140+
141+
// The command line capture_stdout hands to cmd.exe on Windows. Host-independent
142+
// so the Windows rendering is testable from any platform.
143+
std::string windows_stdout_probe_command(const std::vector<std::string>& argv);
144+
113145
// Deadline variants: kill the child once `deadline` elapses and set
114146
// *timed_out. A zero deadline means no limit.
115147
//
@@ -753,6 +785,86 @@ RunResult capture_exec(
753785
#endif
754786
}
755787

788+
// Host-independent (see the declaration): always the Windows shape. The
789+
// redirect names cmd.exe's own null device, and the argv is quoted by the one
790+
// shaper every Windows launch in this file uses.
791+
std::string windows_stdout_probe_command(const std::vector<std::string>& argv) {
792+
return windows_command_from_argv(argv) + " 2>nul";
793+
}
794+
795+
RunResult capture_stdout(
796+
const std::vector<std::string>& argv,
797+
const std::vector<std::pair<std::string, std::string>>& extraEnv,
798+
int* spawn_error)
799+
{
800+
RunResult result;
801+
if (spawn_error) *spawn_error = 0;
802+
if (argv.empty()) { result.exit_code = 127; return result; }
803+
#if defined(__linux__) || defined(__APPLE__)
804+
int fds[2];
805+
if (::pipe(fds) != 0) { result.exit_code = 127; return result; }
806+
807+
auto envStore = merged_environ(extraEnv);
808+
std::vector<char*> envp;
809+
for (auto& s : envStore) envp.push_back(s.data());
810+
envp.push_back(nullptr);
811+
std::vector<char*> cargv;
812+
for (auto& a : argv) cargv.push_back(const_cast<char*>(a.c_str()));
813+
cargv.push_back(nullptr);
814+
815+
posix_spawn_file_actions_t fa;
816+
::posix_spawn_file_actions_init(&fa);
817+
::posix_spawn_file_actions_addopen(&fa, 0, "/dev/null", O_RDONLY, 0);
818+
::posix_spawn_file_actions_adddup2(&fa, fds[1], 1); // stdout -> pipe
819+
::posix_spawn_file_actions_addopen(&fa, 2, "/dev/null", O_WRONLY, 0);
820+
::posix_spawn_file_actions_addclose(&fa, fds[0]);
821+
::posix_spawn_file_actions_addclose(&fa, fds[1]);
822+
823+
// Owned as capture_exec's child is: a probe that outlives an interrupted
824+
// mcpp is the same orphan, only smaller.
825+
posix_spawnattr_t attr;
826+
::posix_spawnattr_init(&attr);
827+
::posix_spawnattr_setpgroup(&attr, 0);
828+
::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP);
829+
830+
pid_t pid = 0;
831+
int sp = ::posix_spawnp(&pid, cargv[0], &fa, &attr, cargv.data(), envp.data());
832+
::posix_spawnattr_destroy(&attr);
833+
::posix_spawn_file_actions_destroy(&fa);
834+
::close(fds[1]);
835+
if (sp != 0) {
836+
::close(fds[0]);
837+
result.exit_code = 127;
838+
if (spawn_error) *spawn_error = sp;
839+
return result;
840+
}
841+
mcpp::platform::unixproc::guard_group_on_signal(pid);
842+
843+
std::array<char, 4096> buf{};
844+
ssize_t n;
845+
while ((n = ::read(fds[0], buf.data(), buf.size())) > 0)
846+
result.output.append(buf.data(), static_cast<size_t>(n));
847+
::close(fds[0]);
848+
int status = 0;
849+
while (::waitpid(pid, &status, 0) < 0) { /* EINTR retry */ }
850+
mcpp::platform::unixproc::unguard_group(pid);
851+
result.exit_code = normalize_exit_code(status);
852+
return result;
853+
#else
854+
// cmd.exe reports a program it cannot find on stderr, which the redirect
855+
// discards, and returns 9009; the output stays empty either way.
856+
return capture_with_env(windows_stdout_probe_command(argv), extraEnv);
857+
#endif
858+
}
859+
860+
RunResult capture_host_tool_stdout(const std::vector<std::string>& argv) {
861+
auto key = mcpp::platform::env::host_tool_runtime_library_path_key();
862+
std::optional<mcpp::platform::env::ScopedEnv> runtime_env;
863+
if (!key.empty())
864+
runtime_env.emplace(key, std::nullopt);
865+
return capture_stdout(argv);
866+
}
867+
756868
// ─── The ONE place the platform question is asked for a bounded run ────────
757869
//
758870
// Both launchers answer the same contract behind a `std`-free interface (see

src/build/execute.cppm

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -810,11 +810,12 @@ void report_freestanding_size(const BuildContext& ctx) {
810810
auto art = ctx.outputDir / lu.output;
811811
std::error_code ec;
812812
if (!std::filesystem::exists(art, ec)) continue;
813-
auto out = mcpp::xlings::run_capture(std::format(
814-
"{} {} 2>/dev/null", mcpp::xlings::shq(tool.string()),
815-
mcpp::xlings::shq(art.string())));
816-
if (!out) continue;
817-
auto s = mcpp::freestanding::parse_size_output(*out);
813+
// An argument vector rather than a `2>/dev/null` command string,
814+
// which cmd.exe cannot open on a Windows host.
815+
auto out = mcpp::platform::process::capture_stdout(
816+
{tool.string(), art.string()});
817+
if (out.exit_code != 0 && out.output.empty()) continue;
818+
auto s = mcpp::freestanding::parse_size_output(out.output);
818819
if (!s) continue;
819820
mcpp::ui::info("Size", std::format(
820821
"{} text {} data {} bss {} total {}",

src/fallback/xlings_binary.cppm

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,12 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
156156
std::string vendored_xlings_version(const std::filesystem::path& bin) {
157157
std::error_code ec;
158158
if (!std::filesystem::exists(bin, ec)) return {};
159-
auto r = mcpp::platform::process::capture(std::format(
160-
"{} --version 2>/dev/null", mcpp::platform::shell::quote(bin.string())));
159+
// An argument vector, not a command string. The string form carried
160+
// `2>/dev/null`, which cmd.exe cannot open: on Windows it printed "The
161+
// system cannot find the path specified." in every command after the first,
162+
// did not run xlings, and so returned an empty version, which made the
163+
// pin comparison in acquire_xlings_binary return early forever.
164+
auto r = mcpp::platform::process::capture_stdout({bin.string(), "--version"});
161165
if (r.exit_code != 0) return {};
162166
// Output carries ANSI colour; take the first dotted-numeric run.
163167
std::string out;

src/pm/publisher.cppm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,10 @@ std::string release_tarball_url(std::string_view repo,
360360

361361
std::string sha256_of_file(const std::filesystem::path& file) {
362362
if (!std::filesystem::exists(file)) return {};
363-
auto cmd = std::format("sha256sum {} 2>/dev/null",
364-
mcpp::platform::shell::quote(file.string()));
365-
auto r = mcpp::platform::process::capture_host_tool(cmd);
363+
// An argument vector rather than a `2>/dev/null` command string, which
364+
// cmd.exe cannot open on a Windows host.
365+
auto r = mcpp::platform::process::capture_host_tool_stdout(
366+
{"sha256sum", file.string()});
366367
if (r.exit_code != 0) return {};
367368
// sha256sum format: "<64-hex> <filename>\n"
368369
auto sp = r.output.find(' ');

src/toolchain/gcc.cppm

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,12 @@ std::optional<std::filesystem::path> find_std_module_source(
123123
}
124124
}
125125

126-
auto cmd = std::format("'{}' -print-file-name=libstdc++.so 2>/dev/null",
127-
cxx_binary.string());
128-
auto r = mcpp::toolchain::run_capture(cmd);
129-
if (r) {
130-
auto trimmed = mcpp::toolchain::trim_line(*r);
126+
// An argument vector: the command-string form was written for /bin/sh
127+
// (single quotes and `2>/dev/null`), and cmd.exe reads neither.
128+
auto r = mcpp::platform::process::capture_host_tool_stdout(
129+
{cxx_binary.string(), "-print-file-name=libstdc++.so"});
130+
if (r.exit_code == 0) {
131+
auto trimmed = mcpp::toolchain::trim_line(r.output);
131132
if (!trimmed.empty()) {
132133
std::filesystem::path libpath = trimmed;
133134
auto root2 = libpath.parent_path().parent_path();

src/toolchain/post_install.cppm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -849,9 +849,11 @@ std::filesystem::path write_clean_link_specs(
849849
// regenerating it every build would spawn a process for a constant.
850850
if (std::filesystem::exists(out, ec)) return out;
851851

852-
auto r = mcpp::platform::process::capture(std::format(
853-
"{} -dumpspecs 2>/dev/null",
854-
mcpp::platform::shell::quote(compilerBin.string())));
852+
// An argument vector: the command-string form carried `2>/dev/null`, which
853+
// cmd.exe cannot open, so on a Windows host this probe printed a spurious
854+
// path error in every prepared GCC build and never produced the file.
855+
auto r = mcpp::platform::process::capture_stdout(
856+
{compilerBin.string(), "-dumpspecs"});
855857
if (r.exit_code != 0 || r.output.empty()) return {};
856858

857859
// `*link:` is a section header on its own line; its body is the next line.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#!/usr/bin/env bash
2+
# requires:
3+
# 687 -- the vendored-xlings version probe is an argument vector.
4+
#
5+
# It was the command string `<xlings> --version 2>/dev/null`. On Windows every
6+
# command string reaches cmd.exe, which cannot open `/dev/null`: it printed "The
7+
# system cannot find the path specified." in every command after the first, did
8+
# not run xlings, and returned an empty version. acquire_xlings_binary reads an
9+
# empty version as "unknown, keep it", so a Windows home never replaced a
10+
# vendored xlings older than the pin. Measured on windows-2022 with 2026.9.14.3.
11+
#
12+
# Criteria, in a home whose vendored xlings exists before the command runs:
13+
# A. The command's stderr carries no path error. The denominator is that the
14+
# vendored binary exists, so the probe did run.
15+
# B. A vendored binary whose `--version` answers a version older than the pin
16+
# is replaced from MCPP_VENDORED_XLINGS, and the command says so. The stand-in
17+
# is the ninja payload, whose `--version` prints a dotted version older than
18+
# any dated xlings. Not `subos/default/bin/ninja`: that is an xlings shim, one
19+
# multicall binary that answers as xlings once it is named `xlings`, and for
20+
# the same reason the real xlings is kept under its own name.
21+
# A and B discriminate on Windows; elsewhere they are the control legs.
22+
set -e
23+
24+
TMP=$(mktemp -d)
25+
trap 'rm -rf "$TMP"' EXIT
26+
27+
fail() { echo "FAIL: $1"; shift; for f in "$@"; do echo "--- $f ---"; cat "$f" 2>/dev/null; done; exit 1; }
28+
29+
EXE=""
30+
case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) EXE=".exe" ;; esac
31+
32+
export MCPP_HOME="$TMP/mcpp-home"
33+
# Offline: configuration loading still acquires the vendored xlings (it copies
34+
# a local binary), and no bootstrap step reaches the network.
35+
export MCPP_OFFLINE=1
36+
source "$(dirname "$0")/_inherit_toolchain.sh"
37+
cd "$TMP"
38+
39+
VENDORED="$MCPP_HOME/registry/bin/xlings$EXE"
40+
41+
"$MCPP" self env > first.out 2> first.err || true
42+
[ -f "$VENDORED" ] || fail "the first command did not vendor xlings at $VENDORED" first.out first.err
43+
44+
# ── A ──────────────────────────────────────────────────────────────────────
45+
"$MCPP" self env > second.out 2> second.err || true
46+
if grep -qi 'cannot find the path specified' second.err; then
47+
fail "A: the version probe reached a shell that could not open its redirect" second.err
48+
fi
49+
echo "ok: A, the probe ran and wrote nothing to stderr"
50+
51+
# ── B ──────────────────────────────────────────────────────────────────────
52+
NINJA=""
53+
for cand in "$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/ninja$EXE \
54+
"$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/bin/ninja$EXE; do
55+
if [ -f "$cand" ]; then NINJA="$cand"; break; fi
56+
done
57+
[ -n "$NINJA" ] || fail "B: no ninja binary to stand in for an older xlings under $MCPP_HOME/registry"
58+
older=$("$NINJA" --version 2>/dev/null | head -1)
59+
case "$older" in
60+
[0-9]*.*) ;;
61+
*) fail "B: the stand-in '$NINJA' answered '$older', not a dotted version" ;;
62+
esac
63+
64+
mkdir -p "$TMP/real"
65+
cp "$VENDORED" "$TMP/real/xlings$EXE"
66+
rm -f "$VENDORED"
67+
cp "$NINJA" "$VENDORED"
68+
chmod +x "$VENDORED" 2>/dev/null || true
69+
70+
MCPP_VENDORED_XLINGS="$TMP/real/xlings$EXE" "$MCPP" self env > third.out 2> third.err || true
71+
grep -q "vendored xlings $older -> " third.err \
72+
|| fail "B: a vendored xlings answering $older was not replaced" third.out third.err
73+
"$VENDORED" --version 2>/dev/null | grep -q '^xlings ' \
74+
|| fail "B: after the replacement the vendored binary is not xlings" third.err
75+
echo "ok: B, a vendored binary older than the pin was replaced"

tests/unit/test_process_run_exec.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,47 @@ TEST(CaptureExecDeadline, SpawnFailureIsInOutputWhenCallerDoesNotAsk) {
215215
EXPECT_NE(r.output.find("error 2"), std::string::npos) << r.output;
216216
}
217217

218+
// ── capture_stdout: the argument-vector form of `<program> 2>/dev/null` ──
219+
220+
TEST(CaptureStdout, CapturesStandardOutputOnly) {
221+
auto r = process::capture_stdout(
222+
{"/bin/sh", "-c", "echo out; echo 9.9.9-on-stderr 1>&2"});
223+
EXPECT_EQ(r.exit_code, 0);
224+
EXPECT_EQ(r.output, "out\n");
225+
}
226+
227+
TEST(CaptureStdout, PropagatesExitCode) {
228+
auto r = process::capture_stdout({"/bin/sh", "-c", "echo partial; exit 3"});
229+
EXPECT_EQ(r.exit_code, 3);
230+
EXPECT_EQ(r.output, "partial\n");
231+
}
232+
233+
// A probe parses what it captured, so a launcher message must never be read
234+
// as the program's answer: a missing program leaves the output empty.
235+
TEST(CaptureStdout, MissingProgramLeavesOutputEmpty) {
236+
int spawnErr = 0;
237+
auto r = process::capture_stdout({"/no/such/program/mcpp-probe-2026.1.2.3"},
238+
{}, &spawnErr);
239+
EXPECT_EQ(r.exit_code, 127);
240+
EXPECT_EQ(spawnErr, ENOENT);
241+
EXPECT_TRUE(r.output.empty()) << r.output;
242+
}
243+
244+
TEST(CaptureStdout, StandardInputIsEmpty) {
245+
// `cat` returns at once with nothing when stdin is /dev/null; with an
246+
// inherited terminal or pipe it would block or echo the parent's input.
247+
auto r = process::capture_stdout({"/bin/sh", "-c", "cat; echo done"});
248+
EXPECT_EQ(r.exit_code, 0);
249+
EXPECT_EQ(r.output, "done\n");
250+
}
251+
252+
TEST(CaptureStdout, ExtraEnvironmentReachesTheChild) {
253+
auto r = process::capture_stdout({"/bin/sh", "-c", "printf %s \"$MCPP_PROBE_ENV\""},
254+
{{"MCPP_PROBE_ENV", "reached"}});
255+
EXPECT_EQ(r.exit_code, 0);
256+
EXPECT_EQ(r.output, "reached");
257+
}
258+
218259
#else // _WIN32
219260

220261
TEST(RunExec, WindowsCoveredByIntegration) {
@@ -224,3 +265,12 @@ TEST(RunExec, WindowsCoveredByIntegration) {
224265
}
225266

226267
#endif
268+
269+
// Host-independent: the command line capture_stdout gives cmd.exe names cmd's
270+
// own null device, never a POSIX path cmd would try to open as `\dev\null`.
271+
TEST(CaptureStdout, WindowsCommandNamesCmdNullDevice) {
272+
auto line = process::windows_stdout_probe_command(
273+
{"C:\\Program Files\\xlings\\xlings.exe", "--version"});
274+
EXPECT_EQ(line, "\"C:\\Program Files\\xlings\\xlings.exe\" \"--version\" 2>nul");
275+
EXPECT_EQ(line.find("/dev/null"), std::string::npos) << line;
276+
}

tests/unit/test_xlings_version_pin.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// one, and an unparseable version is not evidence of being behind.
1414

1515
#include <gtest/gtest.h>
16+
#include <fstream>
1617

1718
import std;
1819
import mcpp.fallback.xlings_binary;
@@ -63,4 +64,38 @@ TEST(XlingsVersionPin, DatedSchemeIsNewerThanTheOldOne) {
6364
EXPECT_TRUE(fb::version_is_older("0.4.51", "0.4.54"));
6465
}
6566

67+
// THE PROBE READS STANDARD OUTPUT, AND ON WINDOWS IT RUNS AT ALL.
68+
//
69+
// The version used to be read through the command string
70+
// `<bin> --version 2>/dev/null`. cmd.exe cannot open `/dev/null`, so on Windows
71+
// the probe printed "The system cannot find the path specified.", never ran
72+
// xlings, and returned an empty version -- which acquire_xlings_binary reads as
73+
// "unknown, keep it", so a Windows home never moved to a newer pin. On Windows
74+
// this test runs a .bat through the real launcher, which is the path that
75+
// failed; elsewhere a shell script. Both print a dotted number on stderr first,
76+
// which must not be taken for the version.
77+
TEST(XlingsVersionPin, ProbeReadsStandardOutputThroughTheLauncher) {
78+
auto dir = std::filesystem::temp_directory_path()
79+
/ std::format("mcpp probe {}", std::chrono::steady_clock::now().time_since_epoch().count());
80+
std::filesystem::create_directories(dir);
81+
#if defined(_WIN32)
82+
auto fake = dir / "xlings.bat";
83+
{
84+
std::ofstream os(fake, std::ios::binary);
85+
os << "@echo off\r\necho warning 9.9.9 1>&2\r\necho xlings 2026.1.2.3\r\n";
86+
}
87+
#else
88+
auto fake = dir / "xlings";
89+
{
90+
std::ofstream os(fake, std::ios::binary);
91+
os << "#!/bin/sh\necho 'warning 9.9.9' 1>&2\nprintf 'xlings 2026.1.2.3\\n'\n";
92+
}
93+
std::filesystem::permissions(fake, std::filesystem::perms::owner_all,
94+
std::filesystem::perm_options::replace);
95+
#endif
96+
EXPECT_EQ(fb::vendored_xlings_version(fake), "2026.1.2.3");
97+
std::error_code ec;
98+
std::filesystem::remove_all(dir, ec);
99+
}
100+
66101
} // namespace

0 commit comments

Comments
 (0)