fix(runtime): root receivers and stored values across GC-capable property-key coercions (#6935)

changelog.d/6941-property-key-operand-rooting.md

@@ -0,0 +1,24 @@
+fix(runtime): root receivers and stored values across GC-capable property-key coercions (#6935)
+
+`ToPropertyKey(key)` runs a user `Symbol.toPrimitive` / `toString` / `valueOf`, and even for a primitive key it allocates the stringified form — either can trigger a GC that **evacuates** live objects. The property-key entry points held the receiver, and on the write paths the *value being stored*, as raw `f64` / raw-pointer Rust locals across that call:
+
+```rust
+let key = js_to_property_key(key_value);        // user JS -> allocate -> GC -> evacuation
+let obj = extract_obj_ptr(obj_value);           // receiver was raw across the coercion
+js_object_set_field_by_name(obj, key_str, value);   // stale receiver AND stale value
+```
+
+A Rust local is neither a GC root nor a shadow slot. This is the `ToPropertyKey` sibling of the operator family fixed in #6934, and strictly worse: there a stale operand produced one wrong answer, whereas here the stale `value` is written **into a live object**, so the dangling pointer outlives the call.
+
+Same idiom as #6934: `crate::gc::RuntimeHandleScope` plus `root_nanbox_f64` / `root_raw_mut_ptr` / `root_string_ptr` / `root_heap_word_u64`, re-reading the receiver and the stored value through their handles after every GC-capable step. Two shared helpers land in `object/property_key.rs`:
+
+- `property_key_coercion_is_inert(key)` — the plain-double-fast-path analogue. Only an already-heap `STRING_TAG` key qualifies (`js_to_primitive` returns any non-`POINTER_TAG` value unchanged, `ordinary_to_primitive_string_key` bails on it, `js_jsvalue_to_string` hands the same pointer back), so the hot `obj[strKey]` / `hasOwnProperty("x")` shapes keep the pre-fix code path verbatim.
+- `to_property_key_rooted(scope, key)` — the coercion inside a caller-owned scope, returning the coerced key as a handle.
+
+Receivers that may arrive NaN-boxed, as a bare heap address (module-level object slots store the untagged pointer), or as an INT32 class-ref are rooted with `root_heap_word_u64`, which rewrites only the real pointers and preserves each encoding. Where the coercion already sat behind an early return (the typed-array / canonical-index fast paths in `js_dyn_index_{get,set}`, `js_object_{get,set}_index_polymorphic`, `js_array_set_index_or_string`) the scope is placed in the cold arm only, so the #5525 hot paths are untouched.
+
+Sites fixed — the seven named in #6935: `object/property_key.rs` (`js_object_set_property_key`, `js_object_get_property_key`, `js_object_set_property_key_method`, plus `js_super_accessor_get` / `js_object_super_call`), `object/object_literal_ops.rs` (`object_literal_key_to_string`, `js_object_literal_set_computed`, `js_object_define_accessor`), `value/dyn_index.rs` (`js_dyn_index_set` object + class-ref arms, `js_dyn_index_get` object numeric-key arm), `object/polymorphic_index.rs` (`js_object_{get,set}_index_polymorphic`, non-canonical-key arms), `object/delete_rest.rs` (`js_object_delete_dynamic`), `array/indexing.rs` (`js_array_{get,set}_index_or_string`), `object/native_call_method.rs` (`js_native_call_method_value`). Five more the sweep turned up in the same family: `object/object_ops/has_own.rs` (`js_object_has_own`, `js_object_property_is_enumerable`), `object/native_call_method/common_methods.rs` (the `hasOwnProperty` / `propertyIsEnumerable` arms, re-read through the caller's existing root handle), `proxy.rs` (`target_set` — `target_get` was already rooted, its write sibling was not), `object/field_get_set/has_property.rs` (`js_object_has_property`, number-key coercion), and `typed_feedback.rs` (the object/closure arm of the typed-feedback index set).
+
+Checked and not affected: `builtins/console.rs`, `object/class_registry/parent_static.rs`, `proxy.rs` `js_proxy_get` / `property_key_to_rust_string`, the `has_own.rs` handle-band arm, and the class-ref / small-handle arms of `js_dyn_index_get`.
+
+New regression suite `crates/perry/tests/gc_property_key_operand_rooting_6935.rs` runs the write paths, the receiver-only paths and the proxy forward-to-target write under `PERRY_GC_FORCE_EVACUATE=1` + `PERRY_GC_VERIFY_EVACUATION=1`. It passes on the pre-fix runtime too, and its module doc says so: no in-language configuration currently reaches a *minor* cycle that evacuates while the raw runtime locals are unpinned. `gc()` runs a full mark-sweep (evacuation is minor-only, so nothing moves); `perry/gc`'s `minor()` does evacuate but engages `ManualGcScanGuard::force_full_scan()` (#4977), whose conservative stack scan pins exactly the raw locals at issue; and `minor()` with `PERRY_CONSERVATIVE_STACK_SCAN=off` is independently unsound on this build (a plain method's `this` is lost across the collection with or without the fix). The suite is therefore a behavioral guard that will start failing the day a minor-evacuating configuration becomes reachable from compiled code.

crates/perry-runtime/src/array/indexing.rs

@@ -1642,19 +1642,34 @@ pub extern "C" fn js_array_get_index_or_string(arr: *const ArrayHeader, idx: f64
             } else {
                 format!("{:.0}", n)
             };
+            // #6935: `js_string_from_bytes` ALLOCATES, so it can trigger a GC
+            // that evacuates the receiver; `arr` is a bare Rust local.
+            let scope = crate::gc::RuntimeHandleScope::new();
+            let arr_handle = scope.root_raw_const_ptr(arr);
             let key_ptr = crate::string::js_string_from_bytes(key.as_ptr(), key.len() as u32);
-            return array_get_property_by_key(arr, key_ptr);
+            return array_get_property_by_key(
+                arr_handle.get_raw_const_ptr::<ArrayHeader>(),
+                key_ptr,
+            );
         }
     }
 
     if unsafe { crate::symbol::js_is_symbol(idx) } != 0 {
         return f64::from_bits(crate::value::TAG_UNDEFINED);
     }
