Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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-*
Expand Down
89 changes: 89 additions & 0 deletions changelog.d/9394-sloppy-array-element-store.md
Original file line number Diff line number Diff line change
@@ -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.
21 changes: 16 additions & 5 deletions crates/perry-codegen/src/expr/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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);
Expand Down Expand Up @@ -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",
Expand Down
10 changes: 10 additions & 0 deletions crates/perry-codegen/src/expr/index_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
// #7341, same hazard as the packed path: the receiver is live across both
// `index` and `value` lowering, and an allocating RHS is a collection
Expand Down Expand Up @@ -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",
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -944,6 +951,7 @@ 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",
Expand All @@ -952,6 +960,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() {
Expand Down Expand Up @@ -1180,6 +1189,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() {
Expand Down
6 changes: 4 additions & 2 deletions crates/perry-codegen/src/runtime_decls/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
Loading
Loading