diff --git a/changelog.d/7687-alloc-point-collections-must-not-move.md b/changelog.d/7687-alloc-point-collections-must-not-move.md new file mode 100644 index 0000000000..7df6cb24b9 --- /dev/null +++ b/changelog.d/7687-alloc-point-collections-must-not-move.md @@ -0,0 +1,78 @@ +### Fixed — an allocation-point collection can no longer MOVE anything (#7682) + +A 189-statement tree-walking interpreter — ordinary TypeScript, no exotic +construct, `scriptc coverage` reports it fully static — returned a **silently +wrong number** on default settings, every run: `1708662` where Node and a fully +static build both give `1708840`. No crash, no `TypeError`, no diagnostic. + +**Root cause.** `gc_check_trigger`'s nursery-churn arm collects from inside +`arena_cell_alloc`, i.e. at whatever half-finished expression happened to need a +fresh arena block. That program point is described by neither root lowering: the +shadow stack names only values codegen has already stored to a slot, and RS4GC +relocates only what it can type as `ptr addrspace(1)`, which a NaN-boxed +`double` operand in an SSA register is not. The arm therefore took +`ManualGcScanGuard::force_full_scan()`, whose job there is not retention but +*immobility* — a conservative native-stack scan makes the copying minor +ineligible (`CopiedMinorFallbackReason::ConservativeStack`), so the non-moving +in-place minor runs and nothing relocates. + +`PERRY_GC_SCAVENGE` gated that guard off. The guard is now unconditional. + +**Why the gate was ever conditional, and why both halves of the reason were +false.** The flag's doc comment said "Phase-1 de-risking flag (OFF by default) … +NOT sound as a production default yet — the alloc point can be +register-imprecise — so it stays behind this flag for measurement only". Eight +lines below it, the body said `ON BY DEFAULT (#7056)`. That is the #6987 shape +CLAUDE.md warns about, and this time the stale half was the one carrying the +soundness argument. The body's own claim — "enabling this also defers +alloc-point collections to a precise safepoint" — was false too: that deferral +is gated on `gc_moving_loop_polls_enabled()`, OFF by default since #7161. In the +shipped configuration the two flags disagree, the deferral is dead code, and the +alloc-point minor ran right there with neither a scan nor a safepoint. + +**The failure, end to end.** `evalNode` lowers `{ names: [n.name], … }` by +reading `n.name` into a register, then inline-bump-allocating the one-element +array. The bump overflows its block, `js_inline_arena_slow_alloc` → +`arena_cell_alloc` → `gc_check_trigger` runs an *evacuating* minor, and the +string moves. Control returns to the shared merge block, which stores the +pre-move address into the new array. `lookup` then compares `names[i] === name` +— a live string against a moved one — falls through to its default, and naive +`fib`, which is just a count of leaves returning `1`, comes back short by +exactly the number of missed lookups. + +**Why every existing gate was green.** `PERRY_GC_VERIFY_MARK` reports OK +(marking is correct; it is the post-move holder that is wrong, and it is a +register, so there is nothing in the heap to find). The heap-wide +`PERRY_GC_FROMSPACE_SCAN` finds no live offender for the same reason — every +owner it reports is `marked=false`, i.e. already dead. `scripts/gc_root_dominance_check.py` +reports 0 violations over 380 root stores: the value is never bound to a slot at +all, so there is no store whose dominance it could question. And the GC probe +corpus holds its subjects across an *explicit* churn call; none holds one across +the allocation of the literal being built. + +**Cost.** Alloc-point nursery collections are non-moving again, which is what +they were before #7056. Copying minors continue to run at the precise safepoints +(`gc_safepoint_moving_minor`), where the root set is real. `PERRY_GC_SCAVENGE` +keeps its other job — routing nursery-churn triggers to the direct minor instead +of the budgeted non-moving stepper — and is documented as the pacing knob it is. +The `gc-ratchet` artifact was measured under the shipped default and pins the +old evacuating behaviour; it needs regenerating on the pinned host. + +**Tests.** + +- `gc::tests::scan_fallback::the_alloc_point_nursery_minor_retains_native_stack_values_under_shipped_pacing` + drives the arm with a value reachable only from a live native-stack word and + asserts it survives, that a collection actually ran, and that the census + counted the forced scan. Its sabotage control + (`the_alloc_point_plant_dies_when_the_scan_is_pinned_off`) runs the identical + plant with the scan pinned off and asserts the plant DIES — without it, "the + malloc sweep never ran" and "the guard held" are the same green. +- `policy::force_shipped_default_gc_pacing` pins polls OFF + scavenge ON. That + combination had no test guard: `force_legacy_gc_pacing` pins both off and + `force_moving_gc_pacing` pins both on, so every test in the crate declared a + pacing mode in which the two flags agreed — and the interaction that broke is + exactly the one where they disagree. +- `test-files/test_gap_gc_alloc_point_no_move.ts` is the interpreter itself, + compared byte-for-byte against Node. Verified non-vacuous: it prints `1708662` + against the pre-fix runtime under the gap harness's own + `PERRY_NO_AUTO_OPTIMIZE=1` configuration, and `1708840` after. diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 547e2a2710..3eaee16560 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -326,18 +326,27 @@ fn gc_verify_evacuation_enabled() -> bool { ) } -/// Phase-1 de-risking flag (OFF by default). When set, the alloc-point -/// nursery-churn arm (`gc_check_trigger`) runs its direct minor with the -/// PRECISE shadow-stack roots instead of forcing the conservative native -/// scan. The conservative scan makes the copying fast path ineligible -/// (`CopiedMinorFallbackReason::ConservativeStack`), pinning the minor to the -/// non-moving in-place sweep that cannot reclaim array-growth stubs; skipping -/// it lets the evacuating scavenge run and reset the whole young arena in -/// O(live). NOT sound as a production default yet — the alloc point can be -/// register-imprecise — so it stays behind this flag for measurement + -/// `PERRY_GC_VERIFY_EVACUATION` probing only. Pairs with -/// `PERRY_GC_MAJOR_PACING_FLOOR_MB=0` so the #6939 pacing doesn't escalate the -/// minor to a full before the copying path is reached. +/// `PERRY_GC_SCAVENGE` — **ON by default since #7056**, kill switch +/// `PERRY_GC_SCAVENGE=0`/`off`/`false`. It is a PACING knob: it routes +/// nursery-churn triggers to the direct minor in `gc_check_trigger` instead of +/// the budgeted non-moving stepper, which on a reallocation-heavy loop frees +/// nothing. Paired with the nursery cap in `policy::effective_next_arena_trigger` +/// that is the -69% RSS result quoted on the getter below. +/// +/// It does **not** decide whether the alloc-point minor may move, and #7682 is +/// what that confusion cost. The flag used to gate the `force_full_scan()` on +/// that arm off, so the shipped default ran an EVACUATING minor at an arbitrary +/// allocation point — a program point neither root lowering describes — and +/// values held only in registers were relocated behind their holders' backs. +/// The guard is now unconditional; see the comment at its site in +/// `policy::gc_check_trigger` for why no pacing knob can answer the question it +/// asks. +/// +/// This doc comment previously read "Phase-1 de-risking flag (OFF by default) +/// … NOT sound as a production default yet". Both halves were false for two +/// hundred releases, eight lines above a body comment saying "ON BY DEFAULT" — +/// the #6987 shape CLAUDE.md warns about, and this time the stale half was the +/// one carrying the soundness argument. #[cfg(test)] thread_local! { /// Test-only override, consulted BEFORE the process-wide OnceLock so a @@ -373,10 +382,14 @@ pub(super) fn gc_scavenge_enabled() -> bool { // them evacuating (O(live) copying) rather than O(heap) sweeps — so the // frequency is cheap instead of expensive. // - // Enabling this also defers alloc-point collections to a precise - // safepoint rather than collecting behind a forced conservative scan. - // That is newly reasonable: native roots became the default in #7370, so - // a precise safepoint is what the shipped configuration now has. + // What this does NOT do, despite what this comment used to claim: it + // does not defer alloc-point collections to a precise safepoint. That + // deferral is gated on `gc_moving_loop_polls_enabled()`, which has been + // OFF by default since #7161 — so in the shipped configuration the two + // flags disagree, the deferral is dead, and the alloc-point minor runs + // right there. It is sound because that minor is non-moving + // (`force_full_scan`), not because it was moved somewhere precise + // (#7682). !matches!( std::env::var("PERRY_GC_SCAVENGE").as_deref(), Ok("0") | Ok("off") | Ok("false") diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index 85172f2fbd..acb66ec3c2 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -568,6 +568,30 @@ pub(super) fn force_moving_gc_pacing() -> LegacyGcPacingGuard { } } +/// Pin the pacing combination a **shipped binary actually runs**: moving-loop +/// polls OFF (`gc_moving_loop_polls_enabled`, default OFF since #7161) and +/// scavenge ON (`gc_scavenge_enabled`, default ON since #7056). +/// +/// This is a third combination, and its absence is part of why #7682 shipped. +/// [`force_legacy_gc_pacing`] pins polls OFF *and* scavenge OFF; +/// [`force_moving_gc_pacing`] pins both ON. Every test in this crate therefore +/// declared a pacing mode in which the two flags agreed — and the +/// alloc-point/deferral interaction that broke is precisely the one where they +/// DISAGREE: scavenge routes nursery pressure to the direct alloc-point minor, +/// while the deferral that was supposed to move that collection to a precise +/// safepoint is gated on the polls flag and never runs. +#[cfg(test)] +pub(super) fn force_shipped_default_gc_pacing() -> LegacyGcPacingGuard { + let previous = GC_MOVING_LOOP_POLLS_TEST_OVERRIDE.with(|cell| cell.replace(Some(false))); + let cap_previous = GC_NURSERY_CAP_TEST_SUPPRESSED.with(|cell| cell.replace(false)); + let scavenge_previous = super::GC_SCAVENGE_TEST_OVERRIDE.with(|cell| cell.replace(Some(true))); + LegacyGcPacingGuard { + previous, + cap_previous, + scavenge_previous, + } +} + pub(super) fn gc_trace_enabled() -> bool { #[cfg(test)] if GC_TRACE_TEST_FORCE.with(Cell::get) { @@ -1822,14 +1846,23 @@ pub fn gc_check_trigger() { } let pre_in_use = crate::arena::arena_in_use_bytes(); let pre_malloc_count = malloc_object_count(); - // PERRY_GC_SCAVENGE (Phase-1 de-risking, OFF by default): skip the - // conservative native-stack scan so this direct minor runs with the - // PRECISE shadow-stack roots and the copying fast path becomes - // eligible (an evacuating scavenge that resets the whole young arena - // in O(live)). The default path keeps `force_full_scan` — at an - // arbitrary alloc point a value mid-construction may live only in - // registers, which the conservative scan retains (and which makes - // copied-minor ineligible, so the non-moving minor runs). + // THE ALLOC POINT IS REGISTER-IMPRECISE, SO THIS MINOR MUST NOT + // MOVE. Unconditional, and the unconditionality is the fix for + // #7682. + // + // Reaching this line means the collection is happening HERE, at an + // arbitrary allocation point inside a half-built expression — not + // at a declared safepoint. Neither root lowering describes that + // point: the shadow stack only names values codegen has already + // stored to a slot, and RS4GC only relocates values it can type as + // `ptr addrspace(1)`, which a NaN-boxed `double` operand in an SSA + // register is not. A value that exists ONLY in a register here is + // therefore invisible to both, so an evacuating minor relocates the + // object and leaves the register naming the pre-move address. The + // conservative native-stack scan is what covers exactly that gap: + // it retains such values AND makes the copying minor ineligible + // (`CopiedMinorFallbackReason::ConservativeStack`), so the + // non-moving in-place minor runs and nothing relocates. // // ★ #7148 disposition: **keep as the bounded valve, now counted.** // The deferral above is the primary path and is sound by @@ -1842,16 +1875,31 @@ pub fn gc_check_trigger() { // reached" is: this arm runs, and it is the reason RSS stays // bounded. Making it *imprecise* instead (collecting without the // scan) is the one thing #7148 rules out — it would trade a cost - // problem for a soundness problem. Making it **countable** is what - // turns "unreachable in practice" into a measurement: - // `ConservativeScanSite::NurseryChurnSlackValve` is 0 on all eight - // ratchet probes and across the stress matrix, and the drain - // counter proves the deferral ran instead. - let _scan = (!super::gc_scavenge_enabled()).then(|| { - super::roots::ManualGcScanGuard::force_full_scan( - super::ConservativeScanSite::NurseryChurnSlackValve, - ) - }); + // problem for a soundness problem. + // + // #7682 is that trade, shipped. `PERRY_GC_SCAVENGE` used to gate + // this guard off, on the strength of a doc comment claiming the + // flag was "OFF by default … for measurement only" and a body + // comment saying it also "defers alloc-point collections to a + // precise safepoint". Neither held in the shipped configuration: + // the flag has been ON by default since #7056, and the deferral + // above is gated on `gc_moving_loop_polls_enabled()`, which is OFF + // by default since #7161. So the default build collected — and + // EVACUATED — right here, with no scan and no deferral. A + // tree-walking interpreter (`test_gap_gc_alloc_point_no_move.ts`) + // then read a relocated heap string out of a stale register and + // silently returned the wrong number. + // + // The scan-skip cannot be recovered by asking "is scavenge on?": + // that question is about pacing, and the precondition being + // asserted here is about the PRECISION OF THIS PROGRAM POINT, + // which no pacing knob can change. Scavenge keeps its other job — + // routing nursery-churn triggers to this direct minor instead of + // the budgeted non-moving stepper — and the moving minor keeps + // running at the precise safepoints, where the root set is real. + let _scan = super::roots::ManualGcScanGuard::force_full_scan( + super::ConservativeScanSite::NurseryChurnSlackValve, + ); let outcome = super::gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(kind)); // Re-baseline the arming trigger after the direct minor, mirroring // `gc_finish_budgeted_cycle`. This arm is taken whenever diff --git a/crates/perry-runtime/src/gc/tests/scan_fallback.rs b/crates/perry-runtime/src/gc/tests/scan_fallback.rs index 4fbf0aea8d..a2f8451732 100644 --- a/crates/perry-runtime/src/gc/tests/scan_fallback.rs +++ b/crates/perry-runtime/src/gc/tests/scan_fallback.rs @@ -438,3 +438,116 @@ fn host_pressure_deferral_still_has_a_drain_when_moving_loop_polls_are_off() { clear_old_reclaim_state(); } + +/// #7682: the same plant, but the collection is driven by the **nursery-churn +/// allocation-point arm** rather than an explicit `gc()`. +/// +/// The distinction is the whole point. `gc()` is called from a place the +/// precise root set describes; this arm runs from inside `arena_cell_alloc`, +/// i.e. at whatever half-finished expression happened to need a fresh block. +/// A value live only in an LLVM register there is named by neither root +/// lowering — the shadow stack holds only what codegen has already stored to a +/// slot, and RS4GC relocates only what it can type as `ptr addrspace(1)`, +/// which a NaN-boxed `double` operand is not. The native-stack plant stands in +/// for exactly that value. +fn plant_on_native_stack_and_check_trigger(triggers: &GcTriggerThresholdTestGuard) -> bool { + #[inline(never)] + fn run(user_ptr: *mut u8, triggers: &GcTriggerThresholdTestGuard) -> bool { + let mut plant = [0u64; 16]; + plant[7] = ptr_bits(user_ptr as usize); + plant[11] = user_ptr as u64; + std::hint::black_box(plant.as_ptr()); + // Arm AFTER the plant's own allocation so the count comparison is + // due, and immediately before the check so nothing can re-baseline + // it. `copied_minor_malloc_sweep_due` reads the same comparison + // directly, so the malloc registry is swept whichever trigger kind + // `gc_budgeted_due_trigger` reports — without that the "it survived" + // assertion below would be vacuous. + triggers.make_malloc_sweep_due(); + gc_check_trigger(); + std::hint::black_box(plant.as_ptr()); + malloc_user_ptr_tracked(user_ptr) + } + + let ptr = gc_malloc( + std::mem::size_of::(), + GC_TYPE_CLOSURE, + ); + unsafe { init_test_closure(ptr) }; + run(ptr, triggers) +} + +#[test] +fn the_alloc_point_nursery_minor_retains_native_stack_values_under_shipped_pacing() { + // The regression test for #7682, and it is pinned to the pacing a shipped + // binary actually has: polls OFF (#7161) so the deferral to a precise + // safepoint never fires, scavenge ON (#7056) so nursery pressure is routed + // to this direct alloc-point minor. In that combination the guard below + // was skipped, the copying minor became eligible at a register-imprecise + // point, and a tree-walking interpreter silently returned the wrong number + // because a relocated heap string was read back out of a stale register. + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_shipped_default_gc_pacing(); + let triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + // The isolation guard pins `Auto`, and an already-pinned override makes + // `force_full_scan` a no-op — so a test that left it pinned could not tell + // a removed force from a suppressed one. Clear it: this arm must see + // exactly what a production binary sees. + let pinned = crate::gc::roots::set_conservative_stack_scan_override(None); + let collections_before = gc_collection_count(); + let survived = plant_on_native_stack_and_check_trigger(&triggers); + let collections_after = gc_collection_count(); + crate::gc::roots::set_conservative_stack_scan_override(pinned); + + assert!( + collections_after > collections_before, + "LIVE SUBJECT: the allocation-point arm must actually have collected — \ + 'the plant survived' is worthless if nothing ran" + ); + assert!( + scan_fallback_count(ConservativeScanSite::NurseryChurnSlackValve) >= 1, + "the alloc point is register-imprecise, so this collection must force \ + the conservative scan — unconditionally, not only when scavenge is off" + ); + assert!( + survived, + "a value reachable ONLY from a live native-stack word must survive an \ + allocation-point collection: it stands for the NaN-boxed operand an \ + expression is holding in a register while its own allocation runs" + ); + + clear_old_reclaim_state(); +} + +#[test] +fn the_alloc_point_plant_dies_when_the_scan_is_pinned_off() { + // SABOTAGE CONTROL for the test above, and the reason its green means + // something. Identical plant, identical trigger, scan pinned off — which + // is precisely the state `PERRY_GC_SCAVENGE`'s scan-skip used to produce. + // The plant must DIE here. If it survives, the detector arm is measuring + // "the malloc sweep never ran" rather than "the guard held", and both + // arms would be green on a tree with the bug back in it. + let _isolation = GcTestIsolationGuard::new(); + let _pacing = crate::gc::policy::force_shipped_default_gc_pacing(); + let triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + clear_old_reclaim_state(); + reset_scan_fallback_counters(); + + let pinned = crate::gc::roots::set_conservative_stack_scan_override(Some( + ConservativeStackScanMode::Disabled, + )); + let survived = plant_on_native_stack_and_check_trigger(&triggers); + crate::gc::roots::set_conservative_stack_scan_override(pinned); + + assert!( + !survived, + "with the conservative scan pinned off the planted native-stack word \ + must NOT retain its object — otherwise the detector arm's green says \ + nothing about whether the scan ran" + ); + + clear_old_reclaim_state(); +} diff --git a/test-files/test_gap_gc_alloc_point_no_move.ts b/test-files/test_gap_gc_alloc_point_no_move.ts new file mode 100644 index 0000000000..9c4754ef6f --- /dev/null +++ b/test-files/test_gap_gc_alloc_point_no_move.ts @@ -0,0 +1,310 @@ +// #7682: an allocation-point collection must not MOVE anything. +// +// A tree-walking interpreter for a small functional language — ordinary +// TypeScript, no exotic construct — that returned a silently WRONG number on +// default settings: 1708662 instead of 1708840, on every run, with no crash and +// no diagnostic. +// +// WHAT IT CATCHES. `gc_check_trigger`'s nursery-churn arm collects from inside +// `arena_cell_alloc`, i.e. at whatever half-finished expression happened to +// need a fresh arena block. Neither root lowering describes that point: the +// shadow stack names only values codegen has already stored to a slot, and +// RS4GC relocates only what it can type as `ptr addrspace(1)`, which a +// NaN-boxed `double` operand is not. `PERRY_GC_SCAVENGE` (ON by default since +// #7056) used to skip the `force_full_scan()` that keeps such a collection +// non-moving, so an EVACUATING minor ran there and relocated objects still +// named by live registers. +// +// WHY THIS SHAPE AND NOT A SMALLER ONE. The victim here is the `[n.name]` +// element of `{ names: [n.name], ... }`: the string is read out of the AST node +// into a register, then the array literal's own inline bump-allocation overflows +// its block and collects. On return the register names the pre-move address, so +// `names[i] === name` compares a live string against a moved one, `lookup` falls +// through to its default, and naive `fib` — which is just a count of leaves +// returning 1 — comes back short by exactly the number of missed lookups. Every +// existing GC probe holds its subject across an EXPLICIT churn call; none holds +// one across the allocation of the literal being built, which is why a suite of +// them was green while this was broken. +// +// LIVE BY CONSTRUCTION: the interpreter allocates a fresh AST, environment +// chain, and value union per round, so the collector runs many times during the +// measured region. Compared byte-for-byte against Node — a wrong answer here is +// a wrong answer, not a timing difference. + +type Node = + | { kind: "num"; num: number } + | { kind: "str"; str: string } + | { kind: "var"; name: string } + | { kind: "bin"; op: string; left: Node; right: Node } + | { kind: "if"; cond: Node; then: Node; alt: Node } + | { kind: "let"; name: string; value: Node; body: Node } + | { kind: "fun"; param: string; body: Node } + | { kind: "call"; target: Node; arg: Node }; + +type Env = { names: string[]; vals: Value[]; parent: Env | null }; + +type Value = + | { kind: "num"; num: number } + | { kind: "str"; str: string } + | { kind: "clo"; param: string; body: Node; env: Env }; + +// ---------- lexer ---------- + +type Token = { kind: string; text: string }; + +function isDigit(c: string): boolean { + return c >= "0" && c <= "9"; +} + +function isIdentStart(c: string): boolean { + return (c >= "a" && c <= "z") || (c >= "A" && c <= "Z") || c === "_"; +} + +function isIdentPart(c: string): boolean { + return isIdentStart(c) || isDigit(c); +} + +function lex(src: string): Token[] { + const out: Token[] = []; + let i = 0; + while (i < src.length) { + const c = src.charAt(i); + if (c === " " || c === "\n" || c === "\t") { + i = i + 1; + continue; + } + if (isDigit(c)) { + let j = i; + while (j < src.length && isDigit(src.charAt(j))) j = j + 1; + out.push({ kind: "num", text: src.substring(i, j) }); + i = j; + continue; + } + if (isIdentStart(c)) { + let j = i; + while (j < src.length && isIdentPart(src.charAt(j))) j = j + 1; + const word = src.substring(i, j); + if (word === "let" || word === "in" || word === "if" || word === "then" || word === "else" || word === "fun") { + out.push({ kind: word, text: word }); + } else { + out.push({ kind: "ident", text: word }); + } + i = j; + continue; + } + if (c === '"') { + let j = i + 1; + while (j < src.length && src.charAt(j) !== '"') j = j + 1; + out.push({ kind: "str", text: src.substring(i + 1, j) }); + i = j + 1; + continue; + } + if (c === "<" && i + 1 < src.length && src.charAt(i + 1) === "=") { + out.push({ kind: "op", text: "<=" }); + i = i + 2; + continue; + } + out.push({ kind: c === "(" || c === ")" ? c : "op", text: c }); + i = i + 1; + } + out.push({ kind: "eof", text: "" }); + return out; +} + +// ---------- parser (precedence climbing) ---------- + +type Parser = { toks: Token[]; pos: number }; + +function peek(p: Parser): Token { + return p.toks[p.pos]; +} + +function advance(p: Parser): Token { + const t = p.toks[p.pos]; + p.pos = p.pos + 1; + return t; +} + +function expect(p: Parser, kind: string): void { + const t = advance(p); + if (t.kind !== kind) { + console.log("parse error: wanted " + kind + " got " + t.kind); + } +} + +function precOf(op: string): number { + if (op === "<" || op === ">" || op === "<=") return 1; + if (op === "+" || op === "-") return 2; + if (op === "*" || op === "/") return 3; + return 0; +} + +function parseAtom(p: Parser): Node { + const t = peek(p); + if (t.kind === "num") { + advance(p); + return { kind: "num", num: parseInt(t.text, 10) }; + } + if (t.kind === "str") { + advance(p); + return { kind: "str", str: t.text }; + } + if (t.kind === "ident") { + advance(p); + return { kind: "var", name: t.text }; + } + if (t.kind === "(") { + advance(p); + const inner = parseExpr(p, 0); + expect(p, ")"); + return inner; + } + if (t.kind === "fun") { + advance(p); + const name = advance(p); + const body = parseExpr(p, 0); + return { kind: "fun", param: name.text, body: body }; + } + if (t.kind === "let") { + advance(p); + const name = advance(p); + expect(p, "op"); // '=' + const value = parseExpr(p, 0); + expect(p, "in"); + const body = parseExpr(p, 0); + return { kind: "let", name: name.text, value: value, body: body }; + } + if (t.kind === "if") { + advance(p); + const cond = parseExpr(p, 0); + expect(p, "then"); + const then = parseExpr(p, 0); + expect(p, "else"); + const alt = parseExpr(p, 0); + return { kind: "if", cond: cond, then: then, alt: alt }; + } + advance(p); + return { kind: "num", num: 0 }; +} + +function parseApply(p: Parser): Node { + let head = parseAtom(p); + while (true) { + const t = peek(p); + if (t.kind === "num" || t.kind === "str" || t.kind === "ident" || t.kind === "(") { + const arg = parseAtom(p); + head = { kind: "call", target: head, arg: arg }; + continue; + } + return head; + } +} + +function parseExpr(p: Parser, minPrec: number): Node { + let left = parseApply(p); + while (true) { + const t = peek(p); + if (t.kind !== "op") return left; + const prec = precOf(t.text); + if (prec === 0 || prec < minPrec) return left; + advance(p); + const right = parseExpr(p, prec + 1); + left = { kind: "bin", op: t.text, left: left, right: right }; + } +} + +function parse(src: string): Node { + const p: Parser = { toks: lex(src), pos: 0 }; + return parseExpr(p, 0); +} + +// ---------- evaluator ---------- + +function lookup(env: Env, name: string): Value { + let e: Env | null = env; + while (e !== null) { + const names = e.names; + for (let i = 0; i < names.length; i++) { + if (names[i] === name) return e.vals[i]; + } + e = e.parent; + } + return { kind: "num", num: 0 }; +} + +function asNum(v: Value): number { + if (v.kind === "num") return v.num; + return 0; +} + +function evalNode(n: Node, env: Env): Value { + if (n.kind === "num") return { kind: "num", num: n.num }; + if (n.kind === "str") return { kind: "str", str: n.str }; + if (n.kind === "var") return lookup(env, n.name); + if (n.kind === "fun") return { kind: "clo", param: n.param, body: n.body, env: env }; + if (n.kind === "bin") { + const l = evalNode(n.left, env); + const r = evalNode(n.right, env); + if (n.op === "+" && l.kind === "str") { + const rs = r.kind === "str" ? r.str : "" + asNum(r); + return { kind: "str", str: l.str + rs }; + } + const a = asNum(l); + const b = asNum(r); + if (n.op === "+") return { kind: "num", num: a + b }; + if (n.op === "-") return { kind: "num", num: a - b }; + if (n.op === "*") return { kind: "num", num: a * b }; + if (n.op === "/") return { kind: "num", num: a / b }; + if (n.op === "<") return { kind: "num", num: a < b ? 1 : 0 }; + if (n.op === ">") return { kind: "num", num: a > b ? 1 : 0 }; + if (n.op === "<=") return { kind: "num", num: a <= b ? 1 : 0 }; + return { kind: "num", num: 0 }; + } + if (n.kind === "if") { + const c = evalNode(n.cond, env); + if (asNum(c) !== 0) return evalNode(n.then, env); + return evalNode(n.alt, env); + } + if (n.kind === "let") { + // Recursive let: bind the name first, then evaluate the value inside the + // new scope and patch the slot. A closure defined here captures an + // environment that contains the closure itself — a genuine reference + // cycle, which is the point. + const inner: Env = { names: [n.name], vals: [{ kind: "num", num: 0 }], parent: env }; + const v = evalNode(n.value, inner); + inner.vals[0] = v; + return evalNode(n.body, inner); + } + const fn = evalNode(n.target, env); + const arg = evalNode(n.arg, env); + if (fn.kind === "clo") { + const inner: Env = { names: [fn.param], vals: [arg], parent: fn.env }; + return evalNode(fn.body, inner); + } + return { kind: "num", num: 0 }; +} + +// ---------- driver ---------- + +const FIB = "let fib = fun n if n <= 1 then n else fib (n - 1) + fib (n - 2) in fib 21"; +const SUMLOOP = "let go = fun i if i <= 0 then 0 else i + go (i - 1) in go 250"; +const STRWORK = 'let cat = fun i if i <= 0 then "" else cat (i - 1) + "x" in cat 400'; + +function main(): void { + let checksum = 0; + const progs: string[] = [FIB, SUMLOOP, STRWORK]; + for (let round = 0; round < 40; round++) { + for (let k = 0; k < progs.length; k++) { + const ast = parse(progs[k]); + const env: Env = { names: [], vals: [], parent: null }; + const out = evalNode(ast, env); + if (out.kind === "num") { + checksum = checksum + out.num; + } else if (out.kind === "str") { + checksum = checksum + out.str.length; + } + } + } + console.log(checksum); +} +main(); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index ed37730c3e..bc6285eddc 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -698,3 +698,28 @@ test_gap_gc_global_builtin_lookup_rooting # file stops covering the walk. test_gap_gc_spread_symbol_iterator_rooting test_gap_repsel_element_shape_loop_clone + +# --- #7682: an allocation-point collection must not MOVE anything ----------- +# +# NOT a rooting witness like every entry above it, and the difference is the +# point. The others plant a value that codegen failed to root and let a moving +# collection expose it. Here codegen's root discipline is COMPLETE for the +# program — `scripts/gc_root_dominance_check.py` reports 0 violations over 380 +# root stores — and the collector still relocated a value out from under a +# register, because it ran an *evacuating* minor at an arbitrary allocation +# point (`gc_check_trigger`'s nursery-churn arm) rather than at a declared +# safepoint. `PERRY_GC_SCAVENGE` gated off the `force_full_scan()` that keeps +# that collection non-moving. See `changelog.d/` for the full route. +# +# NOT LATENT: it prints a wrong NUMBER, not a wrong-but-plausible one. A +# tree-walking interpreter's variable lookup misses bindings that are present, +# and naive `fib` — a count of leaves returning 1 — comes back short by exactly +# the miss count. Measured under the gap harness's own PERRY_NO_AUTO_OPTIMIZE=1: +# +# before (a853135aa runtime) 1708662 / 1708662 / 1708662 oracle 1708840 +# after 1708840 3/3, and 1708840 with auto-optimize +# +# On the `loop_polls` arm this file is a moving-collector witness like the rest +# — back-edge polls put the copying minor at the safepoints, which is where it +# is supposed to be. +test_gap_gc_alloc_point_no_move