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
24 changes: 24 additions & 0 deletions changelog.d/7381-mirror-static-write-refresh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
**Fixed** `js_object_set_field_by_name` passed a stale `obj` to
`mirror_class_object_static_write` on both own-data-write arms.

The two arms perform the property write (an inline slot store, or `overflow_set`
into the overflow map) and then call the class-static mirror hook. Both writes
can reach an allocator — `overflow_set` inserts into a Rust map, and a minor GC
triggers on the malloc-count threshold as well as on arena-block allocation — and
the mirror's first instruction dereferences `obj`. A collection in that write
leaves the hook reading from-space.

The function already carries a `refresh_roots_after_alloc!()` macro and calls it
after every other allocating step; these two sites were missed. Scoped to those
two arms on purpose: the macro also republishes `value` from its handle, so arms
that intentionally rebind `value` locally must not refresh.

Found by #7341's from-space quarantine via
`test_gap_gc_assign_string_source_rooting`. With the fix the fault moves out of
`mirror_class_object_static_write` entirely and into a separate, unrelated catch
in `js_string_index_get`, which remains open.

Baselines checked rather than assumed: `test_gap_2159_defineproperty_class_prototype`
and `test_gap_6301_event_target_subclass` fail identically on pristine `main`, and
`perry-runtime --lib` fails 3/5/3 tests across three runs of pristine `main`
(#7365), so neither is attributable to this change.
22 changes: 22 additions & 0 deletions crates/perry-runtime/src/object/field_set_by_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,17 @@ pub extern "C" fn js_object_set_field_by_name(
// (bundled zod assigns `create` onto ~40 sibling class
// objects), so from the SECOND class on the write lands
// here — the mirror must fire on this path too.
// #7341: the own-data write just above can reach an allocator --
// `overflow_set` inserts into a Rust map, and a minor GC triggers on
// the malloc-count threshold as well as on arena blocks. The mirror's
// FIRST instruction dereferences `obj` (`ldr w8, [x0]` at +24), so a
// collection in that write leaves it reading from-space.
//
// Scoped deliberately to the two own-data-write arms. The macro also
// republishes `value` from its handle, so an arm that intentionally
// rebinds `value` locally must NOT refresh here -- applying it to all
// eight mirror sites is wrong for that reason.
refresh_roots_after_alloc!();
mirror_class_object_static_write(obj, key, value);
return;
}
Expand Down Expand Up @@ -1439,6 +1450,17 @@ pub extern "C" fn js_object_set_field_by_name(
(*obj).field_count = 1;
}
js_object_set_field(obj, 0, JSValue::from_bits(value.to_bits()));
// #7341: the own-data write just above can reach an allocator --
// `overflow_set` inserts into a Rust map, and a minor GC triggers on
// the malloc-count threshold as well as on arena blocks. The mirror's
// FIRST instruction dereferences `obj` (`ldr w8, [x0]` at +24), so a
// collection in that write leaves it reading from-space.
//
// Scoped deliberately to the two own-data-write arms. The macro also
// republishes `value` from its handle, so an arm that intentionally
// rebinds `value` locally must NOT refresh here -- applying it to all
// eight mirror sites is wrong for that reason.
refresh_roots_after_alloc!();
mirror_class_object_static_write(obj, key, value);
// Record the null→single-key transition so the next object
// that starts with `{}` and sets the same first key hits the
Expand Down
Loading