From 7ac49349211fcf0548a1300533ce479c06d2c7b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 26 Jun 2026 16:28:17 +0200 Subject: [PATCH 1/2] fix(runtime): preserve generator step-closure `this` through `yield*` delegation --- .../src/closure/dynamic_props.rs | 16 ++++++- crates/perry-runtime/src/closure/mod.rs | 2 +- crates/perry-runtime/src/closure/registry.rs | 15 +++++- .../src/object/global_this/generator.rs | 46 +++++++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index bfdf6fc523..683ce5b713 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -825,7 +825,11 @@ pub(crate) fn clone_closure_rebind_this(closure_bits: u64, recv_box: f64) -> u64 return closure_bits; } let ptr = (closure_bits & 0x0000_FFFF_FFFF_FFFF) as usize; - if ptr < 0x10000 { + // Reject the `[0, 0x100000)` native-handle band BEFORE the header read: + // fetch/http/axios/fastify ids are NaN-boxed with POINTER_TAG but are not + // heap pointers, and dereferencing one would SIGSEGV (#4740). Matches the + // floor `rebind_explicit_this` uses before probing the closure pointer. + if ptr < 0x100000 { return closure_bits; } unsafe { @@ -839,6 +843,16 @@ pub(crate) fn clone_closure_rebind_this(closure_bits: u64, recv_box: f64) -> u64 if raw_count & CAPTURES_THIS_FLAG == 0 { return closure_bits; } + // Generator state-machine step closures (`next`/`return`/`throw`) capture + // the generator BODY's `this` lexically — it is fixed at generator + // creation and must NOT be re-bound by `.call`/method dispatch. The + // `yield* gen` desugar calls `next.call(iter, v)`; rebinding here would + // clobber the captured body-`this` with the iterator object. The flag is + // stamped on the closure header (per-closure, no global table) by + // `js_generator_attach_prototype` when it wires the generator instance. + if raw_count & NO_THIS_REBIND_FLAG != 0 { + return closure_bits; + } let count = real_capture_count(raw_count) as usize; if count == 0 { return closure_bits; diff --git a/crates/perry-runtime/src/closure/mod.rs b/crates/perry-runtime/src/closure/mod.rs index cd919a26f8..b724ce8681 100644 --- a/crates/perry-runtime/src/closure/mod.rs +++ b/crates/perry-runtime/src/closure/mod.rs @@ -36,7 +36,7 @@ pub use registry::{ js_register_closure_strict_function, js_register_closure_synthetic_arguments, lookup_closure_arity, lookup_closure_length, lookup_closure_rest, lookup_closure_rest_full, real_capture_count, resolve_strategy, DispatchStrategy, BOUND_FUNCTION_FUNC_PTR, - BOUND_METHOD_FUNC_PTR, CAPTURES_THIS_FLAG, CLOSURE_MAGIC, + BOUND_METHOD_FUNC_PTR, CAPTURES_THIS_FLAG, CLOSURE_MAGIC, NO_THIS_REBIND_FLAG, }; pub use dispatch::{ diff --git a/crates/perry-runtime/src/closure/registry.rs b/crates/perry-runtime/src/closure/registry.rs index 6798936c55..921a6472ce 100644 --- a/crates/perry-runtime/src/closure/registry.rs +++ b/crates/perry-runtime/src/closure/registry.rs @@ -881,8 +881,19 @@ pub const BOUND_FUNCTION_FUNC_PTR: *const u8 = 0xBADD_B12D_u64 as *const u8; /// `js_closure_unbind_this` clones it and clears slot 0 so `this` becomes undefined. pub const CAPTURES_THIS_FLAG: u32 = 0x8000_0000; -/// Extract the real capture count (masking out the CAPTURES_THIS_FLAG). +/// Flag stored in bit 30 of `capture_count` marking a closure whose captured +/// `this` slot is LEXICAL and must never be re-bound by `Function.prototype.call` +/// / method dispatch (`clone_closure_rebind_this`). Set per-closure (on the +/// header itself) by `js_generator_attach_prototype` for the generator +/// state-machine `next`/`return`/`throw` step closures: their `this` is the +/// generator BODY's receiver, fixed at generator-creation time, and the `yield*` +/// delegation desugar (`next.call(iter, v)`) would otherwise clobber it with the +/// iterator object. Capture counts never approach 2^30, so bit 30 is free. +pub const NO_THIS_REBIND_FLAG: u32 = 0x4000_0000; + +/// Extract the real capture count (masking out the flag bits stored in the +/// two high bits: `CAPTURES_THIS_FLAG` and `NO_THIS_REBIND_FLAG`). #[inline(always)] pub fn real_capture_count(capture_count: u32) -> u32 { - capture_count & !CAPTURES_THIS_FLAG + capture_count & !(CAPTURES_THIS_FLAG | NO_THIS_REBIND_FLAG) } diff --git a/crates/perry-runtime/src/object/global_this/generator.rs b/crates/perry-runtime/src/object/global_this/generator.rs index 31ac35b267..c254de8896 100644 --- a/crates/perry-runtime/src/object/global_this/generator.rs +++ b/crates/perry-runtime/src/object/global_this/generator.rs @@ -314,6 +314,45 @@ fn install_proto_symbol_self_method( } } +/// Stamp `NO_THIS_REBIND_FLAG` onto the `next`/`return`/`throw` step-closure +/// headers of a generator instance object. These closures capture the generator +/// BODY's `this` lexically (the last capture slot, gated by CAPTURES_THIS_FLAG); +/// the `yield*` delegation desugar calls `next.call(iter, v)`, whose +/// `clone_closure_rebind_this` would otherwise overwrite that captured `this` +/// with the iterator object. Marking the header per-closure (rather than via a +/// global func_ptr table built at module-init) is robust under codegen-units and +/// debug builds, and needs no public-struct change. Only closures that actually +/// carry CAPTURES_THIS_FLAG are stamped — a generator whose body never reads +/// `this` is left untouched. +fn mark_generator_step_closures_no_rebind(obj: f64) { + use crate::closure::{CAPTURES_THIS_FLAG, NO_THIS_REBIND_FLAG}; + for name in [ + b"next".as_slice(), + b"return".as_slice(), + b"throw".as_slice(), + ] { + let v = crate::object::js_object_get_own_field_or_undef(obj, name.as_ptr(), name.len()); + let vv = JSValue::from_bits(v.to_bits()); + if !vv.is_pointer() { + continue; + } + let ptr = vv.as_pointer::() as usize; + if !crate::closure::is_closure_ptr(ptr) { + continue; + } + let header = ptr as *mut crate::closure::ClosureHeader; + unsafe { + let cc = (*header).capture_count; + // Only meaningful for closures that capture `this`; the rebind path + // is a no-op for the rest, but skip them anyway to keep the flag's + // invariant tight. + if cc & CAPTURES_THIS_FLAG != 0 { + (*header).capture_count = cc | NO_THIS_REBIND_FLAG; + } + } + } +} + /// #4141: link a freshly-built generator/async-generator instance object into /// the spec `[[Prototype]]` chain. Perry lowers `gen()` to a `{next,return, /// throw}` object literal; this interposes a fresh intermediate object (the @@ -338,6 +377,9 @@ pub extern "C" fn js_generator_attach_prototype(obj: f64, is_async: i32) -> f64 if obj_ptr == 0 { return obj; } + // Pin each step closure's lexical generator-body `this` so `yield*` + // delegation (`next.call(iter, v)`) can't rebind it to the iterator object. + mark_generator_step_closures_no_rebind(obj); if is_async != 0 { super::super::async_generator_queue::wrap_async_generator_instance( obj_ptr as *mut ObjectHeader, @@ -383,6 +425,10 @@ pub extern "C" fn js_generator_attach_closure_prototype( return obj; } + // Pin the step closures' lexical generator-body `this` (see the fallback + // `js_generator_attach_prototype`); this is the closure-identity wiring path. + mark_generator_step_closures_no_rebind(obj); + let closure = crate::closure::clean_closure_ptr(closure_ptr); if closure.is_null() || crate::closure::get_valid_func_ptr(closure).is_null() { return obj; From 32dda35794bb245e3857243b260faed9d5ad62ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 26 Jun 2026 17:36:40 +0200 Subject: [PATCH 2/2] fix(runtime): validate closure ptr via is_closure_ptr before header probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clone_closure_rebind_this used a hand-rolled `ptr < 0x100000` band check (failing the addr-class lint) and only that band check before the unsafe CLOSURE_MAGIC read — a mis-boxed POINTER_TAG value above the band (e.g. a fetch handle or `i32 << 32`) could SIGSEGV the probe (CodeRabbit). Use is_closure_ptr, which does the handle-band + heap-range + alignment + CLOSURE_MAGIC checks, and drop the now-redundant inline magic read. --- .../perry-runtime/src/closure/dynamic_props.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/perry-runtime/src/closure/dynamic_props.rs b/crates/perry-runtime/src/closure/dynamic_props.rs index 683ce5b713..43c1890f68 100644 --- a/crates/perry-runtime/src/closure/dynamic_props.rs +++ b/crates/perry-runtime/src/closure/dynamic_props.rs @@ -825,18 +825,16 @@ pub(crate) fn clone_closure_rebind_this(closure_bits: u64, recv_box: f64) -> u64 return closure_bits; } let ptr = (closure_bits & 0x0000_FFFF_FFFF_FFFF) as usize; - // Reject the `[0, 0x100000)` native-handle band BEFORE the header read: - // fetch/http/axios/fastify ids are NaN-boxed with POINTER_TAG but are not - // heap pointers, and dereferencing one would SIGSEGV (#4740). Matches the - // floor `rebind_explicit_this` uses before probing the closure pointer. - if ptr < 0x100000 { + // Validate the payload is a real heap closure BEFORE any header read. + // `is_closure_ptr` rejects the native/fetch/proxy small-handle band, any + // address outside the platform heap range, misaligned pointers, AND + // confirms CLOSURE_MAGIC — so a mis-boxed POINTER_TAG value (a fetch handle, + // or an `i32 << 32` style value above the band) can't SIGSEGV the probe + // (#4740, #wall2). This subsumes the old hand-rolled band + magic checks. + if !is_closure_ptr(ptr) { return closure_bits; } unsafe { - let type_tag = std::ptr::read_volatile((ptr as *const u8).add(12) as *const u32); - if type_tag != CLOSURE_MAGIC { - return closure_bits; - } let header = ptr as *const ClosureHeader; let raw_count = (*header).capture_count; // No CAPTURES_THIS_FLAG → the closure body doesn't read `this`, no rebind needed.