From 42ba24f3da05d5ff42fc3bb2bd84d9f1331018f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 10:09:07 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(codegen):=20a=20rejected=20sloppy=20`o.?= =?UTF-8?q?x=20+=3D=201`=20/=20`for=20(o.x=20of=20=E2=80=A6)`=20/=20`[o.x]?= =?UTF-8?q?=20=3D=20arr`=20no=20longer=20throws=20(#9459)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // sloppy (.cts, no "use strict") const o = {x:1}; Object.freeze(o); o.x += 1; // node: silent Perry: TypeError for (o.x of [7]) {} // node: silent Perry: TypeError [o.x] = [7]; // node: silent Perry: TypeError (expression position) o[k] += 1; // node: silent Perry: TypeError o.x++; // node: silent Perry: silent (correct, Expr::PropertyUpdate) o.x = 9; // node: silent Perry: silent (correct, Expr::PutValueSet) ES2024 6.2.5.7 (PutValue) performs Set(O, P, V, Throw) with Throw = IsStrictReference(ref), and 10.1.9 (OrdinarySet) reports `false` -- not a throw -- for a non-writable own or inherited data property, an accessor with no setter, and a new property on a non-extensible object. The reference's own strictness is what turns that `false` into a TypeError. The ordinary-object mirror of #9394 (arrays, fixed by #9426) and the opposite direction from #9422 (an under-throw in strict code). A CommonJS bundle is sloppy top to bottom, so this was a hard failure: a spurious TypeError stopped a program node runs to completion. Root cause: `Expr::PropertySet` carries no strictness field at all, and its codegen tail reaches `js_typed_feedback_object_set_field_by_name_fast` -> `js_object_set_field_by_name`, which has no `strict` parameter and rejects by throwing. `o.x++` was right because it lowers to `Expr::PropertyUpdate` (carries `ctx.current_strict`); `o.x = 9` was right because it lowers to `Expr::PutValueSet` (carries `strict`). Only the spellings that lower to `Expr::PropertySet` -- compound and logical assignment, for-of heads, expression-position destructuring targets -- had no answer to give. The same hole existed on `Expr::IndexSet`'s OBJECT-by-name arms, which #9426 left behind when it carried the flag to that node's array element lanes. The flag comes from the CONTEXT, exactly as #9426 did for `Expr::IndexSet`: `ctx.is_strict_fn` at the ordinary dispatch, `PutValueSet::strict` at the two sites that synthesize a `PropertySet` from a `PutValue`. Deliberately not a new HIR field: `Expr::PropertySet` has 181 mentions across the workspace (119 constructions, 54 in production code), and a large minority live in collectors and transform passes that REBUILD an existing node with no strictness context to copy -- exactly where a wrong default hides. `FnCtx::is_strict_fn` is already the audited answer for the enclosing code (`Function::is_strict`, `Expr::Closure::is_strict`, `Module::init_is_strict` from #9458, and a hard `true` for class methods). Sloppy stores route to `js_put_value_set(target, key, value, receiver, 0)` -- the receiver-aware [[Set]] sloppy `o.x = v` has always used -- so the spellings agree instead of diverging by lane. The class-field fast arm is preserved through `try_lower_sloppy_class_field_store` (#7288/#5094), whose #5093 inline precheck declines every receiver whose store could be rejected, so that arm is mode-independent and only its miss needed a sloppy tail. Strict lowering is byte-identical to before. Two IR tests moved, both because their fixture builders hard-code `is_strict: false` while their subject (the typed-feedback PropertySet site, the property-id store ABI) lives on the strict lane -- the same expectation move #9458 made when `Module::init_is_strict` landed. Each is now asserted on the strict lane AND given a sloppy twin, so neither invariant is pinned on only one of two tails. Verified byte-identical to `node --experimental-strip-types` on test-files/test_gap_9459_property_set_strictness.cts (19 lines differed on unfixed origin/main); perry-codegen --lib 1383 passed / 0 failed; targeted IR suites (typed_feedback, native_proof_regressions, scalar_replaced_slot_roots, class_field_store_pointer_test, shadow_slot_hygiene) 334 passed / 0 failed. Not changed, both pre-existing on main and documented in the fixture: `caller`/`arguments` keep their `js_object_set_field_by_name` route in both modes (that entry's poisoned-accessor handling is not a Throw-flag decision), and strict `+=` against an INHERITED rejecting receiver still skips the prototype walk -- a missing walk rather than a missing Throw flag, filed as #9495. --- .../9459-sloppy-property-set-strictness.md | 95 ++ crates/perry-codegen/src/expr/dispatch.rs | 13 +- crates/perry-codegen/src/expr/index_set.rs | 68 ++ crates/perry-codegen/src/expr/property_set.rs | 154 ++- .../perry-codegen/src/expr/proxy_reflect.rs | 8 + .../tests/native_proof_regressions.rs | 128 ++- crates/perry-codegen/tests/typed_feedback.rs | 87 +- .../test_gap_9459_property_set_strictness.cts | 965 ++++++++++++++++++ 8 files changed, 1510 insertions(+), 8 deletions(-) create mode 100644 changelog.d/9459-sloppy-property-set-strictness.md create mode 100644 test-files/test_gap_9459_property_set_strictness.cts diff --git a/changelog.d/9459-sloppy-property-set-strictness.md b/changelog.d/9459-sloppy-property-set-strictness.md new file mode 100644 index 0000000000..099046e4e1 --- /dev/null +++ b/changelog.d/9459-sloppy-property-set-strictness.md @@ -0,0 +1,95 @@ +### Fixed + +- **A rejected `o.x += 1` / `for (o.x of …)` / `[o.x] = arr` no longer throws in + sloppy code.** + + ```js + // sloppy (.cts, no "use strict") + const o = {x:1}; Object.freeze(o); + o.x += 1; // node: silent Perry: TypeError + for (o.x of [7]) {} // node: silent Perry: TypeError + [o.x] = [7]; // node: silent Perry: TypeError (expression position) + o.x++; // node: silent Perry: silent (correct — Expr::PropertyUpdate) + o.x = 9; // node: silent Perry: silent (correct — Expr::PutValueSet) + ``` + + ES2024 §6.2.5.7 (`PutValue`) performs `Set(O, P, V, Throw)` with + `Throw = IsStrictReference(ref)`, and §10.1.9 (`OrdinarySet`) reports `false` + — not a throw — for a non-writable own or inherited data property, an + accessor with no setter, and a new property on a non-extensible object. The + reference's own strictness is what turns that `false` into a `TypeError`. + + This is the ordinary-object mirror of #9394 (arrays, fixed by #9426) and the + opposite direction from #9422 (an *under*-throw in strict code). A CommonJS + bundle is sloppy from top to bottom, so this was a hard failure — a program + node runs to completion stopped with a spurious `TypeError`. + + Root cause: `Expr::PropertySet` carries **no strictness field at all**, and + its codegen tail reaches `js_typed_feedback_object_set_field_by_name_fast` → + `js_object_set_field_by_name`, which has no `strict` parameter and rejects by + throwing unconditionally. `o.x++` was already right because it lowers to + `Expr::PropertyUpdate`, which carries `ctx.current_strict`; `o.x = 9` was + already right because it lowers to `Expr::PutValueSet`, which carries + `strict`. Only the spellings that lower to `Expr::PropertySet` — compound and + logical assignment, `for`-of heads, and expression-position destructuring + targets — had no answer to give. + + The flag comes from the **context**, exactly as #9426 did for + `Expr::IndexSet`: `ctx.is_strict_fn` at the ordinary dispatch, and + `PutValueSet::strict` at the two sites that synthesize a `PropertySet` from a + `PutValue`. That is deliberately not a new HIR field — `Expr::PropertySet` has + 181 mentions across the workspace (119 constructions, 54 of them in production + code), and a large minority of those live in collectors and transform passes + that *rebuild* an existing node with no strictness context to copy from. A + field would have needed a default at each of those, which is precisely where a + wrong answer hides. `FnCtx::is_strict_fn` is already the audited answer for + the enclosing code: `Function::is_strict`, `Expr::Closure::is_strict`, + `Module::init_is_strict` (#9458), and a hard `true` for class methods. + + Sloppy stores route to `js_put_value_set(target, key, value, receiver, 0)` — + the receiver-aware `[[Set]]` that sloppy `o.x = v` has always used — so the + three spellings now agree instead of diverging by lane. The class-field fast + arm is preserved through `try_lower_sloppy_class_field_store` (#7288/#5094), + whose #5093 inline precheck declines every receiver whose store could be + rejected, so the fast path is mode-independent and only its miss needed a + sloppy-correct tail. Strict lowering is byte-identical to before. + + - `crates/perry-codegen/src/expr/dispatch.rs` — `Expr::PropertySet` passes + `ctx.is_strict_fn`, the twin of the `Expr::IndexSet` line above it. + - `crates/perry-codegen/src/expr/proxy_reflect.rs` — the two `PutValueSet` + routes into `property_set::lower` pass the reference's own `strict`. + - `crates/perry-codegen/src/expr/property_set.rs` — `lower` takes + `assignment_strict`; the `arr.length` arm (`js_array_set_length_strict`) and + the class-field arms (`js_class_field_set_ic` / + `js_class_field_set_fallback`) are strict-only; a new + `lower_sloppy_property_set_by_name` emits the `js_put_value_set(…, 0)` tail + with the same #7154 receiver-rooting window and the same nullish-receiver + guard the strict tail uses (`undefined.x = 1` is a `TypeError` in both + modes — `GetValue` on the base runs before `PutValue` consults `Throw`). + - `test-files/test_gap_9459_property_set_strictness.cts` — `+=`, `-=`, `*=`, + `&&=`, `||=`, `??=`, `for`-of heads (named and computed), `[o.x] = arr` in + statement *and* expression position, `[o[k]] = arr`, `({a: o.x} = obj)`, + against frozen / sealed / non-writable own / inherited non-writable / + getter-only own and inherited / inherited-setter / `preventExtensions` / + frozen class-field / frozen `arr.length` receivers — **both modes**, with + accepted-store and short-circuit controls so a fix that simply stopped + storing would fail. + + Two things this deliberately does not change, both pre-existing on `main` and + both visible in the fixture's own comments: + + - `caller` / `arguments` keep their `js_object_set_field_by_name` route in + both modes. `PutValueSet` sends those two names here specifically to reach + that entry's poisoned-accessor handling, which is not a `Throw`-flag + decision. + - `+=` against a receiver whose rejection lives on the **prototype** (an + inherited non-writable data property, an inherited getter-only accessor, an + inherited setter) is still wrong in **strict** code: the strict tail does an + own-property store and never runs `OrdinarySetWithOwnDescriptor`'s + prototype walk, so the setter does not fire and an own property is created. + That is a missing prototype walk rather than a missing `Throw` flag, it is + wrong on unfixed `main` in both modes, and fixing it means retargeting the + typed-feedback store site that #7480/#5093 gate with their own IR tests. + Filed as #9495. The sloppy half becomes correct here as a side effect of + routing to `js_put_value_set`, and the fixture pins the strict `=` twins so + that change has a baseline. diff --git a/crates/perry-codegen/src/expr/dispatch.rs b/crates/perry-codegen/src/expr/dispatch.rs index e04d1edcba..89ada2b9ed 100644 --- a/crates/perry-codegen/src/expr/dispatch.rs +++ b/crates/perry-codegen/src/expr/dispatch.rs @@ -64,7 +64,18 @@ pub(crate) fn lower_expr(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let strict = ctx.is_strict_fn; super::index_set::lower(ctx, expr, value_discarded, strict) } - Expr::PropertySet { .. } => super::property_set::lower(ctx, expr), + Expr::PropertySet { .. } => { + // #9459, twin of the `IndexSet` line above (#9426): `Expr::PropertySet` + // carries no strictness of its own, so the assignment's `Throw` flag + // comes from the CONTEXT it is lowered in. Every spelling that reaches + // this arm -- `o.x += 1`, `for (o.x of it)`, `[o.x] = arr` -- is a + // `PutValue` on a property reference whose strictness is the enclosing + // code's (ES2024 SS6.2.5.7). A `PutValueSet` that routes here instead + // carries the reference's own flag and passes it explicitly + // (`expr/proxy_reflect.rs`). + let strict = ctx.is_strict_fn; + super::property_set::lower(ctx, expr, strict) + } Expr::PropertyGet { .. } => super::property_get::lower(ctx, expr), Expr::Conditional { .. } => super::conditional::lower(ctx, expr), Expr::ArrayPush { .. } | Expr::ArrayPushSpread { .. } => { diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 2366d4d775..48bdd808f0 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -355,6 +355,65 @@ fn lower_array_index_set_via_runtime_key( ) } +/// #9459: the SLOPPY object-by-name tail for `Expr::IndexSet`. +/// +/// The two string-key arms below reach `js_typed_feedback_object_set_field_by_name` +/// → `js_object_set_field_by_name`, which has no `strict` parameter and rejects a +/// non-writable slot by throwing. #9426 carried `assignment_strict` to the ARRAY +/// element lanes; the object-by-name lanes on the same node still threw, so +/// `for (o[k] of …)` and `o[k] += 1` on a frozen object stopped a sloppy program +/// node runs to completion. +/// +/// `Set(O, ToPropertyKey(k), V, false)` is `js_put_value_set(target, key, value, +/// receiver, 0)` — the same entry sloppy `o[k] = v` already reaches, because +/// `put_value_index_fast_path` keeps statically-known string keys off this file +/// entirely (`expr/proxy_reflect.rs`). Routing here makes the two spellings agree. +/// +/// Rooting is the #7639/#7201 window the strict arms open: receiver AND key are +/// live across `value`'s lowering, which is arbitrary user code and can drive an +/// evacuating minor. `target` and `receiver` are one evaluation of the base, so +/// the single lowered box fills both operand slots. +fn lower_sloppy_object_index_set( + ctx: &mut FnCtx<'_>, + object: &Expr, + index: &Expr, + value: &Expr, +) -> Result { + rooting::with_operands_rooted_across( + ctx, + &[object, index], + &[value], + |ctx| { + lower_value_for_dynamic_index_set( + ctx, + value, + "index_set.sloppy_object_value_bits", + "sloppy_object_index_set_helper_edge", + ) + }, + |ctx, vals, (val_double, _val_bits)| { + let (obj_box, key_box) = (vals[0].clone(), vals[1].clone()); + let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); + // Same guard the strict arms emit: `undefined[k] = 1` is a TypeError + // in BOTH modes -- `GetValue` on the base runs before `PutValue` + // ever consults `Throw`. + super::property_set::emit_nullish_write_guard(ctx, &obj_bits, "index", "iset.sloppy"); + let _ = ctx.block().call( + DOUBLE, + "js_put_value_set", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &obj_box), + (I32, "0"), + ], + ); + Ok(val_double) + }, + ) +} + pub(crate) fn lower( ctx: &mut FnCtx<'_>, expr: &Expr, @@ -1399,6 +1458,11 @@ pub(crate) fn lower( // `obj_box`. Root it across the evaluation and re-read below. // The key is a literal, so it is not an operand here at all — // it is interned into the string pool below. + // + // #9459: strict only -- see `lower_sloppy_object_index_set`. + if !assignment_strict { + return lower_sloppy_object_index_set(ctx, object, index, value); + } return rooting::with_operands_rooted_across( ctx, &[object.as_ref()], @@ -1459,6 +1523,10 @@ pub(crate) fn lower( ); } if is_string_expr(ctx, index) { + // #9459: strict only -- see `lower_sloppy_object_index_set`. + if !assignment_strict { + return lower_sloppy_object_index_set(ctx, object, index, value); + } // #7154: see the literal-key arm above, plus the KEY, which // sits in the same window. A non-literal string key is an // ordinary heap string with no registered root of its own, so diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 6f32c5f57b..6102a6c420 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -557,7 +557,15 @@ fn lower_runtime_property_set_by_name( object: &Expr, property: &str, value: &Expr, + // #9459: `js_object_set_field_by_property_id` resolves the dispatch id and + // hands the key to `js_object_set_field_by_name`, which has no `strict` + // parameter and rejects by throwing. Correct for a strict `PutValue`, wrong + // for sloppy. + assignment_strict: bool, ) -> Result { + if !assignment_strict { + return lower_sloppy_property_set_by_name(ctx, object, property, value); + } // #7154: root the receiver across the value's evaluation, which allocates. // The group re-reads it as part of emitting the store, so no register of // the receiver exists across the window. @@ -576,6 +584,67 @@ fn lower_runtime_property_set_by_name( }) } +/// #9459: the SLOPPY terminal store for `Expr::PropertySet`. +/// +/// `Set(O, P, V, false)` -- ordinary `[[Set]]` with the receiver, and a +/// rejection (frozen / sealed / non-writable own or inherited data property / +/// getter-only accessor / non-extensible new key) reported as `false` and +/// discarded rather than thrown. That is exactly `js_put_value_set(target, key, +/// value, receiver, 0)`, the entry sloppy `o.x = v` has always used through +/// `Expr::PutValueSet`; routing here makes `o.x += 1`, `for (o.x of it)` and +/// `[o.x] = arr` agree with it instead of throwing where node is silent. +/// +/// `target` and `receiver` are the same expression, evaluated ONCE -- the +/// property reference's base is one evaluation, and `with_operands_rooted` +/// hands the single lowered box to both operand slots. +/// +/// Rooting is the #7154 window the strict tail also opens: the receiver is live +/// across `value`'s lowering, which is arbitrary user code and can drive an +/// evacuating minor. +fn lower_sloppy_property_set_by_name( + ctx: &mut FnCtx<'_>, + object: &Expr, + property: &str, + value: &Expr, +) -> Result { + rooting::with_operands_rooted_across( + ctx, + &[object], + &[value], + |ctx| { + lower_value_for_dynamic_property_set( + ctx, + value, + "property_set.sloppy_dynamic_value_bits", + "sloppy_property_set_helper_edge", + ) + }, + |ctx, vals, (val_double, _val_bits)| { + let obj_box = vals[0].clone(); + let key_idx = ctx.strings.intern(property); + let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); + let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); + // The nullish receiver check is the same one the strict tail emits: + // `undefined.x = 1` is a TypeError in BOTH modes (GetValue on the + // base runs before PutValue's Throw flag is ever consulted). + emit_nullish_write_guard(ctx, &obj_bits, property, "pset_sloppy"); + let key_box = ctx.block().load(DOUBLE, &key_handle_global); + let _ = ctx.block().call( + DOUBLE, + "js_put_value_set", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &obj_box), + (I32, "0"), + ], + ); + Ok(val_double) + }, + ) +} + fn lower_value_for_dynamic_property_set( ctx: &mut FnCtx<'_>, value: &Expr, @@ -638,7 +707,19 @@ pub(crate) fn emit_nullish_write_guard( ctx.current_block = ok_idx; } -pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { +/// Lower an `Expr::PropertySet`. +/// +/// `assignment_strict` is the assignment's own `Throw` flag (ES2024 SS6.2.5.7 +/// `PutValue` calls `Set(O, P, V, Throw)` with `Throw = IsStrictReference`). +/// #9459: the HIR node carries no strictness, so it comes from the caller -- +/// `ctx.is_strict_fn` for the ordinary dispatch (`expr/dispatch.rs`, the same +/// source `Expr::IndexSet` uses since #9426), and `PutValueSet::strict` for the +/// two routes that synthesize a `PropertySet` from a `PutValue` +/// (`expr/proxy_reflect.rs`). A rejected SLOPPY `[[Set]]` is a silent no-op, so +/// every arm below whose runtime entry rejects by THROWING is strict-only; the +/// sloppy twin of each is the strictness-aware `js_put_value_set(..., 0)` that +/// the surrounding `PutValueSet` lowering already uses for `o.x = v`. +pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) -> Result { match expr { Expr::PropertySet { object, @@ -672,7 +753,20 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // (route to js_array_set_length when the target is registered as // an array). Deliberately out of scope here; the static-typed // case covers the issue's repro. - if property == "length" && crate::type_analysis::is_array_expr(ctx, object) { + // + // #9459: strict only. `js_array_set_length_strict` is named for the + // `Throw` flag it hard-codes -- `Set(O, "length", n, true)`. A SLOPPY + // `arr.length` write that `OrdinarySet` rejects (frozen array, or an + // explicit `writable: false` on `length`) must be a silent no-op, so + // sloppy falls through to the generic `js_put_value_set(..., 0)` tail + // below. That is already where sloppy `arr.length = 0` goes today -- + // `put_value_static_property_fast_path` refuses this arm for sloppy + // references (`expr/proxy_reflect.rs`), and #9422's fixture pins the + // result -- so the two spellings agree rather than diverging by lane. + if assignment_strict + && property == "length" + && crate::type_analysis::is_array_expr(ctx, object) + { // #7637: this arm had NO store-operand guard, while every other // `PropertySet` arm in this file has had one since #7154. It is // the same window: `arr.length = f()` lowers the receiver first @@ -906,7 +1000,13 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if class_has_computed_runtime_members(ctx, &class_name) && !ctx.is_static_class_this(object) { - return lower_runtime_property_set_by_name(ctx, object, property, value); + return lower_runtime_property_set_by_name( + ctx, + object, + property, + value, + assignment_strict, + ); } let setter_key = (class_name.clone(), format!("__set_{}", property)); // STATIC accessors compile under the static (no-`this`) @@ -920,7 +1020,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if let Some(fn_name) = ctx.methods.get(&setter_key).cloned() { if proven_class_name.is_none() { return lower_runtime_property_set_by_name( - ctx, object, property, value, + ctx, + object, + property, + value, + assignment_strict, ); } return with_class_store_operands( @@ -938,6 +1042,35 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { ); } } + // #9459: SLOPPY code stops here. Every class-field arm below + // terminates in `js_class_field_set_ic` / + // `js_class_field_set_fallback`, whose miss path is + // `js_object_set_field_by_name` -- no `strict` parameter, rejects + // by throwing. That is the same reason + // `put_value_static_property_fast_path` bars sloppy references + // from this route for `PutValueSet` (#6542), and the recovery is + // the same one #7288/#5094 built for that lowering: the #5093 + // inline precheck declines every receiver whose store could be + // REJECTED (frozen, descriptor-bearing, wrong class or keys token, + // accessor in the chain), so its fast arm is mode-independent and + // only its miss needed a sloppy-correct tail. A decline lands on + // the same `js_put_value_set(..., 0)` the generic sloppy tail uses, + // so the two are one behaviour with two speeds. + // + // The setter-dispatch arm above is deliberately AHEAD of this: a + // compiled `__set_` accessor runs in both modes, and a + // setter that throws does so because of its own body, not because + // of the assignment's `Throw` flag. + if !assignment_strict { + if matches!(object.as_ref(), Expr::LocalGet(_) | Expr::This) { + if let Some(result) = + try_lower_sloppy_class_field_store(ctx, object, property, value)? + { + return Ok(result); + } + } + return lower_sloppy_property_set_by_name(ctx, object, property, value); + } // Fast path: known class instance + plain instance field. // The runtime guard checks the receiver's class/shape and // descriptor state before this block touches the raw slot. @@ -1659,6 +1792,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } } } + // #9459: the generic SLOPPY tail. The strict tail below ends in + // `js_typed_feedback_object_set_field_by_name_fast`, whose underlying + // `js_object_set_field_by_name` has no `strict` parameter and rejects + // by throwing; sloppy `PutValue` must discard the rejection instead. + // + // `caller`/`arguments` are excluded in BOTH modes: those two names are + // routed here from `PutValueSet` specifically to reach + // `js_object_set_field_by_name`'s poisoned-accessor handling, which is + // not a `Throw`-flag decision, and diverting them would change + // behaviour this issue is not about. + if !assignment_strict && !matches!(property.as_str(), "caller" | "arguments") { + return lower_sloppy_property_set_by_name(ctx, object, property, value); + } // #7154: the value expression can collect, and an evacuating minor // inside it relocates the receiver out from under `obj_box` -- // `obj.k = f()` then writes `k` into abandoned from-space memory diff --git a/crates/perry-codegen/src/expr/proxy_reflect.rs b/crates/perry-codegen/src/expr/proxy_reflect.rs index 8bb1a87cc4..23ea00e5a4 100644 --- a/crates/perry-codegen/src/expr/proxy_reflect.rs +++ b/crates/perry-codegen/src/expr/proxy_reflect.rs @@ -1905,6 +1905,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { property: property.clone(), value: value.clone(), }, + // #9459: the reference's own `Throw` flag, not the + // enclosing function's -- module init is a synthetic + // function whose strictness is `Module::init_is_strict` + // (#9458), and `PutValueSet` already carries the right + // answer for every reference it desugars. + *strict, ); } } @@ -1918,6 +1924,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { property, value: value.clone(), }, + // #9459: see the `caller`/`arguments` route above. + *strict, ); } // #7288: sloppy code is barred from the class-field route above diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 7d8e0c5330..2fc630df19 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -150,6 +150,29 @@ fn module_with_classes_and_params( } } +/// #9459: the same module, with `probe` (and module init) STRICT. +/// +/// `module_with_classes_and_params` hard-codes `is_strict: false`, which used to +/// be invisible: `Expr::PropertySet` carried no strictness and every store took +/// one lowering. It now takes the assignment's own `Throw` flag from +/// `FnCtx::is_strict_fn`, so a test whose subject is the STRICT store lane has +/// to say so — the same expectation move #9458 made across the codegen test +/// files when `Module::init_is_strict` was introduced. +fn strict_module_with_classes_and_params( + name: &str, + classes: Vec, + params: Vec, + return_type: Type, + body: Vec, +) -> Module { + let mut module = module_with_classes_and_params(name, classes, params, return_type, body); + module.init_is_strict = true; + for function in &mut module.functions { + function.is_strict = true; + } + module +} + fn compile_ir(name: &str, body: Vec) -> String { compile_ir_with_opts(name, body, empty_opts()) } @@ -6913,7 +6936,13 @@ fn artifact_records_array_push_value_bits_before_slot_store() { #[test] fn artifact_records_dynamic_property_set_value_bits_before_helper() { - let module = module_with_classes_and_params( + // #9459: STRICT. The subject is a representation invariant -- the RHS stays + // in `js_value_bits` until the runtime helper edge boxes it -- and it is + // asserted here on the strict tail. The sloppy tail is a different runtime + // entry (`js_put_value_set(..., 0)`, so a rejected sloppy `[[Set]]` is a + // silent no-op) and gets its own twin below, because a representation + // invariant that holds on only one of two tails is not an invariant. + let module = strict_module_with_classes_and_params( "artifact_property_set_slot_js_value_bits.ts", Vec::new(), vec![param(1, "obj", Type::Any), param(2, "value", Type::Any)], @@ -6944,6 +6973,45 @@ fn artifact_records_dynamic_property_set_value_bits_before_helper() { ); } +/// #9459: the sloppy twin of the test above. +/// +/// Same module, `is_strict: false`, so the store takes the sloppy tail. The +/// representation contract is identical -- the RHS is lowered by +/// `lower_value_for_dynamic_property_set` and stays `js_value_bits` until the +/// helper edge -- only the consumer/boxed-at labels and the runtime entry differ. +#[test] +fn artifact_records_sloppy_dynamic_property_set_value_bits_before_helper() { + let module = module_with_classes_and_params( + "artifact_property_set_slot_js_value_bits_sloppy.ts", + Vec::new(), + vec![param(1, "obj", Type::Any), param(2, "value", Type::Any)], + Type::Number, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(local(1)), + property: "field".to_string(), + value: Box::new(local(2)), + }), + Stmt::Return(Some(int(0))), + ], + ); + + let artifact = compile_artifact_json_for_module(module); + let records = artifact["records"].as_array().unwrap(); + assert!( + records.iter().any(|record| { + record["expr_kind"] == "PropertySet" + && record["consumer"] == "property_set.sloppy_dynamic_value_bits" + && record["native_rep_name"] == "js_value_bits" + && record["llvm_ty"] == "i64" + && record["native_value_state"] == "region_local" + && record["access_mode"].is_null() + && record_has_note(record, "boxed_at=sloppy_property_set_helper_edge") + }), + "expected sloppy dynamic property-set RHS to stay as js_value_bits before the helper edge:\n{artifact:#}" + ); +} + #[test] fn artifact_records_dynamic_index_set_value_bits_before_helper() { let module = module_with_classes_and_params( @@ -14423,8 +14491,13 @@ fn scalar_method_int32_bitwise_rejects_unproven_or_unsigned_shapes() { #[test] fn static_property_access_on_computed_class_uses_property_id_wrappers() { + // #9459: STRICT. The property-id store ABI + // (`js_object_set_field_by_property_id` -> `js_object_set_field_by_name`) + // rejects a non-writable slot by throwing, which is correct only for a + // strict `PutValue`; the sloppy tail is asserted by the twin below. The + // READ side is strictness-independent and is asserted in both. let dynamic = class_with_computed_member(141, "DynamicShape", vec![]); - let module = module_with_classes_and_params( + let module = strict_module_with_classes_and_params( "property_id_static_access.ts", vec![dynamic], vec![ @@ -14457,6 +14530,57 @@ fn static_property_access_on_computed_class_uses_property_id_wrappers() { ); } +/// #9459: the sloppy twin of the test above. +/// +/// A sloppy store must NOT take the property-id ABI: that route ends in +/// `js_object_set_field_by_name`, which has no `strict` parameter and throws on +/// a rejected `[[Set]]`. It goes to `js_put_value_set(..., 0)` instead. The READ +/// keeps the property-id ABI in both modes -- strictness is a property of +/// `PutValue`, not of `GetValue` -- and asserting it here is what keeps this +/// test from passing on an empty program. +#[test] +fn sloppy_static_property_store_on_computed_class_avoids_property_id_setter() { + let dynamic = class_with_computed_member(141, "DynamicShape", vec![]); + let module = module_with_classes_and_params( + "property_id_static_access_sloppy.ts", + vec![dynamic], + vec![ + param(1, "obj", Type::Named("DynamicShape".to_string())), + param(2, "value", Type::Number), + ], + Type::Number, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(local(1)), + property: "score".to_string(), + value: Box::new(local(2)), + }), + Stmt::Return(Some(Expr::PropertyGet { + byte_offset: 0, + object: Box::new(local(1)), + property: "score".to_string(), + })), + ], + ); + + let ir = compile_ir_for_module_with_opts(module, empty_opts()).unwrap(); + assert!( + !ir.contains("call void @js_object_set_field_by_property_id"), + "a sloppy store must not reach the throwing property-id setter:\n{ir}" + ); + // The CALL, not the symbol -- `js_put_value_set` is `declare`d in every + // module, so matching the bare name would pass on a program with no store. + assert!( + ir.contains("call double @js_put_value_set("), + "a sloppy store should reach the strictness-aware [[Set]]:\n{ir}" + ); + assert!( + ir.contains("call double @js_object_get_field_by_property_id_f64"), + "computed-member class static property reads keep the property-id ABI in \ + sloppy code too:\n{ir}" + ); +} + #[test] fn static_name_method_fallback_uses_rodata_method_id_wrapper() { let module = module_with_classes_and_params( diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index b1d5bb35b6..c84fd1cd02 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -199,6 +199,25 @@ fn module(name: &str, params: Vec, return_type: Type, body: Vec) -> module_with_classes(name, Vec::new(), params, return_type, body) } +/// #9459: the same module with `probe` (and module init) STRICT. +/// +/// `module_with_classes` hard-codes `is_strict: false`, which used to be +/// invisible: `Expr::PropertySet` carried no strictness and every store took one +/// lowering. It now takes the assignment's own `Throw` flag from +/// `FnCtx::is_strict_fn`, and only the STRICT store tail carries the +/// typed-feedback `PropertySet` site -- the sloppy tail is +/// `js_put_value_set(..., 0)`, so a rejected sloppy `[[Set]]` stays a silent +/// no-op. A test whose subject is that site has to ask for the lane it lives on, +/// the same expectation move #9458 made when `Module::init_is_strict` landed. +fn strict_module(name: &str, params: Vec, return_type: Type, body: Vec) -> Module { + let mut module = module_with_classes(name, Vec::new(), params, return_type, body); + module.init_is_strict = true; + for function in &mut module.functions { + function.is_strict = true; + } + module +} + fn module_with_classes( name: &str, classes: Vec, @@ -343,7 +362,11 @@ fn typed_feedback_instruments_property_and_method_boundaries() { // later tests in this binary never observe the changed environment. let _lock = env_lock(); let _env = EnvVarGuard::set("PERRY_TYPED_FEEDBACK", Some("1")); - let ir = ir_for(module( + // #9459: STRICT -- the `object_set_by_name_guard` site and the + // `js_typed_feedback_object_set_field_by_name_fast` dispatcher this asserts + // live on the strict store tail. See `strict_module`. The sloppy tail is + // asserted by `sloppy_property_set_uses_strictness_aware_put_value` below. + let ir = ir_for(strict_module( "typed_feedback_property.ts", vec![param(1, "obj", Type::Any)], Type::Any, @@ -386,6 +409,68 @@ fn typed_feedback_instruments_property_and_method_boundaries() { assert!(ir.contains("call void @js_typed_feedback_observe_property_get")); } +/// #9459: the sloppy twin of the boundaries test above. +/// +/// A sloppy `o.x = v` must not reach `js_object_set_field_by_name` -- it has no +/// `strict` parameter and rejects a non-writable slot by throwing, where sloppy +/// `PutValue` discards the rejection. It goes to `js_put_value_set(..., 0)`. +/// +/// The GET boundary is asserted too, and is deliberately unchanged: strictness +/// is a property of `PutValue`, not of `GetValue`. That assertion is also what +/// keeps this test from passing on an empty program -- every other check here is +/// a negative. +#[test] +fn sloppy_property_set_uses_strictness_aware_put_value() { + let _lock = env_lock(); + let _env = EnvVarGuard::set("PERRY_TYPED_FEEDBACK", Some("1")); + let ir = ir_for(module( + "typed_feedback_property_sloppy.ts", + vec![param(1, "obj", Type::Any)], + Type::Any, + vec![ + Stmt::Expr(Expr::PropertySet { + object: Box::new(Expr::LocalGet(1)), + property: "x".to_string(), + value: Box::new(Expr::Number(1.0)), + }), + Stmt::Return(Some(Expr::PropertyGet { + byte_offset: 0, + object: Box::new(Expr::LocalGet(1)), + property: "x".to_string(), + })), + ], + )); + + // Match the CALL, not the symbol: every runtime entry is `declare`d in + // every module, so `contains("js_put_value_set")` would pass on a module + // that never stores and `contains("...set_field_by_name_fast")` would fail + // on one that never does either. + assert!( + ir.contains("call double @js_put_value_set("), + "a sloppy property store should reach the strictness-aware [[Set]]:\n{ir}" + ); + assert!( + !ir.contains("call void @js_typed_feedback_object_set_field_by_name_fast("), + "the strict typed-feedback store dispatcher must not be CALLED on a \ + sloppy store -- its underlying setter throws on a rejected write:\n{ir}" + ); + assert!( + !ir.contains("object_set_by_name_guard"), + "no typed-feedback SET site is registered on the sloppy tail:\n{ir}" + ); + // ANTI-VACUITY: the reads are untouched by strictness and must still be + // instrumented, so the negatives above are about the store lane and not + // about an empty module. + assert!( + ir.contains("object_get_by_name_guard"), + "the property READ boundary is strictness-independent and must remain:\n{ir}" + ); + assert!( + ir.contains("js_object_get_field_by_name_f64"), + "the property read itself must still be lowered:\n{ir}" + ); +} + /// The negative twin of the test above, and the whole of #7480 step 4's second /// half: a DEFAULT build emits none of the pure-recording helpers. /// diff --git a/test-files/test_gap_9459_property_set_strictness.cts b/test-files/test_gap_9459_property_set_strictness.cts new file mode 100644 index 0000000000..3729dfc0fc --- /dev/null +++ b/test-files/test_gap_9459_property_set_strictness.cts @@ -0,0 +1,965 @@ +// #9459: `Expr::PropertySet` carries no strictness field, so every assignment +// form that lowers to it threw on a rejected `[[Set]]` in SLOPPY code, where +// node is silent. +// +// ES2024 SS6.2.5.7 (PutValue) performs `Set(O, P, V, Throw)` with +// `Throw = IsStrictReference(ref)`, and SS10.1.9 (OrdinarySet) reports `false` +// -- not a throw -- for a non-writable own or inherited data property, an +// accessor with no setter, and a new property on a non-extensible object. The +// reference's strictness is what turns that `false` into a TypeError. +// +// Three spellings of the same store disagreed on main: +// +// o.x = 9 -> `Expr::PutValueSet` (carries `strict`) CORRECT +// o.x++ -> `Expr::PropertyUpdate` (carries `strict`) CORRECT +// o.x += 1 -> `Expr::PropertySet` (carries NOTHING) THREW +// for (o.x of) -> `Expr::PropertySet` THREW +// [o.x] = arr -> `Expr::PropertySet` (expression position) THREW +// +// This is the object-path mirror of #9394 (arrays, fixed by #9426) and the +// opposite direction from #9422 (which was an UNDER-throw in strict code). +// +// This file is `.cts`, so it is a CommonJS script in BOTH runtimes: `sloppyArm` +// is sloppy code and `strictArm` opts in with its own directive prologue. +// BOTH ARMS ARE ASSERTED. Asserting only the sloppy no-op is what let #9422 +// through, and asserting only the strict throw is what let this through. +// +// The two arms are textual duplicates on purpose: a function inherits the +// strictness of the code it is DEFINED in, never its caller's, so a shared +// helper would test one mode twice. Only the mode prefix and the directive +// differ. +// +// Companions: test_gap_9422_strict_object_store_strictness.cts (the `=` lane), +// test_gap_9394_array_element_store_strictness.cts (the array element lane), +// test_gap_9423_module_init_strictness.ts (the ESM always-strict half). + +function report(name: string, threw: boolean, ...rest: unknown[]): void { + console.log(name, threw ? "TypeError" : "silent", ...rest); +} + +function hasOwn(value: any, key: PropertyKey): boolean { + return Object.prototype.hasOwnProperty.call(value, key); +} + +function nonWritableProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + enumerable: true, + value: 10, + writable: false, + }); + return proto; +} + +function getterOnlyProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 20; + }, + }); + return proto; +} + +function setterProto(calls: any[]): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 30; + }, + set(value: any) { + calls.push(value); + }, + }); + return proto; +} + +function readOnlyOwn(initial: any): any { + const o: any = {}; + Object.defineProperty(o, "x", { + configurable: true, + enumerable: true, + value: initial, + writable: false, + }); + return o; +} + +function getterOnlyOwn(): any { + const o: any = {}; + Object.defineProperty(o, "x", { + configurable: true, + enumerable: true, + get() { + return 40; + }, + }); + return o; +} + +class Cell { + x: number; + constructor(x: number) { + this.x = x; + } +} + +function frozenCell(): any { + const c = new Cell(1); + Object.freeze(c); + return c; +} + +function sloppyArm(): void { + let threw = false; + + // ---- compound assignment, every operator, on a frozen own data property ---- + const plus: any = { x: 1 }; + Object.freeze(plus); + threw = false; + try { + plus.x += 1; + } catch { + threw = true; + } + report("sloppy frozen +=:", threw, plus.x); + + const minus: any = { x: 1 }; + Object.freeze(minus); + threw = false; + try { + minus.x -= 1; + } catch { + threw = true; + } + report("sloppy frozen -=:", threw, minus.x); + + const times: any = { x: 3 }; + Object.freeze(times); + threw = false; + try { + times.x *= 2; + } catch { + threw = true; + } + report("sloppy frozen *=:", threw, times.x); + + // Logical assignment writes only on the branch that reaches the store, so + // each operator gets a starting value that makes it write. + const andAnd: any = { x: 1 }; + Object.freeze(andAnd); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("sloppy frozen &&=:", threw, andAnd.x); + + const orOr: any = { x: 0 }; + Object.freeze(orOr); + threw = false; + try { + orOr.x ||= 5; + } catch { + threw = true; + } + report("sloppy frozen ||=:", threw, orOr.x); + + const nullish: any = { x: undefined }; + Object.freeze(nullish); + threw = false; + try { + nullish.x ??= 5; + } catch { + threw = true; + } + report("sloppy frozen ??=:", threw, nullish.x); + + // The short-circuit CONTROL: `&&=` on a falsy value never stores, so it is + // silent in both modes even on a frozen receiver. A fix that made every + // logical assignment throw would show up here. + const andShort: any = { x: 0 }; + Object.freeze(andShort); + threw = false; + try { + andShort.x &&= 5; + } catch { + threw = true; + } + report("sloppy frozen &&= short-circuit:", threw, andShort.x); + + // ---- for-of head, named and computed ---- + const forNamed: any = { x: 1 }; + Object.freeze(forNamed); + threw = false; + try { + for (forNamed.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy frozen for-of head:", threw, forNamed.x); + + const forComputed: any = { x: 1 }; + Object.freeze(forComputed); + const forKey = "x"; + threw = false; + try { + for (forComputed[forKey] of [7]) { + } + } catch { + threw = true; + } + report("sloppy frozen for-of head computed:", threw, forComputed.x); + + // The computed-key twin of `+=` above. `o[k] += 1` and `for (o[k] of ...)` + // both lower to `Expr::IndexSet`, whose OBJECT-by-name lanes kept throwing + // after #9426 carried the flag to its array element lanes. + const computedPlus: any = { x: 1 }; + Object.freeze(computedPlus); + const computedKey = "x"; + threw = false; + try { + computedPlus[computedKey] += 1; + } catch { + threw = true; + } + report("sloppy frozen o[k] +=:", threw, computedPlus.x); + + // A LITERAL computed key takes a different `Expr::IndexSet` arm from a + // runtime string key, so both are asserted. + const literalPlus: any = { x: 1 }; + Object.freeze(literalPlus); + threw = false; + try { + literalPlus["x"] += 1; + } catch { + threw = true; + } + report("sloppy frozen o[\"x\"] +=:", threw, literalPlus.x); + + // And an UNTYPED key, which reaches the runtime STRING_TAG dispatch rather + // than either static arm. + const anyKeyed: any = { x: 1 }; + Object.freeze(anyKeyed); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("sloppy frozen o[anyKey] +=:", threw, anyKeyed.x); + + // ---- destructuring assignment targets ---- + // Statement position and expression position are DIFFERENT lowerings + // (`destructuring/assignment_stmt.rs` vs `destructuring/assignment_expr.rs`), + // so both are asserted. + const arrDestr: any = { x: 1 }; + Object.freeze(arrDestr); + threw = false; + try { + [arrDestr.x] = [7]; + } catch { + threw = true; + } + report("sloppy frozen [o.x] = arr:", threw, arrDestr.x); + + const arrDestrExpr: any = { x: 1 }; + Object.freeze(arrDestrExpr); + threw = false; + try { + const seen = ([arrDestrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("sloppy frozen ([o.x] = arr) expr:", threw, arrDestrExpr.x); + + const objDestr: any = { x: 1 }; + Object.freeze(objDestr); + threw = false; + try { + ({ a: objDestr.x } = { a: 7 }); + } catch { + threw = true; + } + report("sloppy frozen ({a: o.x}) = obj:", threw, objDestr.x); + + const arrDestrComputed: any = { x: 1 }; + Object.freeze(arrDestrComputed); + const destrKey = "x"; + threw = false; + try { + [arrDestrComputed[destrKey]] = [7]; + } catch { + threw = true; + } + report("sloppy frozen [o[k]] = arr:", threw, arrDestrComputed.x); + + // ---- the same `+=` against every rejecting receiver shape ---- + const sealedOwn: any = { x: 1 }; + Object.seal(sealedOwn); + threw = false; + try { + sealedOwn.x += 1; + } catch { + threw = true; + } + report("sloppy sealed own +=:", threw, sealedOwn.x); + + // `seal` leaves existing properties WRITABLE, so the line above must succeed + // in both modes -- it is the over-throw control. A NEW key on a sealed object + // is the rejecting half. + const sealedNew: any = { x: 1 }; + Object.seal(sealedNew); + threw = false; + try { + sealedNew.y += 1; + } catch { + threw = true; + } + report("sloppy sealed new +=:", threw, hasOwn(sealedNew, "y")); + + const nonWritable = readOnlyOwn(1); + threw = false; + try { + nonWritable.x += 1; + } catch { + threw = true; + } + report("sloppy non-writable own +=:", threw, nonWritable.x); + + // ---- INHERITED rejecting receivers ---- + // + // `+=` against a receiver whose rejection lives on the PROTOTYPE is asserted + // in the sloppy arm only, and the strict twin is spelled `=` instead. That + // asymmetry is deliberate and is NOT what this issue is about: + // + // Perry's `Expr::PropertySet` tail (`js_object_set_field_by_name`) performs + // an OWN-property store. It never runs `OrdinarySetWithOwnDescriptor`'s + // prototype walk, so an inherited non-writable data property, an inherited + // getter-only accessor, and an inherited SETTER are all mishandled the same + // way: the setter never fires and an own property is created instead. That + // is wrong in BOTH modes, it is wrong on unfixed `main` in both modes, and + // it is a missing prototype walk rather than a missing `Throw` flag. + // + // #9459 routes the SLOPPY tail to `js_put_value_set(..., 0)` -- the + // receiver-aware `[[Set]]` that sloppy `o.x = v` has always used -- so the + // sloppy arm below becomes correct as a side effect of getting the + // strictness right. The strict tail keeps its typed-feedback store site + // (`js_typed_feedback_object_set_field_by_name_fast`, a #7480/#5093 gate + // with its own IR tests), so fixing the strict half means retargeting that + // lane, which is a separate change. Filed as #9495; the strict `=` + // twins below pin the shapes so a future fix has a baseline here. + // + // Asserting the sloppy arm alone would be the #9394 mistake, which is why the + // strict side is still exercised -- on the lane that is already correct. + const inheritedNonWritable: any = Object.create(nonWritableProto()); + threw = false; + try { + inheritedNonWritable.x += 1; + } catch { + threw = true; + } + report( + "sloppy non-writable inherited +=:", + threw, + hasOwn(inheritedNonWritable, "x"), + inheritedNonWritable.x, + ); + + const getterOnly = getterOnlyOwn(); + threw = false; + try { + getterOnly.x += 1; + } catch { + threw = true; + } + report("sloppy getter-only own +=:", threw, getterOnly.x); + + const inheritedGetterOnly: any = Object.create(getterOnlyProto()); + threw = false; + try { + inheritedGetterOnly.x += 1; + } catch { + threw = true; + } + report( + "sloppy getter-only inherited +=:", + threw, + hasOwn(inheritedGetterOnly, "x"), + inheritedGetterOnly.x, + ); + + // An inherited SETTER runs in both modes and creates no own property: the + // rejection is about `[[Set]]` returning false, never about reaching the + // accessor. A fix that routed sloppy stores past the prototype walk would + // show up here as a missing call. + const calls: any[] = []; + const withSetter: any = Object.create(setterProto(calls)); + threw = false; + try { + withSetter.x += 1; + } catch { + threw = true; + } + report("sloppy inherited setter +=:", threw, calls.join(","), hasOwn(withSetter, "x")); + + const noExtend: any = { x: 1 }; + Object.preventExtensions(noExtend); + threw = false; + try { + noExtend.y += 1; + } catch { + threw = true; + } + report("sloppy preventExtensions new +=:", threw, hasOwn(noExtend, "y")); + + // preventExtensions leaves existing properties writable -- the second + // over-throw control. + const noExtendOwn: any = { x: 1 }; + Object.preventExtensions(noExtendOwn); + threw = false; + try { + noExtendOwn.x += 1; + } catch { + threw = true; + } + report("sloppy preventExtensions own +=:", threw, noExtendOwn.x); + + // ---- the class-field store lane ---- + const cell = frozenCell(); + threw = false; + try { + cell.x += 1; + } catch { + threw = true; + } + report("sloppy frozen class field +=:", threw, cell.x); + + const liveCell = new Cell(1); + threw = false; + try { + liveCell.x += 1; + } catch { + threw = true; + } + report("sloppy live class field +=:", threw, liveCell.x); + + // ---- the `arr.length` lane ---- + // `a.length += n` is `Set(O, "length", n, Throw)` on the OBJECT lane, which + // reaches `js_array_set_length_strict` -- named for the `Throw` it hard-codes. + const frozenArray: any[] = [1, 2]; + Object.freeze(frozenArray); + threw = false; + try { + frozenArray.length += 1; + } catch { + threw = true; + } + report("sloppy frozen array length +=:", threw, frozenArray.length); + + const nonWritableLength: any[] = [1, 2]; + Object.defineProperty(nonWritableLength, "length", { writable: false }); + threw = false; + try { + nonWritableLength.length += 1; + } catch { + threw = true; + } + report("sloppy non-writable array length +=:", threw, nonWritableLength.length); + + // A live array's `length` accepts the write in both modes: the control that + // proves the lane still WORKS, not just that it stopped throwing. + const liveArray: any[] = [1, 2]; + threw = false; + try { + liveArray.length += 1; + } catch { + threw = true; + } + report("sloppy live array length +=:", threw, liveArray.length); + + // ---- accepted stores, to prove the sloppy tail still STORES ---- + const plain: any = { x: 1 }; + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("sloppy plain +=:", threw, plain.x); + + const plainFor: any = { x: 1 }; + threw = false; + try { + for (plainFor.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy plain for-of head:", threw, plainFor.x); + + const plainDestr: any = { x: 1 }; + threw = false; + try { + [plainDestr.x] = [7]; + } catch { + threw = true; + } + report("sloppy plain [o.x] = arr:", threw, plainDestr.x); + + const plainNew: any = { x: 1 }; + threw = false; + try { + plainNew.y ??= 9; + } catch { + threw = true; + } + report("sloppy plain new key ??=:", threw, plainNew.y); + + // ---- `++` (Expr::PropertyUpdate) alongside `+=`, so the two spellings of + // one operation are asserted in the same file and mode ---- + const upd: any = { x: 1 }; + Object.freeze(upd); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("sloppy frozen ++:", threw, upd.x); + + // ---- and plain `=`, the lane that was already right (#9422) ---- + const assign: any = { x: 1 }; + Object.freeze(assign); + threw = false; + try { + assign.x = 9; + } catch { + threw = true; + } + report("sloppy frozen =:", threw, assign.x); +} + +function strictArm(): void { + "use strict"; + + let threw = false; + + // ---- compound assignment, every operator, on a frozen own data property ---- + const plus: any = { x: 1 }; + Object.freeze(plus); + threw = false; + try { + plus.x += 1; + } catch { + threw = true; + } + report("strict frozen +=:", threw, plus.x); + + const minus: any = { x: 1 }; + Object.freeze(minus); + threw = false; + try { + minus.x -= 1; + } catch { + threw = true; + } + report("strict frozen -=:", threw, minus.x); + + const times: any = { x: 3 }; + Object.freeze(times); + threw = false; + try { + times.x *= 2; + } catch { + threw = true; + } + report("strict frozen *=:", threw, times.x); + + // Logical assignment writes only on the branch that reaches the store, so + // each operator gets a starting value that makes it write. + const andAnd: any = { x: 1 }; + Object.freeze(andAnd); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("strict frozen &&=:", threw, andAnd.x); + + const orOr: any = { x: 0 }; + Object.freeze(orOr); + threw = false; + try { + orOr.x ||= 5; + } catch { + threw = true; + } + report("strict frozen ||=:", threw, orOr.x); + + const nullish: any = { x: undefined }; + Object.freeze(nullish); + threw = false; + try { + nullish.x ??= 5; + } catch { + threw = true; + } + report("strict frozen ??=:", threw, nullish.x); + + // The short-circuit CONTROL: `&&=` on a falsy value never stores, so it is + // silent in both modes even on a frozen receiver. A fix that made every + // logical assignment throw would show up here. + const andShort: any = { x: 0 }; + Object.freeze(andShort); + threw = false; + try { + andShort.x &&= 5; + } catch { + threw = true; + } + report("strict frozen &&= short-circuit:", threw, andShort.x); + + // ---- for-of head, named and computed ---- + const forNamed: any = { x: 1 }; + Object.freeze(forNamed); + threw = false; + try { + for (forNamed.x of [7]) { + } + } catch { + threw = true; + } + report("strict frozen for-of head:", threw, forNamed.x); + + const forComputed: any = { x: 1 }; + Object.freeze(forComputed); + const forKey = "x"; + threw = false; + try { + for (forComputed[forKey] of [7]) { + } + } catch { + threw = true; + } + report("strict frozen for-of head computed:", threw, forComputed.x); + + // The computed-key twin of `+=` above. `o[k] += 1` and `for (o[k] of ...)` + // both lower to `Expr::IndexSet`, whose OBJECT-by-name lanes kept throwing + // after #9426 carried the flag to its array element lanes. + const computedPlus: any = { x: 1 }; + Object.freeze(computedPlus); + const computedKey = "x"; + threw = false; + try { + computedPlus[computedKey] += 1; + } catch { + threw = true; + } + report("strict frozen o[k] +=:", threw, computedPlus.x); + + // A LITERAL computed key takes a different `Expr::IndexSet` arm from a + // runtime string key, so both are asserted. + const literalPlus: any = { x: 1 }; + Object.freeze(literalPlus); + threw = false; + try { + literalPlus["x"] += 1; + } catch { + threw = true; + } + report("strict frozen o[\"x\"] +=:", threw, literalPlus.x); + + // And an UNTYPED key, which reaches the runtime STRING_TAG dispatch rather + // than either static arm. + const anyKeyed: any = { x: 1 }; + Object.freeze(anyKeyed); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("strict frozen o[anyKey] +=:", threw, anyKeyed.x); + + // ---- destructuring assignment targets ---- + // Statement position and expression position are DIFFERENT lowerings + // (`destructuring/assignment_stmt.rs` vs `destructuring/assignment_expr.rs`), + // so both are asserted. + const arrDestr: any = { x: 1 }; + Object.freeze(arrDestr); + threw = false; + try { + [arrDestr.x] = [7]; + } catch { + threw = true; + } + report("strict frozen [o.x] = arr:", threw, arrDestr.x); + + const arrDestrExpr: any = { x: 1 }; + Object.freeze(arrDestrExpr); + threw = false; + try { + const seen = ([arrDestrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("strict frozen ([o.x] = arr) expr:", threw, arrDestrExpr.x); + + const objDestr: any = { x: 1 }; + Object.freeze(objDestr); + threw = false; + try { + ({ a: objDestr.x } = { a: 7 }); + } catch { + threw = true; + } + report("strict frozen ({a: o.x}) = obj:", threw, objDestr.x); + + const arrDestrComputed: any = { x: 1 }; + Object.freeze(arrDestrComputed); + const destrKey = "x"; + threw = false; + try { + [arrDestrComputed[destrKey]] = [7]; + } catch { + threw = true; + } + report("strict frozen [o[k]] = arr:", threw, arrDestrComputed.x); + + // ---- the same `+=` against every rejecting receiver shape ---- + const sealedOwn: any = { x: 1 }; + Object.seal(sealedOwn); + threw = false; + try { + sealedOwn.x += 1; + } catch { + threw = true; + } + report("strict sealed own +=:", threw, sealedOwn.x); + + // `seal` leaves existing properties WRITABLE, so the line above must succeed + // in both modes -- it is the over-throw control. A NEW key on a sealed object + // is the rejecting half. + const sealedNew: any = { x: 1 }; + Object.seal(sealedNew); + threw = false; + try { + sealedNew.y += 1; + } catch { + threw = true; + } + report("strict sealed new +=:", threw, hasOwn(sealedNew, "y")); + + const nonWritable = readOnlyOwn(1); + threw = false; + try { + nonWritable.x += 1; + } catch { + threw = true; + } + report("strict non-writable own +=:", threw, nonWritable.x); + + // See the note in `sloppyArm`: the `+=` spelling on an inherited receiver is + // a separate, mode-independent defect. `=` is the same three receiver shapes + // on the lane that already walks the prototype chain. + const inheritedNonWritable: any = Object.create(nonWritableProto()); + threw = false; + try { + inheritedNonWritable.x = 11; + } catch { + threw = true; + } + report( + "strict non-writable inherited =:", + threw, + hasOwn(inheritedNonWritable, "x"), + inheritedNonWritable.x, + ); + + const getterOnly = getterOnlyOwn(); + threw = false; + try { + getterOnly.x += 1; + } catch { + threw = true; + } + report("strict getter-only own +=:", threw, getterOnly.x); + + const inheritedGetterOnly: any = Object.create(getterOnlyProto()); + threw = false; + try { + inheritedGetterOnly.x = 21; + } catch { + threw = true; + } + report( + "strict getter-only inherited =:", + threw, + hasOwn(inheritedGetterOnly, "x"), + inheritedGetterOnly.x, + ); + + // An inherited SETTER runs in both modes and creates no own property: the + // rejection is about `[[Set]]` returning false, never about reaching the + // accessor. A fix that routed sloppy stores past the prototype walk would + // show up here as a missing call. + const calls: any[] = []; + const withSetter: any = Object.create(setterProto(calls)); + threw = false; + try { + withSetter.x = 31; + } catch { + threw = true; + } + report("strict inherited setter =:", threw, calls.join(","), hasOwn(withSetter, "x")); + + const noExtend: any = { x: 1 }; + Object.preventExtensions(noExtend); + threw = false; + try { + noExtend.y += 1; + } catch { + threw = true; + } + report("strict preventExtensions new +=:", threw, hasOwn(noExtend, "y")); + + // preventExtensions leaves existing properties writable -- the second + // over-throw control. + const noExtendOwn: any = { x: 1 }; + Object.preventExtensions(noExtendOwn); + threw = false; + try { + noExtendOwn.x += 1; + } catch { + threw = true; + } + report("strict preventExtensions own +=:", threw, noExtendOwn.x); + + // ---- the class-field store lane ---- + const cell = frozenCell(); + threw = false; + try { + cell.x += 1; + } catch { + threw = true; + } + report("strict frozen class field +=:", threw, cell.x); + + const liveCell = new Cell(1); + threw = false; + try { + liveCell.x += 1; + } catch { + threw = true; + } + report("strict live class field +=:", threw, liveCell.x); + + // ---- the `arr.length` lane ---- + // `a.length += n` is `Set(O, "length", n, Throw)` on the OBJECT lane, which + // reaches `js_array_set_length_strict` -- named for the `Throw` it hard-codes. + const frozenArray: any[] = [1, 2]; + Object.freeze(frozenArray); + threw = false; + try { + frozenArray.length += 1; + } catch { + threw = true; + } + report("strict frozen array length +=:", threw, frozenArray.length); + + const nonWritableLength: any[] = [1, 2]; + Object.defineProperty(nonWritableLength, "length", { writable: false }); + threw = false; + try { + nonWritableLength.length += 1; + } catch { + threw = true; + } + report("strict non-writable array length +=:", threw, nonWritableLength.length); + + // A live array's `length` accepts the write in both modes: the control that + // proves the lane still WORKS, not just that it stopped throwing. + const liveArray: any[] = [1, 2]; + threw = false; + try { + liveArray.length += 1; + } catch { + threw = true; + } + report("strict live array length +=:", threw, liveArray.length); + + // ---- accepted stores, to prove the sloppy tail still STORES ---- + const plain: any = { x: 1 }; + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("strict plain +=:", threw, plain.x); + + const plainFor: any = { x: 1 }; + threw = false; + try { + for (plainFor.x of [7]) { + } + } catch { + threw = true; + } + report("strict plain for-of head:", threw, plainFor.x); + + const plainDestr: any = { x: 1 }; + threw = false; + try { + [plainDestr.x] = [7]; + } catch { + threw = true; + } + report("strict plain [o.x] = arr:", threw, plainDestr.x); + + const plainNew: any = { x: 1 }; + threw = false; + try { + plainNew.y ??= 9; + } catch { + threw = true; + } + report("strict plain new key ??=:", threw, plainNew.y); + + // ---- `++` (Expr::PropertyUpdate) alongside `+=`, so the two spellings of + // one operation are asserted in the same file and mode ---- + const upd: any = { x: 1 }; + Object.freeze(upd); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("strict frozen ++:", threw, upd.x); + + // ---- and plain `=`, the lane that was already right (#9422) ---- + const assign: any = { x: 1 }; + Object.freeze(assign); + threw = false; + try { + assign.x = 9; + } catch { + threw = true; + } + report("strict frozen =:", threw, assign.x); +} + +sloppyArm(); +strictArm(); From 28bf6f6c515f57711eb462069d77524e1917ad2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 10:09:47 +0200 Subject: [PATCH 2/3] fix(codegen): SIGSEGV storing to a scalar-replaced object-literal field that is never read (#9460) "use strict"; const o = { x: 1 }; o.x = 7; // SIGSEGV -- nothing reads o.x const p = { x: 1 }; for (p.x of [7]) {} // SIGSEGV, sloppy or strict const q = { x: 1 }; q.y++; // TypeError "Cannot assign to read only property 'y'" Three lines of ordinary code, in both modes. The fault is `str d0, [x8]` with x8 = 0x10 -- a raw field store through a NULL receiver at null + sizeof(ObjectHeader). `stmt/let_stmt.rs`'s scalar-replacement arm elides the heap allocation for a non-escaping `new` and gives each field a stack alloca. For the synthetic `__AnonShape_*` class an object literal lowers to, it creates slots only for the fields in `non_escaping_new_used_fields` -- which tracked READS only, on the argument that a store nothing ever reads is unobservable and its slot can be elided. That is true of the STORE and false of the SLOT: the same arm registers `ctx.locals[id]` as an uninitialized DUMMY alloca (the binding has stopped being an object), so a store lowering that looks up the field slot and finds none does not stop -- it falls through to the class-field / Ptr lanes, which load that dummy as an `ObjectHeader*`. The read side has had the matching guard since the synthetic-shape work (`expr/property_get.rs`, whose comment names this exact hazard: "the generic runtime helper that crashes on the dummy slot"). The write side never got it, and needed it on THREE lanes: `Expr::PropertySet` (`o.x += 1`, `for (o.x of ...)`), `Expr::PutValueSet` (`o.x = v`, via `try_lower_sloppy_class_field_store` and the write IC), and `Expr::PropertyUpdate` (`o.y++`). So the fix is at the source, in the two collectors that decide which fields get slots, rather than in each lane: - collectors/escape_news.rs: `non_escaping_new_used_fields` counts a WRITE as a use, so a written field always has a slot. #9024's rule one step further -- #9024 escapes a write to an UNDECLARED property because it would have no slot; this gives a slot to a DECLARED property that would otherwise have none. It costs nothing at runtime (a store into an alloca nothing loads is removed by LLVM). The walker also had NO arm at all for `Expr::PutValueSet`, which is what `o.x = v` lowers to, so neither the written field nor the value's own nested uses were being recorded. - collectors/escape_check.rs: the `Expr::PropertyUpdate` arm gains #9024's `class_chain_has_field` check that the `PropertySet` and `PutValueSet` arms already had. - expr/property_set.rs: a backstop mirroring `property_get.rs` -- a store to a scalar-replaced local with no field slot lowers the value for its side effects and discards the store, the same shape the `this` arm below it has always had. With the collector fixes this should no longer be reachable; kept because the failure it prevents is a null-pointer store and the read side carries the identical guard. Two corrections to the report, which said the crash "does not reproduce in isolation -- the preceding throws are required": - It reproduces in THREE LINES with no exception at all. The original isolated attempt printed `o.x` afterwards, and that read is what creates the slot and hides the crash. "Several rejections first" was the shape it was found in, not the condition. - It is NOT specific to sloppy mode, so it survives the #9459 fix rather than being masked by it -- confirmed by running the fixture against a build with #9459 applied and #9460 not: still SIGSEGV, at the strict case. Neither the `perry_sjlj_try` transport (#9323) nor a rooting hole (#9417/#9444/#9445) is involved: PERRY_GC_PROTECT_FROMSPACE changes nothing, because the address was never a heap object. Verified byte-identical to `node --experimental-strip-types` on test-files/test_gap_9460_unread_scalar_field_store.cts (SIGSEGV before, clean after), and the #9422/#9423 investigation's original `r_lanes.cts` repro now matches node exactly. --- .../9460-unread-scalar-field-store-segv.md | 80 +++++++++ .../src/collectors/escape_check.rs | 13 ++ .../src/collectors/escape_news.rs | 75 +++++++- crates/perry-codegen/src/expr/property_set.rs | 60 +++++++ ...est_gap_9460_unread_scalar_field_store.cts | 167 ++++++++++++++++++ 5 files changed, 389 insertions(+), 6 deletions(-) create mode 100644 changelog.d/9460-unread-scalar-field-store-segv.md create mode 100644 test-files/test_gap_9460_unread_scalar_field_store.cts diff --git a/changelog.d/9460-unread-scalar-field-store-segv.md b/changelog.d/9460-unread-scalar-field-store-segv.md new file mode 100644 index 0000000000..96c3b099b0 --- /dev/null +++ b/changelog.d/9460-unread-scalar-field-store-segv.md @@ -0,0 +1,80 @@ +### Fixed + +- **SIGSEGV storing to a scalar-replaced object-literal field that is never + read.** + + ```js + "use strict"; + const o = { x: 1 }; + o.x = 7; // SIGSEGV — nothing reads o.x + ``` + + ```js + const o = { x: 1 }; + for (o.x of [7]) {} // SIGSEGV, sloppy or strict + const p = { x: 1 }; + p.y++; // TypeError "Cannot assign to read only property 'y'" + ``` + + Three lines of ordinary code, in both modes. The fault is `str d0, [x8]` with + `x8 = 0x10` — a raw field store through a **null** receiver at + `null + sizeof(ObjectHeader)`. + + `stmt/let_stmt.rs`'s scalar-replacement arm elides the heap allocation for a + non-escaping `new` and gives each field a stack alloca. For the synthetic + `__AnonShape_*` class an object literal lowers to, it creates slots only for + the fields in `non_escaping_new_used_fields` — which tracked **reads** only, + on the argument that a store nothing ever reads is unobservable and its slot + can be elided. That is true of the *store* and false of the *slot*: the same + arm registers `ctx.locals[id]` as an **uninitialized dummy alloca** (the + binding has stopped being an object), so a store lowering that looks up the + field slot and finds none does not stop — it falls through to the class-field + / `Ptr` lanes, which load that dummy as an `ObjectHeader*`. + + The read side has had the matching guard since the synthetic-shape work + (`expr/property_get.rs`, whose comment names this exact hazard: "the generic + runtime helper that crashes on the dummy slot"). The write side never got it — + and it needed it on **three different lanes**: `Expr::PropertySet` (`o.x += 1`, + `for (o.x of …)`), `Expr::PutValueSet` (`o.x = v`, via + `try_lower_sloppy_class_field_store` and the write IC), and + `Expr::PropertyUpdate` (`o.y++`). So the fix is at the source, in the two + collectors that decide which fields get slots, rather than in each lane: + + - `collectors/escape_news.rs` — `non_escaping_new_used_fields` now counts a + WRITE as a use, so a written field always has a slot. This is #9024's rule + one step further: #9024 escapes a write to an *undeclared* property because + it would have no slot; this gives a slot to a *declared* property that would + otherwise have none. It costs nothing at runtime — a store into an alloca + nothing loads is removed by LLVM. The walker also had **no arm at all** for + `Expr::PutValueSet`, which is what `o.x = v` lowers to, so neither the + written field nor the value's own nested uses were being recorded. + - `collectors/escape_check.rs` — the `Expr::PropertyUpdate` arm gains #9024's + `class_chain_has_field` check, which the `PropertySet` and `PutValueSet` + arms already had. `o.y++` on an undeclared property has no slot either. + - `expr/property_set.rs` — a backstop mirroring `property_get.rs`: a store to + a scalar-replaced local with no field slot lowers the value for its side + effects and discards the store (`ScalarObjectFieldSetElided`), the same + shape the `this` arm below it has always had for an inlined constructor + whose target field has no slot. With the collector fixes above this should + no longer be reachable; it is kept because the failure mode it prevents is a + null-pointer store, and because the read side carries the identical guard. + + Two things worth recording about the report, which said the crash "does not + reproduce in isolation — the preceding throws are required": + + - It reproduces in **three lines with no exception at all**. The original + isolated attempt printed `o.x` afterwards, and that read is what creates the + slot and hides the crash. "Several rejections first" was the shape it was + found in, not the condition. + - It is **not** specific to sloppy mode, so it survives the #9459 fix rather + than being masked by it. Neither the `perry_sjlj_try` transport (#9323) nor + a rooting hole (#9417/#9444/#9445) is involved: `PERRY_GC_PROTECT_FROMSPACE` + changes nothing, because the address was never a heap object. + + - `test-files/test_gap_9460_unread_scalar_field_store.cts` — every write + spelling (`=`, `+=`, `o[k] =`, `o.x++`, `o.y++`, `for (o.x of …)`, + `[o.x] = arr`, a brand-new field) against an unread scalar-replaced field in + both modes, each printing a sentinel INSTEAD of reading the field, because + reading it is what hides the bug. Controls: the RHS side effects must still + happen, the read-after-store versions must still read back what was stored, + and an escaping receiver must keep its real heap object. diff --git a/crates/perry-codegen/src/collectors/escape_check.rs b/crates/perry-codegen/src/collectors/escape_check.rs index 862b8458ac..ef20f86861 100644 --- a/crates/perry-codegen/src/collectors/escape_check.rs +++ b/crates/perry-codegen/src/collectors/escape_check.rs @@ -320,6 +320,19 @@ pub fn check_escapes_in_expr( escaped.insert(*id); return; } + // #9460: #9024's rule, which the `PropertySet` and + // `PutValueSet` arms above already apply and this one was + // missing. `obj.x++` is a WRITE as well as a read, and a + // write to a property the class does not declare has no + // scalar slot — so `expr/member_update.rs` finds none and + // falls through to the by-name / `Ptr` lanes on the + // uninitialized dummy `ctx.locals[id]` alloca. + // `const o: any = {x:1}; o.y++;` reached that. + if !crate::collectors::class_accessors::class_chain_has_field( + classes, class_name, property, + ) { + escaped.insert(*id); + } // Safe — field increment on a non-escaping local return; } diff --git a/crates/perry-codegen/src/collectors/escape_news.rs b/crates/perry-codegen/src/collectors/escape_news.rs index e62e0d26dc..ef3fd1d397 100644 --- a/crates/perry-codegen/src/collectors/escape_news.rs +++ b/crates/perry-codegen/src/collectors/escape_news.rs @@ -90,11 +90,33 @@ pub fn collect_non_escaping_news( candidates } -/// For scalar-replaced `new` locals, collect the fields that are actually read -/// through the local after construction. This intentionally tracks only reads -/// (plus read-modify-write updates): writes still need their RHS evaluated for -/// JS side effects, but the scalar slot/store can be elided when the field is -/// never observed. +/// For scalar-replaced `new` locals, collect the fields that are accessed +/// through the local after construction — reads, read-modify-write updates, and +/// (since #9460) WRITES. +/// +/// The set decides which fields get a stack alloca in `stmt/let_stmt.rs`'s +/// scalar-replacement arm, for the synthetic `__AnonShape_*` classes that object +/// literals lower to. +/// +/// It used to track reads only, on the argument that a store to a field nothing +/// ever reads is unobservable and its slot can be elided. That is true of the +/// STORE and false of the SLOT. `let_stmt.rs` registers `ctx.locals[id]` as an +/// uninitialized DUMMY alloca for a scalar-replaced binding — the binding has +/// stopped being an object — so a store lowering that looks up the field slot +/// and finds none does not stop: it falls through to the class-field / +/// `Ptr` lanes, which load that dummy as an `ObjectHeader*` and store +/// through `null +
`. `const o: any = {x:1}; o.x = 7;` with no +/// later read of `o.x` segfaulted (#9460), in both modes, through several +/// different store lanes. +/// +/// Reserving a slot for a written-but-unread field is the fix at the source +/// rather than in each lane: `Expr::PropertySet`, `Expr::PutValueSet` and the +/// write IC all resolve the slot the same way, so one answer here covers all of +/// them. It costs nothing at runtime — a store into an alloca nothing loads is +/// removed by LLVM — and it is the same shape as #9024's rule one step further: +/// #9024 escapes a write to an UNDECLARED property because it would have no +/// slot; this gives a slot to a DECLARED property that would otherwise have +/// none. pub fn collect_non_escaping_new_used_fields( stmts: &[perry_hir::Stmt], non_escaping_news: &HashMap, @@ -228,10 +250,51 @@ fn collect_used_new_fields_in_expr( } collect_used_new_fields_in_expr(object, non_escaping_news, used); } - Expr::PropertySet { object, value, .. } => { + Expr::PropertySet { + object, + property, + value, + } => { + // #9460: a WRITE reserves the field's slot — see this function's + // doc comment. Without it the store lowering finds no slot and + // dereferences the dummy `ctx.locals[id]` alloca. + if let Expr::LocalGet(id) = object.as_ref() { + if non_escaping_news.contains_key(id) { + used.entry(*id).or_default().insert(property.clone()); + collect_used_new_fields_in_expr(value, non_escaping_news, used); + return; + } + } collect_used_new_fields_in_expr(object, non_escaping_news, used); collect_used_new_fields_in_expr(value, non_escaping_news, used); } + // #9460: `o.x = v` lowers to `PutValueSet`, not `PropertySet` — and this + // walker had NO arm for it at all, so neither the written field nor the + // value's own nested uses were recorded. Same rule as `PropertySet` + // above, on the same shape `escape_check.rs`'s `PutValueSet` arm + // recognises: a static string key with `target` and `receiver` naming + // the one candidate local. + Expr::PutValueSet { + target, + key, + value, + receiver, + .. + } => { + if let (Expr::LocalGet(id), Expr::LocalGet(receiver_id), Expr::String(property)) = + (target.as_ref(), receiver.as_ref(), key.as_ref()) + { + if id == receiver_id && non_escaping_news.contains_key(id) { + used.entry(*id).or_default().insert(property.clone()); + collect_used_new_fields_in_expr(value, non_escaping_news, used); + return; + } + } + collect_used_new_fields_in_expr(target, non_escaping_news, used); + collect_used_new_fields_in_expr(key, non_escaping_news, used); + collect_used_new_fields_in_expr(value, non_escaping_news, used); + collect_used_new_fields_in_expr(receiver, non_escaping_news, used); + } Expr::Binary { left, right, .. } | Expr::Compare { left, right, .. } | Expr::Logical { left, right, .. } diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 6102a6c420..8a43d9e808 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -898,6 +898,66 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - return Ok(val_double); } } + // #9460: the local IS scalar-replaced, but this property has no + // field slot -- so there is nothing to store into, and nothing that + // could ever read it back. + // + // `stmt/let_stmt.rs`'s scalar-replacement arm creates slots for a + // synthetic `__AnonShape_*` class (an object literal) only for the + // fields in `non_escaping_new_used_fields`, which by design tracks + // READS: "writes still need their RHS evaluated for JS side effects, + // but the scalar slot/store can be elided when the field is never + // observed" (`collectors/escape_news.rs`). It then registers + // `ctx.locals[id]` as a DUMMY entry-block alloca that is never + // initialized, because the binding has stopped being an object at + // all, and overwrites `local_types[id]` with the synthetic class. + // + // Without this arm a slotless store fell through to the class-field / + // `Ptr` lowerings below, which load that dummy slot as an + // `ObjectHeader*` and store through `null +
` -- + // SIGSEGV on `const o: any = {x:1}; o.x = 7;` with no later read of + // `o.x`, in BOTH modes. The read side has had the matching guard + // since the synthetic-shape work (`expr/property_get.rs`, whose + // comment names the same hazard: "the generic runtime helper that + // crashes on the dummy slot"); the write side never got it, which is + // why adding a `console.log(o.x)` made the crash disappear -- the + // read is what creates the slot. + // + // Discarding the store is what the contract above promises and is + // unobservable: the receiver is a non-escaping fresh literal, so no + // alias exists, and a read of a slotless field already answers + // `undefined` on the read side. The RHS is still lowered, so its side + // effects happen -- same shape as the `this` arm just below, which + // has always evaluated the value and dropped the store when the + // inlined constructor's target field has no slot. + if let Expr::LocalGet(id) = object.as_ref() { + if ctx.scalar_replaced.contains_key(id) { + let val_double = lower_expr(ctx, value)?; + let lowered = LoweredValue { + semantic: SemanticKind::JsValue, + rep: NativeRep::JsValue, + llvm_ty: DOUBLE, + value: val_double.clone(), + }; + ctx.record_lowered_value_with_access_mode( + "ScalarObjectFieldSetElided", + Some(*id), + "scalar_object_field_store.unobserved", + &lowered, + None, + None, + None, + None, + false, + false, + vec![ + format!("field={}", property), + "reason=field_never_read_no_scalar_slot".to_string(), + ], + ); + return Ok(val_double); + } + } // Handle `this` during scalar-replaced constructor inlining: if let Expr::This = object.as_ref() { if let Some(target_id) = ctx.scalar_ctor_target.last().copied() { diff --git a/test-files/test_gap_9460_unread_scalar_field_store.cts b/test-files/test_gap_9460_unread_scalar_field_store.cts new file mode 100644 index 0000000000..2b7809c4ce --- /dev/null +++ b/test-files/test_gap_9460_unread_scalar_field_store.cts @@ -0,0 +1,167 @@ +// #9460: SIGSEGV on `o. = v` when `o` is a scalar-replaced object +// literal and that field is never READ. +// +// `stmt/let_stmt.rs`'s scalar-replacement arm elides the heap allocation for a +// non-escaping `new` and gives each field its own stack alloca -- but for a +// synthetic `__AnonShape_*` class (what an object literal lowers to) it only +// creates slots for the fields in `non_escaping_new_used_fields`, which +// deliberately tracks READS: +// +// "writes still need their RHS evaluated for JS side effects, but the +// scalar slot/store can be elided when the field is never observed" +// -- collectors/escape_news.rs +// +// It then registers `ctx.locals[id]` = a DUMMY entry-block alloca that is never +// initialized, because the binding is no longer an object at all. +// +// `expr/property_get.rs` implements the read half of that contract: a property +// of a scalar-replaced local with no slot reads `undefined`, with a comment +// naming the hazard -- "the generic runtime helper that crashes on the dummy +// slot". `expr/property_set.rs` never implemented the write half. A store to a +// slotless field fell past the scalar arm into the class-field / `Ptr` +// lowering, which loads the dummy slot as an `ObjectHeader*` and stores through +// `null + 16`. +// +// The crash needs the field to be written and NEVER read, which is why the +// original report's isolated attempt did not reproduce: it printed `o.x` +// afterwards, and that read is what creates the slot. +// +// It is NOT specific to sloppy mode, to `for (o.x of ...)`, or to a preceding +// throw -- `"use strict"; const o: any = {x:1}; o.x = 7;` segfaults too. This +// file is `.cts` so both modes live in one program. +// +// Every case here prints a sentinel INSTEAD of reading the field: reading it +// is what hides the bug. + +function sloppyForOfHead(): void { + const o: any = { x: 1 }; + for (o.x of [7]) { + } + console.log("sloppy for-of head: survived"); +} + +function strictForOfHead(): void { + "use strict"; + const o: any = { x: 1 }; + for (o.x of [7]) { + } + console.log("strict for-of head: survived"); +} + +function sloppyPlainAssign(): void { + const o: any = { x: 1 }; + o.x = 7; + console.log("sloppy plain assign: survived"); +} + +function strictPlainAssign(): void { + "use strict"; + const o: any = { x: 1 }; + o.x = 7; + console.log("strict plain assign: survived"); +} + +function strictCompoundAssign(): void { + "use strict"; + const o: any = { x: 1 }; + o.x += 1; + console.log("strict compound assign: survived"); +} + +function strictComputedAssign(): void { + "use strict"; + const o: any = { x: 1 }; + const k = "x"; + o[k] = 7; + console.log("strict computed assign: survived"); +} + +function strictUpdateNewField(): void { + "use strict"; + const o: any = { x: 1 }; + o.y++; + console.log("strict update new field: survived"); +} + +function strictUpdate(): void { + "use strict"; + const o: any = { x: 1 }; + o.x++; + console.log("strict update: survived"); +} + +function strictDestructure(): void { + "use strict"; + const o: any = { x: 1 }; + [o.x] = [7]; + console.log("strict destructure: survived"); +} + +// A store to a field the literal never declared, still never read. +function strictNewField(): void { + "use strict"; + const o: any = { x: 1 }; + o.y = 7; + console.log("strict new field: survived"); +} + +// The RHS must still be evaluated for its side effects even when the store +// itself is elided -- that is the half of the contract `escape_news.rs` +// promises, and eliding the whole statement would silently drop this call. +let sideEffects = 0; +function bump(): number { + sideEffects += 1; + return 7; +} + +function strictStoreEvaluatesRhs(): void { + "use strict"; + const o: any = { x: 1 }; + o.x = bump(); + o.x = bump(); + console.log("strict rhs side effects:", sideEffects); +} + +// CONTROL: the same shape WITH a read. This is the version that already +// worked, kept so a fix that broke it would show up here rather than in a +// benchmark. The field must still hold what was stored. +function strictReadAfterStore(): void { + "use strict"; + const o: any = { x: 1 }; + o.x = 7; + console.log("strict read after store:", o.x); +} + +function strictReadAfterForOfHead(): void { + "use strict"; + const o: any = { x: 1 }; + for (o.x of [7]) { + } + console.log("strict read after for-of head:", o.x); +} + +// CONTROL: an ESCAPING receiver keeps its heap object, so the store is a real +// one. `Object.freeze` is a call, which is what makes it escape. +function strictEscapingReceiver(): void { + "use strict"; + const o: any = { x: 1 }; + const seen: any[] = []; + seen.push(o); + o.x = 7; + console.log("strict escaping receiver:", seen[0].x); +} + +sloppyForOfHead(); +strictForOfHead(); +sloppyPlainAssign(); +strictPlainAssign(); +strictCompoundAssign(); +strictComputedAssign(); +strictUpdate(); +strictUpdateNewField(); +strictDestructure(); +strictNewField(); +strictStoreEvaluatesRhs(); +strictReadAfterStore(); +strictReadAfterForOfHead(); +strictEscapingReceiver(); From 1f1c46c9587dfeec8352c863dfbd48d0ec490f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 14:53:40 +0200 Subject: [PATCH 3/3] fix(codegen): strict `o.x += 1` on an inherited non-writable / accessor property walks the prototype chain (#9495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The strict `Expr::PropertySet` tail (`o.x += 1`, logical assignment, `for`-of heads, expression-position destructuring targets) and the strict object-by-name arms of `Expr::IndexSet` (`o["x"] += 1`, `o[k] += 1`) ended in `js_typed_feedback_object_set_field_by_name{,_fast}` -> `js_object_set_field_by_name`, an OWN-property store. The shape-transition fast path inside it already declines any receiver whose prototype is not the ordinary one, so every inherited non-writable data property, getter-only accessor and setter fell to the slow branch, which appended an own property without consulting the chain: no TypeError, the setter never ran, and an own property materialised where ES2024 §10.1.9.2 creates none. Route both tails through the receiver-aware `[[Set]]` that `o.x = v` and (since #9459) the sloppy spellings already use -- `js_put_value_set(target, key, value, receiver, strict)` -- so the two modes are one tail distinguished by the Throw flag alone. `caller`/`arguments` keep their `js_object_set_field_by_name` route (poisoned-accessor handling). The typed-feedback `PropertySet` site moves with the store: registered in both modes, observed by the pure-recording `js_typed_feedback_observe_property_set` under PERRY_TYPED_FEEDBACK only, per #7480 step 4; a default build emits the bare `js_put_value_set` call. The #7480 "dispatching wrappers still emitted in a default build" gate is re-pointed at the property-get and method-call dispatchers the same fixture emits, as calls. Fixture: test_gap_9495_strict_inherited_property_set.cts (both modes; 18 strict lines diverged on the unfixed branch, byte-identical after), and the strict inherited twins in test_gap_9459_property_set_strictness.cts are spelled `+=`. Found and filed separately: #9526 (declared static field loses `K.n += 1`). --- ...t-inherited-property-set-prototype-walk.md | 96 +++ crates/perry-codegen/src/expr/index_set.rs | 228 ++---- crates/perry-codegen/src/expr/property_set.rs | 164 ++-- .../perry-codegen/src/expr/typed_feedback.rs | 8 + .../tests/native_proof_regressions.rs | 14 +- crates/perry-codegen/tests/typed_feedback.rs | 96 ++- .../tests/typed_shape_descriptors.rs | 13 +- .../test_gap_9459_property_set_strictness.cts | 53 +- ...gap_9495_strict_inherited_property_set.cts | 716 ++++++++++++++++++ 9 files changed, 1111 insertions(+), 277 deletions(-) create mode 100644 changelog.d/9495-strict-inherited-property-set-prototype-walk.md create mode 100644 test-files/test_gap_9495_strict_inherited_property_set.cts diff --git a/changelog.d/9495-strict-inherited-property-set-prototype-walk.md b/changelog.d/9495-strict-inherited-property-set-prototype-walk.md new file mode 100644 index 0000000000..15812036ca --- /dev/null +++ b/changelog.d/9495-strict-inherited-property-set-prototype-walk.md @@ -0,0 +1,96 @@ +### Fixed + +- **Strict `o.x += 1` against an INHERITED non-writable property, getter-only + accessor or setter now runs the prototype walk.** + + ```js + "use strict"; + const proto = {}; + Object.defineProperty(proto, "x", { value: 10, writable: false, configurable: true }); + const a = Object.create(proto); + a.x += 1; // node: TypeError Perry: silent, created own a.x = 11 + + const g = {}; + Object.defineProperty(g, "x", { get() { return 20; }, configurable: true }); + const b = Object.create(g); + b.x += 1; // node: TypeError Perry: silent, created own b.x = 21 + + const calls = []; + const s = {}; + Object.defineProperty(s, "x", { get() { return 30; }, set(v) { calls.push(v); }, configurable: true }); + const d = Object.create(s); + d.x += 1; // node: setter runs with 31, no own property + // Perry: setter NEVER ran, created own d.x = 31 + ``` + + ES2024 §10.1.9.2 (`OrdinarySetWithOwnDescriptor`): when the receiver has no + own property, `[[Set]]` walks to the parent and the *parent's* descriptor + decides — a non-writable data property rejects, a getter-only accessor + rejects, a setter runs with the original receiver, and only a writable data + property (or the end of the chain) creates a new own property. `PutValue` + then throws on a rejection iff the reference is strict. + + `o.x = v` was already right — it lowers to `Expr::PutValueSet` → + `js_put_value_set`, whose `ordinary_set_with_receiver` walks the chain — and + #9459 made the *sloppy* half of these spellings right as a side effect of + routing the sloppy `Expr::PropertySet` tail to that same entry. Only the + **strict** spellings that lower to `Expr::PropertySet` — compound and logical + assignment, `for`-of heads, expression-position destructuring targets — and + the strict object-by-name arms of `Expr::IndexSet` (`o["x"] += 1`, + `o[k] += 1`) were still wrong. This is a missing prototype walk, not a + missing `Throw` flag: the opposite direction from #9422 and a different + defect from #9459. + + Root cause: the strict tails ended in + `js_typed_feedback_object_set_field_by_name_fast` / + `js_typed_feedback_object_set_field_by_name` → `js_object_set_field_by_name`, + an **own-property** store. The shape-transition fast path inside it already + declines any receiver whose prototype is not the ordinary one, so every one of + these receivers fell to the slow branch — which appended an own property + without ever consulting the chain. + + Fix: the strict tails now reach the same receiver-aware `[[Set]]` the sloppy + tails and the `=` lane use, `js_put_value_set(target, key, value, receiver, + strict)`, so the two modes are one tail distinguished by the `Throw` flag + alone. The typed-feedback `PropertySet` site moves with the store: it is + registered in both modes (it describes the store, not its strictness) and + observed by the pure-recording `js_typed_feedback_observe_property_set`, + compile-gated on `PERRY_TYPED_FEEDBACK` exactly as #7480 step 4 gates every + other recording helper — a default build emits the bare `js_put_value_set` + call and nothing else. Nothing was left for the old dispatching wrapper to + decide (the receiver-aware entry makes the fast-path choice itself), so the + #7480 "dispatching wrappers still emitted in a default build" gate is + re-pointed at the method-call dispatcher the same fixture emits, and asserted + as a *call* rather than a symbol (the symbol match was satisfied by the + `declare` line alone). + + - `crates/perry-codegen/src/expr/property_set.rs` — + `lower_put_value_property_set_by_name` replaces the sloppy-only helper + and is the generic tail for both modes; `caller` / `arguments` keep their + `js_object_set_field_by_name` route (poisoned-accessor handling, unrelated + to either flag or walk). `emit_typed_feedback_property_set_observation` + carries the site. + - `crates/perry-codegen/src/expr/index_set.rs` — + `lower_object_index_set_put_value` replaces the sloppy-only helper on the + literal-string-key and string-typed-key object arms. + - `crates/perry-codegen/src/expr/typed_feedback.rs` — + `TypedFeedbackContract::put_value_set`. + - `test-files/test_gap_9495_strict_inherited_property_set.cts` — the three + inherited receivers plus a two-level chain, a class accessor on the chain + and a Proxy on the chain (receiver forwarded), across `+=`, `&&=`, `??=` + (short-circuit control), `for`-of heads, destructuring in statement and + expression position, `o["x"]`, `o[k]`, `o[anyKey]`, `[o[k]] = arr`, with + accepted-store controls (inherited writable data, new key beside an + inherited accessor, class-ref receiver) and the already-correct `=` / + `++` lanes — **both modes**, so "silent because sloppy" and "silent because + the walk never ran" are told apart by the setter-call log and `hasOwn`. + - `test-files/test_gap_9459_property_set_strictness.cts` — the strict + inherited twins are spelled `+=` now, as that file's comment promised. + + Left as it was: `js_class_field_set_fallback` (the class-field arm's + guard-miss path) and `js_object_set_field_by_property_id` (the + computed-runtime-members class route) are still own-property stores; neither + is reachable for the receivers above without a class-typed variable holding + an `Object.create`d value, and each is its own lane. A DECLARED `static` + field losing `K.n += 1` in both modes (found while building the fixture) is + a static-slot lane defect, not a walk, and is filed as #9526. diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 48bdd808f0..9be4d7eb80 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -357,27 +357,37 @@ fn lower_array_index_set_via_runtime_key( /// #9459: the SLOPPY object-by-name tail for `Expr::IndexSet`. /// -/// The two string-key arms below reach `js_typed_feedback_object_set_field_by_name` -/// → `js_object_set_field_by_name`, which has no `strict` parameter and rejects a -/// non-writable slot by throwing. #9426 carried `assignment_strict` to the ARRAY -/// element lanes; the object-by-name lanes on the same node still threw, so -/// `for (o[k] of …)` and `o[k] += 1` on a frozen object stopped a sloppy program -/// node runs to completion. +/// #9459 / #9495: the terminal store for the two string-key object arms below, +/// in BOTH modes. /// -/// `Set(O, ToPropertyKey(k), V, false)` is `js_put_value_set(target, key, value, -/// receiver, 0)` — the same entry sloppy `o[k] = v` already reaches, because +/// Those arms used to reach `js_typed_feedback_object_set_field_by_name` → +/// `js_object_set_field_by_name`: no `strict` parameter, so a rejected sloppy +/// write threw (#9459 -- #9426 had carried `assignment_strict` to the ARRAY +/// element lanes only), and an OWN-property store, so a strict write skipped +/// the prototype walk (#9495 -- an inherited setter never ran, an inherited +/// non-writable / getter-only property never threw, and an own property was +/// created where ES2024 SS10.1.9.2 creates none). +/// +/// `Set(O, ToPropertyKey(k), V, Throw)` is `js_put_value_set(target, key, value, +/// receiver, strict)` — the same entry `o[k] = v` already reaches, because /// `put_value_index_fast_path` keeps statically-known string keys off this file -/// entirely (`expr/proxy_reflect.rs`). Routing here makes the two spellings agree. +/// entirely (`expr/proxy_reflect.rs`). Routing here makes the spellings agree. +/// +/// The typed-feedback `PropertySet` site the arms carried moves with the store +/// (`property_set::emit_typed_feedback_property_set_observation`), keyed by the +/// same `operation` label each arm used. /// -/// Rooting is the #7639/#7201 window the strict arms open: receiver AND key are -/// live across `value`'s lowering, which is arbitrary user code and can drive an -/// evacuating minor. `target` and `receiver` are one evaluation of the base, so -/// the single lowered box fills both operand slots. -fn lower_sloppy_object_index_set( +/// Rooting is the #7639/#7201 window: receiver AND key are live across +/// `value`'s lowering, which is arbitrary user code and can drive an evacuating +/// minor. `target` and `receiver` are one evaluation of the base, so the single +/// lowered box fills both operand slots. +fn lower_object_index_set_put_value( ctx: &mut FnCtx<'_>, object: &Expr, index: &Expr, value: &Expr, + assignment_strict: bool, + operation: &str, ) -> Result { rooting::with_operands_rooted_across( ctx, @@ -387,17 +397,26 @@ fn lower_sloppy_object_index_set( lower_value_for_dynamic_index_set( ctx, value, - "index_set.sloppy_object_value_bits", - "sloppy_object_index_set_helper_edge", + "index_set.object_string_key_value_bits", + "object_string_key_index_set_helper_edge", ) }, |ctx, vals, (val_double, _val_bits)| { let (obj_box, key_box) = (vals[0].clone(), vals[1].clone()); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - // Same guard the strict arms emit: `undefined[k] = 1` is a TypeError - // in BOTH modes -- `GetValue` on the base runs before `PutValue` - // ever consults `Throw`. - super::property_set::emit_nullish_write_guard(ctx, &obj_bits, "index", "iset.sloppy"); + // `undefined[k] = 1` is a TypeError in BOTH modes -- `GetValue` on + // the base runs before `PutValue` ever consults `Throw`. + super::property_set::emit_nullish_write_guard(ctx, &obj_bits, "index", "iset.object"); + super::property_set::emit_typed_feedback_property_set_observation( + ctx, + operation, + &obj_bits, + // #7640 section D: the SSO-safe key unbox can allocate, and + // `obj_bits` is the NaN-boxed receiver the group re-read -- + // no raw handle is derived from it before this. + |ctx| unbox_str_handle(ctx.block(), &key_box), + ); + let strict_flag = if assignment_strict { "1" } else { "0" }; let _ = ctx.block().call( DOUBLE, "js_put_value_set", @@ -406,7 +425,7 @@ fn lower_sloppy_object_index_set( (DOUBLE, &key_box), (DOUBLE, &val_double), (DOUBLE, &obj_box), - (I32, "0"), + (I32, strict_flag), ], ); Ok(val_double) @@ -1453,157 +1472,36 @@ pub(crate) fn lower( }); } if let Expr::String(literal) = index.as_ref() { + // #9459 / #9495: the receiver-aware `[[Set]]` in both modes -- + // see `lower_object_index_set_put_value`. The literal key is + // lowered as an operand (an interned-pool load), which is what + // the old strict arm did by hand. + // // #7154: the value expression can collect, and an evacuating // minor inside it relocates the receiver out from under - // `obj_box`. Root it across the evaluation and re-read below. - // The key is a literal, so it is not an operand here at all — - // it is interned into the string pool below. - // - // #9459: strict only -- see `lower_sloppy_object_index_set`. - if !assignment_strict { - return lower_sloppy_object_index_set(ctx, object, index, value); - } - return rooting::with_operands_rooted_across( + // `obj_box`; the helper roots it across the evaluation. + return lower_object_index_set_put_value( ctx, - &[object.as_ref()], - &[value.as_ref()], - |ctx| { - lower_value_for_dynamic_index_set( - ctx, - value, - "index_set.literal_string_value_bits", - "literal_string_index_set_helper_edge", - ) - }, - |ctx, vals, (val_double, _val_bits)| { - let obj_box = vals[0].clone(); - let key_idx = ctx.strings.intern(literal); - let key_handle_global = - format!("@{}", ctx.strings.entry(key_idx).handle_global); - let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - super::property_set::emit_nullish_write_guard( - ctx, - &obj_bits, - literal, - "iset.literal", - ); - let static_classref = super::index_get::index_object_is_class_or_proto_ref( - ctx, - object.as_ref(), - ); - let (obj_handle, key_raw) = { - let blk = ctx.block(); - let obj_handle = super::index_get::classref_preserving_handle( - blk, - &obj_bits, - static_classref, - ); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); - (obj_handle, key_raw) - }; - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - literal, - TypedFeedbackContract::object_set_by_name(), - ); - ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name", - &[ - (I64, &site_id), - (I64, &obj_handle), - (I64, &key_raw), - (DOUBLE, &val_double), - ], - ); - Ok(val_double) - }, + object, + index, + value, + assignment_strict, + literal, ); } if is_string_expr(ctx, index) { - // #9459: strict only -- see `lower_sloppy_object_index_set`. - if !assignment_strict { - return lower_sloppy_object_index_set(ctx, object, index, value); - } - // #7154: see the literal-key arm above, plus the KEY, which - // sits in the same window. A non-literal string key is an - // ordinary heap string with no registered root of its own, so - // an evacuating minor inside the value's evaluation relocates - // it and leaves the register naming from-space — - // `unbox_str_handle` below would hand the setter a pre-move - // `StringHeader*` and the field would land under a garbage key. - // - // #7639: the receiver's window used to be derived from `value` - // ALONE, which is the half-measure #7201 named. The receiver is - // lowered before the KEY as well, so `o[f()] = 1` — a literal - // RHS that cannot collect, an allocating key that can — left it - // unguarded. As one operand group the receiver's window is the - // disjunction over everything after it, which is what #7201 - // established and what `guard_store_operand`'s two-argument - // form structurally could not say. - return rooting::with_operands_rooted_across( + // #9459 / #9495: as above. #7154 / #7639: the KEY sits in the + // same rooting window as the receiver -- a non-literal string + // key is an ordinary heap string with no registered root of its + // own, and `o[f()] = 1` lowers the receiver before an allocating + // key. The helper roots both as one operand group. + return lower_object_index_set_put_value( ctx, - &[object.as_ref(), index.as_ref()], - &[value.as_ref()], - |ctx| { - lower_value_for_dynamic_index_set( - ctx, - value, - "index_set.string_value_bits", - "string_index_set_helper_edge", - ) - }, - |ctx, vals, (val_double, _val_bits)| { - let (obj_box, key_box) = (vals[0].clone(), vals[1].clone()); - let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - super::property_set::emit_nullish_write_guard( - ctx, - &obj_bits, - "index", - "iset.string", - ); - let static_classref = super::index_get::index_object_is_class_or_proto_ref( - ctx, - object.as_ref(), - ); - let (obj_handle, key_handle) = { - let blk = ctx.block(); - // #7640 section D: the SSO-safe key unbox can - // allocate, so it goes FIRST — `obj_bits` is a - // NaN-boxed double the group re-read, but - // `obj_handle` is a raw `i64` no root can name. - let key_handle = unbox_str_handle(blk, &key_box); - let obj_handle = super::index_get::classref_preserving_handle( - blk, - &obj_bits, - static_classref, - ); - (obj_handle, key_handle) - }; - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - "object[string_index]", - TypedFeedbackContract::object_set_by_name(), - ); - ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name", - &[ - (I64, &site_id), - (I64, &obj_handle), - (I64, &key_handle), - (DOUBLE, &val_double), - ], - ); - // One group, one release, below the store. The inner-to-outer - // ordering obligation the two hand-written guards carried — - // `temp_root_truncate` is a stack CUT, so releasing the - // receiver first silently dropped the key's slot as well — is - // gone rather than merely documented. - Ok(val_double) - }, + object, + index, + value, + assignment_strict, + "object[string_index]", ); } // Fallback with runtime STRING_TAG check, matching IndexGet. diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 8a43d9e808..cff33d7dc2 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -42,8 +42,8 @@ use super::{ class_field_store_layout_note_is_conforming, class_field_store_needs_layout_note, class_field_store_needs_string_addref, emit_jsvalue_slot_store_pointer_tested, emit_typed_feedback_register_site, expr_produces_non_pointer_bits_by_construction, lower_expr, - lower_expr_native, raw_f64_layout_fact, try_lower_pod_field_set, unbox_to_i64, FnCtx, - TypedFeedbackContract, TypedFeedbackKind, + lower_expr_native, raw_f64_layout_fact, try_lower_pod_field_set, + typed_feedback_emission_enabled, unbox_to_i64, FnCtx, TypedFeedbackContract, TypedFeedbackKind, }; /// Metadata-only class candidate for the runtime-guarded plain-field store. @@ -564,7 +564,7 @@ fn lower_runtime_property_set_by_name( assignment_strict: bool, ) -> Result { if !assignment_strict { - return lower_sloppy_property_set_by_name(ctx, object, property, value); + return lower_put_value_property_set_by_name(ctx, object, property, value, false); } // #7154: root the receiver across the value's evaluation, which allocates. // The group re-reads it as part of emitting the store, so no register of @@ -584,28 +584,46 @@ fn lower_runtime_property_set_by_name( }) } -/// #9459: the SLOPPY terminal store for `Expr::PropertySet`. +/// #9459 / #9495: the terminal by-name store for `Expr::PropertySet`, in BOTH +/// modes. /// -/// `Set(O, P, V, false)` -- ordinary `[[Set]]` with the receiver, and a -/// rejection (frozen / sealed / non-writable own or inherited data property / -/// getter-only accessor / non-extensible new key) reported as `false` and -/// discarded rather than thrown. That is exactly `js_put_value_set(target, key, -/// value, receiver, 0)`, the entry sloppy `o.x = v` has always used through -/// `Expr::PutValueSet`; routing here makes `o.x += 1`, `for (o.x of it)` and -/// `[o.x] = arr` agree with it instead of throwing where node is silent. +/// `Set(O, P, V, Throw)` -- ordinary `[[Set]]` WITH the receiver. When the +/// receiver has no own property, ES2024 SS10.1.9.2 (`OrdinarySetWithOwnDescriptor`) +/// walks to the parent and lets the PARENT's descriptor decide: an inherited +/// non-writable data property or getter-only accessor rejects, an inherited +/// setter runs with the original receiver, and only a writable data property +/// (or the end of the chain) creates a new own property on the receiver. The +/// rejection is thrown iff `Throw` (SS6.2.5.7 `PutValue`, +/// `Throw = IsStrictReference`). That is exactly +/// `js_put_value_set(target, key, value, receiver, strict)`, the entry `o.x = v` +/// has always used through `Expr::PutValueSet`; routing every `PropertySet` +/// spelling here -- `o.x += 1`, `for (o.x of it)`, `[o.x] = arr` -- makes them +/// agree with it instead of diverging by lane. +/// +/// #9459 brought the SLOPPY half here: its old tail rejected by throwing. #9495 +/// brings the STRICT half: its old tail, +/// `js_typed_feedback_object_set_field_by_name_fast` -> +/// `js_object_set_field_by_name`, was an OWN-property store with no prototype +/// walk, so a strict `o.x += 1` against an inherited setter never ran the setter, +/// an inherited non-writable / getter-only property never threw, and an own +/// property materialised where the spec creates none. +/// +/// The typed-feedback `PropertySet` site moves with the store +/// (`emit_typed_feedback_property_set_observation`): it describes the store, +/// not the store's strictness, so both modes register it. /// /// `target` and `receiver` are the same expression, evaluated ONCE -- the -/// property reference's base is one evaluation, and `with_operands_rooted` +/// property reference's base is one evaluation, and `with_operands_rooted_across` /// hands the single lowered box to both operand slots. /// -/// Rooting is the #7154 window the strict tail also opens: the receiver is live -/// across `value`'s lowering, which is arbitrary user code and can drive an -/// evacuating minor. -fn lower_sloppy_property_set_by_name( +/// Rooting is the #7154 window: the receiver is live across `value`'s +/// lowering, which is arbitrary user code and can drive an evacuating minor. +fn lower_put_value_property_set_by_name( ctx: &mut FnCtx<'_>, object: &Expr, property: &str, value: &Expr, + assignment_strict: bool, ) -> Result { rooting::with_operands_rooted_across( ctx, @@ -615,8 +633,8 @@ fn lower_sloppy_property_set_by_name( lower_value_for_dynamic_property_set( ctx, value, - "property_set.sloppy_dynamic_value_bits", - "sloppy_property_set_helper_edge", + "property_set.dynamic_value_bits", + "dynamic_property_set_helper_edge", ) }, |ctx, vals, (val_double, _val_bits)| { @@ -624,11 +642,17 @@ fn lower_sloppy_property_set_by_name( let key_idx = ctx.strings.intern(property); let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let obj_bits = ctx.block().bitcast_double_to_i64(&obj_box); - // The nullish receiver check is the same one the strict tail emits: - // `undefined.x = 1` is a TypeError in BOTH modes (GetValue on the - // base runs before PutValue's Throw flag is ever consulted). - emit_nullish_write_guard(ctx, &obj_bits, property, "pset_sloppy"); + // `undefined.x = 1` is a TypeError in BOTH modes: `GetValue` on the + // base runs before `PutValue` ever consults `Throw`. + emit_nullish_write_guard(ctx, &obj_bits, property, "pset"); let key_box = ctx.block().load(DOUBLE, &key_handle_global); + emit_typed_feedback_property_set_observation(ctx, property, &obj_bits, |ctx| { + // The key is an interned heap string, so its `StringHeader*` is + // a mask, never an allocation. + let key_bits = ctx.block().bitcast_double_to_i64(&key_box); + ctx.block().and(I64, &key_bits, POINTER_MASK_I64) + }); + let strict_flag = if assignment_strict { "1" } else { "0" }; let _ = ctx.block().call( DOUBLE, "js_put_value_set", @@ -637,7 +661,7 @@ fn lower_sloppy_property_set_by_name( (DOUBLE, &key_box), (DOUBLE, &val_double), (DOUBLE, &obj_box), - (I32, "0"), + (I32, strict_flag), ], ); Ok(val_double) @@ -645,6 +669,50 @@ fn lower_sloppy_property_set_by_name( ) } +/// #9495: the typed-feedback `PropertySet` observation for a by-name store +/// whose tail is `js_put_value_set`. +/// +/// The old strict tail folded the observation into a DISPATCHING wrapper +/// (`js_typed_feedback_object_set_field_by_name_fast`), emitted in every build +/// because it also chose between the shape-transition fast path and the +/// by-name setter. The receiver-aware `[[Set]]` makes that choice inside +/// `js_put_value_set`, so nothing is left for a wrapper to decide and the +/// observation stands alone -- a pure-recording helper, compile-gated on +/// `PERRY_TYPED_FEEDBACK` / `_TRACE` like every other one since #7480 step 4. A +/// default build emits the bare `js_put_value_set` call and nothing else. +/// +/// The site is always REGISTERED (a no-op call-free counter bump in a default +/// build) so that `ic_site_counter` -- and with it every later inline-cache +/// global name in the function -- is numbered exactly as it was when the +/// dispatching wrapper stood here. +/// +/// `key_raw` derives the bare `StringHeader*` the observer hashes, and is only +/// run in an emitting build. It runs BEFORE the observe call and after nothing +/// else, so a derivation that can allocate (an SSO key materialising onto the +/// heap -- #7640 section D) precedes no raw receiver handle: `obj_bits` is the +/// NaN-boxed receiver the rooting group re-read. +pub(super) fn emit_typed_feedback_property_set_observation( + ctx: &mut FnCtx<'_>, + operation: &str, + obj_bits: &str, + key_raw: impl FnOnce(&mut FnCtx<'_>) -> String, +) { + let site_id = emit_typed_feedback_register_site( + ctx, + TypedFeedbackKind::PropertySet, + operation, + TypedFeedbackContract::put_value_set(), + ); + if !typed_feedback_emission_enabled() { + return; + } + let key_raw = key_raw(ctx); + ctx.block().call_void( + "js_typed_feedback_observe_property_set", + &[(I64, &site_id), (I64, obj_bits), (I64, &key_raw)], + ); +} + fn lower_value_for_dynamic_property_set( ctx: &mut FnCtx<'_>, value: &Expr, @@ -719,6 +787,10 @@ pub(crate) fn emit_nullish_write_guard( /// every arm below whose runtime entry rejects by THROWING is strict-only; the /// sloppy twin of each is the strictness-aware `js_put_value_set(..., 0)` that /// the surrounding `PutValueSet` lowering already uses for `o.x = v`. +/// +/// #9495: the generic by-name tail is that same receiver-aware `[[Set]]` in BOTH +/// modes (`lower_put_value_property_set_by_name`). The strict tail used to be an +/// own-property store that skipped the prototype walk; see the helper. pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) -> Result { match expr { Expr::PropertySet { @@ -1129,7 +1201,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - return Ok(result); } } - return lower_sloppy_property_set_by_name(ctx, object, property, value); + return lower_put_value_property_set_by_name( + ctx, object, property, value, false, + ); } // Fast path: known class instance + plain instance field. // The runtime guard checks the receiver's class/shape and @@ -1852,18 +1926,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - } } } - // #9459: the generic SLOPPY tail. The strict tail below ends in - // `js_typed_feedback_object_set_field_by_name_fast`, whose underlying - // `js_object_set_field_by_name` has no `strict` parameter and rejects - // by throwing; sloppy `PutValue` must discard the rejection instead. + // #9459 / #9495: the generic by-name tail is the receiver-aware + // `[[Set]]` with the assignment's own `Throw` flag, in BOTH modes. // // `caller`/`arguments` are excluded in BOTH modes: those two names are // routed here from `PutValueSet` specifically to reach // `js_object_set_field_by_name`'s poisoned-accessor handling, which is - // not a `Throw`-flag decision, and diverting them would change - // behaviour this issue is not about. - if !assignment_strict && !matches!(property.as_str(), "caller" | "arguments") { - return lower_sloppy_property_set_by_name(ctx, object, property, value); + // neither a `Throw`-flag nor a prototype-walk decision, and diverting + // them would change behaviour neither issue is about. + if !matches!(property.as_str(), "caller" | "arguments") { + return lower_put_value_property_set_by_name( + ctx, + object, + property, + value, + assignment_strict, + ); } // #7154: the value expression can collect, and an evacuating minor // inside it relocates the receiver out from under `obj_box` -- @@ -1906,27 +1984,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - let key_box = ctx.block().load(DOUBLE, &key_handle_global); let key_bits = ctx.block().bitcast_double_to_i64(&key_box); let key_raw = ctx.block().and(I64, &key_bits, POINTER_MASK_I64); - if matches!(property.as_str(), "caller" | "arguments") { - ctx.block().call_void( - "js_object_set_field_by_name", - &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], - ); - return Ok(val_double); - } - let site_id = emit_typed_feedback_register_site( - ctx, - TypedFeedbackKind::PropertySet, - property, - TypedFeedbackContract::object_set_by_name(), - ); ctx.block().call_void( - "js_typed_feedback_object_set_field_by_name_fast", - &[ - (I64, &site_id), - (I64, &obj_bits), - (I64, &key_raw), - (DOUBLE, &val_double), - ], + "js_object_set_field_by_name", + &[(I64, &obj_bits), (I64, &key_raw), (DOUBLE, &val_double)], ); Ok(val_double) }, diff --git a/crates/perry-codegen/src/expr/typed_feedback.rs b/crates/perry-codegen/src/expr/typed_feedback.rs index 01e72cc09e..78aff0e2c6 100644 --- a/crates/perry-codegen/src/expr/typed_feedback.rs +++ b/crates/perry-codegen/src/expr/typed_feedback.rs @@ -42,6 +42,14 @@ impl TypedFeedbackContract { Self::new("object_set_by_name_guard", "js_object_set_field_by_name") } + /// #9495: the by-name store whose tail is the receiver-aware `[[Set]]` + /// (`js_put_value_set`) rather than the own-property setter. Same guard + /// label as `object_set_by_name` -- it is the same site kind, observed the + /// same way; only the entry that performs the store differs. + pub(crate) const fn put_value_set() -> Self { + Self::new("object_set_by_name_guard", "js_put_value_set") + } + pub(crate) const fn class_field_get() -> Self { Self::new("class_field_get_guard", "js_object_get_field_by_name_f64") } diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 2fc630df19..483e0863c9 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -6975,10 +6975,12 @@ fn artifact_records_dynamic_property_set_value_bits_before_helper() { /// #9459: the sloppy twin of the test above. /// -/// Same module, `is_strict: false`, so the store takes the sloppy tail. The -/// representation contract is identical -- the RHS is lowered by -/// `lower_value_for_dynamic_property_set` and stays `js_value_bits` until the -/// helper edge -- only the consumer/boxed-at labels and the runtime entry differ. +/// Same module, `is_strict: false`. The representation contract is identical -- +/// the RHS is lowered by `lower_value_for_dynamic_property_set` and stays +/// `js_value_bits` until the helper edge. #9495: the two modes share one tail +/// (`js_put_value_set` with the assignment's own `Throw` flag), so the labels +/// are the same too; the twin stays so that the invariant is asserted on both +/// values of the flag rather than on one. #[test] fn artifact_records_sloppy_dynamic_property_set_value_bits_before_helper() { let module = module_with_classes_and_params( @@ -7001,12 +7003,12 @@ fn artifact_records_sloppy_dynamic_property_set_value_bits_before_helper() { assert!( records.iter().any(|record| { record["expr_kind"] == "PropertySet" - && record["consumer"] == "property_set.sloppy_dynamic_value_bits" + && record["consumer"] == "property_set.dynamic_value_bits" && record["native_rep_name"] == "js_value_bits" && record["llvm_ty"] == "i64" && record["native_value_state"] == "region_local" && record["access_mode"].is_null() - && record_has_note(record, "boxed_at=sloppy_property_set_helper_edge") + && record_has_note(record, "boxed_at=dynamic_property_set_helper_edge") }), "expected sloppy dynamic property-set RHS to stay as js_value_bits before the helper edge:\n{artifact:#}" ); diff --git a/crates/perry-codegen/tests/typed_feedback.rs b/crates/perry-codegen/tests/typed_feedback.rs index c84fd1cd02..f2a19fb0b9 100644 --- a/crates/perry-codegen/tests/typed_feedback.rs +++ b/crates/perry-codegen/tests/typed_feedback.rs @@ -362,10 +362,11 @@ fn typed_feedback_instruments_property_and_method_boundaries() { // later tests in this binary never observe the changed environment. let _lock = env_lock(); let _env = EnvVarGuard::set("PERRY_TYPED_FEEDBACK", Some("1")); - // #9459: STRICT -- the `object_set_by_name_guard` site and the - // `js_typed_feedback_object_set_field_by_name_fast` dispatcher this asserts - // live on the strict store tail. See `strict_module`. The sloppy tail is - // asserted by `sloppy_property_set_uses_strictness_aware_put_value` below. + // #9459: STRICT -- see `strict_module`. #9495: the strict and sloppy tails + // are now ONE tail (`js_put_value_set` with the assignment's own `Throw` + // flag), so the `object_set_by_name_guard` site asserted here is registered + // in both modes; the sloppy twin below asserts the flag, not a different + // lane. let ir = ir_for(strict_module( "typed_feedback_property.ts", vec![param(1, "obj", Type::Any)], @@ -399,8 +400,26 @@ fn typed_feedback_instruments_property_and_method_boundaries() { assert!(ir.contains("object_set_by_name_guard")); assert!(ir.contains("object_get_by_name_guard")); assert!(ir.contains("method_call_guard")); - assert!(ir.contains("js_typed_feedback_object_set_field_by_name_fast")); - assert!(ir.contains("js_object_set_field_by_name")); + // #9495: the strict by-name store is the receiver-aware `[[Set]]` + // (`js_put_value_set(..., 1)`), observed by the pure-recording + // `js_typed_feedback_observe_property_set` in an emitting build -- not the + // own-property `js_typed_feedback_object_set_field_by_name_fast` -> + // `js_object_set_field_by_name` dispatcher, which skipped the prototype + // walk. Match CALLS: every runtime entry is `declare`d in every module. + assert!( + ir.contains("call void @js_typed_feedback_observe_property_set("), + "an emitting build observes the strict by-name store:\n{ir}" + ); + assert_eq!( + put_value_set_strict_flags(&ir), + vec!["1"], + "the strict store reaches `js_put_value_set` with Throw = 1:\n{ir}" + ); + assert!( + !ir.contains("call void @js_typed_feedback_object_set_field_by_name_fast("), + "the own-property store dispatcher must not be CALLED on a strict \ + by-name store -- it skips the prototype walk (#9495):\n{ir}" + ); assert!(ir.contains("js_object_get_field_by_name_f64")); assert!(ir.contains("call double @js_typed_feedback_native_call_method")); assert!(ir.contains("call void @js_typed_feedback_record_guard_pass")); @@ -409,16 +428,35 @@ fn typed_feedback_instruments_property_and_method_boundaries() { assert!(ir.contains("call void @js_typed_feedback_observe_property_get")); } +/// The `i32` `Throw` flag of every `js_put_value_set` CALL in `ir`, in emission +/// order. Matches the call, never the `declare` line, and reads the flag off +/// the call's last operand so a test can pin WHICH mode reached the entry. +fn put_value_set_strict_flags(ir: &str) -> Vec<&str> { + ir.lines() + .filter(|line| line.contains("call double @js_put_value_set(")) + .map(|line| { + let args = line.rsplit_once(')').map(|(head, _)| head).unwrap_or(line); + args.rsplit_once("i32 ") + .map(|(_, flag)| flag.trim()) + .expect("js_put_value_set call ends in its i32 Throw flag") + }) + .collect() +} + /// #9459: the sloppy twin of the boundaries test above. /// /// A sloppy `o.x = v` must not reach `js_object_set_field_by_name` -- it has no /// `strict` parameter and rejects a non-writable slot by throwing, where sloppy /// `PutValue` discards the rejection. It goes to `js_put_value_set(..., 0)`. /// +/// #9495: the strict tail is the same entry with `1`, so the two modes are now +/// distinguished by the flag alone; the typed-feedback `PropertySet` site is +/// registered in both (it describes the store, not its strictness), and the +/// old own-property dispatcher is called in neither. +/// /// The GET boundary is asserted too, and is deliberately unchanged: strictness /// is a property of `PutValue`, not of `GetValue`. That assertion is also what -/// keeps this test from passing on an empty program -- every other check here is -/// a negative. +/// keeps this test from passing on an empty program. #[test] fn sloppy_property_set_uses_strictness_aware_put_value() { let _lock = env_lock(); @@ -445,18 +483,22 @@ fn sloppy_property_set_uses_strictness_aware_put_value() { // every module, so `contains("js_put_value_set")` would pass on a module // that never stores and `contains("...set_field_by_name_fast")` would fail // on one that never does either. - assert!( - ir.contains("call double @js_put_value_set("), - "a sloppy property store should reach the strictness-aware [[Set]]:\n{ir}" + assert_eq!( + put_value_set_strict_flags(&ir), + vec!["0"], + "a sloppy property store should reach the strictness-aware [[Set]] with \ + Throw = 0:\n{ir}" ); assert!( !ir.contains("call void @js_typed_feedback_object_set_field_by_name_fast("), - "the strict typed-feedback store dispatcher must not be CALLED on a \ - sloppy store -- its underlying setter throws on a rejected write:\n{ir}" + "the own-property store dispatcher must not be CALLED on a sloppy \ + store -- its underlying setter throws on a rejected write:\n{ir}" ); assert!( - !ir.contains("object_set_by_name_guard"), - "no typed-feedback SET site is registered on the sloppy tail:\n{ir}" + ir.contains("object_set_by_name_guard") + && ir.contains("call void @js_typed_feedback_observe_property_set("), + "the typed-feedback SET site is registered and observed on the sloppy \ + tail too -- it is one tail since #9495:\n{ir}" ); // ANTI-VACUITY: the reads are untouched by strictness and must still be // instrumented, so the negatives above are about the store lane and not @@ -546,9 +588,29 @@ fn a_default_build_emits_no_typed_feedback_recording_calls() { // And the helpers that DECIDE something, rather than merely counting, are // not gated: this is the line between the two, asserted rather than // described. + // + // #9495: this used to name `js_typed_feedback_object_set_field_by_name_fast` + // on the `obj.x = 1` store. That dispatcher chose between the shape- + // transition fast path and the by-name setter; the store is now the + // receiver-aware `js_put_value_set`, which makes that choice itself, so no + // wrapper stands on the store and its observation is a gated recording + // helper like the rest (asserted absent above). The line is now drawn on + // the two dispatchers this same fixture still emits -- the property GET + // (the set dispatcher's twin) and the method call -- and as CALLS, since + // the old symbol match was satisfied by the `declare` line alone. + assert!( + ir.contains("call double @js_typed_feedback_object_get_field_by_name_f64("), + "dispatching feedback wrappers must still be emitted in a default build \ + (property get):\n{ir}" + ); + assert!( + ir.contains("call double @js_typed_feedback_native_call_method"), + "dispatching feedback wrappers must still be emitted in a default build \ + (method call):\n{ir}" + ); assert!( - ir.contains("js_typed_feedback_object_set_field_by_name_fast"), - "dispatching feedback wrappers must still be emitted in a default build" + ir.contains("call double @js_put_value_set("), + "the by-name store itself must still be lowered in a default build:\n{ir}" ); } diff --git a/crates/perry-codegen/tests/typed_shape_descriptors.rs b/crates/perry-codegen/tests/typed_shape_descriptors.rs index 8f11997fa3..046ec2ddad 100644 --- a/crates/perry-codegen/tests/typed_shape_descriptors.rs +++ b/crates/perry-codegen/tests/typed_shape_descriptors.rs @@ -304,13 +304,18 @@ fn scalar_object_literal_keeps_initializers_read_by_update() { fn assert_typed_feedback_setter_after(ir: &str, start_pos: usize, context: &str) { let after_start = &ir[start_pos..]; + // #9459 / #9495: the dynamic by-name store is the receiver-aware `[[Set]]` + // (`js_put_value_set`) in both modes -- not the own-property + // `js_typed_feedback_object_set_field_by_name` wrapper, which had no + // `strict` parameter and no prototype walk. Match the CALL: every runtime + // entry is `declare`d in every module. assert!( - after_start.contains("call void @js_typed_feedback_object_set_field_by_name"), - "{context} should use the typed-feedback setter wrapper" + after_start.contains("call double @js_put_value_set("), + "{context} should reach the receiver-aware runtime setter" ); assert!( - ir.contains("js_object_set_field_by_name"), - "{context} should keep the safe runtime setter as the typed-feedback fallback" + !after_start.contains("call void @js_typed_feedback_object_set_field_by_name"), + "{context} must not take the own-property setter wrapper (#9495)" ); } diff --git a/test-files/test_gap_9459_property_set_strictness.cts b/test-files/test_gap_9459_property_set_strictness.cts index 3729dfc0fc..f2462efbe3 100644 --- a/test-files/test_gap_9459_property_set_strictness.cts +++ b/test-files/test_gap_9459_property_set_strictness.cts @@ -29,7 +29,9 @@ // helper would test one mode twice. Only the mode prefix and the directive // differ. // -// Companions: test_gap_9422_strict_object_store_strictness.cts (the `=` lane), +// Companions: test_gap_9495_strict_inherited_property_set.cts (the prototype +// walk these spellings skipped in strict code), +// test_gap_9422_strict_object_store_strictness.cts (the `=` lane), // test_gap_9394_array_element_store_strictness.cts (the array element lane), // test_gap_9423_module_init_strictness.ts (the ESM always-strict half). @@ -336,29 +338,10 @@ function sloppyArm(): void { // ---- INHERITED rejecting receivers ---- // - // `+=` against a receiver whose rejection lives on the PROTOTYPE is asserted - // in the sloppy arm only, and the strict twin is spelled `=` instead. That - // asymmetry is deliberate and is NOT what this issue is about: - // - // Perry's `Expr::PropertySet` tail (`js_object_set_field_by_name`) performs - // an OWN-property store. It never runs `OrdinarySetWithOwnDescriptor`'s - // prototype walk, so an inherited non-writable data property, an inherited - // getter-only accessor, and an inherited SETTER are all mishandled the same - // way: the setter never fires and an own property is created instead. That - // is wrong in BOTH modes, it is wrong on unfixed `main` in both modes, and - // it is a missing prototype walk rather than a missing `Throw` flag. - // - // #9459 routes the SLOPPY tail to `js_put_value_set(..., 0)` -- the - // receiver-aware `[[Set]]` that sloppy `o.x = v` has always used -- so the - // sloppy arm below becomes correct as a side effect of getting the - // strictness right. The strict tail keeps its typed-feedback store site - // (`js_typed_feedback_object_set_field_by_name_fast`, a #7480/#5093 gate - // with its own IR tests), so fixing the strict half means retargeting that - // lane, which is a separate change. Filed as #9495; the strict `=` - // twins below pin the shapes so a future fix has a baseline here. - // - // Asserting the sloppy arm alone would be the #9394 mistake, which is why the - // strict side is still exercised -- on the lane that is already correct. + // The rejection lives on the PROTOTYPE, so these exercise the walk of + // ES2024 SS10.1.9.2 as well as the `Throw` flag. Both modes are asserted + // here; the walk itself, across every spelling and lane, is the subject of + // test_gap_9495_strict_inherited_property_set.cts. const inheritedNonWritable: any = Object.create(nonWritableProto()); threw = false; try { @@ -770,18 +753,22 @@ function strictArm(): void { } report("strict non-writable own +=:", threw, nonWritable.x); - // See the note in `sloppyArm`: the `+=` spelling on an inherited receiver is - // a separate, mode-independent defect. `=` is the same three receiver shapes - // on the lane that already walks the prototype chain. + // ---- INHERITED rejecting receivers ---- + // + // The rejection lives on the PROTOTYPE, so these exercise the walk of + // ES2024 SS10.1.9.2 as well as the `Throw` flag. Until #9495 the strict tail + // was an own-property store that skipped the walk, so these three were + // spelled `=` (the lane that already walked) as a baseline; they are the + // `+=` twins of the sloppy arm now. const inheritedNonWritable: any = Object.create(nonWritableProto()); threw = false; try { - inheritedNonWritable.x = 11; + inheritedNonWritable.x += 1; } catch { threw = true; } report( - "strict non-writable inherited =:", + "strict non-writable inherited +=:", threw, hasOwn(inheritedNonWritable, "x"), inheritedNonWritable.x, @@ -799,12 +786,12 @@ function strictArm(): void { const inheritedGetterOnly: any = Object.create(getterOnlyProto()); threw = false; try { - inheritedGetterOnly.x = 21; + inheritedGetterOnly.x += 1; } catch { threw = true; } report( - "strict getter-only inherited =:", + "strict getter-only inherited +=:", threw, hasOwn(inheritedGetterOnly, "x"), inheritedGetterOnly.x, @@ -818,11 +805,11 @@ function strictArm(): void { const withSetter: any = Object.create(setterProto(calls)); threw = false; try { - withSetter.x = 31; + withSetter.x += 1; } catch { threw = true; } - report("strict inherited setter =:", threw, calls.join(","), hasOwn(withSetter, "x")); + report("strict inherited setter +=:", threw, calls.join(","), hasOwn(withSetter, "x")); const noExtend: any = { x: 1 }; Object.preventExtensions(noExtend); diff --git a/test-files/test_gap_9495_strict_inherited_property_set.cts b/test-files/test_gap_9495_strict_inherited_property_set.cts new file mode 100644 index 0000000000..a5e6bdb3dc --- /dev/null +++ b/test-files/test_gap_9495_strict_inherited_property_set.cts @@ -0,0 +1,716 @@ +// #9495: strict `o.x += 1` against an INHERITED non-writable data property, +// getter-only accessor or setter skipped the prototype walk entirely. +// +// ES2024 SS10.1.9.2 (OrdinarySetWithOwnDescriptor): when the receiver has no +// own property, `[[Set]]` walks to the parent and lets THE PARENT'S descriptor +// decide -- a non-writable data property rejects, a getter-only accessor +// rejects, a setter RUNS with the original receiver, and only a writable data +// property (or the end of the chain) creates a new own property on the +// receiver. `PutValue` then turns a rejection into a TypeError iff the +// reference is strict. +// +// Perry's strict `Expr::PropertySet` tail (`js_typed_feedback_object_set_ +// field_by_name_fast` -> `js_object_set_field_by_name`) performed an +// OWN-property store: no walk, so no throw, no setter call, and an own +// property materialised where the spec creates none. The strict object-by-name +// arms of `Expr::IndexSet` (`o["x"] += 1`, `o[k] += 1`) had the same tail. +// `o.x = v` was already right (`Expr::PutValueSet` -> `js_put_value_set`), +// and #9459 made the SLOPPY half of these spellings right as a side effect of +// routing them to that same entry. +// +// This is a missing prototype walk, not a missing `Throw` flag -- it is the +// opposite direction from #9422 and a different defect from #9459 -- so it is +// wrong in BOTH modes on unfixed main, and BOTH ARMS ARE ASSERTED here. The +// sloppy arm distinguishes "silent because sloppy" from "silent because the +// walk never ran": an inherited setter must be CALLED in both modes, and no +// own property may appear in either. +// +// `.cts` so that `sloppyArm` is sloppy and `strictArm` opts in with its own +// directive prologue. The two arms are textual duplicates on purpose: a +// function inherits the strictness of the code it is DEFINED in, never its +// caller's, so a shared helper would test one mode twice. +// +// Companions: test_gap_9459_property_set_strictness.cts (the `Throw` flag on +// these spellings, own-property receivers) and +// test_gap_9422_strict_object_store_strictness.cts (the `=` lane). + +function report(name: string, threw: boolean, ...rest: unknown[]): void { + console.log(name, threw ? "TypeError" : "silent", ...rest); +} + +function hasOwn(value: any, key: PropertyKey): boolean { + return Object.prototype.hasOwnProperty.call(value, key); +} + +function nonWritableProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + enumerable: true, + value: 10, + writable: false, + }); + return proto; +} + +function getterOnlyProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 20; + }, + }); + return proto; +} + +function setterProto(calls: any[]): any { + const proto: any = {}; + Object.defineProperty(proto, "x", { + configurable: true, + get() { + return 30; + }, + set(value: any) { + calls.push(value); + }, + }); + return proto; +} + +class Acc { + calls: any[]; + constructor(calls: any[]) { + this.calls = calls; + } + get x(): number { + return 40; + } + set x(value: number) { + this.calls.push(value); + } +} + +// An INT32-tagged class ref as the receiver (`Klass.prop`), without a declared +// static field: the store lands in the class's dynamic-property bag. (A +// DECLARED `static n` loses `K.n += 1` in both modes on main -- a static-slot +// lane defect, not a prototype walk -- and is #9526.) +class Bag {} + +function sloppyArm(): void { + + let threw = false; + const calls: any[] = []; + + // ---- `o.x += 1` (Expr::PropertySet) against each inherited receiver ---- + const nw: any = Object.create(nonWritableProto()); + threw = false; + try { + nw.x += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited +=:", threw, hasOwn(nw, "x"), nw.x); + + const go: any = Object.create(getterOnlyProto()); + threw = false; + try { + go.x += 1; + } catch { + threw = true; + } + report("sloppy getter-only inherited +=:", threw, hasOwn(go, "x"), go.x); + + calls.length = 0; + const st: any = Object.create(setterProto(calls)); + threw = false; + try { + st.x += 1; + } catch { + threw = true; + } + report("sloppy inherited setter +=:", threw, calls.join(","), hasOwn(st, "x")); + + // Two levels up: the walk must continue past an empty intermediate object. + calls.length = 0; + const deep: any = Object.create(Object.create(setterProto(calls))); + threw = false; + try { + deep.x += 1; + } catch { + threw = true; + } + report("sloppy inherited setter two levels +=:", threw, calls.join(","), hasOwn(deep, "x")); + + // A class accessor on the prototype chain (compiled setter, not a descriptor). + calls.length = 0; + const viaClass: any = Object.create(new Acc(calls)); + threw = false; + try { + viaClass.x += 1; + } catch { + threw = true; + } + report("sloppy class accessor inherited +=:", threw, calls.join(","), hasOwn(viaClass, "x")); + + // A Proxy on the prototype chain: `OrdinarySetWithOwnDescriptor` forwards to + // `parent.[[Set]](P, V, Receiver)` with the ORIGINAL receiver. + const trapLog: any[] = []; + const viaProxy: any = Object.create( + new Proxy( + { x: 50 }, + { + set(t: any, k: any, v: any, r: any) { + trapLog.push(String(k) + "=" + String(v) + ":" + String(r === viaProxy)); + return true; + }, + }, + ), + ); + threw = false; + try { + viaProxy.x += 1; + } catch { + threw = true; + } + report("sloppy proxy inherited +=:", threw, trapLog.join(","), hasOwn(viaProxy, "x")); + + // ---- logical assignment: the branch that stores ---- + calls.length = 0; + const andAnd: any = Object.create(setterProto(calls)); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("sloppy inherited setter &&=:", threw, calls.join(","), hasOwn(andAnd, "x")); + + const goOr: any = Object.create(getterOnlyProto()); + threw = false; + try { + goOr.x ??= 5; + } catch { + threw = true; + } + // getter returns 20, so `??=` never stores: a control that must be silent. + report("sloppy getter-only inherited ??= (no store):", threw, hasOwn(goOr, "x"), goOr.x); + + // ---- for-of head ---- + calls.length = 0; + const forHead: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forHead.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy inherited setter for-of head:", threw, calls.join(","), hasOwn(forHead, "x")); + + const forNw: any = Object.create(nonWritableProto()); + threw = false; + try { + for (forNw.x of [7]) { + } + } catch { + threw = true; + } + report("sloppy non-writable inherited for-of head:", threw, hasOwn(forNw, "x"), forNw.x); + + // ---- destructuring targets, statement and expression position ---- + calls.length = 0; + const destr: any = Object.create(setterProto(calls)); + threw = false; + try { + [destr.x] = [7]; + } catch { + threw = true; + } + report("sloppy inherited setter [o.x] = arr:", threw, calls.join(","), hasOwn(destr, "x")); + + calls.length = 0; + const destrExpr: any = Object.create(setterProto(calls)); + threw = false; + try { + const seen = ([destrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("sloppy inherited setter ([o.x] = arr) expr:", threw, calls.join(","), hasOwn(destrExpr, "x")); + + calls.length = 0; + const destrObj: any = Object.create(setterProto(calls)); + threw = false; + try { + ({ a: destrObj.x } = { a: 7 }); + } catch { + threw = true; + } + report("sloppy inherited setter ({a: o.x}) = obj:", threw, calls.join(","), hasOwn(destrObj, "x")); + + const destrGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + [destrGo.x] = [7]; + } catch { + threw = true; + } + report("sloppy getter-only inherited [o.x] = arr:", threw, hasOwn(destrGo, "x"), destrGo.x); + + // ---- computed keys (Expr::IndexSet object-by-name lanes) ---- + calls.length = 0; + const lit: any = Object.create(setterProto(calls)); + threw = false; + try { + lit["x"] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[\"x\"] +=:", threw, calls.join(","), hasOwn(lit, "x")); + + const litNw: any = Object.create(nonWritableProto()); + threw = false; + try { + litNw["x"] += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited o[\"x\"] +=:", threw, hasOwn(litNw, "x"), litNw.x); + + calls.length = 0; + const keyed: any = Object.create(setterProto(calls)); + const key = "x"; + threw = false; + try { + keyed[key] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[k] +=:", threw, calls.join(","), hasOwn(keyed, "x")); + + const keyedGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + keyedGo[key] += 1; + } catch { + threw = true; + } + report("sloppy getter-only inherited o[k] +=:", threw, hasOwn(keyedGo, "x"), keyedGo.x); + + calls.length = 0; + const anyKeyed: any = Object.create(setterProto(calls)); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("sloppy inherited setter o[anyKey] +=:", threw, calls.join(","), hasOwn(anyKeyed, "x")); + + const anyKeyedNw: any = Object.create(nonWritableProto()); + threw = false; + try { + anyKeyedNw[anyKey] += 1; + } catch { + threw = true; + } + report("sloppy non-writable inherited o[anyKey] +=:", threw, hasOwn(anyKeyedNw, "x"), anyKeyedNw.x); + + calls.length = 0; + const forKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forKeyed[key] of [7]) { + } + } catch { + threw = true; + } + report("sloppy inherited setter for-of head computed:", threw, calls.join(","), hasOwn(forKeyed, "x")); + + calls.length = 0; + const destrKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + [destrKeyed[key]] = [7]; + } catch { + threw = true; + } + report("sloppy inherited setter [o[k]] = arr:", threw, calls.join(","), hasOwn(destrKeyed, "x")); + + // ---- accepted stores: the tail must still STORE, and still create own + // properties where the chain does not object ---- + const plain: any = Object.create({ x: 1 }); + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("sloppy inherited writable data +=:", threw, hasOwn(plain, "x"), plain.x, Object.getPrototypeOf(plain).x); + + const fresh: any = Object.create(setterProto(calls)); + threw = false; + try { + fresh.y ??= 9; + } catch { + threw = true; + } + report("sloppy new key beside inherited accessor ??=:", threw, hasOwn(fresh, "y"), fresh.y); + + // A class ref as the receiver: the receiver-aware `[[Set]]` must keep + // routing an INT32-tagged class value to its dynamic-property bag. + const bag: any = Bag; + threw = false; + try { + bag.sloppy_n = 1; + bag.sloppy_n += 1; + bag["sloppy_n"] += 1; + } catch { + threw = true; + } + report("sloppy class ref +=:", threw, bag.sloppy_n, (Bag as any).sloppy_n); + + // ---- the lanes that were already right, as controls ---- + calls.length = 0; + const assign: any = Object.create(setterProto(calls)); + threw = false; + try { + assign.x = 31; + } catch { + threw = true; + } + report("sloppy inherited setter =:", threw, calls.join(","), hasOwn(assign, "x")); + + calls.length = 0; + const upd: any = Object.create(setterProto(calls)); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("sloppy inherited setter ++:", threw, calls.join(","), hasOwn(upd, "x")); + + const updNw: any = Object.create(nonWritableProto()); + threw = false; + try { + updNw.x++; + } catch { + threw = true; + } + report("sloppy non-writable inherited ++:", threw, hasOwn(updNw, "x"), updNw.x); +} + +function strictArm(): void { + "use strict"; + + let threw = false; + const calls: any[] = []; + + // ---- `o.x += 1` (Expr::PropertySet) against each inherited receiver ---- + const nw: any = Object.create(nonWritableProto()); + threw = false; + try { + nw.x += 1; + } catch { + threw = true; + } + report("strict non-writable inherited +=:", threw, hasOwn(nw, "x"), nw.x); + + const go: any = Object.create(getterOnlyProto()); + threw = false; + try { + go.x += 1; + } catch { + threw = true; + } + report("strict getter-only inherited +=:", threw, hasOwn(go, "x"), go.x); + + calls.length = 0; + const st: any = Object.create(setterProto(calls)); + threw = false; + try { + st.x += 1; + } catch { + threw = true; + } + report("strict inherited setter +=:", threw, calls.join(","), hasOwn(st, "x")); + + // Two levels up: the walk must continue past an empty intermediate object. + calls.length = 0; + const deep: any = Object.create(Object.create(setterProto(calls))); + threw = false; + try { + deep.x += 1; + } catch { + threw = true; + } + report("strict inherited setter two levels +=:", threw, calls.join(","), hasOwn(deep, "x")); + + // A class accessor on the prototype chain (compiled setter, not a descriptor). + calls.length = 0; + const viaClass: any = Object.create(new Acc(calls)); + threw = false; + try { + viaClass.x += 1; + } catch { + threw = true; + } + report("strict class accessor inherited +=:", threw, calls.join(","), hasOwn(viaClass, "x")); + + // A Proxy on the prototype chain: `OrdinarySetWithOwnDescriptor` forwards to + // `parent.[[Set]](P, V, Receiver)` with the ORIGINAL receiver. + const trapLog: any[] = []; + const viaProxy: any = Object.create( + new Proxy( + { x: 50 }, + { + set(t: any, k: any, v: any, r: any) { + trapLog.push(String(k) + "=" + String(v) + ":" + String(r === viaProxy)); + return true; + }, + }, + ), + ); + threw = false; + try { + viaProxy.x += 1; + } catch { + threw = true; + } + report("strict proxy inherited +=:", threw, trapLog.join(","), hasOwn(viaProxy, "x")); + + // ---- logical assignment: the branch that stores ---- + calls.length = 0; + const andAnd: any = Object.create(setterProto(calls)); + threw = false; + try { + andAnd.x &&= 5; + } catch { + threw = true; + } + report("strict inherited setter &&=:", threw, calls.join(","), hasOwn(andAnd, "x")); + + const goOr: any = Object.create(getterOnlyProto()); + threw = false; + try { + goOr.x ??= 5; + } catch { + threw = true; + } + // getter returns 20, so `??=` never stores: a control that must be silent. + report("strict getter-only inherited ??= (no store):", threw, hasOwn(goOr, "x"), goOr.x); + + // ---- for-of head ---- + calls.length = 0; + const forHead: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forHead.x of [7]) { + } + } catch { + threw = true; + } + report("strict inherited setter for-of head:", threw, calls.join(","), hasOwn(forHead, "x")); + + const forNw: any = Object.create(nonWritableProto()); + threw = false; + try { + for (forNw.x of [7]) { + } + } catch { + threw = true; + } + report("strict non-writable inherited for-of head:", threw, hasOwn(forNw, "x"), forNw.x); + + // ---- destructuring targets, statement and expression position ---- + calls.length = 0; + const destr: any = Object.create(setterProto(calls)); + threw = false; + try { + [destr.x] = [7]; + } catch { + threw = true; + } + report("strict inherited setter [o.x] = arr:", threw, calls.join(","), hasOwn(destr, "x")); + + calls.length = 0; + const destrExpr: any = Object.create(setterProto(calls)); + threw = false; + try { + const seen = ([destrExpr.x] = [7]); + void seen; + } catch { + threw = true; + } + report("strict inherited setter ([o.x] = arr) expr:", threw, calls.join(","), hasOwn(destrExpr, "x")); + + calls.length = 0; + const destrObj: any = Object.create(setterProto(calls)); + threw = false; + try { + ({ a: destrObj.x } = { a: 7 }); + } catch { + threw = true; + } + report("strict inherited setter ({a: o.x}) = obj:", threw, calls.join(","), hasOwn(destrObj, "x")); + + const destrGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + [destrGo.x] = [7]; + } catch { + threw = true; + } + report("strict getter-only inherited [o.x] = arr:", threw, hasOwn(destrGo, "x"), destrGo.x); + + // ---- computed keys (Expr::IndexSet object-by-name lanes) ---- + calls.length = 0; + const lit: any = Object.create(setterProto(calls)); + threw = false; + try { + lit["x"] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[\"x\"] +=:", threw, calls.join(","), hasOwn(lit, "x")); + + const litNw: any = Object.create(nonWritableProto()); + threw = false; + try { + litNw["x"] += 1; + } catch { + threw = true; + } + report("strict non-writable inherited o[\"x\"] +=:", threw, hasOwn(litNw, "x"), litNw.x); + + calls.length = 0; + const keyed: any = Object.create(setterProto(calls)); + const key = "x"; + threw = false; + try { + keyed[key] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[k] +=:", threw, calls.join(","), hasOwn(keyed, "x")); + + const keyedGo: any = Object.create(getterOnlyProto()); + threw = false; + try { + keyedGo[key] += 1; + } catch { + threw = true; + } + report("strict getter-only inherited o[k] +=:", threw, hasOwn(keyedGo, "x"), keyedGo.x); + + calls.length = 0; + const anyKeyed: any = Object.create(setterProto(calls)); + const anyKey: any = "x"; + threw = false; + try { + anyKeyed[anyKey] += 1; + } catch { + threw = true; + } + report("strict inherited setter o[anyKey] +=:", threw, calls.join(","), hasOwn(anyKeyed, "x")); + + const anyKeyedNw: any = Object.create(nonWritableProto()); + threw = false; + try { + anyKeyedNw[anyKey] += 1; + } catch { + threw = true; + } + report("strict non-writable inherited o[anyKey] +=:", threw, hasOwn(anyKeyedNw, "x"), anyKeyedNw.x); + + calls.length = 0; + const forKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + for (forKeyed[key] of [7]) { + } + } catch { + threw = true; + } + report("strict inherited setter for-of head computed:", threw, calls.join(","), hasOwn(forKeyed, "x")); + + calls.length = 0; + const destrKeyed: any = Object.create(setterProto(calls)); + threw = false; + try { + [destrKeyed[key]] = [7]; + } catch { + threw = true; + } + report("strict inherited setter [o[k]] = arr:", threw, calls.join(","), hasOwn(destrKeyed, "x")); + + // ---- accepted stores: the tail must still STORE, and still create own + // properties where the chain does not object ---- + const plain: any = Object.create({ x: 1 }); + threw = false; + try { + plain.x += 41; + } catch { + threw = true; + } + report("strict inherited writable data +=:", threw, hasOwn(plain, "x"), plain.x, Object.getPrototypeOf(plain).x); + + const fresh: any = Object.create(setterProto(calls)); + threw = false; + try { + fresh.y ??= 9; + } catch { + threw = true; + } + report("strict new key beside inherited accessor ??=:", threw, hasOwn(fresh, "y"), fresh.y); + + // A class ref as the receiver: the receiver-aware `[[Set]]` must keep + // routing an INT32-tagged class value to its dynamic-property bag. + const bag: any = Bag; + threw = false; + try { + bag.strict_n = 1; + bag.strict_n += 1; + bag["strict_n"] += 1; + } catch { + threw = true; + } + report("strict class ref +=:", threw, bag.strict_n, (Bag as any).strict_n); + + // ---- the lanes that were already right, as controls ---- + calls.length = 0; + const assign: any = Object.create(setterProto(calls)); + threw = false; + try { + assign.x = 31; + } catch { + threw = true; + } + report("strict inherited setter =:", threw, calls.join(","), hasOwn(assign, "x")); + + calls.length = 0; + const upd: any = Object.create(setterProto(calls)); + threw = false; + try { + upd.x++; + } catch { + threw = true; + } + report("strict inherited setter ++:", threw, calls.join(","), hasOwn(upd, "x")); + + const updNw: any = Object.create(nonWritableProto()); + threw = false; + try { + updNw.x++; + } catch { + threw = true; + } + report("strict non-writable inherited ++:", threw, hasOwn(updNw, "x"), updNw.x); +} + +sloppyArm(); +strictArm();