From 1fc365ab5f98a98a13e5924ec15399b37063b391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 4 Aug 2026 14:58:56 +0200 Subject: [PATCH] fix(gc): a RegExp's flags string was stored into the header already stale js_regexp_new allocates the canonical flags string, then calls gc_malloc for the header -- a collection point -- and then writes the PRE-collection flags pointer into flags_ptr. An evacuating minor inside that gc_malloc relocates the string, so a live RegExpHeader holds a retired from-space address permanently, and every later string_as_str((*re).flags_ptr) reads it. The pattern string was already rooted and re-read across exactly this allocation, with a star-marked comment explaining the hazard. The flags string, created after that comment and stored beside it, was not. Root cause of the lookup_fancy_regex cluster in #7341: 5 catches, four different callers, all reaching that one read. Two wrong fixes are recorded in the changelog because both looked obvious and both measured zero: a no-move window inside lookup_fancy_regex (0/5), and rooting the search-value operand in codegen's replace lowering (0/5, even after moving the re-read to the point of use). The helper and the call site were both innocent; the header had been carrying a dead pointer since construction. 4/5 cluster tests clean, 6-line reproducer 6/6 -> 0/6. --- changelog.d/7374-regexp-flags-stale-store.md | 20 ++++++++++++++++++++ crates/perry-runtime/src/regex.rs | 15 +++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 changelog.d/7374-regexp-flags-stale-store.md diff --git a/changelog.d/7374-regexp-flags-stale-store.md b/changelog.d/7374-regexp-flags-stale-store.md new file mode 100644 index 0000000000..c38ce3346c --- /dev/null +++ b/changelog.d/7374-regexp-flags-stale-store.md @@ -0,0 +1,20 @@ +### Fixed + +- **A `RegExp`'s `flags` string could be stored already-stale, permanently.** + `js_regexp_new` allocates the canonical flags string, then calls `gc_malloc` + for the header — a collection point — and then writes the *pre-collection* + flags pointer into `flags_ptr`. An evacuating minor inside that `gc_malloc` + moves the string, so a live `RegExpHeader` ends up holding a retired + from-space address for the rest of its life. + + The pattern string was already rooted and re-read across exactly this + allocation, with a comment explaining why; the flags string was not. Now both + are. + + This is the root cause of the `lookup_fancy_regex` cluster in #7341 — 5 + catches reaching one read (`string_as_str((*re).flags_ptr)`) from four + different callers. Worth recording what it was *not*: a no-move window inside + `lookup_fancy_regex` closes 0/5, and rooting the search-value operand in + codegen closes 0/5. The helper and the call site were both innocent. + + 4 of the 5 cluster tests are now clean and a 6-line reproducer goes 6/6 → 0/6. diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index fdc4e99f94..0d4ba4856a 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -774,6 +774,19 @@ pub extern "C" fn js_regexp_new( // GC-survivable source table all agree on the canonical form, and the // header never holds the caller's possibly-temporary input flags. let canonical_flags_ptr = js_string_from_str(flags_str); + // ★ #7341: root the canonical flags string too. The `gc_malloc` below is an + // allocation and therefore a collection point, exactly as the comment above + // `pattern_root` says — but only the PATTERN was rooted and re-read. The + // flags string is created here and stored into the header AFTER that + // allocation, so an evacuating minor in `gc_malloc` moved it and the header + // kept the pre-collection address. `flags_ptr` is then permanently stale in + // a live header: `lookup_fancy_regex` reads it through `string_as_str` and + // faults on retired from-space, which is 5 of the 31 catches in #7341 + // (four different callers, all reaching that one read). + // + // The write barrier below already treated this as a real GC edge; what was + // missing is that the value written had to survive the allocation first. + let flags_root = scope.root_string_ptr(canonical_flags_ptr); unsafe { let raw = crate::gc::gc_malloc(header_size, crate::gc::GC_TYPE_OBJECT); if raw.is_null() { @@ -789,6 +802,8 @@ pub extern "C" fn js_regexp_new( // so the incoming argument may name from-space; the handle is a mutable // root the collector rewrote. let pattern = pattern_root.get_raw_const_ptr::(); + // #7341: same re-read for the flags, for the same reason. + let canonical_flags_ptr = flags_root.get_raw_const_ptr::(); (*ptr).regex_ptr = regex_ptr; (*ptr).pattern_ptr = pattern;