+    // #6935: read-side sibling of `js_array_set_index_or_string` below —
+    // `js_jsvalue_to_string` on an object key (`a[new Number(1)]`,
+    // `a[{toString(){...}}]`) runs user JS, allocates and can evacuate `arr`.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let arr_handle = scope.root_raw_const_ptr(arr);
     let key = crate::value::js_jsvalue_to_string(idx);
     if key.is_null() {
         return f64::from_bits(crate::value::TAG_UNDEFINED);
     }
-    array_get_property_by_key(arr, key as *const crate::StringHeader)
+    array_get_property_by_key(
+        arr_handle.get_raw_const_ptr::<ArrayHeader>(),
+        key as *const crate::StringHeader,
+    )
 }
 
 /// `arr[idx] = value` where idx may be a NaN-boxed string (numeric-string
@@ -1714,10 +1729,20 @@ pub extern "C" fn js_array_set_index_or_string(
         // number ("4294967295", "-1", "1.5", "NaN") rather than a truncated
         // integer — `js_array_set_string_key` then stores it on the expando
         // map without touching `length` or any element slot. (Issue #4543.)
+        // #6935: `js_jsvalue_to_string` allocates the stringified key, so it can
+        // GC and evacuate both the receiver and the value being stored.
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let arr_handle = scope.root_raw_mut_ptr(arr);
+        let value_handle = scope.root_nanbox_f64(value);
         let key = crate::value::js_jsvalue_to_string(idx);
         if !key.is_null() {
-            return js_array_set_string_key(arr, key as *const crate::StringHeader, value);
+            return js_array_set_string_key(
+                arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
+                key as *const crate::StringHeader,
+                value_handle.get_nanbox_f64(),
+            );
         }
+        return arr_handle.get_raw_mut_ptr::<ArrayHeader>();
     }
     // Fallback for a NON-numeric key: a primitive (`a[null]`, `a[undefined]`,
     // `a[true]`, `a[10n]`) or a boxed object (`a[new Number(1)]`). Per
