fix(runtime): reserve iterator raw-field floor so own .next patches stop corrupting state (#9019)

changelog.d/9066-iterator-reserved-floor.md

@@ -0,0 +1,7 @@
+### Fixed: named properties on builtin iterator objects corrupted their internal state (#9019, PR #9066)
+
+A by-name property write on a builtin collection-iterator object (Set/Map/array/string/buffer/regexp iterators, iterator helpers) derived the new key's field index from the object's empty keys array, so the first user property landed at field index 0 — the backing-collection pointer. `it.foo = 1` made the iterator report `done: true` on a live collection; `it.next = fn` made the next builtin advance dereference the closure as a `SetHeader` and SIGSEGV under `for…of` (issue #9019's reproducer, exit 139 on a clean build).
+
+The first by-name append to such a receiver now seeds its keys array with a reserved floor of leading tombstones (the #9038 hole marker every lookup/enumeration/delete path already skips), so user keys append past the raw internal fields; the hole-squeeze compaction preserves the reserved prefix (`crates/perry-runtime/src/object/reserved_floor.rs`). The class-id iterator dispatchers additionally honor an own `next` before the builtin advance (non-callable own values throw per IteratorNext), while the canonical prototype thunks keep running the builtin algorithm so a patch that delegates to its bound original cannot re-enter itself; the fused `for…of` arms validate the iterator result, and the stored-closure drain paths bind `this` to the iterator per Call(next, iterator).
+
+Validated byte-for-byte against the pinned Node 26.5.1 oracle across 13 cases in the new `test-files/test_gap_iterator_patched_next.ts`, plus 4 `reserved_floor` unit tests (including a field-0 fixture asserting the backing pointer is unchanged after the write).

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

@@ -620,6 +620,26 @@ unsafe fn make_pair_array(idx: u32, value: f64) -> f64 {
 pub unsafe fn dispatch_array_iterator_method(
     iter_obj: *mut ObjectHeader,
     method_name: &str,
+) -> f64 {
+    dispatch_array_iterator_method_inner(iter_obj, method_name, true)
+}
+
+/// Builtin advance only — the canonical prototype thunk's entry (#9019):
+/// `%ArrayIteratorPrototype%.next.call(it)` (or a pre-patch `.bind(it)`)
+/// runs the builtin algorithm even when the instance carries an own patched
+/// `next`, or a patch delegating to the bound original would re-enter
+/// itself forever.
+pub(crate) unsafe fn dispatch_array_iterator_method_builtin(
+    iter_obj: *mut ObjectHeader,
+    method_name: &str,
+) -> f64 {
+    dispatch_array_iterator_method_inner(iter_obj, method_name, false)
+}
+
+unsafe fn dispatch_array_iterator_method_inner(
+    iter_obj: *mut ObjectHeader,
+    method_name: &str,
+    honor_override: bool,
 ) -> f64 {
     // #7475: the raw `iter_obj` parameter is not a GC root, and this function
     // allocates in several places — `js_object_set_field` (shape transition /
@@ -645,10 +665,13 @@ pub unsafe fn dispatch_array_iterator_method(
     };
     match method_name {
         "next" => {
-            if let Some(result) =
-                crate::object::call_overridden_iterator_next(iter_obj(), ARRAY_ITERATOR_CLASS_ID)
-            {
-                return result;
+            if honor_override {
+                if let Some(result) = crate::object::call_overridden_iterator_next(
+                    iter_obj(),
+                    ARRAY_ITERATOR_CLASS_ID,
+                ) {
+                    return result;
+                }
             }
             if kind == KIND_VALUES_NULL_DONE {
                 let epoch_ptr = js_nanbox_get_pointer(f64::from_bits(

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

@@ -1378,6 +1378,10 @@ pub(crate) fn sync_iterator_to_array_if_not_async(iter_f64: f64) -> Option<*mut
     let next_ptr = if next_val.is_undefined() {
         std::ptr::null::<closure::ClosureHeader>()
     } else {
+        // #9019: same non-callable own `next` guard as `js_iterator_to_array`.
+        if !is_callable_value(next_f64) {
+            crate::closure::throw_not_callable();
+        }
         js_nanbox_get_pointer(next_f64) as *const closure::ClosureHeader
     };
     let use_method_dispatch = next_ptr.is_null();
@@ -1403,7 +1407,12 @@ pub(crate) fn sync_iterator_to_array_if_not_async(iter_f64: f64) -> Option<*mut
                 )
             }
         } else {
-            closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED))
+            // Call(next, iterator) — bind `this` like `js_iterator_to_array`
+            // does for its stored-closure path (#9019).
+            let prev_this = crate::object::js_implicit_this_set(iter_f64);
+            let r = closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED));
+            crate::object::js_implicit_this_set(prev_this);
+            r
         };
         if crate::promise::js_value_is_promise(step) != 0 {
             return None;
@@ -1540,6 +1549,14 @@ pub extern "C" fn js_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
     let next_ptr = if next_val.is_undefined() {
         std::ptr::null::<closure::ClosureHeader>()
     } else {
+        // #9019: an own `next` that is not a callable closure must throw
+        // (IteratorNext is GetV + Call), not be reinterpreted as a
+        // `ClosureHeader` — a builtin iterator can now carry a user-assigned
+        // own `next` of any type, and calling through a number's payload
+        // bits crashes.
+        if !is_callable_value(next_f64) {
+            crate::closure::throw_not_callable();
+        }
         js_nanbox_get_pointer(next_f64) as *const closure::ClosureHeader
     };
     // #321: some iterators (perry's runtime array iterator with
@@ -1583,10 +1600,16 @@ pub extern "C" fn js_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
                 )
             }
         } else {
-            closure::js_closure_call1(
+            // Call(next, iterator): bind `this` for the stored-closure path
+            // exactly like `js_iterator_next_result` — a user-assigned
+            // `it.next = function () { … }` may read `this` (#9019).
+            let prev_this = crate::object::js_implicit_this_set(iter_h.get_nanbox_f64());
+            let r = closure::js_closure_call1(
                 js_nanbox_get_pointer(next_h.get_nanbox_f64()) as *const closure::ClosureHeader,
                 f64::from_bits(TAG_UNDEFINED),
-            )
+            );
+            crate::object::js_implicit_this_set(prev_this);
+            r
         };
         // IteratorNext (ECMA-262 §7.4.2 step 3): if Type(result) is not
         // Object, throw a TypeError. `is_pointer()` is true only for
@@ -1672,6 +1695,10 @@ fn js_async_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
     let next_ptr = if next_val.is_undefined() {
         std::ptr::null::<closure::ClosureHeader>()
     } else {
+        // #9019: same non-callable own `next` guard as `js_iterator_to_array`.
+        if !is_callable_value(next_f64) {
+            crate::closure::throw_not_callable();
+        }
         js_nanbox_get_pointer(next_f64) as *const closure::ClosureHeader
     };
     let use_method_dispatch = next_ptr.is_null();
@@ -1696,7 +1723,12 @@ fn js_async_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader {
                 )
             }
         } else {
-            closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED))
+            // Call(next, iterator) — bind `this` for the stored-closure path
+            // (#9019), mirroring `js_iterator_to_array`.
+            let prev_this = crate::object::js_implicit_this_set(iter_f64);
+            let r = closure::js_closure_call1(next_ptr, f64::from_bits(TAG_UNDEFINED));
+            crate::object::js_implicit_this_set(prev_this);
+            r
         };
         let Some(step_result) = settled_promise_value(step) else {
             break;

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

@@ -154,6 +154,7 @@ pub use self::iter_methods::{
     js_array_map_discard, js_array_reduce, js_array_some, js_array_some_captureless,
     js_array_to_locale_string, js_validate_array_callback, js_validate_array_map_callback,
 };
+pub(crate) use self::iter_object::dispatch_array_iterator_method_builtin;
 pub use self::iter_object::{
     arguments_values_iter, array_entries_iter, array_keys_iter, array_values_iter,
     array_values_iter_null_done, dispatch_array_iterator_method, js_array_entries_iter_obj,

crates/perry-runtime/src/buffer/iter.rs

@@ -110,6 +110,13 @@ pub unsafe fn dispatch_buffer_iterator_method(
 ) -> f64 {
     match method_name {
         "next" => {
+            // #9019: an own `next` assigned onto the iterator instance wins
+            // over the builtin advance, exactly as on the Map/Set path.
+            if let Some(result) =
+                crate::object::call_overridden_iterator_next(iter_obj, BUFFER_ITERATOR_CLASS_ID)
+            {
+                return result;
+            }
             // Field 0: backing buffer pointer (NaN-boxed).
             let backing_field = js_object_get_field(iter_obj, 0);
             let backing_f64 = f64::from_bits(backing_field.bits());

crates/perry-runtime/src/collection_iter_object.rs

@@ -238,23 +238,39 @@ fn next_read_index(cursor: u32, last_key_in_place: bool, find_last: impl FnOnce(
 
 /// Dispatch `.next()` / `[Symbol.iterator]()` on a Map iterator object.
 pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 {
-    dispatch_map_iterator_method_emit(iter_obj, method_name, false)
+    dispatch_map_iterator_method_emit(iter_obj, method_name, false, true)
+}
+
+/// Builtin advance only — the canonical prototype thunk's entry (#9019).
+/// `%MapIteratorPrototype%.next.call(it)` (including a `.bind(it)` taken
+/// before a patch was installed) must run the builtin algorithm even when
+/// the instance carries an own patched `next`: honoring the override there
+/// would make a patch that delegates to the bound original re-enter itself
+/// forever, and it is also not what the spec function does.
+pub(crate) unsafe fn dispatch_map_iterator_method_builtin(
+    iter_obj: *mut ObjectHeader,
+    method_name: &str,
+) -> f64 {
+    dispatch_map_iterator_method_emit(iter_obj, method_name, false, false)
 }
 
 unsafe fn dispatch_map_iterator_method_emit(
     iter_obj: *mut ObjectHeader,
     method_name: &str,
     emit_cached: bool,
+    honor_override: bool,
 ) -> f64 {
     let scope = crate::gc::RuntimeHandleScope::new();
     let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64));
     let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader;
     match method_name {
         "next" => {
-            if let Some(result) =
-                crate::object::call_overridden_iterator_next(iter_obj(), MAP_ITERATOR_CLASS_ID)
-            {
-                return result;
+            if honor_override {
+                if let Some(result) =
+                    crate::object::call_overridden_iterator_next(iter_obj(), MAP_ITERATOR_CLASS_ID)
+                {
+                    return result;
+                }
             }
             let backing = f64::from_bits(js_object_get_field(iter_obj(), 0).bits());
             let map_h = scope.root_nanbox_f64(backing);
@@ -317,23 +333,34 @@ unsafe fn dispatch_map_iterator_method_emit(
 
 /// Dispatch `.next()` / `[Symbol.iterator]()` on a Set iterator object.
 pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 {
-    dispatch_set_iterator_method_emit(iter_obj, method_name, false)
+    dispatch_set_iterator_method_emit(iter_obj, method_name, false, true)
+}
+
+/// Builtin advance only — see [`dispatch_map_iterator_method_builtin`].
+pub(crate) unsafe fn dispatch_set_iterator_method_builtin(
+    iter_obj: *mut ObjectHeader,
+    method_name: &str,
+) -> f64 {
+    dispatch_set_iterator_method_emit(iter_obj, method_name, false, false)
 }
 
 unsafe fn dispatch_set_iterator_method_emit(
     iter_obj: *mut ObjectHeader,
     method_name: &str,
     emit_cached: bool,
+    honor_override: bool,
 ) -> f64 {
     let scope = crate::gc::RuntimeHandleScope::new();
     let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64));
     let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader;
     match method_name {
         "next" => {
-            if let Some(result) =
-                crate::object::call_overridden_iterator_next(iter_obj(), SET_ITERATOR_CLASS_ID)
-            {
-                return result;
+            if honor_override {
+                if let Some(result) =
+                    crate::object::call_overridden_iterator_next(iter_obj(), SET_ITERATOR_CLASS_ID)
+                {
+                    return result;
+                }
             }
             let backing = f64::from_bits(js_object_get_field(iter_obj(), 0).bits());
             let set_h = scope.root_nanbox_f64(backing);
@@ -448,11 +475,20 @@ pub unsafe extern "C-unwind" fn js_for_of_next(iter: f64) -> f64 {
                 if header.obj_type == crate::gc::GC_TYPE_OBJECT {
                     let obj = raw as *mut ObjectHeader;
                     let class_id = (*obj).class_id;
+                    // Spec IteratorNext validation applies on the fused arms
+                    // too: a builtin advance always returns an object, but a
+                    // patched own `next` (#9019) can return anything, and
+                    // `for…of` must throw the same TypeError the generic arm
+                    // throws rather than hand the desugar a primitive.
                     if class_id == MAP_ITERATOR_CLASS_ID {
-                        return dispatch_map_iterator_method_emit(obj, "next", true);
+                        return crate::symbol::js_iterator_result_validate(
+                            dispatch_map_iterator_method_emit(obj, "next", true, true),
+                        );
                     }
                     if class_id == SET_ITERATOR_CLASS_ID {
-                        return dispatch_set_iterator_method_emit(obj, "next", true);
+                        return crate::symbol::js_iterator_result_validate(
+                            dispatch_set_iterator_method_emit(obj, "next", true, true),
+                        );
                     }
                 }
             }

crates/perry-runtime/src/iterator_helpers.rs

@@ -445,7 +445,16 @@ pub unsafe fn dispatch_iterator_helper_method(
     };
 
     match method_name {
-        "next" => helper_next(obj),
+        // #9019: an own `next` assigned onto the helper instance wins over
+        // the builtin advance, exactly as on the Map/Set path.
+        "next" => {
+            if let Some(result) =
+                crate::object::call_overridden_iterator_next(obj, ITERATOR_HELPER_CLASS_ID)
+            {
+                return result;
+            }
+            helper_next(obj)
+        }
         "Symbol.iterator" | "@@iterator" => self_f64,
         "return" | "throw" => make_iter_result(JSValue::undefined(), true),
         // Lazy helpers — return a new helper wrapping `self`.

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

@@ -398,7 +398,15 @@ pub extern "C" fn js_object_delete_field(
                 // Threshold: squeeze every hole plus this key in one pass,
                 // then continue through the ordinary compaction bookkeeping
                 // is unnecessary — the squeeze does its own.
-                squeeze_holes_and_delete(obj, keys, i, key_count, alloc_limit, field_count);
+                squeeze_holes_and_delete(
+                    obj,
+                    keys,
+                    i,
+                    key_count,
+                    alloc_limit,
+                    field_count,
+                    crate::object::reserved_slot_floor_for_class_id((*obj).class_id) as usize,
+                );
                 return 1;
             }
         }
@@ -1194,12 +1202,20 @@ unsafe fn squeeze_holes_and_delete(
     key_count: usize,
     alloc_limit: usize,
     field_count: u32,
+    // #9019: a reserved-layout receiver's leading `floor` slots are
+    // STRUCTURAL tombstones guarding its raw internal fields — squeezing
+    // them would slide user keys back under the floor and re-open the
+    // field-0 alias. They are exempt from compaction; only churn holes at
+    // or past the floor are squeezed. (`delete_slot` is always >= floor
+    // there: the reserved slots hold no key a delete could match.)
+    reserved_floor: usize,
 ) {
     let keys = keys as *mut crate::ArrayHeader;
     let elements = (keys as *mut u8).add(std::mem::size_of::<crate::ArrayHeader>()) as *mut f64;
     let fields_ptr = (obj as *mut u8).add(std::mem::size_of::<ObjectHeader>()) as *mut u64;
-    let mut out = 0usize;
-    for s in 0..key_count {
+    let floor = reserved_floor.min(key_count);
+    let mut out = floor;
+    for s in floor..key_count {
         let kv = std::ptr::read(elements.add(s));
         if s == delete_slot || kv.to_bits() == crate::value::TAG_HOLE {
             continue;
@@ -1239,7 +1255,9 @@ unsafe fn squeeze_holes_and_delete(
     set_object_live_slot_count(obj, std::cmp::min(out, alloc_limit) as u32);
     // Slots moved: the per-array key index and any stale descriptors for the
     // pre-squeeze states are wrong now. Drop the index (rebuilt on demand)
-    // and publish the squeezed shape at hole_count = 0.
+    // and publish the squeezed shape at exactly the surviving hole count —
+    // zero for an ordinary receiver, the structural reserved floor (#9019)
+    // for an iterator-family one.
     crate::object::shapes::shape_drop(keys);
-    super::shapes::publish_object_shape_holes(obj, 0);
+    super::shapes::publish_object_shape_holes(obj, floor as u32);
 }

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

@@ -184,6 +184,14 @@ pub extern "C" fn js_object_set_field_by_name(
                     if crate::object::object_is_regular(o)
                         && class_id != 0
                         && class_id != NATIVE_MODULE_CLASS_ID
+                        // #9019: reserved-layout iterator receivers must
+                        // reach the tail so their floor seed runs before
+                        // any transition is consulted — an unseeded one
+                        // shares its keyless birth ShapeId with every
+                        // other keyless object of the same live bound, so
+                        // a cached edge here could hand it a foreign slot
+                        // index below the floor.
+                        && crate::object::reserved_slot_floor_for_class_id(class_id) == 0
                         && !super::prototype_chain::object_has_prototype_override(raw)
                         && super::prop_plan::store_plan_check(class_id, key as usize)
                     {