diff --git a/Cargo.lock b/Cargo.lock index 657d527fcb..e918a433ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "inkwell", @@ -5638,7 +5638,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-hir", @@ -5646,7 +5646,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-hir", @@ -5654,7 +5654,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-dispatch", @@ -5663,7 +5663,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-hir", @@ -5671,7 +5671,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "base64", @@ -5683,7 +5683,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-hir", @@ -5691,7 +5691,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "async-trait", @@ -5720,14 +5720,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "serde", "serde_json", @@ -5735,7 +5735,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1288" +version = "0.5.1289" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5746,7 +5746,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "clap", @@ -5761,7 +5761,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "block2", "objc2", @@ -5771,7 +5771,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "argon2", "perry-ffi", @@ -5779,7 +5779,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "reqwest", @@ -5788,7 +5788,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bcrypt", "perry-ffi", @@ -5796,7 +5796,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "rusqlite", @@ -5804,7 +5804,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "scraper", @@ -5812,7 +5812,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "perry-runtime", @@ -5820,7 +5820,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "chrono", "cron", @@ -5830,7 +5830,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "chrono", "perry-ffi", @@ -5838,7 +5838,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "rust_decimal", @@ -5846,7 +5846,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "serde_json", @@ -5854,7 +5854,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5862,7 +5862,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "perry-runtime", @@ -5870,14 +5870,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bytes", "http-body-util", @@ -5895,7 +5895,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bytes", "lazy_static", @@ -5908,7 +5908,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bytes", "h2", @@ -5932,7 +5932,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "lazy_static", "perry-ffi", @@ -5942,7 +5942,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "jsonwebtoken", @@ -5953,7 +5953,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "lru", "perry-ffi", @@ -5962,7 +5962,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "chrono", "perry-ffi", @@ -5970,7 +5970,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bson", "futures-util", @@ -5982,7 +5982,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "chrono", "perry-ffi", @@ -5992,7 +5992,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "nanoid", "perry-ffi", @@ -6001,7 +6001,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "bytes", "perry-ffi", @@ -6014,7 +6014,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6033,7 +6033,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "lettre", "perry-ffi", @@ -6043,7 +6043,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "printpdf", @@ -6051,7 +6051,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "sqlx", @@ -6060,7 +6060,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "governor", "perry-ffi", @@ -6068,7 +6068,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "fast_image_resize", "image", @@ -6078,14 +6078,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "lazy_static", "perry-ffi", @@ -6094,7 +6094,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "perry-runtime", @@ -6103,7 +6103,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "uuid", @@ -6111,7 +6111,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ffi", "regex", @@ -6121,7 +6121,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "futures-util", "lazy_static", @@ -6134,7 +6134,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "brotli", "flate2", @@ -6144,7 +6144,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "dashmap", "once_cell", @@ -6153,7 +6153,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-api-manifest", @@ -6171,7 +6171,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-diagnostics", @@ -6183,7 +6183,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "base64", @@ -6225,14 +6225,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6327,14 +6327,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "perry-hir", @@ -6343,14 +6343,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "itoa", @@ -6367,7 +6367,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "rand 0.10.1", "serde", @@ -6377,7 +6377,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6400,7 +6400,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "block2", @@ -6416,7 +6416,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "block2", @@ -6431,7 +6431,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1288" +version = "0.5.1289" [[package]] name = "perry-ui-test" @@ -6442,11 +6442,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1288" +version = "0.5.1289" [[package]] name = "perry-ui-tvos" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "block2", @@ -6462,7 +6462,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "block2", @@ -6478,7 +6478,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "block2", "libc", @@ -6491,7 +6491,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "base64", "libc", @@ -6508,14 +6508,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "anyhow", "base64", @@ -6531,7 +6531,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1288" +version = "0.5.1289" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 8111ab8e42..0c97f79bec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1288" +version = "0.5.1289" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7501-layout-declared-at-alloc.md b/changelog.d/7501-layout-declared-at-alloc.md new file mode 100644 index 0000000000..8ccdf20cda --- /dev/null +++ b/changelog.d/7501-layout-declared-at-alloc.md @@ -0,0 +1,64 @@ +**GC element layout is now declared once at allocation for proven pointer arrays, +instead of maintained per push (#7469 mutator-cost campaign).** + +An `out = []` + `out.push({ … })` loop paid two runtime calls on every iteration +of its inline store: `js_gc_note_slot_layout`, which reaches into the +`LAYOUT_SLOT_MASKS` thread-local hashmap to set one bit of a per-array pointer +bitmap, and `js_array_note_numeric_write`, which clears a raw-f64 flag that is +already clear. The mask entry the first push creates also makes +`LAYOUT_SLOT_MASKS` non-empty for the rest of the process, so the `is_empty()` +fast-out in `layout_forget_object` — probed on **every** object allocation — +starts hashing instead. + +When codegen can see the whole life of an array local (a single `[]` binding, +every store a push of a fresh allocation, no rebind, no keyed store, no closure +capture, no in-place mutator, and at least one such push in the same region), it +now emits one `js_array_declare_all_pointer_elements` at the allocation site and +drops both per-push calls. New collector: `perry-codegen/src/collectors/ +all_pointer_arrays.rs`. + +**Soundness does not rest on that static proof.** Every elided store is fronted +by a test of the live GC header, folded into the frozen/sealed integrity test the +inline push already performed — the same `_reserved` load, the same one `and` + +one `icmp`, no added instruction: `(_reserved & 0xF487) == 0xA000`. That demands +the four integrity bits clear (as before), **both** raw-f64 layout bits clear +(which is what makes eliding the numeric-write note sound — there is nothing left +for it to clear), and `GC_LAYOUT_SIDE_MASK | GC_LAYOUT_ALL_POINTERS` set (which is +what makes eliding the layout note sound — in that state the collector visits +every slot in `0..length`, so the slot being written is scanned whether or not a +mask bit was recorded). A push that fails the test falls through to +`js_array_push_f64`, which notes the slot exactly as it always did. + +Testing the live header rather than trusting the declaration is load-bearing, +because the runtime revokes it: `js_array_is_numeric_f64_layout` on a still-empty +declared array verifies vacuously and re-publishes it as RawF64 + `POINTER_FREE`, +and `rebuild_array_layout` (sort/splice) installs a precise mask. Both are now +regression tests. + +`layout_note_slot` gained one refinement: a pointer **append** at an array's +append position (`slot_index == length` — every append protocol in the tree +records the slot before bumping `length`) preserves an all-pointer declaration +instead of downgrading it. Without that the first growth through +`js_array_push_f64` demoted the array and every later push fell off the declared +path permanently. + +`js_string_addref_if_heap_string` is now gated independently of the layout note +at array pushes (`store_needs_string_addref`). The declaration predicate admits +`Expr::New` — HIR rewrites closed-shape object literals to +`New { class_name: "__AnonShape_" }` before codegen runs, so the loop this +targets *is* the `new` form — and a `new` can be re-pointed by a constructor +return override, so it must never gate the string demote. + +Witness coverage in `perry-runtime/src/gc/tests/copying/all_pointer_elements_7469.rs`: +an array filled through the elided sequence survives a copying minor with every +element relocated and every slot rewritten, and a permanent sabotage arm asserts +the *undeclared* array enumerates zero child slots — so a green positive test +means the declaration was load-bearing, not that nothing was tried. End to end, a +compiled smoke over these shapes is byte-identical to `node` both normally and +under `PERRY_GC_MOVING_LOOP_POLLS=1` + `PERRY_GC_ZEAL=1 +PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_VERIFY_EVACUATION=1`, with `PERRY_GC_DIAG=1` +confirming thousands of protected from-space retirements actually ran. + +Known non-firing shapes: both halves of a deforested producer/consumer pair (the +producer's `const out = []` becomes a `__deforest_out` parameter; the consumer's +literal has no push in its own region). diff --git a/crates/perry-codegen/src/collectors/all_pointer_arrays.rs b/crates/perry-codegen/src/collectors/all_pointer_arrays.rs new file mode 100644 index 0000000000..46ea09de50 --- /dev/null +++ b/crates/perry-codegen/src/collectors/all_pointer_arrays.rs @@ -0,0 +1,448 @@ +//! #7469 — array locals whose GC element layout is known **at the allocation +//! site**, so the per-store pointer-mask bookkeeping can be declared once +//! instead of maintained per push. +//! +//! An `out = []` + `out.push(objectLiteral)` loop pays, on every iteration, a +//! `js_gc_note_slot_layout` call that walks into a thread-local hashmap to set +//! one bit of a per-array bitmap, plus a `js_array_note_numeric_write` call +//! that clears an already-clear flag. Worse, the `LAYOUT_SLOT_MASKS` entry it +//! creates makes the map non-empty for the rest of the program, so the +//! `is_empty()` fast-out in `layout_forget_object` — probed on **every** object +//! allocation — starts hashing instead. Both disappear if the array's element +//! layout is declared once, at allocation, as all-pointer. +//! +//! # What this collector proves +//! +//! For a local `id` it admits the declaration only when ALL of the following +//! hold over the whole region: +//! +//! 1. **Exactly one binding, and it is an array literal.** `id` is bound by a +//! `Stmt::Let { init: Some(Expr::Array(elems)) }`, and every element of +//! `elems` is an allocation site +//! ([`crate::expr::expr_produces_fresh_heap_allocation`]); the empty +//! literal — the shape this exists for — passes vacuously. +//! 2. **No other write to the binding.** No `LocalSet`, no `Update`. A rebind +//! would point the local at an array the declaration never covered, and the +//! elided stores would then be describing the wrong object. +//! 3. **Every store into it is a push of an allocation, and there is at least +//! one.** Every `Expr::ArrayPush { array_id: id }` pushes a fresh +//! allocation; no `Expr::ArrayPushSpread`, no `Expr::IndexSet` whose object +//! is `LocalGet(id)` (an indexed store can jump past `length`, a different +//! claim than "append"), no other in-place array mutation. +//! +//! The "at least one" is not decoration. Deforestation +//! (`perry-transform/src/deforest`) rewrites a producer's call site to +//! `const all = []; f(args, all);` — an array literal binding whose stores +//! all live in another region. Declaring THAT array would be a blind bet on +//! a callee this collector cannot see, and if the callee pushes numbers the +//! bet loses (the array is stuck on a conservative scan and has lost its +//! raw-f64 layout). Requiring a proven push in the same region refuses it. +//! 4. **No closure capture, no box, no module global.** Those route stores +//! through paths this region cannot see and cannot gate. +//! +//! # Shapes this deliberately does not reach +//! +//! The deforested PRODUCER is the mirror image of the case above: its +//! `const out = []` is gone (the accumulator arrives as the +//! `__deforest_out` parameter) and only the pushes remain, so there is no +//! allocation site in the region to declare at. Both halves of a deforested +//! producer/consumer pair are therefore out of scope; the analysis fires on +//! arrays built and kept inside one region. +//! +//! # What the proof is and is not responsible for +//! +//! It is NOT the soundness argument for the elided store. Declaring +//! all-pointer is conservative for the collector in the only direction that +//! matters (`GC_LAYOUT_ALL_POINTERS` visits `0..length`, exactly what +//! `GC_LAYOUT_UNKNOWN` visits; a non-pointer slot is re-validated and +//! rejected). What makes the elision sound is the **header test emitted at +//! every elided store** (`expr/array_push.rs`): the inline push happens only +//! when the array's `_reserved` word still reads +//! `SIDE_MASK | ALL_POINTERS` with both raw-f64 bits clear, and any push that +//! fails the test falls through to `js_array_push_f64`, which notes the slot +//! exactly as it always did. That is what covers a declaration the RUNTIME +//! later revokes — `rebuild_array_layout` (sort/splice) installs a precise +//! mask, `js_array_is_numeric_f64_layout` can re-publish a still-empty array +//! as `POINTER_FREE` — which no static proof over this region could see. +//! +//! This proof's job is therefore *profitability*: it picks arrays where the +//! declaration will stick, so the fast path is actually taken. +//! +//! That is also the standard the kill list below is written to. It enumerates +//! the write forms that reach an array local in lowered HIR — rebinds, keyed +//! stores (`IndexSet` / `IndexUpdate` / `PropertySet` / `PropertyUpdate` / +//! `PutValueSet`), and the in-place `Array*` mutators — and a form it MISSES +//! cannot produce a wrong layout: the missed store either carries its own note +//! (every un-elided store path does) or fails the header test and routes +//! through `js_array_push_f64`. The cost of a miss is that the array is +//! downgraded to a conservative scan earlier than predicted, which for an +//! all-pointer array is the same set of slots the precise mask would have +//! visited. + +use std::collections::{HashMap, HashSet}; + +use perry_hir::{Expr, Stmt}; + +/// Locals eligible for an at-allocation all-pointer element-layout +/// declaration. See the module docs for the four admission terms. +pub(crate) fn collect_all_pointer_array_locals( + stmts: &[Stmt], + boxed_vars: &HashSet, + module_globals: &HashMap, +) -> HashSet { + let mut candidates: HashSet = HashSet::new(); + walk_stmts(stmts, &mut |stmt| { + if let Stmt::Let { + id, + init: Some(Expr::Array(elements)), + .. + } = stmt + { + if boxed_vars.contains(id) || module_globals.contains_key(id) { + return; + } + if elements + .iter() + .all(crate::expr::expr_produces_fresh_heap_allocation) + { + candidates.insert(*id); + } + } + }); + if candidates.is_empty() { + return candidates; + } + + let mut killed: HashSet = HashSet::new(); + let mut pushed: HashSet = HashSet::new(); + super::scalar_method_dispatch::for_each_expr_in_stmts(stmts, &mut |expr| match expr { + // Rebinding the local points it at an array the declaration never + // covered. + Expr::LocalSet(id, _) => { + killed.insert(*id); + } + Expr::Update { id, .. } => { + killed.insert(*id); + } + // A keyed store is not an append: `a[10] = x` on a length-2 array is a + // different layout claim than the push protocol's + // `slot_index == length`, and `a.length = 0` is not a store at all. + // `PutValueSet` is the form a lowered `a[i] = x` actually takes when + // the receiver is not statically an array-typed local. + Expr::IndexSet { object, .. } + | Expr::IndexUpdate { object, .. } + | Expr::PropertySet { object, .. } + | Expr::PropertyUpdate { object, .. } => { + if let Expr::LocalGet(id) = object.as_ref() { + killed.insert(*id); + } + } + Expr::PutValueSet { + target, receiver, .. + } => { + for expr in [target, receiver] { + if let Expr::LocalGet(id) = expr.as_ref() { + killed.insert(*id); + } + } + } + Expr::ArraySort { array, .. } => { + // `sort` routes through `rebuild_array_layout`, which installs a + // PRECISE mask in place of the declaration. + if let Expr::LocalGet(id) = array.as_ref() { + killed.insert(*id); + } + } + // Every other in-place array mutation reaches slots that are not the + // append position (`unshift`/`splice`/`copyWithin` shift or replace, + // `pop`/`shift` shrink), and lowers through runtime helpers this + // region cannot gate. None of them is unsound under a declaration — + // the header test still fronts every elided store — but each makes + // the declaration unlikely to survive, so they are refused. + Expr::ArrayPushSpread { array_id, .. } + | Expr::ArrayUnshift { array_id, .. } + | Expr::ArraySplice { array_id, .. } + | Expr::ArrayCopyWithin { array_id, .. } => { + killed.insert(*array_id); + } + Expr::ArrayPop(array_id) | Expr::ArrayShift(array_id) => { + killed.insert(*array_id); + } + Expr::ArrayPush { array_id, value } => { + if crate::expr::expr_produces_fresh_heap_allocation(value) { + pushed.insert(*array_id); + } else { + killed.insert(*array_id); + } + } + // A captured array is stored into from a region this collector is not + // analysing, through a lowering path that cannot carry the gate. + Expr::Closure { captures, .. } => { + killed.extend(captures.iter().copied()); + } + _ => {} + }); + + candidates.retain(|id| !killed.contains(id) && pushed.contains(id)); + candidates +} + +/// Statement-level descent. `for_each_expr_in_stmts` visits expressions, not +/// `Stmt::Let` ids, so the binding scan needs its own walk. It descends into +/// every nested statement position; a `Let` inside a closure body belongs to +/// that closure's own region (which collects its own facts) and its id is +/// distinct, so seeing it here is harmless — the kill walk above descends into +/// closures too, and every use of such an id is inside the closure body it is +/// scoped to. +fn walk_stmts<'a>(stmts: &'a [Stmt], f: &mut impl FnMut(&'a Stmt)) { + for s in stmts { + f(s); + match s { + Stmt::If { + then_branch, + else_branch, + .. + } => { + walk_stmts(then_branch, f); + if let Some(eb) = else_branch { + walk_stmts(eb, f); + } + } + Stmt::While { body, .. } | Stmt::DoWhile { body, .. } => walk_stmts(body, f), + Stmt::For { init, body, .. } => { + if let Some(init) = init { + walk_stmts(std::slice::from_ref(init.as_ref()), f); + } + walk_stmts(body, f); + } + Stmt::Try { + body, + catch, + finally, + } => { + walk_stmts(body, f); + if let Some(c) = catch { + walk_stmts(&c.body, f); + } + if let Some(fin) = finally { + walk_stmts(fin, f); + } + } + Stmt::Switch { cases, .. } => { + for c in cases { + walk_stmts(&c.body, f); + } + } + Stmt::Labeled { body, .. } => walk_stmts(std::slice::from_ref(body.as_ref()), f), + _ => {} + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use perry_hir::types::Type; + + fn let_array(id: u32, elements: Vec) -> Stmt { + Stmt::Let { + id, + name: format!("a{id}"), + ty: Type::Any, + mutable: true, + init: Some(Expr::Array(elements)), + } + } + + fn push(id: u32, value: Expr) -> Stmt { + Stmt::Expr(Expr::ArrayPush { + array_id: id, + value: Box::new(value), + }) + } + + fn object_literal() -> Expr { + Expr::Object(vec![("v".to_string(), Expr::Integer(1))]) + } + + fn collect(stmts: &[Stmt]) -> HashSet { + collect_all_pointer_array_locals(stmts, &HashSet::new(), &HashMap::new()) + } + + #[test] + fn empty_literal_plus_object_pushes_is_admitted() { + let stmts = vec![let_array(1, vec![]), push(1, object_literal())]; + assert!(collect(&stmts).contains(&1)); + } + + #[test] + fn literal_of_object_elements_is_admitted() { + let stmts = vec![ + let_array(1, vec![object_literal(), object_literal()]), + push(1, object_literal()), + ]; + assert!(collect(&stmts).contains(&1)); + } + + #[test] + fn a_numeric_element_in_the_literal_is_refused() { + let stmts = vec![ + let_array(1, vec![object_literal(), Expr::Integer(3)]), + push(1, object_literal()), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + /// HIR rewrites a closed-shape object literal into + /// `New { class_name: "__AnonShape_" }` before codegen runs, so the + /// `new` form IS the object-literal push loop this analysis targets. + #[test] + fn a_new_expression_push_is_admitted() { + let stmts = vec![ + let_array(1, vec![]), + push( + 1, + Expr::New { + class_name: "__AnonShape_deadbeef".to_string(), + args: vec![], + type_args: vec![], + byte_offset: 0, + cap_args_appended: 0, + }, + ), + ]; + assert!(collect(&stmts).contains(&1)); + } + + /// The deforested CONSUMER shape: an array literal binding whose stores all + /// live in a callee. Declaring it would be a bet on a region this collector + /// cannot see. + #[test] + fn a_literal_with_no_push_in_this_region_is_refused() { + let stmts = vec![ + let_array(1, vec![]), + Stmt::Expr(Expr::Call { + callee: Box::new(Expr::FuncRef(7)), + args: vec![Expr::LocalGet(1)], + type_args: vec![], + byte_offset: 0, + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn one_unproven_push_refuses_the_whole_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + push(1, Expr::Integer(7)), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn a_rebind_refuses_the_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::LocalSet(1, Box::new(Expr::Array(vec![])))), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn an_indexed_store_refuses_the_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Integer(4)), + value: Box::new(object_literal()), + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + /// `out[0] = {...}` on an array-typed local lowers to `PutValueSet`, not + /// `IndexSet` — the form the kill list originally missed, caught by the + /// probe in the #7469 PR. + #[test] + fn a_put_value_store_refuses_the_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::LocalGet(1)), + key: Box::new(Expr::Integer(0)), + value: Box::new(object_literal()), + receiver: Box::new(Expr::LocalGet(1)), + strict: false, + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn a_length_write_refuses_the_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(1)), + property: "length".to_string(), + value: Box::new(Expr::Integer(0)), + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn a_spread_push_refuses_the_local() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::ArrayPushSpread { + array_id: 1, + source: Box::new(Expr::LocalGet(2)), + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } + + #[test] + fn a_boxed_or_module_global_binding_is_refused() { + let stmts = vec![let_array(1, vec![]), push(1, object_literal())]; + let boxed: HashSet = [1].into_iter().collect(); + assert!(!collect_all_pointer_array_locals(&stmts, &boxed, &HashMap::new()).contains(&1)); + let globals: HashMap = [(1, "g".to_string())].into_iter().collect(); + assert!(!collect_all_pointer_array_locals(&stmts, &HashSet::new(), &globals).contains(&1)); + } + + /// Pushes inside a nested closure lower through a path that carries no + /// header gate, so a captured array must not be declared. + #[test] + fn a_captured_local_is_refused() { + let stmts = vec![ + let_array(1, vec![]), + push(1, object_literal()), + Stmt::Expr(Expr::Closure { + func_id: 9, + params: vec![], + return_type: Type::Any, + body: vec![push(1, object_literal())], + captures: vec![1], + mutable_captures: vec![], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: false, + }), + ]; + assert!(!collect(&stmts).contains(&1)); + } +} diff --git a/crates/perry-codegen/src/collectors/hir_facts.rs b/crates/perry-codegen/src/collectors/hir_facts.rs index c3928efd08..16d51b46be 100644 --- a/crates/perry-codegen/src/collectors/hir_facts.rs +++ b/crates/perry-codegen/src/collectors/hir_facts.rs @@ -90,6 +90,13 @@ pub(crate) struct ArrayFacts { pub local_kinds: HashMap, pub length_stable_locals: HashSet, pub noalias_locals: HashSet, + /// #7469: array locals whose element layout is declarable **once, at the + /// allocation site**, as all-pointer — an `[]` literal binding whose every + /// store in this region is a push of a by-construction heap pointer. See + /// `collectors/all_pointer_arrays.rs` for the four admission terms and for + /// why this fact governs *profitability* rather than the soundness of the + /// elided per-store note (which the emitted header test owns). + pub all_pointer_element_locals: HashSet, } #[derive(Debug, Clone, Default)] @@ -246,6 +253,14 @@ impl TypeFacts { self.arrays.noalias_locals.contains(&local_id) } + /// #7469: this array local's element layout may be declared all-pointer at + /// its allocation site, and its proven-pointer pushes may then elide the + /// per-store layout note behind the emitted header test. See + /// `collectors/all_pointer_arrays.rs`. + pub(crate) fn declares_all_pointer_elements(&self, local_id: u32) -> bool { + self.arrays.all_pointer_element_locals.contains(&local_id) + } + pub(crate) fn array_length_mutation_locals(&self) -> &HashSet { &self.effect.array_length_mutation_locals } @@ -462,8 +477,15 @@ pub(crate) fn collect_type_facts( }; let not_bigint_locals = super::not_bigint_locals::collect_not_bigint_locals(stmts, params, binding_types); - let (array_facts, effect_facts, materialization_hazards) = + let (mut array_facts, effect_facts, materialization_hazards) = collect_array_facts(stmts, params, module_globals, binding_types); + // #7469: at-allocation all-pointer element-layout declaration candidates. + array_facts.all_pointer_element_locals = + super::all_pointer_arrays::collect_all_pointer_array_locals( + stmts, + boxed_vars, + module_globals, + ); let index_used_locals = super::index_uses::collect_index_used_locals(stmts); // Repsel Phase 1: under `PERRY_CANONICAL_I32_LOCALS` (default on), a // proven in-window const int-typed-array element load counts as a STRICT @@ -1381,6 +1403,9 @@ impl ArrayFactCollector { local_kinds: self.local_kinds, length_stable_locals, noalias_locals, + // Filled in by `collect_type_facts` — its own walk, with its + // own admission terms, over the same statements. + all_pointer_element_locals: HashSet::new(), }, EffectFacts { unknown_call_escape: self.unknown_call_escape, diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 1c316825b9..c2826e3a65 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -5,6 +5,7 @@ //! v0.5.1019 to satisfy the file-size CI gate. mod.rs is a re-export //! hub — public-API shape (`crate::collectors::*`) is preserved. +mod all_pointer_arrays; mod cjs_scaffolding; mod clamp_detect; mod class_accessors; diff --git a/crates/perry-codegen/src/expr/array_push.rs b/crates/perry-codegen/src/expr/array_push.rs index 43a57f91bf..923435ca12 100644 --- a/crates/perry-codegen/src/expr/array_push.rs +++ b/crates/perry-codegen/src/expr/array_push.rs @@ -16,7 +16,7 @@ use crate::types::{DOUBLE, I1, I16, I32, I64, I8}; use super::{ array_store_needs_layout_note, array_store_needs_write_barrier, - emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_on_block, + emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_with_flags_on_block, emit_jsvalue_slot_store_with_value_bits_on_block, emit_root_nanbox_store_on_block, emit_typed_feedback_register_site, emit_write_barrier, expr_has_numeric_pointer_free_array_layout, lower_expr, lower_expr_native, @@ -77,7 +77,29 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // module-level global. The realloc-pointer write-back must // go to whichever storage we read from. let array_expr = Expr::LocalGet(*array_id); - let layout_note_needed = array_store_needs_layout_note(ctx, &array_expr, value); + // #7469: this local's element layout was declared all-pointer at + // its allocation site (`collectors/all_pointer_arrays.rs` proved + // every store into it is a push of a by-construction heap + // pointer), and THIS pushed value is one of them. The inline store + // below then needs neither the per-slot layout note nor the + // numeric-write note — but only behind the header test in the + // `nofwd` block, which re-validates the declaration at every single + // push. Any push that fails it falls through to `js_array_push_f64` + // and records the slot exactly as it always did. + let declared_all_pointer = ctx.native_facts.declares_all_pointer_elements(*array_id) + && crate::expr::expr_produces_fresh_heap_allocation(value); + let layout_note_needed = + !declared_all_pointer && array_store_needs_layout_note(ctx, &array_expr, value); + // The string-addref demote is a DIFFERENT question from the layout + // note and must not ride its gate here: `expr_produces_fresh_heap_ + // allocation` admits `new C()`, whose constructor return override + // can hand back a uniquely-owned heap string. Every other push + // keeps the historical coupling exactly. + let string_addref_needed = if declared_all_pointer { + crate::expr::store_needs_string_addref(ctx, value) + } else { + layout_note_needed + }; let write_barrier_needed = array_store_needs_write_barrier(ctx, value); let value_is_numeric = is_numeric_expr(ctx, value); let require_numeric_layout = @@ -333,9 +355,52 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let flags_addr = blk.sub(I64, &arr_handle, "6"); let flags_ptr = blk.inttoptr(I64, &flags_addr); let obj_flags = blk.load(I16, &flags_ptr); - // FROZEN(0x1)|SEALED(0x2)|NO_EXTEND(0x4)|ARRAY_DESCRIPTORS(0x400). - let integrity_bits = blk.and(I16, &obj_flags, "1031"); - let clean = blk.icmp_eq(I16, &integrity_bits, "0"); + let clean = if declared_all_pointer { + // #7469 — the elided-bookkeeping admission test. Same + // `_reserved` load, same one `and` + one `icmp` as the + // integrity test it replaces, but it additionally + // demands the array still carry the element-layout + // declaration this push's elisions rest on. Bits, from + // `gc/types.rs` + `gc/layout.rs` (GC_TYPE_ARRAY): + // + // 0x0407 FROZEN|SEALED|NO_EXTEND|ARRAY_DESCRIPTORS + // -> must be 0, exactly as below + // 0x0080 GC_ARRAY_RAW_F64_LAYOUT -> must be 0 + // 0x1000 GC_ARRAY_RAW_F64_HOLES -> must be 0 + // 0x2000 GC_LAYOUT_ALL_POINTERS -> must be 1 + // 0xC000 layout state -> SIDE_MASK + // + // mask 0xF487 == 62599, expected 0xA000 == 40960. + // + // The two raw-f64 bits are what makes eliding + // `js_array_note_numeric_write` sound: its whole body + // is "clear the numeric layout when the value is not a + // number", and with both bits already clear there is + // nothing left for it to clear. + // + // `ALL_POINTERS | SIDE_MASK` is what makes eliding + // `js_gc_note_slot_layout` sound: in that state the + // collector visits every slot in `0..length`, so the + // slot this push is about to write is scanned whether + // or not a mask bit was ever recorded for it. + // + // Testing the LIVE header rather than trusting the + // allocation-site declaration is deliberate. The + // runtime can revoke it — `rebuild_array_layout` + // (sort/splice) installs a precise mask, + // `js_array_is_numeric_f64_layout` can re-publish a + // still-empty array as RawF64 + POINTER_FREE — and an + // elided pointer store into a POINTER_FREE array is a + // stranded live child. Failing the test costs this push + // the inline store (it takes `js_array_push_f64`, which + // notes the slot); it can never cost correctness. + let admitted_bits = blk.and(I16, &obj_flags, "62599"); + blk.icmp_eq(I16, &admitted_bits, "40960") + } else { + // FROZEN(0x1)|SEALED(0x2)|NO_EXTEND(0x4)|ARRAY_DESCRIPTORS(0x400). + let integrity_bits = blk.and(I16, &obj_flags, "1031"); + blk.icmp_eq(I16, &integrity_bits, "0") + }; let length = blk.safe_load_i32_from_ptr(&arr_handle); let cap_addr = blk.add(I64, &arr_handle, "4"); let cap_ptr = blk.inttoptr(I64, &cap_addr); @@ -367,25 +432,30 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { value_bits, &arr_handle, &length, + string_addref_needed, layout_note_needed, &arr_handle, &element_addr, write_barrier_needed, ) } else { - emit_jsvalue_slot_store_on_block( + emit_jsvalue_slot_store_with_flags_on_block( blk, &element_ptr, &v, &arr_handle, &length, + string_addref_needed, layout_note_needed, &arr_handle, &element_addr, write_barrier_needed, ) }; - if !value_is_numeric { + // #7469: provably dead under `declared_all_pointer` — the + // `nofwd` admission test proved both raw-f64 bits already + // clear, and clearing them is this call's only effect. + if !value_is_numeric && !declared_all_pointer { let value_bits = value_bits.unwrap_or_else(|| blk.bitcast_double_to_i64(&v)); emit_array_numeric_write_note_on_block(blk, &arr_handle, &value_bits); diff --git a/crates/perry-codegen/src/expr/helpers.rs b/crates/perry-codegen/src/expr/helpers.rs index cc900545fc..a8f1763fdc 100644 --- a/crates/perry-codegen/src/expr/helpers.rs +++ b/crates/perry-codegen/src/expr/helpers.rs @@ -122,6 +122,62 @@ pub(crate) fn expr_produces_non_pointer_bits_by_construction(ctx: &FnCtx<'_>, ex } } +/// The expression's shape IS an allocation site: an object / array / closure +/// literal, or a `new`. +/// +/// Read the claim precisely, because the `Expr::New` arm makes it weaker than +/// [`expr_produces_non_pointer_bits_by_construction`]'s mirror image. What a +/// `new C()` *evaluates to* is a runtime question — a constructor return +/// override (`js_ctor_return_override`) can hand back anything, including a +/// heap string. So this is **not** a proof that the stored bits carry +/// `POINTER_TAG`; it is a proof that the expression's purpose is to allocate +/// one. +/// +/// That is exactly, and only, what its two consumers need: +/// +/// - **Picking arrays worth declaring all-pointer** +/// (`collectors/all_pointer_arrays.rs`). A wrong guess costs scan time — +/// `GC_LAYOUT_ALL_POINTERS` visits the same slots `GC_LAYOUT_UNKNOWN` does, +/// and a non-pointer word is re-validated and rejected — never a stranded +/// child. +/// - **Eliding the per-push layout / numeric-write notes** +/// (`expr/array_push.rs`). Neither elision rests on this predicate at all: +/// both are proven by the header test emitted at the store. +/// +/// It must NEVER gate `js_string_addref_if_heap_string`. That demote is dead +/// only when the value provably is not a heap string — a claim the `New` arm +/// does not make, and whose absence is silent corruption rather than a crash +/// (a refcount==1 string aliased from an array slot, rewritten in place by a +/// later `+=` on the source local). `array_push.rs` gates it separately on +/// [`store_needs_string_addref`]. +/// +/// The `New` arm is load-bearing rather than a widening for its own sake: HIR +/// rewrites a closed-shape object literal into +/// `New { class_name: "__AnonShape_", … }` before codegen ever sees it, +/// so without it the predicate misses the object-literal push loop this whole +/// analysis exists for. +/// +/// Takes no `FnCtx`: the test is purely syntactic, so the per-region fact +/// collector (which runs before any lowering context exists) and the lowering +/// site consult the same function and can never disagree about a store. +pub(crate) fn expr_produces_fresh_heap_allocation(expr: &Expr) -> bool { + match expr { + Expr::Object(_) | Expr::Array(_) | Expr::Closure { .. } | Expr::New { .. } => true, + Expr::Conditional { + then_expr, + else_expr, + .. + } => { + expr_produces_fresh_heap_allocation(then_expr) + && expr_produces_fresh_heap_allocation(else_expr) + } + Expr::Sequence(exprs) => exprs + .last() + .is_some_and(expr_produces_fresh_heap_allocation), + _ => false, + } +} + /// Stores into statically numeric arrays may preserve the initial /// pointer-free layout only when the stored value's bits are known from /// expression construction, not from TypeScript's local type alone. Other @@ -187,6 +243,15 @@ pub(crate) fn class_field_store_needs_layout_note(ctx: &FnCtx<'_>, value: &Expr) /// in-place `+=` to rewrite underneath the stored slot — silent corruption /// with no crash to trace it back from. pub(crate) fn class_field_store_needs_string_addref(ctx: &FnCtx<'_>, value: &Expr) -> bool { + store_needs_string_addref(ctx, value) +} + +/// Receiver-independent form of [`class_field_store_needs_string_addref`]: the +/// demote is keyed on the stored value alone, so an array element store asks +/// exactly the same question. Used by the #7469 elided array push, which drops +/// the layout note but must keep this call whenever the pushed value could be +/// a heap string (a `new C()` with a constructor return override can be one). +pub(crate) fn store_needs_string_addref(ctx: &FnCtx<'_>, value: &Expr) -> bool { !expr_cannot_produce_heap_string(ctx, value) } @@ -215,6 +280,33 @@ fn expr_cannot_produce_heap_string(ctx: &FnCtx<'_>, expr: &Expr) -> bool { } } +/// #7469 — emit the at-allocation all-pointer element-layout declaration for +/// `id`, when this region's fact graph admits it and `init_expr` really is the +/// array literal the fact was proven against. +/// +/// Called from the `Stmt::Let` tail with the lowered (NaN-boxed) initializer, +/// which for an array literal is the fresh array's pointer. Re-testing +/// `Expr::Array` here rather than trusting the id alone keeps the emission +/// pinned to the single binding the collector proved: a fact that somehow named +/// an id bound by something else emits nothing, instead of declaring an element +/// layout for the wrong object. +pub(crate) fn emit_all_pointer_array_declaration( + ctx: &mut FnCtx<'_>, + id: u32, + init_expr: &Expr, + init_value: &str, +) { + if init_value.is_empty() + || !matches!(init_expr, Expr::Array(_)) + || !ctx.native_facts.declares_all_pointer_elements(id) + { + return; + } + let blk = ctx.block(); + let handle = super::unbox_to_i64(blk, init_value); + blk.call_void("js_array_declare_all_pointer_elements", &[(I64, &handle)]); +} + /// `lower_expr` variant that hands an expected-type hint down to the /// object-literal lowerer (so it can pick raw f64 slots when the /// destination has a typed shape). All other expression kinds ignore diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 01c4f5f287..af53065703 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -73,10 +73,11 @@ pub(crate) use channel::{ pub(crate) use helpers::{ array_store_needs_layout_note, array_store_needs_write_barrier, buffer_alias_metadata_suffix, class_field_store_needs_layout_note, class_field_store_needs_string_addref, - expr_has_numeric_pointer_free_array_layout, expr_produces_non_pointer_bits_by_construction, + emit_all_pointer_array_declaration, expr_has_numeric_pointer_free_array_layout, + expr_produces_fresh_heap_allocation, expr_produces_non_pointer_bits_by_construction, is_global_this_builtin_function_name, is_global_this_builtin_name, - lower_expr_with_expected_type, lower_js_args_array, proxy_build_args_array, unbox_str_handle, - unbox_to_i64, + lower_expr_with_expected_type, lower_js_args_array, proxy_build_args_array, + store_needs_string_addref, unbox_str_handle, unbox_to_i64, }; pub(crate) use i32_fast_path::{ can_lower_expr_as_i32, can_lower_expr_as_i32_in_current_region, diff --git a/crates/perry-codegen/src/expr/write_barrier.rs b/crates/perry-codegen/src/expr/write_barrier.rs index 8221c47296..e4d0fcdcb1 100644 --- a/crates/perry-codegen/src/expr/write_barrier.rs +++ b/crates/perry-codegen/src/expr/write_barrier.rs @@ -185,6 +185,15 @@ pub(crate) fn emit_jsvalue_slot_store_with_flags_on_block( ) } +/// As [`emit_jsvalue_slot_store_on_block`] with a caller-supplied `value_bits`, +/// and — like [`emit_jsvalue_slot_store_with_flags_on_block`] — with the +/// string-addref demote gated INDEPENDENTLY of the layout note. +/// +/// The array push (#7469) needs both halves at once: it can retire the layout +/// note on a header-proven all-pointer array while the pushed value is a +/// `new C()` whose constructor return override could still make it a +/// uniquely-owned heap string. +#[allow(clippy::too_many_arguments)] pub(crate) fn emit_jsvalue_slot_store_with_value_bits_on_block( blk: &mut LlBlock, slot_ptr: &str, @@ -192,6 +201,7 @@ pub(crate) fn emit_jsvalue_slot_store_with_value_bits_on_block( value_bits: &str, layout_parent_bits: &str, slot_index: &str, + string_addref_needed: bool, layout_note_needed: bool, barrier_parent_bits: &str, slot_addr: &str, @@ -203,7 +213,7 @@ pub(crate) fn emit_jsvalue_slot_store_with_value_bits_on_block( value_double, layout_parent_bits, slot_index, - layout_note_needed, + string_addref_needed, layout_note_needed, barrier_parent_bits, slot_addr, diff --git a/crates/perry-codegen/src/gc_call_effects.rs b/crates/perry-codegen/src/gc_call_effects.rs index 78fd678d4b..bc7acd7139 100644 --- a/crates/perry-codegen/src/gc_call_effects.rs +++ b/crates/perry-codegen/src/gc_call_effects.rs @@ -103,6 +103,10 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect { | "js_array_clear_numeric_layout" | "js_array_note_numeric_write" | "js_array_is_numeric_f64_layout" + // #7469: two header-bit writes plus the same `layout_forget_object` + // side-table remove `layout_init_pointer_free` already does on every + // allocation. No Perry allocation, no re-entry into generated code. + | "js_array_declare_all_pointer_elements" // TLS dynamic-call context only. | "js_implicit_this_set" | "js_new_target_get" diff --git a/crates/perry-codegen/src/root_reload.rs b/crates/perry-codegen/src/root_reload.rs index 6834e678df..da3150549f 100644 --- a/crates/perry-codegen/src/root_reload.rs +++ b/crates/perry-codegen/src/root_reload.rs @@ -146,6 +146,7 @@ const NON_COLLECTING: &[&str] = &[ "js_tdz_suppress_begin", "js_tdz_suppress_end", "js_array_note_numeric_write", + "js_array_declare_all_pointer_elements", "js_array_length", "js_object_mark_class", "js_class_object_pin_parent", diff --git a/crates/perry-codegen/src/runtime_decls/arrays.rs b/crates/perry-codegen/src/runtime_decls/arrays.rs index 52c3d38ce0..3d67d07f67 100644 --- a/crates/perry-codegen/src/runtime_decls/arrays.rs +++ b/crates/perry-codegen/src/runtime_decls/arrays.rs @@ -78,6 +78,11 @@ pub fn declare_phase_b_arrays(module: &mut LlModule) { // re-boxed live head (identity for everything else). module.declare_function("js_array_refresh_local_head", DOUBLE, &[DOUBLE]); module.declare_function("js_array_note_numeric_write", VOID, &[I64, I64]); + // #7469: at-allocation all-pointer element-layout declaration for a + // proven `[]` + push-loop array. Emitted once per allocation site; the + // per-push layout note it retires is re-armed by the header test in + // `expr/array_push.rs` whenever the declaration is not (or no longer) live. + module.declare_function("js_array_declare_all_pointer_elements", VOID, &[I64]); module.declare_function("js_array_length", I32, &[I64]); // Array.isArray runtime dispatch for values with indeterminate // static type (e.g. JSON.parse results, closure captures, any/ diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 3a6b4f9c25..2064821f4a 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -3,6 +3,10 @@ use super::*; use super::let_buffer_views::{math_min_length_buffer_ids, register_noalias_buffer_view}; +use super::let_stmt_facts::{ + buffer_local_alias_source, collect_scalar_class_data, native_i32_alias_source, + note_ptr_shape_scalar_replaced, pod_view_count_source, record_pod_rejection, +}; use super::unused_expr::lower_unused_expr; use crate::expr::{ box_i1_for_compat_shadow, emit_root_nanbox_store_on_block, @@ -1833,6 +1837,13 @@ pub(crate) fn lower_let( } } crate::expr::record_native_arena_owner_assignment(ctx, id, init_expr); + // #7469: `const out = []` whose every store this region proved a + // by-construction pointer push declares its element layout ONCE here, + // instead of maintaining a per-array pointer bitmap one + // `js_gc_note_slot_layout` call per push. + if !used_i32_init { + crate::expr::emit_all_pointer_array_declaration(ctx, id, init_expr, &v); + } // Buffer data-pointer slot for local (non-global) const buffers. The // HIR fact layer owns the source-shape decision; lowering only consumes // the stable local-id fact and emits the ptr slot used by @@ -1874,124 +1885,3 @@ pub(crate) fn lower_let( } Ok(()) } - -fn pod_view_count_source(ctx: &FnCtx<'_>, expr: &perry_hir::Expr) -> String { - match expr { - perry_hir::Expr::Integer(n) => format!("constant:{n}"), - perry_hir::Expr::Number(n) if n.is_finite() && n.fract() == 0.0 => { - format!("constant:{}", *n as i64) - } - perry_hir::Expr::LocalGet(id) => ctx - .local_id_to_name - .get(id) - .map(|name| format!("local:{name}")) - .unwrap_or_else(|| format!("local_id:{id}")), - _ => "dynamic".to_string(), - } -} - -fn native_i32_alias_source(expr: &perry_hir::Expr) -> Option { - match expr { - perry_hir::Expr::Binary { - op: perry_hir::BinaryOp::BitOr, - left, - right, - } if matches!(right.as_ref(), perry_hir::Expr::Integer(0)) => match left.as_ref() { - perry_hir::Expr::LocalGet(id) => Some(*id), - _ => native_i32_alias_source(left), - }, - perry_hir::Expr::LocalGet(id) => Some(*id), - _ => None, - } -} - -fn buffer_local_alias_source(expr: &perry_hir::Expr) -> Option { - match expr { - perry_hir::Expr::LocalGet(id) => Some(*id), - _ => None, - } -} - -/// Extract all field names (parent chain + own) and the constructor for -/// a class, cloning everything out of `ctx.classes` so the immutable -/// borrow is released before the caller mutates `ctx`. -/// -/// Returns `None` if the class is not found in `ctx.classes`. -pub(crate) fn collect_scalar_class_data( - ctx: &FnCtx<'_>, - class_name: &str, -) -> Option<(Vec, Option)> { - let class = ctx.classes.get(class_name)?; - let mut all_fields: Vec = Vec::new(); - let mut chain: Vec = Vec::new(); - let mut p = class.extends_name.clone(); - while let Some(pname) = p { - chain.push(pname.clone()); - if let Some(pc) = ctx.classes.get(pname.as_str()) { - p = pc.extends_name.clone(); - } else { - break; - } - } - chain.reverse(); - for pname in &chain { - if let Some(pc) = ctx.classes.get(pname.as_str()) { - for f in &pc.fields { - all_fields.push(f.name.clone()); - } - } - } - for f in &class.fields { - all_fields.push(f.name.clone()); - } - let ctor = class.constructor.clone(); - Some((all_fields, ctor)) -} - -fn record_pod_rejection(ctx: &mut FnCtx<'_>, id: u32, reason: String) { - let undef = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - let lowered = LoweredValue::js_value(undef); - ctx.record_lowered_value_with_access_mode( - "PodRecordRejected", - Some(id), - "pod_record_fallback_to_js_object", - &lowered, - None, - None, - Some(BufferAccessMode::DynamicFallback), - Some(MaterializationReason::PodUnsupported), - false, - false, - vec![format!("reason={}", reason)], - ); -} - -/// #7106 follow-up: record that a `Ptr`-proven local was scalar-replaced, -/// so its promotion can never be consumed. -/// -/// Report-only; the caller has already gated on `opt_report::enabled()`. The -/// fact is read through the context-free accessor on purpose — whether the -/// enclosing body would have ALLOWED consumption is a different mechanism with -/// a different rule name, and a value can lose to both. -fn note_ptr_shape_scalar_replaced(ctx: &crate::expr::FnCtx<'_>, id: u32, name: &str) { - let Some(fact) = ctx.native_facts.shape_proven_ptr_local(id) else { - return; - }; - let (reason, issue) = - crate::expr::ptr_shape_context_rule_text(crate::expr::PTR_SHAPE_SCALAR_REPLACED); - crate::opt_report::unconsumed(crate::opt_report::Unconsumed { - position: crate::opt_report::Position::Local, - name, - local_id: Some(id), - analysis: crate::opt_report::Analysis::PtrShape, - rep: "Ptr", - rule: crate::expr::PTR_SHAPE_SCALAR_REPLACED, - reason, - tier: crate::opt_report::Tier::CompilerLimitation, - issue: Some(issue), - detail: Some(format!( - "class {} scalar-replaced into per-field allocas; the allocation is gone", - fact.class_name - )), - }); -} diff --git a/crates/perry-codegen/src/stmt/let_stmt_facts.rs b/crates/perry-codegen/src/stmt/let_stmt_facts.rs new file mode 100644 index 0000000000..79e92aff6f --- /dev/null +++ b/crates/perry-codegen/src/stmt/let_stmt_facts.rs @@ -0,0 +1,129 @@ +//! Local-fact helpers split out of `let_stmt.rs` (POD/native-rep provenance +//! strings, alias sources, scalar-class collection and the rejection/shape +//! diagnostics). Extracted to keep `let_stmt.rs` under the 2000-line cap; +//! no behavior change. + +use super::*; + +use crate::native_value::{BufferAccessMode, PodLocal, SemanticKind}; + +pub(super) fn pod_view_count_source(ctx: &FnCtx<'_>, expr: &perry_hir::Expr) -> String { + match expr { + perry_hir::Expr::Integer(n) => format!("constant:{n}"), + perry_hir::Expr::Number(n) if n.is_finite() && n.fract() == 0.0 => { + format!("constant:{}", *n as i64) + } + perry_hir::Expr::LocalGet(id) => ctx + .local_id_to_name + .get(id) + .map(|name| format!("local:{name}")) + .unwrap_or_else(|| format!("local_id:{id}")), + _ => "dynamic".to_string(), + } +} + +pub(super) fn native_i32_alias_source(expr: &perry_hir::Expr) -> Option { + match expr { + perry_hir::Expr::Binary { + op: perry_hir::BinaryOp::BitOr, + left, + right, + } if matches!(right.as_ref(), perry_hir::Expr::Integer(0)) => match left.as_ref() { + perry_hir::Expr::LocalGet(id) => Some(*id), + _ => native_i32_alias_source(left), + }, + perry_hir::Expr::LocalGet(id) => Some(*id), + _ => None, + } +} + +pub(super) fn buffer_local_alias_source(expr: &perry_hir::Expr) -> Option { + match expr { + perry_hir::Expr::LocalGet(id) => Some(*id), + _ => None, + } +} + +/// Extract all field names (parent chain + own) and the constructor for +/// a class, cloning everything out of `ctx.classes` so the immutable +/// borrow is released before the caller mutates `ctx`. +/// +/// Returns `None` if the class is not found in `ctx.classes`. +pub(crate) fn collect_scalar_class_data( + ctx: &FnCtx<'_>, + class_name: &str, +) -> Option<(Vec, Option)> { + let class = ctx.classes.get(class_name)?; + let mut all_fields: Vec = Vec::new(); + let mut chain: Vec = Vec::new(); + let mut p = class.extends_name.clone(); + while let Some(pname) = p { + chain.push(pname.clone()); + if let Some(pc) = ctx.classes.get(pname.as_str()) { + p = pc.extends_name.clone(); + } else { + break; + } + } + chain.reverse(); + for pname in &chain { + if let Some(pc) = ctx.classes.get(pname.as_str()) { + for f in &pc.fields { + all_fields.push(f.name.clone()); + } + } + } + for f in &class.fields { + all_fields.push(f.name.clone()); + } + let ctor = class.constructor.clone(); + Some((all_fields, ctor)) +} + +pub(super) fn record_pod_rejection(ctx: &mut FnCtx<'_>, id: u32, reason: String) { + let undef = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); + let lowered = LoweredValue::js_value(undef); + ctx.record_lowered_value_with_access_mode( + "PodRecordRejected", + Some(id), + "pod_record_fallback_to_js_object", + &lowered, + None, + None, + Some(BufferAccessMode::DynamicFallback), + Some(MaterializationReason::PodUnsupported), + false, + false, + vec![format!("reason={}", reason)], + ); +} + +/// #7106 follow-up: record that a `Ptr`-proven local was scalar-replaced, +/// so its promotion can never be consumed. +/// +/// Report-only; the caller has already gated on `opt_report::enabled()`. The +/// fact is read through the context-free accessor on purpose — whether the +/// enclosing body would have ALLOWED consumption is a different mechanism with +/// a different rule name, and a value can lose to both. +pub(super) fn note_ptr_shape_scalar_replaced(ctx: &crate::expr::FnCtx<'_>, id: u32, name: &str) { + let Some(fact) = ctx.native_facts.shape_proven_ptr_local(id) else { + return; + }; + let (reason, issue) = + crate::expr::ptr_shape_context_rule_text(crate::expr::PTR_SHAPE_SCALAR_REPLACED); + crate::opt_report::unconsumed(crate::opt_report::Unconsumed { + position: crate::opt_report::Position::Local, + name, + local_id: Some(id), + analysis: crate::opt_report::Analysis::PtrShape, + rep: "Ptr", + rule: crate::expr::PTR_SHAPE_SCALAR_REPLACED, + reason, + tier: crate::opt_report::Tier::CompilerLimitation, + issue: Some(issue), + detail: Some(format!( + "class {} scalar-replaced into per-field allocas; the allocation is gone", + fact.class_name + )), + }); +} diff --git a/crates/perry-codegen/src/stmt/mod.rs b/crates/perry-codegen/src/stmt/mod.rs index 9b0aab9582..fe9f193369 100644 --- a/crates/perry-codegen/src/stmt/mod.rs +++ b/crates/perry-codegen/src/stmt/mod.rs @@ -17,6 +17,7 @@ mod counter_range; mod if_stmt; mod let_buffer_views; mod let_stmt; +mod let_stmt_facts; mod loops; mod masked_window_region; mod switch_stmt; diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 6016a8002d..5805869eac 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -1453,6 +1453,55 @@ pub extern "C" fn js_array_note_numeric_write(arr: *mut ArrayHeader, value_bits: } } +/// #7469 — declare ONCE, at allocation, that every element this array will +/// hold is a heap pointer, so the per-store pointer-mask bookkeeping +/// (`layout_note_slot`, and the `LAYOUT_SLOT_MASKS` entry it grows) is not +/// needed for the stores codegen has proven pointer-valued. +/// +/// Emitted by codegen at the `[]` literal that binds an array local whose every +/// store it can prove pointer-by-construction; see +/// `perry-codegen/src/collectors/all_pointer_arrays.rs` for the proof and +/// `expr/array_push.rs` for the header test that re-validates the declaration +/// at every elided store. +/// +/// Two things happen here, and both are load-bearing: +/// +/// 1. **The raw-f64 numeric layout is cleared.** `js_array_alloc` publishes +/// every fresh array as `RawF64` + `POINTER_FREE` — the numeric fast paths +/// read such an array's slots back as raw doubles, which for a pointer +/// payload is a reinterpretation of a heap address as a number. The +/// all-pointer declaration and the raw-f64 flag are mutually exclusive +/// claims about the same bytes, and the codegen-side header test refuses the +/// elided store unless BOTH raw-f64 bits are clear, so this clear is what +/// keeps that test satisfiable. +/// 2. **`GC_LAYOUT_SIDE_MASK | GC_LAYOUT_ALL_POINTERS` replaces +/// `GC_LAYOUT_POINTER_FREE`.** For the collector that swaps "skip the whole +/// payload" for "visit every slot in `0..length`" — the same set of slots +/// `GC_LAYOUT_UNKNOWN` visits, so the declaration is conservative in the +/// only direction that matters: a wrong element type costs a rejected slot +/// read (`mark_field_into_worklist` re-validates every word), never a +/// stranded live child. +/// +/// Refused on a non-empty array: the claim covers `0..length`, and only on an +/// empty array is it vacuously true of what is already stored. A refusal is +/// silent and safe — the header keeps whatever layout it had, and the codegen +/// header test then declines the elided store and routes the push through +/// `js_array_push_f64`, which notes every slot as it always did. +#[no_mangle] +pub extern "C" fn js_array_declare_all_pointer_elements(arr: *mut ArrayHeader) { + let arr = clean_arr_ptr_mut(arr); + if arr.is_null() { + return; + } + unsafe { + if (*arr).length != 0 { + return; + } + clear_array_numeric_layout(arr); + crate::gc::layout_init_all_pointer_slots(arr as *mut u8); + } +} + #[no_mangle] pub extern "C" fn js_array_is_numeric_f64_layout(arr: *const ArrayHeader) -> i32 { let arr = clean_arr_ptr(arr); @@ -1513,6 +1562,10 @@ static KEEP_JS_ARRAY_NOTE_NUMERIC_WRITE: extern "C" fn(*mut ArrayHeader, u64) = js_array_note_numeric_write; #[cfg(feature = "keepalive-anchors")] #[used] +static KEEP_JS_ARRAY_DECLARE_ALL_POINTER_ELEMENTS: extern "C" fn(*mut ArrayHeader) = + js_array_declare_all_pointer_elements; +#[cfg(feature = "keepalive-anchors")] +#[used] static KEEP_JS_ARRAY_IS_NUMERIC_F64_LAYOUT: extern "C" fn(*const ArrayHeader) -> i32 = js_array_is_numeric_f64_layout; #[cfg(feature = "keepalive-anchors")] diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 4411417e62..a2d0d0b887 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -81,7 +81,8 @@ pub(crate) use self::header::{ rebuild_array_numeric_raw_f64_dense_window, rebuild_array_numeric_raw_f64_dense_window_i32, }; pub use self::header::{ - js_array_clear_numeric_layout, js_array_is_numeric_f64_layout, js_array_mark_arguments_object, + js_array_clear_numeric_layout, js_array_declare_all_pointer_elements, + js_array_is_numeric_f64_layout, js_array_mark_arguments_object, js_array_mark_numeric_f64_layout, js_array_note_numeric_write, js_tagged_template_get_or_init, js_tagged_template_register_raw, js_template_raw, scan_template_raw_roots, scan_template_raw_roots_mut, ArrayHeader, diff --git a/crates/perry-runtime/src/gc/layout.rs b/crates/perry-runtime/src/gc/layout.rs index 23447f5293..63562c54f9 100644 --- a/crates/perry-runtime/src/gc/layout.rs +++ b/crates/perry-runtime/src/gc/layout.rs @@ -685,6 +685,36 @@ pub(super) unsafe fn layout_set_typed_unknown(header: *mut GcHeader, user_ptr: u crate::typed_feedback::invalidate_representation_change(user_ptr); } +/// True when `slot_index` is the **append position** of an array whose live +/// prefix is currently declared all-pointer. +/// +/// Every array append protocol in the tree — `js_array_push_f64`, +/// `js_array_push_f64_grow`, and the codegen-inlined push — writes the element +/// slot and notes it BEFORE bumping `length`, so an append records +/// `slot_index == length`. Writing a pointer there keeps +/// "every slot in `0..length + 1` holds a pointer" exactly true, which is the +/// whole content of [`GC_LAYOUT_ALL_POINTERS`]; a replace (`slot < length`) or +/// a hole-creating jump (`slot > length`) does not, and downgrades. +/// +/// Restricted to `GC_TYPE_ARRAY` on purpose: object fields and closure +/// captures have a FIXED live prefix (`field_count` / `capture_count`), so +/// they have no append position at all and nothing to preserve. `length < +/// capacity` keeps the claim inside the allocation. +#[inline] +unsafe fn layout_all_pointer_array_append( + header: *const GcHeader, + parent_user: usize, + slot_index: usize, +) -> bool { + if (*header).obj_type != GC_TYPE_ARRAY { + return false; + } + let arr = parent_user as *const crate::array::ArrayHeader; + let length = (*arr).length as usize; + let capacity = (*arr).capacity as usize; + slot_index == length && length < capacity +} + pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits: u64) { if slot_index > 16_000_000 { return; @@ -767,8 +797,19 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits // this generic write path; any later ordinary array write may create // holes or replace an element, so conservatively fall back to the // generic scan path regardless of the stored value. + // + // ONE exception (#7469): an APPEND of a pointer at the array's current + // append position keeps the declaration exact rather than violating it, + // so it is preserved instead of downgraded. Without this a codegen + // `[]` + push-loop array (declared all-pointer at allocation) would be + // demoted by the very first growth — `js_array_push_f64_grow` routes + // through this function — and every later push would fall off the + // declared fast path for the rest of the array's life. let all_pointer_layout = (*header)._reserved & GC_LAYOUT_ALL_POINTERS != 0; if all_pointer_layout { + if pointer && layout_all_pointer_array_append(header, parent_user, slot_index) { + return; + } layout_mark_unknown(parent_user as *mut u8); return; } diff --git a/crates/perry-runtime/src/gc/tests/copying.rs b/crates/perry-runtime/src/gc/tests/copying.rs index 626c8687d4..acce664f66 100644 --- a/crates/perry-runtime/src/gc/tests/copying.rs +++ b/crates/perry-runtime/src/gc/tests/copying.rs @@ -1,4 +1,5 @@ mod adaptive_tenuring; +mod all_pointer_elements_7469; mod pointer_publish_7154; mod promise_side_tables; mod survival_and_malloc; diff --git a/crates/perry-runtime/src/gc/tests/copying/all_pointer_elements_7469.rs b/crates/perry-runtime/src/gc/tests/copying/all_pointer_elements_7469.rs new file mode 100644 index 0000000000..1591583c3d --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/copying/all_pointer_elements_7469.rs @@ -0,0 +1,363 @@ +//! #7469 — an array whose element layout codegen DECLARED at its allocation +//! site, then filled with stores that record nothing. +//! +//! Invariant under test: +//! +//! > When codegen elides the per-push `js_gc_note_slot_layout`, the +//! > `js_array_declare_all_pointer_elements` it emitted at the allocation site +//! > is the ONLY thing telling the collector those element slots exist. It must +//! > therefore hold across the whole fill — including growth through +//! > `js_array_push_f64` — and the elided store must be refused whenever it +//! > does not. +//! +//! Every heap object is published `GC_LAYOUT_POINTER_FREE`, and +//! `heap_payload_slot_selection` short-circuits on that state and skips the +//! WHOLE payload without consulting any mask. So an elided pointer store into +//! an array that is *not* declared is not conservative — it is invisible: the +//! evacuating minor neither keeps the element alive nor rewrites the slot when +//! it moves, and the next read returns a pointer into reclaimed nursery memory. +//! +//! [`undeclared_array_enumerates_no_child_slots_which_is_what_the_declaration_buys`] +//! is the sabotage arm, made permanent. It runs the *identical* fill sequence +//! with the one declaration call removed and asserts the collector enumerates +//! ZERO element slots — so a green run of the positive test below means the +//! declaration was load-bearing, not that nothing was tried. It interrogates +//! the child-slot enumerator rather than collecting, so it is deterministic +//! regardless of GC timing and never leaves a stale pointer behind. + +use super::*; + +use crate::array::ArrayHeader; + +/// The push sequence codegen emits on the ELIDED path, byte for byte: a bare +/// `store` at `elements[length]` and then `length += 1`. No +/// `js_gc_note_slot_layout`, no `js_array_note_numeric_write`, no +/// `js_array_push_f64`. +/// +/// Callers must have room (`length < capacity`); the emitted IR branches to the +/// runtime grow path otherwise, which is a different sequence with its own +/// notes. +unsafe fn elided_inline_push(arr: *mut ArrayHeader, value_bits: u64) { + let length = (*arr).length as usize; + assert!( + length < (*arr).capacity as usize, + "the elided inline push models the in-capacity arm only" + ); + let elements = (arr as *mut u8).add(std::mem::size_of::()) as *mut u64; + std::ptr::write(elements.add(length), value_bits); + (*arr).length = length as u32 + 1; +} + +unsafe fn element_bits(arr: *mut ArrayHeader, index: usize) -> u64 { + let elements = (arr as *mut u8).add(std::mem::size_of::()) as *const u64; + *elements.add(index) +} + +fn fresh_string(bytes: &[u8]) -> usize { + crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32) as usize +} + +const ELEMENTS: [&[u8]; 4] = [b"elem_zero", b"elem_one", b"elem_two", b"elem_three"]; + +/// The exact predicate `expr/array_push.rs` emits before taking the elided +/// store: `(_reserved & 0xF487) == 0xA000`. Duplicated numerically on purpose — +/// [`codegen_admission_test_matches_the_runtime_bit_names`] pins the two +/// spellings together, so a bit that moves in `gc/types.rs` fails here instead +/// of silently un-gating generated code in another crate. +const CODEGEN_ADMISSION_MASK: u16 = 62599; +const CODEGEN_ADMISSION_EXPECT: u16 = 40960; + +unsafe fn codegen_would_take_the_elided_store(arr: *mut ArrayHeader) -> bool { + let header = header_from_user_ptr(arr as *const u8); + (*header)._reserved & CODEGEN_ADMISSION_MASK == CODEGEN_ADMISSION_EXPECT +} + +#[test] +fn codegen_admission_test_matches_the_runtime_bit_names() { + assert_eq!( + CODEGEN_ADMISSION_MASK, + OBJ_FLAG_FROZEN + | OBJ_FLAG_SEALED + | OBJ_FLAG_NO_EXTEND + | OBJ_FLAG_ARRAY_DESCRIPTORS + | GC_ARRAY_RAW_F64_LAYOUT + | GC_ARRAY_RAW_F64_HOLES + | GC_LAYOUT_ALL_POINTERS + | GC_LAYOUT_STATE_MASK, + "the mask emitted in perry-codegen/src/expr/array_push.rs must be \ + exactly these bits" + ); + assert_eq!( + CODEGEN_ADMISSION_EXPECT, + GC_LAYOUT_SIDE_MASK | GC_LAYOUT_ALL_POINTERS, + "the expected value emitted in perry-codegen/src/expr/array_push.rs \ + must be exactly the all-pointer declaration" + ); +} + +#[test] +fn declaring_at_allocation_clears_the_raw_f64_layout_the_allocator_published() { + let _guard = CopyingNurseryTestGuard::new(1); + let arr = crate::array::js_array_alloc(4); + unsafe { + // `js_array_alloc` publishes RawF64 + POINTER_FREE, so the codegen + // admission test refuses the elided store on a bare allocation. + assert!(!codegen_would_take_the_elided_store(arr)); + crate::array::js_array_declare_all_pointer_elements(arr); + assert!( + codegen_would_take_the_elided_store(arr), + "the declaration must clear both raw-f64 bits AND install \ + SIDE_MASK|ALL_POINTERS, or the emitted store gate can never pass" + ); + } +} + +/// The runtime CAN revoke the declaration behind codegen's back, and this is +/// the cheapest live demonstration of it: `js_array_is_numeric_f64_layout` on a +/// still-EMPTY declared array verifies vacuously and re-publishes the array as +/// RawF64 + `POINTER_FREE`. +/// +/// That is the whole reason the elided store is fronted by a test of the LIVE +/// header rather than by trusting the allocation-site declaration statically. +/// A static elision would, from here on, write pointers into a `POINTER_FREE` +/// array whose payload the collector skips entirely — a use-after-free factory +/// reachable from ordinary TypeScript (`arr.length`-shaped numeric fast paths +/// probe this on arrays codegen also pushes into). The header test instead +/// declines, the push takes `js_array_push_f64`, and the slot is recorded. +/// +/// `rebuild_array_layout` (sort/splice) is a second such path; it installs a +/// PRECISE mask, which is likewise not a state the elided store may run in. +#[test] +fn a_numeric_layout_probe_revokes_the_declaration_and_the_header_test_catches_it() { + let _guard = CopyingNurseryTestGuard::new(1); + let arr = crate::array::js_array_alloc(4); + crate::array::js_array_declare_all_pointer_elements(arr); + unsafe { + assert!(codegen_would_take_the_elided_store(arr)); + } + + assert_eq!( + crate::array::js_array_is_numeric_f64_layout(arr as *const _), + 1, + "an empty array verifies as numeric vacuously — this is the revocation" + ); + unsafe { + assert!( + !codegen_would_take_the_elided_store(arr), + "codegen must refuse the elided store once the declaration is gone" + ); + } + + // The refused push routes through the runtime, which records the slot. + let child = fresh_string(b"after_revocation"); + let arr = crate::array::js_array_push_f64(arr, f64::from_bits(string_bits(child))); + unsafe { + assert!( + test_heap_child_slot_count(arr as *mut u8) >= 1, + "the runtime push must have recorded the pointer the elided store \ + was refused for" + ); + } +} + +#[test] +fn declaring_a_non_empty_array_is_refused() { + let _guard = CopyingNurseryTestGuard::new(1); + let arr = crate::array::js_array_alloc(4); + crate::array::js_array_push_f64(arr, 1.0); + unsafe { + crate::array::js_array_declare_all_pointer_elements(arr); + assert!( + !codegen_would_take_the_elided_store(arr), + "the all-pointer claim covers 0..length; on a non-empty array it \ + is not vacuously true of what is already stored, so it must be \ + refused rather than asserted over existing elements" + ); + } +} + +/// The witness: four heap children published by stores that recorded nothing, +/// kept alive and re-pointed by an evacuating minor purely because of the +/// allocation-site declaration. +#[test] +fn declared_elements_survive_relocation_by_a_copying_minor() { + let _guard = CopyingNurseryTestGuard::new(1); + let arr = crate::array::js_array_alloc(ELEMENTS.len() as u32); + crate::array::js_array_declare_all_pointer_elements(arr); + + let mut before = Vec::new(); + unsafe { + for bytes in ELEMENTS { + let child = fresh_string(bytes); + before.push(child); + elided_inline_push(arr, string_bits(child)); + } + assert!( + codegen_would_take_the_elided_store(arr), + "the declaration must still be live after the fill; otherwise the \ + pushes modelled above would not have been emitted elided" + ); + assert_eq!( + test_heap_child_slot_count(arr as *mut u8), + ELEMENTS.len(), + "the collector must enumerate every filled element slot" + ); + } + + js_shadow_slot_set(0, ptr_bits(arr as usize)); + let trace = collect_minor_trace(GcTriggerKind::Direct); + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert!( + trace.copying_nursery.copied_objects >= ELEMENTS.len() + 1, + "the cycle must actually have MOVED the array and all {} elements, \ + or this test proves nothing about relocation (copied_objects = {})", + ELEMENTS.len(), + trace.copying_nursery.copied_objects + ); + + let arr_after = (js_shadow_slot_get(0) & POINTER_MASK) as usize as *mut ArrayHeader; + assert_ne!( + arr_after as usize, arr as usize, + "the array itself must move" + ); + unsafe { + assert_eq!((*arr_after).length as usize, ELEMENTS.len()); + for (index, bytes) in ELEMENTS.iter().enumerate() { + let slot = element_bits(arr_after, index); + let moved = (slot & POINTER_MASK) as usize; + assert_ne!( + moved, before[index], + "element {index} must have been relocated and its slot rewritten" + ); + assert!( + crate::arena::pointer_in_nursery(moved), + "element {index} must live in the to-space, not a stale address" + ); + assert_string_bytes(moved as *const crate::StringHeader, bytes); + } + } + js_shadow_slot_set(0, crate::value::TAG_UNDEFINED); +} + +/// SABOTAGE ARM. The identical fill with the one declaration call removed: the +/// array stays `POINTER_FREE`, `heap_payload_slot_selection` skips the whole +/// payload, and the collector enumerates NOTHING — which is precisely the +/// use-after-free the declaration prevents, and precisely why codegen must +/// re-test the header before eliding any store. +/// +/// Asserted on the enumerator rather than through a collection, so nothing here +/// creates a dangling pointer: the array is never rooted and its length is +/// reset before the guard drops. +#[test] +fn undeclared_array_enumerates_no_child_slots_which_is_what_the_declaration_buys() { + let _guard = CopyingNurseryTestGuard::new(1); + let declared = crate::array::js_array_alloc(ELEMENTS.len() as u32); + crate::array::js_array_declare_all_pointer_elements(declared); + let undeclared = crate::array::js_array_alloc(ELEMENTS.len() as u32); + + unsafe { + for bytes in ELEMENTS { + let child = fresh_string(bytes); + elided_inline_push(declared, string_bits(child)); + elided_inline_push(undeclared, string_bits(child)); + } + assert_eq!( + test_heap_child_slot_count(declared as *mut u8), + ELEMENTS.len() + ); + assert_eq!( + test_heap_child_slot_count(undeclared as *mut u8), + 0, + "an undeclared array skips its whole payload — the sabotage this \ + file exists to prove the positive test can detect" + ); + assert!( + !codegen_would_take_the_elided_store(undeclared), + "codegen's emitted header test must REFUSE the elided store on \ + exactly this array, which is why the shape above is unreachable \ + from generated code" + ); + // Leave no stale-pointer landmine for a later cycle on this thread. + (*undeclared).length = 0; + (*declared).length = 0; + } +} + +/// The declaration must survive the array outgrowing its initial capacity. +/// +/// Growth routes through `js_array_push_f64` → `note_array_slot` → +/// `layout_note_slot`, whose all-pointer arm used to downgrade unconditionally. +/// That would demote the array on its FIRST growth and drop every later push +/// off the elided path for the rest of its life. +#[test] +fn a_pointer_append_through_the_runtime_preserves_the_declaration_across_growth() { + let _guard = CopyingNurseryTestGuard::new(1); + let arr = crate::array::js_array_alloc(0); + crate::array::js_array_declare_all_pointer_elements(arr); + let initial_capacity = unsafe { (*arr).capacity }; + + let mut arr = arr; + for i in 0..(initial_capacity as usize * 4 + 8) { + let child = fresh_string(format!("grow_{i}").as_bytes()); + arr = crate::array::js_array_push_f64(arr, f64::from_bits(string_bits(child))); + unsafe { + assert!( + codegen_would_take_the_elided_store(arr), + "the declaration must survive push #{i} (capacity now {})", + (*arr).capacity + ); + } + } + unsafe { + assert!( + (*arr).capacity > initial_capacity, + "the loop must actually have grown the array, or it proves nothing" + ); + } +} + +/// A store that is NOT an append cannot keep the "every live slot is a +/// pointer" claim exact, so it downgrades — to `GC_LAYOUT_UNKNOWN`, which +/// scans every slot, never to `POINTER_FREE`, which scans none. +#[test] +fn a_replace_or_a_numeric_append_downgrades_to_a_conservative_scan() { + let _guard = CopyingNurseryTestGuard::new(1); + + for downgrade_by_replace in [true, false] { + let arr = crate::array::js_array_alloc(4); + crate::array::js_array_declare_all_pointer_elements(arr); + let first = fresh_string(b"kept"); + let arr = crate::array::js_array_push_f64(arr, f64::from_bits(string_bits(first))); + unsafe { + assert!(codegen_would_take_the_elided_store(arr)); + } + + if downgrade_by_replace { + // Overwrite slot 0 (slot_index < length): a replace, not an append. + let other = fresh_string(b"replacement"); + crate::array::js_array_set_f64(arr, 0, f64::from_bits(string_bits(other))); + } else { + // Append a NUMBER: the slot it lands in is no longer a pointer. + crate::array::js_array_push_f64(arr, 42.0); + } + + unsafe { + let header = header_from_user_ptr(arr as *const u8); + assert!( + !codegen_would_take_the_elided_store(arr), + "the declaration must be revoked (downgrade_by_replace = {downgrade_by_replace})" + ); + assert_ne!( + (*header)._reserved & GC_LAYOUT_STATE_MASK, + GC_LAYOUT_POINTER_FREE, + "downgrading an all-pointer array to POINTER_FREE would strand \ + the element already stored in slot 0" + ); + assert!( + test_heap_child_slot_count(arr as *mut u8) >= 1, + "the still-live pointer in slot 0 must remain enumerable after \ + the downgrade" + ); + } + } +}