fix(runtime): run inherited Symbol-keyed accessors with the original receiver

changelog.d/10597-inherited-symbol-getter-receiver.md

@@ -0,0 +1,15 @@
+### Fixed
+
+- **An inherited Symbol-keyed accessor now runs with the original receiver, not `undefined`.**
+  `obj[sym]` where the getter/setter lives on a prototype (`Object.defineProperty(Fn.prototype, sym,
+  ...)`, an object-literal `get [sym]()` reached through `Object.create`, or a declared class
+  prototype) used to invoke the accessor with no receiver at all, so it observed whatever `this`
+  happened to be ambient — `undefined` at module top level. fastify 5.10.0's
+  `Reply.prototype[kRouteContext]` getter crashed every HTTP request with `TypeError: Cannot read
+  properties of undefined (reading 'request')`. `[[Get]]`/`[[Set]]` now thread the read/write's
+  actual receiver through every prototype-chain walk (`crates/perry-runtime/src/symbol/get.rs`,
+  `object/class_registry/prototype_objects.rs`); `Reflect.get`/`Reflect.set` for a Symbol key reach
+  the receiver-aware entry points directly. An inherited *setter* is now consulted too —
+  `obj[sym] = v` used to silently shadow it with a new own data property instead of running it —
+  gated by a symbol-id-keyed accessor filter (`symbol_may_have_accessor`) so the common no-accessor
+  write path stays cheap.

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

