Skip to content

Commit 56ebace

Browse files
SunrisepeakSunrisepeak
committed
fix(build): verify fingerprint-scoped staging by size, so a read-denying holder can't fail the build
The Windows job's own repro exposed the gap: my holder maps the destination with FileShare.None, so `same_content` cannot even OPEN it → equivalence is undecidable → mcpp attempts the write → ERROR_SHARING_VIOLATION(32) → build failed. Which is precisely the #311 outcome, arrived at from the other side. Reading needs an open; SIZE comes from directory metadata and does not. So the verify mode is now per-edge rather than one global setting: std BMI / std.o / std.compat.* --verify size fingerprint-scoped: the cache dir and the build dir share the fp covering compiler identity, triple, stdlib, std source hash and dialect flags ⇒ equal size IS equivalence Windows DLL deploy / runtime_alias content not fp-scoped, and a rebuilt DLL keeps its size (PE sections are page-padded) So #311's actual path — clangd mapping the std BMI — now survives even an EXCLUSIVE lock, while a possibly-stale DLL is still gated byte-for-byte. Carried on a per-edge `$verify` ninja variable; the separating space lives in the rule's command string because ninja trims trailing whitespace in variable values. Verified on Linux with chmod 000 (the closest analogue of a read-denying holder): the fp-scoped edge skips and the build succeeds. Also: bootstrap pin 2026.7.29.1 → 2026.7.29.2. Unrelated to this work and pre-existing on main — the index no longer serves .1 (`available: 2026.7.29.2`), so the aarch64 fresh-install job fails on main too. .2 is released and indexed, and the guard's rule (both pins equal, never newer than the building version) still holds.
1 parent ffc29ed commit 56ebace

6 files changed

Lines changed: 66 additions & 10 deletions

File tree

