diff --git a/changelog.d/9801-regex-program-cache-coherence.md b/changelog.d/9801-regex-program-cache-coherence.md new file mode 100644 index 0000000000..8e8162fea4 --- /dev/null +++ b/changelog.d/9801-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..316e61abd9 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1757,3 +1757,74 @@ 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 `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(); + 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" + ); +}