diff --git a/changelog.d/8809-gc-root-dominance-late-roots.md b/changelog.d/8809-gc-root-dominance-late-roots.md new file mode 100644 index 0000000000..c6840aba64 --- /dev/null +++ b/changelog.d/8809-gc-root-dominance-late-roots.md @@ -0,0 +1,11 @@ +Two GC root stores that did not dominate the collection points after them, and the audit failure that hid both. + +`gc-root-dominance` has been red on `main` since 2026-08-15, and every scheduled run failed at the *same* step: `--audit-poll-reach`, which runs before the compiler build. None of the four gated arms below it executed for ten days, and two rooting regressions landed inside that window. Both were `MOVING: YES`-class defects of the #7192 shape (a root store emitted in-frame but after a call that can collect), and neither PR carried `run-extended-tests`, so the opt-in PR arm never ran either. + +* **Object-literal method closures (`expr/object_literal.rs`).** `lower_by_name_props` lowered a `this`-capturing method closure, installed it with `js_object_set_field_by_name`, and only *then* pushed its root. `js_object_set_field_by_name` is a collection point, and the closure is reachable from the object by the time it runs, so an evacuating minor moves it — after which the queued register names from-space, the push publishes a dangling pointer into a slot the collector scans, and the deferred `this`-patch loop writes the receiver into abandoned memory (the surviving copy's method then runs with `this` unset). The install now happens *inside* the closure's rooted scope and re-reads it like any other rooted argument; `RootedAcc::as_arg` is `pub(crate)` for that one shape. When `protect` is false nothing is emitted and the IR is byte-identical, as before. + + This arm was unreachable from TypeScript between #809 and #8793 — every source literal with a method went through the IIFE builder — so the ordering survived the #7192 sweep unexercised. #8793 routes static-key method literals straight to `Expr::Object`, which lands here, and the latent bug became seven live violations across three fingerprints the next morning. The test module's doc no longer claims the branch is unreachable. + +* **Private-method classes (`lower_call/new.rs`).** `construction_runs_user_code` gates three things that must agree — the instance temp root, the `this`-slot bind, and whether `reload_instance` re-reads. A class whose only private elements are methods or accessors declares no fields, no constructor and no heritage, so it answered `false` — while `emit_field_inits` still emits `js_private_brand_add` for it. That helper allocates the marker key and calls `js_object_set_field_by_name`; its own body says the allocation "can evacuate both the receiver and any live value" and opens a `RuntimeHandleScope` for exactly that reason. `new WithPrivateMethod()` therefore fed a stale handle to `js_gc_init_typed_shape_layout` and published it into the caller's root slot. The predicate now includes `has_private_instance_elements()`. + +* **`scripts/gc_root_dominance_check.py`.** `POLL_CAPABLE_RUNTIME` gains the three symbols `--audit-poll-reach` had been naming since 2026-08-15 (`js_builtin_subclass_construct`, `js_tls_create_secure_context`, `js_tls_secure_context_new`), which is what unblocks the gate. It also gains `js_private_brand_add` — the referent-with-no-name neither audit can ask for, because `--audit-poll-reach` only walks symbols `ALLOC_RE` matches and this one matches no alloc/new/create convention. Without it the private-method window classified `MOVING: no` and every `--moving-only` arm dropped it, the same way the emitted property-GET helpers were dropped before #7284. Measured over the curated corpus, that entry reclassifies exactly the one window this change fixes. No allowlist entry was added and no detection was narrowed; `--self-test` still reports its planted violation. diff --git a/changelog.d/8810-gc-root-dominance-corpus-no-link.md b/changelog.d/8810-gc-root-dominance-corpus-no-link.md new file mode 100644 index 0000000000..b37853b3f2 --- /dev/null +++ b/changelog.d/8810-gc-root-dominance-corpus-no-link.md @@ -0,0 +1,7 @@ +**GC-root-dominance corpus:** two GC-callback-rooting sources were silently contributing no IR to the gate that exists to check exactly that. `scripts/gc_root_dominance_corpus.sh` linked every source, and the corpus deliberately sets `PERRY_NO_AUTO_OPTIMIZE=1` — so the only two sources importing a node builtin (`test_gap_gc_net_once_flags_rekey` → `node:net`, `test_gap_gc_http2_pending_event_callback_rooting` → `node:http2`) needed a prebuilt `libperry_ext_{net,http}.a` that the documented build command does not produce. perry auto-built each wrapper in its own cargo invocation, cargo unified features per invocation, and `compile/shared_tokio.rs` correctly refused the resulting two-tokio link (#507/#7629). perry exited non-zero **after** codegen had already written the `.ll`, and the loop's `continue` threw that IR away: 150/152 compiled, 2 skipped, exit 1 on clean `main` — with a skip message that named only the two files, so the finding read as a compiler regression rather than a link-line refusal (#8810). + +The corpus now compiles with `--no-link`. `--trace llvm` is written during codegen, so the link stage was never part of this gate's subject; not linking removes the failure mode instead of tolerating it (no link line to be wrong, no ext archive to be missing, no stale `libperry_runtime.a` deciding what the corpus contains) and stops the script writing 152 executables it never reads. `scripts/compiler_output_harness/repsel_census.py` already compiles with `--no-link` for the same reason. A source that stops *linking* is still a finding — it is `./run_parity_tests.sh`'s, which compiles and runs every `test_gap_*.ts` under the shipping configuration. Codegen failures still exit non-zero and still fail the `MAX_SKIPPED=0` ratchet (verified by sabotage). + +Measured on the shadow lowering, same compiler both arms: **150/152 sources, 177 `.ll` → 152/152 sources, 179 `.ll`, 0 skipped, exit 0**. The 177 shared `.ll` files are byte-identical across the two runs, so `--no-link` changed nothing about the emitted IR. The two new modules add 30 functions and 53 root stores and **zero** new violations: both arms report the same 7 violations over the same 3 fingerprints (tracked in #8809), and `--unrooted-allocas` stays at 0, now over 179 files instead of 177. + +Two follow-ups in the same file: a failed compile now reports its first error line next to the source name (one skip per line), because a skip that names only a file is a finding you have to reproduce locally before you can read it. And `MIN_SOURCES` was raised 131 → 152, the measured discovery count — it had drifted the same way the old `MIN_COMPILED=90` floor did, leaving room for 21 sources to vanish before the "corpus shrank" arm could fire. diff --git a/crates/perry-codegen/src/expr/object_literal.rs b/crates/perry-codegen/src/expr/object_literal.rs index b0762bcb5e..8d2e6bfbbb 100644 --- a/crates/perry-codegen/src/expr/object_literal.rs +++ b/crates/perry-codegen/src/expr/object_literal.rs @@ -20,10 +20,17 @@ //! loop". Reorder those two statements and the leak is silent. //! //! Nesting one `with_rooted_accumulator` per such property expresses the same -//! lifetime as a scope: each value's root spans exactly the suffix of the -//! literal that follows it, the release is owned on every path out (including a -//! `?` from a later initializer, which the flat form leaked), and the value can -//! only be read in `finish` — below the last initializer, above the release. +//! lifetime as a scope: each value's root spans its own installing +//! `js_object_set_field_by_name` and the whole suffix of the literal that +//! follows it, the release is owned on every path out (including a `?` from a +//! later initializer, which the flat form leaked), and the value can only be +//! read in `finish` — below the last initializer, above the release. +//! +//! The install is INSIDE the scope, not above it (#8809). Rooting after it is +//! the #7192 shape: the setter can run a user setter or a Proxy trap, and the +//! closure it just installed is reachable from the object, so an evacuating +//! minor moves it and the register queued above names from-space. +//! //! No fourth combinator: the three existing `with_operands_rooted*` forms all //! lower their own operand list up front, which would evaluate every property //! before storing any of them and reorder observable side effects, and @@ -255,12 +262,6 @@ fn lower_by_name_props<'f>( let this_idx = auto_caps.len() as u32; let v = lower_expr(ctx, value_expr)?; - let key_raw = emit_interned_key_raw(ctx, &key_handle_global); - obj.call_void( - ctx, - "js_object_set_field_by_name", - &[Arg::Plain(I64, &key_raw), Arg::Plain(DOUBLE, &v)], - ); // The closure value is deferred: the patch loop reads it after every // remaining property has been lowered, so it must survive their @@ -268,13 +269,37 @@ fn lower_by_name_props<'f>( // the slot; the register queued above is stale). `build` owns the // rest of the literal, `finish` is the one place the value escapes, // and the release happens on both paths out. + // + // #8809: the root is pushed BEFORE the installing + // `js_object_set_field_by_name`, and that call re-reads it like any + // other rooted argument. It used to be pushed after, which is #7192 + // exactly — `js_object_set_field_by_name` is a collection point + // (`POLL_CAPABLE_RUNTIME`: it can run a user setter or a Proxy trap, + // and the closure it just installed is reachable from the object, so + // an evacuating minor MOVES it). The register queued above was then + // stale, and pushing a stale pointer into a slot the collector scans + // is strictly worse than not rooting at all: the patch loop below + // read it back and wrote the receiver into abandoned from-space, so + // the surviving copy's method ran with `this` unset. + // + // This arm was unreachable from TypeScript between #809 and #8793 — + // every source-level literal with a method went through the IIFE + // builder — which is why the ordering survived the #7192 sweep. See + // `by_name_method_closure_tests` below, whose HIR-level fixture is + // the only coverage it had. let mut rest: Vec<(String, u32)> = Vec::new(); let closure_value = rooting::with_rooted_accumulator( ctx, Repr::Boxed, &v, protect, - |ctx, _| { + |ctx, closure| { + let key_raw = emit_interned_key_raw(ctx, &key_handle_global); + obj.call_void( + ctx, + "js_object_set_field_by_name", + &[Arg::Plain(I64, &key_raw), closure.as_arg()], + ); rest = lower_by_name_props(ctx, obj, props, i + 1, protect)?; Ok(()) }, @@ -529,14 +554,22 @@ mod by_name_method_closure_tests { /// `lower_object_literal`'s BY-NAME path and with it the deferred /// `this`-patch machinery this module's nested accumulators root. /// - /// Built from HIR rather than from TypeScript on purpose. Since #809 every - /// source-level object literal containing a `Prop::Method` is lowered to a - /// source-ordered IIFE over `{}` (`js_object_set_method_by_name`), so the - /// by-name path with a non-empty prop list is not reachable from - /// TypeScript: over the whole `gc_root_dominance_corpus.sh` corpus (129 - /// sources, 149 modules) every emitted `js_object_alloc` is - /// `(i32 0, i32 0)`. A branch no corpus reaches is a branch no IR A/B can - /// speak for, so it gets a test of its own rather than an assumption. + /// Built from HIR rather than from TypeScript on purpose — originally + /// because the branch was unreachable from source, and now because this is + /// the only place its *shape* is asserted rather than sampled. + /// + /// **That unreachability lapsed, and it cost a shipped rooting bug.** From + /// #809 to #8793 every source-level object literal containing a + /// `Prop::Method` lowered to a source-ordered IIFE over `{}` + /// (`js_object_set_method_by_name`), so this path never ran on real code: + /// over the whole `gc_root_dominance_corpus.sh` corpus every emitted + /// `js_object_alloc` was `(i32 0, i32 0)`. #8793 routes a static-key method + /// literal straight to `Expr::Object`, which lands here — and the late root + /// this arm had carried unexercised since #6951 became seven live + /// `--moving-only` violations the next morning (#8809). A branch no corpus + /// reaches is a branch that keeps whatever bug it has until something + /// reaches it, so treat "not reachable from TypeScript" as a note about + /// today's front end, never as a reason a hazard here is theoretical. /// /// The two closures deliberately reserve DIFFERENT `this` slots — `add` /// captures nothing so its slot index is 0, `scale` captures `base` so its @@ -713,6 +746,59 @@ mod by_name_method_closure_tests { ); } + /// #8809: each method closure's root store must DOMINATE the + /// `js_object_set_field_by_name` that installs it, not follow it. + /// + /// The installer is a collection point — it can run a user setter or a + /// Proxy trap, and by the time it returns the closure is reachable from the + /// object, so an evacuating minor MOVES it. Rooting afterwards publishes + /// the pre-move register into a slot the collector scans, and the deferred + /// patch loop then writes the receiver into abandoned from-space: the + /// surviving copy's method runs with `this` unset. That was the shipped + /// state until this test existed. + /// + /// Asserted positionally over the emitted lines rather than by counting + /// calls: the count is identical in both orderings, which is exactly why + /// #7192's sweep did not catch this one. + #[test] + fn a_method_closure_is_rooted_before_it_is_installed() { + let ir = build_fn(&method_literal_ir()); + let lines: Vec<&str> = ir.lines().collect(); + let closure_allocs: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| l.contains("@js_closure_alloc")) + .map(|(i, _)| i) + .collect(); + assert_eq!( + closure_allocs.len(), + 2, + "the fixture's two method closures must each be allocated here:\n{ir}" + ); + for alloc in closure_allocs { + let root = lines[alloc..] + .iter() + .position(|l| { + l.starts_with("store ptr addrspace(1) %") || l.starts_with("store i64 %") + }) + .unwrap_or_else(|| { + panic!("no root store below the closure allocation on line {alloc}:\n{ir}") + }); + let install = lines[alloc..] + .iter() + .position(|l| l.contains("@js_object_set_field_by_name(")) + .unwrap_or_else(|| { + panic!("no by-name install below the closure allocation on line {alloc}:\n{ir}") + }); + assert!( + root < install, + "the closure allocated on line {alloc} is installed before its root store \ + (root +{root}, install +{install}) — that is #7192's shape and it publishes \ + a moved-from address:\n{ir}" + ); + } + } + /// The rooting shape, read off the IR. Three GC values are live across the /// literal — the object handle and both deferred closure values — so the /// lowering must own three rooted slots and must give each back, innermost diff --git a/crates/perry-codegen/src/lower_call/new.rs b/crates/perry-codegen/src/lower_call/new.rs index 4576bcf463..61f40b907c 100644 --- a/crates/perry-codegen/src/lower_call/new.rs +++ b/crates/perry-codegen/src/lower_call/new.rs @@ -58,6 +58,26 @@ fn construction_runs_user_code(ctx: &FnCtx<'_>, class_name: &str) -> bool { ctx.classes.get(class_name).is_some_and(|class| { class.constructor.is_some() || !class.fields.is_empty() + // #8809: a class whose only private elements are METHODS or + // ACCESSORS declares no fields, no constructor and no heritage, and + // answered `false` here — while `emit_field_inits` still emits + // `js_private_brand_add` for it (#8643 added that call, keyed on + // `has_private_instance_elements`, and its `continue` guard lets a + // fieldless class through precisely so the brand can be installed). + // That helper allocates the marker key and calls + // `js_object_set_field_by_name`; its own body says "the marker-key + // allocation can evacuate both the receiver and any live value" and + // opens a `RuntimeHandleScope` for exactly that reason. So the + // window this predicate claims cannot collect does, and the + // instance was crossing it in a bare register: `new + // WithPrivateMethod()` fed a stale handle to + // `js_gc_init_typed_shape_layout` and then published it into the + // caller's root slot. + // + // One predicate, one place — the temp root, the `this`-slot bind + // and `reload_instance` all read this, which is what stops them + // disagreeing the way #7114's pair did. + || class.has_private_instance_elements() || class.extends.is_some() || class.extends_name.is_some() || class.native_extends.is_some() diff --git a/crates/perry-codegen/src/rooting/mod.rs b/crates/perry-codegen/src/rooting/mod.rs index 25e9a60c7c..ae5e7d16c5 100644 --- a/crates/perry-codegen/src/rooting/mod.rs +++ b/crates/perry-codegen/src/rooting/mod.rs @@ -1185,7 +1185,14 @@ pub(crate) struct RootedAcc { impl RootedAcc { /// The accumulator as a call argument. - fn as_arg(&self) -> Arg<'_> { + /// + /// `pub(crate)` for the one shape that needs it: a call whose argument 0 is + /// one accumulator and whose later argument is *another* (an object-literal + /// method closure being installed into the half-built object it belongs to, + /// #8809). It hands out an [`Arg`], never a register — `materialize` still + /// performs the re-read at the instant the call is emitted — so the "load + /// early, use late" sequence stays unwritable through this door too. + pub(crate) fn as_arg(&self) -> Arg<'_> { match &self.slot { Some(slot) => Arg::Root(slot), None => Arg::Plain(self.repr.llvm_ty(), &self.value), diff --git a/docs/src/internals/gc-rooting-invariant.md b/docs/src/internals/gc-rooting-invariant.md index e720e7f3e2..dd88ae461d 100644 --- a/docs/src/internals/gc-rooting-invariant.md +++ b/docs/src/internals/gc-rooting-invariant.md @@ -303,9 +303,30 @@ like coverage while doing it. Two rounds of this have now been measured: lowering. Adding the one name takes the sabotaged arms to 13 and 8 and leaves the clean arms at 2 and 2. +4. **A poll-capable symbol NO audit can ask for** (#8809). `--audit-poll-reach` + walks only symbols `ALLOC_RE` matches, so a helper that allocates but spells + it neither `_alloc` nor `_new` nor `_create` is outside its domain entirely. + `js_private_brand_add` is one: it reaches `js_object_set_field_by_name` in + three lines, and its own body says *"the marker-key allocation can evacuate + both the receiver and any live value"* — it opens a `RuntimeHandleScope` for + exactly that reason. Unlisted, the window `new C()` opens around it + classified `MOVING: no`, and every `--moving-only` arm dropped a real stale + instance handle. Round 3's instrument is structurally blind here, which is + the standing residual: **when you add a runtime helper that can re-enter JS + or allocate through a path that can, add it to `POLL_CAPABLE_RUNTIME` in the + same commit.** Nothing will ask you to. + `--audit-poll-capable` is the gate for rounds 1–2 and `--audit-poll-reach` is -the gate for round 3; `gc-root-dominance.yml` runs both alongside -`--audit-alloc-re` before the build. `--audit-poll-capable` fails on any entry +the gate for round 3; round 4 has no gate. `gc-root-dominance.yml` runs both +alongside `--audit-alloc-re` before the build. + +**Those pre-build audits are also the job's single point of failure, and it has +already cost ten days.** `--audit-poll-reach` went red on `main` on 2026-08-15 +over three unlisted symbols and stayed red; because it runs *before* the +compiler build, not one of the four gated arms below it executed until #8809. +Two rooting regressions landed inside that window, and the opt-in PR arm did +not see either (neither PR carried `run-extended-tests`). A red audit is not a +warning about the checker's bookkeeping — it is the whole gate off. `--audit-poll-capable` fails on any entry that names no exported `extern "C" fn js_*`. When it goes red, **replace** the phantom with the symbol codegen actually emits rather than deleting it — deleting turns the audit green and leaves the hole. diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index e8f825acdd..b160cbeb0a 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -1341,6 +1341,40 @@ def is_collecting(callee): "js_super_construct_apply", "js_vm_synthetic_module_new", "js_writable_stream_new", + # The third wave, reported by `--audit-poll-reach` and unfixed long enough + # to matter: that step has been the FIRST failing step of every scheduled + # `gc-root-dominance` run on `main` since 2026-08-15, and it runs before the + # compiler build, so none of the four gated arms below it executed for ten + # days. Two rooting regressions landed inside that window (#8809). A gate + # that cannot reach its own subject is hazard 4 with an extra step. + # + # Each is the audit's own premise, one hop each: + # js_builtin_subclass_construct -> js_new_function_construct_with_new_target + # (object/class_registry/construct/class_return.rs — runs a user + # subclass constructor; nothing is more poll-capable) + # js_tls_create_secure_context -> js_tls_secure_context_new (tls.rs:1155, + # a pure delegate) + # js_tls_secure_context_new -> js_object_set_field_by_name (tls.rs:1160, + # reads the caller's options object and writes the result) + "js_builtin_subclass_construct", + "js_tls_create_secure_context", "js_tls_secure_context_new", + # `js_private_brand_add` is the referent-with-no-name that NEITHER audit can + # ask for: `--audit-poll-reach` only walks symbols `ALLOC_RE` matches, and + # `js_private_brand_add` matches no alloc/new/create convention, so the one + # instrument built for this hole is structurally blind to it. It reaches + # `js_object_set_field_by_name` in three lines + # (object/field_get_set/ic_miss.rs), and its own body states the premise + # outright — "the marker-key allocation can evacuate both the receiver and + # any live value" — which is why it opens a `RuntimeHandleScope` before + # deriving raw pointers. Without this entry the window `new C()` opens + # around it classified `MOVING: no` and every `--moving-only` arm dropped + # the #8809 instance-handle violation, exactly the way the emitted + # property-GET helpers were dropped before #7284. + # + # Measured when added: the ONLY window this reclassifies over the curated + # corpus is that one, which the same PR fixes in + # `lower_call/new.rs::construction_runs_user_code`. + "js_private_brand_add", } diff --git a/scripts/gc_root_dominance_corpus.sh b/scripts/gc_root_dominance_corpus.sh index 751433ccab..3df473b14c 100755 --- a/scripts/gc_root_dominance_corpus.sh +++ b/scripts/gc_root_dominance_corpus.sh @@ -49,6 +49,40 @@ # (`STATEPOINT_REWRITE_PASSES`) and checked, not copied: a reproduction of a # pipeline that has silently drifted is a corpus about nothing. See # `rs4gc_pass_string`. +# +# THE CORPUS DOES NOT LINK, and that is load-bearing (#8810) +# --------------------------------------------------------- +# `--trace llvm` dumps what CODEGEN emitted, and codegen finishes long before +# the link line is assembled. Linking anyway made this corpus depend on the +# whole ext-wrapper/link stack -- and that dependency silently DELETED two of +# its subjects: +# +# `PERRY_NO_AUTO_OPTIMIZE=1` (set in the loop below, deliberately) forbids +# the specialized runtime/stdlib rebuild. The only two corpus sources that +# import a node builtin -- `test_gap_gc_net_once_flags_rekey` (`node:net`) +# and `test_gap_gc_http2_pending_event_callback_rooting` (`node:http2`) -- +# therefore need a prebuilt `libperry_ext_{net,http}.a`, which the build +# command above does NOT produce. perry auto-built each wrapper in its own +# cargo invocation, cargo unified features per invocation, and the wrapper +# ended up bundling a different tokio compilation than the prebuilt +# `libperry_stdlib.a`. `compile/shared_tokio.rs` refuses that pair at link +# time (#507/#7629) -- correctly. perry then exited non-zero AFTER writing +# perfectly good IR, the loop counted a skip, and the two sources whose +# names say they cover GC CALLBACK ROOTING contributed nothing to a gate +# that exists to check exactly that. +# +# Not linking removes the failure mode rather than tolerating it: there is no +# link line to be wrong, no ext archive to be missing, and no stale +# `libperry_runtime.a` that could decide what this corpus contains. It is also +# what the compiler-output census does, for the same reason +# (`scripts/compiler_output_harness/repsel_census.py`), and it stops this +# script writing 152 executables of ~11 MB each that nothing ever reads. +# +# A source that stops LINKING is still a finding -- just not THIS gate's. +# `./run_parity_tests.sh` compiles AND RUNS every `test_gap_*.ts` under the +# shipping (auto-optimize) configuration, which is where a broken link line +# belongs. Codegen failures are unaffected: perry still exits non-zero, and +# the skip ratchet below still fails the run. set -euo pipefail @@ -136,14 +170,20 @@ PATTERNS=( # to compile". Two different failures, two different messages. # MAX_SKIPPED how many discovered sources may fail to compile. It is 0, and # 0 is the measured truth on both lowerings, not an aspiration: -# shadow and native each report 131/131, 0 skipped as of -# v0.5.1402. A skip is a finding, not a tolerance. +# shadow reports 152/152, 0 skipped as of v0.5.1519 (it was +# 131/131 at v0.5.1402, before 21 sources were added). A skip +# is a finding, not a tolerance. +# +# MIN_SOURCES had drifted the same way the MIN_COMPILED=90 floor did, just less +# far: PATTERNS discovered 152 files against a floor of 131, so 21 sources +# could vanish before the "corpus shrank" arm could fire. Raised to the +# measured count (#8810). # # Raise MIN_SOURCES when you add a prefix; there is nothing else to keep in # sync, because the compile floor is now DERIVED (MIN_SOURCES - MAX_SKIPPED) # rather than restated. Lowering either to make a run pass is the thing this # comment exists to stop. -MIN_SOURCES="${MIN_SOURCES:-131}" +MIN_SOURCES="${MIN_SOURCES:-152}" MAX_SKIPPED="${MAX_SKIPPED:-0}" if [ ! -x "$PERRY_BIN" ]; then @@ -247,6 +287,22 @@ fi rm -rf "$OUTDIR" mkdir -p "$OUTDIR" +# The first line of a failed compile that looks like an error, for the skip +# report. A skip that names only the source is a finding you have to reproduce +# locally before you can read it at all -- which is how #8810's two skips went +# a release without anyone learning they were a link-line refusal. +first_error_line() { + local log="$1" + local line="" + if [ -f "$log" ]; then + line="$(grep -m1 -E '^[[:space:]]*(Error|error)[:[:space:]]' "$log" || true)" + if [ -z "$line" ]; then + line="$(grep -v '^[[:space:]]*$' "$log" | tail -1 || true)" + fi + fi + printf '%.180s' "${line:-}" +} + compiled=0 skipped=0 skipped_names=() @@ -275,15 +331,19 @@ for src in "${sources[@]}"; do # `rewrite-statepoints-for-gc` turns into `gc.statepoint` # relocation bundles below. Zero binds by construction, which # is why `--statepoints` has its own floors. + # + # `--no-link` stops after codegen, which is where `--trace llvm` writes. See + # the header: linking made this corpus depend on the ext-wrapper/link stack, + # and that dependency deleted two subjects from it (#8810). if [ "$LOWERING" = "native" ]; then rs4gc=1; else rs4gc=0; fi if ! env PERRY_RS4GC="$rs4gc" \ PERRY_GC_MOVING_LOOP_POLLS=1 \ PERRY_INLINE_SHADOW_SLOT=0 \ PERRY_NO_AUTO_OPTIMIZE=1 \ - "$PERRY_BIN" compile "$src" -o "$scratch/$name" --trace llvm \ - >/dev/null 2>&1; then + "$PERRY_BIN" compile "$src" -o "$scratch/$name.o" --no-link --trace llvm \ + >"$scratch/compile.log" 2>&1; then skipped=$((skipped + 1)) - skipped_names+=("$name") + skipped_names+=("$name -- $(first_error_line "$scratch/compile.log")") continue fi emitted=0 @@ -319,7 +379,7 @@ done files="$(find "$OUTDIR" -name '*.ll' | wc -l | tr -d ' ')" echo "corpus ($LOWERING): $compiled/${#sources[@]} sources compiled, $skipped skipped, $files .ll files" if [ "$skipped" -gt 0 ]; then - printf ' skipped: %s\n' "${skipped_names[*]}" + printf ' skipped: %s\n' "${skipped_names[@]}" fi if [ "$opt_failed" -gt 0 ]; then printf ' rewrite failed: %s\n' "${opt_failed_names[*]}" @@ -370,7 +430,7 @@ fi # raises it, which is precisely how MIN_COMPILED=90 came to tolerate 41. if [ "$skipped" -gt "$MAX_SKIPPED" ]; then echo "::error::$skipped of ${#sources[@]} sources failed to compile (budget: $MAX_SKIPPED)." >&2 - echo " ${skipped_names[*]}" >&2 + printf ' %s\n' "${skipped_names[@]}" >&2 echo "Fix them. Raising MAX_SKIPPED hides a compiler regression from every" >&2 echo "downstream floor in this script -- IR that was never emitted reads as" >&2 echo "clean to the checker." >&2