Skip to content

Commit f244b7e

Browse files
committed
fix: 两处按宿主分岔的缺陷,都只在 Linux 之外成立
这两条是把规则包在 macOS 与 Windows 上真正跑一遍时暴露的。两处的共同形状是: 一段代码的正确性依赖于宿主,而这个仓库的 CI 只在其中一个宿主上执行它。 ## 一、构建程序的链接不带工具链自己的运行时目录(macOS) `host_link_tokens` 有两个出口:一个把 clang 的配置逐条写出来,一个信任载荷自带的 `<driver>.cfg`。第二个提前 return,跳过了给 `Toolchain::linkRuntimeDirs` 发 `-L` 与 `-rpath` 的那一段。 信任 cfg 决定的是**链哪些运行时**,它从来没有决定**去哪里找**。macOS 上 `-lc++` 于是经 SDK 解析到系统那份,而头文件来自载荷。两者在头文件引用了系统库还没导出的 符号那一刻分道扬镳 —— macos-14 上编译一个只有 `import std` 的构建程序: ld64.lld: error: undefined symbol: std::__1::__is_posix_terminal(__sFILE*) >>> referenced by std::__1::__print::__is_terminal(__sFILE*) `std::print` 不是 header-only 的,它的两个重载都要到 libc++ **dylib** 里取支持 符号,而那两个符号是在 macOS 14 不带的那一版里加进去的。macOS 15 有,所以这个项目 用到的每一台 macOS runner 都是绿的。 载荷自带 `lib/libc++.1.0.dylib` —— 与它自己的头文件配套的那一份 —— 而 `linkRuntimeDirs` 早就命名了那个目录。给它发 `-L` 与 `-rpath` 是 Linux 那条路 一直在做的事;macOS 上被跳过,只是因为这个分支先 return 了。 ⭐ **构建程序正是绝对 rpath 该在的地方**:它从不被分发,mcpp 编译它、在这台机器上 运行它、按这套工具链的身份缓存它。同一个问题对**产物**的答案由分发契约决定,不归 这个函数回答。 判据 `HostFlags.EveryExitNamesTheToolchainRuntimeDirs`:把两个出口都测一遍。为此 `CfgBypass` 补了第三个取值 `Never` —— 缺陷所在的那个分支在 Linux 上不可达 (`LinuxOnly` 在那里折叠成 `Always`),而一条只能在它写给的两个宿主上跑的判据,会 继承同一个盲点。实测:把修复拿掉,这条当场变红。 ## 二、版本约束里的 `>` 被 cmd.exe 读成重定向(Windows) mcpp 把供给请求作为 JSON 参数放在 shell 命令行上交给 xlings。`shell::quote` 回答 的是**子进程**那一层的解析 —— MSVCRT,它给内嵌引号的转义是 `\"` —— 而 cmd.exe 不认这个转义:对它来说每个 `"` 只是在切换引用状态。于是一段 JSON 走到 `>` 的时候, 它背后的引号数是**偶数**,`>` 成了重定向,重定向目标是 JSON 的其余部分: Provisioning [xlings.workspace] entries declared by dependencies (xim:shaderc@>=2026.3) The filename, directory name, or volume label syntax is incorrect. error: provisioning ... failed: xlings exited 1 windows-2022 实测。`>=` 正是每个规则包声明下界用的形态,而**在此之前没有任何一条能 在 Windows 上生效的声明带过 `>`**,所以整个形态在那个宿主上从没被走到过。 修法是标准的双重转义:先按子进程的规则引用,再给每个 cmd 元字符(包括引号本身)前 缀 `^`。没有任何 `"` 是裸的,cmd 就从不进入引用状态,于是每个元字符都是被转义而不是 被引用的 —— 这是两套规则同时成立的唯一状态。cmd 去掉那些 `^`,子进程看到的正是 `quote_windows` 的输出。 ⚠️ `%` 不转义也无法转义:变量展开发生在 `^` 处理之前,而 `%%` 只在批处理文件里可用。 这一点写在函数注释里;这里的 JSON 参数不含 `%`。 判据是两个模拟器 —— 一个复现 cmd 的解析,一个复现 MSVCRT 的 argv 解析 —— 断言的是 **子进程收到的参数等于本来要传的那个 JSON**,而不是转义的拼写。另有一条反向判据 断言旧的引用方式确实让 cmd 看见了一个活的元字符,否则正向那条什么都不证明。 端到端的判据在 mcpp-plugins:它的 Windows job 目前用精确版本绕开这条缺陷,等这个 修复发布后把声明改回 `>=2026.3`,那条 job 就是这个修复的端到端判据。
1 parent 512d081 commit f244b7e

