fix(runtime): computed object keys on class proto and ClassRef (#6945)

changelog.d/7134-computed-key-class-proto.md

@@ -0,0 +1,30 @@
+**runtime:** computed / dynamic object-key property access on class prototypes
+and class constructors now matches Node (#6945).
+
+Three cooperating gaps:
+
+1. `js_dyn_index_get` treated non-string, non-numeric keys as floats
+   (`format!("{}", f64)`), so `obj[{toString(){return "k"}}]` never ran user
+   ToPropertyKey. Object / boolean / null / undefined / bigint keys now go
+   through `js_to_property_key` (with receiver rooting) before the by-name
+   get — matching the set-side path in `js_dyn_index_set`.
+
+2. Class-instance field get walked the reflective decl-proto object only for
+   *accessors*, deliberately skipping data reads to avoid class-id re-entry.
+   Runtime `C.prototype[k] = v` stores an own data field there, so
+   `(new C()).name` missed it while `C.prototype.name` saw it. Own data is
+   now read via `own_data_field_by_name` (no re-walk).
+
+3. Codegen's IndexGet last-resort path routes non-string keys on a known
+   ClassRef through `js_object_get_index_polymorphic`, which rejected every
+   INT32-tagged receiver as a primitive. Registered class-ids now forward to
+   `js_dyn_index_get`'s class-ref arm so `C[k]` / `C[objectKey]` resolve
+   statics and `CLASS_DYNAMIC_PROPS`.
+
+Regression: `test-files/test_gap_computed_key_class_proto_6945.ts`
+(byte-for-byte vs Node 26.5).
+
+Follow-up (CodeRabbit): set-side dynamic-index fallback and polymorphic
+`rooted_property_key_{get,set}` now use `js_to_property_key` and route a
+Symbol-yielding `@@toPrimitive` through the symbol store (was silently
+undefined / stringified).

crates/perry-runtime/src/object/class_registry/prototype_objects.rs

@@ -352,20 +352,36 @@ unsafe fn resolve_proto_chain_field_inner(
         // getter was invisible to `instance.name` (winston:
         // `Object.defineProperty(Logger.prototype, 'transports', { get })`,
         // read as `this.transports`, came back `undefined` → `.length` threw).
-        // Check the decl-proto object for an ACCESSOR only: it is allocated
-        // WITH this `class_id` (`js_object_alloc(class_id, 0)`), so routing its
-        // DATA reads back through `js_object_get_field_by_name` would re-enter
-        // this same walk for the same id and recurse infinitely (a Transform
-        // subclass's `_read` lookup stack-overflowed → SIGSEGV). Class methods /
-        // data are already covered by the vtable + `class_prototype_object`
-        // path below, so the accessor-only probe here is sufficient.
-        if let Some(receiver) = receiver {
-            let decl_proto = class_decl_prototype_object(cid);
-            if !decl_proto.is_null() {
+        // Decl-proto object (`CLASS_DECL_PROTOTYPE_OBJECTS`) is where a user
+        // `Object.defineProperty(ClassName.prototype, name, { get })` installs
+        // its accessor AND where a runtime `C.prototype[k] = v` (computed /
+        // dynamic index write) stores an OWN data field. It is allocated WITH
+        // this `class_id` (`js_object_alloc(class_id, 0)`), so routing DATA
+        // reads back through `js_object_get_field_by_name` would re-enter this
+        // same walk for the same id and recurse infinitely (a Transform
+        // subclass's `_read` lookup stack-overflowed → SIGSEGV).
+        //
+        // Accessors: fire with the instance as receiver.
+        // Own data fields: read via `own_data_field_by_name` only (no class-id
+        // re-walk) so a computed prototype write is visible as
+        // `(new C()).name` (#6945). Class methods / vtable data still come
+        // from the vtable + `class_prototype_object` path below.
+        let decl_proto = class_decl_prototype_object(cid);
+        if !decl_proto.is_null() {
+            if let Some(receiver) = receiver {
                 if let Some(value) = inherited_proto_accessor_value(decl_proto, key, receiver) {
                     return Some(value);
                 }
             }
+            // #6945: own data property written onto the reflective prototype
+            // (computed / dynamic-key set) — never re-enter by-name get.
+            if let Some(value) =
+                unsafe { super::super::field_get_set::own_data_field_by_name(decl_proto, key) }
+            {
+                if !value.is_undefined() {
+                    return Some(value);
+                }
+            }
         }
         let proto_obj = class_prototype_object(cid);
         if !proto_obj.is_null() {

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

@@ -10,14 +10,6 @@
 
 use super::*;
 
-unsafe fn property_key_string_ptr(value: f64) -> *mut crate::StringHeader {
-    let key = crate::object::js_to_property_key(value);
-    if crate::symbol::js_is_symbol(key) != 0 {
-        return std::ptr::null_mut();
-    }
-    crate::value::js_jsvalue_to_string(key)
-}
-
 /// `obj[key]` READ through a non-canonical (object / exotic) key, with the
 /// receiver rooted across the coercion (#6935).
 ///
@@ -27,14 +19,27 @@ unsafe fn property_key_string_ptr(value: f64) -> *mut crate::StringHeader {
 /// `valueOf`, allocates, and can trigger a GC that **evacuates** the receiver.
 /// `raw` is a bare `u64` address in a Rust local — not a GC root and not a
 /// shadow slot — so it has to be re-read through a handle afterwards.
+///
+/// #6945 / CodeRabbit: if ToPropertyKey yields a Symbol (e.g. `@@toPrimitive`
+/// returns one), route through the symbol side-table — never stringify the
+/// Symbol and never treat the Symbol case as "no key" (the old
+/// `property_key_string_ptr` null-out silently returned `undefined`).
 unsafe fn rooted_property_key_get(raw: u64, idx: f64) -> f64 {
     let scope = crate::gc::RuntimeHandleScope::new();
     let recv = scope.root_raw_mut_ptr(raw as *mut ObjectHeader);
-    let key = property_key_string_ptr(idx);
-    if key.is_null() {
+    let key = crate::object::js_to_property_key(idx);
+    let key_h = scope.root_nanbox_f64(key);
+    let key = key_h.get_nanbox_f64();
+    let recv_bits =
+        crate::value::js_nanbox_pointer(recv.get_raw_const_ptr::<ObjectHeader>() as i64);
+    if crate::symbol::js_is_symbol(key) != 0 {
+        return crate::symbol::js_object_get_symbol_property(recv_bits, key);
+    }
+    let key_ptr = crate::value::js_get_string_pointer_unified(key) as *const crate::StringHeader;
+    if key_ptr.is_null() {
         return f64::from_bits(crate::value::TAG_UNDEFINED);
     }
-    let key_handle = scope.root_string_ptr(key);
+    let key_handle = scope.root_string_ptr(key_ptr);
     let v = js_object_get_field_by_name(
         recv.get_raw_mut_ptr::<ObjectHeader>(),
         key_handle.get_raw_const_ptr::<crate::StringHeader>(),
@@ -47,20 +52,31 @@ unsafe fn rooted_property_key_get(raw: u64, idx: f64) -> f64 {
 /// This is the corruption half: the coercion sits between the receiver/value
 /// arriving and the store, so pre-fix a stale receiver dropped the write onto a
 /// forwarding stub and a stale `value` planted a dangling pointer *inside* a
-/// live object, outliving the call.
+/// live object, outliving the call. Symbol-yielding ToPropertyKey routes to
+/// the symbol store (#6945 / CodeRabbit).
 unsafe fn rooted_property_key_set(raw: u64, idx: f64, value: f64) {
     let scope = crate::gc::RuntimeHandleScope::new();
     let recv = scope.root_raw_mut_ptr(raw as *mut ObjectHeader);
     let value_handle = scope.root_nanbox_f64(value);
-    let key = property_key_string_ptr(idx);
-    if key.is_null() {
+    let key = crate::object::js_to_property_key(idx);
+    let key_h = scope.root_nanbox_f64(key);
+    let key = key_h.get_nanbox_f64();
+    let value = value_handle.get_nanbox_f64();
+    let recv_bits =
+        crate::value::js_nanbox_pointer(recv.get_raw_const_ptr::<ObjectHeader>() as i64);
+    if crate::symbol::js_is_symbol(key) != 0 {
+        crate::symbol::js_object_set_symbol_property(recv_bits, key, value);
+        return;
+    }
+    let key_ptr = crate::value::js_get_string_pointer_unified(key) as *const crate::StringHeader;
+    if key_ptr.is_null() {
         return;
     }
-    let key_handle = scope.root_string_ptr(key);
+    let key_handle = scope.root_string_ptr(key_ptr);
     js_object_set_field_by_name(
         recv.get_raw_mut_ptr::<ObjectHeader>(),
         key_handle.get_raw_const_ptr::<crate::StringHeader>(),
-        value_handle.get_nanbox_f64(),
+        value,
     );
 }
 
@@ -109,17 +125,28 @@ fn numeric_key_i32_index(value: f64) -> Option<i32> {
 pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> f64 {
     let raw = if (obj_handle as u64) >> 48 >= 0x7FF8 {
         // NaN-boxed: only POINTER_TAG (0x7FFD) and STRING_TAG (0x7FFF) carry a
-        // heap pointer in the low 48 bits. INT32 (0x7FFE), BIGINT (0x7FFA) and
-        // the undefined/null/bool tags (0x7FFC) are PRIMITIVES — indexing them
-        // yields `undefined` per JS (`(983055)[0] === undefined`). Treating an
-        // INT32's integer payload as a pointer derefs a wild address → SIGSEGV.
-        // This is the Next.js app-page-turbo render crash: a NaN-boxed-int
-        // receiver (0xf000f = 983055) indexed inside a class `get` method
-        // (js_object_get_index_polymorphic read its GcHeader at raw-8). Reject
-        // non-pointer/non-string NaN-boxed receivers up front (cross-platform —
-        // not dependent on a heap-address floor).
+        // heap pointer in the low 48 bits. INT32 (0x7FFE) is usually a primitive
+        // number — BUT a registered **class-ref** is also INT32-tagged, and
+        // `C[k]` with a non-string key (object ToPropertyKey, numeric static
+        // name, …) reaches this helper from codegen's IndexGet last-resort
+        // path. Class refs must NOT be rejected as primitives: route them
+        // through `js_dyn_index_get`, which has the dedicated class-ref arm
+        // (static methods / CLASS_DYNAMIC_PROPS / ToString key). (#6945)
+        // BIGINT (0x7FFA) and the undefined/null/bool tags (0x7FFC) remain
+        // primitives — indexing them yields `undefined` per JS. Treating an
+        // INT32 *number* payload as a pointer would SIGSEGV (Next.js
+        // app-page-turbo: 0xf000f indexed inside a class `get`); only a
+        // *registered* class-id takes the class-ref arm.
         match (obj_handle as u64) >> 48 {
             0x7FFD | 0x7FFF => (obj_handle as u64) & 0x0000_FFFF_FFFF_FFFF,
+            0x7FFE => {
+                let class_id = (obj_handle as u64 & 0xFFFF_FFFF) as u32;
+                if class_id != 0 && crate::object::class_registry::is_class_id_registered(class_id)
+                {
+                    return crate::value::js_dyn_index_get(f64::from_bits(obj_handle as u64), idx);
+                }
+                return f64::from_bits(crate::value::TAG_UNDEFINED);
+            }
             _ => return f64::from_bits(crate::value::TAG_UNDEFINED),
         }
     } else {

crates/perry-runtime/src/value/dyn_index.rs

@@ -269,6 +269,45 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 {
         }
         return f64::from_bits(TAG_UNDEFINED);
     }
+    // #6945: a non-string, non-numeric index must run ToPropertyKey (object
+    // keys invoke `toString`/`valueOf`/`@@toPrimitive`; booleans/null/
+    // undefined/bigint stringify) before the by-name get. The arms below
+    // cast `index as i32` / `format!("{}", index)`, which treat an object
+    // NaN-box as a float and never call user coercion — so
+    // `proto[{toString(){return "k"}}]` missed a write that
+    // `proto.k` / `proto["k"]` could see. Mirrors the set-side
+    // `js_jsvalue_to_string` path in `js_dyn_index_set`.
+    {
+        let idx_js = JSValue::from_bits(idx_bits);
+        // INT32-tagged keys are integer property names (and class-ref values
+        // used as keys, rare); pure f64 numbers keep the element path. Every
+        // other tag is a ToPropertyKey case.
+        if !idx_js.is_number() && !idx_js.is_int32() {
+            let scope = crate::gc::RuntimeHandleScope::new();
+            let recv = scope.root_raw_mut_ptr(raw_ptr as *mut crate::object::ObjectHeader);
+            // Prefer `js_to_property_key` so a Symbol-returning toString is
+            // preserved (and then routed via the symbol arm). Root the
+            // coerced key: ToPropertyKey can allocate / run user JS.
+            let key = unsafe { crate::object::js_to_property_key(index) };
+            let key_h = scope.root_nanbox_f64(key);
+            let key = key_h.get_nanbox_f64();
+            if unsafe { crate::symbol::js_is_symbol(key) } != 0 {
+                let recv_bits = crate::value::js_nanbox_pointer(
+                    recv.get_raw_const_ptr::<crate::object::ObjectHeader>() as i64,
+                );
+                return unsafe { crate::symbol::js_object_get_symbol_property(recv_bits, key) };
+            }
+            let key_ptr =
+                crate::value::js_get_string_pointer_unified(key) as *const crate::StringHeader;
+            if key_ptr.is_null() {
+                return f64::from_bits(TAG_UNDEFINED);
+            }
+            return crate::object::js_object_get_field_by_name_f64(
+                recv.get_raw_const_ptr::<crate::object::ObjectHeader>(),
+                key_ptr,
+            );
+        }
+    }
     let idx_i32 = if index.is_nan() || index.is_infinite() {
         return f64::from_bits(TAG_UNDEFINED);
     } else {
@@ -582,18 +621,36 @@ pub extern "C" fn js_dyn_index_set(obj: f64, index: f64, value: f64) -> f64 {
         );
         return value;
     }
-    // #6935: this is the corruption case. `js_jsvalue_to_string(index)` runs a
-    // user `toString` / `valueOf` for an object index (`obj[{toString(){...}}] = v`)
-    // and allocates for every other shape, so it can GC and EVACUATE. Both the
+    // #6935: ToPropertyKey (below) runs a user `toString` / `valueOf` /
+    // `@@toPrimitive` for an object index (`obj[{toString(){...}}] = v`) and
+    // allocates for every other shape, so it can GC and EVACUATE. Both the
     // receiver `raw_ptr` and the `value` being stored were raw Rust locals
     // across it: a stale receiver dropped the write onto a forwarding stub, and
     // a stale `value` wrote a dangling pointer INTO a live object, where it
     // outlives the call.
+    //
+    // #6945 / CodeRabbit: use `js_to_property_key` (not `js_jsvalue_to_string`)
+    // so an `@@toPrimitive` that returns a Symbol is preserved and routed to
+    // the symbol store — matching the get-side fallback. Stringifying that
+    // Symbol would miss the target property (and Spec ToPropertyKey must not
+    // turn a Symbol result into a string).
     let scope = crate::gc::RuntimeHandleScope::new();
     let recv = scope.root_raw_mut_ptr(raw_ptr as *mut crate::object::ObjectHeader);
     let value_handle = scope.root_nanbox_f64(value);
-    let key_ptr = crate::value::js_jsvalue_to_string(index);
+    let key = unsafe { crate::object::js_to_property_key(index) };
+    let key_h = scope.root_nanbox_f64(key);
+    let key = key_h.get_nanbox_f64();
     let value = value_handle.get_nanbox_f64();
+    if unsafe { crate::symbol::js_is_symbol(key) } != 0 {
+        let recv_bits = crate::value::js_nanbox_pointer(
+            recv.get_raw_const_ptr::<crate::object::ObjectHeader>() as i64,
+        );
+        unsafe {
+            crate::symbol::js_object_set_symbol_property(recv_bits, key, value);
+        }
+        return value;
+    }
+    let key_ptr = crate::value::js_get_string_pointer_unified(key) as *const crate::StringHeader;
     if key_ptr.is_null() {
         return value;
     }

test-files/test_gap_computed_key_class_proto_6945.ts

@@ -0,0 +1,76 @@
+// #6945: a computed object-key write onto a class prototype (or class
+// constructor) must be visible to:
+//   - the computed read with the same key object (ToPropertyKey),
+//   - the plain string-key read,
+//   - instance inheritance through the prototype chain.
+// Pre-fix, the write landed somewhere the dotted `C.prototype.name` read
+// found, but `C.prototype[k]` and `(new C()).name` returned undefined because
+// `js_dyn_index_get` treated object indices as floats (`format!("{}", f64)`)
+// instead of running ToPropertyKey / user `toString`.
+
+class C {
+  m(): number {
+    return 1;
+  }
+}
+const k: any = {
+  toString(): string {
+    return "protoKey";
+  },
+};
+(C.prototype as any)[k] = { tag: 6 };
+console.log("computed via instance:", JSON.stringify((new C() as any).protoKey));
+console.log("computed direct:", JSON.stringify((C.prototype as any).protoKey));
+console.log("computed via key obj:", JSON.stringify((C.prototype as any)[k]));
+
+const plain: any = {};
+plain[k] = { tag: 7 };
+console.log("plain:", JSON.stringify(plain.protoKey));
+console.log("plain via key obj:", JSON.stringify(plain[k]));
+
+// class-constructor (static) side: same ToPropertyKey obligation
+class D {
+  static s(): number {
+    return 1;
+  }
+}
+const ks: any = {
+  toString(): string {
+    return "statKey";
+  },
+};
+(D as any)[ks] = { tag: 6 };
+console.log("static via name:", JSON.stringify((D as any).statKey));
+console.log("static via key obj:", JSON.stringify((D as any)[ks]));
+
+// coercion is observable even when the property is absent
+let calls = 0;
+const absent: any = {
+  toString(): string {
+    calls++;
+    return "nope";
+  },
+};
+console.log("absent via key obj:", JSON.stringify((C.prototype as any)[absent]));
+console.log("absent coercion count:", calls);
+
+// boolean / null keys also go through ToPropertyKey
+const mixed: any = {};
+mixed[true as any] = "t";
+mixed[null as any] = "n";
+console.log("bool key:", mixed["true"]);
+console.log("null key:", mixed["null"]);
+console.log("bool via true:", mixed[true as any]);
+
+// @@toPrimitive returning a Symbol must use the symbol store (get + set),
+// not stringify the Symbol (get-side ToPropertyKey parity on set — #7134 CR).
+const sym = Symbol("viaPrim");
+const viaSym: any = {
+  [Symbol.toPrimitive](_hint: string): symbol {
+    return sym;
+  },
+};
+const holder: any = {};
+holder[viaSym] = { tag: 9 };
+console.log("viaPrim set+get:", JSON.stringify(holder[viaSym]));
+console.log("viaPrim symbol key:", JSON.stringify(holder[sym]));