From 6569620b6763ae6771a176516ccb80e7e2a9aacc Mon Sep 17 00:00:00 2001 From: Ralph Date: Mon, 15 Jun 2026 01:52:14 -0700 Subject: [PATCH] fix(runtime): route Proxy receivers through traps for compound-assign + array ops; fix Function.toString static read (#5135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importing `immer` and calling `produce(base, d => { d.count++; d.list.push(3) })` crashed with SIGSEGV. immer drafts are `Proxy` objects statically typed as the plain base type, which exposed three independent bugs: 1. A compound-assignment write through a Proxy (`d.count++`) lowers to `js_object_set_field_by_name` with the proxy's NaN-box tag masked off. The write path had no proxy branch (only the read path did), so it dereferenced the masked proxy id as an `ObjectHeader` → SIGSEGV. Mirror the read-side dispatch: a registered proxy receiver routes to `js_proxy_set`. 2. The statically-typed `Function.toString` static-member read collapsed to `globalThis.toString` (folded to a number), so `Function.toString.call(Ctor)` in immer's `isPlainObject` threw "Function.prototype.call was called on a value that is not a function". `toString` is a universal inherited method; add it to the reroute-undo keep-list alongside `valueOf`/`hasOwnProperty`/… so the reified constructor receiver is preserved. 3. A native array method (`push`) / `length` read on a value that is a Proxy at runtime (`d.list.push(x)`) dereferenced the masked proxy id as an `ArrayHeader` → SIGSEGV. `js_array_push_f64` now performs the spec push for a single element through the proxy's `get`/`set` traps, and `js_array_length` reads `length` through the `get` trap, when the receiver is a registered proxy. (Recursing through the native method dispatcher would re-enter the same helper; the spec ops avoid that.) The exact issue repro now prints `0 1 1,2,3 2`, matching Node. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/perry-hir/src/lower/expr_member.rs | 8 ++ crates/perry-runtime/src/array/header.rs | 25 ++++ crates/perry-runtime/src/array/indexing.rs | 13 ++ crates/perry-runtime/src/array/mod.rs | 2 +- crates/perry-runtime/src/array/push_pop.rs | 38 ++++++ .../src/object/field_set_by_name.rs | 21 +++ ...35_proxy_compound_and_function_tostring.rs | 122 ++++++++++++++++++ 7 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index c434b3b0a2..e5ec4e99b7 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -2043,6 +2043,13 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re // bare `GlobalGet(0)` — otherwise the predicate/dispatch runs // against globalThis. The reroute above already resolved the // receiver to `globalThis.`; don't undo it here. + // #5135: `toString` is a universal inherited method too — + // `Function.toString` / `Array.toString` resolve to a real + // function in Node. Without keeping the reified constructor + // receiver the read collapses to `globalThis.toString`, + // which codegen folds to a number, so + // `Function.toString.call(Ctor)` (immer's `isPlainObject`) + // threw "call on a non-function". let outer_is_inherited_object_proto_method = matches!( outer_static_member, Some( @@ -2050,6 +2057,7 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re | "isPrototypeOf" | "propertyIsEnumerable" | "toLocaleString" + | "toString" | "valueOf" ) ); diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index e5bb569bf7..4d47ba9b08 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -600,6 +600,31 @@ pub(crate) fn clean_arr_ptr_mut(arr: *mut ArrayHeader) -> *mut ArrayHeader { clean_arr_ptr(arr as *const ArrayHeader) as *mut ArrayHeader } +/// #5135: detect a Proxy id arriving where an `ArrayHeader` pointer is +/// expected. immer's array drafts are Proxies typed (statically) as plain +/// arrays, so `draft.push(x)` / `draft.length` reach the native array helpers +/// with the masked proxy id instead of a real heap pointer. Deref-ing one as an +/// `ArrayHeader` reads unmapped memory and SIGSEGVs. Callers use this to detect +/// the case and route the operation through the proxy's traps. Returns the +/// re-boxed (`POINTER_TAG`) proxy value when `arr` is a *registered* proxy. +#[inline] +pub(crate) fn array_ptr_as_proxy(arr: *const ArrayHeader) -> Option { + let bits = arr as u64; + let raw = if (bits >> 48) >= 0x7FF8 { + bits & 0x0000_FFFF_FFFF_FFFF + } else { + bits + }; + if crate::value::addr_class::is_proxy_id_band(raw as usize) { + const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000; + let boxed = f64::from_bits(POINTER_TAG | raw); + if crate::proxy::js_proxy_is_proxy(boxed) != 0 { + return Some(boxed); + } + } + None +} + /// Normalize an Array.prototype method receiver into a real ArrayHeader. /// /// `Array.prototype..call(arrayLike, ...)` lets a *generic array-like diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 397e29d59e..a710867bcc 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -352,6 +352,19 @@ fn array_get_property_by_key(arr: *const ArrayHeader, key: *const crate::StringH #[no_mangle] pub extern "C" fn js_array_length(arr: *const ArrayHeader) -> u32 { + // #5135: a Proxy typed (statically) as an array (immer drafts) reaches here + // with the masked proxy id. Read `length` through the proxy `get` trap + // rather than deref-ing the id as an `ArrayHeader`. + if let Some(proxy) = array_ptr_as_proxy(arr) { + let key = crate::string::js_string_from_bytes(b"length".as_ptr(), 6); + let key_f64 = crate::value::js_nanbox_string(key as i64); + let n = crate::builtins::js_number_coerce(crate::proxy::js_proxy_get(proxy, key_f64)); + return if n.is_finite() && n > 0.0 { + n.min(u32::MAX as f64) as u32 + } else { + 0 + }; + } let arr = { let bits = arr as u64; let top16 = bits >> 48; diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 64fe20551f..62d4cabf6e 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -134,7 +134,7 @@ pub(crate) use self::header::{ array_named_property_get, array_named_property_get_by_name, array_named_property_has, array_named_property_names, array_named_property_set, array_numeric_raw_f64_get, array_numeric_raw_f64_push_inbounds, array_numeric_raw_f64_set_inbounds, array_object_flags, - canonicalize_array_numeric_store_value, clean_arr_ptr, clean_arr_ptr_mut, + array_ptr_as_proxy, canonicalize_array_numeric_store_value, clean_arr_ptr, clean_arr_ptr_mut, clear_array_numeric_layout, clear_array_numeric_layout_ptr, gc_element_slot_range, mark_array_layout_unknown, normalize_array_receiver, note_array_slot, note_array_slot_layout_only, rebuild_array_layout, rebuild_array_layout_exact, diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 01d23c50f0..f181adec9f 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -137,9 +137,47 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu } /// Push an element to the end of an array, growing if needed +/// #5135: read `Get(proxy, "length")` and ToLength-coerce it. Used by the +/// proxy-array push path so immer drafts (Proxies typed as arrays) mutate +/// through their traps instead of a native ArrayHeader deref. +unsafe fn proxy_array_length(proxy: f64) -> u64 { + let key = crate::string::js_string_from_bytes(b"length".as_ptr(), 6); + let key_f64 = crate::value::js_nanbox_string(key as i64); + let n = crate::builtins::js_number_coerce(crate::proxy::js_proxy_get(proxy, key_f64)); + if n.is_finite() && n >= 0.0 { + n as u64 + } else { + 0 + } +} + +/// #5135: `Set(proxy, , value)` through the proxy's `set` trap. The +/// key string is allocated fresh per call so an intervening GC can't leave a +/// stale interior pointer. +unsafe fn proxy_set_str_key(proxy: f64, key_bytes: &[u8], value: f64) { + let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + let key_f64 = crate::value::js_nanbox_string(key as i64); + crate::proxy::js_proxy_set(proxy, key_f64, value); +} + /// Returns a pointer to the (possibly reallocated) array #[no_mangle] pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut ArrayHeader { + // #5135: a Proxy whose static type is an array (immer drafts) reaches here + // with the masked proxy id. Perform the spec `Array.prototype.push` for a + // single element directly through the proxy's `get`/`set` traps: + // len = ToLength(Get(P, "length")); Set(P, len, value); Set(P, "length", len+1) + // Routing through the native push (`js_native_call_method`) would recurse + // back here with the same proxy. Return `arr` unchanged so the codegen's + // realloc write-back is a no-op (the proxy mutates its target in place). + if let Some(proxy) = array_ptr_as_proxy(arr) { + let len = unsafe { proxy_array_length(proxy) }; + unsafe { + proxy_set_str_key(proxy, len.to_string().as_bytes(), value); + proxy_set_str_key(proxy, b"length", (len as f64) + 1.0); + } + return arr; + } let arr = clean_arr_ptr_mut(arr); if arr.is_null() { return js_array_alloc(0); diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index 1f1017d08e..2f6a2a39be 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -180,6 +180,27 @@ pub extern "C" fn js_object_set_field_by_name( key: *const crate::StringHeader, value: f64, ) { + // #5135: the receiver may be a Proxy id arriving with its NaN-box tag + // already masked off (the `obj.prop++` / `PropertyUpdate` codegen path + // hands us the bare pointer band, not the full POINTER_TAG value). A Proxy + // is encoded as a small registered id; deref-ing one as an `ObjectHeader` + // reads unmapped memory and SIGSEGVs. Mirror the read-side dispatch in + // `js_object_get_field_by_name` so a `proxy.foo = v` write goes through the + // `set` trap instead of corrupting the cell. `js_proxy_is_proxy` validates + // the value is a *registered* proxy so a real heap object whose masked + // address happens to be small isn't misrouted. + { + let addr = obj as u64; + if crate::value::addr_class::is_proxy_id_band(addr as usize) && !key.is_null() { + const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000; + let boxed = f64::from_bits(POINTER_TAG | (addr & 0x0000_FFFF_FFFF_FFFF)); + if crate::proxy::js_proxy_is_proxy(boxed) != 0 { + let key_f64 = f64::from_bits(crate::value::js_nanbox_string(key as i64).to_bits()); + crate::proxy::js_proxy_set(boxed, key_f64, value); + return; + } + } + } // `Object.prototype["2"] = v` (stringified-index write) makes the index // visible through array hole/OOB reads. Cheap gate: one relaxed flag // load, then an address compare against the cached canonical diff --git a/crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs b/crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs new file mode 100644 index 0000000000..dc90691f7a --- /dev/null +++ b/crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs @@ -0,0 +1,122 @@ +//! Regression tests for #5135: importing `immer` and calling +//! `produce(base, draft => { draft.count++; draft.list.push(3) })` crashed with +//! SIGSEGV. immer's drafts are `Proxy` objects that are statically typed as the +//! plain base type, which exposed three independent Perry bugs. These tests +//! reproduce each with a plain `Proxy` (no immer dependency needed): +//! +//! 1. A compound-assignment write through a `Proxy` (`p.count++`) lowered to +//! `js_object_set_field_by_name` with the proxy's NaN-box tag masked off; +//! the runtime had no proxy branch there and dereferenced the masked id as +//! an `ObjectHeader` → SIGSEGV. (The read side already routed proxies.) +//! 2. The statically-typed `Function.toString` static-member read collapsed to +//! `globalThis.toString` and folded to a number, so +//! `Function.toString.call(Ctor)` (immer's `isPlainObject`) threw +//! "Function.prototype.call was called on a value that is not a function". +//! 3. A native array method / `length` read on a value that is a `Proxy` at +//! runtime (`draft.list.push(x)`) dereferenced the masked proxy id as an +//! `ArrayHeader` → SIGSEGV. The array helpers now route a proxy receiver +//! through its traps. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + let entry = root.join("main.ts"); + let output = root.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(root) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed (signal/exit) — likely a SIGSEGV regression\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).to_string() +} + +/// Fix #1: `proxy.count++` writes through the `set` trap instead of crashing. +#[test] +fn proxy_compound_assignment_routes_through_set_trap() { + let out = compile_and_run( + r#" +const target: any = { count: 0 }; +const p: any = new Proxy(target, { + get(t: any, k: any) { return t[k]; }, + set(t: any, k: any, v: any) { t[k] = v; return true; }, +}); +p.count++; +console.log(p.count, target.count); +"#, + ); + assert_eq!( + out, "1 1\n", + "p.count++ must write through the proxy set trap" + ); +} + +/// Fix #2: `Function.toString` (and `Array.toString`) read as a value are real +/// functions, not numbers. +#[test] +fn function_tostring_static_member_is_callable() { + let out = compile_and_run( + r#" +console.log(typeof Function.toString); +console.log(typeof Array.toString); +// immer's isPlainObject reaches `Function.toString.call(Ctor)`: +console.log(typeof Function.toString.call(Array)); +"#, + ); + assert_eq!( + out, "function\nfunction\nstring\n", + "Function.toString / Array.toString must resolve to callable functions" + ); +} + +/// Fix #3: a native array method (`push`) on a value that is a Proxy at runtime +/// dispatches through the proxy's traps instead of dereferencing the masked +/// proxy id as an ArrayHeader. This mirrors immer's `draft.list.push(x)`, where +/// the receiver is a member access (`obj.list`) that returns a proxy array — the +/// `js_array_push_f64` runtime helper path the issue actually exercised. +#[test] +fn proxy_array_push_via_member_routes_through_traps() { + let out = compile_and_run( + r#" +const target: any = [1, 2]; +const inner: any = new Proxy(target, { + get(t: any, k: any) { return t[k]; }, + set(t: any, k: any, v: any) { t[k] = v; return true; }, +}); +const holder: any = { list: inner }; +holder.list.push(3); +console.log(target.join(","), holder.list.length); +"#, + ); + assert_eq!( + out, "1,2,3 3\n", + "obj.list.push must mutate the proxied array through its set trap" + ); +}