6 files changed

Lines changed: 341 additions & 17 deletions

File tree

modules/platform/src/shell.cppm

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,37 @@ std::string quote(std::string_view s);
2727
std::string quote_windows(std::string_view s);
2828
std::string quote_posix(std::string_view s);
2929

30+
// An argument that must survive TWO parsers: cmd.exe's, and then the child
31+
// program's own argv parsing.
32+
//
33+
// `quote_windows` answers only the second. Its `\"` escape belongs to the
34+
// MSVCRT argv rules, and cmd.exe does not know it: to cmd every `"` simply
35+
// toggles a quote state, so a payload carrying an EVEN number of them before a
36+
// metacharacter leaves that character unquoted. Measured on windows-2022 with
37+
// a JSON argument -- `{"targets":["xim:shaderc@>=2026.3"],"yes":true}` puts
38+
// four quotes before the `>`, and cmd read it as a REDIRECTION, answering
39+
//
40+
// The filename, directory name, or volume label syntax is incorrect.
41+
//
42+
// which arrived as a package-provisioning failure naming a package.
43+
//
44+
// The answer is the standard double escape: quote for the child, then prefix
45+
// every cmd metacharacter -- the quotes included -- with `^`. With no `"` left
46+
// unescaped cmd never enters a quoted region, so every metacharacter is
47+
// escaped rather than quoted, which is the only state in which both rules
48+
// hold. cmd removes the carets and the child sees exactly `quote_windows`.
49+
//
50+
// `%` IS NOT ESCAPED AND CANNOT BE. Variable expansion happens before caret
51+
// processing, and the batch-file escape (`%%`) is not available on a command
52+
// line. Callers passing text that may contain `%` need a different mechanism;
53+
// the JSON arguments this exists for do not.
54+
std::string quote_windows_through_cmd(std::string_view s);
55+
56+
// Host-selecting: `quote_windows_through_cmd` on Windows, `quote_posix`
57+
// elsewhere. Use this wherever an argument reaches a shell and may contain
58+
// metacharacters -- notably JSON, and any version constraint spelled `>=`.
59+
std::string quote_through_shell(std::string_view s);
60+
3061
// Silent redirect — stdout + stderr → /dev/null (or NUL on Windows).
3162
// stdin is NOT touched here; that's the responsibility of
3263
// mcpp::platform::process::seal_stdin, which is auto-applied by capture /
@@ -67,6 +98,24 @@ std::string quote_posix(std::string_view s) {
6798
return out;
6899
}
69100

101+
std::string quote_windows_through_cmd(std::string_view s) {
102+
const std::string inner = quote_windows(s);
103+
std::string out;
104+
out.reserve(inner.size() * 2);
105+
for (char c : inner) {
106+
switch (c) {
107+
case '"': case '<': case '>': case '&': case '|':
108+
case '^': case '(': case ')':
109+
out.push_back('^');
110+
break;
111+
default:
112+
break;
113+
}
114+
out.push_back(c);
115+
}
116+
return out;
117+
}
118+
70119
std::string quote(std::string_view s) {
71120
#if defined(_WIN32)
72121
return quote_windows(s);
@@ -75,4 +124,12 @@ std::string quote(std::string_view s) {
75124
#endif
76125
}
77126

127+
std::string quote_through_shell(std::string_view s) {
128+
#if defined(_WIN32)
129+
return quote_windows_through_cmd(s);
130+
#else
131+
return quote_posix(s);
132+
#endif
133+
}
134+
78135
} // namespace mcpp::platform::shell

