Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog.d/9764-regex-site-cache-coherence.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Fixed the RegExp construction cache remembering an incoherent program pair. The
compiled-program caches are capped and cleared independently, so `FANCY_CACHE`
can drop a lookbehind/backreference pattern while `REGEX_CACHE` still answers
for it with the never-match placeholder. The maps heal on their next clear; a
memoized entry does not, because a construction that hits it is born built and
never consults the maps again — so a lookbehind literal could be born
permanently non-matching. The triple is now only remembered against the text
when it is coherent.
51 changes: 46 additions & 5 deletions crates/perry-runtime/src/regex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,24 @@ pub(crate) unsafe fn regex_header_finalize_for_gc(re: *mut RegExpHeader) {
regex_header_clear_dead_for_gc(re as usize);
}

/// Model one `evict_regex_cache_if_full` overflow clear of `FANCY_CACHE`.
///
/// The production guard's entire action on overflow is `cache.clear()` (see
/// [`evict_regex_cache_if_full`]) — this seam re-implements no predicate, only
/// spares a test 512 fancy compiles to reach the cap. What matters for the test
/// is that the two program caches have INDEPENDENT caps, so one can drop a
/// pattern the other still holds.
#[cfg(all(test, feature = "regex-engine"))]
pub(crate) fn test_clear_fancy_cache() {
FANCY_CACHE.with(|fc| fc.borrow_mut().clear());
}

/// The same, for `REGEX_CACHE` — the clear that lets an incoherent pair heal.
#[cfg(all(test, feature = "regex-engine"))]
pub(crate) fn test_clear_std_cache() {
REGEX_CACHE.with(|c| c.borrow_mut().clear());
}

#[cfg(test)]
pub(crate) fn test_regex_pointer_entry_exists(addr: usize) -> bool {
REGEX_POINTERS.with(|table| table.borrow().contains(&addr))
Expand Down Expand Up @@ -481,6 +499,29 @@ fn evict_regex_cache_if_full<V>(cache: &mut HashMap<(String, String), V>) {
}
}

/// The never-match program `REGEX_CACHE` holds for a pattern only `fancy-regex`
/// can match (lookbehind, backreferences), and for one both engines rejected.
///
/// It is a process-wide singleton so that "is this entry a placeholder?" is an
/// `Arc::ptr_eq`, not a pattern-text compare. That question has to be asked
/// before anything MEMOIZES a `(std, fancy, repeat)` triple: a placeholder with
/// no fancy twin is an incoherent triple, and memoizing it makes the pattern
/// permanently non-matching (see `lazy::build_and_install_programs`).
#[cfg(feature = "regex-engine")]
fn never_match_program() -> Arc<Regex> {
static NEVER: std::sync::OnceLock<Arc<Regex>> = std::sync::OnceLock::new();
NEVER
.get_or_init(|| Arc::new(Regex::new(r"[^\s\S]").unwrap()))
.clone()
}

/// Is `program` the shared never-match placeholder rather than a real compiled
/// program? See [`never_match_program`].
#[cfg(feature = "regex-engine")]
pub(crate) fn is_never_match_program(program: &Arc<Regex>) -> bool {
Arc::ptr_eq(program, &never_match_program())
}