@@ -1726,11 +1751,25 @@ pub extern "C" fn js_array_set_index_or_string(
     // Arrays previously DROPPED these writes (plain objects handled them).
     // Restricted to `numeric.is_none()`: numeric keys (including non-integer
     // finite floats) are handled above. Symbols stay symbol-keyed.
+    //
+    // #6935: this is the boxed-object arm the doc comment above names, so
+    // `js_jsvalue_to_string` here runs a USER `toString` / `valueOf` — allocate
+    // → GC → evacuation. Pre-fix `arr` and `value` were both raw Rust locals
+    // across it, so a stale receiver dropped the write and a stale `value`
+    // stored a dangling pointer inside a live array.
     if numeric.is_none() && unsafe { crate::symbol::js_is_symbol(idx) } == 0 {
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let arr_handle = scope.root_raw_mut_ptr(arr);
+        let value_handle = scope.root_nanbox_f64(value);
         let key = crate::value::js_jsvalue_to_string(idx);
         if !key.is_null() {
-            return js_array_set_string_key(arr, key as *const crate::StringHeader, value);
+            return js_array_set_string_key(
+                arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
+                key as *const crate::StringHeader,
+                value_handle.get_nanbox_f64(),
+            );
         }
+        return arr_handle.get_raw_mut_ptr::<ArrayHeader>();
     }
     arr
 }

crates/perry-runtime/src/object/delete_rest.rs

@@ -484,7 +484,16 @@ pub extern "C" fn js_object_delete_dynamic(obj: *mut ObjectHeader, key: f64) ->
         return js_object_delete_field(obj, key_str);
     }
 
+    // #6935: the string-key case returned above, so `key` here is a number, a
+    // BigInt, a boolean, `null`/`undefined` — or an OBJECT, whose
+    // `Symbol.toPrimitive` / `toString` / `valueOf` runs user JS. Either way
+    // `js_to_property_key` allocates and can trigger a GC that **evacuates**
+    // the receiver, and `obj` is a bare Rust local across it. Root it and read
+    // it back through the handle for both the symbol and string delete arms.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let obj_handle = scope.root_raw_mut_ptr(obj);
     let property_key = unsafe { js_to_property_key(key) };
+    let obj = obj_handle.get_raw_mut_ptr::<ObjectHeader>();
     if unsafe { crate::symbol::js_is_symbol(property_key) } != 0 {
         // Symbol-keyed delete (`delete obj[Symbol.iterator]`). Previously this
         // fell through to the vacuous `return 1`, so the delete *reported*
@@ -496,9 +505,13 @@ pub extern "C" fn js_object_delete_dynamic(obj: *mut ObjectHeader, key: f64) ->
         let obj_f64 = crate::value::js_nanbox_pointer(obj as i64);
         return unsafe { crate::symbol::js_object_delete_symbol_property(obj_f64, property_key) };
     }
-    let key_str = crate::value::js_jsvalue_to_string(property_key);
+    let property_key_handle = scope.root_nanbox_f64(property_key);
+    let key_str = crate::value::js_jsvalue_to_string(property_key_handle.get_nanbox_f64());
     if !key_str.is_null() {
-        return js_object_delete_field(obj, key_str as *const crate::StringHeader);
+        return js_object_delete_field(
+            obj_handle.get_raw_mut_ptr::<ObjectHeader>(),
+            key_str as *const crate::StringHeader,
+        );
     }
 
     // For other types, delete succeeds vacuously

crates/perry-runtime/src/object/field_get_set/has_property.rs

