From 1d5c4c22f772b5fd04471ec8a92fa388c41b1a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 17:33:02 +0200 Subject: [PATCH] fix(gc): refresh obj at the remaining six class-static mirror sites Completes #7381, which fixed two of eight and scoped itself there on the theory that refresh_roots_after_alloc!() -- which republishes obj, key, value and interned_key together -- could clobber an arm that rebinds value locally. That theory was wrong. None of the eight arms rebinds any of the four after its handle is taken, so republishing is a no-op except for the relocation it repairs. Measured rather than inspected: the full-coverage build scores 58 pass / 2 fail on the object/assign/class/field/shape gap set, byte-identical to pristine main, with both failures pre-existing. With all eight refreshed the fault leaves mirror_class_object_static_write entirely and surfaces the next catch in the chain at js_jsvalue_equals. --- changelog.d/7382-mirror-remaining-sites.md | 16 +++++++++ .../src/object/field_set_by_name.rs | 36 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 changelog.d/7382-mirror-remaining-sites.md diff --git a/changelog.d/7382-mirror-remaining-sites.md b/changelog.d/7382-mirror-remaining-sites.md new file mode 100644 index 0000000000..a1e1058adb --- /dev/null +++ b/changelog.d/7382-mirror-remaining-sites.md @@ -0,0 +1,16 @@ +**Fixed** the remaining six `mirror_class_object_static_write` call sites in +`js_object_set_field_by_name` passed a stale `obj`, completing #7381. + +#7381 fixed two of the eight sites and scoped itself there on the theory that +the `refresh_roots_after_alloc!()` macro — which republishes `obj`, `key`, +`value` and `interned_key` together — could clobber an arm that rebinds `value` +locally. That theory was wrong: none of the eight arms rebinds any of the four +after its handle is taken, so republishing is a no-op except for the relocation +it repairs. Verified by measurement, not inspection — the full-coverage build +scores 58 pass / 2 fail on the object/assign/class/field/shape gap set, byte-identical +to pristine `main` (both failures pre-existing, one already in +`known_failures.json`). + +With all eight refreshed, `test_gap_gc_assign_string_source_rooting`'s fault +leaves `mirror_class_object_static_write` entirely and surfaces the next catch in +the chain at `js_jsvalue_equals`, which remains open. 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 e8f068b9b4..900a26358d 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -1574,6 +1574,12 @@ pub extern "C" fn js_object_set_field_by_name( }; overflow_set(obj as usize, i, vbits); } + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); return; } @@ -1644,6 +1650,12 @@ pub extern "C" fn js_object_set_field_by_name( super::shapes::shape_keys_grown(prev_keys_usize, new_keys); } overflow_set(obj as usize, new_index, vbits); + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); transition_cache_insert( prev_keys_usize, @@ -1685,6 +1697,12 @@ pub extern "C" fn js_object_set_field_by_name( (*obj).field_count = new_index as u32 + 1; } js_object_set_field(obj, new_index as u32, JSValue::from_bits(value.to_bits())); + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); transition_cache_insert( prev_keys_usize, @@ -1755,6 +1773,12 @@ pub extern "C" fn js_object_set_field_by_name( }; overflow_set(obj as usize, i, vbits); } + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); return; } @@ -1843,6 +1867,12 @@ pub extern "C" fn js_object_set_field_by_name( super::shapes::shape_keys_grown(prev_keys_usize, new_keys); } overflow_set(obj as usize, new_index, vbits); + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); // Record the shape transition so the next object sharing // `prev_keys` that adds the same key hits the fast path. @@ -1886,6 +1916,12 @@ pub extern "C" fn js_object_set_field_by_name( (*obj).field_count = new_index as u32 + 1; } js_object_set_field(obj, new_index as u32, JSValue::from_bits(value.to_bits())); + // #7341: same as the two own-data-write arms -- the write above can + // reach an allocator and the mirror's first instruction dereferences + // `obj`. None of these arms rebinds `obj`/`key`/`value` after the + // handles are taken, so republishing all four is a no-op except for + // the relocation it repairs. + refresh_roots_after_alloc!(); mirror_class_object_static_write(obj, key, value); // Record the shape transition — see above for semantics. transition_cache_insert(