/// Compile `(pattern, flags)` into the caches if absent, reporting whether
/// SOME engine accepted the flag-prefixed pattern. One NFA build total.
///
Expand Down Expand Up @@ -526,8 +567,8 @@ fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool {
// prefix the flags imply. Shared with `lazy::std_engine_syntax_ok` so the
// eager syntax check and this build can never inspect different strings.
let regex_pattern = lazy::flag_prefixed_pattern(pattern, flags);
let regex = match build_std_regex(&regex_pattern) {
Ok(re) => re,
let regex: Arc<Regex> = match build_std_regex(&regex_pattern) {
Ok(re) => Arc::new(re),
Err(_) => {
// Pattern has features regex crate doesn't support
// (lookbehind, lookahead). Try fancy-regex which supports
Expand All @@ -554,7 +595,7 @@ fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool {
if !fancy_ok {
return false;
}
Regex::new(r"[^\s\S]").unwrap()
never_match_program()
}
};
if crate::hot_diag::regex_on() {
Expand All @@ -563,7 +604,7 @@ fn compile_and_cache_regex_checked(pattern: &str, flags: &str) -> bool {
REGEX_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::new(regex));
cache.insert((pattern.to_string(), flags.to_string()), regex);
});
true
}
Expand All @@ -587,7 +628,7 @@ fn get_or_compile_regex(pattern: &str, flags: &str) -> Arc<Regex> {
}
// 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 = never_match_program();
evict_regex_cache_if_full(&mut cache);
cache.insert((pattern.to_string(), flags.to_string()), arc.clone());
arc
Expand Down
36 changes: 27 additions & 9 deletions crates/perry-runtime/src/regex/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,33 @@ fn build_and_install_programs(re: *const RegExpHeader) {
});
// 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(
&pattern,
&flags,
super::site_cache::Programs {
std: std_arc.clone(),
fancy: fancy_arc.clone(),
repeat: repeat_arc.clone(),
},
);
//
// ONLY a coherent triple may be memoized. `get_or_compile_regex` answers
// from `REGEX_CACHE`, and for a lookbehind / backreference pattern the
// entry it finds there is the never-match PLACEHOLDER whose real program
// lives in `FANCY_CACHE`. Those two maps have independent
// clear-on-overflow caps (`evict_regex_cache_if_full`), so `FANCY_CACHE`
// can have dropped the pattern while `REGEX_CACHE` still holds its
// placeholder — and then the pair read here is (never-match, None), which
// matches nothing.
//
// Left alone, the maps heal: the next `REGEX_CACHE` clear makes the
// pattern recompile and repopulate both. A site-cache entry never heals —
// a construction that hits it is born built and never consults the maps
// again — so memoizing the incoherent pair would make a lookbehind literal
// PERMANENTLY non-matching, silently, for the life of the thread. Declining
// to memoize costs one recompile and keeps the self-healing behaviour.
if !(super::is_never_match_program(&std_arc) && fancy_arc.is_none()) {
super::site_cache::install_programs(
&pattern,
&flags,
super::site_cache::Programs {
std: std_arc.clone(),
fancy: fancy_arc.clone(),
repeat: repeat_arc.clone(),
},
);
}
let regex_ptr = Arc::into_raw(std_arc) as *mut Regex;
let fancy_ptr: *const () =
fancy_arc.map_or(std::ptr::null(), |arc| Arc::into_raw(arc) as *const ());
Expand Down
56 changes: 56 additions & 0 deletions crates/perry-runtime/src/regex/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1649,6 +1649,62 @@ fn global_replace_substitutes_at_every_empty_match() {
assert_eq!(string_as_str(out), "[a][]");
}

/// #9764 + the regex lane's #9796 finding: the construction cache must never
/// memoize an INCOHERENT program triple.
///
/// `REGEX_CACHE` holds a never-match placeholder for a lookbehind /
/// backreference pattern whose real program lives in `FANCY_CACHE`, and the two
/// maps have independent clear-on-overflow caps. So `FANCY_CACHE` can drop a
/// pattern while `REGEX_CACHE` still answers for it, and a build then reads
/// (never-match, no fancy) — a pair that matches nothing.
///
/// The maps heal on their next clear. A site-cache entry does not: a
/// construction that hits it is born built and never consults the maps again.
/// Memoizing the incoherent pair therefore makes the literal PERMANENTLY
/// non-matching. Without the guard in `build_and_install_programs` the first
/// assertion below reads `Some(true)` and the last one reads `-1`.
#[test]
fn an_incoherent_program_pair_is_never_memoized_by_the_site_cache() {
let _lock = crate::gc::global_side_table_test_lock();
// A fixed-width lookbehind: the linear engine rejects it, so the standard
// cache gets the placeholder and the real program goes to `FANCY_CACHE`.
let pat = r"(?<=Q9zq)zz";
site_cache::test_reset();
test_clear_std_cache();
test_clear_fancy_cache();

let re1 = js_regexp_new(make_string(pat), make_string(""));
assert_eq!(
js_string_search_regex(make_string("aQ9zqzz"), re1),
5,
"the fancy fallback matches on a coherent runtime"
);

// `FANCY_CACHE` overflows and clears; `REGEX_CACHE` keeps the placeholder.
// Drop the site entry too, so the next construction takes the build path.
site_cache::test_reset();
test_clear_fancy_cache();

let re2 = js_regexp_new(make_string(pat), make_string(""));
let _ = js_string_search_regex(make_string("aQ9zqzz"), re2);
assert_eq!(
site_cache::test_has_programs(pat, ""),
Some(false),
"a (never-match, no-fancy) pair must not be remembered against the text"
);

// The maps heal — and because nothing incoherent was memoized, so does the
// pattern. With the pair memoized this construction is born built from it
// and returns -1 forever.
test_clear_std_cache();
let re3 = js_regexp_new(make_string(pat), make_string(""));
assert_eq!(
js_string_search_regex(make_string("aQ9zqzz"), re3),
5,
"the literal must recover once the program caches heal"
);
}

/// The construction cache (`regex::site_cache`): once a header built from
/// some `(pattern, flags)` has been executed, the next construction of the
/// same text is born built — it shares the executed header's program and
Expand Down
Loading