diff --git a/changelog.d/7381-mirror-static-write-refresh.md b/changelog.d/7381-mirror-static-write-refresh.md new file mode 100644 index 0000000000..f3b519b5b8 --- /dev/null +++ b/changelog.d/7381-mirror-static-write-refresh.md @@ -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. diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index 5d5b0aa62c..e8f068b9b4 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -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; } @@ -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