From 85abe63a456e9e23286aea3252e65f0097cf5d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 18:16:22 +0200 Subject: [PATCH 1/2] fix(runtime): a rejected array element write throws only in strict mode (#9394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit const a = [1]; Object.freeze(a); a[0] = 9; // node silent, Perry TypeError const a2 = [1]; Object.freeze(a2); a2[5] = 9; // node silent, Perry TypeError Object.defineProperty(a3, 0, {writable:false}); a3[0]=9; // node silent, Perry TypeError Object.preventExtensions(a4); a4[5] = 9; // node silent, Perry TypeError const o = {x:1}; Object.freeze(o); o.x = 9; // node silent, Perry silent (correct) ES2024 6.2.5.7 (PutValue) calls Set(O, P, V, Throw) with Throw = IsStrictReference, so a failed [[Set]] throws ONLY in strict mode — for an Array exactly as for the ordinary object that was already right. A CommonJS bundle is sloppy code from top to bottom, which is where this surfaced. Introduced by #9326 (the merge of #9297, live again via #9370). That change is right about what it set out to fix — an inherited accessor must run, an inherited non-writable index must reject — but it reached the rejection by routing the cold element-store continuation through the STRICT runtime entry unconditionally. The inline store guard declines exactly the receivers whose write can be rejected (frozen, sealed, non-extensible, descriptor-bearing, prototype-sensitive), so every one of those shapes arrived there and threw. The fix carries the assignment's own Throw flag, which codegen already had and already passes to the ordinary-object [[Set]] and to `js_dyn_index_set_strict`. Finding the target is unchanged in both modes — the #9220 inherited-descriptor walk still runs, so a prototype setter still fires on a sloppy assignment; only the rejection differs. - codegen: `assignment_strict` reaches `js_typed_feedback_array_index_set_fallback_boxed` and `js_typed_feedback_array_set_index_or_string` (one new trailing i32 each). - array/indexing.rs: the strict entry's body is strictness-parameterised (`js_array_set_f64_extend_sloppy` is the sloppy twin); `array_spec_set` takes Throw and returns the receiver unchanged instead of throwing when it is false. Array mutators keep Throw = true: their own algorithms specify it regardless of the calling code. - value/dyn_index.rs: `js_dyn_index_set_strict` already carried the flag and its array arm forced true; it now uses it. The realloc arm in expr/index.rs deliberately keeps the strict entry: it runs only for a receiver the guard already accepted, which cannot reject. test-files/test_gap_9394_array_element_store_strictness.cts is a `.cts`, so it is a CommonJS script in BOTH runtimes, with a sloppy arm and a "use strict" arm. BOTH ARMS ARE ASSERTED. Asserting only the throw is precisely what let this through: #9326 shipped with a 64-check differential and a 205-line gap fixture, all green, none of it sloppy code. Byte-compared against node 26.5.1; a compiler built from unfixed origin/main reports TypeError for six sloppy cases where node is silent, and with this change is identical to node. #9326's own fixture (test_gap_9220_9221_array_proto_paths.ts, an ES module and therefore strict) is unchanged and still byte-identical to node. Unit tests assert both arms too: `element_store_rejection_throws_only_in_strict_mode`, and #9326's `typed_feedback_array_set_guards_reject_frozen_arrays`, which now asserts the silent sloppy call alongside the strict throw. Both were confirmed to FAIL with the sloppy entry rewired to the strict one. Three pieces of test infrastructure had to admit a `.cts` fixture at all, each of which would have made it a DARK TEST: the suite's `find … -name '*.ts'` does not match `foo.cts`, so the harness never selected it (`--filter test_gap_9394` selected 0 tests before, and PASSes after); `basename … .ts` named it `…strictness.c`; and `.gitignore` re-included only `.ts`/`.tsx` under test-files/, so it could not be committed. Not addressed here, found while writing the fixture: Perry emits `js_put_value_set(..., strict = 0)` at EVERY property-set site, so a rejected strict ordinary-object write is silent where Node throws — the mirror-image gap on the object path. --- .gitignore | 8 + .../9394-sloppy-array-element-store.md | 89 ++++++ crates/perry-codegen/src/expr/index.rs | 21 +- crates/perry-codegen/src/expr/index_set.rs | 11 + .../src/runtime_decls/objects.rs | 6 +- crates/perry-runtime/src/array/indexing.rs | 74 ++++- .../perry-runtime/src/array/indexing_keyed.rs | 19 +- crates/perry-runtime/src/array/mod.rs | 3 + crates/perry-runtime/src/array/push_pop.rs | 7 +- .../src/array/strict_store_tests.rs | 78 +++++ crates/perry-runtime/src/typed_feedback.rs | 44 ++- .../perry-runtime/src/typed_feedback/tests.rs | 28 +- .../perry-runtime/src/typed_feedback/trace.rs | 4 +- crates/perry-runtime/src/value/dyn_index.rs | 10 +- run_parity_tests.sh | 16 +- ...ap_9394_array_element_store_strictness.cts | 299 ++++++++++++++++++ 16 files changed, 669 insertions(+), 48 deletions(-) create mode 100644 changelog.d/9394-sloppy-array-element-store.md create mode 100644 test-files/test_gap_9394_array_element_store_strictness.cts diff --git a/.gitignore b/.gitignore index ee0230acfa..4856dbcdc9 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,14 @@ test-files/test-* !test-files/test_*.tsx !test-files/test-*.ts !test-files/test-*.tsx +# A fixture that must be CommonJS in BOTH runtimes is a `.cts` (this repo's +# package is `"type": "module"`, so a `.ts` is strict-mode ESM for Node and for +# Perry alike). Without these it would be an ignored file — a DARK TEST, the +# exact failure mode `scripts/check_test_registration.py` exists to prevent. +!test-files/test_*.cts +!test-files/test_*.mts +!test-files/test-*.cts +!test-files/test-*.mts !test-files/*/ tests/test_* tests/test-* diff --git a/changelog.d/9394-sloppy-array-element-store.md b/changelog.d/9394-sloppy-array-element-store.md new file mode 100644 index 0000000000..2642dc17ac --- /dev/null +++ b/changelog.d/9394-sloppy-array-element-store.md @@ -0,0 +1,89 @@ +### Fixed + +- **A rejected array element write no longer throws in sloppy code.** + + ```js + const a = [1]; Object.freeze(a); a[0] = 9; // node: silent Perry: TypeError + const a2 = [1]; Object.freeze(a2); a2[5] = 9; // node: silent Perry: TypeError + Object.defineProperty(a3, 0, {writable:false}); a3[0]=9; // node: silent Perry: TypeError + Object.preventExtensions(a4); a4[5] = 9; // node: silent Perry: TypeError + const o = {x:1}; Object.freeze(o); o.x = 9; // node: silent Perry: silent (correct) + ``` + + ES2024 §6.2.5.7 (`PutValue`) calls `Set(O, P, V, Throw)` with + `Throw = IsStrictReference`, so a failed `[[Set]]` throws **only in strict + mode** — for an Array exactly as for the ordinary object that was already + right. A CommonJS bundle is sloppy code from top to bottom, which is where + this surfaced. + + Introduced by #9326 (the merge of #9297, live again on `main` via #9370). + That change is right about what it set out to fix — an inherited accessor + must run, an inherited non-writable index must reject — but it reached the + rejection by routing the cold element-store continuation through the STRICT + runtime entry unconditionally. The inline store guard declines exactly the + receivers whose write can be rejected (frozen, sealed, non-extensible, + descriptor-bearing, prototype-sensitive), so every one of those shapes + arrived at that continuation and threw. + + The fix carries the assignment's own `Throw` flag, which codegen already had + and already passes to the ordinary-object `[[Set]]` and to + `js_dyn_index_set_strict`. Finding the target is unchanged in both modes — + the #9220 inherited-descriptor walk still runs, so a prototype setter still + fires on a sloppy assignment; only the rejection differs. + + - `crates/perry-codegen/src/expr/index.rs`, + `crates/perry-codegen/src/expr/index_set.rs`, + `crates/perry-codegen/src/runtime_decls/objects.rs` — pass the site's + `assignment_strict` to `js_typed_feedback_array_index_set_fallback_boxed` + and `js_typed_feedback_array_set_index_or_string` (one new trailing `i32` + each). + - `crates/perry-runtime/src/typed_feedback.rs` — both helpers take that flag + and dispatch on it. + - `crates/perry-runtime/src/array/indexing.rs` — the strict entry's body + becomes strictness-parameterised (`js_array_set_f64_extend_sloppy` is the + sloppy twin); `array_spec_set` takes `Throw` and returns the receiver + unchanged instead of throwing when it is false. Array mutators keep + `Throw = true`: their own algorithms specify it regardless of the calling + code. + - `crates/perry-runtime/src/array/indexing_keyed.rs` — the same for the + numeric/string-key dispatcher. + - `crates/perry-runtime/src/value/dyn_index.rs` — `js_dyn_index_set_strict` + already carried the flag and its array arm forced `true`; it now uses it. + + The realloc arm in `expr/index.rs` deliberately keeps the strict entry: it + runs only for a receiver the guard already accepted, which cannot reject. + + Validation: `test-files/test_gap_9394_array_element_store_strictness.cts` + — a `.cts` file, so it is a CommonJS script in **both** runtimes, with a + sloppy arm and a `"use strict"` arm. **Both arms are asserted.** Asserting + only the throw is precisely what let this through: #9326 shipped with a + 64-check differential and a 205-line gap fixture, all green, none of it + sloppy code. Byte-compared against node 26.5.1; Perry built from unfixed + `origin/main` reports `TypeError` for six sloppy cases where node is silent, + and with this change is identical to node. The #9326 fixture + (`test_gap_9220_9221_array_proto_paths.ts`, an ES module and therefore + strict) is unchanged and still byte-identical to node. + + Unit tests, both arms: `array/strict_store_tests.rs` + `element_store_rejection_throws_only_in_strict_mode`, and #9326's own + `typed_feedback_array_set_guards_reject_frozen_arrays`, which now asserts the + silent sloppy call alongside the strict throw. + + Three pieces of test infrastructure had to admit a `.cts` fixture at all — + each of which would have made it a **dark test**, green because it never ran: + + - `run_parity_tests.sh` discovered the suite with `find … -name '*.ts'`, + which does **not** match `foo.cts` (the suffix is `.cts`). The fixture was + invisible to the harness — confirmed empirically: `--filter test_gap_9394` + selected 0 tests before the change and reports + `PASS test_gap_9394_array_element_store_strictness` after it. + - the same script derived a test's name with `basename … .ts`, which left + such a file called `…strictness.c`. + - `.gitignore` ignores `test-files/test_*` (compiled test binaries) and + re-included only `.ts` / `.tsx`, so the fixture could not be committed. + + Not addressed here, found while writing the fixture: Perry emits + `js_put_value_set(..., strict = 0)` at **every** property-set site, so a + rejected *strict* ordinary-object write (`"use strict"; Object.freeze(o); + o.x = 9`) is silent where node throws. That is the mirror-image gap on the + object path and is out of scope for #9394. diff --git a/crates/perry-codegen/src/expr/index.rs b/crates/perry-codegen/src/expr/index.rs index 2a94aed43b..0d975282bc 100644 --- a/crates/perry-codegen/src/expr/index.rs +++ b/crates/perry-codegen/src/expr/index.rs @@ -134,6 +134,12 @@ pub(crate) fn lower_index_set_fast( // `expr_produces_canonical_raw_f64` — the slot store may skip the // `js_array_numeric_value_to_raw_f64` canonicalization call entirely. value_is_canonical_raw_f64: bool, + // #9394: the assignment's own `Throw` flag (ES2024 §6.2.5.7). The guard + // declines exactly the receivers whose element write can be REJECTED + // (frozen, sealed, descriptor-bearing, prototype-sensitive), so this is + // the flag the fallback continuation needs to decide between a TypeError + // and a silent no-op. + assignment_strict: bool, feedback_site_id: &str, ) -> Result<()> { // #8583-followup: if evaluating an operand diverged — a throwing @@ -389,6 +395,7 @@ pub(crate) fn lower_index_set_fast( ctx.current_block = guard_fallback_idx; { + let strict_flag = if assignment_strict { "1" } else { "0" }; let fallback_box = ctx.block().call( DOUBLE, "js_typed_feedback_array_index_set_fallback_boxed", @@ -397,6 +404,7 @@ pub(crate) fn lower_index_set_fast( (DOUBLE, arr_box), (DOUBLE, idx_double), (DOUBLE, val_double), + (I32, strict_flag), ], ); ctx.block().store(DOUBLE, &fallback_box, &slot); @@ -754,11 +762,14 @@ pub(crate) fn lower_index_set_fast( "js_typed_feedback_record_fallback_call", &[(I64, feedback_site_id)], ); - // Strict `arr[i] = v`: a frozen array's element is non-writable and a - // non-extensible array rejects a new index, so route to the throwing - // variant. (The inline fast/medium paths above are only reached for - // arrays with a proven dense-numeric layout, which excludes frozen / - // sealed / non-extensible arrays — those always fall to this call.) + // Growth for a receiver the guard already ACCEPTED. That guard + // (`plain_array_index_set_guard`) declines frozen, sealed and + // non-extensible arrays, descriptor-bearing arrays, and every + // prototype-sensitive shape — all of which take the `fallback` edge + // above instead — so no store reaching here can be rejected and the + // entry's `Throw` argument is unobservable. The strict entry is kept + // because it is the one that carries the fused key/policy/store + // path (#9394 left this arm alone deliberately). let new_handle = blk.call( I64, "js_array_set_f64_extend_strict", diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 015d21fb41..1792aab463 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -284,6 +284,9 @@ fn lower_array_index_set_via_runtime_key( index: &Expr, value: &Expr, source_label: &str, + // #9394: the assignment's own `Throw` flag, carried to the runtime helper + // so a rejected element write is a TypeError only in strict code. + assignment_strict: bool, ) -> Result { // #7341, same hazard as the packed path: the receiver is live across both // `index` and `value` lowering, and an allocating RHS is a collection @@ -321,6 +324,7 @@ fn lower_array_index_set_via_runtime_key( source_label, TypedFeedbackContract::array_set_index_or_string(), ); + let strict_flag = if assignment_strict { "1" } else { "0" }; let new_handle = ctx.block().call( I64, "js_typed_feedback_array_set_index_or_string", @@ -329,6 +333,7 @@ fn lower_array_index_set_via_runtime_key( (I64, &arr_handle), (DOUBLE, &idx_double), (DOUBLE, &val_double), + (I32, strict_flag), ], ); if let Expr::LocalGet(id) = object { @@ -778,6 +783,7 @@ pub(crate) fn lower( index.as_ref(), value.as_ref(), "array[dynamic_numeric_index]", + assignment_strict, ); } // Same dispatch tree as IndexGet: known array → fast inline, @@ -880,6 +886,7 @@ pub(crate) fn lower( index.as_ref(), value.as_ref(), "array[dynamic_numeric_index]", + assignment_strict, ); }; let layout_note_needed = array_store_needs_layout_note(ctx, object, value); @@ -944,6 +951,8 @@ pub(crate) fn lower( ctx.current_block = fallback_idx; { + let strict_flag = + if assignment_strict { "1" } else { "0" }; let fallback_box = ctx.block().call( DOUBLE, "js_typed_feedback_array_index_set_fallback_boxed", @@ -952,6 +961,7 @@ pub(crate) fn lower( (DOUBLE, &arr_box), (DOUBLE, &idx_double), (DOUBLE, &val_double), + (I32, strict_flag), ], ); if let Some(slot) = ctx.locals.get(arr_id).cloned() { @@ -1180,6 +1190,7 @@ pub(crate) fn lower( value_is_numeric, require_numeric_layout, value_is_canonical_raw_f64, + assignment_strict, &feedback_site_id, )?; } else if let Some(global_name) = ctx.module_globals.get(&id).cloned() { diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index c9efe40066..645665f81d 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -462,10 +462,11 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { I32, &[I64, DOUBLE, DOUBLE], ); + // Trailing I32: the assignment's own strict/`Throw` flag (#9394). module.declare_function( "js_typed_feedback_array_index_set_fallback_boxed", DOUBLE, - &[I64, DOUBLE, DOUBLE, DOUBLE], + &[I64, DOUBLE, DOUBLE, DOUBLE, I32], ); module.declare_function( "js_typed_feedback_observe_array_element", @@ -477,10 +478,11 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { I64, &[I64, I64, I64, DOUBLE], ); + // Trailing I32: the assignment's own strict/`Throw` flag (#9394). module.declare_function( "js_typed_feedback_array_set_index_or_string", I64, - &[I64, I64, DOUBLE, DOUBLE], + &[I64, I64, DOUBLE, DOUBLE, I32], ); module.declare_function( "js_typed_feedback_object_set_index_polymorphic", diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index f08344312a..24ce713be2 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -12,6 +12,7 @@ pub use keyed::{ js_array_get_index_or_string, js_array_set_index_or_string, js_array_set_index_or_string_strict, js_array_set_string_key, }; +pub(crate) use keyed::js_array_set_index_or_string_with_strictness; use proto_chain::array_oob_prototype_get; pub(crate) use proto_chain::{array_custom_prototype, array_spec_get, array_spec_has_index}; use proto_chain::{array_object_proto_index_owner, ArrayCustomProto}; @@ -1185,18 +1186,37 @@ pub extern "C" fn js_array_set_f64_extend_strict( index: u32, value: f64, ) -> *mut ArrayHeader { - js_array_set_f64_extend_strict_impl(arr, index, value, false) + js_array_set_f64_extend_strict_impl(arr, index, value, true, false) } -/// Strict indexed assignment after optionally completing the inherited -/// descriptor walk. `prototype_already_checked` is true only for the callback -/// from [`array_spec_set`]; it prevents a writable inherited data property -/// from recursing when the spec walk proceeds to create the receiver's own -/// element. +/// The same element assignment for a SLOPPY `arr[i] = v` (#9394). +/// +/// ES2024 §6.2.5.7 calls `Set(O, P, V, Throw)` with `Throw = +/// IsStrictReference`, so a rejected element write is a silent no-op outside +/// strict code — for an Array exactly as for an ordinary object. Everything +/// the strict entry does to *find* the right target still runs (the dense +/// lanes, and the #9220 inherited-descriptor walk, so a prototype setter is +/// still invoked with this array as its receiver); only the rejection +/// changes from a TypeError to returning the receiver unchanged. +pub(crate) fn js_array_set_f64_extend_sloppy( + arr: *mut ArrayHeader, + index: u32, + value: f64, +) -> *mut ArrayHeader { + js_array_set_f64_extend_strict_impl(arr, index, value, false, false) +} + +/// Indexed assignment after optionally completing the inherited descriptor +/// walk. `strict` is the assignment's own `Throw` flag (see +/// [`js_array_set_f64_extend_sloppy`]). `prototype_already_checked` is true +/// only for the callback from [`array_spec_set`]; it prevents a writable +/// inherited data property from recursing when the spec walk proceeds to +/// create the receiver's own element. fn js_array_set_f64_extend_strict_impl( arr: *mut ArrayHeader, index: u32, value: f64, + strict: bool, prototype_already_checked: bool, ) -> *mut ArrayHeader { // Two exact fast lanes, each storing only what the general path below @@ -1223,8 +1243,12 @@ fn js_array_set_f64_extend_strict_impl( { // Preserve the existing polymorphic/subclass behavior on receivers // that are not live plain arrays. These are cold and cannot use the - // resolved-header contract below. - array_strict_index_write_guard(arr, index); + // resolved-header contract below. The guard is the *throwing* half of + // the policy, so a sloppy assignment skips it and keeps + // `js_array_set_f64_extend`'s silent contract. + if strict { + array_strict_index_write_guard(arr, index); + } return js_array_set_f64_extend(arr, index, value); } @@ -1248,14 +1272,16 @@ fn js_array_set_f64_extend_strict_impl( && unsafe { array_custom_prototype(clean).is_some() })) && unsafe { !array_has_own_index(clean, index) } { - return array_spec_set(clean, index, value); + return array_spec_set(clean, index, value, strict); } // SAFETY: the clean above resolved this exact live plain-array head. The // guard performs no Perry allocation/safepoint, so the proof remains live // for the store core. let flags = unsafe { array_object_flags_resolved(clean) }; - array_strict_index_write_guard_resolved(clean, index, flags); + if strict { + array_strict_index_write_guard_resolved(clean, index, flags); + } crate::string::js_string_addref_if_heap_string(value); unsafe { js_array_set_f64_extend_resolved(clean, index, value, flags) } } @@ -1610,12 +1636,24 @@ unsafe fn js_array_set_f64_extend_resolved( // to `raw_handle_debt.py --no-raise-vs` as debt appearing in a module that // did not exist at the merge base, which is a raise it refuses by design // (#7659) even though the repository total is unchanged. -/// Spec `Set(O, ToString(index), value, true)` for an Array receiver. Unlike +/// Spec `Set(O, ToString(index), value, Throw)` for an Array receiver. Unlike /// the internal dense setter, this observes an inherited indexed accessor /// before creating an own element. Array mutators use it on their exotic path /// because a prototype setter may mutate the receiver (including freezing it /// or making `length` non-writable) before the mutator's final length Set. -pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> *mut ArrayHeader { +/// +/// `strict` is the assignment's `Throw` argument (#9394): finding the right +/// target is the same work in both modes — an inherited setter runs either +/// way — and only a REJECTION differs, throwing in strict code and returning +/// the receiver unchanged in sloppy code. A spec-internal caller (an array +/// mutator's own `Set`) passes `true`, because those algorithms specify +/// `Throw = true` regardless of the calling code's strictness. +pub(crate) fn array_spec_set( + arr: *mut ArrayHeader, + index: u32, + value: f64, + strict: bool, +) -> *mut ArrayHeader { let arr = clean_arr_ptr_mut(arr); if arr.is_null() { return arr; @@ -1633,6 +1671,7 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> * arr_handle.get_raw_mut_ptr::(), index, value_handle.get_nanbox_f64(), + strict, true, ); } @@ -1675,6 +1714,11 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> * if inherited_owner != 0 { if let Some(accessor) = crate::object::get_accessor_descriptor(inherited_owner, &key) { if accessor.set == 0 { + // A getter-only inherited index rejects the Set. Sloppy + // code observes that as a no-op (#9394). + if !strict { + return arr_handle.get_raw_mut_ptr::(); + } crate::collection_iter::throw_type_error(&format!( "Cannot set property {index} which has only a getter" )); @@ -1689,6 +1733,11 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> * if crate::object::get_property_attrs(inherited_owner, &key) .is_some_and(|attrs| !attrs.writable()) { + // A non-writable inherited data property rejects the Set + // without creating an own element. Silent in sloppy code. + if !strict { + return arr_handle.get_raw_mut_ptr::(); + } throw_frozen_array_index_write(index); } } @@ -1697,6 +1746,7 @@ pub(crate) fn array_spec_set(arr: *mut ArrayHeader, index: u32, value: f64) -> * arr_handle.get_raw_mut_ptr::(), index, value_handle.get_nanbox_f64(), + strict, true, ) } diff --git a/crates/perry-runtime/src/array/indexing_keyed.rs b/crates/perry-runtime/src/array/indexing_keyed.rs index b812fe4b6a..a80ab873e1 100644 --- a/crates/perry-runtime/src/array/indexing_keyed.rs +++ b/crates/perry-runtime/src/array/indexing_keyed.rs @@ -352,6 +352,19 @@ pub extern "C" fn js_array_set_index_or_string_strict( arr: *mut ArrayHeader, idx: f64, value: f64, +) -> *mut ArrayHeader { + js_array_set_index_or_string_with_strictness(arr, idx, value, true) +} + +/// [`js_array_set_index_or_string_strict`] with the assignment's own `Throw` +/// flag (#9394). A sloppy `arr[k] = v` still takes the fused element path — +/// same key canonicalization, same inherited-descriptor walk — but a rejected +/// write is a silent no-op instead of a TypeError. +pub(crate) fn js_array_set_index_or_string_with_strictness( + arr: *mut ArrayHeader, + idx: f64, + value: f64, + strict: bool, ) -> *mut ArrayHeader { if !arr.is_null() { // Resolve the canonical array-index interpretation of the key (mirrors @@ -365,7 +378,11 @@ pub extern "C" fn js_array_set_index_or_string_strict( // canonical index is already proved here, so use the fused strict // element path and share one receiver resolution across policy // and store. - return js_array_set_f64_extend_strict(arr, i, value); + return if strict { + js_array_set_f64_extend_strict(arr, i, value) + } else { + crate::array::js_array_set_f64_extend_sloppy(arr, i, value) + }; } } js_array_set_index_or_string(arr, idx, value) diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 5fb029c72a..65f77ac221 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -149,6 +149,9 @@ pub use self::indexing::{ js_array_set_f64_extend, js_array_set_f64_extend_strict, js_array_set_f64_unchecked, js_array_set_index_or_string, js_array_set_index_or_string_strict, js_array_set_string_key, }; +pub(crate) use self::indexing::{ + js_array_set_f64_extend_sloppy, js_array_set_index_or_string_with_strictness, +}; #[cfg(test)] pub(crate) use self::indexing_support::test_keys_array_slot_fallbacks; pub(crate) use self::indexing_support::{ diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 260b4457ff..5dffd08002 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -896,10 +896,13 @@ fn push_array_spec_path(arr: *mut ArrayHeader, value: f64) -> *mut ArrayHeader { crate::array::array_length_range_error(); } + // `Array.prototype.push` step 4.d specifies `Set(O, …, true)` — the + // mutator's own Throw, independent of the caller's strictness (#9394). let next = crate::array::array_spec_set( arr_handle.get_raw_mut_ptr::(), length, value_handle.get_nanbox_f64(), + true, ); let next = clean_arr_ptr_mut(next); if !next.is_null() { @@ -1575,7 +1578,9 @@ fn shift_array_spec_set( ) { let (next, post_gc) = arr_handle.across_mut::(|| { let value = value_handle.get_nanbox_f64(); - arr_handle.with_mut_ptr(|current| crate::array::array_spec_set(current, index, value)) + // The mutators specify `Set(O, …, true)` regardless of the calling + // code's strictness (#9394). + arr_handle.with_mut_ptr(|current| crate::array::array_spec_set(current, index, value, true)) }); let next = clean_arr_ptr_mut(next); let current = if next.is_null() { diff --git a/crates/perry-runtime/src/array/strict_store_tests.rs b/crates/perry-runtime/src/array/strict_store_tests.rs index 3f3f3c092a..e2ed0450c0 100644 --- a/crates/perry-runtime/src/array/strict_store_tests.rs +++ b/crates/perry-runtime/src/array/strict_store_tests.rs @@ -153,3 +153,81 @@ fn strict_dense_number_store_fast_lane_matches_the_general_path() { crate::object::prototype_chain::test_swap_array_static_proto_recorded(latch_was); } } + +/// Whether `f` threw a runtime exception. +fn catch_runtime_throw(f: impl FnOnce()) -> bool { + let env = crate::exception::js_try_push(); + let jumped = unsafe { crate::ffi::setjmp::setjmp(env as *mut std::os::raw::c_int) }; + if jumped == 0 { + f(); + crate::exception::js_try_end(); + false + } else { + crate::exception::js_try_end(); + crate::exception::js_clear_exception(); + true + } +} + +/// #9394: a rejected element write throws only in STRICT code. +/// +/// ES2024 §6.2.5.7 calls `Set(O, P, V, Throw)` with `Throw = +/// IsStrictReference`, so a sloppy `arr[i] = v` against a frozen / +/// non-extensible / non-writable target is a silent no-op — exactly as it is +/// for an ordinary object, and exactly as Node behaves. +/// +/// BOTH arms are asserted here on purpose. #9326 shipped the throw-only half +/// with a 64-check differential and a 205-line gap fixture, all of it module +/// (strict) code, and every one of them stayed green while sloppy code — the +/// whole of a CommonJS bundle — started throwing. +#[test] +fn element_store_rejection_throws_only_in_strict_mode() { + // SAFETY: plain array construction plus the public element setters; every + // pointer below is a live head this test allocated. + unsafe { + let values = [1.0, 2.0, 3.0]; + + let frozen = js_array_from_f64(values.as_ptr(), values.len() as u32); + crate::object::js_object_freeze(crate::value::js_nanbox_pointer(frozen as i64)); + + assert!( + catch_runtime_throw(|| { + js_array_set_f64_extend_strict(frozen, 0, 9.0); + }), + "strict: a frozen element is non-writable" + ); + assert_eq!(js_array_get_f64(frozen, 0), 1.0); + + assert!( + !catch_runtime_throw(|| { + crate::array::js_array_set_f64_extend_sloppy(frozen, 0, 9.0); + }), + "sloppy: the same rejection is silent" + ); + assert_eq!(js_array_get_f64(frozen, 0), 1.0); + + // A new index on a non-extensible array is the other rejection shape. + assert!( + catch_runtime_throw(|| { + js_array_set_f64_extend_strict(frozen, 7, 9.0); + }), + "strict: a frozen array cannot gain an element" + ); + assert!( + !catch_runtime_throw(|| { + crate::array::js_array_set_f64_extend_sloppy(frozen, 7, 9.0); + }), + "sloppy: the same rejection is silent" + ); + assert_eq!((*frozen).length, 3); + + // A writable element is stored in both modes — the sloppy entry is a + // no-op only where the strict one would have thrown. + let open = js_array_from_f64(values.as_ptr(), values.len() as u32); + let out = crate::array::js_array_set_f64_extend_sloppy(open, 0, 42.0); + assert_eq!(js_array_get_f64(out, 0), 42.0); + let out = crate::array::js_array_set_f64_extend_sloppy(out, 3, 4.0); + assert_eq!((*out).length, 4); + assert_eq!(js_array_get_f64(out, 3), 4.0); + } +} diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index 97e74a4c09..04be9adad4 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -2545,12 +2545,22 @@ pub extern "C" fn js_typed_feedback_numeric_array_push_guard( } } +/// Cold continuation of a source-level `arr[index] = value` after the inline +/// guard declines the receiver. +/// +/// `strict` is the assignment's own `Throw` flag (ES2024 §6.2.5.7): codegen +/// passes `1` from strict code and `0` from sloppy code. #9394: this helper +/// used the strict element entry unconditionally, so a rejected write (frozen +/// array, non-writable own or inherited index, non-extensible receiver) threw +/// a TypeError in sloppy code where Node is silent — the guard declines +/// exactly those shapes, so every one of them arrived here. #[no_mangle] pub extern "C" fn js_typed_feedback_array_index_set_fallback_boxed( site_id: u64, receiver: f64, index: f64, value: f64, + strict: i32, ) -> f64 { record_fallback_call(site_id); @@ -2598,17 +2608,18 @@ pub extern "C" fn js_typed_feedback_array_index_set_fallback_boxed( (raw_addr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader; match (*gc_header).obj_type { crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => { - // #9220: this is the cold continuation of a source-level - // `arr[index] = value` after the inline guard rejects a - // retargeted/prototype-sensitive array. It must preserve the - // assignment's strict Set semantics; the non-strict helper - // bypassed `js_array_set_f64_extend_strict` entirely, so an - // inherited setter/non-writable index was silently replaced - // by a new own element. - let new_arr = crate::array::js_array_set_index_or_string_strict( + // #9220: the inherited-descriptor walk must run in BOTH + // modes — a prototype setter fires on a sloppy assignment + // too, and the pre-#9220 non-strict helper bypassed it + // entirely, silently replacing an inherited setter / + // non-writable index with a new own element. #9394: only the + // REJECTION is strictness-dependent, so carry the + // assignment's own `Throw` rather than forcing `true`. + let new_arr = crate::array::js_array_set_index_or_string_with_strictness( raw_addr as *mut ArrayHeader, index, value, + strict != 0, ); crate::value::js_nanbox_pointer(new_arr as i64) } @@ -2680,19 +2691,27 @@ pub extern "C" fn js_typed_feedback_array_set_string_key( crate::array::js_array_set_string_key(arr, key, value) } +/// Assignment-site wrapper for `arr[] = value`. `strict` is the +/// assignment's own `Throw` flag (#9394). #[no_mangle] pub extern "C" fn js_typed_feedback_array_set_index_or_string( site_id: u64, arr: *mut ArrayHeader, idx: f64, value: f64, + strict: i32, ) -> *mut ArrayHeader { // #5094 for the assignment site: with recording off (the default) every // helper below early-returns, but the index conversion and two // out-of-line calls to reach those returns were 1.5% of an ECS frame on // `column[index] = record`. One flag test, then the store. if !typed_feedback_enabled() { - return crate::array::js_array_set_index_or_string_strict(arr, idx, value); + return crate::array::js_array_set_index_or_string_with_strictness( + arr, + idx, + value, + strict != 0, + ); } let index = finite_nonnegative_u32_index(idx).unwrap_or(u32::MAX); observe_array(site_id, arr, index); @@ -2702,9 +2721,10 @@ pub extern "C" fn js_typed_feedback_array_set_index_or_string( } else { record_guard_pass(site_id); } - // Assignment-site wrapper → strict `Set` with `Throw = true` (frozen / - // non-extensible array element write throws a TypeError). - crate::array::js_array_set_index_or_string_strict(arr, idx, value) + // Assignment-site wrapper → spec `Set` with the reference's own `Throw`: + // a frozen / non-extensible array element write throws a TypeError in + // strict code and is a silent no-op in sloppy code. + crate::array::js_array_set_index_or_string_with_strictness(arr, idx, value, strict != 0) } #[no_mangle] diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index 58cabd8bab..4cd407a81c 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -552,7 +552,7 @@ fn typed_feedback_non_bounded_array_set_guard_failure_uses_jsvalue_object_fallba let guard = js_typed_feedback_plain_array_index_set_guard(24, obj_box, 0, 99.0, 0); assert_eq!(guard, 0); - let returned = js_typed_feedback_array_index_set_fallback_boxed(24, obj_box, 0.0, 99.0); + let returned = js_typed_feedback_array_index_set_fallback_boxed(24, obj_box, 0.0, 99.0, 1); assert_eq!(returned.to_bits(), obj_box.to_bits()); let key = crate::string::js_string_from_bytes(b"0".as_ptr(), 1); @@ -586,8 +586,18 @@ fn typed_feedback_array_set_guards_reject_frozen_arrays() { 0 ); + // #9394: BOTH arms. A rejected element write throws only in strict code — + // asserting the throw alone is exactly what let the sloppy regression + // through a 64-check differential and a 205-line gap fixture. assert!(catch_runtime_throw(|| { - js_typed_feedback_array_index_set_fallback_boxed(70, arr_box, 0.0, 99.0); + js_typed_feedback_array_index_set_fallback_boxed(70, arr_box, 0.0, 99.0, 1); + })); + assert_eq!( + crate::array::js_array_get_f64(arr, 0).to_bits(), + 1.0f64.to_bits() + ); + assert!(!catch_runtime_throw(|| { + js_typed_feedback_array_index_set_fallback_boxed(70, arr_box, 0.0, 99.0, 0); })); assert_eq!( crate::array::js_array_get_f64(arr, 0).to_bits(), @@ -610,7 +620,7 @@ fn typed_feedback_array_set_boxed_fallback_preserves_original_index_value() { let key = crate::string::js_string_from_bytes(b"foo".as_ptr(), 3); let key_value = crate::value::js_nanbox_string(key as i64); - let returned = js_typed_feedback_array_index_set_fallback_boxed(72, obj_box, key_value, 77.0); + let returned = js_typed_feedback_array_index_set_fallback_boxed(72, obj_box, key_value, 77.0, 1); assert_eq!(returned.to_bits(), obj_box.to_bits()); assert_eq!( crate::object::js_object_get_field_by_name_f64(obj, key).to_bits(), @@ -693,24 +703,24 @@ fn typed_feedback_boxed_set_fallback_does_not_truncate_fractional_array_like_key let buf = crate::buffer::js_buffer_alloc(3, 0); crate::buffer::js_buffer_set(buf, 1, 22); let buf_box = crate::value::js_nanbox_pointer(buf as i64); - js_typed_feedback_array_index_set_fallback_boxed(74, buf_box, 1.5, 99.0); + js_typed_feedback_array_index_set_fallback_boxed(74, buf_box, 1.5, 99.0, 1); assert_eq!(crate::buffer::js_buffer_get(buf, 1), 22); - js_typed_feedback_array_index_set_fallback_boxed(74, buf_box, 1.0, 99.0); + js_typed_feedback_array_index_set_fallback_boxed(74, buf_box, 1.0, 99.0, 1); assert_eq!(crate::buffer::js_buffer_get(buf, 1), 99); let ta = crate::typedarray::js_typed_array_new_empty(crate::typedarray::KIND_UINT8 as i32, 3); crate::typedarray::js_typed_array_set(ta, 1, 33.0); let ta_box = crate::value::js_nanbox_pointer(ta as i64); - js_typed_feedback_array_index_set_fallback_boxed(74, ta_box, 1.5, 88.0); + js_typed_feedback_array_index_set_fallback_boxed(74, ta_box, 1.5, 88.0, 1); assert_eq!(crate::typedarray::js_typed_array_get(ta, 1), 33.0); - js_typed_feedback_array_index_set_fallback_boxed(74, ta_box, 1.0, 88.0); + js_typed_feedback_array_index_set_fallback_boxed(74, ta_box, 1.0, 88.0, 1); assert_eq!(crate::typedarray::js_typed_array_get(ta, 1), 88.0); let set = crate::set::js_set_alloc(4); crate::set::js_set_add(set, 10.0); crate::set::js_set_add(set, 20.0); let set_box = crate::value::js_nanbox_pointer(set as i64); - js_typed_feedback_array_index_set_fallback_boxed(74, set_box, 1.5, 77.0); + js_typed_feedback_array_index_set_fallback_boxed(74, set_box, 1.5, 77.0, 1); assert_eq!(crate::set::js_set_size(set), 2); assert_eq!(crate::set::js_set_value_at(set, 1), 20.0); @@ -718,7 +728,7 @@ fn typed_feedback_boxed_set_fallback_does_not_truncate_fractional_array_like_key crate::map::js_map_set(map, 10.0, 100.0); crate::map::js_map_set(map, 20.0, 200.0); let map_box = crate::value::js_nanbox_pointer(map as i64); - js_typed_feedback_array_index_set_fallback_boxed(74, map_box, 1.5, 66.0); + js_typed_feedback_array_index_set_fallback_boxed(74, map_box, 1.5, 66.0, 1); assert_eq!(crate::map::js_map_size(map), 2); assert_eq!(crate::map::js_map_entry_key_at(map, 1), 20.0); diff --git a/crates/perry-runtime/src/typed_feedback/trace.rs b/crates/perry-runtime/src/typed_feedback/trace.rs index bfdf1b79dd..370d4a5ee2 100644 --- a/crates/perry-runtime/src/typed_feedback/trace.rs +++ b/crates/perry-runtime/src/typed_feedback/trace.rs @@ -430,13 +430,13 @@ mod keep_typed_feedback { #[cfg(feature = "keepalive-anchors")] #[used] static K21: extern "C" fn(u64, f64, f64) -> i32 = js_typed_feedback_numeric_array_push_guard; #[cfg(feature = "keepalive-anchors")] -#[used] static K22: extern "C" fn(u64, f64, f64, f64) -> f64 = js_typed_feedback_array_index_set_fallback_boxed; +#[used] static K22: extern "C" fn(u64, f64, f64, f64, i32) -> f64 = js_typed_feedback_array_index_set_fallback_boxed; #[cfg(feature = "keepalive-anchors")] #[used] static K23: extern "C" fn(u64, *const ArrayHeader, u32) = js_typed_feedback_observe_array_element; #[cfg(feature = "keepalive-anchors")] #[used] static K24: extern "C" fn(u64, *mut ArrayHeader, *const crate::StringHeader, f64) -> *mut ArrayHeader = js_typed_feedback_array_set_string_key; #[cfg(feature = "keepalive-anchors")] -#[used] static K25: extern "C" fn(u64, *mut ArrayHeader, f64, f64) -> *mut ArrayHeader = js_typed_feedback_array_set_index_or_string; +#[used] static K25: extern "C" fn(u64, *mut ArrayHeader, f64, f64, i32) -> *mut ArrayHeader = js_typed_feedback_array_set_index_or_string; #[cfg(feature = "keepalive-anchors")] #[used] static K26: extern "C" fn(u64, i64, f64, f64) = js_typed_feedback_object_set_index_polymorphic; #[cfg(feature = "keepalive-anchors")] diff --git a/crates/perry-runtime/src/value/dyn_index.rs b/crates/perry-runtime/src/value/dyn_index.rs index 857df74a48..9d94ab7329 100644 --- a/crates/perry-runtime/src/value/dyn_index.rs +++ b/crates/perry-runtime/src/value/dyn_index.rs @@ -529,7 +529,8 @@ pub extern "C" fn js_dyn_index_get(value: f64, index: f64) -> f64 { /// computed assignments use [`js_dyn_index_set_strict`] below. /// /// Routes by the receiver's `gc_type` byte: arrays go through -/// `js_array_set_index_or_string_strict` (numeric/string-key spec dispatch); +/// `js_array_set_index_or_string_with_strictness` (numeric/string-key spec +/// dispatch, carrying this entry's `strict` flag); /// ordinary objects retain receiver-aware property `[[Set]]` semantics. /// Strings are immutable — no-op (matches /// strict-mode `s[i] = x` semantics, close enough for the `++result[key]` @@ -767,10 +768,15 @@ pub extern "C" fn js_dyn_index_set_strict(obj: f64, index: f64, value: f64, stri } let is_array = receiver_tag.is_some_and(|(obj_type, _)| obj_type == crate::gc::GC_TYPE_ARRAY); if is_array { - crate::array::js_array_set_index_or_string_strict( + // #9394: this entry already carries the assignment's own `Throw` + // flag (`js_dyn_index_set` passes 0 for the sloppy runtime callers); + // the array arm forced `true` and threw on a frozen / non-writable / + // non-extensible element write that Node silently drops. + crate::array::js_array_set_index_or_string_with_strictness( raw_ptr as *mut crate::array::ArrayHeader, index, value, + strict != 0, ); return value; } diff --git a/run_parity_tests.sh b/run_parity_tests.sh index 2014dd2f1d..5c0039d079 100755 --- a/run_parity_tests.sh +++ b/run_parity_tests.sh @@ -1187,7 +1187,14 @@ case "$TEST_SUITE" in all) while IFS= read -r test_file; do TEST_FILES+=("$test_file") - done < <(find "$TEST_DIR" -maxdepth 1 -type f -name '*.ts' | sort) + # `.cts` / `.mts` as well as `.ts`: a fixture whose semantics depend on + # the module goal has to name it in the extension, because this repo's + # package is `"type": "module"` and a plain `.ts` is therefore + # strict-mode ESM for Node and for Perry alike. `-name '*.ts'` does NOT + # match `foo.cts` (the suffix is `.cts`), so such a fixture was invisible + # to the suite — a dark test, green because it never ran. + done < <(find "$TEST_DIR" -maxdepth 1 -type f \ + \( -name '*.ts' -o -name '*.cts' -o -name '*.mts' \) | sort) ;; parity|smoke) while IFS= read -r test_file; do @@ -1232,7 +1239,12 @@ for test_file in "${TEST_FILES[@]}"; do # Skip directories (multi/ folder) [[ -d "$test_file" ]] && continue - test_name=$(basename "$test_file" .ts) + # Strip any TypeScript extension, not just `.ts`: a fixture that has to be + # CommonJS in BOTH runtimes is a `.cts` (see + # test_gap_9394_array_element_store_strictness.cts, which needs sloppy-mode + # semantics that a `.ts` under this repo's `"type": "module"` cannot have). + # `basename … .ts` left such a file named `…strictness.c`. + test_name=$(basename "$test_file" | sed -E 's/\.(m|c)?ts$//') if [[ "$test_file" == "$NODE_SUITE_DIR"/* ]]; then test_rel="${test_file#"$NODE_SUITE_DIR"/}" test_id="node-suite/${test_rel%.ts}" diff --git a/test-files/test_gap_9394_array_element_store_strictness.cts b/test-files/test_gap_9394_array_element_store_strictness.cts new file mode 100644 index 0000000000..d80398bf8a --- /dev/null +++ b/test-files/test_gap_9394_array_element_store_strictness.cts @@ -0,0 +1,299 @@ +// #9394: a failed indexed [[Set]] on an Array throws ONLY in strict mode. +// +// ES2024 SS6.2.5.7 (PutValue) calls `Set(O, P, V, Throw)` with +// Throw = IsStrictReference(ref). A rejected write is therefore a silent no-op +// in sloppy code and a TypeError in strict code -- for arrays exactly as for +// ordinary objects. #9326 ("indexed writes honour a custom array prototype", +// live again via #9370) routed the cold element-store continuation through the +// STRICT runtime entry unconditionally, so every rejected array element write +// began throwing regardless of the assignment's own strictness. Plain objects +// were unaffected, which is why a 64-check differential and a 205-line gap +// fixture -- all of it module (strict) code -- stayed green. +// +// 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. +// ASSERTING ONLY THE THROW IS WHAT LET THIS THROUGH, so every case below is +// asserted in both modes. +// +// The ordinary-object control appears in the sloppy arm only. Perry emits +// `js_put_value_set(..., strict = 0)` at every property-set site, so a rejected +// STRICT ordinary-object write is silent too -- a separate, pre-existing gap +// that is not what #9394 is about and is not fixed here. +// +// 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 sloppy twice. Only the mode prefix differs. + +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 lockedProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "6", { + configurable: true, + enumerable: true, + value: "lockedSix", + writable: false, + }); + return proto; +} + +function getterOnlyProto(): any { + const proto: any = {}; + Object.defineProperty(proto, "4", { + configurable: true, + get() { + return "getterOnly4"; + }, + }); + return proto; +} + +function setterProto(calls: any[]): any { + const proto: any = {}; + Object.defineProperty(proto, "4", { + configurable: true, + get() { + return "acc4"; + }, + set(value: any) { + calls.push(value); + }, + }); + return proto; +} + +function sloppyArm(): void { + let threw = false; + + const frozen: any[] = [1, 2, 3]; + Object.freeze(frozen); + threw = false; + try { + frozen[0] = 9; + } catch { + threw = true; + } + report("sloppy frozen in-bounds:", threw, frozen[0], frozen.length); + + const frozenOob: any[] = [1]; + Object.freeze(frozenOob); + threw = false; + try { + frozenOob[5] = 9; + } catch { + threw = true; + } + report("sloppy frozen new-index:", threw, frozenOob[5], frozenOob.length); + + const readOnly: any[] = [1]; + Object.defineProperty(readOnly, 0, { writable: false }); + threw = false; + try { + readOnly[0] = 9; + } catch { + threw = true; + } + report("sloppy non-writable own index:", threw, readOnly[0], readOnly.length); + + const noExtend: any[] = [1]; + Object.preventExtensions(noExtend); + threw = false; + try { + noExtend[5] = 9; + } catch { + threw = true; + } + report("sloppy preventExtensions new-index:", threw, noExtend[5], noExtend.length); + + // Sealed leaves existing elements writable, so this succeeds in both modes. + const sealed: any[] = [1]; + Object.seal(sealed); + threw = false; + try { + sealed[0] = 9; + } catch { + threw = true; + } + report("sloppy sealed in-bounds:", threw, sealed[0], sealed.length); + + // Control: the ordinary-object [[Set]] path, which already honours Throw. + const obj: any = { x: 1 }; + Object.freeze(obj); + threw = false; + try { + obj.x = 9; + } catch { + threw = true; + } + report("sloppy frozen plain object:", threw, obj.x); + + // #9220 shapes: an inherited index is consulted before an own element is + // created. Rejecting it is exactly what has to become mode-sensitive. + const lockedTarget: any = [1]; + Object.setPrototypeOf(lockedTarget, lockedProto()); + threw = false; + try { + lockedTarget[6] = "changed"; + } catch { + threw = true; + } + report( + "sloppy inherited non-writable:", + threw, + hasOwn(lockedTarget, 6), + lockedTarget[6], + lockedTarget.length, + ); + + const getterOnlyTarget: any = [1]; + Object.setPrototypeOf(getterOnlyTarget, getterOnlyProto()); + threw = false; + try { + getterOnlyTarget[4] = "changed"; + } catch { + threw = true; + } + report( + "sloppy inherited getter-only:", + threw, + hasOwn(getterOnlyTarget, 4), + getterOnlyTarget[4], + getterOnlyTarget.length, + ); + + // An inherited setter runs in both modes and creates no own element. + const calls: any[] = []; + const setterTarget: any = [1]; + Object.setPrototypeOf(setterTarget, setterProto(calls)); + threw = false; + try { + setterTarget[4] = 11; + } catch { + threw = true; + } + report( + "sloppy inherited setter:", + threw, + calls.join(","), + hasOwn(setterTarget, 4), + setterTarget.length, + ); +} + +function strictArm(): void { + "use strict"; + + let threw = false; + + const frozen: any[] = [1, 2, 3]; + Object.freeze(frozen); + threw = false; + try { + frozen[0] = 9; + } catch { + threw = true; + } + report("strict frozen in-bounds:", threw, frozen[0], frozen.length); + + const frozenOob: any[] = [1]; + Object.freeze(frozenOob); + threw = false; + try { + frozenOob[5] = 9; + } catch { + threw = true; + } + report("strict frozen new-index:", threw, frozenOob[5], frozenOob.length); + + const readOnly: any[] = [1]; + Object.defineProperty(readOnly, 0, { writable: false }); + threw = false; + try { + readOnly[0] = 9; + } catch { + threw = true; + } + report("strict non-writable own index:", threw, readOnly[0], readOnly.length); + + const noExtend: any[] = [1]; + Object.preventExtensions(noExtend); + threw = false; + try { + noExtend[5] = 9; + } catch { + threw = true; + } + report("strict preventExtensions new-index:", threw, noExtend[5], noExtend.length); + + // Sealed leaves existing elements writable, so this succeeds in both modes. + const sealed: any[] = [1]; + Object.seal(sealed); + threw = false; + try { + sealed[0] = 9; + } catch { + threw = true; + } + report("strict sealed in-bounds:", threw, sealed[0], sealed.length); + + // #9220 shapes: an inherited index is consulted before an own element is + // created. Rejecting it is exactly what has to become mode-sensitive. + const lockedTarget: any = [1]; + Object.setPrototypeOf(lockedTarget, lockedProto()); + threw = false; + try { + lockedTarget[6] = "changed"; + } catch { + threw = true; + } + report( + "strict inherited non-writable:", + threw, + hasOwn(lockedTarget, 6), + lockedTarget[6], + lockedTarget.length, + ); + + const getterOnlyTarget: any = [1]; + Object.setPrototypeOf(getterOnlyTarget, getterOnlyProto()); + threw = false; + try { + getterOnlyTarget[4] = "changed"; + } catch { + threw = true; + } + report( + "strict inherited getter-only:", + threw, + hasOwn(getterOnlyTarget, 4), + getterOnlyTarget[4], + getterOnlyTarget.length, + ); + + // An inherited setter runs in both modes and creates no own element. + const calls: any[] = []; + const setterTarget: any = [1]; + Object.setPrototypeOf(setterTarget, setterProto(calls)); + threw = false; + try { + setterTarget[4] = 11; + } catch { + threw = true; + } + report( + "strict inherited setter:", + threw, + calls.join(","), + hasOwn(setterTarget, 4), + setterTarget.length, + ); +} + +sloppyArm(); +strictArm(); From 144b1e5503991ea4d53971c053a0382a94d4a5d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 1 Sep 2026 19:39:00 +0200 Subject: [PATCH 2/2] refactor(runtime): one exit for prototype-handled array stores #9394's two sloppy-mode no-op returns took indexing.rs from 7 raw-handle sites to 9, over its ceiling. Routing every prototype-handled path through a single exit re-derives the receiver once instead of three times, which is fewer real re-reads rather than a wrapper that only hides them. --- crates/perry-codegen/src/expr/index_set.rs | 3 +- crates/perry-runtime/src/array/indexing.rs | 44 +++++++++++-------- .../perry-runtime/src/typed_feedback/tests.rs | 3 +- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 1792aab463..2366d4d775 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -951,8 +951,7 @@ pub(crate) fn lower( ctx.current_block = fallback_idx; { - let strict_flag = - if assignment_strict { "1" } else { "0" }; + let strict_flag = if assignment_strict { "1" } else { "0" }; let fallback_box = ctx.block().call( DOUBLE, "js_typed_feedback_array_index_set_fallback_boxed", diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 24ce713be2..67fdc2cbff 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -8,11 +8,11 @@ use std::sync::atomic::Ordering; mod keyed; #[path = "indexing_proto_chain.rs"] mod proto_chain; +pub(crate) use keyed::js_array_set_index_or_string_with_strictness; pub use keyed::{ js_array_get_index_or_string, js_array_set_index_or_string, js_array_set_index_or_string_strict, js_array_set_string_key, }; -pub(crate) use keyed::js_array_set_index_or_string_with_strictness; use proto_chain::array_oob_prototype_get; pub(crate) use proto_chain::{array_custom_prototype, array_spec_get, array_spec_has_index}; use proto_chain::{array_object_proto_index_owner, ArrayCustomProto}; @@ -1711,36 +1711,44 @@ pub(crate) fn array_spec_set( }; } + // Every path that the inherited property fully handles — setter + // invoked, or the Set rejected — leaves through the one exit below. + // A single re-derive of the receiver serves all of them: each of these + // arms can run JS or allocate, so the pointer has to be re-read, and + // reading it once here rather than at three returns keeps that explicit. + let mut handled_by_prototype = false; if inherited_owner != 0 { if let Some(accessor) = crate::object::get_accessor_descriptor(inherited_owner, &key) { if accessor.set == 0 { // A getter-only inherited index rejects the Set. Sloppy - // code observes that as a no-op (#9394). - if !strict { - return arr_handle.get_raw_mut_ptr::(); + // code observes that as a no-op (#9394); strict throws. + if strict { + crate::collection_iter::throw_type_error(&format!( + "Cannot set property {index} which has only a getter" + )); } - crate::collection_iter::throw_type_error(&format!( - "Cannot set property {index} which has only a getter" - )); + } else { + crate::object::invoke_accessor_setter( + accessor.set, + receiver(), + value_handle.get_nanbox_f64(), + ); } - crate::object::invoke_accessor_setter( - accessor.set, - receiver(), - value_handle.get_nanbox_f64(), - ); - return arr_handle.get_raw_mut_ptr::(); - } - if crate::object::get_property_attrs(inherited_owner, &key) + handled_by_prototype = true; + } else if crate::object::get_property_attrs(inherited_owner, &key) .is_some_and(|attrs| !attrs.writable()) { // A non-writable inherited data property rejects the Set // without creating an own element. Silent in sloppy code. - if !strict { - return arr_handle.get_raw_mut_ptr::(); + if strict { + throw_frozen_array_index_write(index); } - throw_frozen_array_index_write(index); + handled_by_prototype = true; } } + if handled_by_prototype { + return arr_handle.get_raw_mut_ptr::(); + } js_array_set_f64_extend_strict_impl( arr_handle.get_raw_mut_ptr::(), diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index 4cd407a81c..e4ecb07de1 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -620,7 +620,8 @@ fn typed_feedback_array_set_boxed_fallback_preserves_original_index_value() { let key = crate::string::js_string_from_bytes(b"foo".as_ptr(), 3); let key_value = crate::value::js_nanbox_string(key as i64); - let returned = js_typed_feedback_array_index_set_fallback_boxed(72, obj_box, key_value, 77.0, 1); + let returned = + js_typed_feedback_array_index_set_fallback_boxed(72, obj_box, key_value, 77.0, 1); assert_eq!(returned.to_bits(), obj_box.to_bits()); assert_eq!( crate::object::js_object_get_field_by_name_f64(obj, key).to_bits(),