diff --git a/.agents/docs/2026-09-04-four-gaps-after-the-ecosystem-batch.md b/.agents/docs/2026-09-04-four-gaps-after-the-ecosystem-batch.md new file mode 100644 index 00000000..a7e6e597 --- /dev/null +++ b/.agents/docs/2026-09-04-four-gaps-after-the-ecosystem-batch.md @@ -0,0 +1,359 @@ +# Four gaps left by the ecosystem batch, and what to do about each + +2026-09-04. The batch described in +`2026-09-04-named-runners-and-the-universal-command-surface.md` delivered four of +its six numbered batches in full. This note covers what is left: two items that +were in scope and were not finished, and two defects the batch surfaced and did +not close. Section 1 is the root-cause work for the fourth, because the plan for +it depends on the analysis. + +Each section states what is ESTABLISHED by measurement separately from what is +INFERRED, because one of these four has a signature and no stack trace. + +--- + +## 1. Gap 4 analysed: the ninja that spins with nothing to do + +### 1.1 What is established + +Observed three times, in three separate fresh sandboxes, always on the `mcpp run` +that follows a `mcpp build` in the same project, on `riscv64-none-elf` and +`aarch64-none-elf`: + +| observation | reading | +|---|---| +| CPU | 99.9% of one core, sustained (utime 165168 ticks over 1629s elapsed) | +| child processes | zero | +| system time | zero | +| `/proc//syscall` | empty — the process is not in a syscall | +| the graph | 8 edges, every declared output present, no missing input, no future mtime | +| the same project on the host | builds and runs in about two seconds | + +A ninja that is neither running commands nor making system calls, on a graph +whose outputs all exist, is not working. It is looping in its own scheduler. + +### 1.2 What that signature narrows it to + +ninja's build loop is, in shape: + +``` +while (plan.more_to_do()) { + edge = plan.FindWork(); + if (edge) { StartEdge(edge); continue; } + if (commands_running) WaitForCommand(); // a syscall + else break; +} +``` + +Zero children and zero system time exclude both branches that leave user space. +What remains is a loop in which `more_to_do()` stays true and `FindWork()` keeps +returning an edge whose completion never decrements the outstanding count. For +that to burn CPU without spawning anything, the edge must be one ninja finishes +WITHOUT running a command. + +**In this graph exactly one edge finishes without a command: the phony.** + +``` +build _mcpp_staged_cache : phony obj/…/riscv_virt_rt.m.o pcm.cache/….pcm +build obj/main.cpp.ddi : cxx_scan …/src/main.cpp || _mcpp_staged_cache +build obj/main.o : cxx_object …/src/main.cpp | obj/main.cpp.ddi.dd \ + || _mcpp_staged_cache +``` + +Its two inputs are `stage_file` edges, and `stage_file` is the one rule in mcpp's +generated manifest whose command is REGULARLY SKIPPED while succeeding: +`mcpp stage` does not write when the destination is already equivalent, which is +the case the rule's `restat = 1` exists to exploit — a skipped stage must not +dirty every importer of the staged BMI. + +So the shape present here is: **restat edges whose outputs do not change, feeding +a phony, consumed as an order-only dependency.** That is the narrowest +description of the suspect, and every part of it is mcpp's own construction +rather than something a user wrote. + +### 1.3 What is inferred and not proven + +That this shape CAUSES the loop is inference, not measurement. Three attempts to +obtain a stack all failed, and the failure of each is itself informative: + +* `gdb -p` is refused: `ptrace_scope = 1`, and the spinning ninja is not a + descendant of any shell that can attach to it. +* `perf record -p` is refused: `perf_event_paranoid = 4`. +* Wrapping ninja so that gdb is its PARENT — which is legal under yama level 1 — + makes the spin stop happening. Under gdb the same invocation ran seven + subprocesses and exited normally. + +The last of those is the important one. It says the trigger is timing- or +environment-sensitive rather than a pure function of the graph, which is +consistent with a scheduler bookkeeping race and inconsistent with "this manifest +always loops". + +Two further measurements bound the suspect from the other side: + +* Both ninja binaries, run 60 times each over the completed graph by hand, never + hung and never took longer than 500ms. A no-op run of this manifest is not + sufficient to trigger it. +* The same project, built and then run on the host, completes in two seconds, + and its `build.ninja` carries the same two `stage_file` edges and the same + phony. The graph shape alone is not sufficient either. + +### 1.4 A separate defect found while looking, which is fully established + +**mcpp does not kill the ninja it spawned.** Every `timeout`-terminated `mcpp +run` left an orphan spinning at 100% of a core. One of them outlived the removal +of the entire sandbox it belonged to; its working directory read `(deleted)` and +it was still burning a core half an hour later. + +This is deterministic, has nothing to do with the spin's cause, and makes the +spin far more expensive than it would otherwise be: any CI that wraps mcpp in +`timeout` leaks a busy core per timeout. + +### 1.5 A third finding, independent of both + +`xim:ninja@1.12.1` does not name one artefact. The descriptor promises +xlings-res's glibc-static build (`ninja-1.12.1-linux-x86_64.tar.gz`); this host +has that one, at 2202320 bytes, statically linked. Several sandboxes have a +273768-byte dynamically linked binary built with GCC 4.8.5 on Red Hat — the +shape and build date (2024-05-11, ninja 1.12.1's release day) of upstream's +official `ninja-linux.zip`. + +Both answer `--version` with `1.12.1`, and their SHA-256 differ. Neither hung in +the 120-run comparison above, so this is not offered as the cause. It is offered +as a fact that has to be false before any conclusion about "ninja 1.12.1 +behaves like X" can be trusted. + +--- + +## 2. Gap 1: openarch has no 32-bit machine with an address space + +### 2.1 Why the discovery is still owed + +The plan expected the first 32-bit backend to surface a width assumption: +`arch_pte_make_leaf` returns `arch_u64`, and nothing had ever asked whether that +is right on a machine whose page-table entry is 32 bits wide. + +Cortex-M arrived and did not settle it. M-profile has an MPU and no MMU, so the +backend's `pte_impl.cpp` implements the group as functions that exist and refuse, +and the package withholds the `openarch:address-space` capability so that a +consumer needing an address space is refused at resolution rather than at run +time. That is the correct design for that machine, and it means the width +assumption was never executed. + +The machine that settles it is 32-bit AND has paging: ARMv7-A. + +### 2.2 Proposal + +Add `backends/armv7a` implementing all four groups, with the pte group backed by +the ARMv7-A **short-descriptor** format, whose second-level entries are 32 bits. + +The deliverable is not the backend. **The deliverable is the answer to one +question**: can a 32-bit page-table entry be expressed through an interface that +types it as `arch_u64`? + +* If yes — the value fits, the extra bits are ignored, and no caller stores the + return into a machine-width slot — then record that the interface is + width-independent, with the ARMv7-A implementation as the evidence, and the + question is closed rather than open. +* If no — some caller or some table needs the machine's own width — then the + interface changes now, while openarch is at 0.x and four backends exist to + change together, rather than after a fifth consumer has depended on it. + +Either answer is worth the work; only the second changes the interface. + +### 2.3 Shape of the work + +* `backends/armv7a/` with `cpu_impl`, `context`, `trap_impl`, `pte_impl`. +* mcpp already has the target rows (`armv7a-none-eabi`, `armv7a-none-eabihf`), + the matrix cells and e2e `336_armv7a_builds_and_boots.sh`, so the toolchain + side needs nothing. +* qemu `-M virt -cpu cortex-a15` runs it; the existing e2e already boots an + ARMv7-A image. +* Release openarch 0.9.0. The `arch_trap_switch` signature is already frozen by + four implementations, so this backend implements a settled interface. + +The one assertion that must exist: a test that builds a leaf entry, reads it +back, and compares against a hand-written 32-bit descriptor. Without it the +backend can be written to whatever the interface says and the width question +stays unasked a second time. + +Independent of everything else here; blocks nothing. + +--- + +## 3. Gap 2: two board packages provision an emulator they may not run + +### 3.1 What is wrong + +`riscv-virt-rt` and `aarch64-virt-rt` declare + +```toml +[xlings.workspace] +"xim:qemu-riscv" = "9.2.4-1" +``` + +which is the untiered form, meaning `ToolWhen::Always`: provisioned by every verb +that builds. A CI job that compiles firmware and never runs it downloads a +33 MB emulator. `cortex-m-rt` declares the tiered form and does not. + +The engine side of batch 2 shipped all four values (`build`, `run`, `dev`, and +the implicit `always`); only the adoption is incomplete, and it is incomplete in +the two packages that most obviously want `run`. + +### 3.2 Proposal + +```toml +[xlings.workspace] +"xim:qemu-riscv" = { version = "9.2.4-1", when = "run" } +``` + +Release `riscv-virt-rt 0.7.1` and `aarch64-virt-rt 0.2.1`, one index entry each. + +The assertion goes in each package's CI and follows the criterion batch 2 already +settled (§14.4 of the plan): **test what would be installed, not what was +installed.** With `MCPP_NO_AUTO_INSTALL` set, a build must report that it needs +no emulator and a run must report that it needs exactly one. That runs in seconds +and needs no download, so it can sit in the ordinary build job rather than in a +job that has an emulator. + +Both halves are load-bearing. "The build does not install qemu" is also true when +the package is broken and installs nothing at all; the pair distinguishes them. + +Low risk, small, and it removes an inconsistency inside one batch. + +--- + +## 4. Gap 3: `mcpp run` folds every non-zero exit status to 1 + +### 4.1 The current contract + +Both spawn sites in `execute.cppm` end in `return rc == 0 ? 0 : 1`, and the +comment states the intent: 2 means "could not start", 1 means "ran and failed", +distinct from each other on purpose. + +Measured consequences: + +* Hosted: a program whose `main` returns 3 makes `mcpp run` exit 1. +* Freestanding: the same image under qemu directly exits 3; through `mcpp run` + it exits 1. + +So a program's own status is not observable through the command this ecosystem +tells people to type. That matters more after this batch than before it, because +the batch's claim is that running on a device is like running hosted, and a +hosted `run` that cannot report a status is not that. + +### 4.2 Three options + +**A. Pass the child's status through; move mcpp's own failures to 125–127.** +Borrows the convention `env`, `timeout` and `nice` already use and that shells +document: 125 = the tool itself failed, 126 = found but not executable, 127 = not +found. "Could not start" then lands on 126/127 by meaning rather than by +allocation, and every value below 125 belongs to the program. + +Cost: the current 2 stops appearing from these two sites, which is a +compatibility change for anything that tests for it. Scripts testing `!= 0` are +unaffected. + +**B. Pass the child's status through; keep 2 for mcpp's own failures.** +Smaller change, but a program that exits 2 becomes indistinguishable from a +launcher failure — reintroducing exactly the ambiguity the current code was +written to avoid. + +**C. Keep the current contract; expose the real status in the machine +interface.** `mcpp run --format json` (or the existing NDJSON path) carries +`exit_code`, and the human exit stays 0/1/2. + +Cost: the common case still cannot be scripted with `$?`, which is what people +actually do. + +**Recommendation: A.** It is the only one of the three in which the obvious +thing a user types produces the obvious thing they expect, and the convention it +borrows is old enough that 126/127 will read correctly to anyone who meets them. + +### 4.3 What A costs to implement + +* Both sites in `execute.cppm`, plus `mcpp test`'s aggregation, which reports + per-test status already and would report the true code. +* `docs/11-machine-output.md` and its Chinese counterpart: the exit-status table + is part of the machine interface contract and must state the new allocation. + A published contract page that describes the old one is worse than none. +* One e2e asserting a hosted program returning 3 gives 3, and one asserting a + missing runner still gives 127 rather than 3. + +**This is the one item in this note that needs a decision before work starts.** +The other three have a right answer; this one has a trade-off. + +--- + +## 5. Gap 4: what to do about the spin + +Split into two, because one half is established and the other is not. + +### 5.1 Stage one — fix what is proven, now + +**mcpp must terminate the ninja it spawned.** On POSIX, put the child in its own +process group and signal the group; on Windows, a job object with +`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` (the console-control-event route is already +known to be wrong here — it reaches the whole console, which once took down an +entire e2e runner with no `FAIL:` line to explain it). + +The criterion is not "the code calls kill". It is: **after mcpp is terminated +mid-build, no ninja remains.** An e2e that starts a slow build, kills mcpp, waits, +and then asserts that no ninja process survives — asserting on process state, not +on a log line. + +This is worth doing whatever causes the spin, and it converts the spin from +"leaks a core forever" into "one build hangs until its timeout". + +### 5.2 Stage two — instrument, then decide + +The one instrument that works on a process nobody can attach to is ninja's own +`-d explain`, which prints its dirtiness reasoning as it goes. A spinning ninja +that is re-deciding the same edge will emit a repeating line naming that edge, +which either confirms §1.2's suspect or names a different one. + +Concretely: an `MCPP_NINJA_DEBUG` escape hatch that appends `-d explain` and +captures the output, used to re-run the verification until it hangs again. The +sandbox reproduces it about once per run, so a handful of runs suffices. + +Second instrument, if the machine's owner is willing: `ptrace_scope = 0` on the +development box makes `gdb -p` work on the live spinner, which answers it in one +attempt. + +### 5.3 The fix, if §1.2's suspect is confirmed + +The phony exists to give consumers one order-only token to depend on instead of +listing every staged output. If it is implicated, the direct alternative is to +drop it and attach the staged outputs to each consumer's order-only list +directly. That costs a longer edge line per consumer and removes from the plan +the only edge that completes without a command. + +Removing `restat = 1` from `stage_file` is the other obvious candidate and is +the WORSE one: the rule's comment states what restat buys — a skipped stage must +not dirty every importer of the staged BMI — and that benefit is real and +measured. Do not trade it away for a defect whose cause is not yet established. + +### 5.4 Independent, and cheap + +Make `xim:ninja@1.12.1` name one artefact. Whichever build is intended, the +descriptor and the payload should agree, and a machine that already has the other +one should be able to tell. A single-line assertion — the installed binary's +SHA-256 against the descriptor's — would have made §1.5 a report rather than a +discovery. + +--- + +## 6. Order, and what needs an answer + +| # | item | depends on | needs a decision | +|---|---|---|---| +| 5.1 | mcpp kills its ninja | nothing | no | +| 3 | two boards adopt `when = "run"` | nothing | no | +| 5.4 | one artefact per ninja version | nothing | no | +| 2 | openarch ARMv7-A backend | nothing | no | +| 4 | `mcpp run` exit status | — | **yes, before work starts** | +| 5.2 | instrument the spin | 5.1 landing first is convenient, not required | no | +| 5.3 | change the staging graph | 5.2 | not until 5.2 answers | + +Everything except item 4 can start without further input. Item 4 is a +compatibility decision about a published contract, and the recommendation in +§4.2 is a recommendation rather than a conclusion. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df0325e..daa0cf6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,52 @@ > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。 +## [2026.9.4.3] — 2026-09-04 + +### ⭐⭐ `mcpp run` 报告程序自己的退出码 + +在此之前所有非零退出码都被折成 `1`,为的是让 `2` 表示「起不来」以区别于「跑了但 +失败」。区别值得保留,代价不值得:`main` 返回 `3` 的程序让 `mcpp run` 退 `1`, +qemu 报 `3` 的裸机镜像同样到达为 `1`。一条报不出退出码的命令没法写进脚本,而这 +正是 `mcpp run` 的主要用途。 + +取值空间分三段,只有第一段属于程序: + +| 区间 | 含义 | +|---|---| +| `0`–`124` | 程序自己的退出码,原样透传 | +| `125`–`127` | 尝试启动但被拒绝(`127` 找不到 / `126` 不可执行 / `125` 其他) | +| `2` | mcpp 在尝试启动之前就拒绝了(用法、配置、解析) | + +中间那段是 `env`、`timeout`、`nice` 早已在用且被 shell 文档化的取值,所以 `126` +与 `127` 带着惯常含义到达。程序自己也可以退 `125`–`127`,mcpp 不靠数字区分 —— +启动失败一定向 stderr 写出原因,程序自己的退出码从不写。 + +`mcpp test` 不变,仍为 `0`/`1`:它聚合多个程序,没有单一退出码可透传。 + +**兼容性**:mcpp 自身的配置错误仍是 `2`,与其余所有命令一致 —— 变的只有「尝试 +启动后被拒」这一种情况,而那时程序根本没运行。契约写在 `docs/11` §6。 + +### ⚠️⚠️ 没有构建进程能比启动它的 mcpp 活得更久 + +实测:每一次被 `timeout` 终止的 `mcpp run` 都留下一个空转占满一个核的 ninja, +其中一个的工作目录已经是 `(deleted)`、比它所属的整个沙箱活得还久。任何用 +`timeout` 包住 mcpp 的 CI,每超时一次泄漏一个忙核。 + +子进程现在进入**自己的进程组**(Windows 上是 job object),mcpp 在收到 +SIGINT/SIGTERM/SIGHUP 时对该组发 **SIGKILL**。用 SIGKILL 而不是 SIGTERM 是必要 +的:ninja 把信号记进标志位,只在等待子进程处才检查;一个没有命令在跑的 ninja +永远到不了那个检查点,礼貌的信号被记录且永不执行 —— 那正是那些孤儿所处的状态。 + +守卫从单槽改为**多槽登记表**:一个跨构建的 `[hooks]` 命令与构建自己的 ninja 会 +同时被守卫,单槽会让后注册者解除前者的守卫。 + +### `MCPP_NINJA_DEBUG` + +设置后向 ninja 追加 `-d `(如 `explain`)。用于那个还没定因的 ninja 空转: +`ptrace_scope=1` 与 `perf_event_paranoid=4` 都取不到栈,而把 gdb 变成 ninja 的父 +进程之后空转就不再发生 —— `-d explain` 是唯一能对这种进程取证的手段。 + ## [2026.9.4.2] — 2026-09-04 ### ⭐⭐ Cortex-M 有 C 库了:`libdir` 填上,而它的键是**三元组** diff --git a/docs/11-machine-output.md b/docs/11-machine-output.md index bc9c0b80..47566b9a 100644 --- a/docs/11-machine-output.md +++ b/docs/11-machine-output.md @@ -179,7 +179,38 @@ output, and a warning would land in the middle of it. Both spellings are produced from the same source, so they always describe the same thing — one answer, two shapes. -## 6. Stability guarantees +## 6. Exit status + +`mcpp run` REPORTS THE PROGRAM'S OWN EXIT STATUS. Three bands divide the space, +and only the first belongs to the program: + +| range | meaning | +|---|---| +| `0`–`124` | the program ran; this is its own status, passed through unchanged | +| `125`–`127` | the spawn was attempted and refused — `127` not found, `126` found but not executable, `125` anything else | +| `2` | mcpp refused before attempting anything: a usage, configuration or resolution error | + +Until 2026.9.4.3 every non-zero status was folded to `1`, so that `2` could mean +"could not start" as distinct from "ran and failed". The distinction was worth +keeping; the price was not. A program whose `main` returned `3` made `mcpp run` +exit `1`, and a bare-metal image that qemu reported as `3` arrived as `1` as +well — so the command this project tells people to type could not be branched on. + +The middle band is the one `env`, `timeout` and `nice` already use and that +shells document, so `126` and `127` arrive with their usual meanings rather than +as numbers this project allocated. + +A PROGRAM MAY ITSELF EXIT `125`–`127`, AND mcpp DOES NOT TRY TO DISAMBIGUATE BY +NUMBER. What separates the two is that a launcher failure always writes a reason +to stderr and a program's own status never does. A client that must be certain +should read stderr, or use `--format json` where the status is a field rather +than a channel. + +`mcpp test` is unchanged and remains `0` or `1`: it aggregates many programs, so +there is no single status to pass through. Per-test codes are in the JSON +stream's `exit_code` field (§8). + +## 7. Stability guarantees For each `kind`, within a `kindVersion`: @@ -194,7 +225,7 @@ schema — `xlings interface --list` declares 20 capabilities whose `outputSchema` is, for all 20, only `{"exitCode": integer}`, and a client that sees a version number assumes there is a contract behind it. -## 7. Kinds +## 8. Kinds ### `mcpp.env` — where mcpp keeps things @@ -316,13 +347,13 @@ them apart: "impl": "openkal-musl@0.3.5", "origin": "graph" } ] ``` -A field was added rather than `cLibrary` renamed or `mode` widened, because §6 +A field was added rather than `cLibrary` renamed or `mode` widened, because §7 promises that fields are added and never removed and that a field's meaning never changes. ⚠️ **`layers[].interface` changed VALUE for a payload-supplied glibc in 2026.9.1.1** — from `gnu` to `glibc`, and on Windows from `gnu` to `ucrt`. The -field's meaning is unchanged (it still names the implementation), so §6 holds; +field's meaning is unchanged (it still names the implementation), so §7 holds; what changed is that it stopped reporting the triple's env segment, which is a request rather than an implementation and is not the name of any C library. The values are now the ones [14 — The Target Side](14-target-side.md) has always @@ -367,7 +398,7 @@ mcpp test [pattern] [--workspace] --message-format json This stream predates the envelope of §2 and is not wrapped in it: it is NDJSON, one record per test as each finishes, then one summary record per member. A -`--workspace` run ends with one `workspace_summary` record. The §6 guarantees +`--workspace` run ends with one `workspace_summary` record. The §7 guarantees apply to it — fields are added and never removed, and a field's meaning never changes — and the fields below are the contract as of 2026.9.2.1. diff --git a/docs/zh/11-machine-output.md b/docs/zh/11-machine-output.md index db38e6bf..5ba42ad1 100644 --- a/docs/zh/11-machine-output.md +++ b/docs/zh/11-machine-output.md @@ -153,7 +153,32 @@ mcpp cache list --json -> {"root": …, "entries": [ … ]} 两种拼写由同一个来源产出,所以永远描述同一件事:一个答案,两种形状。 -## 6. 稳定性承诺 +## 6. 退出码 + +`mcpp run` 报告程序自己的退出码。整个取值空间分三段,只有第一段属于程序: + +| 区间 | 含义 | +|---|---| +| `0`–`124` | 程序跑过了,这是它自己的退出码,原样透传 | +| `125`–`127` | 尝试启动但被拒绝 —— `127` 找不到,`126` 找到但不可执行,`125` 其他 | +| `2` | mcpp 在尝试启动之前就拒绝了:用法、配置或解析错误 | + +2026.9.4.3 之前,所有非零退出码都被折成 `1`,为的是让 `2` 表示「起不来」以区别于 +「跑了但失败」。这个区别值得保留,代价不值得:`main` 返回 `3` 的程序会让 +`mcpp run` 退 `1`,qemu 报 `3` 的裸机镜像同样到达为 `1` —— 本项目让人使用的这条 +命令因此无法用于分支判断。 + +中间那一段是 `env`、`timeout`、`nice` 早已在用、且被 shell 文档化的取值,所以 +`126` 与 `127` 带着它们惯常的含义到达,而不是本项目分配的编号。 + +**程序自己也可以退 `125`–`127`,mcpp 不试图靠数字区分。** 区分二者的是:启动失败 +一定向 stderr 写出原因,而程序自己的退出码从不写。需要确定的客户端应当读 stderr, +或使用 `--format json` —— 那里退出码是一个字段而不是一条通道。 + +`mcpp test` 不变,仍为 `0` 或 `1`:它聚合多个程序,没有单一退出码可以透传。 +每个测试各自的退出码在 JSON 流的 `exit_code` 字段里(§8)。 + +## 7. 稳定性承诺 对每个 `kind`,在同一 `kindVersion` 内: @@ -166,7 +191,7 @@ mcpp cache list --json -> {"root": …, "entries": [ … ]} capability,其 `outputSchema` 全部只有 `{"exitCode": integer}`,而客户端看到版本号就会 以为背后有契约。 -## 7. 各 kind +## 8. 各 kind ### `mcpp.env` —— mcpp 把东西放在哪 @@ -280,12 +305,12 @@ mcpp why toolchain [--target ] [--toolchain ] --format json "impl": "openkal-musl@0.3.5", "origin": "graph" } ] ``` -是**新增一个字段**而不是给 `cLibrary` 改名或给 `mode` 加取值,因为 §6 承诺字段 +是**新增一个字段**而不是给 `cLibrary` 改名或给 `mode` 加取值,因为 §7 承诺字段 只增不删、且一个字段的含义永不改变。 ⚠️ **2026.9.1.1 起,载荷供给的 glibc 让 `layers[].interface` 的**取值**变了** —— 从 `gnu` 变为 `glibc`,Windows 上从 `gnu` 变为 `ucrt`。字段的**含义**没变(它仍然是 -「哪个实现」),所以 §6 仍然成立;变的是它不再报三元组的 env 段 —— 那是一次请求而 +「哪个实现」),所以 §7 仍然成立;变的是它不再报三元组的 env 段 —— 那是一次请求而 不是一个实现,也不是任何一个 C 库的名字。现在的取值就是 [14 —— 目标侧](14-target-side.md)一直列着的那些,并且包可以在 `cfg(c-abi = …)` 谓词里与它们比较。按字面量 `gnu` 取值的客户端需要更新; @@ -324,7 +349,7 @@ mcpp test [pattern] [--workspace] --message-format json ``` 这条流早于 §2 的信封,也不被信封包裹:它是 NDJSON,每个测试结束时一条记录,随后每个 -成员一条汇总记录。`--workspace` 运行以一条 `workspace_summary` 记录结束。§6 的保证 +成员一条汇总记录。`--workspace` 运行以一条 `workspace_summary` 记录结束。§7 的保证 对它同样成立 —— 字段只增不减,字段含义不变 —— 下表是 2026.9.2.1 时的契约。 每个测试: diff --git a/mcpp.toml b/mcpp.toml index 3543b1cf..74628754 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "2026.9.4.2" +version = "2026.9.4.3" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/modules/platform/src/process.cppm b/modules/platform/src/process.cppm index db751579..18ca8c0a 100644 --- a/modules/platform/src/process.cppm +++ b/modules/platform/src/process.cppm @@ -189,7 +189,7 @@ void stop_background(const BackgroundCommand& child, // user can no longer name. Only one command is guarded at a time; a build owns // at most one. void guard_background_on_signal(const BackgroundCommand& child); -void clear_background_guard(); +void clear_background_guard(const BackgroundCommand& child); // `spawn_error`: run_exec's contract; with it null a refused spawn is // formatted into `output`, as capture_exec does. @@ -596,9 +596,32 @@ int run_exec(const std::vector& argv, for (auto& a : argv) cargv.push_back(const_cast(a.c_str())); cargv.push_back(nullptr); + // THE CHILD GETS ITS OWN PROCESS GROUP, AND mcpp KILLS THAT GROUP IF IT IS + // ITSELF KILLED. + // + // Without this, terminating mcpp leaves the child running. Measured: every + // `timeout`-terminated `mcpp run` left an orphaned ninja spinning at 100% + // of a core, and one of them outlived the removal of the entire sandbox it + // belonged to — its working directory read `(deleted)` and it was still + // burning a core half an hour later. Any CI that wraps mcpp in `timeout` + // leaks a busy core per timeout. + // + // The group rather than the pid, because the child starts children of its + // own: killing ninja alone would leave its compilers behind. + // + // A signal is not enough on its own, which is why the guard sends SIGKILL. + // ninja records a signal in a flag and acts on it where it waits for a + // subprocess; a ninja with no command running never reaches that check, so + // a polite signal is recorded and never obeyed. + posix_spawnattr_t attr; + ::posix_spawnattr_init(&attr); + ::posix_spawnattr_setpgroup(&attr, 0); // 0 ⇒ new group, id == pid + ::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP); + pid_t pid = 0; - if (int sp = ::posix_spawnp(&pid, cargv[0], nullptr, nullptr, cargv.data(), envp.data()); - sp != 0) { + int sp = ::posix_spawnp(&pid, cargv[0], nullptr, &attr, cargv.data(), envp.data()); + ::posix_spawnattr_destroy(&attr); + if (sp != 0) { // Reported once: by the caller when it asked for the errno, here // otherwise. Never dropped — the errno in hand at this line is the // whole difference between "Exec format error" and a blank line. @@ -606,14 +629,42 @@ int run_exec(const std::vector& argv, else std::fputs(spawn_failure(argv.front(), sp).c_str(), stderr); return 127; } + mcpp::platform::unixproc::guard_group_on_signal(pid); int status = 0; while (::waitpid(pid, &status, 0) < 0) { /* EINTR retry */ } + mcpp::platform::unixproc::unguard_group(pid); return normalize_exit_code(status); #else + // THE SAME OWNERSHIP AS THE POSIX BRANCH, EXPRESSED IN THIS PLATFORM'S TERMS. + // + // `std::system` gave the child away: it runs through a cmd.exe mcpp does not + // hold a handle to, so terminating mcpp left the tree running exactly as the + // POSIX branch did before its process group. A job object with + // JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE is the equivalent unit — it takes the + // whole tree, which matters here for the same reason the group does there: + // the child starts compilers of its own. + // + // The command line is built by the same `windows_shell_command_line` the + // rest of this file uses, so the cmd.exe quoting rule has ONE derivation. + // Re-deriving it here is how `/d /s /c` loses an argument. std::string prefix = mcpp::platform::env::build_env_prefix(extraEnv); // wrap only — run_exec inherits stdio on purpose (see finalize_shell_command). - std::string cmd = wrap_for_cmd_c(prefix + command_from_argv(argv)); - return normalize_exit_code(std::system(cmd.c_str())); + std::string cmd = windows_shell_command_line(prefix + command_from_argv(argv)); + auto child = mcpp::platform::winproc::spawn_background(cmd.c_str(), nullptr, 1); + if (!child.ok) { + const int refused = static_cast(child.refused); + if (spawn_error) *spawn_error = refused; + else std::fputs(spawn_failure(argv.front(), refused).c_str(), stderr); + return 127; + } + mcpp::platform::winproc::guard_job_on_signal(child.job); + int code = 127; + mcpp::platform::winproc::wait_background(child.process, &code); + mcpp::platform::winproc::unguard_job(child.job); + // Closes both handles; the child has already exited, so this is cleanup + // rather than a kill. + mcpp::platform::winproc::background_stop(child.job, child.process, 0); + return code; #endif } @@ -652,10 +703,22 @@ RunResult capture_exec( ::posix_spawn_file_actions_addclose(&fa, fds[0]); ::posix_spawn_file_actions_addclose(&fa, fds[1]); + // Owned exactly as `run_exec`'s child is, and for the same reason: this is + // the launcher a FULL build uses, so a `mcpp build` interrupted here is the + // common case rather than the rare one. Fixing only `run_exec` left the + // orphan in place — measured, with the two launchers giving opposite + // answers to the same test. + posix_spawnattr_t attr; + ::posix_spawnattr_init(&attr); + ::posix_spawnattr_setpgroup(&attr, 0); + ::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP); + pid_t pid = 0; - int sp = ::posix_spawnp(&pid, cargv[0], &fa, nullptr, cargv.data(), envp.data()); + int sp = ::posix_spawnp(&pid, cargv[0], &fa, &attr, cargv.data(), envp.data()); + ::posix_spawnattr_destroy(&attr); ::posix_spawn_file_actions_destroy(&fa); ::close(fds[1]); + if (sp == 0) mcpp::platform::unixproc::guard_group_on_signal(pid); if (sp != 0) { ::close(fds[0]); result.exit_code = 127; @@ -671,6 +734,7 @@ RunResult capture_exec( ::close(fds[0]); int status = 0; while (::waitpid(pid, &status, 0) < 0) { /* EINTR retry */ } + mcpp::platform::unixproc::unguard_group(pid); result.exit_code = normalize_exit_code(status); return result; #else @@ -889,11 +953,14 @@ void guard_background_on_signal(const BackgroundCommand& child) { mcpp::platform::unixproc::guard_group_on_signal(child.group); } -void clear_background_guard() { +// Releases THIS child, not the guard as a whole: a build's ninja is guarded at +// the same time as a spanning hook, and disarming everything when either +// finishes would leave the other able to outlive mcpp. +void clear_background_guard(const BackgroundCommand& child) { if constexpr (mcpp::platform::is_windows) - mcpp::platform::winproc::clear_job_guard(); + mcpp::platform::winproc::unguard_job(child.job); else - mcpp::platform::unixproc::clear_group_guard(); + mcpp::platform::unixproc::unguard_group(child.group); } RunResult capture_exec_deadline( diff --git a/modules/platform/src/unix/bounded_process.cppm b/modules/platform/src/unix/bounded_process.cppm index a8403ec2..8c1c000b 100644 --- a/modules/platform/src/unix/bounded_process.cppm +++ b/modules/platform/src/unix/bounded_process.cppm @@ -36,6 +36,7 @@ module; #include // errno, EINTR #include // fcntl O_NONBLOCK #include // nanosleep +#include // fputs, stderr — the out-of-slots diagnostic #if defined(__APPLE__) #include // _NSGetEnviron #endif @@ -142,7 +143,19 @@ void background_stop(long long group, long long graceMs); // // The handler does the minimum that is async-signal-safe: killpg (which is), // then the default action. +// A REGISTRY AND NOT ONE SLOT, BECAUSE THERE IS MORE THAN ONE OWNER. +// +// A spanning `[hooks]` command is guarded for the length of the build, and the +// build's own ninja is guarded for the length of its run — concurrently. With a +// single slot the second registration overwrites the first, so killing mcpp +// mid-build would take down ninja and leave the hook's process running, which +// is the exact outcome the guard exists to prevent. +// +// Fixed capacity and no allocation: the handler reads this array and may not +// allocate. Registering past capacity fails loudly rather than silently +// dropping a group, because a dropped group is an orphan nobody will find. void guard_group_on_signal(long long group); +void unguard_group(long long group); void clear_group_guard(); } // namespace mcpp::platform::unixproc @@ -293,14 +306,23 @@ namespace { // Read by a signal handler, so `volatile sig_atomic_t` and nothing else: the // handler may run between any two instructions and may not lock, allocate, or // call anything that is not async-signal-safe. 0 means "nothing to clean up". -volatile sig_atomic_t g_guardedGroup = 0; +constexpr int kMaxGuardedGroups = 8; +volatile sig_atomic_t g_guardedGroups[kMaxGuardedGroups] = {}; extern "C" void background_signal_handler(int sig) { - const auto group = g_guardedGroup; // killpg is async-signal-safe. SIGKILL rather than SIGTERM: this is the // path where mcpp is about to stop existing, and there is nobody left to // escalate if the group ignores the polite request. - if (group > 0) ::killpg(static_cast(group), SIGKILL); + // + // SIGKILL also settles a case SIGTERM does not. ninja records a signal in a + // flag and acts on it only where it waits for a subprocess; a ninja with no + // command running never reaches that check, so a polite signal is recorded + // and never obeyed. Measured: an orphaned ninja spinning at 100% of a core + // outlived the removal of the entire sandbox it belonged to. + for (int i = 0; i < kMaxGuardedGroups; ++i) { + const auto group = g_guardedGroups[i]; + if (group > 0) ::killpg(static_cast(group), SIGKILL); + } // Die of the signal we were sent, so the exit status is the one the shell // and any outer script expect from a Ctrl-C. ::signal(sig, SIG_DFL); @@ -402,14 +424,45 @@ void background_stop(long long group, long long graceMs) { } void guard_group_on_signal(long long group) { - g_guardedGroup = static_cast(group); - ::signal(SIGINT, background_signal_handler); - ::signal(SIGTERM, background_signal_handler); - ::signal(SIGHUP, background_signal_handler); + if (group <= 0) return; + for (int i = 0; i < kMaxGuardedGroups; ++i) { + if (g_guardedGroups[i] == 0) { + g_guardedGroups[i] = static_cast(group); + ::signal(SIGINT, background_signal_handler); + ::signal(SIGTERM, background_signal_handler); + ::signal(SIGHUP, background_signal_handler); + return; + } + } + // Out of slots. Say so rather than return silently: an unguarded group is + // a process that outlives mcpp, and the whole point of this file is that + // such a process is never acceptable. + std::fputs("mcpp: internal: more than 8 concurrently guarded process " + "groups; the newest is NOT guarded and may outlive mcpp\n", + stderr); +} + +// Removing the group this call owns, rather than clearing the array, is what +// makes concurrent owners safe: a nested or overlapping run must not disarm +// the guard an outer one still needs. +void unguard_group(long long group) { + if (group <= 0) return; + bool any = false; + for (int i = 0; i < kMaxGuardedGroups; ++i) { + if (g_guardedGroups[i] == static_cast(group)) + g_guardedGroups[i] = 0; + else if (g_guardedGroups[i] != 0) + any = true; + } + if (!any) { + ::signal(SIGINT, SIG_DFL); + ::signal(SIGTERM, SIG_DFL); + ::signal(SIGHUP, SIG_DFL); + } } void clear_group_guard() { - g_guardedGroup = 0; + for (int i = 0; i < kMaxGuardedGroups; ++i) g_guardedGroups[i] = 0; ::signal(SIGINT, SIG_DFL); ::signal(SIGTERM, SIG_DFL); ::signal(SIGHUP, SIG_DFL); @@ -431,6 +484,7 @@ BackgroundChild spawn_background(const char* const*, unsigned long, int background_running(long long, int*) { return -1; } void background_stop(long long, long long) {} void guard_group_on_signal(long long) {} +void unguard_group(long long) {} void clear_group_guard() {} #endif diff --git a/modules/platform/src/windows/bounded_process.cppm b/modules/platform/src/windows/bounded_process.cppm index caf7b415..7f525f5a 100644 --- a/modules/platform/src/windows/bounded_process.cppm +++ b/modules/platform/src/windows/bounded_process.cppm @@ -55,6 +55,7 @@ module; #define NOMINMAX #endif #include +#include // fputs, stderr — the out-of-slots diagnostic #endif export module mcpp.platform.windows.bounded_process; @@ -120,6 +121,11 @@ struct BackgroundChild { bool ok = false; unsigned long long job = 0; // HANDLE to the job object unsigned long long process = 0; // HANDLE to the child + // GetLastError() from a refused CreateProcess. Carried rather than dropped + // for the reason the POSIX peer carries errno: the code in hand at the + // failure is the whole difference between "not found" and "not executable", + // and a caller that maps a spawn failure onto an exit status needs it. + unsigned long refused = 0; }; // `inheritStdio == 0` sends the child's output to NUL. A spanning hook writes @@ -150,9 +156,18 @@ void background_stop(unsigned long long job, unsigned long long process, // Ctrl-C. The job already covers process death, so this exists only so that a // deliberate interrupt stops the tree BEFORE mcpp unwinds, rather than as a // side effect of it exiting. +// A REGISTRY AND NOT ONE SLOT, for the reason the POSIX peer gives: a spanning +// `[hooks]` command and the build's own ninja are guarded at the same time, and +// a single slot lets the second registration disarm the first. void guard_job_on_signal(unsigned long long job); +void unguard_job(unsigned long long job); void clear_job_guard(); +// Wait for a child started by `spawn_background` and return its exit code. +// Blocking, so an owned run costs no polling latency; `background_running` stays +// for the supervisor that must not block. +int wait_background(unsigned long long process, int* exitCode); + } // namespace mcpp::platform::winproc namespace mcpp::platform::winproc { @@ -378,16 +393,19 @@ namespace { // Read by a console control handler, which runs on a thread of the OS's // choosing. Only the handle is shared, and closing a job handle is atomic from // the caller's point of view. -volatile unsigned long long g_guardedJob = 0; +constexpr int kMaxGuardedJobs = 8; +volatile unsigned long long g_guardedJobs[kMaxGuardedJobs] = {}; BOOL WINAPI background_console_handler(DWORD) { - const auto job = g_guardedJob; // TerminateJobObject, not CloseHandle: this handler races `background_stop` // on the normal path, and terminating is idempotent while closing the same // handle twice is not. The handle stays valid for whoever closes it. - if (job) - ::TerminateJobObject( - reinterpret_cast(static_cast(job)), 1); + for (int i = 0; i < kMaxGuardedJobs; ++i) { + const auto job = g_guardedJobs[i]; + if (job) + ::TerminateJobObject( + reinterpret_cast(static_cast(job)), 1); + } return FALSE; // FALSE = also run the default handler, i.e. still exit } @@ -445,6 +463,7 @@ BackgroundChild spawn_background(const char* commandLine, | (inheritStdio ? 0u : CREATE_NO_WINDOW), nullptr, (cwd && *cwd) ? cwd : nullptr, &si, &pi); if (!ok) { + out.refused = ::GetLastError(); if (job) ::CloseHandle(job); return out; } @@ -514,15 +533,43 @@ void background_stop(unsigned long long job, unsigned long long process, } void guard_job_on_signal(unsigned long long job) { - g_guardedJob = job; - ::SetConsoleCtrlHandler(background_console_handler, TRUE); + if (!job) return; + for (int i = 0; i < kMaxGuardedJobs; ++i) { + if (g_guardedJobs[i] == 0) { + g_guardedJobs[i] = job; + ::SetConsoleCtrlHandler(background_console_handler, TRUE); + return; + } + } + std::fputs("mcpp: internal: more than 8 concurrently guarded jobs; the " + "newest is NOT guarded and may outlive mcpp\n", stderr); +} + +void unguard_job(unsigned long long job) { + if (!job) return; + bool any = false; + for (int i = 0; i < kMaxGuardedJobs; ++i) { + if (g_guardedJobs[i] == job) g_guardedJobs[i] = 0; + else if (g_guardedJobs[i] != 0) any = true; + } + if (!any) ::SetConsoleCtrlHandler(background_console_handler, FALSE); } void clear_job_guard() { - g_guardedJob = 0; + for (int i = 0; i < kMaxGuardedJobs; ++i) g_guardedJobs[i] = 0; ::SetConsoleCtrlHandler(background_console_handler, FALSE); } +int wait_background(unsigned long long process, int* exitCode) { + HANDLE h = reinterpret_cast(process); + if (!h) return -1; + ::WaitForSingleObject(h, INFINITE); + DWORD code = 0; + if (!::GetExitCodeProcess(h, &code)) return -1; + if (exitCode) *exitCode = static_cast(code); + return 0; +} + #else DeadlineRun capture_with_deadline(const char*, const char* const*, unsigned long, @@ -535,7 +582,9 @@ BackgroundChild spawn_background(const char*, const char*, int) { return {}; } int background_running(unsigned long long, int*) { return -1; } void background_stop(unsigned long long, unsigned long long, long long) {} void guard_job_on_signal(unsigned long long) {} +void unguard_job(unsigned long long) {} void clear_job_guard() {} +int wait_background(unsigned long long, int*) { return -1; } #endif diff --git a/modules/versioning/src/version.cppm b/modules/versioning/src/version.cppm index d7de998e..ac670375 100644 --- a/modules/versioning/src/version.cppm +++ b/modules/versioning/src/version.cppm @@ -31,6 +31,6 @@ import std; export namespace mcpp { -inline constexpr std::string_view MCPP_VERSION = "2026.9.4.2"; +inline constexpr std::string_view MCPP_VERSION = "2026.9.4.3"; } // namespace mcpp diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 12856037..d78e1386 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -5,6 +5,7 @@ module; #include #include +#include // ENOENT/EACCES/ENOEXEC — the launcher status band export module mcpp.build.execute; @@ -42,6 +43,43 @@ import mcpp.ui; namespace mcpp::build { +// ─── The exit status of a run ──────────────────────────────────────── +// +// `mcpp run` REPORTS THE PROGRAM'S OWN EXIT STATUS, AND NOTHING BELOW 125 +// BELONGS TO mcpp. +// +// It used to fold every non-zero status to 1, so that 2 could mean "could not +// start" as distinct from "ran and failed". The distinction was worth keeping; +// the price was not. Measured before the change: a program whose `main` returns +// 3 made `mcpp run` exit 1, and a bare-metal image that qemu reported as 3 +// arrived as 1 as well. A command that cannot report a status is one nobody can +// use in a script, and this ecosystem tells people that running on a device is +// like running hosted. +// +// The band is the one `env`, `timeout` and `nice` already use and that shells +// document, so 126 and 127 arrive with their usual meanings rather than as +// numbers this project allocated: +// +// 127 the program was not found +// 126 it was found and could not be executed (permission, wrong format) +// 125 the launcher itself failed for some other reason +// +// A program that legitimately exits 125-127 is indistinguishable from these, +// which is the residual cost and the reason the message on stderr is not +// optional: a launcher failure always prints why, a program's own status never +// does. +int launcher_status(int spawnErrno) { + switch (spawnErrno) { + case ENOENT: + case ESRCH: return 127; + case EACCES: + case EPERM: + case ENOEXEC: + case EISDIR: return 126; + default: return 125; + } +} + // ─── P0: build cache for fast-path rebuilds ───────────────────────── constexpr std::string_view kBuildCacheFile = "target/.build_cache"; @@ -979,6 +1017,23 @@ std::optional run_ninja_fast(const std::string& ninjaProgram, argv.push_back("-C"); argv.push_back(outputDir.string()); if (verbose) argv.push_back("-v"); + // MCPP_NINJA_DEBUG: append ninja's own `-d` topics. The one instrument that + // works on a process nobody can attach to. + // + // A ninja that spins with no command running has been observed three times + // in fresh sandboxes, and neither gdb nor perf can reach it there + // (ptrace_scope=1, perf_event_paranoid=4); making gdb its parent stops the + // spin happening at all. `-d explain` prints the dirtiness decision as it + // is made, so a ninja re-deciding the same edge names that edge in its own + // output — which is readable from the log the runner already captures. + // + // Unset by default and unset in CI. It changes nothing but ninja's + // verbosity, and it is spelled as a topic list rather than a boolean so + // that `-d stats` and `-d keeprsp` are reachable without another variable. + if (const char* topics = std::getenv("MCPP_NINJA_DEBUG"); topics && *topics) { + argv.push_back("-d"); + argv.push_back(topics); + } std::vector> childEnv; if (runtimeEnvKey == "@env") { @@ -1431,8 +1486,8 @@ std::optional try_fast_run(const std::filesystem::path& projectRoot, })) childEnv.push_back(std::move(kv)); - // Same contract as the prepare path below: a refused spawn is reported - // and exits 2, never folded into the artifact's own exit status (#544). + // Same contract as the prepare path below: a refused spawn is reported and + // exits in the 125-127 band, never folded into the artifact's own status. // No runner can be declared for an entry this path accepts (see the // `runnerDeclared` gate above), so the artifact is the only thing that // could have been refused. @@ -1445,9 +1500,9 @@ std::optional try_fast_run(const std::filesystem::path& projectRoot, std::println(stderr, "error: {}", unrunnable_message(triple, exe, spawnErr)); else std::println(stderr, "error: {}", spawn_failed_message(exe.string(), spawnErr)); - return 2; + return launcher_status(spawnErr); } - return exitRc == 0 ? 0 : 1; + return exitRc; } // `mcpp run` driver: build, locate the binary target, exec it with the @@ -1692,13 +1747,12 @@ export int build_run_target(const std::optional& targetName, // never mcpp or a host shell. Fixes the bundled-glibc-vs-host-libtinfo // crash on newer-glibc distros. // - // A refused spawn is typed and reported here, and exits 2 — the code the - // freestanding no-runner path already uses for "could not start", distinct - // from 1, which keeps meaning "ran and failed". With a runner the failure - // is the runner's own (verbatim errno, no advice); without one, ENOEXEC - // is the kernel saying this host cannot load the artifact, and the message - // carries the key that would change that. Anything else is reported as - // itself — EACCES is a permission problem, not an absence. + // A refused spawn is typed and reported here, and exits in the 125-127 band + // — never inside the range the program itself owns. With a runner the + // failure is the runner's own (verbatim errno, no advice); without one, + // ENOEXEC is the kernel saying this host cannot load the artifact, and the + // message carries the key that would change that. Anything else is reported + // as itself — EACCES is a permission problem, not an absence. int spawnErr = 0; const int rc = mcpp::platform::process::run_exec(argv, childEnv, &spawnErr); if (spawnErr != 0) { @@ -1710,9 +1764,9 @@ export int build_run_target(const std::optional& targetName, unrunnable_message(choice.tripleKey, exe, spawnErr)); else std::println(stderr, "error: {}", spawn_failed_message(exe.string(), spawnErr)); - return 2; + return launcher_status(spawnErr); } - return rc == 0 ? 0 : 1; + return rc; } export enum class TestMessageFormat { Human, Json }; diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 23a3f0a9..7f9a160c 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -2826,6 +2826,23 @@ std::expected NinjaBackend::build(const BuildPlan& plan nargv.push_back(plan.outputDir.string()); if (opts.verbose) nargv.push_back("-v"); + // MCPP_NINJA_DEBUG: append ninja's own `-d` topics. The one instrument that + // works on a process nobody can attach to. + // + // A ninja that spins with no command running has been observed three times + // in fresh sandboxes, and neither gdb nor perf can reach it there + // (ptrace_scope=1, perf_event_paranoid=4); making gdb its parent stops the + // spin happening at all. `-d explain` prints the dirtiness decision as it + // is made, so a ninja re-deciding the same edge names that edge in its own + // output — which is readable from the log the runner already captures. + // + // Unset by default and unset in CI. It changes nothing but ninja's + // verbosity, and it is spelled as a topic list rather than a boolean so + // that `-d stats` and `-d keeprsp` are reachable without another variable. + if (const char* topics = std::getenv("MCPP_NINJA_DEBUG"); topics && *topics) { + nargv.push_back("-d"); + nargv.push_back(topics); + } if (opts.parallelJobs) nargv.push_back(std::format("-j{}", opts.parallelJobs)); diff --git a/src/hooks.cppm b/src/hooks.cppm index bd84c393..6f3e721f 100644 --- a/src/hooks.cppm +++ b/src/hooks.cppm @@ -223,7 +223,7 @@ Span::Span(const mcpp::manifest::Hooks& hooks, // it forked may not be, and the group is addressable only while // the unreaped leader still holds its id. proc::stop_background(child_, std::chrono::milliseconds(0)); - proc::clear_background_guard(); + proc::clear_background_guard(child_); if (stop_.load()) return; // "Failed to stay up" is BOTH halves: short AND unsuccessful. A @@ -260,7 +260,7 @@ void Span::close() { stop_.store(true); if (supervisor_.joinable()) supervisor_.join(); if (started_) { - proc::clear_background_guard(); + proc::clear_background_guard(child_); proc::stop_background(child_, kStopGrace); } } diff --git a/tests/e2e/330_runner_hosted_targets.sh b/tests/e2e/330_runner_hosted_targets.sh index a09b6d39..91284d48 100644 --- a/tests/e2e/330_runner_hosted_targets.sh +++ b/tests/e2e/330_runner_hosted_targets.sh @@ -104,8 +104,20 @@ if [[ "$(uname -s)" == Linux ]]; then bin=$(ls target/*/*/bin/app | head -1) [[ -f "$bin" ]] || fail "no artifact at target/*/*/bin/app" printf '\xff\xff' | dd of="$bin" bs=1 seek=18 conv=notrunc status=none + # 126, NOT 2, AND THE BAND IS THE POINT. + # + # `mcpp run` now reports the program's own exit status, so mcpp's answers had + # to move out of the range a program owns. A refused spawn lands in the band + # `env`, `timeout` and every shell already use: 127 not found, 126 found and + # not executable, 125 anything else. This artifact has a wrecked e_machine, + # so the kernel answers ENOEXEC and the code is 126. + # + # mcpp's own refusals BEFORE a spawn is attempted — no binary target, no + # runner declared, runner program not on PATH — keep exit 2, which is what + # every other mcpp command uses for "cannot do what was asked". The three + # assertions above this one cover that half and are deliberately unchanged. out=$("$MCPP" run 2>&1); rc=$? - [[ $rc -eq 2 ]] || fail "unrunnable artifact: exit $rc, want 2: $out" + [[ $rc -eq 126 ]] || fail "unrunnable artifact: exit $rc, want 126: $out" grep -q "this host cannot execute" <<<"$out" || fail "unrunnable message missing: $out" grep -q "Exec format error" <<<"$out" || fail "the kernel's answer is missing: $out" grep -q "\[target.$HOST\]" <<<"$out" || fail "paste-able key missing: $out" diff --git a/tests/e2e/339_run_reports_the_program_status.sh b/tests/e2e/339_run_reports_the_program_status.sh new file mode 100755 index 00000000..a9e0c43f --- /dev/null +++ b/tests/e2e/339_run_reports_the_program_status.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# requires: gcc unix-shell +# +# `mcpp run` REPORTS THE PROGRAM'S OWN EXIT STATUS. +# +# It used to fold every non-zero status to 1 so that 2 could mean "could not +# start". The distinction was worth keeping; the price was not. Measured before +# the change: a program whose `main` returns 3 made `mcpp run` exit 1, and a +# bare-metal image qemu reported as 3 arrived as 1 as well. A command that +# cannot report a status cannot be used in a script, which is most of what +# `mcpp run` is for. +# +# THREE BANDS, AND THE TEST COVERS ALL THREE: +# +# 0-124 the program's own status, passed through unchanged +# 125-127 the spawn was attempted and refused (127 not found, +# 126 found but not executable, 125 anything else) +# 2 mcpp refused BEFORE attempting anything — a usage or +# configuration error, the same code every other mcpp command uses +# +# The middle band is the shell's, not this project's: `env`, `timeout` and +# `nice` already answer 126/127 with these meanings, so a reader who meets one +# does not have to look it up. +set -euo pipefail +fail() { echo "FAIL: $*"; exit 1; } +MCPP="${MCPP:?set MCPP to the mcpp binary under test}" + +work="$(mktemp -d)"; trap 'rm -rf "$work"' EXIT +cd "$work" +mkdir -p src + +cat > mcpp.toml <<'EOF' +[package] +name = "status" +version = "0.1.0" + +[build] +sources = ["src/main.cpp"] +EOF + +status_of() { # $1 = what main returns + printf 'int main() { return %s; }\n' "$1" > src/main.cpp + set +e + "$MCPP" run > run.log 2>&1 + local rc=$? + set -e + echo "$rc" +} + +# ── 1. the ordinary case: a status a script would branch on ─────────────── +for want in 0 1 3 42 124; do + got=$(status_of "$want") + [[ "$got" == "$want" ]] \ + || fail "main returned $want, mcpp run exited $got (want $want)" +done +echo " ok 0 1 3 42 124 all pass through" + +# ── 2. THE BOUNDARY, BECAUSE THAT IS WHERE A BAND SPLIT GOES WRONG ──────── +# +# A program is allowed to exit 125-127 too. mcpp cannot distinguish that from +# its own answer by the number alone, and does not try: what separates them is +# that a launcher failure always prints a reason to stderr and a program's own +# status never does. Assert the number here and the silence with it, so that a +# future change which starts printing on the success path is caught. +got=$(status_of 127) +[[ "$got" == "127" ]] || fail "main returned 127, mcpp run exited $got" +grep -qiE '^error:' run.log \ + && fail "a program exiting 127 must not produce an mcpp error line: $(cat run.log)" +echo " ok a program may exit 127 itself, and mcpp stays silent about it" + +# ── 3. the spawn band: found, not executable ────────────────────────────── +# +# A file that exists, is marked executable, and is not a program the kernel can +# load. The runner is declared so that the failure is the RUNNER's spawn rather +# than the artefact's, which keeps this case independent of the host triple. +printf 'int main() { return 0; }\n' > src/main.cpp +"$MCPP" build >/dev/null 2>&1 || fail "build for the runner cases" +HOST=$(ls target | head -1) +[[ -n "$HOST" ]] || fail "no target directory to read the host triple from" +printf 'this is not an executable format\n' > not-a-program +chmod +x not-a-program +cat >> mcpp.toml <&1); rc=$? +set -e +[[ $rc -eq 126 ]] || fail "unexecutable runner: exit $rc, want 126: $out" +grep -qiE '^error:' <<<"$out" || fail "a refused spawn must say why: $out" +echo " ok a refused spawn exits 126 and says why" + +# ── 4. the spawn band: not found ────────────────────────────────────────── +sed -i.bak "s|$PWD/not-a-program|$PWD/no-such-runner-at-all|" mcpp.toml +set +e +out=$("$MCPP" run 2>&1); rc=$? +set -e +# mcpp looks a declared runner up before spawning, so this is its own refusal +# (2) rather than the kernel's (127). Both are outside 0-124, which is the +# property a script depends on; the test states which one this is so that a +# change of mind about it is visible rather than silent. +[[ $rc -eq 2 || $rc -eq 127 ]] \ + || fail "missing runner: exit $rc, want 2 (pre-flight) or 127 (spawn): $out" +echo " ok a missing runner exits $rc, outside the program's band" + +echo "PASS: 339 mcpp run reports the program's own exit status" diff --git a/tests/e2e/340_no_orphan_survives_a_killed_mcpp.sh b/tests/e2e/340_no_orphan_survives_a_killed_mcpp.sh new file mode 100755 index 00000000..9851ec57 --- /dev/null +++ b/tests/e2e/340_no_orphan_survives_a_killed_mcpp.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# requires: gcc unix-shell +# +# NO BUILD PROCESS SURVIVES THE mcpp THAT STARTED IT. +# +# Measured before the fix, in the ecosystem sandbox: every `timeout`-terminated +# `mcpp run` left an orphaned ninja spinning at 100% of a core. One of them +# outlived the removal of the entire sandbox it belonged to — its working +# directory read `(deleted)` and it was still burning a core half an hour +# later. Any CI that wraps mcpp in `timeout` leaked a busy core per timeout. +# +# THE SIGNAL GOES TO mcpp ALONE, AND THAT IS THE WHOLE TEST. +# +# `timeout` and an interactive Ctrl-C both signal the process GROUP, which +# reached ninja even before the fix — so a test built on `timeout` passes +# either way and proves nothing. Signalling mcpp's pid alone is what separates +# "the child was told by someone else" from "mcpp took its child down". +# +# SIGKILL RATHER THAN SIGTERM IS WHY THE FIX WORKS AT ALL. ninja records a +# signal in a flag and acts on it where it waits for a subprocess; a ninja with +# no command running never reaches that check, so a polite signal is recorded +# and never obeyed. That is the state the orphans were in. +# +# THE BUILD MUST STILL BE RUNNING WHEN THE SIGNAL ARRIVES. An earlier draft of +# this test killed mcpp after the build had already finished and passed without +# testing anything; the assertion below therefore requires that ninja was seen +# alive first, and fails if it never was. +set -euo pipefail +fail() { echo "FAIL: $*"; exit 1; } +MCPP="${MCPP:?set MCPP to the mcpp binary under test}" + +work="$(mktemp -d)"; trap 'rm -rf "$work"' EXIT +cd "$work" +mkdir -p src + +# Counting by executable name, not by a `pgrep -f` pattern: this script's own +# command line contains the word, and a pattern match finds itself. +count_ninja() { + local n=0 p + for p in /proc/[0-9]*; do + [[ "$(cat "$p/comm" 2>/dev/null)" == "ninja" ]] && n=$((n+1)) + done + echo "$n" +} + +# `-j1` IS WHAT MAKES THE WINDOW DETERMINISTIC, NOT THE NUMBER OF FILES. +# +# An earlier draft used 200 parallel translation units and PASSED AGAINST A +# BUILD THAT HAS THE DEFECT: on a 32-core host the batch finished inside the +# ten seconds between the signal and the check, so nothing survived either way +# and the control could not tell the two apart. Serialising the build makes it +# outlast the check on every host, by construction rather than by hoping the +# machine is slow. +n_tu=60 +for i in $(seq 0 $((n_tu-1))); do + cat > "src/u$i.cpp" < +#include +#include +#include +#include +template struct T_$i { static int v(){ return N + T_$i::v(); } }; +template<> struct T_$i<0> { static int v(){ return 0; } }; +int f$i() { + std::map>> m; + std::set s; + for (int k = 0; k < 800; ++k) { m[std::to_string(k)].push_back({k, std::to_string(k*2)}); s.insert(std::to_string(k*3)); } + std::vector keys; + for (auto& kv : m) keys.push_back(kv.first); + std::sort(keys.begin(), keys.end()); + return (int)m.size() + T_$i<400>::v() + (int)keys.size() + (int)s.size(); +} +EOF +done +printf 'int main() { return 0; }\n' > src/main.cpp + +cat > mcpp.toml <<'EOF' +[package] +name = "orphan" +version = "0.1.0" + +[build] +sources = ["src/*.cpp"] +EOF + +baseline=$(count_ninja) + +"$MCPP" build --jobs 1 >/dev/null 2>&1 & +mcpp_pid=$! + +seen=0 +for _ in $(seq 1 60); do + if [[ "$(count_ninja)" -gt "$baseline" ]]; then seen=1; break; fi + sleep 0.25 +done +[[ "$seen" -eq 1 ]] \ + || { kill -9 "$mcpp_pid" 2>/dev/null || true + fail "ninja never started: this test measured nothing"; } + +sleep 2 +kill -TERM "$mcpp_pid" 2>/dev/null || true +wait "$mcpp_pid" 2>/dev/null || true + +# Long enough that a ninja which merely finished on its own is indistinguishable +# from one that was killed — and short enough that the suite does not stall. The +# build takes far longer than this, so anything still alive here is an orphan. +sleep 8 +after=$(count_ninja) +if [[ "$after" -gt "$baseline" ]]; then + for p in /proc/[0-9]*; do + [[ "$(cat "$p/comm" 2>/dev/null)" == "ninja" ]] \ + && echo " orphan $(basename "$p") cwd=$(readlink "$p/cwd" 2>/dev/null)" + done + fail "ninja survived a killed mcpp: $after alive, baseline $baseline" +fi + +echo " ok ninja was running, mcpp was signalled alone, nothing survived" +echo "PASS: 340 no orphan survives a killed mcpp"