@@ -96,7 +96,7 @@ pub use state::{
 // ── prototype_objects.rs ────────────────────────────────────────────────────
 pub(crate) use prototype_objects::{
     class_prototype_object, ensure_function_prototype_object, function_class_id,
-    function_value_for_class_id, resolve_proto_chain_field,
+    function_value_for_class_id, proto_chain_symbol_slot, resolve_proto_chain_field,
     resolve_proto_chain_field_with_receiver, resolve_proto_chain_symbol,
 };
 pub use prototype_objects::{

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

@@ -657,7 +657,24 @@ unsafe fn resolve_proto_chain_field_inner(
 /// At each node we follow the proto object's own class id (the
 /// `Object.create` prototype link) first, then fall back to
 /// `parent_class_id` (the `extends` link); a `visited` set bounds cycles.
-pub(crate) unsafe fn resolve_proto_chain_symbol(class_id: u32, sym_f64: f64) -> Option<f64> {
+///
+/// #10481: an accessor found on a prototype object runs with `this ===
+/// receiver`, the object the read started from — never the prototype object
+/// that holds it.
+pub(crate) unsafe fn resolve_proto_chain_symbol(
+    class_id: u32,
+    sym_f64: f64,
+    receiver: f64,
+) -> Option<f64> {
+    proto_chain_symbol_slot(class_id, sym_f64).map(|slot| slot.read(receiver))
+}
+
+/// The walk behind [`resolve_proto_chain_symbol`], stopping at the nearest
+/// prototype object that owns `sym_f64` without invoking an accessor there.
+pub(crate) unsafe fn proto_chain_symbol_slot(
+    class_id: u32,
+    sym_f64: f64,
+) -> Option<crate::symbol::OwnSymbolSlot> {
     let mut cid = class_id;
     let mut depth = 0usize;
     let mut visited: [u32; 32] = [0; 32];
@@ -672,8 +689,8 @@ pub(crate) unsafe fn resolve_proto_chain_symbol(class_id: u32, sym_f64: f64) ->
             let proto_f64 = f64::from_bits(JSValue::pointer(proto_obj as *const u8).bits());
             // OWN lookup only — this fn IS the chain walk, so recursing into
             // the full chain-walking getter would re-walk per prototype.
-            if let Some(v) = crate::symbol::own_symbol_property(proto_f64, sym_f64) {
-                return Some(v);
+            if let Some(slot) = crate::symbol::own_symbol_slot(proto_f64, sym_f64) {
+                return Some(slot);
             }
             // Prefer the `Object.create` prototype link: the next chain node
             // is the proto object's own class id (which maps to ITS proto in

crates/perry-runtime/src/proxy.rs

@@ -1363,6 +1363,19 @@ fn own_set_descriptor(target: f64, key: f64) -> Option<OwnSetDescriptor> {
         if !unsafe { crate::symbol::has_own_symbol_property(target, key) } {
             return None;
         }
+        // #10481: a symbol ACCESSOR is not a data slot — report its setter so
+        // the walk runs it with the receiver instead of shadowing it.
+        let (owner, sym_key) = unsafe {
+            (
+                crate::symbol::obj_key_from_f64(target),
+                crate::symbol::sym_key_from_f64(key),
+            )
+        };
+        if let Some((_, setter_bits)) =
+            crate::symbol::symbol_accessor_descriptor_bits(owner, sym_key)
+        {
+            return Some(OwnSetDescriptor::Accessor { setter_bits });
+        }
         // An existing symbol-keyed own data property is non-writable when the
         // receiver is frozen or its per-symbol attrs say so — so a strict
         // `obj[sym] = v` is rejected (throws) rather than silently no-op'd

crates/perry-runtime/src/proxy/reflect.rs

@@ -97,6 +97,18 @@ pub extern "C" fn js_reflect_get(target: f64, key: f64, receiver: f64) -> f64 {
     } else {
         receiver_handle.get_nanbox_f64()
     };
+    // #10481: the Symbol resolver invokes every accessor it finds with an
+    // explicit receiver, so hand it the one this call was given; an inherited
+    // getter must not see the target the lookup started from.
+    if unsafe { crate::symbol::js_is_symbol(property_key_handle.get_nanbox_f64()) } != 0 {
+        return unsafe {
+            crate::symbol::js_object_get_symbol_property_with_receiver(
+                target_handle.get_nanbox_f64(),
+                property_key_handle.get_nanbox_f64(),
+                recv,
+            )
+        };
+    }
     let prev = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(recv));
     let result = target_get_property_key(
         target_handle.get_nanbox_f64(),

crates/perry-runtime/src/symbol.rs

@@ -27,6 +27,8 @@ pub(crate) use accessors::{
 mod constructors;
 mod gc_roots;
 mod get;
+#[cfg(test)]
+mod inherited_accessor_tests;
 mod iterator;
 mod properties;
 
@@ -56,7 +58,11 @@ pub use properties::{
 
 // Symbol-keyed property reads.
 pub(crate) use get::has_declared_prototype_symbol_property;
-pub(crate) use get::{has_own_symbol_property, inherited_symbol_property, own_symbol_property};
+pub(crate) use get::{
+    has_own_symbol_property, inherited_symbol_property,
+    js_object_get_symbol_property_with_receiver, own_symbol_property, own_symbol_slot,
+    OwnSymbolSlot,
+};
 pub use get::{
     js_object_get_symbol_property, js_object_get_symbol_property_ic_miss,
     js_object_get_symbol_then_field_ic_miss, SymbolPicCache, SymbolPicCacheSlot, SYMBOL_PIC_WORDS,

crates/perry-runtime/src/symbol/accessors.rs

@@ -78,6 +78,66 @@ pub(crate) fn test_seed_symbol_accessor_property(obj_key: usize, sym_key: usize,
     );
 }
 
+/// One bit per symbol that has ever carried an accessor, hashed by the
+/// symbol's ID (#10481). `obj[sym] = v` consults it before walking the
+/// prototype chain for an inherited setter: a clear bit proves no accessor
+/// exists under that symbol anywhere, so the walk cannot find one and the
+/// write goes straight to the own-data store. A set bit is only a maybe, and
+/// costs the walk that a correct answer needed anyway.
+///
+/// Keyed by `SymbolHeader::id`, not by the symbol's ADDRESS: a moving
+/// collection rewrites the accessor table's pointer keys (see
+/// `scan_symbol_accessor_roots_mut`) but copies the id verbatim, so an
+/// id-keyed filter needs no rescan, no rekey and no GC root. Monotonic —
+/// removing an accessor leaves the bit set, which only costs a walk.
+static SYMBOL_ACCESSOR_IDS: [std::sync::atomic::AtomicU64; SYMBOL_ACCESSOR_ID_WORDS] =
+    [const { std::sync::atomic::AtomicU64::new(0) }; SYMBOL_ACCESSOR_ID_WORDS];
+const SYMBOL_ACCESSOR_ID_WORDS: usize = 4;
+
+/// `(word, mask)` for a symbol's id. Ids are a monotonic counter, so the low
+/// bits discriminate perfectly until the filter saturates at 256 distinct
+/// accessor symbols.
+///
+/// # Safety
+/// `sym_key` must be a live `SymbolHeader` address (a non-zero
+/// `sym_key_from_f64` result, which has already checked the magic).
+#[inline]
+unsafe fn symbol_accessor_id_bit(sym_key: usize) -> (usize, u64) {
+    let id = (*(sym_key as *const crate::symbol::SymbolHeader)).id;
+    let bit = (id % (SYMBOL_ACCESSOR_ID_WORDS as u64 * 64)) as usize;
+    (bit / 64, 1u64 << (bit % 64))
+}
+
+/// `false` ⟹ no accessor has ever been installed under this symbol, on any
+/// object. Checked before the inherited-accessor walk on the write path.
+///
+/// # Safety
+/// Same contract as [`symbol_accessor_id_bit`].
+#[inline]
+pub(super) unsafe fn symbol_may_have_accessor(sym_key: usize) -> bool {
+    let (word, mask) = symbol_accessor_id_bit(sym_key);
+    SYMBOL_ACCESSOR_IDS[word].load(std::sync::atomic::Ordering::Acquire) & mask != 0
+}
+
+/// Record that `sym_key` carries an accessor. Published BEFORE the table
+/// insert, so a reader that sees a clear bit cannot miss the entry.
+///
+/// # Safety
+/// Same contract as [`symbol_accessor_id_bit`].
+#[inline]
+unsafe fn note_symbol_accessor_key(sym_key: usize) {
+    let (word, mask) = symbol_accessor_id_bit(sym_key);
+    SYMBOL_ACCESSOR_IDS[word].fetch_or(mask, std::sync::atomic::Ordering::AcqRel);
+}
+
+#[cfg(test)]
+pub(crate) fn test_symbol_accessor_id_bits_set() -> u32 {
+    SYMBOL_ACCESSOR_IDS
+        .iter()
+        .map(|w| w.load(std::sync::atomic::Ordering::Acquire).count_ones())
+        .sum()
+}
+
 pub(crate) unsafe fn set_symbol_accessor_property(
     obj_f64: f64,
     sym_f64: f64,
@@ -90,6 +150,7 @@ pub(crate) unsafe fn set_symbol_accessor_property(
         return;
     }
     crate::symbol::note_symbol_key_installed(sym_key);
+    note_symbol_accessor_key(sym_key);
     {
         // `SYMBOL_PROPERTIES` is the only insertion-ordered record of symbol
         // property CREATION order, which `[[OwnPropertyKeys]]` must report
@@ -161,6 +222,27 @@ pub(super) fn symbol_accessor_property_by_key(
         .and_then(|m| m.get(&(obj_key, sym_key)).copied())
 }
 
+/// Setter twin of [`invoke_symbol_accessor_getter`] (#10481): runs `set_bits`
+/// with `this === receiver` and returns the assigned value. The value is
+/// re-read from its root afterwards — the setter body is user code, so a
+/// collection inside it can move whatever `value` points at.
+pub(super) unsafe fn invoke_symbol_accessor_setter(
+    set_bits: u64,
+    receiver: f64,
+    value: f64,
+) -> f64 {
+    let closure = (set_bits & crate::value::POINTER_MASK) as *const crate::closure::ClosureHeader;
+    if set_bits == 0 || closure.is_null() {
+        return value;
+    }
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let value_h = scope.root_nanbox_f64(value);
+    let prev = scope.root_nanbox_f64(crate::object::js_implicit_this_set(receiver));
+    crate::closure::js_closure_call1(closure, value_h.get_nanbox_f64());
+    crate::object::js_implicit_this_set(prev.get_nanbox_f64());
+    value_h.get_nanbox_f64()
+}
+
 pub(super) unsafe fn invoke_symbol_accessor_getter(get_bits: u64, receiver: f64) -> f64 {
     if get_bits == 0 {
         return f64::from_bits(TAG_UNDEFINED);