Skip to content

Commit 7313b52

Browse files
committed
fix(cli,build): stop the option pre-scan at --; cap BMI stamp masking at two
—— 自我复审抓到的两处 `--jobs` 走的是 `--offline` 那条 env 侧信道,而那个预扫描会一路扫完整个 argv, 于是 `mcpp run -- -j 4` 里属于**被运行程序**的 `-j` 被 mcpp 当成了自己的并发设置。 `-j` 是个足够常见的 flag,这是「什么时候撞上」而不是「会不会撞上」的问题。 预扫描遇到裸 `--` 即停;`--quiet`/`--offline` 同样受益(它们本来也不该越过分隔符)。 `bmi_equivalent` 只掩蔽长得像时间戳的字节,但没限制**数量** —— 用户代码里一个 形如 `"buildtime: 2020/01/01 00:00:00 UTC"` 的字符串常量也会被掩掉, 于是改动它不会传播给导入者。实测真实 BMI 从 10 KiB 到 645 KiB 都**恰好 2 处** (一个 buildtime + 一个 localtime),超过就说明来源不是 GCC 的头部,退回严格比较。 测试:新增 e2e 231(`--jobs N|auto` 生效、坏值必须**告警而非静默降级**、 `--` 之后的参数必须原样送达程序且 mcpp 不得解读), 单测新增 `MoreStampsThanGccEmitsFallsBackToStrictCompare`(两侧都钉: 两处必须掩、三处必须不掩 —— 只钉一侧的话「什么都不掩」的实现也能通过)。 本地:e2e 230/231 通过,test_bmi_equivalent 9/9 通过。
1 parent 232da7a commit 7313b52

4 files changed

Lines changed: 103 additions & 1 deletion

File tree

src/build/stage.cppm

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,13 @@ bool looks_like_stamp(std::string_view v) {
225225
&& digit(17) && digit(18) && v.substr(19) == " UTC";
226226
}
227227

