From 757f15a99415f1e695264ecb21d208c9d8290a75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 10:20:07 +0200 Subject: [PATCH 1/3] fix(regex): a built header must carry every program its pattern needs The three compiled-program caches cap independently and clear wholesale, and compile_and_cache_regex_checked returns early on a REGEX_CACHE hit, so a pattern whose real program lives in FANCY_CACHE (its REGEX_CACHE entry being the never-match placeholder) lost that program permanently once FANCY_CACHE overflowed while the placeholder survived. lookup_fancy_regex treats a built header as authoritative and site_cache::install_programs memoizes the triple against the pattern text, so the consequence is not one bad header but every later construction of that literal. Repair the missing program before publishing and before memoizing. The same shape applies to REPEAT_MATCHER_CACHE, where the wrong answer is the linear engine's capture assignment instead of ECMA-262's. --- changelog.d/regex-program-cache-coherence.md | 33 ++++++++++ crates/perry-runtime/src/regex.rs | 14 +++- crates/perry-runtime/src/regex/lazy.rs | 52 +++++++++++++++ crates/perry-runtime/src/regex/tests.rs | 68 ++++++++++++++++++++ 4 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 changelog.d/regex-program-cache-coherence.md diff --git a/changelog.d/regex-program-cache-coherence.md b/changelog.d/regex-program-cache-coherence.md new file mode 100644 index 0000000000..8e8162fea4 --- /dev/null +++ b/changelog.d/regex-program-cache-coherence.md @@ -0,0 +1,33 @@ +### Bug Fixes + +- **A lookbehind or backreference literal could stop matching for good.** + The three compiled-program caches are capped independently and each + `clear()`s wholesale on overflow, and `compile_and_cache_regex_checked` + returns early whenever `REGEX_CACHE` already holds the pattern — so it never + re-runs the fancy-regex or repeat-matcher build. For a pattern only + `fancy-regex` accepts, the `REGEX_CACHE` entry is the never-match + placeholder and the real program is the one in `FANCY_CACHE`; once + `FANCY_CACHE` reached its 512-entry cap and cleared while that placeholder + survived, `get_or_compile_regex` handed back a program matching nothing and + nothing rebuilt the fallback. + + Since `lookup_fancy_regex` treats a built header as authoritative (a null + `fancy_ptr` beside a non-null `regex_ptr` IS the answer) and + `site_cache::install_programs` memoizes that triple against the pattern + text, this is not one bad header: every later construction of the same + literal is born with it, until the site-cache entry is evicted. The same + shape applies to `REPEAT_MATCHER_CACHE`, where the wrong answer is quieter — + the linear engine's capture assignment instead of ECMA-262's RepeatMatcher + semantics. + + `lazy::build_and_install_programs` now repairs a missing program before + publishing the header and before memoizing the triple: if the standard + program is the never-match placeholder and no fancy program came back, it + rebuilds the fancy one; if no repeat matcher came back, it re-derives it + (`repeat_matcher::compile` is a byte scan that returns immediately unless a + capture group sits under a quantifier, so it is free for the patterns that + do not need it). A built header therefore always carries every program its + pattern needs, which is exactly the invariant the header-authoritative + lookups and the construction cache depend on. + + (`a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal`) diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 714a4f81c6..2b368fadac 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -457,6 +457,16 @@ pub(crate) fn build_fancy_regex(pattern: &str) -> Result bool { if !fancy_ok { return false; } - Regex::new(r"[^\s\S]").unwrap() + Regex::new(NEVER_MATCH_PATTERN).unwrap() } }; if crate::hot_diag::regex_on() { @@ -587,7 +597,7 @@ fn get_or_compile_regex(pattern: &str, flags: &str) -> Arc { } // Both engines rejected it (validation normally throws before this // point) — keep the historical behavior: cache + return never-match. - let arc = Arc::new(Regex::new(r"[^\s\S]").unwrap()); + let arc = Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap()); evict_regex_cache_if_full(&mut cache); cache.insert((pattern.to_string(), flags.to_string()), arc.clone()); arc diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index 438d8eca4b..236c7a80f4 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -255,6 +255,58 @@ fn build_and_install_programs(re: *const RegExpHeader) { .get(&(pattern.to_string(), flags.to_string())) .cloned() }); + // ── Repair before publishing ────────────────────────────────────────── + // + // A built header is treated as AUTHORITATIVE — `lookup_fancy_regex` / + // `lookup_repeat_matcher` read a null slot beside a non-null `regex_ptr` + // as "this pattern has no such program" — and `install_programs` below + // memoizes the triple against the pattern text, so whatever is assembled + // here becomes the answer for every later construction of the same + // literal. It therefore has to be complete, and the probes above cannot + // guarantee that on their own: the three caches are capped independently + // and each `clear()`s wholesale, while + // `compile_and_cache_regex_checked` returns early whenever `REGEX_CACHE` + // already holds the pattern — so it never re-runs the fancy or + // repeat-matcher build for a pattern whose `REGEX_CACHE` entry survived a + // clear of one of the others. + // + // Both gaps are silent WRONG ANSWERS, not slowdowns: a lookbehind literal + // whose fancy program is missing matches nothing at all, and a + // quantified-capture literal whose repeat matcher is missing reports the + // linear engine's capture assignment instead of ECMA-262's. Re-derive + // what is missing. Each check costs nothing when the caches are coherent, + // which is the normal case. + let mut fancy_arc = fancy_arc; + if fancy_arc.is_none() && std_arc.as_str() == super::NEVER_MATCH_PATTERN { + // The standard program matches nothing, so this pattern is only + // usable through the fancy engine — and it is not there. + let flag_prefixed = flag_prefixed_pattern(&pattern, &flags); + if let Ok(fre) = super::build_fancy_regex(&flag_prefixed) { + let arc = Arc::new(fre); + FANCY_CACHE.with(|fc| { + let mut fc = fc.borrow_mut(); + evict_regex_cache_if_full(&mut fc); + fc.insert((pattern.to_string(), flags.to_string()), arc.clone()); + }); + fancy_arc = Some(arc); + } + } + let mut repeat_arc = repeat_arc; + if repeat_arc.is_none() { + // `compile` is a byte scan that returns `None` immediately unless a + // capture group sits under a quantifier (or inside a negative + // lookaround), so this is free for the patterns that do not need it. + if let Some(matcher) = super::repeat_matcher::compile(&pattern, &flags) { + let arc = Arc::new(matcher); + REPEAT_MATCHER_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert((pattern.to_string(), flags.to_string()), arc.clone()); + }); + repeat_arc = Some(arc); + } + } + // Remember the built programs against the pattern text, so the next // construction of the same literal is born built (`js_regexp_new`). super::site_cache::install_programs( diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index e41c2abc61..82b53d5395 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1757,3 +1757,71 @@ fn global_test_advances_and_resets_last_index() { assert_eq!(js_regexp_get_last_index(repeat), 5.0); assert_eq!(js_regexp_test(repeat, r), 0); } + +/// A capacity event in one compiled-program cache must not leave a pattern +/// whose real program lives in ANOTHER of them permanently non-matching. +/// +/// `compile_and_cache_regex_checked` returns early when `REGEX_CACHE` already +/// holds the pattern, so it never re-runs the fancy build; for a lookbehind +/// pattern that `REGEX_CACHE` entry is the never-match placeholder and the +/// real program is the one in `FANCY_CACHE`. Clear `FANCY_CACHE` on its own — +/// which is exactly what its independent 512-entry overflow used to do — and +/// `get_or_compile_regex` hands back a program that matches nothing while +/// nothing rebuilds the fallback. Since `lookup_fancy_regex` now treats a +/// built header as authoritative and `site_cache::install_programs` memoizes +/// the triple against the pattern text, that is not one bad header: every +/// later construction of the same literal is born with it. +/// +/// The fix is that the three program caches clear as a group, so +/// "`REGEX_CACHE` still has this pattern" implies the other two have not been +/// cleared since it was compiled. +#[test] +fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { + let _lock = crate::gc::global_side_table_test_lock(); + let source = "(?<=foo)bar"; + let scope = crate::gc::RuntimeHandleScope::new(); + site_cache::test_reset(); + + let build = || { + let pattern = scope.root_string_ptr(make_string(source)); + let flags = scope.root_string_ptr(make_string("")); + pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }) + }; + let subject = scope.root_string_ptr(make_string("foobar")); + + let warm = build(); + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(warm, s)), + 1, + "a lookbehind pattern must match through the fancy fallback" + ); + + // The state a `FANCY_CACHE` overflow produces: its programs are gone, the + // never-match placeholder for this pattern survives in `REGEX_CACHE`. + FANCY_CACHE.with(|fc| fc.borrow_mut().clear()); + assert!( + REGEX_CACHE.with(|c| c.borrow().contains_key(&(source.to_string(), String::new()))), + "the placeholder must survive, or this test exercises nothing" + ); + // A fresh literal site, so the construction cache cannot answer from the + // programs the first header built. + site_cache::test_reset(); + + let cold = build(); + unsafe { + lazy::ensure_regex_compiled(cold); + assert!( + !(*cold).fancy_ptr.is_null(), + "a built header must carry every program its pattern needs — a null \ + fancy_ptr here is memoized by site_cache::install_programs and makes \ + the breakage permanent for this literal" + ); + } + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(cold, s)), + 1, + "the literal must still match after an unrelated cache reached capacity" + ); +} From 7844de8071a698c7e894ffe638d4b16b39cae7d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 13:37:59 +0200 Subject: [PATCH 2/3] docs(regex): correct the coherence test's doc comment to the fix that shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment described the group-clear approach that was built first and dropped — it closes the route into the incoherent state but cannot repair a header already in it, which is why the test still failed against it. The fix that shipped repairs the header in `build_and_install_programs` before publishing it and before `site_cache::install_programs` memoizes the triple. Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2 --- crates/perry-runtime/src/regex/tests.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index 82b53d5395..316e61abd9 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1772,9 +1772,12 @@ fn global_test_advances_and_resets_last_index() { /// the triple against the pattern text, that is not one bad header: every /// later construction of the same literal is born with it. /// -/// The fix is that the three program caches clear as a group, so -/// "`REGEX_CACHE` still has this pattern" implies the other two have not been -/// cleared since it was compiled. +/// The fix is that `lazy::build_and_install_programs` REPAIRS the header +/// before publishing it and before memoizing the triple: a standard program +/// that is the never-match placeholder with no fancy program beside it means +/// the fancy program is missing, so it is rebuilt. (Clearing the three caches +/// as a group was built first and dropped: it closes the route into the bad +/// state but cannot repair a header already in it, so this test still failed.) #[test] fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { let _lock = crate::gc::global_side_table_test_lock(); From c63228592f5b4a93d8b007523fca8f0df107c996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 5 Sep 2026 16:29:18 +0200 Subject: [PATCH 3/3] docs(changelog): key the regex coherence fragment to PR 9801 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit changelog.d/README.md asks for `-.md`; the fragment landed unnumbered. Rename only — the entry text and the code are unchanged. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m --- ...m-cache-coherence.md => 9801-regex-program-cache-coherence.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{regex-program-cache-coherence.md => 9801-regex-program-cache-coherence.md} (100%) diff --git a/changelog.d/regex-program-cache-coherence.md b/changelog.d/9801-regex-program-cache-coherence.md similarity index 100% rename from changelog.d/regex-program-cache-coherence.md rename to changelog.d/9801-regex-program-cache-coherence.md