diff --git a/CLAUDE.md b/CLAUDE.md index 3c31081f70..e1ad9eb1f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1368 +**Current Version:** 0.5.1369 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 14ba59a530..d9146a9372 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1368" +version = "0.5.1369" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1368" +version = "0.5.1369" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1368" +version = "0.5.1369" [[package]] name = "perry-ui-tvos" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1368" +version = "0.5.1369" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 63d00bd43d..1a3ffcefa4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1368" +version = "0.5.1369" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7651-layer1-slice6-rooted-group.md b/changelog.d/7651-layer1-slice6-rooted-group.md new file mode 100644 index 0000000000..1ed372bc9d --- /dev/null +++ b/changelog.d/7651-layer1-slice6-rooted-group.md @@ -0,0 +1,145 @@ +### Layer 1 rooting migration, slice 6 — the multi-point re-read scope (#7615) + +`crate::rooting::RootedGroup` is the combinator slice 5 could not build: +**one temp-root scope — already-lowered operands and mutable accumulator arrays +together — re-readable at any number of caller-chosen points and released once, +for the whole stack.** `lower_call/mod.rs`, `lower_call/func_ref.rs` and +`lower_call/console_promise.rs` are migrated onto it and listed in the +`MIGRATED_MODULES` ledger; all three named `expr::temp_root` before the +migration, so all three lines are load-bearing on the committed source. + +**Slice 5's hypothesis for the missing shape was wrong in a way worth +recording**, because the next slice would have inherited it. It named the +variadic/rest shape (per-element re-reads between allocating pushes). Only one +of the three blocked modules is variadic: + +* `console_promise.rs`'s `lower_dynamic_closure_call` consumes the group in + **two instructions with an allocating step between them** — receiver and callee + feed `js_closure_unbox_callee_checked_rebind`, which clones a `this`-capturing + closure, and the arguments feed `js_closure_callN` below it; +* `mod.rs`'s `lower_rest_call_args_rooted` re-reads in a **loop**, one per + `js_array_push_f64`; +* `func_ref.rs`'s **release** must post-dominate four block-splitting + specialized-ABI dispatch diamonds, ~450 lines below the lowering. + +What all three want is the scope, not the loop. The variadic case is the scope +that also holds an array, which is why `RootedGroup` carries both kinds rather +than there being a second type for it. + +**Two entry points, and the asymmetry is argued in the source rather than +assumed.** `with_rooted_group` owns the release like every other combinator in +`rooting.rs`. `open_rooted_group` hands the scope back, which the rest of the +file deliberately refuses to do, and the justification is that the two halves of +guard mismanagement are not equally dangerous. An **early or mis-ordered** +release is a use-after-free — a truncate is a stack *cut*, so truncating the +wrong slot drops every slot above it, which is how a saved implicit `this` +becomes the number `0`. A **forgotten** release is over-retention: the slot stays +bound, the object stays live, the emitted code is merely conservative. +`RootedGroup` removes the dangerous half by construction and for both entry +points — it is not `Clone`, `release` consumes it, and there is no way to obtain +the slot index — so escaping leaves only the safe half writable. That is +strictly better than the `Option` slot index it replaces, which a caller +could truncate anywhere. Inverting control in `func_ref.rs` would not remove the +hazard, only relocate it into a 450-line closure. + +`implicit_this_save` / `implicit_this_restore` **moved** into `crate::rooting` +rather than being re-exported, so the pair has one spelling; two spellings of one +decision is the drift that produced #7114. That incidentally clears the escape +hatch out of `early_branches.rs`, `method_override.rs` and both `property_get` +dispatchers, which are deliberately **not** added to the ledger: a line there +asserts that a module makes every rooting decision through this API, and those +four have not been read for windows with no decision at all (slice 4's +"listed ≠ audited" distinction). + +**Four live bugs in the `console.*` arms.** The three behavioural ones are +A/B'd byte-for-byte against node 26.5.1 with a baseline compiler built from +`main` in a separate target directory; `diff` of node's output against the fixed +arm is empty, and against the baseline it is four hunks. + +1. **`console.dir` sequenced side effects after its own print** — #7649's non-GC + arm, which the issue flagged as unverified. It lowered `args[2..]` *below* + `js_console_dir_with_options`, so `console.dir(x, y, sideEffect())` printed + the object and only then ran `sideEffect`. Node evaluates a call's whole + argument list before invoking anything. +2. **`console.time` / `timeEnd` / `timeLog` / `count` / `countReset` dropped + their surplus arguments entirely.** Not resequenced — never lowered. + `console.time("t", sideEffect())` simply did not run `sideEffect`. +3. **`console.table(a, b, c)` stopped being `console.table`.** The arity gate was + `args.len() == 1 || args.len() == 2`, so three or more arguments fell through + to the generic multi-argument `console.log` arm and printed + `[ { a: 1 } ] [ 'a' ] 1` where node renders the table. Node ignores the + surplus arguments; it does not switch renderer. +4. **#7649's rooting half**, demonstrated in IR. `console.table(data, properties)` + and `console.dir(obj, options)` held operand 0 in a bare SSA register across + operand 1's lowering. For `console.table(makeRows(), [churn(300)])` the + baseline emits `%r1 = call @makeRows()`, `%r2 = call @churn(300)` (user code: + allocates, polls), then reads `%r1` — and `root_reload` structurally cannot + repair it, because a call result has no slot to be re-read from (#7280 + taxonomy (c) and (d) at once). + + **The runtime fault is arrangement-dependent and was not reproduced.** Under + `PERRY_GC_MOVING_LOOP_POLLS=1` at compile time plus `PERRY_GC_ZEAL=1` + (`PERRY_GC_DIAG=1` confirms `copied_objects=6005`, so the subject was live), + and again with `PERRY_GC_PROTECT_FROMSPACE=1 …_DEPTH=800`, the baseline + printed the correct table and exited 0. The IR is the evidence that the window + exists; a thrown `TypeError` would only have been evidence that it is + reachable in one arrangement. + +**Two more unprotected windows in the same module, found by reading the arms the +migration did not have to touch** — which is the "listed ≠ audited" distinction +being paid for rather than restated: + +* **`Promise.try(cb, ...extra)` was #7154's accumulator shape verbatim.** + `current_arr` was a raw `*mut ArrayHeader` threaded through the push loop in a + bare SSA register, holding the only reference to everything pushed so far while + the NEXT argument — arbitrary user code — was lowered, and `callback` sat in + another bare register across `js_array_alloc`, every push and every one of + those lowerings. Identical to the `namespace_call.rs` rest-path defect slice 5 + called the most serious of its four. The pre-fix IR threads + `%r55 → %r60 → %r65` through three `churn()` calls with no root; the fixed IR + re-reads the accumulator from its slot between every push. +* **`Array.fromAsync(input, mapFn, thisArg)` held three operands across each + other's lowering** — #7280 taxonomy (c), which `root_reload` cannot repair. + One re-read point serves, so this is the plain `with_operands_rooted` form. + +Neither faults in the arrangements tried (including `PERRY_GC_ZEAL=1` on a +`PERRY_GC_MOVING_LOOP_POLLS=1` build); the IR ordering is the evidence. +`Array.fromAsync` deliberately keeps `take(3)`, so this is a pure rooting fix: +the `Promise.*` statics and `Array.fromAsync` all fail to EVALUATE their surplus +arguments, which is the same node-visible defect as `console.time`'s above, and +fixing that family belongs in its own change with its own oracle A/B. + +**Cost is zero where the window cannot collect, and it is measured.** A probe +exercising direct calls, rest calls, `arguments`, `new`, method dispatch, +dynamic closure calls, `Map.set` and eight `console.*` arms was compiled under +both compilers with `--trace llvm`. Normalising SSA numbering and block labels, +the entire semantic delta is **35 removed lines and 0 added**: two unconditional +`temp_root_push_double` / re-read / clear sequences protecting values +`expr_is_known_non_pointer_shadow_value` proves are not heap references — a +`double` from a scalar-replaced method call, and the literal `true` in +`console.assert(true, …)`. Both arms bypassed the shared `operand_protection` +decision; routing them through it drops the traffic. Over the +`gc_root_dominance_corpus.sh` corpus the root-store count falls 9846 → 9799 with +zero change in violations. + +**Tests.** `lower_call/console_rooting_tests.rs` asserts on IR *ordering* rather +than slot counts — a count would let the other operand's rooting pay for the +assertion — and each test first asserts by callee name that the arm under test +was reached, so a shape measured over a lowering that never ran cannot pass. +Sabotage-verified against the pre-fix source with the file reverted to its `HEAD` +content: `error[` count 0, `Running unittests` present (so the plant compiled +*and* the test binary was reached), 4 of 5 red. The fifth is the zero-cost pin +and correctly passes in both arms. The ledger sabotage arm was run once per newly +listed module — a compiling `temp_root_push_double` / `temp_root_truncate` pair +planted in each, `error[` count 0 in all three, ledger test red and naming both +planted lines. + +**One gate observation, recorded rather than fixed.** +`scripts/gc_root_dominance_check.py --seeded-violations N` runs only after the +real check returns 0, and without `--moving-only` the corpus reports 171 +violations (all non-moving, the +`js_object_alloc_class_inline_keys → js_gc_declare_typed_shape_layout` class) on +`main` and on this branch alike — so a run without that filter never reaches the +"can this gate still fail?" arm and prints nothing about it. In the gate's own +mode (`--moving-only`) it is exercised and reports 40 planted, 40 caught, 0 +missed, with 0 violations in both arms. diff --git a/crates/perry-codegen/src/expr/temp_root.rs b/crates/perry-codegen/src/expr/temp_root.rs index ba35f27975..b8ff6c059b 100644 --- a/crates/perry-codegen/src/expr/temp_root.rs +++ b/crates/perry-codegen/src/expr/temp_root.rs @@ -244,62 +244,6 @@ pub(crate) fn temp_root_truncate(ctx: &mut FnCtx<'_>, idx: &str) { .call_void("js_gc_temp_root_truncate", &[(I32, idx)]); } -/// A saved implicit `this`, held in a temp-root slot for the duration of a -/// dispatch (#7211). -/// -/// `js_implicit_this_set` swaps the `IMPLICIT_THIS` cell and returns what was -/// there. That cell is a registered MUTABLE root — `scan_implicit_this_roots_mut` -/// (`object/this_binding.rs:176`) marks it and rewrites it on an evacuating -/// cycle — and the swap has already overwritten it, so the returned value is -/// now held ONLY in an SSA register, across the whole call the bind exists to -/// scope. A minor inside that call moves the object and rewrites every root -/// that names it, leaving this register on from-space; the restore then writes -/// that pre-move address BACK INTO the cell, so the corruption outlives the -/// call and lands on whatever reads `this` next. -/// -/// Seven lowerings emit this save/restore pair — `js_closure_callN`, the -/// `js_native_call_value` override arms in `method_override.rs` and both -/// `property_get` dispatchers, the static-dispatch arm, the direct-call -/// `#3576` reset in `func_ref.rs` and the two closure-call arms in -/// `early_branches.rs`. They had seven copies of the same three lines and -/// therefore seven copies of the same bug, which is why this is a helper -/// rather than seven edits: the next lowering that needs the pair gets the -/// root for free. -/// -/// Unconditional, unlike [`RootedOperands`]: the window is a user or native -/// call, so [`operand_protection`]'s "can this window collect?" test has -/// exactly one answer and there is nothing to gate on. -pub(crate) struct ImplicitThisSave { - slot: String, -} - -/// Bind `new_this` as the implicit `this` and root the value it displaced. -pub(crate) fn implicit_this_save(ctx: &mut FnCtx<'_>, new_this: &str) -> ImplicitThisSave { - let prev = ctx - .block() - .call(DOUBLE, "js_implicit_this_set", &[(DOUBLE, new_this)]); - let slot = temp_root_push_double(ctx, &prev); - ImplicitThisSave { slot } -} - -/// Restore the saved implicit `this`, re-read from its root. -/// -/// Reading the slot rather than the register is the fix, not a precaution: the -/// slot is a mutable root, so an evacuating cycle inside the dispatch rewrote -/// it and the register pushed beforehand names from-space. -/// -/// The truncate is emitted BEFORE the restore call so that nested saves — an -/// override arm inside an outer bind — release inner to outer. -/// `js_gc_temp_root_truncate` drops everything at or above its argument, so a -/// caller holding a LOWER group (`RootedOperands`) may release it afterwards -/// and drop this slot again harmlessly. -pub(crate) fn implicit_this_restore(ctx: &mut FnCtx<'_>, save: ImplicitThisSave) { - let prev = temp_root_get_double(ctx, &save.slot); - temp_root_truncate(ctx, &save.slot); - ctx.block() - .call(DOUBLE, "js_implicit_this_set", &[(DOUBLE, &prev)]); -} - /// Push `value` onto the array held in temp-root slot `idx`, writing the /// possibly-reallocated array pointer back into the slot. /// diff --git a/crates/perry-codegen/src/lower_call/console_promise.rs b/crates/perry-codegen/src/lower_call/console_promise.rs index d2734e0a27..9c072efd73 100644 --- a/crates/perry-codegen/src/lower_call/console_promise.rs +++ b/crates/perry-codegen/src/lower_call/console_promise.rs @@ -13,10 +13,11 @@ use anyhow::Result; use perry_hir::types::Type as HirType; use perry_hir::Expr; -use crate::expr::temp_root::{ - rooted_array_begin, rooted_array_read, temp_root_get_double, temp_root_push_double, - temp_root_truncate, temp_rooted_array_push, +use crate::rooting::{ + any_operand_may_collect, implicit_this_restore, implicit_this_save, operand_may_collect, + with_operands_rooted, with_rooted_group, }; + use crate::expr::{ emit_typed_feedback_register_site, lower_expr, nanbox_pointer_inline, FnCtx, TypedFeedbackContract, TypedFeedbackKind, @@ -39,6 +40,33 @@ fn util_types_arg_is_async_function_static(ctx: &FnCtx<'_>, expr: &Expr) -> Opti } } +/// Lower every argument of a `console.*` call into `group`, left to right, with +/// each already-evaluated value rooted across the evaluation of the ones that +/// follow it, and re-read below the last of them. +/// +/// **Every** argument, including the ones the method ignores. Node evaluates a +/// call's whole argument list before it invokes the callee, so +/// `console.dir(o, opts, sideEffect())` runs `sideEffect` BEFORE anything is +/// printed. Two arms here used to evaluate the surplus arguments after the +/// print (`dir`) or not at all (`time` / `count`), which is an observable +/// difference rather than a formatting one — see #7649. +/// +/// The window per argument is "the arguments after it": the consuming +/// `js_console_*` call is emitted immediately below the last one, and an +/// argument that is a literal or a proven non-pointer costs nothing, so a call +/// like `console.table(rows)` emits exactly the IR it emitted before. +fn lower_console_args<'a>( + ctx: &mut FnCtx<'_>, + group: &mut crate::rooting::RootedGroup<'a>, + args: &'a [Expr], +) -> Result> { + for (i, arg) in args.iter().enumerate() { + let collects = any_operand_may_collect(ctx, args[i + 1..].iter()); + group.lower(ctx, arg, collects)?; + } + group.reread_all(ctx) +} + fn nanbox_bool_literal(value: bool) -> String { double_literal(f64::from_bits(if value { crate::nanbox::TAG_TRUE @@ -282,32 +310,49 @@ pub fn try_lower_console_call( ctx.block().call_void("js_console_trace", &[(DOUBLE, &val)]); } else { let cap = (args.len() as u32).to_string(); - let acc = rooted_array_begin(ctx, &cap); - for arg in args.iter() { - let v = lower_expr(ctx, arg)?; - temp_rooted_array_push(ctx, &acc, &v); - } - let current_arr = rooted_array_read(ctx, &acc); - ctx.block() - .call_void("js_console_trace_spread", &[(I64, ¤t_arr)]); - temp_root_truncate(ctx, &acc); + with_rooted_group(ctx, 0, |ctx, group| { + let acc = group.begin_array(ctx, &cap); + for arg in args.iter() { + let v = lower_expr(ctx, arg)?; + group.push_array(ctx, acc, &v); + } + let current_arr = group.read_array(ctx, acc); + ctx.block() + .call_void("js_console_trace_spread", &[(I64, ¤t_arr)]); + Ok(()) + })?; } return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); } // console.table(data[, properties]) — dedicated table renderer. - if property == "table" && (args.len() == 1 || args.len() == 2) { - let v = lower_expr(ctx, &args[0])?; - if args.len() == 2 { - let props = lower_expr(ctx, &args[1])?; - ctx.block().call_void( - "js_console_table_with_properties", - &[(DOUBLE, &v), (DOUBLE, &props)], - ); - } else { - ctx.block().call_void("js_console_table", &[(DOUBLE, &v)]); - } + // + // #7649, two defects in five lines. `data` was lowered into a bare + // SSA register and then held across `properties`' lowering, which is + // arbitrary user code: `console.table(makeRows(), [churn()])` emits + // `%r1 = call @makeRows`, `%r2 = call @churn`, then reads `%r1` — + // and `root_reload` structurally cannot repair it, because a call + // result has no slot to be re-read from (#7280 taxonomy (c)+(d)). + // And the arity gate rejected three or more arguments, so + // `console.table(rows, cols, x)` fell through to the generic + // multi-arg console.log arm and printed the array instead of a + // table. Node ignores the surplus arguments; it does not stop being + // `console.table`. + if property == "table" && !args.is_empty() { + with_rooted_group(ctx, args.len(), |ctx, group| { + let values = lower_console_args(ctx, group, args)?; + if values.len() >= 2 { + ctx.block().call_void( + "js_console_table_with_properties", + &[(DOUBLE, &values[0]), (DOUBLE, &values[1])], + ); + } else { + ctx.block() + .call_void("js_console_table", &[(DOUBLE, &values[0])]); + } + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -322,25 +367,26 @@ pub fn try_lower_console_call( "time" | "timeEnd" | "timeLog" | "count" | "countReset" ) && !args.is_empty() { - let v = lower_expr(ctx, &args[0])?; if property == "timeLog" && args.len() > 1 { - // `v` (the label) is itself an evaluated temporary held - // across the extra arguments' evaluation, so it needs a - // root of its own alongside the accumulator (#6951). - let label = temp_root_push_double(ctx, &v); - let cap = ((args.len() - 1) as u32).to_string(); - let acc = rooted_array_begin(ctx, &cap); - for arg in args.iter().skip(1) { - let extra = lower_expr(ctx, arg)?; - temp_rooted_array_push(ctx, &acc, &extra); - } - let current_arr = rooted_array_read(ctx, &acc); - let v = temp_root_get_double(ctx, &label); - ctx.block().call_void( - "js_console_time_log_spread", - &[(DOUBLE, &v), (I64, ¤t_arr)], - ); - temp_root_truncate(ctx, &label); + // The label is an evaluated temporary held across the extra + // arguments' evaluation, so it is rooted alongside the + // accumulator (#6951) — both in one scope, released once. + with_rooted_group(ctx, 1, |ctx, group| { + let label = group.lower(ctx, &args[0], true)?; + let cap = ((args.len() - 1) as u32).to_string(); + let acc = group.begin_array(ctx, &cap); + for arg in args.iter().skip(1) { + let extra = lower_expr(ctx, arg)?; + group.push_array(ctx, acc, &extra); + } + let current_arr = group.read_array(ctx, acc); + let v = group.reread(ctx, label)?; + ctx.block().call_void( + "js_console_time_log_spread", + &[(DOUBLE, &v), (I64, ¤t_arr)], + ); + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -353,7 +399,17 @@ pub fn try_lower_console_call( "countReset" => "js_console_count_reset_value", _ => unreachable!(), }; - ctx.block().call_void(runtime_fn, &[(DOUBLE, &v)]); + // #7649's sibling: only the label is used, but every argument + // is EVALUATED, because node evaluates the whole list before + // the call. `console.time("t", sideEffect())` used to lower + // `args[0]` alone and drop the rest on the floor — the side + // effect simply never happened. With a single argument nothing + // is rooted and the emitted IR is unchanged. + with_rooted_group(ctx, args.len(), |ctx, group| { + let values = lower_console_args(ctx, group, args)?; + ctx.block().call_void(runtime_fn, &[(DOUBLE, &values[0])]); + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -405,21 +461,25 @@ pub fn try_lower_console_call( .call_void("js_console_assert", &[(DOUBLE, &cond_v), (I64, "0")]); } else { // Multi-arg messages: bundle args[1..] into a heap - // array and call the spread variant. - let cond_root = temp_root_push_double(ctx, &cond_v); - let cap = ((args.len() - 1) as u32).to_string(); - let acc = rooted_array_begin(ctx, &cap); - for arg in args.iter().skip(1) { - let v = lower_expr(ctx, arg)?; - temp_rooted_array_push(ctx, &acc, &v); - } - let current_arr = rooted_array_read(ctx, &acc); - let cond_v = temp_root_get_double(ctx, &cond_root); - ctx.block().call_void( - "js_console_assert_spread", - &[(DOUBLE, &cond_v), (I64, ¤t_arr)], - ); - temp_root_truncate(ctx, &cond_root); + // array and call the spread variant. The condition is an + // evaluated temporary held across every message's + // evaluation, so it shares the accumulator's scope. + with_rooted_group(ctx, 1, |ctx, group| { + let cond = group.adopt(ctx, &args[0], &cond_v, true); + let cap = ((args.len() - 1) as u32).to_string(); + let acc = group.begin_array(ctx, &cap); + for arg in args.iter().skip(1) { + let v = lower_expr(ctx, arg)?; + group.push_array(ctx, acc, &v); + } + let current_arr = group.read_array(ctx, acc); + let cond_v = group.reread(ctx, cond)?; + ctx.block().call_void( + "js_console_assert_spread", + &[(DOUBLE, &cond_v), (I64, ¤t_arr)], + ); + Ok(()) + })?; } return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, @@ -431,20 +491,25 @@ pub fn try_lower_console_call( // customInspect=false #1201). Missing `options` arg becomes // `undefined` and the option decoders fall back to their // Node-compatible defaults. + // + // #7649, and the non-GC half is the one that shows up in output. + // `args[2..]` were lowered AFTER the call that produces the print, + // so `console.dir(x, y, sideEffect())` printed the object and only + // then ran `sideEffect` — node runs it first, because it evaluates + // the whole argument list before invoking anything. The GC half is + // `console.table`'s: `obj` was held in a bare register across + // `options`' lowering. if property == "dir" && !args.is_empty() { - let v = lower_expr(ctx, &args[0])?; - let opts = if args.len() >= 2 { - lower_expr(ctx, &args[1])? - } else { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - }; - ctx.block().call_void( - "js_console_dir_with_options", - &[(DOUBLE, &v), (DOUBLE, &opts)], - ); - for a in args.iter().skip(2) { - let _ = lower_expr(ctx, a)?; - } + with_rooted_group(ctx, args.len(), |ctx, group| { + let values = lower_console_args(ctx, group, args)?; + let undef = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); + let opts = values.get(1).cloned().unwrap_or(undef); + ctx.block().call_void( + "js_console_dir_with_options", + &[(DOUBLE, &values[0]), (DOUBLE, &opts)], + ); + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -461,14 +526,6 @@ pub fn try_lower_console_call( } else { lower_expr(ctx, arg)? }; - // The accumulator is allocated AFTER the argument, so - // `js_array_alloc` is itself a collection point with `v` live - // only in an SSA register. Root `v` across it (#6951). - let v_root = temp_root_push_double(ctx, &v); - let acc = rooted_array_begin(ctx, "1"); - let v = temp_root_get_double(ctx, &v_root); - temp_rooted_array_push(ctx, &acc, &v); - let current_arr = rooted_array_read(ctx, &acc); let runtime_fn = match property.as_str() { "info" => "js_console_info_spread", "debug" => "js_console_debug_spread", @@ -476,10 +533,20 @@ pub fn try_lower_console_call( "error" => "js_console_error_spread", _ => "js_console_log_spread", }; - ctx.block().call_void(runtime_fn, &[(I64, ¤t_arr)]); - // Drops `v_root` and everything above it, the accumulator - // included — a truncate is a stack cut, not a single pop. - temp_root_truncate(ctx, &v_root); + // The accumulator is allocated AFTER the argument, so + // `js_array_alloc` is itself a collection point with `v` live + // only in an SSA register. Root `v` across it (#6951). One + // release drops the value's slot and the accumulator's — a + // truncate is a stack cut, not a single pop. + with_rooted_group(ctx, 1, |ctx, group| { + let held = group.adopt(ctx, arg, &v, true); + let acc = group.begin_array(ctx, "1"); + let v = group.reread(ctx, held)?; + group.push_array(ctx, acc, &v); + let current_arr = group.read_array(ctx, acc); + ctx.block().call_void(runtime_fn, &[(I64, ¤t_arr)]); + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -499,16 +566,6 @@ pub fn try_lower_console_call( // Keep it in a temp root and re-read it, so it survives and follows // an evacuating cycle. let cap = (args.len() as u32).to_string(); - let acc = rooted_array_begin(ctx, &cap); - for arg in args.iter() { - let v = if let Some(v) = lower_util_types_predicate_arg(ctx, arg)? { - v - } else { - lower_expr(ctx, arg)? - }; - temp_rooted_array_push(ctx, &acc, &v); - } - let current_arr = rooted_array_read(ctx, &acc); let runtime_fn = match property.as_str() { "info" => "js_console_info_spread", "debug" => "js_console_debug_spread", @@ -516,8 +573,20 @@ pub fn try_lower_console_call( "error" => "js_console_error_spread", _ => "js_console_log_spread", }; - ctx.block().call_void(runtime_fn, &[(I64, ¤t_arr)]); - temp_root_truncate(ctx, &acc); + with_rooted_group(ctx, 0, |ctx, group| { + let acc = group.begin_array(ctx, &cap); + for arg in args.iter() { + let v = if let Some(v) = lower_util_types_predicate_arg(ctx, arg)? { + v + } else { + lower_expr(ctx, arg)? + }; + group.push_array(ctx, acc, &v); + } + let current_arr = group.read_array(ctx, acc); + ctx.block().call_void(runtime_fn, &[(I64, ¤t_arr)]); + Ok(()) + })?; return Ok(Some(double_literal(f64::from_bits( crate::nanbox::TAG_UNDEFINED, )))); @@ -594,29 +663,39 @@ pub fn try_lower_promise_static_call( return Ok(Some(nanbox_pointer_inline(blk, &handle))); } "try" => { - let callback = if args.is_empty() { - double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)) - } else { - lower_expr(ctx, &args[0])? - }; + // #7154's accumulator shape verbatim, with the callback on + // top of it. `current_arr` was a raw `*mut ArrayHeader` in a + // bare SSA register threaded through the push loop, holding + // the only reference to everything pushed so far while the + // NEXT argument — arbitrary user code — was lowered; and + // `callback` sat in another bare register across + // `js_array_alloc`, every push and every one of those + // lowerings. Identical to the `namespace_call.rs` rest-path + // defect slice 5 found, which printed a wrong answer on the + // default build with no GC instrumentation at all. let extra_count = args.len().saturating_sub(1); - let mut current_arr = - ctx.block() - .call(I64, "js_array_alloc", &[(I32, &extra_count.to_string())]); - for arg in args.iter().skip(1) { - let value = lower_expr(ctx, arg)?; - current_arr = ctx.block().call( + let handle = with_rooted_group(ctx, 1, |ctx, group| { + let callback = match args.first() { + Some(cb) => Some(group.lower(ctx, cb, true)?), + None => None, + }; + let acc = group.begin_array(ctx, &extra_count.to_string()); + for arg in args.iter().skip(1) { + let value = lower_expr(ctx, arg)?; + group.push_array(ctx, acc, &value); + } + let current_arr = group.read_array(ctx, acc); + let callback = match callback { + Some(i) => group.reread(ctx, i)?, + None => double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)), + }; + Ok(ctx.block().call( I64, - "js_array_push_f64", - &[(I64, ¤t_arr), (DOUBLE, &value)], - ); - } + "js_promise_try", + &[(DOUBLE, &callback), (I64, ¤t_arr)], + )) + })?; let blk = ctx.block(); - let handle = blk.call( - I64, - "js_promise_try", - &[(DOUBLE, &callback), (I64, ¤t_arr)], - ); return Ok(Some(nanbox_pointer_inline(blk, &handle))); } _ => {} @@ -624,28 +703,28 @@ pub fn try_lower_promise_static_call( } // `Array.fromAsync(input, mapFn?, thisArg?)` — Node 22+ static method. if is_global_constructor_expr(object, "Array") && property == "fromAsync" { - let undefined = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - let input = if let Some(arg) = args.first() { - lower_expr(ctx, arg)? - } else { - undefined.clone() - }; - let map_fn = if let Some(arg) = args.get(1) { - lower_expr(ctx, arg)? - } else { - undefined.clone() - }; - let this_arg = if let Some(arg) = args.get(2) { - lower_expr(ctx, arg)? - } else { - undefined - }; - let blk = ctx.block(); - return Ok(Some(blk.call( - DOUBLE, - "js_array_from_async", - &[(DOUBLE, &input), (DOUBLE, &map_fn), (DOUBLE, &this_arg)], - ))); + // Operand-to-operand windows, three deep: `input` was held in a bare + // register across `mapFn`'s and `thisArg`'s lowering and `mapFn` + // across `thisArg`'s, both of which are arbitrary user code. + // #7280 taxonomy (c), which `root_reload` cannot repair. + // + // One re-read point serves: all three are consumed by the single + // `js_array_from_async` below. `take(3)` keeps this a pure rooting + // fix — surplus arguments are not evaluated here today, which is a + // separate (node-visible) defect shared with the `Promise.*` statics + // above, and changing it belongs with theirs. + let operands: Vec<&Expr> = args.iter().take(3).collect(); + return with_operands_rooted(ctx, &operands, |ctx, values| { + let undefined = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); + let input = values.first().cloned().unwrap_or_else(|| undefined.clone()); + let map_fn = values.get(1).cloned().unwrap_or_else(|| undefined.clone()); + let this_arg = values.get(2).cloned().unwrap_or(undefined); + Ok(Some(ctx.block().call( + DOUBLE, + "js_array_from_async", + &[(DOUBLE, &input), (DOUBLE, &map_fn), (DOUBLE, &this_arg)], + ))) + }); } } Ok(None) @@ -874,81 +953,76 @@ pub fn try_lower_native_method_str_dispatch( // Same shape, same fix as #7192's property/element STORE receiver: // a temp root, not a re-lower. Re-lowering `object` would observe an // assignment made by an argument, which is a miscompile rather than - // a rooting fix (see `temp_root::operand_is_reloadable`). Each - // argument is likewise rooted before the NEXT one is lowered, so an - // earlier argument cannot go stale across a later one. - let arg_collects: Vec = args - .iter() - .map(|a| crate::expr::temp_root::expr_may_trigger_gc(ctx, a)) - .collect(); - let any_arg_collects = arg_collects.iter().any(|&c| c); + // a rooting fix. Each argument is likewise rooted before the NEXT + // one is lowered, so an earlier argument cannot go stale across a + // later one. + // + // One re-read point serves this arm — everything the group protects + // is consumed by the single dispatch below it — so this is the + // plain `with_operands_rooted` form rather than the multi-stage one + // its `lower_dynamic_closure_call` sibling needs. The per-operand + // window it derives (`the operands after this one`) is exactly the + // hand-computed `any_arg_collects` / `arg_collects[i + 1..]` pair it + // replaces. let operand_exprs: Vec<&Expr> = std::iter::once(object.as_ref()) .chain(args.iter()) .collect(); - let mut roots = crate::expr::temp_root::root_operands_begin(args.len() + 1); - let recv_box = lower_expr(ctx, object)?; - roots.push(ctx, object.as_ref(), &recv_box, any_arg_collects); - for (i, a) in args.iter().enumerate() { - let v = lower_expr(ctx, a)?; - roots.push(ctx, a, &v, arg_collects[i + 1..].iter().any(|&c| c)); - } - // Re-read below the last collection point. Mandatory, not - // defensive: the temp-root slot is a MUTABLE root, so an evacuating - // cycle rewrites it and the register pushed beforehand is stale. - let rereads = roots.reread(ctx, &operand_exprs)?; - let recv_box = rereads[0].clone(); - let lowered_args: Vec = rereads[1..].to_vec(); - // Pass a tagged pointer to the immutable StringPool dispatch - // descriptor. A GC-backed string handle belongs to the main - // thread's arena and cannot be resolved safely by a - // `perry/thread` worker executing this compiled closure. - let key_idx = ctx.strings.intern(property); - let dispatch_global = ctx.strings.static_dispatch_global(key_idx); - let method_id = crate::strings::emit_static_dispatch_id(ctx.block(), &dispatch_global); - // Stack-allocate the args array if any. The alloca MUST live in - // the function entry block — emitting it into the current block - // (which may be a loop body) makes LLVM lower it as a runtime - // `sub %rsp, N` that never gets restored, eating the stack at - // ~16 bytes/iteration. See issue #167. - let (args_ptr, args_len_str) = if lowered_args.is_empty() { - ("null".to_string(), "0".to_string()) - } else { - let n = lowered_args.len(); - let buf_reg = ctx.func.alloca_entry_array(DOUBLE, n); + return with_operands_rooted(ctx, &operand_exprs, |ctx, rereads| { + let recv_box = rereads[0].clone(); + let lowered_args: Vec = rereads[1..].to_vec(); + // Pass a tagged pointer to the immutable StringPool dispatch + // descriptor. A GC-backed string handle belongs to the main + // thread's arena and cannot be resolved safely by a + // `perry/thread` worker executing this compiled closure. + let key_idx = ctx.strings.intern(property); + let dispatch_global = ctx.strings.static_dispatch_global(key_idx); + let method_id = + crate::strings::emit_static_dispatch_id(ctx.block(), &dispatch_global); + // Stack-allocate the args array if any. The alloca MUST live in + // the function entry block — emitting it into the current block + // (which may be a loop body) makes LLVM lower it as a runtime + // `sub %rsp, N` that never gets restored, eating the stack at + // ~16 bytes/iteration. See issue #167. + let (args_ptr, args_len_str) = if lowered_args.is_empty() { + ("null".to_string(), "0".to_string()) + } else { + let n = lowered_args.len(); + let buf_reg = ctx.func.alloca_entry_array(DOUBLE, n); + let blk = ctx.block(); + for (i, v) in lowered_args.iter().enumerate() { + let slot = blk.gep(DOUBLE, &buf_reg, &[(I64, &format!("{}", i))]); + blk.store(DOUBLE, v, &slot); + } + (buf_reg, n.to_string()) + }; + let site_id = emit_typed_feedback_register_site( + ctx, + TypedFeedbackKind::MethodCall, + property, + TypedFeedbackContract::method_call(), + ); + // #5247: record the source location right before the dynamic + // dispatch so a thrown "X is not a function" TypeError carries + // `at :`. Args are already lowered above, so a nested + // call's location no longer shadows this one. + crate::expr::calls::emit_call_location_at(ctx, call_byte_offset); let blk = ctx.block(); - for (i, v) in lowered_args.iter().enumerate() { - let slot = blk.gep(DOUBLE, &buf_reg, &[(I64, &format!("{}", i))]); - blk.store(DOUBLE, v, &slot); - } - (buf_reg, n.to_string()) - }; - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::MethodCall, - property, - TypedFeedbackContract::method_call(), - ); - // #5247: record the source location right before the dynamic - // dispatch so a thrown "X is not a function" TypeError carries - // `at :`. Args are already lowered above, so a nested - // call's location no longer shadows this one. - crate::expr::calls::emit_call_location_at(ctx, call_byte_offset); - let blk = ctx.block(); - let result = blk.call( - DOUBLE, - "js_typed_feedback_native_call_method_by_id", - &[ - (I64, &site_id), - (DOUBLE, &recv_box), - (I64, &method_id), - (PTR, &args_ptr), - (I64, &args_len_str), - ], - ); - // Release AFTER the dispatch, not before: the dispatcher allocates - // while it reads these values. - roots.release(ctx); - return Ok(Some(result)); + let result = blk.call( + DOUBLE, + "js_typed_feedback_native_call_method_by_id", + &[ + (I64, &site_id), + (DOUBLE, &recv_box), + (I64, &method_id), + (PTR, &args_ptr), + (I64, &args_len_str), + ], + ); + // The group is released AFTER the dispatch, not before: the + // dispatcher allocates while it reads these values. That is the + // combinator's own contract now, not a line a future edit can move. + Ok(Some(result)) + }); } } Ok(None) @@ -1297,25 +1371,42 @@ pub fn try_lower_closure_call_fallthrough( // // Temp roots, not re-lowering: re-lowering the callee or the receiver would // observe an assignment made by an argument, which is a miscompile rather - // than a rooting fix (`temp_root::operand_is_reloadable`). - let arg_collects: Vec = args - .iter() - .map(|a| crate::expr::temp_root::expr_may_trigger_gc(ctx, a)) - .collect(); + // than a rooting fix. + // + // This is the lowering that named the missing combinator. It consumes the + // group in TWO instructions with an allocating step between them, so it + // needs a re-read at two points, and every `with_operands_rooted*` form has + // exactly one. The scope now belongs to `with_rooted_group`; the body is a + // separate function only so that owning the release costs a wrapper rather + // than a 260-line reindentation. + with_rooted_group(ctx, args.len() + 2, |ctx, group| { + lower_closure_call_rooted(ctx, group, callee, args, call_byte_offset) + }) +} + +/// The rooted body of [`try_lower_closure_call_fallthrough`]. +/// +/// Everything it protects lives in `group`, which its caller releases on every +/// path out — including this function's `?`. +fn lower_closure_call_rooted<'a>( + ctx: &mut FnCtx<'_>, + group: &mut crate::rooting::RootedGroup<'a>, + callee: &'a Expr, + args: &'a [Expr], + call_byte_offset: u32, +) -> Result> { + let arg_collects: Vec = args.iter().map(|a| operand_may_collect(ctx, a)).collect(); let any_arg_collects = arg_collects.iter().any(|&c| c); // Reading the callee off the receiver: a by-name property get walks a // prototype chain and can run an accessor, so it is a collection point in // the receiver's window (and only in the receiver's). - let callee_read_collects = crate::expr::temp_root::expr_may_trigger_gc(ctx, callee); + let callee_read_collects = operand_may_collect(ctx, callee); // Operands are recorded in the order their values are produced — receiver, - // callee, then arguments — because `RootedOperands` roots each one BEFORE - // the next is lowered. Rooting a finished list afterwards is worse than - // doing nothing: by then an earlier operand may already have been swept and - // the push publishes a dangling pointer into a slot the collector scans. - let mut roots = crate::expr::temp_root::root_operands_begin(args.len() + 2); - let mut operand_exprs: Vec<&Expr> = Vec::with_capacity(args.len() + 2); - + // callee, then arguments — because the group roots each one BEFORE the next + // is lowered. Rooting a finished list afterwards is worse than doing + // nothing: by then an earlier operand may already have been swept and the + // push publishes a dangling pointer into a slot the collector scans. let prelowered_recv: Option<(String, String)> = if let Expr::PropertyGet { object, property, .. @@ -1358,9 +1449,10 @@ pub fn try_lower_closure_call_fallthrough( None => lower_expr(ctx, obj_expr)?, }; // Rooted here, before the callee read below: nothing has collected - // between the lowering above and this push. - roots.push(ctx, obj_expr, &v, callee_read_collects || any_arg_collects); - operand_exprs.push(obj_expr); + // between the lowering above and this push. `adopt` rather than + // `lower`, because the prelowered arm produced this value itself + // and the receiver must be evaluated exactly once. + group.adopt(ctx, obj_expr, &v, callee_read_collects || any_arg_collects); Some(v) } None => None, @@ -1388,8 +1480,9 @@ pub fn try_lower_closure_call_fallthrough( } else { lower_expr(ctx, callee)? }; - roots.push(ctx, callee, &recv_box, any_arg_collects); - operand_exprs.push(callee); + // `adopt`: on the prelowered arm the callee's value is the hand-emitted + // by-name read above, not `lower_expr(callee)`. + group.adopt(ctx, callee, &recv_box, any_arg_collects); // The rebind unbox allocates (see the header above), and it sits between // the last argument and `js_closure_callN`, so every argument's window @@ -1397,10 +1490,8 @@ pub fn try_lower_closure_call_fallthrough( // their old IR. let rebind_allocates = method_recv.is_some(); for (i, a) in args.iter().enumerate() { - let v = lower_expr(ctx, a)?; let collects = rebind_allocates || arg_collects[i + 1..].iter().any(|&c| c); - roots.push(ctx, a, &v, collects); - operand_exprs.push(a); + group.lower(ctx, a, collects)?; } // Re-read the receiver and the callee HERE: below every argument, above the @@ -1408,10 +1499,10 @@ pub fn try_lower_closure_call_fallthrough( // is a MUTABLE root, so an evacuating cycle rewrites it and the register // pushed beforehand is stale. let method_recv: Option = match recv_slot { - Some(i) => Some(roots.reread_one(ctx, &operand_exprs, i)?), + Some(i) => Some(group.reread(ctx, i)?), None => None, }; - let recv_box = roots.reread_one(ctx, &operand_exprs, callee_slot)?; + let recv_box = group.reread(ctx, callee_slot)?; // #7211: the value `js_implicit_this_set` hands back is the PREVIOUS // implicit `this`, read straight out of the `IMPLICIT_THIS` cell — which @@ -1450,7 +1541,7 @@ pub fn try_lower_closure_call_fallthrough( // `implicit_this_restore` is the shared form, so a seventh cannot // reintroduce this by copy-paste. let prev_this_root = if let Some(ref this_val) = method_recv { - Some(crate::expr::temp_root::implicit_this_save(ctx, this_val)) + Some(implicit_this_save(ctx, this_val)) } else if !matches!(callee, Expr::PropertyGet { .. }) { // Receiverless closure-value call (`fn()`, IIFE, `curry(1)(2)`): // OrdinaryCallBindThis binds `this` to undefined — without the @@ -1463,7 +1554,7 @@ pub fn try_lower_closure_call_fallthrough( // from inside `o.m()`, and dropping that object is #3576's leak with // the sign flipped. let undef = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - Some(crate::expr::temp_root::implicit_this_save(ctx, &undef)) + Some(implicit_this_save(ctx, &undef)) } else { None }; @@ -1519,7 +1610,7 @@ pub fn try_lower_closure_call_fallthrough( let arg_base = callee_slot + 1; let mut lowered_args: Vec = Vec::with_capacity(args.len()); for i in 0..args.len() { - lowered_args.push(roots.reread_one(ctx, &operand_exprs, arg_base + i)?); + lowered_args.push(group.reread(ctx, arg_base + i)?); } let result = if lowered_args.len() <= 16 { @@ -1573,10 +1664,9 @@ pub fn try_lower_closure_call_fallthrough( // what makes the order correct in both shapes rather than in the common // one. if let Some(prev) = prev_this_root { - crate::expr::temp_root::implicit_this_restore(ctx, prev); + implicit_this_restore(ctx, prev); } - // Released AFTER the dispatch, not before: the dispatcher allocates while - // it reads these values. - roots.release(ctx); + // The group is released by `with_rooted_group` AFTER this returns, which is + // after the dispatch — the dispatcher allocates while it reads these values. Ok(Some(result)) } diff --git a/crates/perry-codegen/src/lower_call/console_rooting_tests.rs b/crates/perry-codegen/src/lower_call/console_rooting_tests.rs new file mode 100644 index 0000000000..3572520440 --- /dev/null +++ b/crates/perry-codegen/src/lower_call/console_rooting_tests.rs @@ -0,0 +1,278 @@ +//! Rooting and evaluation-order coverage for the `console.*` arms slice 6 of +//! the Layer 1 migration (#7615) repaired — #7649 and its two siblings. +//! +//! # Why unit tests on IR rather than gap tests +//! +//! Three of the four defects here ARE observable from a `.ts` program and one +//! is not, and mixing them in one suite would hide which is which: +//! +//! * `console.dir(x, y, f())` running `f` after the print, `console.time(l, +//! f())` never running `f` at all, and `console.table(a, b, c)` printing a +//! log line instead of a table are all plain evaluation-order / dispatch +//! facts. They are asserted here on IR *ordering* because that is where the +//! property is unconditional, and separately A/B'd against node in the PR; +//! * the rooting half — `data` held in a bare register across `properties`' +//! lowering — needs `PERRY_GC_MOVING_LOOP_POLLS=1` at **compile** time (off +//! by default since #7161) plus an arrangement in which the victim's bytes +//! are actually recycled. A gap test would be green on the default build +//! whether or not the fix is present, which is CLAUDE.md hazard 4. +//! +//! # What is asserted, and why it cannot pass vacuously +//! +//! Never a slot count for the rooting arms: counting root slots across two +//! programs that differ in an operand lets the *other* operand's rooting pay +//! for the assertion. What is asserted is the ordering that IS the bug — the +//! register the consuming call reads must be defined BELOW the allocation it +//! must survive, which can only happen if the value was rooted above the window +//! and re-read below it. Each test first asserts, by callee name, that the arm +//! under test was reached at all. +//! +//! The zero-cost arm is the counterpart: a single-argument `console.table` +//! cannot collect between its operand and the call, so `operand_protection` +//! must route it to `Reuse` and emit no temp-root traffic. Without that, a +//! future "root everything" change would tax every `console.table(rows)` in +//! every program and no test would notice. + +use perry_hir::types::Type; +use perry_hir::{Expr, Function, Module as HirModule, Stmt}; + +/// Compile a one-function module and return its LLVM IR. +fn compile_body(name: &str, body: Vec) -> String { + let mut hir = HirModule::new(name); + hir.functions.push(Function { + id: 0, + name: "build".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body, + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }); + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + let bytes = crate::compile_module(&hir, opts).expect("test module compiles"); + String::from_utf8(bytes).expect("LLVM IR is UTF-8") +} + +/// `console.(args…)`. +/// +/// The receiver must be a `GlobalGet` — that is what `try_lower_console_call` +/// dispatches on — and it is never lowered by these arms, so any id serves. +fn console_call(method: &str, args: Vec) -> Vec { + vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::GlobalGet(0)), + property: method.to_string(), + byte_offset: 0, + }), + args, + type_args: Vec::new(), + byte_offset: 0, + })] +} + +/// A heap value whose lowering allocates, so the window it sits in collects. +/// +/// An object literal rather than a call: codegen does not type-check the +/// argument, `operand_needs_root` protects it exactly as it protects any other +/// heap operand, and it keeps the HIR small enough to read. +fn allocating(tag: &str) -> Expr { + Expr::Object(vec![(tag.to_string(), Expr::Number(1.0))]) +} + +/// Line index of a **call** to `callee`, or `None`. +/// +/// Excluding `declare` is load-bearing rather than tidy: the module carries a +/// `declare` for the helper whether or not anything calls it, so a liveness +/// check that accepted it would be satisfied by a lowering that never ran — +/// hazard 4 wearing the subject's name. +fn call_line(ir: &str, callee: &str) -> Option { + let needle = format!("@{callee}("); + ir.lines() + .position(|l| l.contains(&needle) && !l.trim_start().starts_with("declare")) +} + +fn require_call_line(ir: &str, callee: &str) -> usize { + call_line(ir, callee).unwrap_or_else(|| panic!("no call to {callee} in:\n{ir}")) +} + +/// The first `double` SSA operand of the call to `callee`. +fn first_double_operand(ir: &str, callee: &str) -> String { + let idx = require_call_line(ir, callee); + let line = ir.lines().nth(idx).expect("line index came from this IR"); + line.split("(double ") + .nth(1) + .unwrap_or_else(|| panic!("{callee} takes a double first argument: {line}")) + .split([',', ')']) + .next() + .expect("the first argument is comma- or paren-terminated") + .trim() + .to_string() +} + +/// Line index at which `reg` is defined. +fn definition_line(ir: &str, reg: &str) -> usize { + let prefix = format!("{reg} = "); + ir.lines() + .position(|l| l.trim_start().starts_with(&prefix)) + .unwrap_or_else(|| panic!("no definition for {reg} in:\n{ir}")) +} + +/// Line index of the LAST allocation emitted before `before`. +fn last_alloc_before(ir: &str, before: usize) -> usize { + ir.lines() + .enumerate() + .take(before) + .filter(|(_, l)| l.contains("@js_object_alloc")) + .map(|(i, _)| i) + .last() + .unwrap_or_else(|| panic!("no object allocation above line {before} in:\n{ir}")) +} + +/// Temp-root traffic, excluding the `declare` lines that name the helpers +/// whether or not anything calls them. +fn temp_root_calls(ir: &str) -> usize { + ir.lines() + .filter(|l| !l.trim_start().starts_with("declare")) + .filter(|l| l.contains("js_gc_temp_root")) + .count() +} + +/// #7649: `console.table(data, properties)` held `data` in a bare register +/// across `properties`' lowering. +#[test] +fn console_table_roots_its_data_across_the_properties_operand() { + let ir = compile_body( + "table_hot", + console_call("table", vec![allocating("data"), allocating("props")]), + ); + + let call = require_call_line(&ir, "js_console_table_with_properties"); + let data = first_double_operand(&ir, "js_console_table_with_properties"); + let data_def = definition_line(&ir, &data); + let props_alloc = last_alloc_before(&ir, call); + assert!( + data_def > props_alloc, + "console.table's `data` register {data} is defined at line {data_def}, ABOVE the \ + `properties` allocation at line {props_alloc}. That is the unrooted window: an \ + evacuating minor while `properties` is evaluated relocates `data` and the renderer \ + then reads from-space. `data` must be rooted above the window and re-read below it.\n{ir}" + ); +} + +/// The zero-cost counterpart: nothing follows a lone `data` operand, so the +/// protection decision must be `Reuse` and the emission unchanged. +#[test] +fn console_table_with_one_argument_emits_no_rooting_traffic() { + let ir = compile_body( + "table_cold", + console_call("table", vec![allocating("data")]), + ); + + require_call_line(&ir, "js_console_table"); + assert_eq!( + temp_root_calls(&ir), + 0, + "a single-argument console.table cannot collect between its operand and the call, so \ + operand_protection must route it to Reuse. Rooting unconditionally taxes every \ + `console.table(rows)` in every program.\n{ir}" + ); +} + +/// #7649's arity half: three or more arguments used to fall through to the +/// generic multi-argument `console.log` arm and print the array. +#[test] +fn console_table_still_renders_a_table_with_surplus_arguments() { + let ir = compile_body( + "table_surplus", + console_call( + "table", + vec![allocating("data"), allocating("props"), allocating("extra")], + ), + ); + + assert!( + call_line(&ir, "js_console_table_with_properties").is_some(), + "console.table(a, b, c) must still render a table; node ignores the surplus arguments \ + rather than stopping being console.table\n{ir}" + ); + assert!( + call_line(&ir, "js_console_log_spread").is_none(), + "console.table(a, b, c) fell through to the generic multi-arg console.log arm\n{ir}" + ); +} + +/// #7649's evaluation-order half: `console.dir` lowered `args[2..]` AFTER the +/// call that produces the print, so `console.dir(x, y, f())` ran `f` second. +/// Node evaluates every argument before invoking anything. +#[test] +fn console_dir_evaluates_surplus_arguments_before_it_prints() { + let ir = compile_body( + "dir_order", + console_call( + "dir", + vec![ + allocating("obj"), + allocating("opts"), + allocating("side_effect"), + ], + ), + ); + + let call = require_call_line(&ir, "js_console_dir_with_options"); + let allocs_below = ir + .lines() + .skip(call) + .filter(|l| l.contains("@js_object_alloc")) + .count(); + assert_eq!( + allocs_below, 0, + "console.dir lowered a surplus argument below its own print at line {call}. Node \ + evaluates the whole argument list before it invokes the callee, so a side effect in \ + argument 3 must run FIRST.\n{ir}" + ); +} + +/// The same defect one arm over, with the surplus argument dropped entirely +/// rather than resequenced: `console.time(label, f())` never lowered `f`. +#[test] +fn console_time_evaluates_its_surplus_arguments() { + // Differential rather than absolute. One object literal lowers to several + // `js_object_alloc*` lines (an inline-bump fast path, a slow path and the + // shape declaration), so a fixed expected count would be pinning an + // unrelated lowering detail. What must hold is that adding an argument adds + // its evaluation. + let allocs_above_call = |ir: &str| { + let call = require_call_line(ir, "js_console_time_value"); + ir.lines() + .take(call) + .filter(|l| l.contains("@js_object_alloc")) + .count() + }; + + let label_only = compile_body( + "time_label", + console_call("time", vec![allocating("label")]), + ); + let with_surplus = compile_body( + "time_order", + console_call("time", vec![allocating("label"), allocating("side_effect")]), + ); + + assert!( + allocs_above_call(&with_surplus) > allocs_above_call(&label_only), + "console.time uses only the label, but node EVALUATES every argument. \ + `console.time(l, sideEffect())` emitted the same allocations as `console.time(l)`, \ + which means the surplus argument was never lowered and its side effect never \ + happened.\n{with_surplus}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/early_branches.rs b/crates/perry-codegen/src/lower_call/early_branches.rs index 0682ec1ec9..435fb1ac33 100644 --- a/crates/perry-codegen/src/lower_call/early_branches.rs +++ b/crates/perry-codegen/src/lower_call/early_branches.rs @@ -407,7 +407,7 @@ pub fn try_lower_closure_typed_local_call( // so the slot index crosses the diamond exactly as the // bare register used to. let prev_this = if callee_reads_this { - Some(crate::expr::temp_root::implicit_this_save(ctx, &undef_this)) + Some(crate::rooting::implicit_this_save(ctx, &undef_this)) } else { None }; @@ -929,7 +929,7 @@ pub fn try_lower_closure_typed_local_call( // body codegen never saw — reset `this` here (and only // here) when the static gating skipped the outer reset. let fallback_prev_this = if prev_this.is_none() { - Some(crate::expr::temp_root::implicit_this_save(ctx, &undef_this)) + Some(crate::rooting::implicit_this_save(ctx, &undef_this)) } else { None }; @@ -944,7 +944,7 @@ pub fn try_lower_closure_typed_local_call( // slot (restored in the merge block) is still live and the // temp-root depth matches on both paths into the merge. if let Some(prev) = fallback_prev_this { - crate::expr::temp_root::implicit_this_restore(ctx, prev); + crate::rooting::implicit_this_restore(ctx, prev); } let after_fallback = ctx.block().label.clone(); if !ctx.block().is_terminated() { @@ -960,7 +960,7 @@ pub fn try_lower_closure_typed_local_call( ], ); if let Some(prev) = prev_this { - crate::expr::temp_root::implicit_this_restore(ctx, prev); + crate::rooting::implicit_this_restore(ctx, prev); } return Ok(Some(merged)); } @@ -969,14 +969,14 @@ pub fn try_lower_closure_typed_local_call( // params, or arity mismatch): the runtime-resolved callee may // read `this`, so the reset is unconditional here. // #7211: rooted save/restore across the runtime-resolved callee. - let prev_this = crate::expr::temp_root::implicit_this_save(ctx, &undef_this); + let prev_this = crate::rooting::implicit_this_save(ctx, &undef_this); let runtime_fn = format!("js_closure_call{}", lowered_args.len()); let mut call_args: Vec<(crate::types::LlvmType, &str)> = vec![(I64, &closure_handle)]; for v in &lowered_args { call_args.push((DOUBLE, v.as_str())); } let result = ctx.block().call(DOUBLE, &runtime_fn, &call_args); - crate::expr::temp_root::implicit_this_restore(ctx, prev_this); + crate::rooting::implicit_this_restore(ctx, prev_this); return Ok(Some(result)); } } diff --git a/crates/perry-codegen/src/lower_call/extern_func.rs b/crates/perry-codegen/src/lower_call/extern_func.rs index fdc74b2802..2ba4d10487 100644 --- a/crates/perry-codegen/src/lower_call/extern_func.rs +++ b/crates/perry-codegen/src/lower_call/extern_func.rs @@ -1757,7 +1757,7 @@ pub fn try_lower_extern_func_call( ctx.pending_declares .push((fname.clone(), DOUBLE, param_types)); let mut lowered: Vec = Vec::with_capacity(target_arity); - let arg_guard: Option; + let arg_group: crate::rooting::RootedGroup<'_>; if has_rest { // #7154: the rest twin of the arm below. Fixed params were lowered into // bare registers and then held across `js_array_alloc` plus a @@ -1780,12 +1780,12 @@ pub fn try_lower_extern_func_call( mark_arguments_object: has_synthetic_args, }], )?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); } else { // #7154: the registry's residual. See `super::lower_call_args_rooted`. let (values, guard) = super::lower_call_args_rooted(ctx, args)?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); // Pad with TAG_UNDEFINED for the missing trailing args. let undefined_lit = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); @@ -1793,6 +1793,6 @@ pub fn try_lower_extern_func_call( lowered.push(undefined_lit.clone()); } } - let call = super::emit_rooted_call(ctx, &fname, &lowered, arg_guard); + let call = super::emit_rooted_call(ctx, &fname, &lowered, arg_group); Ok(Some(call)) } diff --git a/crates/perry-codegen/src/lower_call/func_ref.rs b/crates/perry-codegen/src/lower_call/func_ref.rs index 6e5fb653e8..69107d35ac 100644 --- a/crates/perry-codegen/src/lower_call/func_ref.rs +++ b/crates/perry-codegen/src/lower_call/func_ref.rs @@ -398,7 +398,7 @@ pub fn try_lower_func_ref_call( // release has to sit in the merge block that post-dominates all of them, // not next to the lowering. let mut lowered: Vec = Vec::with_capacity(declared_count); - let arg_guard: Option; + let arg_group: crate::rooting::RootedGroup<'_>; if ctx.func_synthetic_arguments.contains(fid) && has_rest && !synthetic_is_rest { // #1816: a real `...rest` AND a synthetic `arguments`, over the same // argument list at two different offsets. @@ -418,7 +418,7 @@ pub fn try_lower_func_ref_call( }, ], )?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); } else if has_rest && ctx.func_synthetic_arguments.contains(fid) { let fixed_count = declared_count.saturating_sub(1); @@ -431,7 +431,7 @@ pub fn try_lower_func_ref_call( mark_arguments_object: true, }], )?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); } else if has_rest { // Rest is always the LAST declared param. Pass the @@ -447,11 +447,11 @@ pub fn try_lower_func_ref_call( mark_arguments_object: false, }], )?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); } else { let (values, guard) = super::lower_call_args_rooted(ctx, args)?; - arg_guard = guard; + arg_group = guard; lowered.extend(values); } let arg_slices: Vec<(crate::types::LlvmType, &str)> = @@ -471,7 +471,7 @@ pub fn try_lower_func_ref_call( // method's receiver, held across the callee body — arbitrary user code. let prev_this = if resets_this { let undef = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - Some(crate::expr::temp_root::implicit_this_save(ctx, &undef)) + Some(crate::rooting::implicit_this_save(ctx, &undef)) } else { None }; @@ -902,9 +902,9 @@ pub fn try_lower_func_ref_call( // its doc calls out that a caller holding a lower group may release // afterwards and drop the slot a second time harmlessly. if let Some(prev) = prev_this { - crate::expr::temp_root::implicit_this_restore(ctx, prev); + crate::rooting::implicit_this_restore(ctx, prev); } - crate::expr::temp_root::temp_root_release(ctx, arg_guard); + arg_group.release(ctx); if ctx.local_generator_funcs.contains(fid) { let wrap_ptr = format!("@__perry_wrap_{}", fname); let closure_handle = diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 45f6526f4e..22f62e5a7b 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -121,7 +121,7 @@ pub(super) fn emit_own_method_override_check( }; // #7211: rooted save/restore — the displaced implicit `this` is live // across `js_native_call_value`, which runs arbitrary user code. - let prev_this = crate::expr::temp_root::implicit_this_save(ctx, &recv_for_this); + let prev_this = crate::rooting::implicit_this_save(ctx, &recv_for_this); let v_override = ctx.block().call( DOUBLE, "js_native_call_value", @@ -131,7 +131,7 @@ pub(super) fn emit_own_method_override_check( (I64, &args_len), ], ); - crate::expr::temp_root::implicit_this_restore(ctx, prev_this); + crate::rooting::implicit_this_restore(ctx, prev_this); let after_override = ctx.block().label.clone(); if !ctx.block().is_terminated() { ctx.block().br(&merge_label); diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index b58d9e12d5..9103273921 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -38,6 +38,10 @@ mod builtin_table_gate; mod capture_writeback; mod closure_analysis; mod console_promise; +/// Rooting and evaluation-order coverage for the `console.*` arms slice 6 +/// repaired (#7649) — see the module header for why these assert on IR. +#[cfg(test)] +mod console_rooting_tests; mod dataview_intrinsic; mod early_branches; mod event_target; @@ -170,24 +174,37 @@ pub(crate) use native_table::iter_native_module_table; /// global held the post-move address, the register held the retired from-space /// one. /// -/// [`temp_root::lower_exprs_rooted`] gates each argument on -/// `any_later_ref_may_trigger_gc`, so an argument list nothing allocating -/// follows emits exactly the IR it emitted before. +/// Each argument's window is "everything after it", so an argument list +/// nothing allocating follows emits exactly the IR it emitted before — +/// `operand_protection` routes it to `Reuse` and nothing is pushed. +/// +/// Returns the values to pass and the scope for [`emit_rooted_call`]. /// -/// Returns the values to pass and the guard for [`emit_rooted_call`]. +/// The scope ESCAPES rather than being owned by a closure, and the reason is +/// `func_ref.rs` rather than this function: its four specialized-ABI dispatch +/// arms split the block, so the release must sit in a merge that post-dominates +/// all of them, ~450 lines below. [`crate::rooting::open_rooted_group`] states +/// what escaping does and does not leave writable. /// -/// [`OperandProtection::Reload`]: crate::expr::temp_root -/// [`temp_root::lower_exprs_rooted`]: crate::expr::temp_root::lower_exprs_rooted -pub(crate) fn lower_call_args_rooted( +/// [`OperandProtection::Reload`]: crate::rooting +pub(crate) fn lower_call_args_rooted<'a>( ctx: &mut FnCtx<'_>, - args: &[Expr], -) -> Result<(Vec, Option)> { - let refs: Vec<&Expr> = args.iter().collect(); - crate::expr::temp_root::lower_exprs_rooted(ctx, &refs) + args: &'a [Expr], +) -> Result<(Vec, crate::rooting::RootedGroup<'a>)> { + let mut group = crate::rooting::open_rooted_group(args.len()); + for (i, arg) in args.iter().enumerate() { + // The window is "can anything between this argument and the consuming + // call collect?", which for a plain direct call is the arguments that + // follow it. + let collects = crate::rooting::any_operand_may_collect(ctx, args[i + 1..].iter()); + group.lower(ctx, arg, collects)?; + } + let values = group.reread_all(ctx)?; + Ok((values, group)) } /// Emit a direct call over an already-lowered argument list, then release the -/// [`lower_call_args_rooted`] guard. +/// [`lower_call_args_rooted`] scope. /// /// The release has to sit BELOW the call, not above it: the callee allocates /// while reading these arguments, so the slots have to outlive the call itself. @@ -195,14 +212,14 @@ pub(crate) fn emit_rooted_call( ctx: &mut FnCtx<'_>, fname: &str, lowered: &[String], - guard: Option, + group: crate::rooting::RootedGroup<'_>, ) -> String { let arg_slices: Vec<(crate::types::LlvmType, &str)> = lowered .iter() .map(|s| (crate::types::DOUBLE, s.as_str())) .collect(); let result = ctx.block().call(crate::types::DOUBLE, fname, &arg_slices); - crate::expr::temp_root::temp_root_release(ctx, guard); + group.release(ctx); result } @@ -232,75 +249,73 @@ pub(crate) struct RestBundle { /// window does not end when the last argument is lowered. The rest array /// is materialized *afterwards*, and materializing it runs /// `js_array_alloc` plus one `js_array_push_f64` per trailing argument, -/// every one of which allocates. So `lower_exprs_rooted` is the wrong tool -/// here: it re-reads immediately, and the collection point it must re-read -/// below is a step it never sees. [`RootedOperands`] exists for precisely -/// that, and the caller picking the re-read point is the whole difference. +/// every one of which allocates. So a combinator that re-reads once, at the +/// end of the operand list, is the wrong tool here: the collection points +/// it must re-read below are a step it never sees. /// /// 2. **The accumulator itself** — and this is the one with no analogue in -/// the non-rest arm. `current` is a RAW `*mut ArrayHeader` in a bare SSA +/// the non-rest arm. `current` was a RAW `*mut ArrayHeader` in a bare SSA /// register, threaded through the push loop, holding the ONLY reference to /// every argument pushed so far while the NEXT argument's expression is -/// lowered — arbitrary user code. Nothing roots it, so a minor in that -/// window does not merely move the array, it is free to sweep it. -/// [`temp_root::rooted_array_begin`]'s doc names this exact shape as "the -/// shape behind every variadic / spread / rest argument list"; the helper -/// has existed since #6951 and this path never adopted it. +/// lowered — arbitrary user code. Nothing rooted it, so a minor in that +/// window did not merely move the array, it was free to sweep it. +/// [`crate::rooting::RootedGroup::begin_array`] is that shape, and it lives +/// in the same scope as the operands so one release covers both. /// /// `collects` is unconditionally true for the fixed parameters, and that is a /// statement about the code rather than a conservative shrug: the rest array /// is materialized on every path (a callee's rest binding must be `[]` even /// when nothing trailing was passed), so `js_array_alloc` is always between a /// fixed parameter and the call that consumes it. Scalar arguments still cost -/// nothing — [`temp_root::operand_protection`] routes anything +/// nothing — the protection decision routes anything /// `expr_is_known_non_pointer_shadow_value` proves is not a heap reference to /// `Reuse`, so `f(1, 2, ...rest)` emits the IR it emitted before. /// /// Returns the values to pass — fixed parameters first, re-read from their -/// roots, then one boxed array per [`RestBundle`] — and the guard for +/// roots, then one boxed array per [`RestBundle`] — and the scope for /// [`emit_rooted_call`]. /// -/// [`RootedOperands`]: crate::expr::temp_root::RootedOperands -/// [`temp_root::rooted_array_begin`]: crate::expr::temp_root::rooted_array_begin -/// [`temp_root::operand_protection`]: crate::expr::temp_root::operand_protection -pub(crate) fn lower_rest_call_args_rooted( +/// This is the shape that named the missing combinator (#7615 slice 5): the +/// operands and the accumulator arrays are ONE temp-root scope, and the +/// per-element re-reads are a loop rather than a point. Both live in the +/// [`crate::rooting::RootedGroup`] now, so one release drops the whole stack — +/// which is what the hand-rolled version was doing by hand, via the +/// `rooted.guard().or_else(accs.first())` cut it no longer has to compute. +pub(crate) fn lower_rest_call_args_rooted<'a>( ctx: &mut FnCtx<'_>, - args: &[Expr], + args: &'a [Expr], fixed_count: usize, bundles: &[RestBundle], -) -> Result<(Vec, Option)> { - use crate::expr::temp_root; +) -> Result<(Vec, crate::rooting::RootedGroup<'a>)> { use crate::types::I64; - let refs: Vec<&Expr> = args.iter().collect(); + let mut group = crate::rooting::open_rooted_group(args.len()); // Incrementally, one argument at a time: root each BEFORE the next is // lowered. Lowering the whole list and rooting it afterwards is not merely // late, it is worse than doing nothing — by then an earlier value may // already have been swept and the push publishes a dangling pointer into a - // slot the collector scans. See `root_operands_begin`. - let mut rooted = temp_root::root_operands_begin(refs.len()); - for expr in &refs { - let value = crate::expr::lower_expr(ctx, expr)?; - rooted.push(ctx, expr, &value, true); + // slot the collector scans. + for arg in args { + group.lower(ctx, arg, true)?; } let mut lowered: Vec = Vec::with_capacity(fixed_count + bundles.len()); - // Build every array FIRST and leave each one in its temp-root slot, then - // read them all back at the end. Building array 2 allocates, so array 1's - // pointer must not be sitting in a bare register while it happens — #1816's - // shape wants both a `...rest` and an `arguments` bundle over the same - // list, and that second `js_array_alloc` is a collection point for the - // first array exactly as the push loop is for its elements. - let mut accs: Vec = Vec::with_capacity(bundles.len()); + // Build every array FIRST and leave each one in its slot, then read them + // all back at the end. Building array 2 allocates, so array 1's pointer + // must not be sitting in a bare register while it happens — #1816's shape + // wants both a `...rest` and an `arguments` bundle over the same list, and + // that second `js_array_alloc` is a collection point for the first array + // exactly as the push loop is for its elements. + let mut accs: Vec = Vec::with_capacity(bundles.len()); for bundle in bundles { let cap = (args.len().saturating_sub(bundle.from) as u32).to_string(); - let acc = temp_root::rooted_array_begin(ctx, &cap); - for i in bundle.from..refs.len() { + let acc = group.begin_array(ctx, &cap); + for i in bundle.from..group.len() { // Re-read per element: the previous push allocated, so the register // this argument was lowered into is already stale. - let value = rooted.reread_one(ctx, &refs, i)?; - temp_root::temp_rooted_array_push(ctx, &acc, &value); + let value = group.reread(ctx, i)?; + group.push_array(ctx, acc, &value); } accs.push(acc); } @@ -310,7 +325,7 @@ pub(crate) fn lower_rest_call_args_rooted( // is safe between the slot read and the box. let mut boxed_bundles: Vec = Vec::with_capacity(bundles.len()); for (bundle, acc) in bundles.iter().zip(accs.iter()) { - let mut current = temp_root::rooted_array_read(ctx, acc); + let mut current = group.read_array(ctx, *acc); if bundle.mark_arguments_object { current = ctx .block() @@ -321,19 +336,15 @@ pub(crate) fn lower_rest_call_args_rooted( let undefined_lit = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); for i in 0..fixed_count { - lowered.push(if i < refs.len() { - rooted.reread_one(ctx, &refs, i)? + lowered.push(if i < group.len() { + group.reread(ctx, i)? } else { undefined_lit.clone() }); } lowered.extend(boxed_bundles); - // The operand group was pushed BEFORE the accumulators, so its guard is the - // lower index and one truncate at it drops both. When nothing needed a real - // root the first accumulator is the lowest slot and becomes the guard. - let guard = rooted.guard().or_else(|| accs.first().cloned()); - Ok((lowered, guard)) + Ok((lowered, group)) } /// Lower a `Call` expression. Two shapes are supported: diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index 92354eba0d..5daeb15e53 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -407,8 +407,7 @@ pub(crate) fn try_lower_instance_method_call( // call fallthrough pattern (#519). let recv_for_this_probe = recv_box.clone(); // #7211: rooted save/restore across the user-code dispatch. - let prev_this_probe = - crate::expr::temp_root::implicit_this_save(ctx, &recv_for_this_probe); + let prev_this_probe = crate::rooting::implicit_this_save(ctx, &recv_for_this_probe); let v_override_probe = ctx.block().call( DOUBLE, "js_native_call_value", @@ -418,7 +417,7 @@ pub(crate) fn try_lower_instance_method_call( (I64, &probe_args_len_str), ], ); - crate::expr::temp_root::implicit_this_restore(ctx, prev_this_probe); + crate::rooting::implicit_this_restore(ctx, prev_this_probe); let after_override_probe = ctx.block().label.clone(); if !ctx.block().is_terminated() { ctx.block().br(&probe_outer_merge_label); @@ -1400,7 +1399,7 @@ fn emit_collapsed_instance_dispatch( // function value (#632 — a class-field non-arrow function reads `this`). ctx.current_block = override_idx; // #7211: rooted save/restore across the user-code dispatch. - let prev_this = crate::expr::temp_root::implicit_this_save(ctx, recv_box); + let prev_this = crate::rooting::implicit_this_save(ctx, recv_box); let v_override = ctx.block().call( DOUBLE, "js_native_call_value", @@ -1410,7 +1409,7 @@ fn emit_collapsed_instance_dispatch( (I64, &args_len), ], ); - crate::expr::temp_root::implicit_this_restore(ctx, prev_this); + crate::rooting::implicit_this_restore(ctx, prev_this); let after_override = ctx.block().label.clone(); if !ctx.block().is_terminated() { ctx.block().br(&merge_label); diff --git a/crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs index e40467ddfd..7d200bbb73 100644 --- a/crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/static_dispatch.rs @@ -215,7 +215,7 @@ pub(crate) fn try_lower_static_dispatch( } // #7211: rooted save/restore — the displaced implicit `this` is // live across the static method body below, which is user code. - let prev_this = crate::expr::temp_root::implicit_this_save(ctx, &recv_box); + let prev_this = crate::rooting::implicit_this_save(ctx, &recv_box); // Receiver-sensitive static `this`: arm the one-shot override with // the ACTUAL receiver box so the callee prologue's // `js_static_this_resolve` binds `this` to it (spec @@ -240,7 +240,7 @@ pub(crate) fn try_lower_static_dispatch( let arg_slices: Vec<(crate::types::LlvmType, &str)> = lowered.iter().map(|s| (DOUBLE, s.as_str())).collect(); let result = ctx.block().call(DOUBLE, &fn_name, &arg_slices); - crate::expr::temp_root::implicit_this_restore(ctx, prev_this); + crate::rooting::implicit_this_restore(ctx, prev_this); return Ok(Some(result)); } // #1787 / #321: the call target is a static FIELD holding a callable, diff --git a/crates/perry-codegen/src/rooting.rs b/crates/perry-codegen/src/rooting.rs index bbadfd9522..bf820b1b4e 100644 --- a/crates/perry-codegen/src/rooting.rs +++ b/crates/perry-codegen/src/rooting.rs @@ -622,6 +622,331 @@ pub(crate) fn any_operand_may_collect<'a>( crate::expr::temp_root::any_may_trigger_gc(ctx, exprs) } +/// [`any_operand_may_collect`] for a single expression. +/// +/// The per-operand form is what a group with *unequal* windows needs: in the +/// generic dynamic call the receiver is live across the callee read and every +/// argument, the callee across every argument, and argument `i` across the +/// arguments after it plus an allocating rebind. One `collects` for the whole +/// list cannot say that. +pub(crate) fn operand_may_collect(ctx: &FnCtx<'_>, expr: &Expr) -> bool { + crate::expr::temp_root::expr_may_trigger_gc(ctx, expr) +} + +// --------------------------------------------------------------------------- +// The multi-point re-read scope (#7615 slice 6). +// +// WHY THE `with_operands_rooted*` FAMILY COULD NOT TAKE THE `lower_call/` +// MODULES, stated as a property of the API rather than of those files. +// +// Every form above re-reads at exactly ONE point: the end of the operand list, +// after an optional `across` step. That is the whole shape for `m.set(k, v)` +// and `u8[i]` — lower, protect, re-read once, consume once. Three lowerings in +// `lower_call/` are not that shape, and each is a different way of not being +// it: +// +// * `lower_dynamic_closure_call` consumes the group in TWO instructions with +// an allocating step between them. The receiver and callee feed +// `js_closure_unbox_callee_checked_rebind`, which CLONES a `this`-capturing +// closure and therefore allocates; the arguments feed `js_closure_callN` +// BELOW it. One re-read point can serve one of those two and must strand +// the other (#7154's own reasoning, `RootedOperands::reread_one`). +// * `lower_rest_call_args_rooted` re-reads element `i` between the pushes +// that materialise the rest array — `js_array_alloc` plus one +// `js_array_push_f64` per element, all of which allocate — so its re-read +// points are a LOOP, not a point. +// * `try_lower_func_ref_call` releases ~450 lines below the lowering, in the +// merge block of four block-splitting specialized-ABI dispatch diamonds. A +// closure form can express that only by swallowing the dispatch chain. +// +// So the missing combinator is not "the variadic/rest shape" (slice 5's +// hypothesis, and the shape that made it visible) but the thing all three want: +// ONE temp-root scope whose contents may be re-read at ANY number of +// caller-chosen points. The rest/variadic case then falls out as a group that +// happens to hold an accumulator array as well as operands, which is why +// [`RootedGroup`] carries both rather than there being a second type for it. +// +// TWO ENTRY POINTS, AND THE ASYMMETRY IS DELIBERATE. +// +// [`with_rooted_group`] owns the release, like every other combinator here. +// [`open_rooted_group`] hands the group back, which every other combinator in +// this file deliberately refuses to do — so it needs an argument. +// +// The argument is that the two halves of a mis-managed guard are not equally +// dangerous. A release that is EARLY or MIS-ORDERED is a use-after-free: the +// slot is cut while the consumer still reads it. A release that never happens +// is over-retention — the slot stays bound for the rest of the function, the +// object stays live, and the emitted code is merely conservative (in the FFI +// fallback the runtime stack also grows, which is #7462's symptom and a real +// bug, but still not a dangling pointer). +// +// [`RootedGroup`] removes the dangerous half BY CONSTRUCTION and for both +// entry points: it is not `Clone`, `release` consumes it, and there is no way +// to obtain the slot index — so a caller cannot truncate at the wrong slot, in +// the wrong order, or twice. What escaping leaves writable is exactly the safe +// half. That is strictly better than the raw API it replaces, where the caller +// holds an `Option` slot index it can truncate anywhere. +// +// Prefer [`with_rooted_group`]. Reach for [`open_rooted_group`] only where the +// release must post-dominate blocks the lowering does not lexically contain. +// --------------------------------------------------------------------------- + +/// One temp-root scope: an ordered stack of rooted values — already-lowered +/// **operands** and mutable **accumulator arrays** — re-readable at any number +/// of caller-chosen points and released once, for the whole stack. +/// +/// Two things it does NOT do, both on purpose: +/// +/// * it never hands out a slot index, so the release cannot be mis-ordered. +/// `temp_root_truncate` is a stack CUT — truncating the wrong slot drops +/// every slot above it, which is how a receiver save becomes the number `0` +/// (`func_ref.rs`'s note on release ordering); +/// * it never lowers an operand it was not asked to. [`RootedGroup::lower`] +/// lowers, [`RootedGroup::adopt`] takes a value the caller emitted itself — +/// which the generic dynamic call needs, because its callee operand is a +/// hand-emitted by-name property read rather than `lower_expr(callee)`. +pub(crate) struct RootedGroup<'a> { + operands: crate::expr::temp_root::RootedOperands, + exprs: Vec<&'a Expr>, + accs: Vec, + /// The LOWEST slot this group pushed, of either kind. One truncate at it + /// drops the whole scope, because a truncate is a stack cut. + first_slot: Option, +} + +/// A handle on one accumulator array inside a [`RootedGroup`]. +/// +/// Opaque and `Copy`. What it buys is the half that matters: it is not a slot +/// index, so it cannot be truncated, mis-ordered or released — only handed back +/// to the group, which owns every emission that touches the slot. +/// +/// It is **not branded per group**. It is a bare index into the group's own +/// list, so passing a handle to a *different* group selects that group's +/// accumulator at the same position (or panics if it has fewer). Pass a handle +/// only to the group that returned it. Branding it would need a group identity +/// this file has nowhere to get without global state, and the mistake is not +/// one any caller is positioned to make: a group is always a local, and the two +/// entry points hand it out one at a time. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct AccArray(usize); + +impl<'a> RootedGroup<'a> { + fn new(capacity: usize) -> Self { + RootedGroup { + operands: crate::expr::temp_root::root_operands_begin(capacity), + exprs: Vec::with_capacity(capacity), + accs: Vec::new(), + first_slot: None, + } + } + + /// Record the lowest slot the group holds. Slots are handed out in + /// increasing watermark order, so the first one recorded is the lowest and + /// later ones must not displace it. + fn note_slot(&mut self, slot: Option) { + if self.first_slot.is_none() { + self.first_slot = slot; + } + } + + /// Lower `expr` and root it across a window the caller states. + /// + /// Returns the operand's index for [`RootedGroup::reread`] — and returns + /// *only* that. The lowered register is deliberately not handed back: a + /// caller holding it is the second half of every bug in this family, and + /// the group is now the only way to name the value. + pub(crate) fn lower( + &mut self, + ctx: &mut FnCtx<'_>, + expr: &'a Expr, + collects: bool, + ) -> Result { + let value = crate::expr::lower_expr(ctx, expr)?; + Ok(self.adopt(ctx, expr, &value, collects)) + } + + /// Root a value the caller emitted itself. + /// + /// `expr` still decides the protection, so the group answers "root, + /// re-derive, or reuse?" through `operand_protection` exactly as a lowered + /// operand does. + /// + /// **Precondition.** `value` must be what lowering `expr` produces, or + /// `expr` must not be `operand_is_reloadable` — because a `Reload` operand + /// is re-read by *re-lowering* `expr`, and re-lowering something the caller + /// did not lower would answer with a different value. Only `Expr::String` + /// is reloadable, so every current caller (a `PropertyGet` callee, a + /// receiver) satisfies this trivially; the note is here for the next one. + pub(crate) fn adopt( + &mut self, + ctx: &mut FnCtx<'_>, + expr: &'a Expr, + value: &str, + collects: bool, + ) -> usize { + self.operands.push(ctx, expr, value, collects); + let pushed = self.operands.guard(); + self.note_slot(pushed); + self.exprs.push(expr); + self.exprs.len() - 1 + } + + /// Re-read operand `i` **here**, below whatever has collected since it was + /// rooted. + /// + /// Mandatory rather than defensive, and it is the reason this type exists: + /// the slot is a MUTABLE root that an evacuating cycle rewrites in place, + /// so a register read before the cycle names from-space. + pub(crate) fn reread(&self, ctx: &mut FnCtx<'_>, i: usize) -> Result { + self.operands.reread_one(ctx, &self.exprs, i) + } + + /// Re-read every operand at this point, in order. + pub(crate) fn reread_all(&self, ctx: &mut FnCtx<'_>) -> Result> { + self.operands.reread(ctx, &self.exprs) + } + + /// How many operands the group holds. + pub(crate) fn len(&self) -> usize { + self.exprs.len() + } + + /// Allocate an argument-accumulator array of capacity `cap` and root it in + /// this scope. + /// + /// This is the variadic / spread / rest shape: `js_array_alloc(n)`, then one + /// push per argument. The accumulator holds the ONLY reference to everything + /// pushed so far while the next argument is lowered, and every push + /// allocates — so it is an accumulator in exactly [`RootedAcc`]'s sense, and + /// it lives in the group so that ONE release drops the operands and the + /// arrays together. + pub(crate) fn begin_array(&mut self, ctx: &mut FnCtx<'_>, cap: &str) -> AccArray { + let slot = crate::expr::temp_root::rooted_array_begin(ctx, cap); + self.note_slot(Some(slot.clone())); + self.accs.push(slot); + AccArray(self.accs.len() - 1) + } + + /// Push one element, re-reading the array from its slot and publishing the + /// possibly-reallocated pointer back into it. + pub(crate) fn push_array(&mut self, ctx: &mut FnCtx<'_>, acc: AccArray, value: &str) { + let slot = self.accs[acc.0].clone(); + crate::expr::temp_root::temp_rooted_array_push(ctx, &slot, value); + } + + /// Re-read the finished array as a raw `i64` pointer. Does not release: the + /// consuming call allocates while it reads the array. + pub(crate) fn read_array(&self, ctx: &mut FnCtx<'_>, acc: AccArray) -> String { + let slot = self.accs[acc.0].clone(); + crate::expr::temp_root::rooted_array_read(ctx, &slot) + } + + /// Drop the whole scope. Call it *after* the consuming call: the consumer + /// allocates while reading these values. + pub(crate) fn release(self, ctx: &mut FnCtx<'_>) { + crate::expr::temp_root::temp_root_release(ctx, self.first_slot); + } +} + +/// Open a [`RootedGroup`] for the duration of `body` and release it on every +/// path out, including `body`'s `?`. +pub(crate) fn with_rooted_group<'f, 'a, R>( + ctx: &mut FnCtx<'f>, + capacity: usize, + body: impl FnOnce(&mut FnCtx<'f>, &mut RootedGroup<'a>) -> Result, +) -> Result { + let mut group = RootedGroup::new(capacity); + let out = body(ctx, &mut group); + group.release(ctx); + out +} + +/// Open a [`RootedGroup`] whose release the CALLER performs, because it must +/// post-dominate blocks this lowering does not lexically contain. +/// +/// One shape needs this and it is named rather than left general: +/// `func_ref.rs`'s direct call lowers its arguments, then dispatches through up +/// to four specialized-ABI diamonds that split the block, and the release has +/// to sit in the merge. Releasing inside either side of a diamond would leave +/// the other side's call reading dropped slots. +/// +/// `#[must_use]` because dropping the group silently is the one mistake left +/// writable — see the block comment above for why that mistake is +/// over-retention rather than a dangling pointer, and why the dangerous half +/// (an early or mis-ordered truncate) is not writable at all. +#[must_use = "a RootedGroup must be released with `release`, below the call that reads it"] +pub(crate) fn open_rooted_group<'a>(capacity: usize) -> RootedGroup<'a> { + RootedGroup::new(capacity) +} + +/// A saved implicit `this`, held in a rooted slot for the duration of a +/// dispatch (#7211). +/// +/// **Moved here in slice 6 rather than re-exported.** It was already a paired +/// combinator with this file's contract — root before the window, re-read +/// after it, never hand out the register — but it lived in the raw API, so six +/// `lower_call/` modules had to name `expr::temp_root` while making no ordering +/// decision at all. Leaving a copy behind would have given the pair two +/// spellings, which is the drift that produced #7114; there is one. +/// +/// `js_implicit_this_set` swaps the `IMPLICIT_THIS` cell and returns what was +/// there, read straight out of a cell `scan_implicit_this_roots_mut` +/// (`object/this_binding.rs:176`) registers as a scanned MUTABLE root. The swap +/// has already overwritten the cell, so the returned value is now held ONLY in +/// an SSA register, across the whole call the bind exists to scope. +/// +/// Two ways that hurts, and the second is what makes it worse than an ordinary +/// stale read: +/// +/// * the enclosing frame still roots the same object, so an evacuating minor +/// inside the callee MOVES it and rewrites that root — leaving this register +/// naming from-space. The restore then publishes a pre-move address back +/// INTO a root the collector scans, so the corruption outlives the call that +/// caused it and surfaces in whatever reads `this` next; +/// * where no other root holds it, the object is simply collected. +/// +/// Seven lowerings emit this pair. They had seven copies of the same three +/// lines and therefore seven copies of the same bug, which is why it is a +/// combinator rather than seven edits. +pub(crate) struct ImplicitThisSave { + slot: RootedSlot, +} + +/// Bind `new_this` as the implicit `this` and root the value it displaced. +/// +/// Unconditional, unlike an operand group: the window is a user or native call, +/// so `operand_protection`'s "can this window collect?" test has exactly one +/// answer here and there is nothing to gate on. +pub(crate) fn implicit_this_save(ctx: &mut FnCtx<'_>, new_this: &str) -> ImplicitThisSave { + let prev = ctx + .block() + .call(DOUBLE, "js_implicit_this_set", &[(DOUBLE, new_this)]); + let idx = crate::expr::temp_root::temp_root_push_double(ctx, &prev); + ImplicitThisSave { + slot: RootedSlot { + idx, + repr: Repr::Boxed, + }, + } +} + +/// Restore the saved implicit `this`, re-read from its root. +/// +/// Reading the slot rather than the register is the fix, not a precaution: the +/// slot is a mutable root, so an evacuating cycle inside the dispatch rewrote +/// it and the register pushed beforehand names from-space. +/// +/// The release is emitted BEFORE the restore call so that nested saves — an +/// override arm inside an outer bind — release inner to outer. A release is a +/// stack cut, so a caller holding a LOWER group may release it afterwards and +/// drop this slot a second time harmlessly. +pub(crate) fn implicit_this_restore(ctx: &mut FnCtx<'_>, save: ImplicitThisSave) { + let prev = read_slot(ctx, &save.slot); + save.slot.release(ctx); + ctx.block() + .call(DOUBLE, "js_implicit_this_set", &[(DOUBLE, &prev)]); +} + /// A GC-managed value that generated code keeps **updating** while it lowers /// further expressions: an object literal's half-built handle, `Object.assign`'s /// threaded target, `Math.min(...)`'s growing argument array. @@ -880,6 +1205,42 @@ pub(crate) fn with_rooted_accumulator<'f, R>( /// say what it means is a gap in the API, and recording it here is what stops /// the next slice from rediscovering it. The variadic/rest shape (per-element /// re-reads between allocating pushes) is the concrete missing combinator. +/// +/// **Slice 6 built that combinator and it is not the one slice 5 named.** +/// Slice 5's hypothesis was the variadic/rest shape; three modules wanted it and +/// only one of them is variadic. What all three want is +/// [`RootedGroup`] — one temp-root scope, re-readable at ANY number of +/// caller-chosen points — of which the rest shape is the case that also holds +/// an accumulator array. The block comment above [`RootedGroup`] argues the +/// shape and the two entry points; the reason there are two is that +/// `func_ref.rs`'s release must post-dominate four block-splitting dispatch +/// diamonds, which no closure form can own without swallowing the dispatch +/// chain. +/// +/// Slice 6 lists three modules, all load-bearing on the committed source: +/// +/// * `lower_call/mod.rs` — `lower_call_args_rooted` / +/// `lower_rest_call_args_rooted` / `emit_rooted_call`. Named +/// `lower_exprs_rooted`, `root_operands_begin`, `rooted_array_begin`, +/// `temp_rooted_array_push`, `rooted_array_read` and `temp_root_release`. +/// It now hands its callers a [`RootedGroup`] instead of an +/// `Option` slot index, which is what makes `extern_func.rs`, +/// `namespace_call.rs` and `func_ref.rs` unable to truncate at the wrong +/// slot even though they still hold the scope. +/// * `lower_call/func_ref.rs` — the escaping-release consumer, plus +/// `implicit_this_save` / `implicit_this_restore`. +/// * `lower_call/console_promise.rs` — the two-stage dynamic closure call, +/// the `js_native_call_method_by_id` dispatch (which turned out to be the +/// plain single-re-read shape after all), and eight `console.*` arms. +/// +/// **`implicit_this_save` / `implicit_this_restore` MOVED here** rather than +/// being re-exported, so the pair has one spelling. That incidentally clears +/// the escape hatch out of `early_branches.rs`, `method_override.rs` and both +/// `property_get` dispatchers, whose only uses were that pair. They are +/// deliberately NOT listed: a line here would assert that the module makes +/// every rooting decision through this API, and nobody has read those four +/// modules for windows with no decision at all. An unlisted module is honest; +/// a listed unaudited one is the distinction slice 4 had to draw the hard way. #[cfg(test)] const MIGRATED_MODULES: &[(&str, &str)] = &[ ( @@ -970,6 +1331,18 @@ const MIGRATED_MODULES: &[(&str, &str)] = &[ "crates/perry-codegen/src/lower_call/namespace_call.rs", include_str!("lower_call/namespace_call.rs"), ), + ( + "crates/perry-codegen/src/lower_call/mod.rs", + include_str!("lower_call/mod.rs"), + ), + ( + "crates/perry-codegen/src/lower_call/func_ref.rs", + include_str!("lower_call/func_ref.rs"), + ), + ( + "crates/perry-codegen/src/lower_call/console_promise.rs", + include_str!("lower_call/console_promise.rs"), + ), ]; /// Lines in `src` that reach past [`crate::rooting`] into the raw rooting API.