From 62b48036f21c0aa45269cba05ec948ed67590246 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 17:08:19 +0200 Subject: [PATCH] fix(gc): refresh obj before the class-static mirror on both own-data-write arms js_object_set_field_by_name performs the property write -- an inline slot store, or overflow_set into the overflow map -- and then calls mirror_class_object_static_write. 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 blocks), and the mirror's first instruction dereferences obj. A collection in that write leaves the hook reading from-space. The function already has refresh_roots_after_alloc!() and calls it after every other allocating step; these two sites were missed. Scoped to these two arms deliberately. The macro republishes value from its handle as well as obj, so arms that intentionally rebind value locally must not refresh -- applying it to all eight mirror sites is wrong for that reason. Found via test_gap_gc_assign_string_source_rooting under #7341 quarantine. The fault moves out of the mirror entirely and into a separate catch in js_string_index_get, which stays open. Baselines verified rather than assumed: the two gap tests that fail alongside this also fail on pristine main, and perry-runtime --lib fails 3/5/3 across three runs of pristine main (#7365). --- .../7381-mirror-static-write-refresh.md | 24 +++++++++++++++++++ .../src/object/field_set_by_name.rs | 22 +++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 changelog.d/7381-mirror-static-write-refresh.md 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