228+
// GCC writes exactly one `buildtime:` and one `localtime:` into a BMI header —
229+
// verified across BMIs from 10 KiB to 645 KiB, always 2. Anything beyond that
230+
// came from somewhere else (a string literal in user code that happens to look
231+
// like a stamp), and masking it would hide a REAL difference. Finding more than
232+
// this many makes the comparison fall back to strict equality.
233+
constexpr std::size_t kMaxStampSpans = 2;
234+
228235
// Byte spans to ignore, in ascending order. Only spans whose payload actually
229236
// looks like a timestamp are masked — a prefix that happens to appear in some
230237
// other position is left to compare strictly.
@@ -265,7 +272,7 @@ bool bmi_equivalent(const std::filesystem::path& a, const std::filesystem::path&
265272
// Disagreement about WHERE the stamps are is itself a structural
266273
// difference; fall back to strict equality rather than guessing.
267274
if (sa != sb) return *da == *db;
268-
if (sa.empty()) return *da == *db;
275+
if (sa.empty() || sa.size() > kMaxStampSpans) return *da == *db;
269276

270277
std::size_t cursor = 0;
271278
for (const auto& [start, end] : sa) {

src/cli.cppm

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ int run(int argc, char** argv) {
106106
// the App below so they show up in --help and pass schema checks.
107107
for (int i = 1; i < argc; ++i) {
108108
std::string_view a = argv[i];
109+
// Everything after a bare `--` belongs to the program being run or the
110+
// test binary being invoked, not to mcpp. Without this, `mcpp run -- -j 4`
111+
// reads the child's flag as mcpp's own concurrency setting — `-j` is a
112+
// common enough flag that this is a matter of when, not whether.
113+
if (a == "--") break;
109114
if (a == "--quiet" || a == "-q") mcpp::ui::set_quiet(true);
110115
else if (a == "--no-color") mcpp::ui::disable_color();
111116
else if (a == "--verbose" || a == "-v") mcpp::log::set_verbose(true);

tests/e2e/231_jobs_option.sh

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#!/usr/bin/env bash
2+
# `--jobs N|auto` and the `--` boundary that keeps it from eating a program's flags.
3+
#
4+
# Two separate contracts, both easy to break without noticing:
5+
# 1. the option is honoured and a bad value is REPORTED, not silently dropped
6+
# (a typo that quietly restores the default is a build mysteriously slower
7+
# than the user asked for);
8+
# 2. `-j` is a common enough flag on other programs that `mcpp run -- -j 4`
9+
# must reach the child untouched. `--jobs` reaches its consumer through the
10+
# MCPP_JOBS side channel, and that pre-scan used to walk the whole argv.
11+
set -e
12+
13+
TMP=$(mktemp -d)
14+
trap "rm -rf $TMP" EXIT
15+
cd "$TMP"
16+
17+
cat > mcpp.toml <<'EOF'
18+
[package]
19+
name = "jobsopt"
20+
version = "0.1.0"
21+
22+
[toolchain]
23+
default = "gcc@16.1.0"
24+
macos = "llvm@22.1.8"
25+
windows = "llvm@20.1.7"
26+
EOF
27+
mkdir -p src
28+
cat > src/main.cpp <<'EOF'
29+
#include <cstdio>
30+
int main(int argc, char** argv) {
31+
for (int i = 1; i < argc; ++i) std::printf("%s\n", argv[i]);
32+
}
33+
EOF
34+
35+
# 1. A numeric value builds.
36+
"$MCPP" build --release --jobs 2 > "$TMP/j2.txt" 2>&1 \
37+
|| { echo "--jobs 2 failed:"; cat "$TMP/j2.txt"; exit 1; }
38+
39+
# 2. `auto` builds too. Its value depends on the host, so the assertion is that
40+
# it is ACCEPTED — asserting a particular number would encode this machine.
41+
"$MCPP" build --release --jobs auto > "$TMP/jauto.txt" 2>&1 \
42+
|| { echo "--jobs auto failed:"; cat "$TMP/jauto.txt"; exit 1; }
43+
if grep -qi 'invalid job count' "$TMP/jauto.txt"; then
44+
echo "'auto' was rejected as an invalid job count:"; cat "$TMP/jauto.txt"; exit 1
45+
fi
46+
47+
# 3. A bad value must WARN and still build (degrading to the backend default).
48+
# Asserted from both sides: a silent drop and a hard failure are both wrong.
49+
"$MCPP" build --release --jobs bogus > "$TMP/jbad.txt" 2>&1 \
50+
|| { echo "a bad --jobs value should warn, not fail the build:"; cat "$TMP/jbad.txt"; exit 1; }
51+
grep -qi 'invalid job count' "$TMP/jbad.txt" \
52+
|| { echo "a bad --jobs value was accepted silently:"; cat "$TMP/jbad.txt"; exit 1; }
53+
54+
# 4. THE BOUNDARY. Everything after `--` belongs to the program.
55+
"$MCPP" run -- -j bogus > "$TMP/sep.txt" 2>&1 \
56+
|| { echo "run with trailing program args failed:"; cat "$TMP/sep.txt"; exit 1; }
57+
grep -qx -- '-j' "$TMP/sep.txt" || { echo "'-j' did not reach the program:"; cat "$TMP/sep.txt"; exit 1; }
58+
grep -qx -- 'bogus' "$TMP/sep.txt" || { echo "'bogus' did not reach the program:"; cat "$TMP/sep.txt"; exit 1; }
59+
# ...and mcpp must not have interpreted it as its own concurrency setting.
60+
if grep -qi 'invalid job count' "$TMP/sep.txt"; then
61+
echo "mcpp consumed a flag that belonged to the program:"; cat "$TMP/sep.txt"; exit 1
62+
fi
63+
64+
echo "jobs option OK"

tests/unit/test_bmi_equivalent.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,29 @@ TEST(BmiEquivalent, MissingFileIsNotEquivalent) {
105105
auto a = write(d / "e1.gcm", bmi_like("2026/08/12 02:25:01 UTC"));
106106
EXPECT_FALSE(mcpp::build::stage::bmi_equivalent(a, d / "does-not-exist.gcm"));
107107
}
108+
109+
// A real BMI carries exactly two stamps — one `buildtime:`, one `localtime:` —
110+
// checked across BMIs from 10 KiB to 645 KiB. A third one did not come from
111+
// GCC's header, so it is not mcpp's to ignore: masking a timestamp-shaped
112+
// string literal in user code would hide a change the importers must see.
113+
//
114+
// Asserted from BOTH sides. Checking only that the extra span is not masked
115+
// would also pass an implementation that masks nothing at all, which is the
116+
// bug this whole function exists to fix.
117+
TEST(BmiEquivalent, MoreStampsThanGccEmitsFallsBackToStrictCompare) {
118+
const auto d = tmpdir();
119+
// Two stamps: masked, so the differing clock does not count.
120+
auto two_a = write(d / "t1.gcm", bmi_like("2026/08/12 02:25:01 UTC"));
121+
auto two_b = write(d / "t2.gcm", bmi_like("2026/08/12 02:25:09 UTC"));
122+
EXPECT_TRUE(mcpp::build::stage::bmi_equivalent(two_a, two_b));
123+
124+
// The same two, plus a third stamp-shaped string in the payload that
125+
// differs. It must NOT be masked, so the two files are different.
126+
auto three_a = write(d / "u1.gcm",
127+
bmi_like("2026/08/12 02:25:01 UTC",
128+
"buildtime: 2020/01/01 00:00:00 UTC"));
129+
auto three_b = write(d / "u2.gcm",
130+
bmi_like("2026/08/12 02:25:09 UTC",
131+
"buildtime: 2020/01/02 00:00:00 UTC"));
132+
EXPECT_FALSE(mcpp::build::stage::bmi_equivalent(three_a, three_b));
133+
}

0 commit comments

Comments
 (0)