@@ -203,12 +203,21 @@ pub extern "C" fn js_object_has_property(obj: f64, key: f64) -> f64 {
     // redirect on the happy path) surfaced as a fatal 500 instead of a 307.
     // (Symbols and strings pass through unchanged; a proxy/handle receiver is
     // handled below with the coerced key.)
-    let key = {
+    //
+    // #6935: that coercion ALLOCATES the stringified key, so it can trigger a
+    // GC that evacuates the receiver — and `obj` / `obj_val` are raw locals
+    // captured above. (Only the number arm coerces, so no user JS runs here,
+    // but an allocation-triggered evacuation moves the receiver just the same.)
+    let (obj, obj_val, key) = {
         let kv = JSValue::from_bits(key.to_bits());
         if kv.is_number() {
-            unsafe { crate::object::js_to_property_key(key) }
+            let scope = crate::gc::RuntimeHandleScope::new();
+            let obj_handle = scope.root_heap_word_u64(obj.to_bits());
+            let key = unsafe { crate::object::js_to_property_key(key) };
+            let obj = f64::from_bits(obj_handle.get_heap_word_u64());
+            (obj, JSValue::from_bits(obj.to_bits()), key)
         } else {
-            key
+            (obj, obj_val, key)
         }
     };
     let key_val = JSValue::from_bits(key.to_bits());

crates/perry-runtime/src/object/native_call_method.rs

@@ -388,11 +388,28 @@ pub unsafe extern "C" fn js_native_call_method_value(
         }
     }
 
-    let property_key = if is_symbol_key {
-        key
-    } else {
-        crate::object::js_to_property_key(key)
-    };
+    // #6935: on the non-symbol path `js_to_property_key` runs a user
+    // `Symbol.toPrimitive` / `toString` / `valueOf` (and allocates for every
+    // primitive key), so it can trigger a GC that **evacuates** the receiver.
+    // `object` is a raw NaN-boxed Rust local held across it and is dereferenced
+    // by every dispatch arm below. Root it and read it back through the handle.
+    // The inert case (an already-heap string key) keeps the pre-fix shape so
+    // the hot `obj[strKey](...)` dispatch pays nothing.
+    let (property_key, object) =
+        if is_symbol_key || crate::object::property_key_coercion_is_inert(key) {
+            // A heap string is its own property key — `js_to_property_key`
+            // returns the identical NaN-boxed bits without allocating — so the
+            // pre-fix shape is preserved verbatim for the hot path.
+            (key, object)
+        } else {
+            let scope = crate::gc::RuntimeHandleScope::new();
+            let object_handle = scope.root_heap_word_u64(object.to_bits());
+            let property_key = crate::object::js_to_property_key(key);
+            (
+                property_key,
+                f64::from_bits(object_handle.get_heap_word_u64()),
+            )
+        };
     if !is_symbol_key && crate::symbol::js_is_symbol(property_key) != 0 {
         return js_native_call_method_value(object, property_key, args_ptr, args_len);
     }

crates/perry-runtime/src/object/native_call_method/common_methods.rs

@@ -69,7 +69,17 @@ pub(super) unsafe fn dispatch_common(
             } else {
                 f64::from_bits(crate::value::TAG_UNDEFINED)
             };
+            // #6935: `js_to_property_key` can run a user `Symbol.toPrimitive` /
+            // `toString` / `valueOf` (and allocates for every primitive key), so
+            // it can trigger a GC that **evacuates** the receiver. `object` —
+            // and the `jsval` tag view derived from it at the top of this
+            // function — are raw locals captured *before* the coercion; re-read
+            // the receiver through the caller's `object_handle`, which IS a
+            // root, and re-derive the tag view from that.
             let key_value = crate::object::js_to_property_key(key_value);
+            let key_value = root_scope.root_nanbox_f64(key_value).get_nanbox_f64();
+            let object = object_handle.get_nanbox_f64();
+            let jsval = JSValue::from_bits(object.to_bits());
             if crate::symbol::js_is_symbol(key_value) != 0 {
                 return Some(super::object_ops::js_object_has_own(object, key_value));
             }
@@ -224,7 +234,14 @@ pub(super) unsafe fn dispatch_common(
             // `toString`/`valueOf` yields a Symbol must be treated as that
             // Symbol (test262 propertyIsEnumerable/symbol_property_*), invoking
             // the user conversion exactly once.
+            //
+            // #6935: that user conversion can GC and evacuate the receiver, so
+            // re-read `object`/`jsval` through the caller's root handle
+            // afterwards — see the `hasOwnProperty` arm above.
             let key_value = crate::object::js_to_property_key(key_value);
+            let key_value = root_scope.root_nanbox_f64(key_value).get_nanbox_f64();
+            let object = object_handle.get_nanbox_f64();
+            let jsval = JSValue::from_bits(object.to_bits());
             // Symbol keys must not be string-coerced — route through the
             // canonical entry, which consults the SYMBOL_PROPERTIES side
             // table (mirrors hasOwnProperty's symbol arm).