src/build/build_program.cppm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,11 @@ std::expected<void, std::string> run_build_program(
775775
compilerIdentity += "\nbuild-program-link=";
776776
compilerIdentity += muslStaticHelper ? "musl-static-v1"
777777
: mingwStaticHelper ? "mingw-static-v1"
778-
: "default-v2"; // v2: DT_RPATH on Linux
778+
: "default-v3"; // v2: DT_RPATH on Linux
779+
// v3: + the same on
780+
// macOS/Windows,
781+
// where the cfg
782+
// path skipped it
779783
std::string programHash = mcpp::toolchain::hash_file(src);
780784
std::string compilerHash = mcpp::toolchain::hash_string(compilerIdentity);
781785

src/toolchain/hostflags.cppm

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,14 @@ struct HostFlagOptions {
4949
// needs_explicit_libcxx path owns; duplicating that for a
5050
// host compile produced undefined __cxa_* /
5151
// __gxx_personality_v0 (build_program.cppm, pre-existing).
52-
enum class CfgBypass { Always, LinuxOnly };
52+
// Never — always trust the cfg. No caller selects it; it completes
53+
// the enum, and it is what makes the cfg-trusting branch of
54+
// `host_link_tokens` reachable from a test on a Linux
55+
// runner. `LinuxOnly` folds into `Always` there, so without
56+
// this value that branch could only be exercised on the two
57+
// hosts it was written for -- which is how it came to be
58+
// missing the runtime-directory tokens in the first place.
59+
enum class CfgBypass { Always, LinuxOnly, Never };
5360
CfgBypass cfgBypass = CfgBypass::Always;
5461

5562
// binutils `-B` so the driver finds as/ld. A GCC/libstdc++ payload
@@ -206,7 +213,8 @@ std::vector<std::string> host_compile_tokens(const Toolchain& tc,
206213

207214
const bool bypassCfg =
208215
dm.hasCfg && (opt.cfgBypass == HostFlagOptions::CfgBypass::Always
209-
|| mcpp::platform::is_linux);
216+
|| (opt.cfgBypass == HostFlagOptions::CfgBypass::LinuxOnly
217+
&& mcpp::platform::is_linux));
210218

211219
// Trusting the cfg means contributing no include paths, stdlib selection
212220
// or runtime choices — it already carries them. It does NOT mean
@@ -297,6 +305,20 @@ std::vector<std::string> bmi_reference_tokens(std::string_view usePrefix,
297305
std::string(p.substr(sp + 1)) + bmi.string() };
298306
}
299307

308+
// The toolchain's own runtime directories, on both exits of the function
309+
// below. `-L` is link-time and wanted everywhere; rpath is an ELF and Mach-O
310+
// concept. A PE target reaches here too, where the rpath flag is inert and
311+
// self-containment comes from the static link instead (#299).
312+
void append_runtime_lib_dirs(const Toolchain& tc, const HostFlagOptions& opt,
313+
const PathEscape& esc, std::vector<std::string>& out) {
314+
if (!opt.runtimeLibDirs) return;
315+
for (auto& d : tc.linkRuntimeDirs) {
316+
out.push_back("-L" + esc(d));
317+
if constexpr (mcpp::platform::supports_rpath)
318+
out.push_back("-Wl,-rpath," + esc(d));
319+
}
320+
}
321+
300322
std::vector<std::string> host_link_tokens(const Toolchain& tc,
301323
const HostFlagOptions& opt,
302324
const PathEscape& esc) {
@@ -308,7 +330,8 @@ std::vector<std::string> host_link_tokens(const Toolchain& tc,
308330

309331
const bool bypassCfg =
310332
dm.hasCfg && (opt.cfgBypass == HostFlagOptions::CfgBypass::Always
311-
|| mcpp::platform::is_linux);
333+
|| (opt.cfgBypass == HostFlagOptions::CfgBypass::LinuxOnly
334+
&& mcpp::platform::is_linux));
312335

313336
if (bypassCfg) {
314337
for (auto& t : dm.link_tokens(esc)) out.push_back(t);
@@ -336,6 +359,37 @@ std::vector<std::string> host_link_tokens(const Toolchain& tc,
336359
// lld ships with the very toolchain doing the compile, so it cannot
337360
// be diverted to a libc++ it was not built against.
338361
if constexpr (mcpp::platform::is_macos) out.push_back("-fuse-ld=lld");
362+
// AND THE RUNTIME DIRECTORIES, WHICH THIS PATH USED TO SKIP.
363+
//
364+
// Trusting clang's cfg decides WHICH runtimes are linked. It does not
365+
// decide WHERE they are found, and on macOS `-lc++` resolves through
366+
// the SDK to /usr/lib/libc++.tbd -- the system copy, whose version
367+
// floats with the host OS -- while the headers come from the payload.
368+
// The two disagree the moment the headers reference a symbol the
369+
// system library does not export yet:
370+
//
371+
// ld64.lld: error: undefined symbol:
372+
// std::__1::__is_posix_terminal(__sFILE*)
373+
// >>> referenced by std::__1::__print::__is_terminal(__sFILE*)
374+
//
375+
// measured on macos-14 compiling a build program that does nothing but
376+
// `import std`. `std::print` is not header-only: its FILE* and ostream
377+
// overloads call into the libc++ dylib, and those two support symbols
378+
// arrived in a version macOS 14 does not ship. macOS 15's copy has
379+
// them, which is why every macOS runner this project uses was green.
380+
//
381+
// The payload ships its own `lib/libc++.1.0.dylib` -- the toolchain's
382+
// own copy, matching its own headers -- and `linkRuntimeDirs` already
383+
// names that directory. Emitting `-L` and `-rpath` for it is what the
384+
// Linux path has always done here; it was skipped on macOS only
385+
// because this branch returned first.
386+
//
387+
// A BUILD PROGRAM IS THE RIGHT PLACE FOR AN ABSOLUTE RPATH. It is
388+
// never distributed: mcpp compiles it, runs it on this machine, and
389+
// caches it against this toolchain's identity. The equivalent question
390+
// for an ARTIFACT is decided by the distribution contract, and is not
391+
// this function's to answer.
392+
append_runtime_lib_dirs(tc, opt, esc, out);
339393
return out;
340394
}
341395

@@ -346,16 +400,7 @@ std::vector<std::string> host_link_tokens(const Toolchain& tc,
346400
out.push_back("-B" + esc(ar.parent_path()));
347401
}
348402

349-
if (opt.runtimeLibDirs) {
350-
// -L is link-time and wanted everywhere; rpath is an ELF-only concept.
351-
// A PE target reaches here too, where the flag is inert and
352-
// self-containment comes from the static link instead (#299).
353-
for (auto& d : tc.linkRuntimeDirs) {
354-
out.push_back("-L" + esc(d));
355-
if constexpr (mcpp::platform::supports_rpath)
356-
out.push_back("-Wl,-rpath," + esc(d));
357-
}
358-
}
403+
append_runtime_lib_dirs(tc, opt, esc, out);
359404

360405
return out;
361406
}

src/xlings/xlings.cppm

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,10 @@ namespace paths {
259259
// Shell-escape (single-quote) a string for the command line.
260260
std::string shq(std::string_view s);
261261

262+
// `shq` for an argument that may carry a shell metacharacter (JSON, `>=`).
263+
// See `shell::quote_windows_through_cmd`.
264+
std::string shq_meta(std::string_view s);
265+
262266
// ─── Shell command builders ─────────────────────────────────────────
263267

264268
// Build the standard xlings command prefix with proper env vars.
@@ -772,6 +776,13 @@ std::string shq(std::string_view s) {
772776
return mcpp::platform::shell::quote(s);
773777
}
774778

779+
// The same, for an argument that may carry a shell metacharacter -- which the
780+
// JSON payloads below do, and which a version constraint spelled `>=` does.
781+
// See `quote_windows_through_cmd` for what `shq` alone leaves unprotected.
782+
std::string shq_meta(std::string_view s) {
783+
return mcpp::platform::shell::quote_through_shell(s);
784+
}
785+
775786
// ─── Path helpers ───────────────────────────────────────────────────
776787

777788
namespace paths {
@@ -1157,7 +1168,7 @@ std::string build_interface_command(const Env& env,
11571168
std::string_view capability,
11581169
std::string_view argsJson) {
11591170
return std::format("{} interface {} --args {} {}",
1160-
build_command_prefix(env), capability, shq(argsJson),
1171+
build_command_prefix(env), capability, shq_meta(argsJson),
11611172
mcpp::platform::null_redirect);
11621173
}
11631174

@@ -1417,15 +1428,15 @@ int install_with_progress(const Env& env, std::string_view target,
14171428
if constexpr (mcpp::platform::is_windows) {
14181429
return std::format("{} interface install_packages --args {} {} <NUL",
14191430
build_command_prefix(env),
1420-
shq(argsJson),
1431+
shq_meta(argsJson),
14211432
mcpp::platform::null_redirect);
14221433
} else {
14231434
return std::format(
14241435
"cd {} && env -u XLINGS_PROJECT_DIR XLINGS_HOME={} {} interface install_packages --args {} {} </dev/null",
14251436
shq(env.home.string()),
14261437
shq(env.home.string()),
14271438
shq(env.binary.string()),
1428-
shq(argsJson),
1439+
shq_meta(argsJson),
14291440
mcpp::platform::null_redirect);
14301441
}
14311442
}();

tests/unit/test_hostflags.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,83 @@ TEST(HostFlags, CfgBypassLinuxOnlyDiffersFromAlwaysOffLinux) {
206206
}
207207
}
208208

209+
// ── Every exit of host_link_tokens names the toolchain's runtime dirs ───────
210+
//
211+
// THE DEFECT THIS STATES. `host_link_tokens` has two exits: one that spells
212+
// clang's configuration out and one that trusts the bundled `<driver>.cfg`.
213+
// The second returned early, before the block that emits `-L` and `-rpath`
214+
// for `Toolchain::linkRuntimeDirs`. Trusting the cfg decides WHICH runtimes
215+
// are linked; it never decided WHERE they are found, and on macOS `-lc++`
216+
// then resolved through the SDK to the system library while the headers came
217+
// from the payload. Measured on macos-14, compiling a build program that does
218+
// nothing but `import std`:
219+
//
220+
// ld64.lld: error: undefined symbol: std::__1::__is_posix_terminal(__sFILE*)
221+
//
222+
// -- a symbol `std::print`'s inline machinery references and that release's
223+
// libc++ does not export. The payload ships its own copy, one directory the
224+
// early return had dropped.
225+
//
226+
// AND WHY THIS TEST CAN RUN ON LINUX. The branch is chosen by the host: with
227+
// `LinuxOnly`, a Linux build always takes the spelled-out exit, so the exit
228+
// that had the defect is unreachable from a Linux runner. `CfgBypass::Never`
229+
// exists to make it reachable -- the defect lived in a branch that only two
230+
// of the three CI hosts could execute, and a test that could only run there
231+
// would inherit the same blind spot.
232+
namespace {
233+
234+
// A directory that looks enough like a clang payload for `resolve_clang_driver`
235+
// to report `hasCfg`: a driver file and a sibling `<driver>.cfg`.
236+
struct FakeClangPayload {
237+
std::filesystem::path root;
238+
explicit FakeClangPayload(std::string_view name) {
239+
root = std::filesystem::temp_directory_path()
240+
/ ("mcpp-hostflags-" + std::string(name));
241+
std::filesystem::remove_all(root);
242+
std::filesystem::create_directories(root / "bin");
243+
std::filesystem::create_directories(root / "lib");
244+
std::ofstream{root / "bin" / "clang++"} << "";
245+
std::ofstream{root / "bin" / "clang++.cfg"} << "";
246+
}
247+
~FakeClangPayload() { std::error_code ec; std::filesystem::remove_all(root, ec); }
248+
};
249+
250+
bool names_dir(const std::vector<std::string>& tokens, std::string_view dir) {
251+
return std::ranges::any_of(tokens, [&](auto const& t) {
252+
return t == std::string("-L") + std::string(dir);
253+
});
254+
}
255+
256+
} // namespace
257+
258+
TEST(HostFlags, EveryExitNamesTheToolchainRuntimeDirs) {
259+
FakeClangPayload payload{"runtime-dirs"};
260+
auto tc = tc_for(CompilerId::Clang);
261+
tc.binaryPath = payload.root / "bin" / "clang++";
262+
tc.linkRuntimeDirs = { payload.root / "lib" };
263+
264+
HostFlagOptions opt;
265+
opt.runtimeLibDirs = true;
266+
267+
// The cfg-trusting exit -- the one that returned early.
268+
opt.cfgBypass = HostFlagOptions::CfgBypass::Never;
269+
auto trusting = mcpp::toolchain::host_link_tokens(tc, opt, mcpp::toolchain::no_escape);
270+
EXPECT_TRUE(names_dir(trusting, (payload.root / "lib").string()))
271+
<< "the cfg-trusting exit dropped the toolchain's own runtime directory, "
272+
"so `-lc++` resolves to whatever the system has";
273+
274+
// The spelled-out exit, which always did.
275+
opt.cfgBypass = HostFlagOptions::CfgBypass::Always;
276+
auto spelled = mcpp::toolchain::host_link_tokens(tc, opt, mcpp::toolchain::no_escape);
277+
EXPECT_TRUE(names_dir(spelled, (payload.root / "lib").string()));
278+
279+
// And the option is still an option: nothing is emitted when it is off.
280+
opt.runtimeLibDirs = false;
281+
opt.cfgBypass = HostFlagOptions::CfgBypass::Never;
282+
auto off = mcpp::toolchain::host_link_tokens(tc, opt, mcpp::toolchain::no_escape);
283+
EXPECT_FALSE(names_dir(off, (payload.root / "lib").string()));
284+
}
285+
209286
TEST(HostFlags, DeploymentTargetOnlyOnMacos) {
210287
auto tc = tc_for(CompilerId::GCC);
211288
HostFlagOptions opt;

0 commit comments

Comments
 (0)