fix(runtime): a prototype-override receiver must not lose synthesized methods

changelog.d/9247-prototype-override-consumers.md

@@ -0,0 +1,13 @@
+A receiver with a per-instance `[[Prototype]]` override no longer loses the
+methods Perry synthesizes rather than storing on a real prototype. The
+override fast path assumed the resolved method was a user closure, so a
+property-lookup miss called `undefined` (`[...gen().map(f)]` threw
+"undefined is not iterable"), a field-get miss hid the plain-function
+`.prototype` and the boxed-wrapper builtins, and a native method was invoked
+with no receiver bound (`Object.prototype.isPrototypeOf` and
+`Object(true).valueOf()` both threw).
+
+The fast path now runs only for a resolved callable, a field-get miss defers
+to the rest of the lookup instead of answering `undefined`, and the receiver
+is bound for the duration of the call. A resolved hit still wins over the
+class vtable, so an explicit `Object.setPrototypeOf` is unaffected.

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

@@ -221,6 +221,7 @@ mod probe_dispatch;
 /// #9131: per-instance `[[Prototype]]` override lookup, split out of
 /// `get_field_by_name_tail.rs` for the 2000-line cap.
 mod prototype_override;
+#[allow(dead_code)] // #9244: field-get short-circuits removed; kept for the method path.
 
 /// Size of the direct-mapped `(keys_ptr, key_hash, field_index)` inline
 /// cache backing `js_object_get_field_by_name`'s slow tail.

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

