diff --git a/CLAUDE.md b/CLAUDE.md index 6f0f55235b..6aff1f29ce 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.1297 +**Current Version:** 0.5.1298 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 3999f1027c..572315ccd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1297" +version = "0.5.1298" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1297" +version = "0.5.1298" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1297" +version = "0.5.1298" [[package]] name = "perry-ui-tvos" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1297" +version = "0.5.1298" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index ed5acc1f5c..605d65614a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1297" +version = "0.5.1298" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7527-spread-symbol-iterator-rooting.md b/changelog.d/7527-spread-symbol-iterator-rooting.md new file mode 100644 index 0000000000..da13af1362 --- /dev/null +++ b/changelog.d/7527-spread-symbol-iterator-rooting.md @@ -0,0 +1,63 @@ +### Fixed + +**`[...obj.arr]`'s inlined lowering dereferenced retired from-space in the +`[Symbol.iterator]` prototype walk (#7498).** The spread has two lowerings. Out +of line it calls `js_iterator_to_array` — the drain rooted by #7495. Inlined, it +routes through `array_from_spread_value`, which resolves `[Symbol.iterator]` +through the whole property-lookup tower first, and three frames on that walk held +a GC-managed value somewhere the collector cannot see. Latent for every user on +both link modes: evacuation copies rather than zeroes, so the stale read returned +the correct old bytes and the program printed the right answer — until allocation +timing recycled those bytes, at which point it becomes the family's usual +`TypeError: value is not a function` somewhere else entirely. + +* **`symbol::get::req_handle_symbol_fallback`** read the receiver into a bare + `usize`, interned a `"_req"` key (an allocation), then read a field off the + pre-move address. `js_object_get_field_by_name` followed that copy's stale + `keys_array` into a retired 40-byte `GC_TYPE_ARRAY`. This helper runs on every + heap-object symbol read whose own-symbol lookup missed, so the window is + unconditional — the reproducer faults 5/5, not intermittently. +* **A `&str` / `&[u8]` borrowed out of the key's `StringHeader`.** + `get_field_by_name_object_tail` slices the property name straight out of the + key payload and hands it to `array_prototype_property_value`, which allocates + three times before reading it — including inside `js_string_from_bytes`, which + reads its *source* bytes after its own `string_storage_alloc`. **No + `RuntimeHandleScope` can fix that shape**: rooting the key rewrites the slot, + not a `&str` that already names the pre-move address, and a borrow is not a + slot. The new `HeapKeyBytes` copies the bytes off the heap once, before the + arm's first allocation — a 64-byte stack buffer in the common case, spilling + only for a longer key so the guarantee is total rather than typical. +* **`array_from_spread_value`'s receiver**, carried through a dozen + classification probes and the entire symbol walk and then used to rebind + `this` for the `[Symbol.iterator]()` factory. Rooted before the function's + first allocation, with the argument shadowed by a reader so the + pre-collection address is not nameable afterwards. + +Also rooted, same shape and same measured path: +`default_object_prototype_property_value`'s key and receiver (plus the displaced +`this` and accessor-receiver override its own doc comment flagged as a residual), +and the `fetch_subclass_handle_id` / `temporal_subclass_cell` subclass-marker +probes, each of which allocates a key string between reading its receiver and +using it. Every handle is NaN-boxed, so `scripts/raw_handle_debt.py` stays at +999. + +**Witness:** `test-files/test_gap_gc_spread_symbol_iterator_rooting.ts`, +registered in `test-parity/gc_repsel_corpus.txt`. It is +`test_gap_gc_iterator_drain_rooting`'s shrunk twin — small enough that `clone` +inlines, so the spread takes the other lowering, and its size is load-bearing. +Measured on macOS/arm64: `PERRY_GC_PROTECT_FROMSPACE=1 +PERRY_GC_PROTECT_FROMSPACE_DEPTH=200` FAULTs 5/5 before and is clean 5/5 after, +on **both** the default (auto-optimize) and `PERRY_NO_AUTO_OPTIMIZE=1` links, +with the auto-optimize rows A/B'd against a runtime archive rebuilt from source +on each side. The clean verdict is non-vacuous: the same run reports four +`[gc-fromspace-protect] retired_set=#N` page-sets and `[gc-copy-minor] ran +copied_objects=11794` on the first minor. + +**The protected run is not clean everywhere, and the remainder is filed rather +than absorbed.** `test_gap_gc_iterator_drain_rooting` — the out-of-line sibling +— still faults, in `js_native_call_method` (#7528), which reads its *rooted* +receiver into a local reused across a dozen allocating probes before +`is_closure_ptr` dereferences it. Patching the one faulting line was tried and reverted: the +fault moved 800 bytes further into the same function, which is the signal that +the whole receiver needs re-reading rather than one arm of it. That file's +corpus note is updated to say so — its own text asked for exactly this check. diff --git a/crates/perry-runtime/src/array/iterator.rs b/crates/perry-runtime/src/array/iterator.rs index 127be1280b..a719fd451a 100644 --- a/crates/perry-runtime/src/array/iterator.rs +++ b/crates/perry-runtime/src/array/iterator.rs @@ -1,5 +1,6 @@ //! Iterator-protocol → array converter. use super::*; +use crate::value::nanbox_string_key; /// Materialize an arbitrary iterable into a plain Array, used by the /// `for...of` desugar when the receiver's static type can NOT be proven @@ -709,12 +710,39 @@ fn object_like_iterator_result(value: f64) -> bool { pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { use crate::value::{js_nanbox_get_pointer, js_nanbox_pointer, JSValue, POINTER_MASK}; - let jsv = JSValue::from_bits(value.to_bits()); + // #7498: the spread receiver is a GC-managed value, and this function + // carries it across a dozen classification probes AND the whole + // `[Symbol.iterator]` prototype walk. That walk allocates a key string on + // every hop (`array_prototype_property_value` → + // `default_object_prototype_property_value` → `js_object_get_field_by_name` + // is where `PERRY_GC_PROTECT_FROMSPACE=1` faults), so the copying minor can + // move the receiver while it exists only in this bare `value` argument — + // which the collector cannot see and therefore never rewrites. + // + // The uses AFTER the walk are the consequential ones: + // * `clone_closure_rebind_this(method, value)` would bind a from-space + // `this`, so a user `[Symbol.iterator]()` factory reads a dead + // receiver and yields nothing; + // * `js_implicit_this_set(value)` publishes the same dead receiver to + // the canonical bound-method path; + // * the `js_array_is_array(value)` fallback reads a recycled GcHeader + // and reports a live array as "not iterable". + // + // Root it FIRST — before anything here allocates — and SHADOW the argument + // with readers, so the pre-collection address is not nameable below. Every + // handle is NaN-boxed, so a read-back is `get_nanbox_f64` and this module + // stays out of `scripts/raw_handle_debt.py`'s ledger. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_h = scope.root_nanbox_f64(value); + let value = || value_h.get_nanbox_f64(); + let raw_ptr = || js_nanbox_get_pointer(value_h.get_nanbox_f64()) as usize; + + let jsv = JSValue::from_bits(value().to_bits()); if jsv.is_null() || jsv.is_undefined() { - throw_not_iterable(value); + throw_not_iterable(value()); } if jsv.is_any_string() { - let str_ptr = crate::value::js_get_string_pointer_unified(value); + let str_ptr = crate::value::js_get_string_pointer_unified(value()); let str_bits = crate::value::STRING_TAG | (str_ptr as u64 & POINTER_MASK); return crate::string::js_string_to_char_array(str_bits as i64) as *mut ArrayHeader; } @@ -723,29 +751,28 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // INT32-tagged ClassRef. Drive its (possibly inherited) `[Symbol.iterator]`; // with none it is not iterable, like node. Must run before the raw-pointer // reads below, which would misread the class id as a heap address. - if crate::object::class_ref_id(value).is_some() { - if crate::symbol::class_ref_resolves_iterator(value) { - let iter = crate::symbol::js_get_iterator(value); + if crate::object::class_ref_id(value()).is_some() { + if crate::symbol::class_ref_resolves_iterator(value()) { + let iter = crate::symbol::js_get_iterator(value()); return js_iterator_to_array(iter); } - throw_not_iterable(value); + throw_not_iterable(value()); } - let raw_ptr = js_nanbox_get_pointer(value) as usize; - if raw_ptr == 0 { - throw_not_iterable(value); + if raw_ptr() == 0 { + throw_not_iterable(value()); } - if let Some(entries) = entries_array_for_small_handle_id(raw_ptr as i64) { + if let Some(entries) = entries_array_for_small_handle_id(raw_ptr() as i64) { return entries; } - if crate::buffer::is_registered_buffer(raw_ptr) { - return crate::buffer::buffer_to_array(raw_ptr as *const crate::buffer::BufferHeader); + if crate::buffer::is_registered_buffer(raw_ptr()) { + return crate::buffer::buffer_to_array(raw_ptr() as *const crate::buffer::BufferHeader); } - if crate::set::is_registered_set(raw_ptr) { - return crate::set::js_set_to_array(raw_ptr as *const crate::set::SetHeader); + if crate::set::is_registered_set(raw_ptr()) { + return crate::set::js_set_to_array(raw_ptr() as *const crate::set::SetHeader); } - if crate::map::is_registered_map(raw_ptr) { - return crate::map::js_map_entries(raw_ptr as *const crate::map::MapHeader); + if crate::map::is_registered_map(raw_ptr()) { + return crate::map::js_map_entries(raw_ptr() as *const crate::map::MapHeader); } // `class X extends Map | Set` instance — spread (`[...container]`, // `Array.from(container)`, `fn(...container)`) over the hidden backing @@ -754,7 +781,7 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // Map/Set value, so a subclass instance (a plain object with a backing // field) falls through to here. Skipped when the subclass overrides // `[Symbol.iterator]` so the override drives the spread. - match crate::object::map_set_subclass::subclass_backing_for_default_iteration(value) { + match crate::object::map_set_subclass::subclass_backing_for_default_iteration(value()) { Some(crate::object::map_set_subclass::CollectionBacking::Map(m)) => { return crate::map::js_map_entries(m as *const crate::map::MapHeader); } @@ -770,27 +797,33 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // Matches the Map/Set-subclass branch above. Skipped when the subclass // declared its own `[Symbol.iterator]`, so the override drives the spread // via the generic symbol lookup below. - if crate::array::is_array_subclass_instance(value) - && !crate::array::array_subclass_has_iterator_override(value) + if crate::array::is_array_subclass_instance(value()) + && !crate::array::array_subclass_has_iterator_override(value()) { - let snap = crate::array::array_subclass_dense_snapshot(value); + let snap = crate::array::array_subclass_dense_snapshot(value()); return crate::value::js_nanbox_get_pointer(snap) as *mut ArrayHeader; } - if crate::typedarray::lookup_typed_array_kind(raw_ptr).is_some() { + if crate::typedarray::lookup_typed_array_kind(raw_ptr()).is_some() { return crate::typedarray::typed_array_to_array( - raw_ptr as *const crate::typedarray::TypedArrayHeader, + raw_ptr() as *const crate::typedarray::TypedArrayHeader ); } - if raw_ptr >= crate::gc::GC_HEADER_SIZE + 0x1000 { + if raw_ptr() >= crate::gc::GC_HEADER_SIZE + 0x1000 { let obj_type = unsafe { - let hdr = - (raw_ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; + let hdr = (raw_ptr() as *const u8).sub(crate::gc::GC_HEADER_SIZE) + as *const crate::gc::GcHeader; (*hdr).obj_type }; if obj_type == crate::gc::GC_TYPE_OBJECT { - let obj = raw_ptr as *mut crate::object::ObjectHeader; - if crate::url::try_read_as_search_params(obj).is_some() { - let boxed = crate::url::js_url_search_params_entries_arr(obj); + // `try_read_as_search_params` interns its own key string, so the + // receiver address is re-read from the root for the entries call + // rather than reused from before that probe. + if crate::url::try_read_as_search_params(raw_ptr() as *mut crate::object::ObjectHeader) + .is_some() + { + let boxed = crate::url::js_url_search_params_entries_arr( + raw_ptr() as *mut crate::object::ObjectHeader + ); let ptr = crate::value::js_nanbox_get_pointer(boxed) as *mut ArrayHeader; if !ptr.is_null() { return ptr; @@ -805,14 +838,14 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // inherited thunk and call it WITHOUT binding `this` — which yields a bad // result. Short-circuit here to keep `Array.from(arr.values())` / `[...it]` // working. - if is_builtin_iterator_class_id(raw_ptr) { - return js_iterator_to_array(value); + if is_builtin_iterator_class_id(raw_ptr()) { + return js_iterator_to_array(value()); } // Arguments objects spread like arrays (spec: // `arguments[Symbol.iterator] === Array.prototype.values`). - if crate::object::is_arguments_object(raw_ptr as *const crate::object::ObjectHeader) { + if crate::object::is_arguments_object(raw_ptr() as *const crate::object::ObjectHeader) { if let Some(arr) = unsafe { - crate::object::arguments_object_to_array(raw_ptr as *const crate::object::ObjectHeader) + crate::object::arguments_object_to_array(raw_ptr() as *const crate::object::ObjectHeader) } { return arr; } @@ -820,16 +853,26 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { let iter_wk = crate::symbol::well_known_symbol("iterator"); if !iter_wk.is_null() { - let sym_f64 = f64::from_bits(crate::value::JSValue::pointer(iter_wk as *const u8).bits()); - let method = unsafe { crate::symbol::js_object_get_symbol_property(value, sym_f64) }; - if method.to_bits() != crate::value::TAG_UNDEFINED { - if !is_callable_value(method) { + // The well-known symbol is itself a heap object, and the lookup it is + // about to key can collect, so it gets a root of its own rather than a + // raw `iter_wk` carried across the call. + let sym_h = scope.root_nanbox_f64(f64::from_bits( + crate::value::JSValue::pointer(iter_wk as *const u8).bits(), + )); + let method = unsafe { + crate::symbol::js_object_get_symbol_property(value(), sym_h.get_nanbox_f64()) + }; + // The resolved method is a fresh closure value that must survive + // `clone_closure_rebind_this` (an allocation) and the factory call. + let method_h = scope.root_nanbox_f64(method); + if method_h.get_nanbox_f64().to_bits() != crate::value::TAG_UNDEFINED { + if !is_callable_value(method_h.get_nanbox_f64()) { throw_iterator_method_not_callable(); } - let rebound = crate::closure::clone_closure_rebind_this(method.to_bits(), value); - let call_target = f64::from_bits(rebound); - let fn_ptr = js_nanbox_get_pointer(call_target) as *const crate::closure::ClosureHeader; - if fn_ptr.is_null() { + let rebound = + crate::closure::clone_closure_rebind_this(method_h.get_nanbox_u64(), value()); + let rebound_h = scope.root_nanbox_u64(rebound); + if js_nanbox_get_pointer(rebound_h.get_nanbox_f64()) == 0 { throw_iterator_method_not_callable(); } // Spec `GetIterator(obj)` → `Call(method, obj)`: the @@ -838,44 +881,57 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { // from IMPLICIT_THIS, so set it here too — mirroring `js_get_iterator`. // Without this the wrapper saw a stale `this` and the generator // yielded nothing (empty spread). - let prev_this = crate::object::js_implicit_this_set(value); + let prev_this = crate::object::js_implicit_this_set(value()); + // The DISPLACED receiver rides through arbitrary user code before + // being republished, so it is rooted too — republishing a from-space + // `this` is the same defect one frame out. + let prev_this_h = scope.root_nanbox_f64(prev_this); let trap_buf = crate::exception::js_try_push(); let jumped = unsafe { crate::ffi::setjmp::setjmp(trap_buf as *mut std::os::raw::c_int) }; + // `js_try_push` captured the handle-stack depth AFTER these roots + // were pushed, so the `longjmp` restore below leaves them intact and + // reading them here is sound. let iter = if jumped == 0 { - crate::closure::js_closure_call0(fn_ptr) + crate::closure::js_closure_call0(js_nanbox_get_pointer(rebound_h.get_nanbox_f64()) + as *const crate::closure::ClosureHeader) } else { // Factory threw: restore the receiver and unwind the trap frame // before re-propagating, so IMPLICIT_THIS can't leak into later // calls (mirrors `async_from_sync_call_cached_raw` above). let exc = crate::exception::js_get_exception(); crate::exception::js_clear_exception(); - crate::object::js_implicit_this_set(prev_this); + crate::object::js_implicit_this_set(prev_this_h.get_nanbox_f64()); crate::exception::js_try_end(); crate::exception::js_throw(exc) }; - crate::object::js_implicit_this_set(prev_this); + let iter_h = scope.root_nanbox_f64(iter); + crate::object::js_implicit_this_set(prev_this_h.get_nanbox_f64()); crate::exception::js_try_end(); - if crate::array::js_array_is_array(iter).to_bits() == crate::value::TAG_TRUE { - return js_iterator_to_array(crate::array::array_values_iter(iter)); + if crate::array::js_array_is_array(iter_h.get_nanbox_f64()).to_bits() + == crate::value::TAG_TRUE + { + return js_iterator_to_array(crate::array::array_values_iter( + iter_h.get_nanbox_f64(), + )); } - if !object_like_iterator_result(iter) { + if !object_like_iterator_result(iter_h.get_nanbox_f64()) { let msg = b"Result of the Symbol.iterator method is not an object"; let msg_str = crate::string::js_string_from_bytes(msg.as_ptr(), msg.len() as u32); let err = crate::error::js_typeerror_new(msg_str); crate::exception::js_throw(js_nanbox_pointer(err as i64)); } - return js_iterator_to_array(iter); + return js_iterator_to_array(iter_h.get_nanbox_f64()); } } - if crate::array::js_array_is_array(value).to_bits() == crate::value::TAG_TRUE { - return js_iterator_to_array(crate::array::array_values_iter(value)); + if crate::array::js_array_is_array(value()).to_bits() == crate::value::TAG_TRUE { + return js_iterator_to_array(crate::array::array_values_iter(value())); } - if has_named_next(value) { - return js_iterator_to_array(value); + if has_named_next(value()) { + return js_iterator_to_array(value()); } - throw_not_iterable(value); + throw_not_iterable(value()); } #[no_mangle] @@ -1272,14 +1328,6 @@ pub extern "C" fn js_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader { read_result() } -/// NaN-box a `StringHeader*` so it can live in a `RuntimeHandleScope` slot as -/// an ordinary `Nanbox` root (marked AND rewritten) instead of a `RawTagged` -/// one that would have to be read back through `get_raw_const_ptr`. -#[inline] -fn nanbox_string_key(ptr: *mut crate::StringHeader) -> f64 { - f64::from_bits(crate::value::JSValue::string_ptr(ptr).bits()) -} - /// `BindingRestElement` / `AssignmentRestElement` iterator drain for /// destructuring (`let [...rest] = src`, `method([...rest]) {}`). Spec §8.5.3 /// ArrayBindingPattern step for a rest element: if the iterator is already diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index e523bdba75..503a408d4a 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -8,6 +8,68 @@ use super::*; +/// An owned copy of a property key's bytes (#7498). +/// +/// **A `&[u8]` sliced out of a `StringHeader`'s payload is a borrow of the GC +/// heap, and the collector cannot see it.** Rooting the key in a +/// `RuntimeHandleScope` keeps the object alive and rewrites the *slot* — it +/// does nothing for a `&[u8]`/`&str` already pointing at the pre-move address. +/// The property-lookup tower is full of that shape: a key is sliced once at the +/// top of an arm and compared, hashed and forwarded for hundreds of lines, and +/// most of the probes in between (`resolve_inherited_field`, +/// `fetch_subclass_handle_id`, `temporal_subclass_cell`, +/// `array_prototype_property_value`, …) intern a key string of their own, which +/// allocates. +/// +/// The only sound shape is to stop borrowing. Copy the bytes out once, before +/// the arm's first allocation, and use the copy everywhere below. Property names +/// are short, so the common case is a stack buffer and no allocator traffic at +/// all; the spill keeps that total rather than "usually". +pub(crate) struct HeapKeyBytes { + inline: [u8; Self::INLINE], + len: usize, + spill: Vec, +} + +impl HeapKeyBytes { + /// Every property name this tower sees in practice (`length`, + /// `constructor`, `@@iterator`, `__perry_temporal_cell__`, a numeric index) + /// fits. Longer keys spill rather than falling back to the borrow. + pub(crate) const INLINE: usize = 64; + + pub(crate) fn copy_of(src: &[u8]) -> Self { + let mut inline = [0u8; Self::INLINE]; + let mut spill = Vec::new(); + if src.len() <= Self::INLINE { + inline[..src.len()].copy_from_slice(src); + } else { + spill = src.to_vec(); + } + Self { + inline, + len: src.len(), + spill, + } + } + + /// Copy a heap key's payload. `key` must be a live, non-null + /// `StringHeader`; callers check that before reaching here. + pub(crate) unsafe fn copy_of_key(key: *const crate::StringHeader) -> Self { + Self::copy_of(std::slice::from_raw_parts( + (key as *const u8).add(std::mem::size_of::()), + (*key).byte_len as usize, + )) + } + + pub(crate) fn as_bytes(&self) -> &[u8] { + if self.len <= Self::INLINE { + &self.inline[..self.len] + } else { + &self.spill + } + } +} + /// Hidden own-field name under which a `class X extends Request/Response` /// instance stashes the id of its underlying native Web-Fetch handle. Written /// by the `js_request_subclass_init` / `js_response_subclass_init` super-init @@ -37,11 +99,19 @@ pub(crate) unsafe fn fetch_subclass_handle_id(obj: usize) -> Option { if (*gc_header).obj_type != crate::gc::GC_TYPE_OBJECT { return None; } + // #7498: the key allocation below can trigger a copying minor, which moves + // `obj` and rewrites only the slots it can see — a bare `usize` is not one. + // This frame is on the `[...obj.arr]` prototype-walk stack that + // `PERRY_GC_PROTECT_FROMSPACE=1` faults in. Root the receiver first, then + // read its post-collection address for the field read. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as i64)); let key = crate::string::js_string_from_bytes( FETCH_SUBCLASS_HANDLE_FIELD.as_ptr(), FETCH_SUBCLASS_HANDLE_FIELD.len() as u32, ); - let v = js_object_get_field_by_name(obj as *const ObjectHeader, key); + let obj = crate::value::js_nanbox_get_pointer(obj_h.get_nanbox_f64()) as *const ObjectHeader; + let v = js_object_get_field_by_name(obj, key); if v.is_undefined() { return None; } @@ -86,11 +156,16 @@ pub(crate) unsafe fn temporal_subclass_cell(obj: usize) -> Option { if (*gc_header).obj_type != crate::gc::GC_TYPE_OBJECT { return None; } + // #7498: same shape as `fetch_subclass_handle_id` above — `obj` must not + // ride the key allocation as a bare `usize`. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as i64)); let key = crate::string::js_string_from_bytes( TEMPORAL_SUBCLASS_CELL_FIELD.as_ptr(), TEMPORAL_SUBCLASS_CELL_FIELD.len() as u32, ); - let v = js_object_get_field_by_name(obj as *const ObjectHeader, key); + let obj = crate::value::js_nanbox_get_pointer(obj_h.get_nanbox_f64()) as *const ObjectHeader; + let v = js_object_get_field_by_name(obj, key); if v.is_undefined() { return None; } diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index 2a9ae7ea91..170ee97f55 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/crates/perry-runtime/src/object/field_get_set/accessors.rs @@ -144,6 +144,27 @@ unsafe fn default_object_prototype_property_value( key: *const crate::StringHeader, ) -> Option { let _guard = object_prototype_lookup_guard()?; + // #7498: THIS IS THE FRAME `PERRY_GC_PROTECT_FROMSPACE=1` FAULTS IN on the + // `[...obj.arr]` path — a 56-byte from-space `GC_TYPE_STRING`, i.e. `key`. + // Both arguments are GC-managed and both are live across the two calls + // below before their first use: `js_get_global_this_builtin_value` interns + // its own `"Object"` key (an allocation), and `closure_get_dynamic_prop` + // can run an accessor, which is user code. A copying minor at either point + // moves the key string and the receiver and rewrites only the slots it can + // see; a bare argument is not one. + // + // Root both before the first of those calls and read each back at its + // point of use. NaN-boxed handles only, so this module adds no bare + // `get_raw_*_ptr` to `scripts/raw_handle_debt.py`. + let scope = crate::gc::RuntimeHandleScope::new(); + let key_h = scope.root_nanbox_f64(crate::value::nanbox_string_key(key)); + let receiver_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(receiver_addr as i64)); + let key = || { + crate::value::js_nanbox_get_pointer(key_h.get_nanbox_f64()) as *const crate::StringHeader + }; + let receiver_addr = + || crate::value::js_nanbox_get_pointer(receiver_h.get_nanbox_f64()) as usize; + let object_ctor = js_get_global_this_builtin_value(b"Object".as_ptr(), 6); let ctor_value = JSValue::from_bits(object_ctor.to_bits()); if !ctor_value.is_pointer() { @@ -156,15 +177,22 @@ unsafe fn default_object_prototype_property_value( return None; } let proto_ptr = proto_value.as_pointer::(); - if proto_ptr.is_null() || proto_ptr as usize == receiver_addr { + if proto_ptr.is_null() || proto_ptr as usize == receiver_addr() { return None; } - let receiver = f64::from_bits(crate::value::js_nanbox_pointer(receiver_addr as i64).to_bits()); + let receiver = crate::value::js_nanbox_pointer(receiver_addr() as i64); let previous_this = super::super::js_implicit_this_set(receiver); + // The DISPLACED `this` and the displaced accessor receiver both ride + // through `js_object_get_field_by_name` — which can run a getter — before + // being republished. Rooting the ACCESSOR_RECEIVER_OVERRIDE cell (#7231) + // protects the armed value, not these saved ones; that residual is what + // these two handles close. + let previous_this_h = scope.root_nanbox_f64(previous_this); let prev_override = accessor_receiver_override_begin(receiver); - let property = js_object_get_field_by_name(proto_ptr, key); - accessor_receiver_override_end(prev_override); - super::super::js_implicit_this_set(previous_this); + let prev_override_h = prev_override.map(|v| scope.root_nanbox_f64(v)); + let property = js_object_get_field_by_name(proto_ptr, key()); + accessor_receiver_override_end(prev_override_h.map(|h| h.get_nanbox_f64())); + super::super::js_implicit_this_set(previous_this_h.get_nanbox_f64()); if property.is_undefined() { None } else { @@ -460,6 +488,28 @@ pub(crate) unsafe fn array_prototype_property_value( name: &str, receiver_addr: usize, ) -> Option { + // #7498 — THE FAULT `PERRY_GC_PROTECT_FROMSPACE=1` REPORTS FOR + // `[...obj.arr]`, measured with lldb: `EXC_BAD_ACCESS` on the `ldrsb` of + // the UTF-8 scan inside `js_string_from_bytes` below, reading a 56-byte + // retired-from-space `GC_TYPE_STRING`. + // + // `name` is not an owned string. `get_field_by_name_object_tail` slices it + // straight out of the key `StringHeader`'s payload + // (`slice::from_raw_parts(key_ptr, key_len)`), so it is a BORROW OF THE GC + // HEAP — and a borrow is exactly the thing the collector cannot see or + // rewrite. Every call below allocates: `js_get_global_this_builtin_value` + // interns `"Array"`, `closure_get_dynamic_prop` can run an accessor, and + // `js_string_from_bytes` reads its SOURCE bytes *after* its own + // `string_storage_alloc`. Any one of those can move the key out from under + // `name`. + // + // A `RuntimeHandleScope` cannot fix this: rooting the key would keep the + // object alive and rewrite the slot, but `name`'s pointer is a `&str`, not + // a slot. The only sound shape is to stop borrowing the heap — see + // [`HeapKeyBytes`]. + let name_copy = super::HeapKeyBytes::copy_of(name.as_bytes()); + let name: &str = std::str::from_utf8_unchecked(name_copy.as_bytes()); + let ctor = super::super::js_get_global_this_builtin_value(b"Array".as_ptr(), 5); let ctor_value = JSValue::from_bits(ctor.to_bits()); if !ctor_value.is_pointer() { @@ -471,26 +521,44 @@ pub(crate) unsafe fn array_prototype_property_value( if !proto_value.is_pointer() { return None; } - let proto_ptr = proto_value.as_pointer::() as usize; - let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); - if let Some(v) = own_data_field_by_name(proto_ptr as *const ObjectHeader, key) { + // #7498: `js_string_from_bytes` ALLOCATES, so `Array.prototype` and the + // receiver cannot be carried across it as bare `usize`s — and the key it + // produces is itself a fresh heap string this function then hands to two + // more calls that can collect (`js_object_get_field_by_name` runs getters; + // `default_object_prototype_property_value` interns another key). Root all + // three and read each back at its point of use. + let scope = crate::gc::RuntimeHandleScope::new(); + let proto_h = scope.root_nanbox_f64(proto); + let receiver_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(receiver_addr as i64)); + let key_h = scope.root_nanbox_f64(crate::value::nanbox_string_key( + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32), + )); + let proto_ptr = || crate::value::js_nanbox_get_pointer(proto_h.get_nanbox_f64()) as usize; + let receiver_addr = + || crate::value::js_nanbox_get_pointer(receiver_h.get_nanbox_f64()) as usize; + let key = || { + crate::value::js_nanbox_get_pointer(key_h.get_nanbox_f64()) as *const crate::StringHeader + }; + + if let Some(v) = own_data_field_by_name(proto_ptr() as *const ObjectHeader, key()) { return Some(v); } if let Some(v) = crate::array::array_named_property_get_by_name( - proto_ptr as *const crate::array::ArrayHeader, + proto_ptr() as *const crate::array::ArrayHeader, name, ) { return Some(JSValue::from_bits(v.to_bits())); } - if proto_ptr == receiver_addr { - return default_object_prototype_property_value(receiver_addr, key); + if proto_ptr() == receiver_addr() { + return default_object_prototype_property_value(receiver_addr(), key()); } - let receiver = f64::from_bits(crate::value::js_nanbox_pointer(receiver_addr as i64).to_bits()); + let receiver = crate::value::js_nanbox_pointer(receiver_addr() as i64); let prev_override = accessor_receiver_override_begin(receiver); - let v = js_object_get_field_by_name(proto_ptr as *const ObjectHeader, key); - accessor_receiver_override_end(prev_override); + let prev_override_h = prev_override.map(|v| scope.root_nanbox_f64(v)); + let v = js_object_get_field_by_name(proto_ptr() as *const ObjectHeader, key()); + accessor_receiver_override_end(prev_override_h.map(|h| h.get_nanbox_f64())); if v.is_undefined() { - default_object_prototype_property_value(receiver_addr, key) + default_object_prototype_property_value(receiver_addr(), key()) } else { Some(v) } diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index 7c5d6d6518..4724ba8751 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -886,9 +886,13 @@ pub(crate) fn get_field_by_name_object_tail( // statically known, so this branch catches the dynamic case. if gc_type == crate::gc::GC_TYPE_ARRAY { if !key.is_null() { - let key_ptr = (key as *const u8).add(std::mem::size_of::()); - let key_len = (*key).byte_len as usize; - let key_bytes = std::slice::from_raw_parts(key_ptr, key_len); + // #7498: OWNED — this is the `[...obj.arr]` arm, and it hands + // `key_bytes`/`name` to `array_prototype_property_value` and + // `js_get_global_this_builtin_value`, both of which allocate, + // before `is_array_method_value_name(key_bytes)` reads it again. + // See [`HeapKeyBytes`] for why a borrow cannot be rooted. + let key_copy = crate::object::field_get_set::HeapKeyBytes::copy_of_key(key); + let key_bytes = key_copy.as_bytes(); let arr = obj as *const crate::array::ArrayHeader; if key_bytes == b"length" { return JSValue::number(crate::array::js_array_length(arr) as f64); @@ -1526,10 +1530,17 @@ pub(crate) fn get_field_by_name_object_tail( // Fast path: check field index cache (keys_array_ptr + key_hash → field_index) // Objects with the same shape share the same keys_array, so we cache per-shape lookups. - let key_bytes = std::slice::from_raw_parts( - (key as *const u8).add(std::mem::size_of::()), - (*key).byte_len as usize, - ); + // + // #7498: OWNED, not borrowed. This arm carries `key_bytes` across ~430 + // lines of probes that allocate — `resolve_inherited_field`, + // `ordinary_object_prototype_property_value`, `fetch_subclass_handle_id` + // and `temporal_subclass_cell` each intern a key string — and a slice + // into the key's `StringHeader` is a borrow of the GC heap that no root + // can rewrite. `PERRY_GC_PROTECT_FROMSPACE=1` faults on the + // `key_bytes != TEMPORAL_SUBCLASS_CELL_FIELD` comparison below, + // reading a 56-byte retired-from-space `GC_TYPE_STRING`. + let key_copy = crate::object::field_get_set::HeapKeyBytes::copy_of_key(key); + let key_bytes = key_copy.as_bytes(); // Gate-neutral builtin accessors mark only their owning object. Consult // the descriptor table before an accessor's empty backing slot is read; // unrelated objects pay only this already-loaded header-bit test. diff --git a/crates/perry-runtime/src/symbol/get.rs b/crates/perry-runtime/src/symbol/get.rs index 69091295ab..1a6306c3e7 100644 --- a/crates/perry-runtime/src/symbol/get.rs +++ b/crates/perry-runtime/src/symbol/get.rs @@ -77,10 +77,25 @@ unsafe fn req_handle_symbol_fallback(obj_f64: f64, sym_f64: f64) -> Option if !crate::object::is_valid_obj_ptr(raw as *const u8) { return None; } + // #7498: THIS IS THE STALE DEREF `PERRY_GC_PROTECT_FROMSPACE=1` REPORTS + // for `[...obj.arr]`. `js_string_from_bytes` below ALLOCATES, so a copying + // minor can relocate the receiver while it exists only in the bare `raw` + // usize — which the collector cannot see and therefore never rewrites. + // `js_object_get_field_by_name` then reads that pre-move copy's + // `keys_array` field out of retired from-space, and the fault lands on a + // 40-byte `GC_TYPE_ARRAY` two frames down. + // + // This helper runs on EVERY heap-object symbol read whose own-symbol + // lookup missed — including every `[Symbol.iterator]` resolution behind an + // array or object spread — so the window is unconditional, which is why + // the reproducer faults 5/5 rather than intermittently. + let scope = crate::gc::RuntimeHandleScope::new(); + let recv_h = scope.root_nanbox_f64(obj_f64); let key = b"_req"; let kh = crate::string::js_string_from_bytes(key.as_ptr(), key.len() as u32); let req = crate::object::js_object_get_field_by_name_f64( - raw as *const crate::object::ObjectHeader, + crate::value::js_nanbox_get_pointer(recv_h.get_nanbox_f64()) + as *const crate::object::ObjectHeader, kh as *const crate::StringHeader, ); let rbits = req.to_bits(); @@ -520,9 +535,21 @@ pub unsafe extern "C" fn js_object_get_symbol_property(obj_f64: f64, sym_f64: f6 // to a small native handle (POINTER-tagged, below HANDLE_BAND_MAX, not a // real heap object), and only returns a value the handle actually holds — // so ordinary objects (no `_req`, or a heap `_req`) are unaffected. - if let Some(v) = req_handle_symbol_fallback(obj_f64, sym_f64) { + // #7498: `req_handle_symbol_fallback` interns a `"_req"` key, so it is the + // first unconditional allocation on this resolver's path — every + // `[Symbol.iterator]` read behind a spread reaches it. Root the receiver + // and the symbol across it and rebind BOTH `obj_f64` and the derived + // `bits` from the roots afterwards, so no line below can name a + // pre-collection address. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_nanbox_f64(obj_f64); + let sym_h = scope.root_nanbox_f64(sym_f64); + if let Some(v) = req_handle_symbol_fallback(obj_h.get_nanbox_f64(), sym_h.get_nanbox_f64()) { return v; } + let obj_f64 = obj_h.get_nanbox_f64(); + let sym_f64 = sym_h.get_nanbox_f64(); + let bits = obj_f64.to_bits(); let sym_key = sym_key_from_f64(sym_f64); if sym_key != 0 { let jsval = crate::value::JSValue::from_bits(bits); diff --git a/crates/perry-runtime/src/value/mod.rs b/crates/perry-runtime/src/value/mod.rs index ebd0b2c457..607afcb82d 100644 --- a/crates/perry-runtime/src/value/mod.rs +++ b/crates/perry-runtime/src/value/mod.rs @@ -94,6 +94,7 @@ pub use handle::{ }; // ----- Basic NaN-box pack / unpack FFI ----- +pub(crate) use nanbox::nanbox_string_key; pub use nanbox::{ js_checkpoint, js_debug_val, js_get_string_pointer_unified, js_nanbox_bigint, js_nanbox_get_bigint, js_nanbox_get_pointer, js_nanbox_get_string_pointer, js_nanbox_is_bigint, diff --git a/crates/perry-runtime/src/value/nanbox.rs b/crates/perry-runtime/src/value/nanbox.rs index 38e9828b34..551edb90a4 100644 --- a/crates/perry-runtime/src/value/nanbox.rs +++ b/crates/perry-runtime/src/value/nanbox.rs @@ -121,6 +121,20 @@ pub extern "C" fn js_nanbox_string(ptr: i64) -> f64 { f64::from_bits(jsval.bits()) } +/// NaN-box a `StringHeader*` so a heap string key can live in a +/// [`crate::gc::RuntimeHandleScope`] slot as an ordinary `Nanbox` root — marked +/// AND rewritten — instead of a `RawTagged` one that would have to be read back +/// through `get_raw_const_ptr` (which `scripts/raw_handle_debt.py` counts). +/// +/// Unlike [`js_nanbox_string`] this NEVER allocates for a null pointer: it is +/// used to root a value *before* the next collection point, so allocating here +/// would reintroduce the very window the root exists to close. A null boxes to +/// a tagged zero, which every root visitor ignores. +#[inline] +pub(crate) fn nanbox_string_key(ptr: *const crate::string::StringHeader) -> f64 { + f64::from_bits(JSValue::string_ptr(ptr as *mut crate::string::StringHeader).bits()) +} + /// Debug checkpoint function: prints checkpoint number to stderr. /// Used to narrow down crash locations in generated code. #[no_mangle] diff --git a/test-files/test_gap_gc_spread_symbol_iterator_rooting.ts b/test-files/test_gap_gc_spread_symbol_iterator_rooting.ts new file mode 100644 index 0000000000..b87aebfa52 --- /dev/null +++ b/test-files/test_gap_gc_spread_symbol_iterator_rooting.ts @@ -0,0 +1,74 @@ +// #7498: `[...obj.arr]` when the enclosing function is small enough to INLINE. +// +// TWO LOWERINGS, TWO BUGS. Out of line, the spread calls `js_iterator_to_array` +// directly — that is the drain #7495 rooted, and +// `test_gap_gc_iterator_drain_rooting.ts` is deliberately sized to stay on it. +// Inlined, the spread routes through `array_from_spread_value` instead, which +// resolves `[Symbol.iterator]` through the whole prototype-walk tower first. +// `clone` here is the shrunk twin of that file's `deepClone`: same shape, small +// enough to inline, so it takes the OTHER route. +// +// WHAT WENT STALE. Every frame on that walk held a GC-managed value in a bare +// Rust local across an allocation, and a bare local is exactly what a copying +// minor cannot rewrite: +// +// * `req_handle_symbol_fallback` (`symbol/get.rs`) read the receiver into a +// `usize`, then interned a `"_req"` key — an allocation — and read a field +// off the PRE-move address. `PERRY_GC_PROTECT_FROMSPACE=1` faults there on +// a 40-byte `GC_TYPE_ARRAY`: the pre-move copy's `keys_array`. It runs on +// EVERY heap-object symbol read whose own-symbol lookup missed, so the +// window is unconditional — the fault is 5/5, not intermittent. +// * `array_prototype_property_value` (`field_get_set/accessors.rs`) took its +// property name as a `&str` BORROWED OUT OF THE KEY'S `StringHeader`, then +// allocated three times before reading it. That is the 56-byte +// `GC_TYPE_STRING` fault in #7498's second trace, and no root can fix it: a +// `&str` is not a slot the collector can rewrite. The name is copied off +// the heap before the first allocation instead. +// * `array_from_spread_value` itself carried the spread RECEIVER through a +// dozen classification probes and the entire symbol walk, then used it to +// rebind `this` for the `[Symbol.iterator]()` factory. +// +// NOT OBSERVABLE FROM OUTPUT, BY CONSTRUCTION. Evacuation copies rather than +// zeroes, so a stale address still reads the correct old bytes and this file +// prints the right checksum before and after, on both link modes. Only +// unmapping retired from-space turns the latent read into a signal: +// +// PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=200 ./out +// +// THE PROTECTED RUN IS NOT CLEAN AFTER #7498. The two faults above are gone; +// the walk now reaches further and faults at two OTHER sites, each filed +// separately — see the corpus note in `test-parity/gc_repsel_corpus.txt`. +// Saying "clean" here would be the overclaim this repo keeps paying for. + +const N = 50_000; + +interface Item { + id: number; + meta: { tags: string[] }; +} + +const proto: Item = { + id: 0, + meta: { tags: ["a", "b", "c"] }, +}; + +// Deliberately small. Growing this function pushes the spread back out of line +// and onto `js_iterator_to_array`, i.e. onto #7495's path instead of this one. +function clone(o: Item): Item { + return { id: o.id, meta: { tags: [...o.meta.tags] } }; +} + +let totalIds = 0; +let badTagLen = 0; +let badTagVal = 0; +for (let i = 0; i < N; i++) { + proto.id = i; + const c = clone(proto); + totalIds += c.id; + if (c.meta.tags.length !== 3) { + badTagLen++; + } else if (c.meta.tags[2] !== "c") { + badTagVal++; + } +} +console.log("checksum:", totalIds, "badLen", badTagLen, "badVal", badTagVal); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 836e74768e..d80de7de42 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -601,6 +601,20 @@ test_gap_gc_optional_param_receiver_rooting # corrupt this file's result, so the file gates the drain and nothing else. When # #7498 lands, a protected run of this file should go silent — if it does not, # there is a third site. +# +# #7498 HAS LANDED AND THIS FILE DID NOT GO SILENT — so, per the sentence +# above, THERE IS A THIRD SITE, and it is filed as #7528. #7498's +# two faults are gone from this file's protected run; what remains is +# `js_native_call_method` reading its ROOTED receiver into a local ~330 lines +# and a dozen allocating probes before `is_closure_ptr` dereferences it +# (`array_from_spread_value` → `js_closure_call0` → `js_native_call_method + +# 5604`). Measured on the #7498 branch, macOS/arm64: this file prints +# `badLen 1` instead of `badLen 0` (5/5), and `PERRY_GEN_GC=0`, +# `PERRY_GC_SCAVENGE=0` and `PERRY_WRITE_BARRIERS=0` each make it correct while +# `PERRY_GEN_GC_EVACUATE=0` does not — the copying minor, not policy +# evacuation. Patching the one faulting line was tried and REVERTED: the fault +# simply moved 800 bytes further into the same function, which is the signal +# that the whole receiver needs re-reading, not one arm of it. test_gap_gc_iterator_drain_rooting # --- #7497: the globalThis builtin-value lookup ----------------------------- @@ -633,3 +647,53 @@ test_gap_gc_iterator_drain_rooting # is registered here because the moving arms are what turn it into a fault with # an address instead of a wrong answer. test_gap_gc_global_builtin_lookup_rooting +# --- #7498: the OTHER `[...obj.arr]` lowering (inlined → the symbol walk) ---- +# +# The spread has two lowerings. Out of line it calls `js_iterator_to_array` +# (above). Inlined, it routes through `array_from_spread_value`, which resolves +# `[Symbol.iterator]` through the whole prototype-walk tower first — and three +# frames on that walk held a GC value the collector cannot see: +# +# * `req_handle_symbol_fallback` read the receiver into a `usize`, interned a +# `"_req"` key (an allocation), then read a field off the PRE-move address. +# It runs on every heap-object symbol read whose own-symbol lookup missed, +# so the window is unconditional — 5/5, not intermittent. +# * `array_prototype_property_value` and the array/object arms of +# `get_field_by_name_object_tail` took the property name as a `&str` / a +# `&[u8]` BORROWED OUT OF THE KEY'S `StringHeader`. No root fixes that: a +# borrow is not a slot the collector can rewrite. They copy the bytes off +# the heap before their first allocation now. +# * `array_from_spread_value` carried the spread receiver through a dozen +# probes and the entire walk, then used it to rebind `this` for the +# factory. +# +# LATENT BY CONSTRUCTION, like `10_store_receiver_across_alloc.ts`: evacuation +# copies rather than zeroes, so the stale read returns the correct old bytes and +# this file printed the RIGHT checksum before the fix, on both links. Only +# unmapping retired from-space makes it a signal. Measured on this branch: +# +# before, default link + default env byte-exact with the oracle +# before, auto-optimize link byte-exact with the oracle +# before, PROTECT_FROMSPACE=1 DEPTH=200 FAULT 5/5 on BOTH links, at +# array_from_spread_value → +# js_object_get_symbol_property → +# js_object_get_field_by_name_f64 → +# js_object_get_field_by_name, on a +# 40-byte GC_TYPE_ARRAY (the pre-move +# copy's keys_array) at minor #1 +# after, both links byte-exact with the oracle +# after, PROTECT_FROMSPACE=1 DEPTH=200 CLEAN 5/5 on BOTH links, with the +# instrument proved live: four +# `retired_set=#N` page-sets and +# `[gc-copy-minor] ran +# copied_objects=11794` on the first +# minor alone +# +# The auto-optimize arm was A/B'd with the runtime archive rebuilt from source +# on both sides (`perry-auto-*`, asserted on the `[link] invoking:` line), so +# the two rows are the same link with different runtime source, not two links. +# +# KEEP `clone` SMALL. Its size is the whole point: grow it and the spread goes +# back out of line onto `js_iterator_to_array`, i.e. onto #7475's path, and this +# file stops covering the walk. +test_gap_gc_spread_symbol_iterator_rooting