‎.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,20 @@ staged,理由是 fp 判据;但同一条 rule 还搬 **DLL**(`runtimeDeployFiles`
229229
的是正确性。`--verify size` / `MCPP_STAGE_VERIFY=size` 保留给「调用方确知源是 fp 作用域」的
230230
快路径。选逐字节比较而不是 hash:同样的 I/O 成本,但没有碰撞面,且能提前退出。
231231

232+
**但 verify 档位是 per-edge 的,不是全局一档**(这一条也是 Windows CI 逼出来的)。判等要读目标
233+
就必须**打开**它,而持有者可以连读都不给:e2e 171 的 PowerShell 用 `FileShare.None` 映射,
234+
`same_content` 连 open 都失败 → 判不出等价 → 去写 → `ERROR_SHARING_VIOLATION(32)` → 构建挂。
235+
而 **size 来自目录元数据,不需要 open**,所以:
236+
237+
| edge | verify | 理由 |
238+
|---|---|---|
239+
| std BMI / std.o / std.compat.* | `--verify size` | fp 作用域 ⇒ 等长即等价;且能在「连读都被拒」的持有下照样跳过 |
240+
| Windows DLL 部署 / `runtime_alias` | 内容(默认) | 源不是 fp 作用域,PE 节对齐让等长-不同内容很常见 |
241+
242+
也就是说 #311 那条真正的路径(clangd 映射 std BMI)现在**连排他锁都能扛**,而可能过期的
243+
DLL 仍然逐字节把关。ninja 里用一个 per-edge 变量 `$verify` 承载(注意 ninja 会裁掉变量值的
244+
尾部空白,所以空格必须留在 rule 的 command 串里)。
245+
232246
**为什么步 2 连时间戳都不碰**（这一条实测修正过一次）：
233247

234248
| 做法 | 下游是否被级联 | 之后 edge 状态 |

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
「已等价」由**逐字节比较**判定 —— 无条件正确:内容相同就是不需要写。这个判断只在 ninja 已经认定 edge 脏了才会跑,所以代价可以忽略。(`--verify size` 保留给确知源是 fingerprint 作用域的调用方:build dir 与缓存目录共享同一个 fp,而 fp 已覆盖编译器身份/target triple/stdlib/std 源哈希/标准与方言 flag。但它**不是默认** —— 同一条 rule 还搬 DLL,而 PE 的节对齐让「真的重建了、大小却一样」十分常见。)dep BMI 缓存一直就是「已存在就不动」的(`bmi_cache.cppm`: *"Existing project outputs are left untouched"*)—— std staging 是全仓库唯一强制覆写的那一处,这个不对称本身就是缺陷。
1717

18+
**verify 档位是 per-edge 的**:std BMI / `std.o` / `std.compat.*` 走 `--verify size`,Windows DLL 部署与 `runtime_alias` 走默认的逐字节比较。原因是判等要读目标就必须 open 它,而持有者可以连读都不给(Windows `FileShare.None` 映射 → open 即 `ERROR_SHARING_VIOLATION`);size 来自目录元数据、不需要 open。于是 #311 那条真实路径(clangd 映射 std BMI)**连排他锁都扛得住**,而可能过期的 DLL 仍逐字节把关 —— 前者靠 fingerprint 作用域保证等长即等价,后者没有这个保证。
19+
1820
同一条 rule 也用于 **Windows 运行期 DLL 部署**,以及 Windows 上的 `runtime_alias`(PE 没有 soname 符号链接,别名就是刚建出来的 DLL 的副本),因此「往上一次 `mcpp run` 还加载着的 DLL 上覆写」这个同类失败一并治好。POSIX 的 `runtime_alias` 保持符号链接不变 —— 那里符号链接是语义,不只是写法。
1921

2022
- **重新 stage 一个未变的 std BMI 不再重编整张模块图。** staged BMI 是每个 importer 的 implicit input,而 staging rule 既不保留 mtime 也没有 `restat`,于是缓存侧 BMI 只要 mtime 变新(在下面那个缓存根缺陷下,**换个 cwd 跑就会发生**),所有 `import std` 的 TU 全部重编 —— 即使字节完全相同。

‎src/build/ninja_backend.cppm‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,21 @@ std::string emit_ninja_string(const BuildPlan& plan) {
440440
// that clangd had memory-mapped failed the whole build with error 1224.
441441
// `restat = 1` is what makes the no-write path actually pay off — a skipped
442442
// stage must not dirty every importer of the staged BMI.
443+
// $verify is per-edge: empty (⇒ content comparison, the safe default) for
444+
// payloads whose bytes can change under an unchanged name — a rebuilt DLL
445+
// keeps its size, PE sections are page-padded — and `--verify size` for the
446+
// std artifacts, which are fingerprint-scoped: the cache dir and this build
447+
// dir share the fp that covers compiler identity, target triple, stdlib,
448+
// std source hash and the dialect flags, so equal size IS equivalence.
449+
//
450+
// That distinction is not a micro-optimization: reading the destination
451+
// needs to OPEN it, and a holder may deny even that (Windows CI: a
452+
// MemoryMappedFile opened with FileShare.None → ERROR_SHARING_VIOLATION on
453+
// open, so content can't be compared and staging fails). Size comes from
454+
// directory metadata, which stays readable — so the fingerprint-scoped
455+
// edges survive a lock that would otherwise be unsurvivable.
443456
append("rule stage_file\n");
444-
append(" command = $mcpp stage --output $out $in\n");
457+
append(" command = $mcpp stage $verify --output $out $in\n");
445458
append(" description = STAGE $out\n");
446459
append(" restat = 1\n\n");
447460

@@ -821,6 +834,11 @@ std::string emit_ninja_string(const BuildPlan& plan) {
821834
}
822835

823836
// Stage prebuilt std artifacts into the compiler-specific BMI cache.
837+
// These four are fingerprint-scoped (see the stage_file rule comment), so
838+
// they carry the size-only equivalence check.
839+
// (ninja trims trailing whitespace in variable values, hence the space
840+
// lives in the rule's command string, not here)
841+
static constexpr std::string_view kFpScopedVerify = " verify = --verify size\n";
824842
auto std_bmi_dst = mcpp::toolchain::staged_std_bmi_path(plan.toolchain, {});
825843
auto std_o_dst = std::filesystem::path("obj")
826844
/ std::format("std{}", dial.objExt);
@@ -829,8 +847,11 @@ std::string emit_ninja_string(const BuildPlan& plan) {
829847
if (has_std_artifacts) {
830848
append(std::format("build {} : stage_file {}\n", escape_ninja_path(std_bmi_dst),
831849
escape_ninja_path(plan.stdBmiPath)));
832-
append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(std_o_dst),
850+
append(std::string(kFpScopedVerify));
851+
append(std::format("build {} : stage_file {}\n", escape_ninja_path(std_o_dst),
833852
escape_ninja_path(plan.stdObjectPath)));
853+
append(std::string(kFpScopedVerify));
854+
append("\n");
834855
}
835856

836857
bool has_std_compat = !plan.stdCompatBmiPath.empty() && !plan.stdCompatObjectPath.empty();
@@ -844,8 +865,11 @@ std::string emit_ninja_string(const BuildPlan& plan) {
844865
append(std::format("build {} : stage_file {} | {}\n", escape_ninja_path(compat_bmi_dst),
845866
escape_ninja_path(plan.stdCompatBmiPath),
846867
escape_ninja_path(std_bmi_dst)));
847-
append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(compat_o_dst),
868+
append(std::string(kFpScopedVerify));
869+
append(std::format("build {} : stage_file {}\n", escape_ninja_path(compat_o_dst),
848870
escape_ninja_path(plan.stdCompatObjectPath)));
871+
append(std::string(kFpScopedVerify));
872+
append("\n");
849873
}
850874

851875
auto bmi_path = [&traits](std::string_view name) {

‎tests/e2e/170_bmi_staging_no_cascade.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ done
7171
touch "$SRC"
7272
"$MCPP" build -v > build2.log 2>&1 || { cat build2.log; echo "FAIL: rebuild after re-stage"; exit 1; }
7373

74-
grep -q "stage --output" build2.log || {
74+
grep -qE "stage .*--output" build2.log || {
7575
cat build2.log; echo "FAIL: the staging edge did not re-run at all"; exit 1; }
7676
if grep -qE '(-c|/c) .*main\.cpp' build2.log; then
7777
cat build2.log

‎tests/e2e/171_bmi_staging_locked_dest.sh‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@
1414
# through a sentinel file, so a PowerShell that never started fails the test
1515
# instead of letting it pass for the wrong reason.
1616
#
17+
# Note the holder is STRICTER than clangd: CreateFromFile(path, FileMode.Open)
18+
# takes FileShare.None, so the destination cannot even be OPENED — content
19+
# comparison is impossible. Surviving that is why the std staging edges verify
20+
# by SIZE (fingerprint-scoped ⇒ equal size is equivalence, and size comes from
21+
# directory metadata, which needs no open). Passing here therefore implies the
22+
# real clangd case, which only denies writes.
23+
#
1724
# The load-bearing assertion is platform-neutral and root-proof: an equivalent
1825
# destination must not be written AT ALL (same inode, same mtime, same size).
1926
# Permissions alone would not do — root ignores them, and the container e2e job
@@ -100,7 +107,7 @@ if [[ $rc -ne 0 ]]; then
100107
echo "FAIL: staging an already-equivalent BMI failed while the destination was held"
101108
exit 1
102109
fi
103-
grep -q "stage --output" build2.log || {
110+
grep -qE "stage .*--output" build2.log || {
104111
cat build2.log; echo "FAIL: the staging edge did not run"; exit 1; }
105112

106113
# The real assertion: nothing was written. Not "the write succeeded anyway".

‎tests/unit/test_ninja_backend.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ TEST(NinjaBackend, StageRuleRunsThroughMcppWithRestat) {
807807
auto ninja = emit_ninja_string(plan);
808808

809809
EXPECT_NE(ninja.find("rule stage_file\n"), std::string::npos) << ninja;
810-
EXPECT_NE(ninja.find(" command = $mcpp stage --output $out $in\n"),
810+
EXPECT_NE(ninja.find(" command = $mcpp stage $verify --output $out $in\n"),
811811
std::string::npos) << ninja;
812812
// restat is what keeps a skipped (already-equivalent) stage from dirtying
813813
// every importer of the staged BMI.
@@ -876,19 +876,28 @@ TEST(NinjaBackend, StdArtifactsAndRuntimeDllsUseTheStageRule) {
876876

877877
auto ninja = emit_ninja_string(plan);
878878

879+
// The std artifacts are fingerprint-scoped, so they carry the size-only
880+
// check: it needs no OPEN of the destination, which is what lets staging
881+
// survive a holder that denies reads (Windows FileShare.None).
879882
EXPECT_NE(ninja.find("build pcm.cache/std.pcm : stage_file "
880-
"/cache/bmi/fp/pcm.cache/std.pcm"),
883+
"/cache/bmi/fp/pcm.cache/std.pcm\n"
884+
" verify = --verify size\n"),
881885
std::string::npos) << ninja;
882-
EXPECT_NE(ninja.find("build obj/std.o : stage_file /cache/bmi/fp/std.o"),
886+
EXPECT_NE(ninja.find("build obj/std.o : stage_file /cache/bmi/fp/std.o\n"
887+
" verify = --verify size\n"),
883888
std::string::npos) << ninja;
884889
// std.compat keeps its order-only dependency on the staged std BMI.
885890
EXPECT_NE(ninja.find("build pcm.cache/std.compat.pcm : stage_file "
886891
"/cache/bmi/fp/pcm.cache/std.compat.pcm | pcm.cache/std.pcm"),
887892
std::string::npos) << ninja;
888893
// Windows DLL deployment shares the rule — and therefore the
889894
// skip-if-equivalent behaviour that keeps a loaded DLL from failing a build.
890-
EXPECT_NE(ninja.find("build bin/libfoo.dll : stage_file /pkg/lib/libfoo.dll"),
891-
std::string::npos) << ninja;
895+
// But NOT the size-only shortcut: a rebuilt DLL keeps its size (PE sections
896+
// are page-padded), so this edge must compare content.
897+
auto dllEdge = ninja.find("build bin/libfoo.dll : stage_file /pkg/lib/libfoo.dll\n");
898+
ASSERT_NE(dllEdge, std::string::npos) << ninja;
899+
auto afterDll = ninja.substr(dllEdge, ninja.find('\n', dllEdge + 1) - dllEdge + 1);
900+
EXPECT_EQ(afterDll.find("verify"), std::string::npos) << afterDll;
892901
}
893902

894903
// ── #311: staging failures must stay readable through the output filter ──

0 commit comments

Comments
 (0)