From 520c88266fa9f4952f193c2dd535021363cd1d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:28:23 +0200 Subject: [PATCH 01/10] fix(runtime): root receivers and stored values across js_string_coerce property-key coercions (#6943) --- crates/perry-runtime/src/builtins/mod.rs | 5 + crates/perry-runtime/src/builtins/numbers.rs | 27 ++++ crates/perry-runtime/src/error.rs | 106 +++++++++++--- .../src/object/array_object_ops.rs | 10 ++ .../src/object/descriptor_state.rs | 23 ++- .../perry-runtime/src/object/descriptors.rs | 131 +++++++++++++++--- .../native_call_method/common_methods.rs | 10 ++ .../src/object/object_ops/define_property.rs | 44 +++++- .../src/object/object_ops/from_entries.rs | 27 +++- .../src/object/object_ops/has_own.rs | 37 ++++- .../src/object/reflect_support.rs | 36 ++++- .../src/object/typed_array_define.rs | 24 ++++ crates/perry-runtime/src/object/with_env.rs | 11 +- crates/perry-runtime/src/proxy.rs | 54 +++++++- crates/perry-runtime/src/symbol/properties.rs | 8 ++ 15 files changed, 498 insertions(+), 55 deletions(-) diff --git a/crates/perry-runtime/src/builtins/mod.rs b/crates/perry-runtime/src/builtins/mod.rs index b526b77b9e..9123bfb8d6 100644 --- a/crates/perry-runtime/src/builtins/mod.rs +++ b/crates/perry-runtime/src/builtins/mod.rs @@ -101,6 +101,11 @@ pub use globals::{ pub(crate) use globals::{drain_queued_microtasks_count, queued_microtasks_pending}; +/// #6943: inertness predicate for the `js_string_coerce`-as-property-key +/// rooting family. Crate-internal — callers use it to skip a +/// `RuntimeHandleScope` when the coercion provably cannot GC. +pub(crate) use numbers::string_coerce_is_inert; + pub use numbers::{ js_is_finite, js_is_nan, js_number_coerce, js_number_is_finite, js_number_is_integer, js_number_is_nan, js_number_is_safe_integer, js_parse_float, js_parse_int, js_string_coerce, diff --git a/crates/perry-runtime/src/builtins/numbers.rs b/crates/perry-runtime/src/builtins/numbers.rs index 821c4ec0fc..27fd792422 100644 --- a/crates/perry-runtime/src/builtins/numbers.rs +++ b/crates/perry-runtime/src/builtins/numbers.rs @@ -673,6 +673,33 @@ pub extern "C" fn js_string_coerce(value: f64) -> *mut StringHeader { js_string_from_bytes(result.as_ptr(), result.len() as u32) } +/// True when [`js_string_coerce`] provably neither allocates nor calls back +/// into user JS for `value`, so a caller may hold a raw receiver / stored value +/// across it without a [`RuntimeHandleScope`] (#6943). +/// +/// Only an already-heap `STRING_TAG` value qualifies — that is the one arm of +/// [`js_string_coerce`] that returns before touching the allocator (it hands +/// the very same `StringHeader` pointer back). Every other shape allocates: +/// `undefined` / `null` / booleans / numbers / BigInt build their +/// stringification, an SSO short string (`SHORT_STRING_TAG`, a *different* tag) +/// materializes onto the heap, and a `POINTER_TAG` object routes through +/// `js_jsvalue_to_string`, which can invoke a user `toString` / `valueOf`. Any +/// of those can trigger a GC that **evacuates** live objects — moving the +/// caller's receiver and the value it is about to store — so callers must root +/// across the coercion instead. +/// +/// This is the `js_string_coerce` analogue of #6935's +/// `object::property_key_coercion_is_inert`, which makes the same claim about +/// `js_to_property_key`. The two predicates coincide today, but they are +/// assertions about two different functions: this one is justified by +/// [`js_string_coerce`]'s own `is_string()` early return, directly above. +/// +/// [`RuntimeHandleScope`]: crate::gc::RuntimeHandleScope +#[inline] +pub(crate) fn string_coerce_is_inert(value: f64) -> bool { + crate::value::JSValue::from_bits(value.to_bits()).is_string() +} + /// `RequireObjectCoercible(this)` + `ToString(this)` for the inline-lowered /// `String.prototype` methods (`charAt` / `charCodeAt` / `codePointAt` / /// `split` / `toUpperCase` / …) when the receiver is NOT statically diff --git a/crates/perry-runtime/src/error.rs b/crates/perry-runtime/src/error.rs index 881d9373fa..824d6bc9db 100644 --- a/crates/perry-runtime/src/error.rs +++ b/crates/perry-runtime/src/error.rs @@ -978,8 +978,16 @@ pub extern "C" fn js_global_get_or_throw_unresolved(name_value: f64) -> f64 { let g = crate::object::js_get_global_this(); let gj = crate::value::JSValue::from_bits(g.to_bits()); if gj.is_pointer() { - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; + // #6943: `js_string_coerce` allocates for every non-heap-string name, + // so it can trigger a GC that **evacuates**. The global object's header + // was extracted into a raw Rust local *before* the coercion and + // dereferenced by `js_object_get_field_by_name` after it. Root the + // receiver and re-derive the header from the refreshed value. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_heap_word_u64(g.to_bits()); let key = crate::builtins::js_string_coerce(name_value); + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; if !gptr.is_null() && !key.is_null() { let v = unsafe { crate::object::js_object_get_field_by_name(gptr, key) }; if !v.is_undefined() { @@ -992,7 +1000,10 @@ pub extern "C" fn js_global_get_or_throw_unresolved(name_value: f64) -> f64 { // can't tell "absent" from "present, value undefined", so confirm // the property actually exists (as an OWN property — a global var // binding always is) before falling through to the throw. - let has = crate::object::js_object_has_own(g, name_value); + let has = crate::object::js_object_has_own( + f64::from_bits(g_handle.get_heap_word_u64()), + name_value, + ); if crate::value::js_is_truthy(has) != 0 { return f64::from_bits(crate::value::JSValue::undefined().bits()); } @@ -1027,15 +1038,35 @@ pub extern "C" fn js_global_update(name_value: f64, is_increment: f64, is_prefix let is_prefix = crate::value::js_is_truthy(is_prefix) != 0; let g = crate::object::js_get_global_this(); let gj = crate::value::JSValue::from_bits(g.to_bits()); + // #6943: `js_string_coerce` allocates for every non-heap-string name, and + // the read-modify-write below adds `js_object_get_field_by_name`, + // `js_object_has_own`, `js_to_numeric` and `js_numeric_step` — every one of + // them GC-capable. The global object (`g`, and the `gptr` header derived + // from the pre-coercion `gj`) and the coerced key string were raw Rust + // locals across all of it, and `gptr` is the receiver of the WRITE-BACK at + // the end. Root both and re-derive the header at each use. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_heap_word_u64(g.to_bits()); let key = crate::builtins::js_string_coerce(name_value); + let key_handle = scope.root_string_ptr(key); let mut present = false; let old = if gj.is_pointer() && !key.is_null() { - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; if !gptr.is_null() { - let v = unsafe { crate::object::js_object_get_field_by_name(gptr, key) }; + let v = unsafe { + crate::object::js_object_get_field_by_name( + gptr, + key_handle.get_raw_const_ptr::(), + ) + }; if !v.is_undefined() || unsafe { - crate::object::js_object_has_own(g, name_value).to_bits() + crate::object::js_object_has_own( + f64::from_bits(g_handle.get_heap_word_u64()), + name_value, + ) + .to_bits() == crate::value::TAG_TRUE } { @@ -1055,10 +1086,23 @@ pub extern "C" fn js_global_update(name_value: f64, is_increment: f64, is_prefix let err_ptr = js_referenceerror_new(msg_str); return crate::exception::js_throw(crate::value::js_nanbox_pointer(err_ptr as i64)); } - let numeric = unsafe { crate::value::js_to_numeric(old) }; - let stepped = unsafe { crate::value::js_numeric_step(numeric, is_increment) }; - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; - unsafe { crate::object::js_object_set_field_by_name(gptr, key, stepped) }; + let old_handle = scope.root_nanbox_f64(old); + let numeric = unsafe { crate::value::js_to_numeric(old_handle.get_nanbox_f64()) }; + let numeric_handle = scope.root_nanbox_f64(numeric); + let stepped = + unsafe { crate::value::js_numeric_step(numeric_handle.get_nanbox_f64(), is_increment) }; + let stepped_handle = scope.root_nanbox_f64(stepped); + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; + unsafe { + crate::object::js_object_set_field_by_name( + gptr, + key_handle.get_raw_const_ptr::(), + stepped_handle.get_nanbox_f64(), + ) + }; + let numeric = numeric_handle.get_nanbox_f64(); + let stepped = stepped_handle.get_nanbox_f64(); if is_prefix { stepped } else { @@ -1090,15 +1134,35 @@ static KEEP_JS_GLOBAL_ASSIGN_EXISTING_OR_THROW: extern "C" fn(f64, f64) -> f64 = pub extern "C" fn js_global_assign_existing_or_throw(name_value: f64, value: f64) -> f64 { let g = crate::object::js_get_global_this(); let gj = crate::value::JSValue::from_bits(g.to_bits()); + // #6943: the textbook shape of this family — a receiver AND the value being + // stored into it, both raw across a GC-capable `js_string_coerce`. The + // presence probe (`js_object_get_field_by_name`, `js_object_has_own`) and + // the not-defined path (`js_string_from_bytes`, `js_referenceerror_new`) + // allocate on top of that, and `gptr` is the receiver of the final write. + // Root the global, the coerced key and `value` for the whole helper. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_heap_word_u64(g.to_bits()); + let value_handle = scope.root_nanbox_f64(value); let key = crate::builtins::js_string_coerce(name_value); + let key_handle = scope.root_string_ptr(key); let mut present = false; if gj.is_pointer() && !key.is_null() { - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; if !gptr.is_null() { - let v = unsafe { crate::object::js_object_get_field_by_name(gptr, key) }; + let v = unsafe { + crate::object::js_object_get_field_by_name( + gptr, + key_handle.get_raw_const_ptr::(), + ) + }; if !v.is_undefined() || unsafe { - crate::object::js_object_has_own(g, name_value).to_bits() + crate::object::js_object_has_own( + f64::from_bits(g_handle.get_heap_word_u64()), + name_value, + ) + .to_bits() == crate::value::TAG_TRUE } { @@ -1113,10 +1177,15 @@ pub extern "C" fn js_global_assign_existing_or_throw(name_value: f64, value: f64 let err_ptr = js_referenceerror_new(msg_str); return crate::exception::js_throw(crate::value::js_nanbox_pointer(err_ptr as i64)); } - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; - crate::object::js_object_set_field_by_name(gptr, key, value); + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *mut crate::object::ObjectHeader; + crate::object::js_object_set_field_by_name( + gptr, + key_handle.get_raw_const_ptr::(), + value_handle.get_nanbox_f64(), + ); // An assignment expression evaluates to its RHS. - value + value_handle.get_nanbox_f64() } /// Non-throwing variant of [`js_global_get_or_throw_unresolved`] for @@ -1129,8 +1198,13 @@ pub extern "C" fn js_global_get_optional(name_value: f64) -> f64 { let g = crate::object::js_get_global_this(); let gj = crate::value::JSValue::from_bits(g.to_bits()); if gj.is_pointer() { - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; + // #6943: root the global across the GC-capable coercion and re-derive + // its header afterwards — see `js_global_get_or_throw_unresolved`. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_heap_word_u64(g.to_bits()); let key = crate::builtins::js_string_coerce(name_value); + let g = f64::from_bits(g_handle.get_heap_word_u64()); + let gptr = (g.to_bits() & crate::value::POINTER_MASK) as *const crate::object::ObjectHeader; if !gptr.is_null() && !key.is_null() { let v = unsafe { crate::object::js_object_get_field_by_name(gptr, key) }; return f64::from_bits(v.bits()); diff --git a/crates/perry-runtime/src/object/array_object_ops.rs b/crates/perry-runtime/src/object/array_object_ops.rs index eeb4092c68..20cf549141 100644 --- a/crates/perry-runtime/src/object/array_object_ops.rs +++ b/crates/perry-runtime/src/object/array_object_ops.rs @@ -263,7 +263,17 @@ pub(crate) unsafe fn array_length_reflect_define( if obj.is_null() || !is_array_object(obj) { return None; } + // #6943: `js_string_coerce` allocates for every non-heap-string key and can + // run a user `toString` / `valueOf` for an object key, so it can trigger a + // GC that **evacuates**. `obj` (the array header that + // `array_set_length_from_descriptor` truncates through) and + // `descriptor_value` were raw Rust locals across the call. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let desc_handle = scope.root_nanbox_f64(descriptor_value); let key_str = crate::builtins::js_string_coerce(key_value); + let obj = obj_handle.get_raw_mut_ptr::(); + let descriptor_value = desc_handle.get_nanbox_f64(); if key_str.is_null() { return None; } diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 0ea529e559..c6e8f74fef 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -730,7 +730,17 @@ pub(crate) fn reflect_getter_closure_bits(value: f64, key: f64) -> Option { if !state().descriptors.accessors_in_use.get() { return None; } - let key_str = crate::builtins::js_string_coerce(key); + // #6943: `js_string_coerce` allocates for every non-heap-string key and can + // run a user `toString` / `valueOf` for an object key, so it can trigger a + // GC that **evacuates**. `value` (the prototype-chain walk's starting + // receiver, dereferenced by `extract_obj_ptr` below) and `key` (re-read at + // the own-property shadow check inside the loop) were raw Rust locals + // across it. Both stay rooted for the walk. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_handle = scope.root_heap_word_u64(value.to_bits()); + let key_handle = scope.root_nanbox_f64(key); + let key_str = crate::builtins::js_string_coerce(key_handle.get_nanbox_f64()); + let value = f64::from_bits(value_handle.get_heap_word_u64()); if key_str.is_null() { return None; } @@ -749,10 +759,14 @@ pub(crate) fn reflect_getter_closure_bits(value: f64, key: f64) -> Option { // some level shadows inherited accessors, so stop the walk there and let // the caller fall back to an ordinary (receiver-aware) field read. (test262 // Reflect/get/return-value-from-receiver: inherited-getter-via-receiver.) - let mut current = value; + // `current` walks the chain through its own handle: `obj_value_has_own_key` + // and `js_object_get_prototype_of` both allocate, so the link a raw local + // held could be evacuated out from under the next iteration (#6943). + let current_handle = scope.root_heap_word_u64(value.to_bits()); // Bounded to guard against a cyclic prototype side-table; real chains are // a handful of links deep. for _ in 0..10_000 { + let current = f64::from_bits(current_handle.get_heap_word_u64()); let obj = unsafe { extract_obj_ptr(current) }; if obj.is_null() { return None; @@ -768,14 +782,15 @@ pub(crate) fn reflect_getter_closure_bits(value: f64, key: f64) -> Option { }; } // An own (data) property at this level shadows any inherited accessor. - if obj_value_has_own_key(current, key) { + if obj_value_has_own_key(current, key_handle.get_nanbox_f64()) { return None; } + let current = f64::from_bits(current_handle.get_heap_word_u64()); let proto = crate::object::js_object_get_prototype_of(current); if unsafe { extract_obj_ptr(proto) }.is_null() { return None; } - current = proto; + current_handle.set_heap_word_u64(proto.to_bits()); } None } diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 0c96509d2d..a8ee8d02d0 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -192,8 +192,19 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu && crate::symbol::js_is_symbol(key_value) == 0 { if let Some(method_name) = metadata_key_to_string(key_value) { - let obj = extract_obj_ptr(obj_value); + // #6943: `js_string_coerce` allocates for every non-heap-string + // key and can run a user `toString` / `valueOf` for an object + // key, so it can trigger a GC that **evacuates**. `obj` — the + // receiver's header, resolved on the line above and + // dereferenced by `own_key_present` / `js_object_get_class_id` + // below — and `obj_value` (passed to `js_class_method_bind`) + // were raw Rust locals across the call. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_value_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let obj_handle = scope.root_raw_mut_ptr(extract_obj_ptr(obj_value)); let key_str = crate::builtins::js_string_coerce(key_value); + let obj_value = f64::from_bits(obj_value_handle.get_heap_word_u64()); + let obj = obj_handle.get_raw_mut_ptr::(); if !obj.is_null() && !key_str.is_null() && !own_key_present(obj, key_str) { let class_id = super::js_object_get_class_id(obj as *const ObjectHeader); if class_id != 0 @@ -268,7 +279,13 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu } if let Some(addr) = crate::typedarray_props::typed_array_addr_from_value(obj_value) { + // #6943: `addr` is the TypedArray's heap address, resolved before + // the GC-capable key coercion and dereferenced as a + // `TypedArrayHeader` after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let addr_handle = scope.root_raw_mut_ptr(addr as *mut u8); let key_str = crate::builtins::js_string_coerce(key_value); + let addr = addr_handle.get_raw_mut_ptr::() as usize; if key_str.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } @@ -468,7 +485,14 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu if jsv.is_pointer() { let ptr = jsv.as_pointer::() as usize; if crate::closure::is_closure_ptr(ptr) { + // #6943: `ptr` is the closure's heap address, taken from + // `obj_value` above and used all through this arm (deleted-key + // probe, attrs/accessor side-table lookups, `closure_length`, + // the `func_ptr` read) *after* the GC-capable coercion below. + let scope = crate::gc::RuntimeHandleScope::new(); + let ptr_handle = scope.root_raw_mut_ptr(ptr as *mut u8); let key_str = crate::builtins::js_string_coerce(key_value); + let ptr = ptr_handle.get_raw_mut_ptr::() as usize; if key_str.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } @@ -611,8 +635,23 @@ pub extern "C" fn js_object_get_own_property_descriptor(obj_value: f64, key_valu if obj.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } - // Extract key string - let key_str = crate::builtins::js_string_coerce(key_value); + // Extract key string. + // + // #6943: `obj` is the receiver's header, resolved on the line above and + // dereferenced below (`arguments_object_descriptor`, the GcHeader + // probe, the array/`keys_array` walks). It was a raw Rust local across + // the GC-capable coercion. The already-heap-string key — the + // overwhelmingly common `getOwnPropertyDescriptor(o, "x")` — keeps the + // pre-fix path: `js_string_coerce` returns that pointer unchanged + // without touching the allocator. + let (obj, key_str) = if crate::builtins::string_coerce_is_inert(key_value) { + (obj, crate::builtins::js_string_coerce(key_value)) + } else { + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let key_str = crate::builtins::js_string_coerce(key_value); + (obj_handle.get_raw_mut_ptr::(), key_str) + }; if key_str.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } @@ -947,7 +986,16 @@ pub(crate) unsafe fn build_accessor_descriptor( /// descriptor (writable:false, enumerable:false, configurable:false). Any /// other key is absent → undefined. unsafe fn string_primitive_descriptor(str_value: f64, key_value: f64) -> f64 { + // #6943: the receiver here is itself a heap value — `str_value` is the + // boxed/primitive string whose bytes are read below via + // `str_bytes_from_jsvalue`. It was a raw Rust local across the GC-capable + // key coercion, so an evacuating collection left it pointing at a + // forwarding stub and the index/`length` descriptor was computed from + // moved-out bytes. + let scope = crate::gc::RuntimeHandleScope::new(); + let str_handle = scope.root_heap_word_u64(str_value.to_bits()); let key_str = crate::builtins::js_string_coerce(key_value); + let str_value = f64::from_bits(str_handle.get_heap_word_u64()); if key_str.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } @@ -1354,54 +1402,95 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 crate::value::js_nanbox_get_pointer(names_value) as *const crate::array::ArrayHeader; // Fresh result object that collects { key: descriptor } entries. - // Like js_object_entries / js_object_get_own_property_names above, the - // intermediate allocations aren't rooted — Perry's builder helpers - // follow this convention. - let result = js_object_alloc(0, 0); + // + // #6943: this loop is the family's worst shape — the receiver (`result`) + // and the value being stored *into* it (`desc`) were both raw Rust + // locals across the GC-capable key coercion, so a stale `result` + // dropped the write onto a forwarding stub and a stale `desc` planted a + // dangling pointer inside a live object, where it outlives the call. + // `names_arr` is the key source the loop keeps re-reading. Root all + // three for the duration of the loop and read them back through their + // handles after every step that can allocate. The two per-entry handles + // are allocated ONCE and rewritten per iteration (`set_*`) so a + // 10k-key receiver doesn't push 20k slots onto the handle stack. + let scope = crate::gc::RuntimeHandleScope::new(); + let result_handle = scope.root_raw_mut_ptr(js_object_alloc(0, 0)); + let names_handle = scope.root_raw_mut_ptr(names_arr as *mut crate::array::ArrayHeader); + let key_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); + let desc_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); if !names_arr.is_null() { let len = crate::array::js_array_length(names_arr) as usize; for i in 0..len { + let names_arr = names_handle.get_raw_const_ptr::(); let key_val = crate::array::js_array_get(names_arr, i as u32); - let key_f64 = f64::from_bits(key_val.bits()); - let desc = js_object_get_own_property_descriptor(obj_value, key_f64); + key_handle.set_nanbox_u64(key_val.bits()); + let desc = + js_object_get_own_property_descriptor(obj_value, key_handle.get_nanbox_f64()); // Spec step: only add the entry when the descriptor is not // undefined (the key was removed between key-collection and the // descriptor read, e.g. by a Proxy trap). if desc.to_bits() == crate::value::TAG_UNDEFINED { continue; } - let key_str = crate::builtins::js_string_coerce(key_f64); + desc_handle.set_nanbox_f64(desc); + let key_str = crate::builtins::js_string_coerce(key_handle.get_nanbox_f64()); if !key_str.is_null() { - js_object_set_field_by_name(result, key_str, desc); + js_object_set_field_by_name( + result_handle.get_raw_mut_ptr::(), + key_str, + desc_handle.get_nanbox_f64(), + ); } } } - // [[OwnPropertyKeys]] includes symbol keys after the string keys, and // `Object.getOwnPropertyDescriptors` must report a descriptor for each // (including non-enumerable ones). `getOwnPropertyNames` above only // covers the string subset, so enumerate the symbol keys separately and // install each descriptor under its symbol key on the result object. // (test262 getOwnPropertyDescriptors/symbols-included, order-after-*.) - let result_value = f64::from_bits((result as u64) | POINTER_TAG); + // + // The result object stays rooted through this loop too (`result_handle` + // is still live), so `result_value` is re-derived from the handle on + // each use rather than captured once before the allocating symbol + // enumeration. + let result_value = |handle: &crate::gc::RuntimeHandle<'_>| -> f64 { + f64::from_bits((handle.get_raw_mut_ptr::() as u64) | POINTER_TAG) + }; let sym_arr_raw = crate::symbol::js_object_get_own_property_symbols(obj_value); if sym_arr_raw != 0 { - let sym_arr = sym_arr_raw as *const crate::array::ArrayHeader; - if !sym_arr.is_null() { - let slen = crate::array::js_array_length(sym_arr) as usize; + let sym_handle = scope.root_raw_mut_ptr(sym_arr_raw as *mut crate::array::ArrayHeader); + if !sym_handle + .get_raw_const_ptr::() + .is_null() + { + let slen = crate::array::js_array_length( + sym_handle.get_raw_const_ptr::(), + ) as usize; for i in 0..slen { - let sym_val = crate::array::js_array_get(sym_arr, i as u32); - let sym_f64 = f64::from_bits(sym_val.bits()); - let desc = js_object_get_own_property_descriptor(obj_value, sym_f64); + let sym_val = crate::array::js_array_get( + sym_handle.get_raw_const_ptr::(), + i as u32, + ); + key_handle.set_nanbox_u64(sym_val.bits()); + let desc = js_object_get_own_property_descriptor( + obj_value, + key_handle.get_nanbox_f64(), + ); if desc.to_bits() == crate::value::TAG_UNDEFINED { continue; } - crate::symbol::js_object_set_symbol_property(result_value, sym_f64, desc); + desc_handle.set_nanbox_f64(desc); + crate::symbol::js_object_set_symbol_property( + result_value(&result_handle), + key_handle.get_nanbox_f64(), + desc_handle.get_nanbox_f64(), + ); } } } - result_value + result_value(&result_handle) } } diff --git a/crates/perry-runtime/src/object/native_call_method/common_methods.rs b/crates/perry-runtime/src/object/native_call_method/common_methods.rs index ff2cdaf8c2..2ca886376a 100644 --- a/crates/perry-runtime/src/object/native_call_method/common_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/common_methods.rs @@ -89,7 +89,14 @@ pub(super) unsafe fn dispatch_common( return Some(f64::from_bits(JSValue::bool(present).bits())); } if jsval.is_pointer() { + // #6943: `js_string_coerce` allocates for every non-heap-string + // key, so it can trigger a GC that **evacuates** the receiver. + // `object` — and the `jsval` tag view taken from it at the top + // of this function — are raw locals; re-read them through the + // caller's `object_handle`, which IS a root. let key_str = crate::builtins::js_string_coerce(key_value); + let object = object_handle.get_nanbox_f64(); + let jsval = JSValue::from_bits(object.to_bits()); if key_str.is_null() { return Some(f64::from_bits(JSValue::bool(false).bits())); } @@ -233,7 +240,10 @@ pub(super) unsafe fn dispatch_common( object, key_value, )); } + // #6943: root the receiver across the GC-capable coercion — see + // the `hasOwnProperty` arm above. let key_str = crate::builtins::js_string_coerce(key_value); + let jsval = JSValue::from_bits(object_handle.get_nanbox_f64().to_bits()); if key_str.is_null() { return Some(f64::from_bits(JSValue::bool(false).bits())); } diff --git a/crates/perry-runtime/src/object/object_ops/define_property.rs b/crates/perry-runtime/src/object/object_ops/define_property.rs index 8a30373834..9688451c4f 100644 --- a/crates/perry-runtime/src/object/object_ops/define_property.rs +++ b/crates/perry-runtime/src/object/object_ops/define_property.rs @@ -603,7 +603,24 @@ pub extern "C" fn js_object_define_property( } return obj_value; } + // #6943: `js_string_coerce` on an object key runs a user + // `toString` / `valueOf`, and allocates the stringified form for + // every primitive key — either can trigger a GC that **evacuates** + // live objects. `obj_value` (the receiver), `descriptor_value`, and + // the already-dereferenced `closure_ptr` were raw Rust locals + // across the call — neither GC roots nor shadow slots. A stale + // receiver rebinds the accessors onto a forwarding stub; a stale + // `closure_ptr` files the property under a dead address, where the + // matching read can never find it. Root all three across the + // coercion and read them back through their handles. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let desc_handle = scope.root_nanbox_f64(descriptor_value); + let closure_handle = scope.root_raw_mut_ptr(closure_ptr as *mut u8); let key_str = crate::builtins::js_string_coerce(key_value); + let obj_value = f64::from_bits(obj_handle.get_heap_word_u64()); + let descriptor_value = desc_handle.get_nanbox_f64(); + let closure_ptr = closure_handle.get_raw_mut_ptr::() as usize; if key_str.is_null() { return obj_value; } @@ -787,7 +804,18 @@ pub extern "C" fn js_object_define_property( ); return obj_value; } + // #6943: same GC-capable coercion as the closure arm above. Here + // the raw local at risk is `addr` — the TypedArray's heap address, + // resolved from `obj_value` *before* the coercion and dereferenced + // as a `TypedArrayHeader` after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let desc_handle = scope.root_nanbox_f64(descriptor_value); + let addr_handle = scope.root_raw_mut_ptr(addr as *mut u8); let key_str = crate::builtins::js_string_coerce(key_value); + let obj_value = f64::from_bits(obj_handle.get_heap_word_u64()); + let descriptor_value = desc_handle.get_nanbox_f64(); + let addr = addr_handle.get_raw_mut_ptr::() as usize; if key_str.is_null() { return obj_value; } @@ -889,8 +917,22 @@ pub extern "C" fn js_object_define_property( return obj_value; } } - // Extract key string + // Extract key string. + // + // #6943: the ordinary arm's raw local is `obj` — the receiver's + // `ObjectHeader`, resolved above and dereferenced below (class-id + // probe, typed-array define, `define_array_property`, the keys_array + // walk). It, `obj_value` and `descriptor_value` are rooted across the + // GC-capable coercion; see the closure arm above for the full + // reasoning. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let obj_value_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let desc_handle = scope.root_nanbox_f64(descriptor_value); let key_str = crate::builtins::js_string_coerce(key_value); + let obj = obj_handle.get_raw_mut_ptr::(); + let obj_value = f64::from_bits(obj_value_handle.get_heap_word_u64()); + let descriptor_value = desc_handle.get_nanbox_f64(); if key_str.is_null() { return obj_value; } diff --git a/crates/perry-runtime/src/object/object_ops/from_entries.rs b/crates/perry-runtime/src/object/object_ops/from_entries.rs index 55aa5a4732..e83391c91f 100644 --- a/crates/perry-runtime/src/object/object_ops/from_entries.rs +++ b/crates/perry-runtime/src/object/object_ops/from_entries.rs @@ -171,16 +171,37 @@ pub extern "C" fn js_object_from_entries(entries_value: f64) -> f64 { return f64::from_bits(crate::value::TAG_UNDEFINED); } + // #6943: `js_string_coerce` on an entry key allocates for every + // non-heap-string shape and runs a user `toString` / `valueOf` for an + // object key, so it can trigger a GC that **evacuates**. The result + // object `obj` (the receiver), `val_val` (the value being written INTO + // it) and `arr_ptr` (the entry source re-read every iteration) were all + // raw Rust locals across the call — a stale receiver drops the write on + // a forwarding stub and a stale value plants a dangling pointer inside + // a live object. The three handles are allocated once and rewritten per + // iteration so a long entry list doesn't grow the handle stack. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let arr_handle = scope.root_raw_mut_ptr(arr_ptr as *mut crate::array::ArrayHeader); + let val_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); for i in 0..length { - let entry_val = crate::array::js_array_get_f64(arr_ptr, i as u32); + let entry_val = crate::array::js_array_get_f64( + arr_handle.get_raw_const_ptr::(), + i as u32, + ); let (key_val, val_val) = object_from_entries_entry_values(entry_val); + val_handle.set_nanbox_f64(val_val); let key_str = crate::builtins::js_string_coerce(key_val); if key_str.is_null() { continue; } - js_object_set_field_by_name(obj, key_str, val_val); + js_object_set_field_by_name( + obj_handle.get_raw_mut_ptr::(), + key_str, + val_handle.get_nanbox_f64(), + ); } - crate::value::js_nanbox_pointer(obj as i64) + crate::value::js_nanbox_pointer(obj_handle.get_raw_mut_ptr::() as i64) } } diff --git a/crates/perry-runtime/src/object/object_ops/has_own.rs b/crates/perry-runtime/src/object/object_ops/has_own.rs index f8982afc82..45e6130727 100644 --- a/crates/perry-runtime/src/object/object_ops/has_own.rs +++ b/crates/perry-runtime/src/object/object_ops/has_own.rs @@ -132,7 +132,31 @@ pub extern "C" fn js_object_has_own(obj_value: f64, key_value: f64) -> f64 { return f64::from_bits(if present { TAG_TRUE } else { TAG_FALSE }); } - let key_str = crate::builtins::js_string_coerce(key_value); + // #6943: `js_string_coerce` allocates for every non-heap-string key and + // runs a user `toString` / `valueOf` for an object key, so it can + // trigger a GC that **evacuates**. `obj_value` — and the `obj_js` tag + // view derived from it at the top of this function — were raw Rust + // locals across the call, and every arm below dereferences one or the + // other. The already-heap-string key, which is what + // `o.hasOwnProperty("x")` compiles to for names past the SSO bound, + // keeps the pre-fix path verbatim. + let (obj_value, obj_js, key_str) = if crate::builtins::string_coerce_is_inert(key_value) { + ( + obj_value, + obj_js, + crate::builtins::js_string_coerce(key_value), + ) + } else { + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let key_str = crate::builtins::js_string_coerce(key_value); + let obj_value = f64::from_bits(obj_handle.get_heap_word_u64()); + ( + obj_value, + crate::JSValue::from_bits(obj_value.to_bits()), + key_str, + ) + }; if key_str.is_null() { return f64::from_bits(TAG_FALSE); } @@ -473,7 +497,16 @@ pub extern "C" fn js_object_property_is_enumerable(obj_value: f64, key_value: f6 return f64::from_bits(if enumerable { TAG_TRUE } else { TAG_FALSE }); } - let key_str = crate::builtins::js_string_coerce(key_value); + // #6943: root the receiver across the GC-capable key coercion — see + // `js_object_has_own` above for the full reasoning. + let (obj_value, key_str) = if crate::builtins::string_coerce_is_inert(key_value) { + (obj_value, crate::builtins::js_string_coerce(key_value)) + } else { + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); + let key_str = crate::builtins::js_string_coerce(key_value); + (f64::from_bits(obj_handle.get_heap_word_u64()), key_str) + }; if key_str.is_null() { return f64::from_bits(TAG_FALSE); } diff --git a/crates/perry-runtime/src/object/reflect_support.rs b/crates/perry-runtime/src/object/reflect_support.rs index 8df5f79930..e12e36a4fa 100644 --- a/crates/perry-runtime/src/object/reflect_support.rs +++ b/crates/perry-runtime/src/object/reflect_support.rs @@ -56,7 +56,15 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool { // typed arrays are plain-`alloc`ed without a `GcHeader`, so reading // `addr - 8` is allocator-metadata garbage. if crate::typedarray::lookup_typed_array_kind(obj_addr).is_some() { + // #6943: `js_string_coerce` allocates for every non-heap-string key + // and runs a user `toString` / `valueOf` for an object key, so it + // can trigger a GC that **evacuates**. `obj` was resolved from + // `value` before the call and is dereferenced as a + // `TypedArrayHeader` after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); let key_str = crate::builtins::js_string_coerce(key); + let obj = obj_handle.get_raw_mut_ptr::(); if key_str.is_null() { return false; } @@ -74,7 +82,12 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool { if arr.is_null() { return false; } + // #6943: `arr` is the (tag-cleaned) array header, resolved + // before the GC-capable coercion and walked after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let arr_handle = scope.root_raw_const_ptr(arr); let key_str = crate::builtins::js_string_coerce(key); + let arr = arr_handle.get_raw_const_ptr::(); if key_str.is_null() { return false; } @@ -107,7 +120,19 @@ pub(crate) fn obj_value_has_own_key(value: f64, key: f64) -> bool { } } } - let key_str = crate::builtins::js_string_coerce(key); + // #6943: the ordinary arm dereferences `obj` for its `keys_array` + // *after* the GC-capable coercion, so the receiver is rooted across it. + // An already-heap-string key — the common `Reflect.defineProperty(o, + // "x", …)` shape — keeps the pre-fix path: `js_string_coerce` returns + // that pointer unchanged without touching the allocator. + let (obj, key_str) = if crate::builtins::string_coerce_is_inert(key) { + (obj, crate::builtins::js_string_coerce(key)) + } else { + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let key_str = crate::builtins::js_string_coerce(key); + (obj_handle.get_raw_mut_ptr::(), key_str) + }; if key_str.is_null() { return false; } @@ -135,7 +160,16 @@ pub(crate) fn obj_value_attrs(value: f64, key: f64) -> Option<(bool, bool)> { if obj.is_null() { return None; } + // #6943: `key_to_rust_string` runs the GC-capable `js_string_coerce`, + // and `obj as usize` is the descriptor side table's OWNER KEY. A stale + // address doesn't crash here — it silently misses, so a + // `Reflect.defineProperty` on a non-configurable property would report + // the all-true default and let the redefine through. Root the receiver + // across the coercion. (Not in #6943's site list; found by reading.) + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); let k = key_to_rust_string(key)?; + let obj = obj_handle.get_raw_mut_ptr::(); super::get_property_attrs(obj as usize, &k).map(|a| (a.writable(), a.configurable())) } } diff --git a/crates/perry-runtime/src/object/typed_array_define.rs b/crates/perry-runtime/src/object/typed_array_define.rs index 4d3bf640f8..b701eb8834 100644 --- a/crates/perry-runtime/src/object/typed_array_define.rs +++ b/crates/perry-runtime/src/object/typed_array_define.rs @@ -130,6 +130,13 @@ unsafe fn field_present(desc: *mut ObjectHeader, name: &[u8]) -> bool { /// If `key_value` is a String key that is a CanonicalNumericIndexString, return /// its numeric value. Returns `None` for symbols, non-string keys, and strings /// that aren't canonical numeric indices (those go through ordinary semantics). +/// +/// #6943 names this function's `js_string_coerce` call as an unrooted-operand +/// site, but the coercion here is self-contained: the key bytes are consumed +/// into an owned `String`/`f64` before returning and nothing heap-referencing +/// spans the call *within* this function. The hazard is at the two CALLERS +/// below, which resolve the TypedArray's raw address before calling in and +/// dereference it afterwards — that is where the rooting lives. unsafe fn canonical_index_for_key(key_value: f64) -> Option { if crate::symbol::js_is_symbol(key_value) != 0 { return None; @@ -162,9 +169,17 @@ pub(crate) unsafe fn typed_array_own_index(obj_value: f64, key_value: f64) -> Ty let Some((addr, is_buf, length)) = typed_array_view_info(obj_value) else { return TypedArrayOwnIndex::NotTypedArray; }; + // #6943: `canonical_index_for_key` runs `js_string_coerce`, which allocates + // for every non-heap-string key and can run a user `toString` / `valueOf` + // for an object key — so it can trigger a GC that **evacuates**. `addr` is + // the view's raw heap address, resolved on the line above and dereferenced + // as a `TypedArrayHeader` / `BufferHeader` below. + let scope = crate::gc::RuntimeHandleScope::new(); + let addr_handle = scope.root_raw_mut_ptr(addr as *mut u8); let Some(numeric_index) = canonical_index_for_key(key_value) else { return TypedArrayOwnIndex::NotTypedArray; }; + let addr = addr_handle.get_raw_mut_ptr::() as usize; if !is_valid_integer_index(numeric_index, length) { return TypedArrayOwnIndex::OutOfBounds; } @@ -193,10 +208,19 @@ pub(crate) unsafe fn typed_array_define_own_property( return TypedArrayDefineOutcome::NotTypedArray; }; + // #6943: same GC-capable coercion as `typed_array_own_index` above. Here + // BOTH the view address `addr` (written through at the element store) and + // `descriptor_value` (whose `value` field is the payload being stored) were + // raw Rust locals across it. + let scope = crate::gc::RuntimeHandleScope::new(); + let addr_handle = scope.root_raw_mut_ptr(addr as *mut u8); + let desc_handle = scope.root_nanbox_f64(descriptor_value); let Some(numeric_index) = canonical_index_for_key(key_value) else { // Symbol / non-string / non-canonical key → ordinary define handles it. return TypedArrayDefineOutcome::NotTypedArray; }; + let addr = addr_handle.get_raw_mut_ptr::() as usize; + let descriptor_value = desc_handle.get_nanbox_f64(); // Canonical numeric index → integer-indexed branch. From here every path // returns Rejected or Defined; we never fall back to ordinary define. diff --git a/crates/perry-runtime/src/object/with_env.rs b/crates/perry-runtime/src/object/with_env.rs index 7baf9db029..eccb2c3594 100644 --- a/crates/perry-runtime/src/object/with_env.rs +++ b/crates/perry-runtime/src/object/with_env.rs @@ -149,9 +149,16 @@ pub extern "C" fn js_with_implicit_read(value: f64, name: f64) -> f64 { // (`var o = {foo:1}; with (o) { foo = 42; } foo` — with/12.10-0-7). let g = crate::object::js_get_global_this(); if js_is_truthy(crate::object::js_object_has_property(g, name)) != 0 { - let gj = JSValue::from_bits(g.to_bits()); - let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const ObjectHeader; + // #6943: `js_string_coerce` allocates for every non-heap-string + // name, so it can trigger a GC that **evacuates**. `gptr` — the + // global object's header, extracted on the line above and + // dereferenced by `js_object_get_field_by_name` below — was a raw + // Rust local across the call. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_heap_word_u64(g.to_bits()); let key = crate::builtins::js_string_coerce(name); + let gj = JSValue::from_bits(g_handle.get_heap_word_u64()); + let gptr = (gj.bits() & crate::value::POINTER_MASK) as *const ObjectHeader; if !gptr.is_null() && !key.is_null() { let v = unsafe { crate::object::js_object_get_field_by_name(gptr, key) }; return f64::from_bits(v.bits()); diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 2cd5250dde..4c6bbb6f5c 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -811,6 +811,11 @@ fn target_set(target: f64, key: f64, value: f64) { } return; } + // #6943 audit: this `js_string_coerce` is provably INERT and needs no + // rooting. `js_to_property_key` returns either a Symbol — taken by the + // early return above — or `js_nanbox_string(heap_ptr)`, i.e. an + // already-heap `STRING_TAG` value, which `js_string_coerce` hands straight + // back without touching the allocator. let key_ptr = crate::builtins::js_string_coerce(property_key) as *const crate::StringHeader; if crate::object::class_ref_id(target).is_some() { // Preserve the INT32-tagged class-ref bits so class dynamic props @@ -1324,13 +1329,18 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) && header._reserved & SLOW_FLAGS == 0 { let class_id = (*(addr as *const crate::ObjectHeader)).class_id; - let fast_safe = if class_id == 0 { + let (fast_safe, target, value) = if class_id == 0 { // Plain object: prototype is exactly Object.prototype, and // Object.prototype doesn't intercept this key (per-key, not // the coarse process-wide descriptor flag — that made wide // builds O(n²)). - crate::object::prototype_chain::object_static_prototype(addr).is_none() - && !crate::object::object_proto_may_intercept_key(key) + ( + crate::object::prototype_chain::object_static_prototype(addr) + .is_none() + && !crate::object::object_proto_may_intercept_key(key), + target, + value, + ) } else { // `DisposableStack#disposed` is a getter-only // builtin accessor on a reserved native prototype. @@ -1369,8 +1379,41 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // class chain — SLOW_FLAGS above already excluded // frozen/sealed/descriptor bits; add the per-instance // divergence flags (setPrototypeOf override / null proto). + // + // #6943: `js_string_coerce` is GC-capable for every + // key shape EXCEPT an already-heap `STRING_TAG` + // one (it hands that pointer straight back without + // touching the allocator) — an SSO short key + // materializes onto the heap, a numeric key builds + // its stringification, and an object key runs a + // user `toString` / `valueOf`. Any of those can + // trigger a GC that **evacuates**. `addr` (the + // receiver, dereferenced twice for `plan_eligible` + // and passed to `class_instance_set_may_intercept`), + // `target` and the `value` about to be written + // INTO it were all raw Rust locals across the call. + // The heap-string key — what `obj.field = v` + // lowers to for any name longer than the SSO + // bound — keeps the pre-fix path and pays nothing. + let inert = crate::builtins::string_coerce_is_inert(key); + let scope = (!inert).then(crate::gc::RuntimeHandleScope::new); + let roots = scope.as_ref().map(|s| { + ( + s.root_heap_word_u64(target.to_bits()), + s.root_nanbox_f64(value), + s.root_raw_mut_ptr(addr as *mut u8), + ) + }); let key_ptr = crate::builtins::js_string_coerce(key) as *const crate::StringHeader; + let (target, value, addr) = match &roots { + Some((t, v, a)) => ( + f64::from_bits(t.get_heap_word_u64()), + v.get_nanbox_f64(), + a.get_raw_mut_ptr::() as usize, + ), + None => (target, value, addr), + }; let interned = crate::object::interned_key_ptr(key_ptr); // #6595: a per-evaluation CLASS OBJECT (what a // capture-carrying class materializes as, @@ -1394,7 +1437,7 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) && (*(addr as *const crate::ObjectHeader)).object_type == crate::error::OBJECT_TYPE_REGULAR && interned != 0; - if plan_eligible + let verdict = if plan_eligible && crate::object::prop_plan::store_plan_check(class_id, interned) { true @@ -1406,7 +1449,8 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) crate::object::prop_plan::store_plan_record(class_id, interned); } clear - } + }; + (verdict, target, value) }; if fast_safe { target_set(target, key, value); diff --git a/crates/perry-runtime/src/symbol/properties.rs b/crates/perry-runtime/src/symbol/properties.rs index 2d7487d75f..86b467e74a 100644 --- a/crates/perry-runtime/src/symbol/properties.rs +++ b/crates/perry-runtime/src/symbol/properties.rs @@ -514,7 +514,15 @@ pub unsafe extern "C" fn js_class_register_static_symbol(class_id: u32, sym: f64 // is a TypeError per ClassDefinitionEvaluation; anything else // becomes an ordinary own static data property (numeric keys, a // computed "constructor", drizzle-style `static [name] = v`). + // #6943: `js_string_coerce` allocates for every non-heap-string key and + // runs a user `toString` / `valueOf` for an object key, so it can + // trigger a GC that **evacuates**. `value` is the static field's stored + // payload — it goes straight into `class_dynamic_prop_root_store` + // below, so a stale one plants a dangling pointer in a live side table. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_handle = scope.root_nanbox_f64(value); let key_str = crate::builtins::js_string_coerce(sym); + let value = value_handle.get_nanbox_f64(); if key_str.is_null() { return; } From 6b58c0a9feb39304de8785c6da2bf6fb60960c48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:33:49 +0200 Subject: [PATCH 02/10] test(gc): #6943 forced-evacuation guard for js_string_coerce property-key paths --- ...string_coerce_property_key_rooting_6943.rs | 527 ++++++++++++++++++ 1 file changed, 527 insertions(+) create mode 100644 crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs diff --git a/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs b/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs new file mode 100644 index 0000000000..40a263723c --- /dev/null +++ b/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs @@ -0,0 +1,527 @@ +//! Regression tests for #6943 — raw receivers and raw *stored values* held +//! across a `js_string_coerce` call that is being used as the property-key +//! coercion. +//! +//! Third and last family in the unrooted-operand-across-GC-capable-coercion +//! series (#6655/#6934 dynamic arith, #6935/#6941 `ToPropertyKey`). These entry +//! points never call `js_to_property_key`; they stringify the key with +//! `js_string_coerce` directly: +//! +//! ```ignore +//! let obj = extract_obj_ptr(obj_value); // receiver, raw local +//! let key_str = js_string_coerce(key_value); // user JS -> allocate -> GC +//! own_key_present(obj, key_str); // stale receiver +//! ``` +//! +//! `js_string_coerce` returns early — with no allocation at all — for an +//! already-heap `STRING_TAG` value. Every other shape allocates: an SSO short +//! string materializes onto the heap, a number/bool/null/BigInt builds its +//! stringification, and a `POINTER_TAG` object routes through +//! `js_jsvalue_to_string`, which invokes a user `toString` / `valueOf`. Any of +//! those can trigger a GC that **evacuates** live objects, and a Rust local is +//! neither a GC root nor a shadow slot. +//! +//! The programs run with `PERRY_GC_FORCE_EVACUATE=1` (stress-copies every +//! marked non-pinned nursery object) and `PERRY_GC_VERIFY_EVACUATION=1` (panics +//! if a live slot still points at a forwarded object). Stored payloads are heap +//! objects whose fields are read back **after a further collection**, so a +//! stale store shows up as a wrong field value rather than passing by luck. +//! +//! ## What this suite does and does not prove +//! +//! Same caveat as the #6935 suite, and it must not be overstated: **these tests +//! pass on the pre-fix runtime too.** No in-language configuration currently +//! reaches the state the bug needs — a *minor* cycle that evacuates while the +//! raw runtime locals are unpinned: +//! +//! * The `gc()` hook these programs call runs a **full mark-sweep**, and +//! evacuation is minor-only, so nothing moves (#6946). +//! * `perry/gc`'s `minor()` does evacuate but engages +//! `ManualGcScanGuard::force_full_scan()`, whose conservative stack scan pins +//! exactly the raw receiver/value locals this bug is about (#4977, #6942). +//! * `minor()` + `PERRY_CONSERVATIVE_STACK_SCAN=off` evacuates them, but that +//! combination is independently unsound on this build, so any failure it +//! produces is uninterpretable. #6941's agent got an apparent repro that way +//! and retracted it after a control reproduced with AND without the fix. +//! +//! So this is a **behavioral guard**: it pins the observable semantics of every +//! rooted path under the strongest GC stress the language surface can express +//! today, and it starts failing the day a minor-evacuating configuration +//! becomes reachable from compiled code. It is not evidence that the pre-fix +//! runtime was reproducibly wrong; the audit in #6943 is. +//! +//! Coverage: `js_object_define_property` (ordinary / closure / typed-array +//! arms), `js_object_get_own_property_descriptor` (ordinary / closure / +//! typed-array / class-object / string-primitive arms), +//! `js_object_get_own_property_descriptors`, `obj_value_has_own_key`, +//! `obj_value_attrs`, `reflect_getter_closure_bits`, +//! `array_length_reflect_define`, `typed_array_own_index`, +//! `typed_array_define_own_property`, `js_object_from_entries`, +//! `js_object_has_own`, `js_object_property_is_enumerable`, the +//! `ordinary_set_with_receiver` class-instance store fast path, +//! `js_class_register_static_symbol`'s non-symbol arm, and the +//! `globalThis`-by-name helpers in `error.rs`. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run_forced_evacuation(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + // The runtime-only macOS link path does not pass `-framework CoreFoundation`, + // but `perry-runtime` pulls `iana_time_zone`, which references `_CFRelease` + // & co. Append the framework through the supported escape hatch so this + // suite links regardless (same shim as the #6655 / #6935 suites). + let mut compile_cmd = Command::new(perry_bin()); + compile_cmd + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache"); + if cfg!(target_os = "macos") { + let extra = match std::env::var("PERRY_EXTRA_LINK_ARGS") { + Ok(existing) if !existing.trim().is_empty() => { + format!("{existing} -framework CoreFoundation") + } + _ => "-framework CoreFoundation".to_string(), + }; + compile_cmd.env("PERRY_EXTRA_LINK_ARGS", extra); + } + let compile = compile_cmd.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) + .current_dir(dir) + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1") + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed under forced evacuation (exit {:?})\nstdout:\n{}\nstderr:\n{}", + run.status.code(), + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// Shared prelude. `heavyKey` is the object-shaped key whose `toString` runs +/// user JS that allocates and collects — that is the arm of `js_string_coerce` +/// that can run arbitrary code. `numericKey` and `ssoKey` cover the two +/// allocate-only arms (stringify a Number, materialize an SSO short string), +/// which are the shapes a real program hits far more often. +const PRELUDE: &str = r#" +function churnAndCollect(): void { + let sink = 0; + for (let i = 0; i < 20000; i++) { + const tmp = { i, s: "pad" + i }; + sink += tmp.s.length > 0 ? 1 : 0; + } + if (sink !== 20000) throw new Error("churn miscounted"); + (globalThis as any).gc?.(); + // Refill: evacuation leaves the vacated region intact-but-dead, so a stale + // read right after a copy usually still finds the original bytes. Re-filling + // gives that region a chance to be handed out and overwritten. + for (let i = 0; i < 20000; i++) { + const tmp2 = { a: i, b: "fill" + i, c: [i, i + 1] }; + sink += tmp2.c[0] >= 0 ? 1 : 0; + } + if (sink !== 40000) throw new Error("refill miscounted"); +} + +// Keeps receivers / payloads reachable from a real GC root. An object reachable +// only through the (unrooted) raw local is DEAD at the collection and would +// merely be swept, so a stale read might find intact bytes. Reachable objects +// are EVACUATED — the address genuinely changes and every rooted holder is +// rewritten, while the raw local is not. +const keepalive: any[] = []; + +// Object key: `js_string_coerce` -> `js_jsvalue_to_string` -> user `toString`. +function heavyKey(name: string): any { + const o: any = { + n: name, + toString(): string { + churnAndCollect(); + return this.n; + }, + }; + keepalive.push(o); + return o; +} + +// The STORED VALUE: a movable heap object carrying an identity we read back +// after a further collection. +function payload(tag: number): any { + const o: any = { tag: tag, arr: [tag, tag + 1], s: "payload-" + tag }; + keepalive.push(o); + return o; +} + +// A fresh receiver that is rooted (so it is evacuated, not swept). +function receiver(): any { + const o: any = { seed: 1 }; + keepalive.push(o); + return o; +} + +let failures = 0; +function check(name: string, got: any, want: any): void { + if (got !== want) { + failures++; + console.log("FAIL " + name + " got=" + String(got) + " want=" + String(want)); + } +} +function checkPayload(name: string, got: any, tag: number): void { + if (got === undefined || got === null) { + failures++; + console.log("FAIL " + name + " payload missing"); + return; + } + check(name + ".tag", got.tag, tag); + check(name + ".arr0", got.arr[0], tag); + check(name + ".arr1", got.arr[1], tag + 1); + check(name + ".s", got.s, "payload-" + tag); +} +"#; + +/// `Object.defineProperty` / `Object.defineProperties`: the receiver, the +/// descriptor object and the already-dereferenced header (plain `ObjectHeader`, +/// closure pointer, TypedArray address) are all raw across the key coercion, +/// and the descriptor's `value` is written INTO the receiver afterwards. +#[test] +fn define_property_operands_survive_forced_evacuation() { + let dir = tempfile::tempdir().expect("tempdir"); + let source = format!( + "{PRELUDE}{}", + r#" +// --- ordinary object arm (define_property.rs, the `obj` header local) --- +const o1: any = receiver(); +Object.defineProperty(o1, heavyKey("p1"), { + value: payload(1), + writable: true, + enumerable: true, + configurable: true, +}); +churnAndCollect(); +checkPayload("define-ordinary", o1.p1, 1); +check("define-ordinary-enumerable", Object.keys(o1).indexOf("p1") >= 0, true); + +// A NUMBER key stringifies (allocates) without running user JS — the arm a +// real program hits constantly. +const o2: any = receiver(); +Object.defineProperty(o2, 12345, { value: payload(2), configurable: true }); +churnAndCollect(); +checkPayload("define-numeric-key", o2[12345], 2); + +// --- closure arm (the `closure_ptr` local) --- +function fn1(): number { return 1; } +keepalive.push(fn1); +Object.defineProperty(fn1, heavyKey("meta"), { + value: payload(3), + configurable: true, +}); +churnAndCollect(); +checkPayload("define-on-closure", (fn1 as any).meta, 3); +check("define-on-closure-callable", fn1(), 1); + +// --- typed-array arm (the `addr` local) --- +const ta = new Int32Array([7, 8, 9]); +keepalive.push(ta); +Object.defineProperty(ta, heavyKey("label"), { + value: payload(4), + configurable: true, +}); +churnAndCollect(); +checkPayload("define-on-typed-array", (ta as any).label, 4); +check("typed-array-elements-intact", ta[0] + ta[1] + ta[2], 24); + +// A CANONICAL numeric index on a typed array takes the Integer-Indexed exotic +// define — `typed_array_define_own_property`, whose view address was raw. +Object.defineProperty(ta, 1, { value: 42 }); +churnAndCollect(); +check("typed-array-index-define", ta[1], 42); + +// --- non-configurable redefine must still be rejected after a collection --- +const frozenish: any = receiver(); +Object.defineProperty(frozenish, "locked", { + value: 1, + configurable: false, + writable: false, +}); +let threw = false; +try { + Object.defineProperty(frozenish, heavyKey("locked"), { value: 2 }); +} catch (e) { + threw = true; +} +churnAndCollect(); +check("nonconfigurable-redefine-rejected", threw, true); +check("nonconfigurable-value-intact", frozenish.locked, 1); + +// --- Object.defineProperties over a bag of descriptors --- +const bag: any = receiver(); +Object.defineProperties(bag, { + x: { value: payload(5), enumerable: true, configurable: true }, + y: { value: payload(6), enumerable: true, configurable: true }, +}); +churnAndCollect(); +checkPayload("define-properties-x", bag.x, 5); +checkPayload("define-properties-y", bag.y, 6); + +console.log("failures:", failures); +"# + ); + let stdout = compile_and_run_forced_evacuation(dir.path(), &source); + assert_eq!( + stdout, "failures: 0\n", + "a defineProperty path used a stale receiver or stored a stale value \ + under forced evacuation" + ); +} + +/// `Object.getOwnPropertyDescriptor(s)`: every arm resolves the receiver's raw +/// header (plain object, closure, TypedArray view, class object, string +/// primitive) before the key coercion and dereferences it afterwards. The +/// plural form additionally stores each descriptor INTO a fresh result object. +#[test] +fn descriptor_reads_survive_forced_evacuation() { + let dir = tempfile::tempdir().expect("tempdir"); + let source = format!( + "{PRELUDE}{}", + r#" +// --- ordinary object arm --- +const o1: any = receiver(); +o1.here = payload(10); +churnAndCollect(); +const d1 = Object.getOwnPropertyDescriptor(o1, heavyKey("here")); +churnAndCollect(); +checkPayload("gopd-ordinary", d1 === undefined ? undefined : d1.value, 10); +check("gopd-missing-key", Object.getOwnPropertyDescriptor(o1, heavyKey("nope")), undefined); + +// --- closure arm (the `ptr` local drives every branch below the coercion) --- +function fn1(a: number, b: number): number { return a + b; } +keepalive.push(fn1); +churnAndCollect(); +const dLen = Object.getOwnPropertyDescriptor(fn1, heavyKey("length")); +churnAndCollect(); +check("gopd-closure-length", dLen === undefined ? undefined : dLen.value, 2); +const dName = Object.getOwnPropertyDescriptor(fn1, heavyKey("name")); +churnAndCollect(); +check("gopd-closure-name", dName === undefined ? undefined : dName.value, "fn1"); + +// --- typed-array arm --- +const ta = new Int32Array([3, 4, 5]); +keepalive.push(ta); +churnAndCollect(); +const dIdx = Object.getOwnPropertyDescriptor(ta, heavyKey("1")); +churnAndCollect(); +check("gopd-typed-array-index", dIdx === undefined ? undefined : dIdx.value, 4); +check("gopd-typed-array-oob", Object.getOwnPropertyDescriptor(ta, heavyKey("99")), undefined); + +// --- string primitive arm (`string_primitive_descriptor`, whose `str_value` +// receiver is itself a movable heap string) --- +const s: any = "abcdef" + String(keepalive.length % 1); +keepalive.push(s); +churnAndCollect(); +const dChar = Object.getOwnPropertyDescriptor(s, heavyKey("2")); +churnAndCollect(); +check("gopd-string-index", dChar === undefined ? undefined : dChar.value, "c"); +const dSLen = Object.getOwnPropertyDescriptor(s, heavyKey("length")); +churnAndCollect(); +check("gopd-string-length", dSLen === undefined ? undefined : dSLen.value, 6); + +// --- class-object static arm --- +class C { + static stat(): number { return 5; } +} +keepalive.push(C); +churnAndCollect(); +const dStat = Object.getOwnPropertyDescriptor(C, heavyKey("stat")); +churnAndCollect(); +check("gopd-class-static-present", typeof dStat === "object" && dStat !== null, true); +check( + "gopd-class-static-callable", + dStat === undefined ? undefined : typeof dStat.value, + "function" +); + +// --- getOwnPropertyDescriptors: result receiver + stored descriptor --- +const multi: any = receiver(); +multi.a = payload(11); +multi.b = payload(12); +multi.c = payload(13); +churnAndCollect(); +const all: any = Object.getOwnPropertyDescriptors(multi); +churnAndCollect(); +checkPayload("gopds-a", all.a === undefined ? undefined : all.a.value, 11); +checkPayload("gopds-b", all.b === undefined ? undefined : all.b.value, 12); +checkPayload("gopds-c", all.c === undefined ? undefined : all.c.value, 13); +check("gopds-seed-present", all.seed === undefined ? undefined : all.seed.value, 1); + +console.log("failures:", failures); +"# + ); + let stdout = compile_and_run_forced_evacuation(dir.path(), &source); + assert_eq!( + stdout, "failures: 0\n", + "a getOwnPropertyDescriptor path used a stale receiver under forced evacuation" + ); +} + +/// The `Reflect.*` support predicates plus the remaining string-coerced key +/// entry points: `obj_value_has_own_key` / `obj_value_attrs` (whose stale +/// receiver ADDRESS silently misses the descriptor side table rather than +/// crashing), `array_length_reflect_define`, `reflect_getter_closure_bits`, +/// `Object.fromEntries`, and the own-property predicates. +#[test] +fn reflect_and_own_key_paths_survive_forced_evacuation() { + let dir = tempfile::tempdir().expect("tempdir"); + let source = format!( + "{PRELUDE}{}", + r#" +// --- Reflect.defineProperty -> obj_value_has_own_key + obj_value_attrs --- +const o1: any = receiver(); +check( + "reflect-define-new", + Reflect.defineProperty(o1, heavyKey("fresh"), { + value: payload(20), + configurable: true, + }), + true +); +churnAndCollect(); +checkPayload("reflect-define-new-value", o1.fresh, 20); + +// A non-configurable existing property must make the redefine report false. +// `obj_value_attrs` keys the side table by the receiver's ADDRESS; a stale one +// misses and would wrongly report the all-true default, letting this through. +Object.defineProperty(o1, "pinned", { value: 1, configurable: false }); +churnAndCollect(); +check( + "reflect-define-nonconfigurable", + Reflect.defineProperty(o1, heavyKey("pinned"), { value: 2 }), + false +); +check("reflect-define-nonconfigurable-intact", o1.pinned, 1); + +// Non-extensible receiver: a brand-new key must report false. +const sealed: any = receiver(); +Object.preventExtensions(sealed); +churnAndCollect(); +check( + "reflect-define-non-extensible", + Reflect.defineProperty(sealed, heavyKey("nope"), { value: 3 }), + false +); + +// --- array `length` exotic define (array_length_reflect_define) --- +const arr: any[] = [1, 2, 3, 4, 5]; +keepalive.push(arr); +churnAndCollect(); +check( + "reflect-array-length-define", + Reflect.defineProperty(arr, heavyKey("length"), { value: 2 }), + true +); +churnAndCollect(); +check("reflect-array-length-applied", arr.length, 2); +check("reflect-array-length-head", arr[0], 1); + +// --- Reflect.get through an inherited accessor (reflect_getter_closure_bits, +// which walks the prototype chain with `value`/`key` raw across the coercion). +const base: any = {}; +Object.defineProperty(base, "acc", { + get(): any { return this.backing; }, + configurable: true, +}); +keepalive.push(base); +const derived: any = Object.create(base); +derived.backing = payload(21); +keepalive.push(derived); +churnAndCollect(); +checkPayload("reflect-get-inherited-accessor", Reflect.get(base, "acc", derived), 21); + +// --- Object.fromEntries: fresh result receiver + stored value across the +// per-entry key coercion. +const built: any = Object.fromEntries([ + [heavyKey("e1"), payload(22)], + [heavyKey("e2"), payload(23)], + [24680, payload(24)], +]); +keepalive.push(built); +churnAndCollect(); +checkPayload("from-entries-e1", built.e1, 22); +checkPayload("from-entries-e2", built.e2, 23); +checkPayload("from-entries-numeric", built[24680], 24); + +// --- own-property predicates (js_object_has_own / propertyIsEnumerable) --- +const probe: any = receiver(); +probe.present = payload(25); +churnAndCollect(); +check("hasOwn-object-key", Object.hasOwn(probe, heavyKey("present")), true); +check("hasOwn-object-key-miss", Object.hasOwn(probe, heavyKey("absent")), false); +check("hasOwnProperty-object-key", probe.hasOwnProperty(heavyKey("present")), true); +check( + "propertyIsEnumerable-object-key", + probe.propertyIsEnumerable(heavyKey("present")), + true +); +// Numeric keys take the allocate-only arm of the same coercion. +probe[86420] = payload(26); +churnAndCollect(); +check("hasOwn-numeric-key", Object.hasOwn(probe, 86420), true); +checkPayload("probe-still-intact", probe.present, 25); + +// --- class-instance store fast path (ordinary_set_with_receiver): SSO short +// keys materialize onto the heap inside `js_string_coerce`, so the receiver and +// the value were raw across an allocation on the dominant `obj.f = v` path. +class Holder { + v: any; + constructor(v: any) { this.v = v; } +} +const h: any = new Holder(payload(27)); +keepalive.push(h); +for (let i = 0; i < 200; i++) { + h["k" + (i % 3)] = payload(30 + (i % 3)); +} +churnAndCollect(); +checkPayload("class-instance-store-k0", h.k0, 30); +checkPayload("class-instance-store-k1", h.k1, 31); +checkPayload("class-instance-store-k2", h.k2, 32); +checkPayload("class-instance-ctor-field", h.v, 27); + +// --- class static computed field with a NON-symbol key +// (js_class_register_static_symbol's string arm stores `value` across the +// coercion). +const staticName: any = heavyKey("computedStatic"); +class WithStatic { + static [staticName] = payload(28); +} +keepalive.push(WithStatic); +churnAndCollect(); +checkPayload("class-static-computed-field", (WithStatic as any).computedStatic, 28); + +console.log("failures:", failures); +"# + ); + let stdout = compile_and_run_forced_evacuation(dir.path(), &source); + assert_eq!( + stdout, "failures: 0\n", + "a Reflect / own-key path used a stale receiver or stored a stale value \ + under forced evacuation" + ); +} From d80bc5d2aec263aa2b6ac46a38c15c4f92a63502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:38:01 +0200 Subject: [PATCH 03/10] changelog: #6948 js_string_coerce property-key rooting fragment --- ...6948-string-coerce-property-key-rooting.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 changelog.d/6948-string-coerce-property-key-rooting.md diff --git a/changelog.d/6948-string-coerce-property-key-rooting.md b/changelog.d/6948-string-coerce-property-key-rooting.md new file mode 100644 index 0000000000..f38fecf1ae --- /dev/null +++ b/changelog.d/6948-string-coerce-property-key-rooting.md @@ -0,0 +1,51 @@ +**GC rooting: the `js_string_coerce`-as-property-key family (#6943).** Third and last known family in +the unrooted-operand-across-GC-capable-coercion series (after #6934's dynamic arith and #6941's +`ToPropertyKey`). A set of property entry points stringify the key with `js_string_coerce` directly, +without an earlier `ToPropertyKey` — and held the receiver, and on the write paths the value about to +be stored, as raw Rust locals across it. `js_string_coerce` allocates for every key shape except an +already-heap `STRING_TAG` one (an SSO short key materializes onto the heap, a numeric key builds its +stringification, an object key runs a user `toString`/`valueOf`), and an allocation can trigger a GC +that **evacuates** live objects. A Rust local is neither a GC root nor a shadow slot, so a stale +receiver dropped the write onto a forwarding stub and a stale stored value planted a dangling pointer +inside a live object, where it outlived the call. + +Fixed with the established idiom — `crate::gc::RuntimeHandleScope` plus +`root_heap_word_u64`/`root_raw_mut_ptr`/`root_nanbox_f64`/`root_string_ptr`, re-reading each operand +through its handle after the coercion — across: + +- `object/object_ops/define_property.rs` — `Object.defineProperty`'s closure, typed-array and + ordinary arms (the receiver, the descriptor, and the already-dereferenced `closure_ptr` / + TypedArray address / `ObjectHeader`). +- `object/descriptors.rs` — `getOwnPropertyDescriptor`'s class-object / typed-array / closure / + ordinary arms, `string_primitive_descriptor` (whose receiver is itself a movable heap string), and + `getOwnPropertyDescriptors` (result receiver + each stored descriptor). +- `object/descriptor_state.rs` — `reflect_getter_closure_bits`, including the prototype-walk cursor. +- `object/reflect_support.rs` — `obj_value_has_own_key`'s three arms plus `obj_value_attrs`, where a + stale receiver **address** silently misses the descriptor side table instead of crashing, so a + `Reflect.defineProperty` on a non-configurable property could slip through. +- `object/array_object_ops.rs` — `array_length_reflect_define`. +- `object/typed_array_define.rs` — `typed_array_own_index` and `typed_array_define_own_property`. The + issue named the shared `canonical_index_for_key` helper; reading it showed the helper is clean and + the hazard is at these two callers, which resolve the view address before the coercion and + dereference it after. +- `proxy.rs` — the class-instance store fast path in `ordinary_set_with_receiver` (`obj.f = v`), with + an inert-key fast path so the common already-heap-string key keeps the pre-fix code path verbatim. +- `object/object_ops/has_own.rs`, `object/native_call_method/common_methods.rs` — + `hasOwnProperty` / `propertyIsEnumerable` in both their entry-point and method-call forms. +- `object/object_ops/from_entries.rs` — `Object.fromEntries` (fresh result receiver **and** the entry + value written into it). +- `symbol/properties.rs` — `js_class_register_static_symbol`'s non-symbol arm (the stored payload). +- `object/with_env.rs`, `error.rs` — the `globalThis`-by-name helpers, whose window spans a read, a + `ToNumeric`/step, and a write-back. + +New shared predicate `builtins::string_coerce_is_inert(value)`, the `js_string_coerce` analogue of +#6941's `property_key_coercion_is_inert`, justified by `js_string_coerce`'s own `is_string()` early +return. Hot surfaces are gated on it so an already-heap-string key pays nothing. + +`proxy.rs`'s `target_set` was audited and is provably inert (its argument is always a +`js_to_property_key` result, i.e. an already-heap string); a comment now records that so the next +sweep doesn't re-examine it. `crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs` adds +three forced-evacuation behavioral guards. As with both predecessors, no deterministic pre-fix +failure is reachable from compiled code today — `gc()` runs a full mark-sweep and pins raw locals via +the conservative stack scan (#6946), and `perry/gc`'s `minor()` engages the same scan (#6942) — so +the suite is a guard, not a red-to-green regression test, and its module doc says so. From aed3b001c1c1af7fdb8ed1c2efab949ce3da659d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:41:48 +0200 Subject: [PATCH 04/10] fix(runtime): root the plain-object arm of the proxy store fast path too (#6943) object_proto_may_intercept_key reaches obj_value_has_own_key, which performs the same js_string_coerce, so the class_id == 0 arm held target/value raw across a GC-capable coercion as well. Hoist the single optional scope above both arms. --- Cargo.lock | 2 - crates/perry-runtime/src/proxy.rs | 85 ++++++++++++++++--------------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 187c1fd352..12fb90bd86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8781,14 +8781,12 @@ checksum = "b1afc06e45b0d63943c777d0233523fa2e23a12431a73c37b1c0366777b31717" dependencies = [ "calendrical_calculations", "core_maths", - "iana-time-zone", "icu_calendar", "icu_locale_core", "ixdtf", "num-traits", "timezone_provider", "tinystr", - "web-time", "writeable", ] diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 4c6bbb6f5c..7ed57c993f 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -1329,18 +1329,39 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) && header._reserved & SLOW_FLAGS == 0 { let class_id = (*(addr as *const crate::ObjectHeader)).class_id; - let (fast_safe, target, value) = if class_id == 0 { + // #6943: BOTH arms below reach a GC-capable + // `js_string_coerce` on `key` — the class arm calls it + // directly for the store-plan key, and the plain-object + // arm reaches it through + // `object_proto_may_intercept_key` -> + // `obj_value_has_own_key`. The coercion is inert only + // for an already-heap `STRING_TAG` key; an SSO short + // key materializes onto the heap, a numeric key builds + // its stringification, and an object key runs a user + // `toString` / `valueOf`. Any of those can trigger a GC + // that **evacuates**, and `addr` (the receiver, + // dereferenced for `plan_eligible` and passed to + // `class_instance_set_may_intercept`), `target` and the + // `value` about to be written INTO it were all raw Rust + // locals across it. The heap-string key — what + // `obj.field = v` lowers to for any name past the SSO + // bound — takes no scope and keeps the pre-fix path. + let scope = (!crate::builtins::string_coerce_is_inert(key)) + .then(crate::gc::RuntimeHandleScope::new); + let roots = scope.as_ref().map(|s| { + ( + s.root_heap_word_u64(target.to_bits()), + s.root_nanbox_f64(value), + s.root_raw_mut_ptr(addr as *mut u8), + ) + }); + let fast_safe = if class_id == 0 { // Plain object: prototype is exactly Object.prototype, and // Object.prototype doesn't intercept this key (per-key, not // the coarse process-wide descriptor flag — that made wide // builds O(n²)). - ( - crate::object::prototype_chain::object_static_prototype(addr) - .is_none() - && !crate::object::object_proto_may_intercept_key(key), - target, - value, - ) + crate::object::prototype_chain::object_static_prototype(addr).is_none() + && !crate::object::object_proto_may_intercept_key(key) } else { // `DisposableStack#disposed` is a getter-only // builtin accessor on a reserved native prototype. @@ -1379,40 +1400,13 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // class chain — SLOW_FLAGS above already excluded // frozen/sealed/descriptor bits; add the per-instance // divergence flags (setPrototypeOf override / null proto). - // - // #6943: `js_string_coerce` is GC-capable for every - // key shape EXCEPT an already-heap `STRING_TAG` - // one (it hands that pointer straight back without - // touching the allocator) — an SSO short key - // materializes onto the heap, a numeric key builds - // its stringification, and an object key runs a - // user `toString` / `valueOf`. Any of those can - // trigger a GC that **evacuates**. `addr` (the - // receiver, dereferenced twice for `plan_eligible` - // and passed to `class_instance_set_may_intercept`), - // `target` and the `value` about to be written - // INTO it were all raw Rust locals across the call. - // The heap-string key — what `obj.field = v` - // lowers to for any name longer than the SSO - // bound — keeps the pre-fix path and pays nothing. - let inert = crate::builtins::string_coerce_is_inert(key); - let scope = (!inert).then(crate::gc::RuntimeHandleScope::new); - let roots = scope.as_ref().map(|s| { - ( - s.root_heap_word_u64(target.to_bits()), - s.root_nanbox_f64(value), - s.root_raw_mut_ptr(addr as *mut u8), - ) - }); let key_ptr = crate::builtins::js_string_coerce(key) as *const crate::StringHeader; - let (target, value, addr) = match &roots { - Some((t, v, a)) => ( - f64::from_bits(t.get_heap_word_u64()), - v.get_nanbox_f64(), - a.get_raw_mut_ptr::() as usize, - ), - None => (target, value, addr), + // #6943: re-read the receiver through its handle — + // everything below dereferences it. + let addr = match &roots { + Some((_, _, a)) => a.get_raw_mut_ptr::() as usize, + None => addr, }; let interned = crate::object::interned_key_ptr(key_ptr); // #6595: a per-evaluation CLASS OBJECT (what a @@ -1450,7 +1444,16 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) } clear }; - (verdict, target, value) + verdict + }; + // #6943: the store itself takes the refreshed receiver + // and payload — both were rooted across whichever + // coercion the arm above performed. + let (target, value) = match &roots { + Some((t, v, _)) => { + (f64::from_bits(t.get_heap_word_u64()), v.get_nanbox_f64()) + } + None => (target, value), }; if fast_safe { target_set(target, key, value); From d6824430e3c0f6fd906586fdd588e42bda1a433e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:43:06 +0200 Subject: [PATCH 05/10] changelog: record the proxy plain-object arm in the #6948 fragment --- changelog.d/6948-string-coerce-property-key-rooting.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/changelog.d/6948-string-coerce-property-key-rooting.md b/changelog.d/6948-string-coerce-property-key-rooting.md index f38fecf1ae..1ab3a9780c 100644 --- a/changelog.d/6948-string-coerce-property-key-rooting.md +++ b/changelog.d/6948-string-coerce-property-key-rooting.md @@ -28,8 +28,10 @@ through its handle after the coercion — across: issue named the shared `canonical_index_for_key` helper; reading it showed the helper is clean and the hazard is at these two callers, which resolve the view address before the coercion and dereference it after. -- `proxy.rs` — the class-instance store fast path in `ordinary_set_with_receiver` (`obj.f = v`), with - an inert-key fast path so the common already-heap-string key keeps the pre-fix code path verbatim. +- `proxy.rs` — the store fast path in `ordinary_set_with_receiver` (`obj.f = v`). The issue named the + class-instance arm; the plain-object arm reaches the same coercion transitively through + `object_proto_may_intercept_key` → `obj_value_has_own_key`, so one optional scope now covers both. + An inert-key check keeps the common already-heap-string key on the pre-fix code path verbatim. - `object/object_ops/has_own.rs`, `object/native_call_method/common_methods.rs` — `hasOwnProperty` / `propertyIsEnumerable` in both their entry-point and method-call forms. - `object/object_ops/from_entries.rs` — `Object.fromEntries` (fresh result receiver **and** the entry From 178dafe1f1881f9347c7853a1b727fe4fb27085c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:45:45 +0200 Subject: [PATCH 06/10] fix(runtime): root the key/entry array before the result allocation (#6943) js_object_alloc is itself GC-capable, so rooting names_arr / arr_ptr after it rooted an already-stale pointer. Found reviewing the first patch. --- .../perry-runtime/src/object/descriptors.rs | 14 +++++++--- .../src/object/object_ops/from_entries.rs | 27 ++++++++++--------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index a8ee8d02d0..6d384c0f53 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1413,14 +1413,22 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 // handles after every step that can allocate. The two per-entry handles // are allocated ONCE and rewritten per iteration (`set_*`) so a // 10k-key receiver doesn't push 20k slots onto the handle stack. + // `names_arr` is rooted BEFORE the result allocation: `js_object_alloc` + // is itself GC-capable, so rooting the key array after it would root an + // already-stale pointer. let scope = crate::gc::RuntimeHandleScope::new(); - let result_handle = scope.root_raw_mut_ptr(js_object_alloc(0, 0)); let names_handle = scope.root_raw_mut_ptr(names_arr as *mut crate::array::ArrayHeader); + let result_handle = scope.root_raw_mut_ptr(js_object_alloc(0, 0)); let key_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); let desc_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); - if !names_arr.is_null() { - let len = crate::array::js_array_length(names_arr) as usize; + if !names_handle + .get_raw_const_ptr::() + .is_null() + { + let len = crate::array::js_array_length( + names_handle.get_raw_const_ptr::(), + ) as usize; for i in 0..len { let names_arr = names_handle.get_raw_const_ptr::(); let key_val = crate::array::js_array_get(names_arr, i as u32); diff --git a/crates/perry-runtime/src/object/object_ops/from_entries.rs b/crates/perry-runtime/src/object/object_ops/from_entries.rs index e83391c91f..50863a0e1c 100644 --- a/crates/perry-runtime/src/object/object_ops/from_entries.rs +++ b/crates/perry-runtime/src/object/object_ops/from_entries.rs @@ -165,24 +165,27 @@ pub extern "C" fn js_object_from_entries(entries_value: f64) -> f64 { let arr_ptr = object_from_entries_materialize_entries(entries_value); let length = crate::array::js_array_length(arr_ptr) as usize; - // Allocate empty object — class_id 0 = generic object - let obj = js_object_alloc(0, length as u32); - if obj.is_null() { - return f64::from_bits(crate::value::TAG_UNDEFINED); - } - // #6943: `js_string_coerce` on an entry key allocates for every // non-heap-string shape and runs a user `toString` / `valueOf` for an // object key, so it can trigger a GC that **evacuates**. The result - // object `obj` (the receiver), `val_val` (the value being written INTO - // it) and `arr_ptr` (the entry source re-read every iteration) were all - // raw Rust locals across the call — a stale receiver drops the write on - // a forwarding stub and a stale value plants a dangling pointer inside - // a live object. The three handles are allocated once and rewritten per + // object (the receiver), `val_val` (the value being written INTO it) + // and `arr_ptr` (the entry source re-read every iteration) were all raw + // Rust locals across the call — a stale receiver drops the write on a + // forwarding stub and a stale value plants a dangling pointer inside a + // live object. The per-entry handle is allocated once and rewritten per // iteration so a long entry list doesn't grow the handle stack. + // + // `arr_ptr` is rooted BEFORE the result allocation: `js_object_alloc` + // is itself GC-capable, so rooting the entry array after it would root + // an already-stale pointer. let scope = crate::gc::RuntimeHandleScope::new(); - let obj_handle = scope.root_raw_mut_ptr(obj); let arr_handle = scope.root_raw_mut_ptr(arr_ptr as *mut crate::array::ArrayHeader); + + // Allocate empty object — class_id 0 = generic object + let obj_handle = scope.root_raw_mut_ptr(js_object_alloc(0, length as u32)); + if obj_handle.get_raw_mut_ptr::().is_null() { + return f64::from_bits(crate::value::TAG_UNDEFINED); + } let val_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); for i in 0..length { let entry_val = crate::array::js_array_get_f64( From 7aadd95eeac69a731f28154089ea3fb568830424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:47:48 +0200 Subject: [PATCH 07/10] style: rustfmt gc/mod.rs and temporal/now.rs (pre-existing main drift from #6939) Unrelated to #6943. `cargo fmt --all -- --check` has been red on main since 83a6767ff, which blocks the required `lint` gate on every open PR. Pure rustfmt output, no semantic change. Drop this commit if it is being fixed on main directly. --- crates/perry-runtime/src/gc/mod.rs | 3 ++- crates/perry-runtime/src/temporal/now.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index 1b9dcf7315..383e4ce562 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -135,7 +135,8 @@ pub(super) fn gc_collect_minor_with_trigger(trigger: GcTriggerSnapshot) -> GcCol // live bytes exceed K× the last full's live set (belt-and-suspenders for // callers that reach a minor outside the budgeted pressure path). if arena_growth_full_escalation_due() { - let outcome = gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(trigger.kind)); + let outcome = + gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(trigger.kind)); restore_minor_in_alloc(prev_in_alloc); return outcome; } diff --git a/crates/perry-runtime/src/temporal/now.rs b/crates/perry-runtime/src/temporal/now.rs index 280a8093f8..ed814590fe 100644 --- a/crates/perry-runtime/src/temporal/now.rs +++ b/crates/perry-runtime/src/temporal/now.rs @@ -9,9 +9,9 @@ use super::dispatch::{self, ok_or_throw, raw_arg, string}; use super::{alloc_temporal_cell, TemporalValue}; use temporal_rs::host::{HostClock, HostHooks, HostTimeZone}; +use temporal_rs::now::Now; use temporal_rs::provider::TimeZoneProvider; use temporal_rs::unix_time::EpochNanoseconds; -use temporal_rs::now::Now; use temporal_rs::{TemporalError, TemporalResult, TimeZone}; /// Perry's own `Temporal.Now` host system, replacing temporal_rs's @@ -42,7 +42,10 @@ impl HostTimeZone for PerryHostSystem { &self, provider: &(impl TimeZoneProvider + ?Sized), ) -> TemporalResult { - TimeZone::try_from_identifier_str_with_provider(crate::date::host_time_zone_name(), provider) + TimeZone::try_from_identifier_str_with_provider( + crate::date::host_time_zone_name(), + provider, + ) } } @@ -70,9 +73,7 @@ fn tz_arg(v: f64) -> Option { } pub fn instant(_args: &[f64]) -> f64 { - alloc_temporal_cell(TemporalValue::Instant(ok_or_throw( - perry_now().instant(), - ))) + alloc_temporal_cell(TemporalValue::Instant(ok_or_throw(perry_now().instant()))) } pub fn time_zone_id(_args: &[f64]) -> f64 { From a44940e5d59aa154b252d2245c0e2157f010a973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:50:53 +0200 Subject: [PATCH 08/10] fix(runtime): re-read the TypedArray view after the descriptor's ToNumber (#6943) typed_array_define_own_property's element store sits after js_string_from_bytes and an OrdinaryToPrimitive that runs user JS; the view address was still the pre-coercion raw copy. Reuses the handle already rooted in this function. --- crates/perry-runtime/src/object/typed_array_define.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/perry-runtime/src/object/typed_array_define.rs b/crates/perry-runtime/src/object/typed_array_define.rs index b701eb8834..03db375217 100644 --- a/crates/perry-runtime/src/object/typed_array_define.rs +++ b/crates/perry-runtime/src/object/typed_array_define.rs @@ -267,6 +267,12 @@ pub(crate) unsafe fn typed_array_define_own_property( } else { value }; + // #6943: `js_string_from_bytes` above allocated, and the + // `OrdinaryToPrimitive` just above ran USER JS — either can have + // evacuated the view since `addr` was last read. Re-read it through the + // handle before writing the element; the store is the whole point of + // this branch, so a stale address here writes into a forwarding stub. + let addr = addr_handle.get_raw_mut_ptr::() as usize; let idx = numeric_index as u32; if is_buf { let n = crate::value::JSValue::from_bits(primitive.to_bits()).to_number(); From 9ad67dd16e667fe1c72e0456e23967208f59a2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 05:55:46 +0200 Subject: [PATCH 09/10] chore: drop the incidental Cargo.lock rewrite from this branch Any cargo invocation on this workspace rewrites temporal_rs's dep list (main's lock is stale relative to the manifests); it is not caused by #6943, so keep it out of this diff. --- Cargo.lock | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 12fb90bd86..187c1fd352 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8781,12 +8781,14 @@ checksum = "b1afc06e45b0d63943c777d0233523fa2e23a12431a73c37b1c0366777b31717" dependencies = [ "calendrical_calculations", "core_maths", + "iana-time-zone", "icu_calendar", "icu_locale_core", "ixdtf", "num-traits", "timezone_provider", "tinystr", + "web-time", "writeable", ] From 61a2ae482f0de091f48ef856486c662e6edee219 Mon Sep 17 00:00:00 2001 From: Ralph Date: Tue, 28 Jul 2026 21:22:32 -0700 Subject: [PATCH 10/10] fix(runtime): root the property key and four more operands missed in the first pass (#6943) Review of the first pass (CodeRabbit + a re-read) found five more operands in this same family that the fix itself left raw: - proxy.rs ordinary_set_with_receiver: the KEY. An object key is a POINTER_TAG heap value and is exactly the shape whose user toString can evacuate it; it was re-used at the interception check and at target_set. The comment that produced the gap framed the inert case as the interesting one; it now states the rule (non-inert => every surviving operand is rooted, and there are four). - proxy.rs own_set_descriptor: the receiver, whose raw address keys the ACCESSOR_DESCRIPTORS / PROPERTY_DESCRIPTORS tables. A stale address does not crash, it silently misses, so a rejected [[Set]] would go through. The null guard stays before the coercion so no user toString becomes newly observable. - has_own.rs js_object_property_is_enumerable: the obj_jv tag view, left on pre-coercion bits while later arms test and dereference it. - descriptors.rs js_object_get_own_property_descriptors: the enumerated receiver, re-entered on every iteration of both loops. - define_property.rs: key_value, passed raw into the obj_value_has_own_key fallback which re-coerces it. These are MISSED rootings, not regressions: main has the same holes, so the branch does not make anything worse, but it would have shipped sites this change claims to fix. Also corrects the suite's gopd-string-length expectation (the receiver was "abcdef0", 7 chars, not 6) and adds object-key coverage on the class-instance and plain-object store lanes. --- ...6948-string-coerce-property-key-rooting.md | 11 ++- .../perry-runtime/src/object/descriptors.rs | 17 +++- .../src/object/object_ops/define_property.rs | 6 ++ .../src/object/object_ops/has_own.rs | 19 +++- crates/perry-runtime/src/proxy.rs | 98 ++++++++++++++----- ...string_coerce_property_key_rooting_6943.rs | 20 +++- 6 files changed, 135 insertions(+), 36 deletions(-) diff --git a/changelog.d/6948-string-coerce-property-key-rooting.md b/changelog.d/6948-string-coerce-property-key-rooting.md index 1ab3a9780c..c25c24cbaf 100644 --- a/changelog.d/6948-string-coerce-property-key-rooting.md +++ b/changelog.d/6948-string-coerce-property-key-rooting.md @@ -41,8 +41,15 @@ through its handle after the coercion — across: `ToNumeric`/step, and a write-back. New shared predicate `builtins::string_coerce_is_inert(value)`, the `js_string_coerce` analogue of -#6941's `property_key_coercion_is_inert`, justified by `js_string_coerce`'s own `is_string()` early -return. Hot surfaces are gated on it so an already-heap-string key pays nothing. +the `property_key_coercion_is_inert` predicate from #6941, justified by `js_string_coerce`'s own +`is_string()` early return. Hot surfaces are gated on it so an already-heap-string key pays nothing. + +Also rooted, from review of the first pass: the property KEY itself at the `ordinary_set_with_receiver` +store lane (an object key is a heap value and is exactly the shape whose user `toString` can evacuate +it); `own_set_descriptor`'s receiver, whose raw address keys the descriptor side tables; the `obj_jv` +tag view in `js_object_property_is_enumerable`; the enumerated receiver across both loops of +`getOwnPropertyDescriptors`; and `key_value` where `js_object_define_property`'s fallback re-coerces +it through `obj_value_has_own_key`. `proxy.rs`'s `target_set` was audited and is provably inert (its argument is always a `js_to_property_key` result, i.e. an already-heap string); a comment now records that so the next diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 6d384c0f53..67a53b02f2 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1418,6 +1418,11 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 // already-stale pointer. let scope = crate::gc::RuntimeHandleScope::new(); let names_handle = scope.root_raw_mut_ptr(names_arr as *mut crate::array::ArrayHeader); + // The ENUMERATED receiver is re-entered on every iteration of both + // loops below, across descriptor allocation, a key coercion that can + // run user `toString`, and `js_object_set_field_by_name`. It needs a + // root just as much as the result object does. + let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); let result_handle = scope.root_raw_mut_ptr(js_object_alloc(0, 0)); let key_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); let desc_handle = scope.root_nanbox_f64(f64::from_bits(crate::value::TAG_UNDEFINED)); @@ -1433,8 +1438,10 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 let names_arr = names_handle.get_raw_const_ptr::(); let key_val = crate::array::js_array_get(names_arr, i as u32); key_handle.set_nanbox_u64(key_val.bits()); - let desc = - js_object_get_own_property_descriptor(obj_value, key_handle.get_nanbox_f64()); + let desc = js_object_get_own_property_descriptor( + f64::from_bits(obj_handle.get_heap_word_u64()), + key_handle.get_nanbox_f64(), + ); // Spec step: only add the entry when the descriptor is not // undefined (the key was removed between key-collection and the // descriptor read, e.g. by a Proxy trap). @@ -1466,7 +1473,9 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 let result_value = |handle: &crate::gc::RuntimeHandle<'_>| -> f64 { f64::from_bits((handle.get_raw_mut_ptr::() as u64) | POINTER_TAG) }; - let sym_arr_raw = crate::symbol::js_object_get_own_property_symbols(obj_value); + let sym_arr_raw = crate::symbol::js_object_get_own_property_symbols(f64::from_bits( + obj_handle.get_heap_word_u64(), + )); if sym_arr_raw != 0 { let sym_handle = scope.root_raw_mut_ptr(sym_arr_raw as *mut crate::array::ArrayHeader); if !sym_handle @@ -1483,7 +1492,7 @@ pub extern "C" fn js_object_get_own_property_descriptors(obj_value: f64) -> f64 ); key_handle.set_nanbox_u64(sym_val.bits()); let desc = js_object_get_own_property_descriptor( - obj_value, + f64::from_bits(obj_handle.get_heap_word_u64()), key_handle.get_nanbox_f64(), ); if desc.to_bits() == crate::value::TAG_UNDEFINED { diff --git a/crates/perry-runtime/src/object/object_ops/define_property.rs b/crates/perry-runtime/src/object/object_ops/define_property.rs index 9688451c4f..2e3951353e 100644 --- a/crates/perry-runtime/src/object/object_ops/define_property.rs +++ b/crates/perry-runtime/src/object/object_ops/define_property.rs @@ -929,10 +929,16 @@ pub extern "C" fn js_object_define_property( let obj_handle = scope.root_raw_mut_ptr(obj); let obj_value_handle = scope.root_heap_word_u64(obj_value.to_bits()); let desc_handle = scope.root_nanbox_f64(descriptor_value); + // `key_value` is rooted too: the non-indexable fallback far below + // passes it RAW into `obj_value_has_own_key`, which re-coerces it. An + // object / BigInt key evacuated by the coercion on the next line would + // be dereferenced again there. + let key_handle = scope.root_nanbox_f64(key_value); let key_str = crate::builtins::js_string_coerce(key_value); let obj = obj_handle.get_raw_mut_ptr::(); let obj_value = f64::from_bits(obj_value_handle.get_heap_word_u64()); let descriptor_value = desc_handle.get_nanbox_f64(); + let key_value = key_handle.get_nanbox_f64(); if key_str.is_null() { return obj_value; } diff --git a/crates/perry-runtime/src/object/object_ops/has_own.rs b/crates/perry-runtime/src/object/object_ops/has_own.rs index 45e6130727..0a38c3db3c 100644 --- a/crates/perry-runtime/src/object/object_ops/has_own.rs +++ b/crates/perry-runtime/src/object/object_ops/has_own.rs @@ -499,13 +499,26 @@ pub extern "C" fn js_object_property_is_enumerable(obj_value: f64, key_value: f6 // #6943: root the receiver across the GC-capable key coercion — see // `js_object_has_own` above for the full reasoning. - let (obj_value, key_str) = if crate::builtins::string_coerce_is_inert(key_value) { - (obj_value, crate::builtins::js_string_coerce(key_value)) + // `obj_jv` must be re-derived alongside `obj_value`: it is the tag view + // taken at the top of this function, and the arms below both TEST it + // (`is_any_string`) and DEREFERENCE it (`as_pointer`), so leaving it on + // pre-coercion bits reintroduces exactly the hazard this change closes. + let (obj_value, obj_jv, key_str) = if crate::builtins::string_coerce_is_inert(key_value) { + ( + obj_value, + obj_jv, + crate::builtins::js_string_coerce(key_value), + ) } else { let scope = crate::gc::RuntimeHandleScope::new(); let obj_handle = scope.root_heap_word_u64(obj_value.to_bits()); let key_str = crate::builtins::js_string_coerce(key_value); - (f64::from_bits(obj_handle.get_heap_word_u64()), key_str) + let obj_value = f64::from_bits(obj_handle.get_heap_word_u64()); + ( + obj_value, + crate::JSValue::from_bits(obj_value.to_bits()), + key_str, + ) }; if key_str.is_null() { return f64::from_bits(TAG_FALSE); diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 7ed57c993f..59fa630748 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -1027,11 +1027,29 @@ fn own_set_descriptor(target: f64, key: f64) -> Option { return Some(OwnSetDescriptor::Data { writable }); } + // The null-receiver guard stays BEFORE the coercion: `key_to_rust_string` + // can run a user `toString`, and moving it earlier would make that side + // effect observable on a path that previously short-circuited. + if extract_pointer(target.to_bits()) as usize == 0 { + return None; + } + // #6943: `key_to_rust_string` runs the GC-capable `js_string_coerce`, and + // `obj_ptr` is BOTH a dereferenced heap address (the typed-array probe) and + // the raw ADDRESS KEY of the `ACCESSOR_DESCRIPTORS` / + // `PROPERTY_DESCRIPTORS` side tables. A stale one does not crash: it + // silently misses, so a non-writable own property or a setter-less accessor + // reads back as "no descriptor" and the [[Set]] that should have been + // rejected goes through. Root the receiver across the coercion and derive + // the address afterwards. (Found in the same review pass as the `key` gap + // below.) + let scope = crate::gc::RuntimeHandleScope::new(); + let target_handle = scope.root_heap_word_u64(target.to_bits()); + let key_name = key_to_rust_string(key)?; + let target = f64::from_bits(target_handle.get_heap_word_u64()); let obj_ptr = extract_pointer(target.to_bits()) as usize; if obj_ptr == 0 { return None; } - let key_name = key_to_rust_string(key)?; // A typed array keeps its ordinary (non-index) own expando properties and // their descriptors in the typed-array side tables, which the generic // address-keyed lookups below skip (`object_has_descriptors` is @@ -1339,13 +1357,27 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // key materializes onto the heap, a numeric key builds // its stringification, and an object key runs a user // `toString` / `valueOf`. Any of those can trigger a GC - // that **evacuates**, and `addr` (the receiver, - // dereferenced for `plan_eligible` and passed to - // `class_instance_set_may_intercept`), `target` and the - // `value` about to be written INTO it were all raw Rust - // locals across it. The heap-string key — what - // `obj.field = v` lowers to for any name past the SSO - // bound — takes no scope and keeps the pre-fix path. + // that **evacuates**. + // + // The rule is: when the coercion is NOT inert, every + // operand that outlives it must be rooted and re-read + // afterwards. That is four of them, not three — + // `addr` (the receiver, dereferenced for `plan_eligible` + // and passed to `class_instance_set_may_intercept`), + // `target` and the `value` about to be written INTO it, + // and **`key` itself**, which is re-used at the + // interception check and again at `target_set`. An + // object key is a `POINTER_TAG` heap value and is + // exactly the shape that runs the user JS which can + // evacuate it. (`key` was missed in the first pass of + // this fix; caught in review.) + // + // Only an already-heap `STRING_TAG` key is inert — it + // is handed straight back with no allocation — so that + // one shape takes no scope. Every other shape does: an + // SSO short key materializes onto the heap, a numeric + // key builds its stringification, and an object key + // runs a user `toString` / `valueOf`. let scope = (!crate::builtins::string_coerce_is_inert(key)) .then(crate::gc::RuntimeHandleScope::new); let roots = scope.as_ref().map(|s| { @@ -1353,8 +1385,27 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) s.root_heap_word_u64(target.to_bits()), s.root_nanbox_f64(value), s.root_raw_mut_ptr(addr as *mut u8), + s.root_nanbox_f64(key), ) }); + // Re-read an operand through its handle (or pass the + // original through untouched on the inert path). + let cur_target = || match &roots { + Some((t, ..)) => f64::from_bits(t.get_heap_word_u64()), + None => target, + }; + let cur_value = || match &roots { + Some((_, v, ..)) => v.get_nanbox_f64(), + None => value, + }; + let cur_addr = || match &roots { + Some((_, _, a, _)) => a.get_raw_mut_ptr::() as usize, + None => addr, + }; + let cur_key = || match &roots { + Some((_, _, _, k)) => k.get_nanbox_f64(), + None => key, + }; let fast_safe = if class_id == 0 { // Plain object: prototype is exactly Object.prototype, and // Object.prototype doesn't intercept this key (per-key, not @@ -1379,7 +1430,10 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) ) && property_key_to_rust_string(key) .as_deref() == Some("disposed") - && own_set_descriptor(target, key).is_none(); + // #6943: `property_key_to_rust_string` runs + // `ToPropertyKey` + `js_string_coerce`, so both + // operands must be re-read before this probe. + && own_set_descriptor(cur_target(), cur_key()).is_none(); if inherited_disposed_readonly { return false; } @@ -1400,14 +1454,12 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // class chain — SLOW_FLAGS above already excluded // frozen/sealed/descriptor bits; add the per-instance // divergence flags (setPrototypeOf override / null proto). - let key_ptr = crate::builtins::js_string_coerce(key) + let key_ptr = crate::builtins::js_string_coerce(cur_key()) as *const crate::StringHeader; - // #6943: re-read the receiver through its handle — - // everything below dereferences it. - let addr = match &roots { - Some((_, _, a)) => a.get_raw_mut_ptr::() as usize, - None => addr, - }; + // #6943: re-read the receiver AND the key through + // their handles — everything below uses both. + let addr = cur_addr(); + let key = cur_key(); let interned = crate::object::interned_key_ptr(key_ptr); // #6595: a per-evaluation CLASS OBJECT (what a // capture-carrying class materializes as, @@ -1446,17 +1498,11 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) }; verdict }; - // #6943: the store itself takes the refreshed receiver - // and payload — both were rooted across whichever - // coercion the arm above performed. - let (target, value) = match &roots { - Some((t, v, _)) => { - (f64::from_bits(t.get_heap_word_u64()), v.get_nanbox_f64()) - } - None => (target, value), - }; + // #6943: the store itself takes the refreshed receiver, + // KEY and payload — all three were rooted across + // whichever coercion the arm above performed. if fast_safe { - target_set(target, key, value); + target_set(cur_target(), cur_key(), cur_value()); return true; } } diff --git a/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs b/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs index 40a263723c..1b453ee6e4 100644 --- a/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs +++ b/crates/perry/tests/gc_string_coerce_property_key_rooting_6943.rs @@ -334,7 +334,9 @@ check("gopd-typed-array-oob", Object.getOwnPropertyDescriptor(ta, heavyKey("99") // --- string primitive arm (`string_primitive_descriptor`, whose `str_value` // receiver is itself a movable heap string) --- -const s: any = "abcdef" + String(keepalive.length % 1); +// Built at runtime (join defeats constant folding) so the receiver is a real +// heap string rather than a static one: exactly 6 chars, "abcdef". +const s: any = ["a", "b", "c", "d", "e", "f"].join(""); keepalive.push(s); churnAndCollect(); const dChar = Object.getOwnPropertyDescriptor(s, heavyKey("2")); @@ -504,6 +506,22 @@ checkPayload("class-instance-store-k1", h.k1, 31); checkPayload("class-instance-store-k2", h.k2, 32); checkPayload("class-instance-ctor-field", h.v, 27); +// An OBJECT key on the same store lane: `js_string_coerce` runs the user +// `toString`, so the KEY itself — a POINTER_TAG heap value — can be evacuated +// mid-coercion, alongside the receiver and the payload. This is the operand the +// first pass of the #6943 fix missed at `ordinary_set_with_receiver`. +h[heavyKey("objKeyed")] = payload(34); +churnAndCollect(); +checkPayload("class-instance-store-object-key", h.objKeyed, 34); +check("class-instance-store-object-key-name", Object.hasOwn(h, "objKeyed"), true); + +// Same on a plain (class_id == 0) receiver, which reaches the coercion through +// `object_proto_may_intercept_key` rather than the store-plan key. +const plain: any = receiver(); +plain[heavyKey("plainObjKeyed")] = payload(35); +churnAndCollect(); +checkPayload("plain-store-object-key", plain.plainObjKeyed, 35); + // --- class static computed field with a NON-symbol key // (js_class_register_static_symbol's string arm stores `value` across the // coercion).