From 5d114830bb9a1df5689d96e4b9b03282e0205e09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 8 Jul 2026 22:17:56 +0200 Subject: [PATCH 1/2] fix(runtime,lower): `in` walks Object.create protos; for-in skips deleted keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related prototype-chain correctness fixes. 1. `in` operator (#6145): `key in Object.create(proto)` wrongly returned false for inherited properties — the custom proto's own keys and Object.prototype members alike. Object.create models [[Prototype]] via a synthetic class_id → prototype object (CLASS_PROTOTYPE_OBJECTS), but `ordinary_has_property` only walked recorded static prototypes (object_static_prototype), which Object.create never populates, and the Object.prototype fallback bails on class_id != 0. Fix: when no static prototype is recorded, hop through the synthetic prototype object (class_prototype_object) and continue the walk — the same chain the field-GET path resolves via resolve_proto_chain_field. 2. for-in (#6146): a key deleted from the receiver before it is visited was still visited, because for-in snapshots the key list up front (ForInKeys). Fix: spill the receiver to a temp and wrap the loop body in `if (typeof obj === "string" ? true : (key in obj)) { ... }`, skipping keys no longer present. Primitive strings (the only primitive with a non-empty for-in snapshot) bypass the recheck since `in` throws on primitives — which is why fix (1) is a prerequisite for Object.create receivers. Applied at both desugar sites via shared helper guard_for_in_body. --- crates/perry-hir/src/lower/for_head.rs | 43 +++++++++++++++++++ crates/perry-hir/src/lower/mod.rs | 2 +- crates/perry-hir/src/lower/stmt_loops.rs | 17 +++++++- crates/perry-hir/src/lower_decl/body_stmt.rs | 15 ++++++- .../src/object/field_get_set/has_property.rs | 20 +++++++-- 5 files changed, 90 insertions(+), 7 deletions(-) diff --git a/crates/perry-hir/src/lower/for_head.rs b/crates/perry-hir/src/lower/for_head.rs index 81cd25fbd4..e95256b43f 100644 --- a/crates/perry-hir/src/lower/for_head.rs +++ b/crates/perry-hir/src/lower/for_head.rs @@ -208,3 +208,46 @@ pub(crate) fn for_head_binding_stmts( } } } + +/// Wrap a desugared for-in loop body so a key that is deleted from the +/// receiver *before it is visited* is skipped, per ECMAScript for-in deletion +/// semantics (EnumerateObjectProperties: "If a property that has not yet been +/// visited during enumeration is deleted, then it will not be visited"). +/// +/// The keys are snapshotted once (`ForInKeys`), so without this guard a key +/// deleted mid-iteration would still be visited. `obj_id` holds the receiver +/// (spilled to a temp by the caller so it can be re-read each iteration), +/// `keys_id`/`idx_id` the snapshot array and cursor. +/// +/// A primitive string is the only primitive whose for-in snapshot is non-empty +/// (its indices); its keys cannot be deleted, and the `in` operator *throws* on +/// a primitive receiver — so strings bypass the recheck and are always visited. +/// Objects/functions go through `key in obj`, which is `false` for a deleted +/// key and skips it. Nullish receivers never reach here (empty snapshot). +pub(crate) fn guard_for_in_body( + obj_id: LocalId, + keys_id: LocalId, + idx_id: LocalId, + body: Vec, +) -> Vec { + let guard = Expr::Conditional { + condition: Box::new(Expr::Compare { + op: CompareOp::Eq, + left: Box::new(Expr::TypeOf(Box::new(Expr::LocalGet(obj_id)))), + right: Box::new(Expr::String("string".to_string())), + }), + then_expr: Box::new(Expr::Bool(true)), + else_expr: Box::new(Expr::In { + property: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(keys_id)), + index: Box::new(Expr::LocalGet(idx_id)), + }), + object: Box::new(Expr::LocalGet(obj_id)), + }), + }; + vec![Stmt::If { + condition: guard, + then_branch: body, + else_branch: None, + }] +} diff --git a/crates/perry-hir/src/lower/mod.rs b/crates/perry-hir/src/lower/mod.rs index 62ef5db3f2..08e65a4ccc 100644 --- a/crates/perry-hir/src/lower/mod.rs +++ b/crates/perry-hir/src/lower/mod.rs @@ -49,7 +49,7 @@ mod stmt; mod unimpl_hints; pub(crate) use stmt::*; mod for_head; -pub(crate) use for_head::{for_head_binding_stmts, predefine_for_head}; +pub(crate) use for_head::{for_head_binding_stmts, guard_for_in_body, predefine_for_head}; mod stmt_loops; pub(crate) use stmt_loops::{ insert_iterator_close_on_abrupt, lazy_iter_for_stmt, lazy_or_index_elem, lower_stmt_for_in, diff --git a/crates/perry-hir/src/lower/stmt_loops.rs b/crates/perry-hir/src/lower/stmt_loops.rs index 43f64fe754..ea43e01142 100644 --- a/crates/perry-hir/src/lower/stmt_loops.rs +++ b/crates/perry-hir/src/lower/stmt_loops.rs @@ -1893,15 +1893,25 @@ pub(crate) fn lower_stmt_for_in( // lowered below can reference them). let head_binding = predefine_for_head(ctx, &for_in_stmt.left, Type::String)?; - // Lower the object expression + // Lower the object expression once, spilling it into a temp so each + // iteration can re-check that the current key still exists on the + // receiver (for-in deletion semantics — see `guard_for_in_body`). let obj_expr = lower_expr(ctx, &for_in_stmt.right)?; + let obj_id = ctx.fresh_local(); + module.init.push(Stmt::Let { + id: obj_id, + name: format!("__forin_obj_{}", obj_id), + ty: Type::Any, + mutable: false, + init: Some(obj_expr), + }); // for-in enumerates the receiver's own AND inherited enumerable string // keys (deduplicated), and is a no-op — not a throw — on null/undefined. // `ForInKeys` carries those semantics; `ObjectKeys` (Object.keys) would // throw on nullish and miss inherited keys. Refs language/statements/for-in // S12.6.4_A1/A2 (nullish) and A6/A6.1 (prototype chain). - let keys_expr = Expr::ForInKeys(Box::new(obj_expr)); + let keys_expr = Expr::ForInKeys(Box::new(Expr::LocalGet(obj_id))); // Create internal variables for the keys array and index let keys_id = ctx.fresh_local(); @@ -1929,6 +1939,9 @@ pub(crate) fn lower_stmt_for_in( loop_body.insert(i, stmt); } + // Skip keys deleted from the receiver before they are visited. + let loop_body = guard_for_in_body(obj_id, keys_id, idx_id, loop_body); + // Create the for loop: // for (let __i = 0; __i < __keys.length; __i++) { ... } module.init.push(Stmt::For { diff --git a/crates/perry-hir/src/lower_decl/body_stmt.rs b/crates/perry-hir/src/lower_decl/body_stmt.rs index 071d058fb9..6f3ee79827 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt.rs @@ -1809,10 +1809,20 @@ pub fn lower_body_stmt(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result Result break, + // No explicit static `[[Prototype]]` recorded. But `Object.create(proto)` + // and `Function.prototype = obj` model the prototype link via a synthetic + // class_id → prototype object (`CLASS_PROTOTYPE_OBJECTS`), which the + // recorded-static-prototype walk above can't see. Without hopping it, + // `key in Object.create({ key: … })` — and even inherited + // `Object.prototype` members on such a receiver (its synthetic class_id + // makes the `Object.prototype` tail below bail) — were wrongly reported + // absent. Hop through that synthetic prototype object and continue; the + // field-GET path resolves the same chain via `resolve_proto_chain_field`. + None => { + let synth_proto = crate::object::class_prototype_object(unsafe { (*cur).class_id }); + if !synth_proto.is_null() && synth_proto as *const ObjectHeader != cur { + cur = synth_proto as *const ObjectHeader; + continue; + } + break; + } } } // Wall 10 — a class instance's prototype METHODS / GETTERS / SETTERS live in From 2959df1e4bfab8064d5755368db4a5f57c644469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 8 Jul 2026 22:34:24 +0200 Subject: [PATCH 2/2] fix(runtime): guard Object.create synthetic-proto hop against array prototype nodes Address CodeRabbit review on #6147: a prototype hop in `ordinary_has_property` can land on a real `ArrayHeader` (`Foo.prototype = [1,2,3]`), which has no `class_id` field. Skip the synthetic-prototype lookup when `cur_is_array` so the array's `length`/`capacity` bytes are never misread as a class id. --- .../src/object/field_get_set/has_property.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/object/field_get_set/has_property.rs b/crates/perry-runtime/src/object/field_get_set/has_property.rs index a50326c0a9..88b087ec58 100644 --- a/crates/perry-runtime/src/object/field_get_set/has_property.rs +++ b/crates/perry-runtime/src/object/field_get_set/has_property.rs @@ -685,10 +685,18 @@ unsafe fn ordinary_has_property( // absent. Hop through that synthetic prototype object and continue; the // field-GET path resolves the same chain via `resolve_proto_chain_field`. None => { - let synth_proto = crate::object::class_prototype_object(unsafe { (*cur).class_id }); - if !synth_proto.is_null() && synth_proto as *const ObjectHeader != cur { - cur = synth_proto as *const ObjectHeader; - continue; + // A prototype hop can land on a real `ArrayHeader` (`Foo.prototype + // = [1,2,3]`), whose layout has no `class_id` field — reading one + // would misinterpret the array's `length`/`capacity` as a class id + // and could spuriously hop. Arrays never model a synthetic + // prototype, so skip the lookup for them. + if !cur_is_array { + let synth_proto = + crate::object::class_prototype_object(unsafe { (*cur).class_id }); + if !synth_proto.is_null() && synth_proto as *const ObjectHeader != cur { + cur = synth_proto as *const ObjectHeader; + continue; + } } break; }