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/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/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/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/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..9be4d7eb80 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -355,6 +355,84 @@ fn lower_array_index_set_via_runtime_key( ) } +/// #9459: the SLOPPY object-by-name tail for `Expr::IndexSet`. +/// +/// #9459 / #9495: the terminal store for the two string-key object arms below, +/// in BOTH modes. +/// +/// 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 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: 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, + &[object, index], + &[value], + |ctx| { + lower_value_for_dynamic_index_set( + ctx, + value, + "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); + // `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", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &obj_box), + (I32, strict_flag), + ], + ); + Ok(val_double) + }, + ) +} + pub(crate) fn lower( ctx: &mut FnCtx<'_>, expr: &Expr, @@ -1394,148 +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. - 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) { - // #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 6f32c5f57b..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. @@ -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_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 // the receiver exists across the window. @@ -576,6 +584,135 @@ fn lower_runtime_property_set_by_name( }) } +/// #9459 / #9495: the terminal by-name store for `Expr::PropertySet`, in BOTH +/// modes. +/// +/// `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_across` +/// hands the single lowered box to both operand slots. +/// +/// 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, + &[object], + &[value], + |ctx| { + lower_value_for_dynamic_property_set( + ctx, + value, + "property_set.dynamic_value_bits", + "dynamic_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); + // `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", + &[ + (DOUBLE, &obj_box), + (DOUBLE, &key_box), + (DOUBLE, &val_double), + (DOUBLE, &obj_box), + (I32, strict_flag), + ], + ); + Ok(val_double) + }, + ) +} + +/// #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, @@ -638,7 +775,23 @@ 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`. +/// +/// #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 { object, @@ -672,7 +825,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 @@ -804,6 +970,66 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { 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() { @@ -906,7 +1132,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 +1152,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 +1174,37 @@ 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_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 // descriptor state before this block touches the raw slot. @@ -1659,6 +1926,23 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } } } + // #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 + // 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` -- // `obj.k = f()` then writes `k` into abandoned from-space memory @@ -1700,27 +1984,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { 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/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/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 7d8e0c5330..483e0863c9 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,47 @@ fn artifact_records_dynamic_property_set_value_bits_before_helper() { ); } +/// #9459: the sloppy twin of the test above. +/// +/// 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( + "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.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=dynamic_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 +14493,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 +14532,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..f2a19fb0b9 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,12 @@ 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 -- 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)], Type::Any, @@ -376,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")); @@ -386,6 +428,91 @@ 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. +#[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_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 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") + && 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 + // 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. /// @@ -461,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 new file mode 100644 index 0000000000..f2462efbe3 --- /dev/null +++ b/test-files/test_gap_9459_property_set_strictness.cts @@ -0,0 +1,952 @@ +// #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_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). + +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 ---- + // + // 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 { + 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); + + // ---- 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 += 1; + } 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 += 1; + } 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 += 1; + } 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(); 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(); 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();