diff --git a/CLAUDE.md b/CLAUDE.md index 11e4873e57..c0366d9d98 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.1374 +**Current Version:** 0.5.1375 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 57f4463e49..a153a8441e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1374" +version = "0.5.1375" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1374" +version = "0.5.1375" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1374" +version = "0.5.1375" [[package]] name = "perry-ui-tvos" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1374" +version = "0.5.1375" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 1205a1a65c..b70b662e64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1374" +version = "0.5.1375" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7660-element-shape-loop-growth-forwarding.md b/changelog.d/7660-element-shape-loop-growth-forwarding.md new file mode 100644 index 0000000000..9b9fbe687b --- /dev/null +++ b/changelog.d/7660-element-shape-loop-growth-forwarding.md @@ -0,0 +1,102 @@ +### Fixed + +**repsel: the element-shape versioned loop clone bus-errored on any array that +had grown outside the reading scope (#7480 / #7612).** + +`js_array_grow` does not grow in place: it allocates the larger array +elsewhere, copies, and leaves a **forwarding stub** at the old address whose +first payload word (`length`‖`capacity`) is overwritten with the new head — +that overwrite *is* the chain `clean_arr_ptr` follows. Every runtime entry +point resolves it, so a binding holding a stale head still behaves correctly +through the runtime, and only bindings the growing code itself wrote through +get re-pointed. A callee that grows a caller's array, or a function that grows +a local and returns it, leaves a stale head behind — repsel 4a.2's canonical +case (#6904), which is why `js_array_refresh_local_head` exists. + +#7612's preheader did not use it. It derived both the bound check and the +elements base from the raw pointer, and on a stub those two facts fail in the +worst possible combination: `length` reads the low 32 bits of a heap pointer, +so the "verified prefix covers the whole index range" test **passes**, while +the elements base still addresses the pre-growth buffer. The guard call one +block earlier resolved the chain internally and answered truthfully about the +live array, so the class-id test passed too. The clone then read correct +elements up to the old capacity and ran off the end of the block after it — +masked, dereferenced at `-8`, `SIGBUS`. With `MIN_ARRAY_CAPACITY == 16` that is +a bug with a threshold: right at 16 elements, bus error at 17. + +```ts +function build(n: number): Node[] { + const out: Node[] = []; + for (let i = 0; i < n; i++) out.push(new Node(i, i * 2)); + return out; // grew 16 -> 32, stub returned +} +function sweep(keep: Node[], n: number): number { + let sum = 0; + for (let j = 0; j < n; j++) sum += keep[j].v; // bound is a LocalGet + return sum; +} +const keep = build(17); // 17, not 16 +sweep(keep, keep.length); // exit 138 before this fix +``` + +**Two ingredients, and the second is the one that makes this hard to +reproduce.** (1) the array must have grown past `MIN_ARRAY_CAPACITY` (16) in a +callee that returned it — minimal N is 17; and (2) the clone must actually be +emitted, which requires a loop bound that is `Expr::Integer` or +`Expr::LocalGet`. An inline `arr.length` bound is an `Expr::PropertyGet`, which +`match_element_shape_versioned_loop` rejects, so `for (let j = 0; j < +keep.length; j++)` emits no clone and **cannot** fault; hoisting the identical +bound into a local flips it. Nothing else matters: `--release` and +`--profile perry-dev` fault identically, auto-optimize is irrelevant, no +run-time knob is involved, and the read does not need to cross a function +boundary (a module-scope loop with a hoisted bound faults the same way). + +Fixed with a new `element_shape.loop.preheader.repair` block between the brand +test and the guard call: follow the chain once with +`js_array_refresh_local_head`, write the live head back into the binding, and +re-derive the tag/band predicate from it. Both halves of that placement are +load-bearing. It cannot go *after* the guard call, because the deref block's +contract is "no call from here to the end of the clone" and the refresh can +itself allocate (`clean_arr_ptr` force-materialises a lazy array) — a refresh +there would reintroduce the very "base derived across an allocating call" +hazard step (4) exists to prevent. And it cannot skip the **write-back**, +because the query and deref blocks both deliberately re-read the binding to +survive a move by the guard call, and would pull the stub straight back out. +The write-back also lands the durable half of #6904's self-heal: after the +first visit the binding holds the live head, so later loop entries and the slow +clone address the current array directly. Closure-captured arrays are now +declined, since their capture cell is not updated by a plain slot store. + +That the binding really was a stub rather than a live head is established +directly, not inferred: adding a module-scope `keep.push(…); keep.pop();` after +the `build()` — which forces a write-back of the *resolved* head into the +global — makes the same pre-fix compiler print the right answer and exit 0. +(It also follows by construction, since `js_array_refresh_local_head` returns +its input untouched when there is nothing to follow, so on an already-live +binding this fix would be a no-op and the crash would survive it.) A +**producer**-side gap is therefore also open, tracked as #7661: +`expr/array_push.rs` writes the reallocated head back to the pushing scope's +own slot, so the stub is being reintroduced somewhere between that slot and the +caller's binding. The consumer fix is the right layer regardless — a stale head +can arrive by several routes, which is exactly why every runtime entry point +resolves the chain. + +**Why nothing caught it.** Every existing case — gap test and codegen census +alike — built its array in the same scope that read it, so the binding always +held the live head, and the largest was 64 elements pushed at module scope, +where each `push`'s write-back updates the global. The raw-pointer derivation +was never handed a stub. The new gap case covers both bound forms, so a future +narrowing of ingredient (2) cannot silently drop the coverage. `test_gap_repsel_element_shape_loop_clone.ts` gains +case 10 with both stale-head shapes above 16 elements (callee builds and +returns; callee grows the caller's array): **exit 138 on the pre-fix compiler, +byte-identical to node with the fix**. Three codegen census tests cover the +emitted form; the load-bearing one asserts the value stored back *is the +refresh's result* rather than merely that a store exists, and is +sabotage-checked by deleting the write-back arm. + +Found while re-measuring #7480, whose own object-literal kernel +(`keep: {v,w}[]`, 200k × 50 sweeps) is unaffected by this path and re-measures +at 414 ms against node 12 ms / bun 12 ms on the pinned quiet host — the +issue's recorded 93 ms / 6.2× is stale, and the object-literal element type is +still the open half of the gap (`element_class_name` resolves only +`Array(Named(C))`). diff --git a/crates/perry-codegen/src/expr/element_shape_guard.rs b/crates/perry-codegen/src/expr/element_shape_guard.rs index 934d39eb33..aa42ecf28f 100644 --- a/crates/perry-codegen/src/expr/element_shape_guard.rs +++ b/crates/perry-codegen/src/expr/element_shape_guard.rs @@ -89,12 +89,20 @@ const ELEM_HEADER_EXPECT: &str = "268435458"; // 0x1000_0002 /// terminates with `cond_br(shape_ok, fast, slow)`. Never entering a clone /// whose call-freeness is unproven is the whole revocation argument. /// -/// Sequencing is load-bearing. The brand test comes first so the pointer -/// handed to the runtime is known to be a real array; the guard call comes -/// next; and the elements base pointer is derived only AFTERWARDS, from a -/// fresh load of the array's rooted slot — the guard call can allocate, and an -/// allocation can move the array, so a base pointer derived before it could be -/// a from-space address. +/// Sequencing is load-bearing, in four steps and this order: +/// +/// 1. the receiver is a heap pointer at all; +/// 2. the **brand** test, so the pointer handed to the runtime is known to be +/// a real array and not an `extends Array` instance (#7573/#7603); +/// 3. the **growth-forwarding repair** (#7480) — the binding may hold a stale +/// head, and steps below read `length` and the elements base off the raw +/// pointer, where a forwarding stub is not merely wrong but *plausibly* +/// wrong (see the block comment). It goes before the guard call, not after, +/// because the refresh can itself allocate; +/// 4. the guard call, and only THEN the elements base — derived from a fresh +/// load of the array's rooted slot, because the guard call can allocate and +/// an allocation can move the array, so a base derived before it could be a +/// from-space address. /// /// Returns `(elements_base, expected_keys, shape_ok)`. pub(crate) fn emit_element_shape_loop_preheader_check( @@ -106,9 +114,11 @@ pub(crate) fn emit_element_shape_loop_preheader_check( slow_label: &str, ) -> anyhow::Result<(String, String, String)> { let brand_idx = ctx.new_block("element_shape.loop.preheader.brand"); + let repair_idx = ctx.new_block("element_shape.loop.preheader.repair"); let query_idx = ctx.new_block("element_shape.loop.preheader.query"); let deref_idx = ctx.new_block("element_shape.loop.preheader.deref"); let brand_label = ctx.block_label(brand_idx); + let repair_label = ctx.block_label(repair_idx); let query_label = ctx.block_label(query_idx); let deref_label = ctx.block_label(deref_idx); @@ -141,7 +151,61 @@ pub(crate) fn emit_element_shape_loop_preheader_check( let gt_ptr = blk.inttoptr(I64, >_addr); let gc_type = blk.load(I8, >_ptr); let is_array = blk.icmp_eq(I8, &gc_type, GC_TYPE_ARRAY); - blk.cond_br(&is_array, &query_label, slow_label); + blk.cond_br(&is_array, &repair_label, slow_label); + } + + // (2b) GROWTH-FORWARDING REPAIR (#7480). The binding may hold a *stale* + // array head: `js_array_grow` allocates the larger array elsewhere and + // leaves a forwarding stub at the old address, and only the bindings the + // growing code itself wrote through are re-pointed. Every runtime entry + // point resolves the chain (`clean_arr_ptr`) — including + // `js_array_ensure_element_shape` below, which therefore answers about the + // LIVE array — but the emitted code below reads `length` and the elements + // base off the raw pointer, and on a stub those are catastrophically wrong: + // growth overwrites the stub's first payload word (`length`‖`capacity`) + // with the forwarding address, so `length` reads the low 32 bits of a heap + // pointer (a huge number that passes `len_ok`), while the elements base + // still addresses the pre-growth buffer. Elements below the old capacity + // read stale-but-valid pointers and everything above it runs off the end of + // the block into whatever allocation follows — masked, dereferenced at + // `-8`, SIGBUS. With `MIN_ARRAY_CAPACITY == 16` that is exactly the + // "correct for a 16-element array, faults at 17" shape #7480 reproduced. + // + // The repair is repsel 4a.2's (#6904) documented self-heal: follow the + // chain once and write the live head back to the binding. It must happen + // BEFORE the query call, not after, because `js_array_refresh_local_head` + // can allocate (a lazy array materializes inside `clean_arr_ptr`) — putting + // it here keeps the "no call after the base is derived" invariant intact, + // and the write-back means step (4)'s re-load of the rooted slot picks up + // the repaired head no matter what the query call moved. + ctx.current_block = repair_idx; + { + let fresh = ctx.block().call( + DOUBLE, + "js_array_refresh_local_head", + &[(DOUBLE, arr0.as_str())], + ); + // The matcher (`stmt/element_shape_loop.rs`) admits only bindings one + // of these two arms covers, so the head is always repairable here. + if let Some(slot) = ctx.locals.get(&array_local_id).cloned() { + ctx.block().store(DOUBLE, &fresh, &slot); + } else if let Some(global_name) = ctx.module_globals.get(&array_local_id).cloned() { + let g_ref = format!("@{global_name}"); + // GC_STORE_AUDIT(ROOT): module global array slot is a registered + // mutable GC root; the value is the same JS array's live head. + super::write_barrier::emit_root_nanbox_store_on_block(ctx.block(), &fresh, &g_ref); + } + // Re-derive from the repaired head. `js_array_refresh_local_head` + // returns its input untouched when there was nothing to follow, so + // this repeats (1) rather than replacing it. + let blk = ctx.block(); + let bitsr = blk.bitcast_double_to_i64(&fresh); + let tagr = blk.lshr(I64, &bitsr, "48"); + let is_ptrr = blk.icmp_eq(I64, &tagr, POINTER_TAG_HI16); + let handler = blk.and(I64, &bitsr, crate::nanbox::POINTER_MASK_I64); + let abover = blk.icmp_ugt(I64, &handler, HANDLE_BAND_TOP); + let okr = blk.and(I1, &is_ptrr, &abover); + blk.cond_br(&okr, &query_label, slow_label); } // (3) The live-header query. `js_array_ensure_element_shape` establishes @@ -149,10 +213,17 @@ pub(crate) fn emit_element_shape_loop_preheader_check( // either way it reads the array's CURRENT `GcHeader` bit and its record, // and self-heals (clearing the bit) when the record went stale. Static // declarations are never consulted — #7501's lesson. + // + // Deliberately re-loads the (now repaired) binding rather than reusing the + // repair block's handle: `js_array_refresh_local_head` can allocate, so a + // handle derived before it is a pre-move address. ctx.current_block = query_idx; + let arrq = super::lower_expr(ctx, &perry_hir::Expr::LocalGet(array_local_id))?; { let blk = ctx.block(); - let class_id = blk.call(I32, "js_array_ensure_element_shape", &[(I64, &handle0)]); + let bitsq = blk.bitcast_double_to_i64(&arrq); + let handleq = blk.and(I64, &bitsq, crate::nanbox::POINTER_MASK_I64); + let class_id = blk.call(I32, "js_array_ensure_element_shape", &[(I64, &handleq)]); let cid_ok = blk.icmp_eq(I32, &class_id, expected_class_id); blk.cond_br(&cid_ok, &deref_label, slow_label); } diff --git a/crates/perry-codegen/src/stmt/element_shape_loop.rs b/crates/perry-codegen/src/stmt/element_shape_loop.rs index ae1538e9a2..3bc269463e 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop.rs @@ -371,6 +371,15 @@ fn match_element_shape_versioned_loop( if !ctx.locals.contains_key(&array_id) && !ctx.module_globals.contains_key(&array_id) { return None; } + // #7480: the preheader must be able to write the growth-forwarding-repaired + // head BACK into the binding (see + // `expr::element_shape_guard::emit_element_shape_loop_preheader_check` + // step 2b). A closure-captured array lives in a capture cell that a plain + // slot store would not update, so the two views could disagree; decline + // rather than repair only half of them. + if ctx.closure_captures.contains_key(&array_id) { + return None; + } if !local_bound_is_loop_invariant(condition?, update, body, array_id) { return None; } diff --git a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs index 885e0a5bb5..7f2c67a881 100644 --- a/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs +++ b/crates/perry-codegen/src/stmt/element_shape_loop_tests.rs @@ -199,14 +199,27 @@ fn emit(m: &Module) -> String { } /// The blocks that exist only when the clone was really built AND entered. -const CLONE_LABELS: [&str; 5] = [ +const CLONE_LABELS: [&str; 6] = [ "element_shape.loop.preheader.brand", + "element_shape.loop.preheader.repair", "element_shape.loop.preheader.query", "element_shape.loop.preheader.deref", "element_shape.loop.fast.preheader", "element_shape.load", ]; +/// The emitted text of one named block, up to the next block label. +fn block_slice<'a>(ir: &'a str, label: &str) -> &'a str { + let start = ir + .find(&format!("\n{label}")) + .unwrap_or_else(|| panic!("block `{label}` should be present in the emitted IR")); + let body = &ir[start + 1..]; + let end = body + .find("\n\n") + .unwrap_or_else(|| panic!("block `{label}` should be terminated by a blank line")); + &body[..end] +} + /// The emitted text the fast clone owns: from its cond block to the slow /// clone's. fn fast_clone_slice(ir: &str) -> &str { @@ -399,3 +412,121 @@ fn a_program_with_no_qualifying_loop_pays_nothing() { "a module with no qualifying loop must not call the guard" ); } + +// --------------------------------------------------------------------------- +// #7480: growth-forwarding repair. +// +// `js_array_grow` allocates the larger array elsewhere and leaves a forwarding +// stub behind whose first payload word (`length`‖`capacity`) is OVERWRITTEN +// with the new head. Every runtime entry point resolves the chain, so the +// guard call answers about the LIVE array — but the emitted code reads +// `length` and the elements base off the raw pointer, and on a stub both are +// wrong in the worst possible combination: `length` reads the low half of a +// heap pointer (a huge number, so the `length >= bound` test PASSES), while +// the base still addresses the pre-growth buffer. The clone then reads correct +// elements up to the old capacity and runs off the end of the block after it — +// masked, dereferenced at `-8`, SIGBUS. With `MIN_ARRAY_CAPACITY == 16` that +// is exactly the "right answer at 16 elements, bus error at 17" shape #7480 +// reproduced. +// +// The repair is repsel 4a.2's (#6904) self-heal: follow the chain once and +// write the live head back to the binding, BEFORE the guard call — the refresh +// can itself allocate (a lazy array materializes inside `clean_arr_ptr`), so +// doing it afterwards would reintroduce the very "base derived across an +// allocating call" hazard that step (4) exists to avoid. +// --------------------------------------------------------------------------- + +#[test] +fn preheader_repairs_the_array_head_before_the_guard_call() { + let ir = emit(&element_shape_module( + vec![accumulate_stmt( + SUM_ID, + ARRAY_ID, + Expr::LocalGet(COUNTER_ID), + )], + None, + )); + let repair = block_slice(&ir, "element_shape.loop.preheader.repair"); + assert!( + repair.contains("call double @js_array_refresh_local_head"), + "the repair block must follow the growth-forwarding chain; emitted:\n{repair}" + ); + + // Ordering is the whole fix. A refresh emitted AFTER the guard call would + // leave the elements base derived from the stub. + let refresh_at = ir + .find("call double @js_array_refresh_local_head") + .expect("growth-forwarding refresh call"); + let query_at = ir + .find("call i32 @js_array_ensure_element_shape") + .expect("element-shape guard call"); + assert!( + refresh_at < query_at, + "the growth-forwarding refresh must precede the element-shape query" + ); +} + +#[test] +fn the_repaired_head_is_written_back_to_the_binding() { + let ir = emit(&element_shape_module( + vec![accumulate_stmt( + SUM_ID, + ARRAY_ID, + Expr::LocalGet(COUNTER_ID), + )], + None, + )); + let repair = block_slice(&ir, "element_shape.loop.preheader.repair"); + + // WITHOUT a write-back the repair would be inert: the query and deref + // blocks both RE-READ the binding, so they would read the stub straight + // back out — which is precisely how the bug shipped. So the assertion is + // not "a store exists" but "the value stored is the refresh's result". + let refreshed = repair + .lines() + .find_map(|l| { + l.trim() + .split_once(" = call double @js_array_refresh_local_head") + }) + .map(|(reg, _)| reg.to_string()) + .expect("the refresh call should bind a register"); + assert!( + repair.lines().any(|l| l.trim().starts_with("store ")), + "the repair block must write the live head back; emitted:\n{repair}" + ); + // A root-slot store is rewritten by `function/precise_roots.rs` into + // `bitcast double to i64` + `inttoptr` + `store ptr addrspace(1)`, + // so the bitcast naming the refresh's result IS the write-back. + assert!( + repair.lines().any(|l| { + let l = l.trim(); + l.starts_with("%rs4gc.b") && l.contains(&format!("bitcast double {refreshed} to i64")) + }), + "the value stored back must be the refreshed head {refreshed}; emitted:\n{repair}" + ); +} + +#[test] +fn the_repair_does_not_put_a_call_inside_the_fast_clone() { + let ir = emit(&element_shape_module( + vec![accumulate_stmt( + SUM_ID, + ARRAY_ID, + Expr::LocalGet(COUNTER_ID), + )], + None, + )); + // The repair adds a call to the PREHEADER, which is fine. One inside the + // clone would void the revocation argument — and, because the lowering + // then branches unconditionally to the slow clone, would silently delete + // the optimization instead of failing. + let fast = fast_clone_slice(&ir); + assert!( + !fast.contains("call "), + "the fast clone must stay call-free after the #7480 repair; emitted:\n{fast}" + ); + assert!( + ir.contains("element_shape.loop.fast.preheader"), + "the clone must still be reached after the #7480 repair" + ); +} diff --git a/docs/engine-plan.md b/docs/engine-plan.md index 5956214624..57abf972b3 100644 --- a/docs/engine-plan.md +++ b/docs/engine-plan.md @@ -94,11 +94,15 @@ has three homes, each needing its own mechanism.* Phases **1 / 2 / 3a (#6909) / 3b (#6911) / 4a (#6915 + #7421/#7425) / 4b (#6919)** are all merged; #6904's 26× histogram is closed (#7485 deleted the dead 4b prototype flag). Next gap: -**element-shape proofs through array reads** — `keep[j].v` measured **6.2× vs -node** on the pure shape — route decided in **#7480**: both candidate routes -share one prerequisite (a per-array homogeneous-element-shape invariant, -construction-maintained, self-healing like 4a's dense bit), consumed first by -the #5093 versioned-loop clone, then by element `Ptr`. +**element-shape proofs through array reads** — `keep[j].v`, route decided in +**#7480**: both candidate routes share one prerequisite (a per-array +homogeneous-element-shape invariant, construction-maintained, self-healing +like 4a's dense bit), consumed first by the #5093 versioned-loop clone, then +by element `Ptr`. Prerequisite and consumer both landed (#7496, #7612); +the consumer's growth-forwarding crash is #7660. **The element `Ptr` +half is still open and the kernel re-measures at 34.5× node, not the 6.2× +#7480 recorded** — see backlog item 6 for the current table and for why the +named-class arm's node baseline is not the literal arm's. ### Object construction — the dominant cost (#7469 campaign) @@ -456,10 +460,48 @@ already working, on a workload that happens to reach it through `JSON.parse`. `gc-rooting-invariant.md` records as having already shipped broken. Today's ~20 rooting bugs were all found by hand with `PERRY_GC_PROTECT_FROMSPACE` because nothing else can find them. This is the structural fix. -6. **Repsel** — the element-shape invariant landed (#7496); the versioned-loop - consumer and element `Ptr` remain. Deliberately sequenced **after** - the bookkeeping levers: element reads are 13% of `churn` at 4.3×, the best - ratio in the table, so this is an RSS/footprint play more than a time one. +6. **Repsel** — the element-shape invariant landed (#7496) and its + versioned-loop consumer landed (#7612, matrix #7608, corpus #7619). What + remains of #7480 is **element `Ptr` for object-literal element + types**, and it is the whole measured gap, not a tail: #7612's + `element_class_name` resolves `Array(Named(C))` only, so #7480's own kernel + (`keep: {v,w}[]`) never reaches the clone. Re-measured 2026-08-08 on the + pinned quiet host, 200k elements × 50 sweeps, checksums equal, interleaved: + + | kernel | perry | node | bun | ratio | + |---|--:|--:|--:|--:| + | `keep: {v,w}[]` — #7480's kernel | 414 ms | 12 ms | 12 ms | **34.5×** | + | `keep: Node[]` — what #7612 covers | 13 ms‡ | 57 ms† | 15 ms | **0.23×** | + + **The issue's recorded 93 ms / 6.2× is stale and was optimistic** — the + fourth time a figure in this plan has gone stale, and the first in that + direction. Two traps in this one table, both of which cost time to find: + + †Node is *slower* on the named-class arm than on the literal arm (57 vs + 12 ms). `v: number;` cannot be erased by type stripping — without the + annotation it is still a valid class field declaration — so V8 + pre-initialises the slot to `undefined`, pinning the field to tagged + representation. **"Beat node" is therefore a different number on the two + arms**; re-measure the arm you are gating on, in the same run. + + ‡That cell did not exist before #7660: on `main` the named-class arm + *SIGBUSes*, because the versioned-loop consumer derived its elements base + from an unresolved growth-forwarding stub for any array grown outside the + scope that reads it. Step 2's win was real but gated behind a crash. + + The IR does not match the issue's recorded cost model either. #7480 says + the body has "no out-of-line guard calls, the cost is stacked inline + diamonds"; the object-literal arm actually carries **three calls per + iteration** — `js_typed_feedback_observe_property_get`, + `js_typed_feedback_record_guard_pass` (on the *hit* path) and + `js_dynamic_string_or_number_add`, because with no resolvable class the + field read falls into the by-name PIC tower and the accumulator loses its + numeric proof. That is a second, separable lever, and it is the kind of win + that gets misattributed to the element-shape work. + + Sequencing note carried from before: element reads are 13% of `churn` at + 4.3×, so against the bookkeeping levers this stays an RSS/footprint play + more than a time one — but the 34.5× above is the kernel, and it is real. 7. **Layer 1** — migrate remaining lowerings onto the rooted-combinator API (`crates/perry-codegen/src/rooting.rs`). **#7615 is the ordered worklist**: 88 modules, 694 raw-pointer sites, 262 hazard sites, grouped into ten diff --git a/test-files/test_gap_repsel_element_shape_loop_clone.ts b/test-files/test_gap_repsel_element_shape_loop_clone.ts index 80f89979c5..183526973c 100644 --- a/test-files/test_gap_repsel_element_shape_loop_clone.ts +++ b/test-files/test_gap_repsel_element_shape_loop_clone.ts @@ -265,3 +265,53 @@ for (let j = 0; j < 4; j++) { overOut += row === undefined ? "_" : String(row.v); } console.log("bound-past-length:", overOut); + +// --------------------------------------------------------------------------- +// 10. GROWTH FORWARDING (#7480). Every case above builds its array in the same +// scope that reads it, so the binding always held the LIVE head and the +// preheader's raw-pointer derivation happened to be right. It is not right +// in general: `js_array_grow` moves the array and leaves a forwarding stub +// whose first payload word (length‖capacity) is overwritten with the new +// head. The runtime resolves the chain, so the guard call still answers +// about the live array; a preheader that derived `length` and the elements +// base from the stub instead read the low half of a heap pointer as +// `length` (so the bound check PASSED) and addressed the pre-growth +// buffer — correct answers up to the initial capacity of 16, and a bus +// error at 17. +// +// Both shapes below cross that boundary. Keep the counts above 16. +// --------------------------------------------------------------------------- +function buildRows(n: number): Node[] { + const rows: Node[] = []; + for (let i = 0; i < n; i++) { + rows.push(new Node(i, i * 2)); + } + return rows; +} + +const returned = buildRows(40); +console.log("grown-callee-built:", sumField(returned, returned.length)); +// Re-entering must still be right: the binding is repaired in place, so the +// second visit takes the O(1) confirm path on an already-live head. +console.log("grown-callee-built-again:", sumField(returned, returned.length)); + +// A prefix of the same array — the index where a stale base first runs off the +// pre-growth block. +let prefix = 0; +for (let j = 0; j < 17; j++) { + prefix += returned[j].v; +} +console.log("grown-prefix-17:", prefix); + +// The canonical stale-head shape: the CALLER allocates, the CALLEE grows. The +// callee's write-backs update its own parameter slot, so the caller's binding +// keeps the pre-growth head. +function fill(rows: Node[], n: number): void { + for (let i = 0; i < n; i++) { + rows.push(new Node(i, i * 3)); + } +} + +const callerOwned: Node[] = []; +fill(callerOwned, 40); +console.log("grown-callee-filled:", sumField(callerOwned, callerOwned.length));