@@ -1350,6 +1350,8 @@ pub(crate) fn get_field_by_name_object_tail(
 
         if keys.is_null() {
             // #9131; see `prototype_override::inherited_field_if_overridden`.
+            // A miss returns None so the synthesized arms below stay reachable
+            // (#9244).
             if let Some(v) = super::prototype_override::inherited_field_if_overridden(obj, key) {
                 return v;
             }

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

@@ -8,14 +8,27 @@ use crate::object::ObjectHeader;
 use crate::value::JSValue;
 
 /// An explicit per-instance `[[Prototype]]` REPLACES the class's declaration
-/// prototype; it is not an extra link in front of the original vtable. So when
-/// the own-key scan misses, walk that authoritative chain before exposing class
-/// getters or methods, and do not resurrect the old class surface when the
-/// custom chain also misses — hence `Some(undefined)` rather than `None` once
-/// an override is present.
+/// prototype, so when the own-key scan misses, that chain is what decides —
+/// `Some(value)` here is authoritative and the caller must not fall back to the
+/// class vtable.
 ///
-/// `None` means no override was installed and the caller keeps its existing
-/// class-vtable fallback.
+/// A miss on the custom chain returns `None`, NOT `Some(undefined)`. #9131
+/// originally returned `Some(undefined)` to avoid resurrecting the old class
+/// surface, but the arms BELOW both call sites are not only the class vtable:
+/// they are also everything Perry *synthesizes* rather than stores on a real
+/// prototype — a plain-function `.prototype`, the boxed-wrapper builtins, the
+/// iterator helpers. Swallowing the miss made those unreachable for every
+/// flagged receiver, which is #9244 (`Object(true).valueOf()` →
+/// `called on incompatible receiver`, `FooObj.prototype` → `undefined`).
+///
+/// This is the same polarity `canonical_shape_excludes_own_property` uses: a
+/// question we cannot answer here defers to the tail rather than fabricating a
+/// verdict. The flag is also set by ~20 runtime prototype-wiring sites that are
+/// not user `setPrototypeOf` calls at all, so "flagged" is far weaker evidence
+/// than the original code assumed.
+///
+/// `None` therefore means either no override, or an override that does not
+/// carry this key — in both cases the caller keeps its existing fallback.
 pub(super) fn inherited_field_if_overridden(
     obj: *const ObjectHeader,
     key: *const crate::string::StringHeader,
@@ -26,8 +39,5 @@ pub(super) fn inherited_field_if_overridden(
     if !crate::object::prototype_chain::object_has_prototype_override(obj as usize) {
         return None;
     }
-    Some(
-        crate::object::prototype_chain::resolve_inherited_field(obj as usize, key)
-            .unwrap_or_else(JSValue::undefined),
-    )
+    crate::object::prototype_chain::resolve_inherited_field(obj as usize, key)
 }

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

@@ -1150,6 +1150,29 @@ pub unsafe extern "C-unwind" fn js_native_call_method_nullsafe(
     js_native_call_method(object, method_name_ptr, method_name_len, args_ptr, args_len)
 }
 
+/// Bind `IMPLICIT_THIS` for the duration of one call and restore the previous
+/// value on the way out — including when the callee unwinds, which this
+/// `extern "C-unwind"` dispatch surface makes an ordinary outcome rather than an
+/// exotic one. A plain set/restore pair would leak the receiver into every later
+/// implicit-`this` read once a method throws. #9244.
+struct ImplicitThisScope {
+    previous: f64,
+}
+
+impl ImplicitThisScope {
+    fn bind(receiver: f64) -> Self {
+        Self {
+            previous: crate::object::js_implicit_this_set(receiver),
+        }
+    }
+}
+
+impl Drop for ImplicitThisScope {
+    fn drop(&mut self) {
+        crate::object::js_implicit_this_set(self.previous);
+    }
+}
+
 #[no_mangle]
 // Dynamic native calls may synchronously throw from the selected module
 // implementation. Keep this bridge unwind-capable so a generated caller's JS
@@ -1268,18 +1291,46 @@ pub unsafe extern "C-unwind" fn js_native_call_method(
                 let receiver_ptr =
                     JSValue::from_bits(receiver.to_bits()).as_pointer::<ObjectHeader>();
                 let method = super::js_object_get_field_by_name(receiver_ptr, method_key);
-                let method_handle = root_scope.root_nanbox_f64(f64::from_bits(method.bits()));
-                let receiver = object();
-                let bound = crate::closure::clone_closure_rebind_this(
-                    method_handle.get_nanbox_f64().to_bits(),
-                    receiver,
-                );
-                let args = refreshed_args();
-                return crate::closure::js_native_call_value(
-                    f64::from_bits(bound),
-                    args.as_ptr(),
-                    args.len(),
-                );
+                // Only the RESOLVED case is authoritative. A miss means the
+                // overridden chain simply does not carry this name, and the
+                // dispatch tower below still has arms that legitimately answer
+                // it — notably the #2874 iterator-helper interception, which
+                // resolves `map`/`filter`/`take`/... on a raw iterator that has
+                // no such own or inherited property. Returning here on a miss
+                // called `undefined` and turned `[...gen().map(f)]` into
+                // `TypeError: undefined is not iterable` (#9244). Falling
+                // through costs an ordinary lookup on a path that was already
+                // taking one.
+                let resolved = !method.is_undefined()
+                    && crate::closure::is_closure_ptr(crate::value::js_nanbox_get_pointer(
+                        f64::from_bits(method.bits()),
+                    ) as usize);
+                if resolved {
+                    let method_handle = root_scope.root_nanbox_f64(f64::from_bits(method.bits()));
+                    let receiver = object();
+                    let bound = crate::closure::clone_closure_rebind_this(
+                        method_handle.get_nanbox_f64().to_bits(),
+                        receiver,
+                    );
+                    let args = refreshed_args();
+                    // `clone_closure_rebind_this` only rewrites a closure that
+                    // carries CAPTURES_THIS_FLAG; it returns a native builtin
+                    // (and an arrow, and a generator step) UNCHANGED. Native
+                    // method bodies read their receiver from
+                    // `js_implicit_this_get()` — the same #7576 property that
+                    // made the `%IteratorPrototype%.next` THUNK throw — so
+                    // without binding it here `Object.prototype.isPrototypeOf`
+                    // saw no `this` ("called on null or undefined") and
+                    // `Object(true).valueOf()` saw the wrong one ("called on
+                    // incompatible receiver"). #9244. Restored on the way out,
+                    // including when the callee throws.
+                    let _this_scope = ImplicitThisScope::bind(receiver);
+                    return crate::closure::js_native_call_value(
+                        f64::from_bits(bound),
+                        args.as_ptr(),
+                        args.len(),
+                    );
+                }
             }
         }
     }

crates/perry/tests/issue_9131_prototype_method_replacement.rs

@@ -101,3 +101,39 @@ console.log("swap-after:", readA(a));
         "replace: 3 300\nmid: 153\ncounter-before: 2\ncounter-after: 777\nswap-before: a\nswap-after: b\n"
     );
 }
+
+/// #9244: the per-instance prototype-override fast path in
+/// `js_native_call_method` resolves the method by ordinary property lookup.
+/// A generator carries the override flag but has no own or inherited `map` —
+/// Perry synthesizes the iterator helpers (#2874) further down the dispatch
+/// tower. Returning on the lookup MISS called `undefined`, so
+/// `[...gen().map(f)]` threw `TypeError: undefined is not iterable`. Only a
+/// resolved callable may take the fast path; a miss must fall through.
+#[test]
+fn overridden_receiver_miss_falls_through_to_synthesized_helpers() {
+    let stdout = compile_and_run(
+        r#"
+function* gen() {
+  yield 1;
+  yield 2;
+  yield 3;
+}
+console.log("map:", [...gen().map((x: number) => x * 2)].join(","));
+console.log("filter:", [...gen().filter((x: number) => x > 1)].join(","));
+console.log("take:", Iterator.from([1, 2, 3, 4]).take(2).toArray().join(","));
+
+// A genuine override that DOES resolve still wins over the class vtable.
+class Counter {
+  inc() { return 2; }
+}
+const counter = new Counter();
+Object.setPrototypeOf(counter, { inc: () => 777 });
+console.log("override:", (counter as any).inc());
+"#,
+    );
+
+    assert_eq!(
+        stdout,
+        "map: 2,4,6\nfilter: 2,3\ntake: 1,2\noverride: 777\n"
+    );
+}