From d8daa4fd42e02bc1c8315abd6a1572255606271b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:15:49 +0200 Subject: [PATCH 1/8] perf(regex): remove the traced-source side table --- .../gc/tests/copying/survival_and_malloc.rs | 6 - crates/perry-runtime/src/hot_diag.rs | 26 +++- crates/perry-runtime/src/regex.rs | 147 +++++++----------- crates/perry-runtime/src/regex/compile.rs | 21 ++- crates/perry-runtime/src/regex/escape.rs | 79 +++++++++- crates/perry-runtime/src/regex/lazy.rs | 16 +- crates/perry-runtime/src/regex/program_key.rs | 5 +- crates/perry-runtime/src/regex/site_cache.rs | 9 +- crates/perry-runtime/src/regex/tests_part2.rs | 70 +++++++-- 9 files changed, 232 insertions(+), 147 deletions(-) diff --git a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs index aed0b6c681..5765446f41 100644 --- a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs +++ b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs @@ -924,7 +924,6 @@ fn test_movable_regexp_evacuation_migrates_all_address_owned_state() { let old_addr = re as usize; assert!(crate::arena::pointer_in_nursery(old_addr)); assert!(crate::regex::test_regex_pointer_entry_exists(old_addr)); - assert!(crate::regex::test_regex_source_entry_exists(old_addr)); crate::object::exotic_expando::test_seed_exotic_expando_entry( old_addr, @@ -942,8 +941,6 @@ fn test_movable_regexp_evacuation_migrates_all_address_owned_state() { assert!(crate::regex::test_regex_pointer_entry_exists(new_addr)); assert!(!crate::regex::test_regex_pointer_entry_exists(old_addr)); - assert!(crate::regex::test_regex_source_entry_exists(new_addr)); - assert!(!crate::regex::test_regex_source_entry_exists(old_addr)); assert!(crate::object::exotic_expando::test_exotic_expando_entry_exists(new_addr)); assert!(!crate::object::exotic_expando::test_exotic_expando_entry_exists(old_addr)); @@ -1037,7 +1034,6 @@ fn nursery_regexp_that_dies_young_is_finalized_by_the_copied_minor() { "the header must be nursery-allocated" ); assert!(crate::regex::test_regex_pointer_entry_exists(dead_addr)); - assert!(crate::regex::test_regex_source_entry_exists(dead_addr)); // Both headers share one program through the site cache. let count_before = crate::regex::test_regexp_std_program_strong_count(live); assert!(count_before >= 2); @@ -1051,13 +1047,11 @@ fn nursery_regexp_that_dies_young_is_finalized_by_the_copied_minor() { assert_ne!(live_new, live_addr, "the rooted RegExp must be evacuated"); assert!(crate::regex::regex_header_has_magic(live_new as *const _)); assert!(crate::regex::test_regex_pointer_entry_exists(live_new)); - assert!(crate::regex::test_regex_source_entry_exists(live_new)); assert!( !crate::regex::test_regex_pointer_entry_exists(dead_addr), "a nursery RegExp that died must be removed from REGEX_POINTERS by the copied minor" ); - assert!(!crate::regex::test_regex_source_entry_exists(dead_addr)); assert_eq!( crate::regex::test_regexp_std_program_strong_count(live_new as *const _), count_before - 1, diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 51615783ca..c397b88370 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -169,11 +169,21 @@ pub struct RegexDiag { /// or missed: this is the `memcmp` volume alone, which is what a 12 KB /// emoji pattern makes expensive and a 60-byte one does not. pub new_site_verify_bytes: u64, - /// Address-keyed side-table inserts performed per construction - /// (`REGEX_POINTERS` and `REGEX_SOURCE_TABLE`) — two per header, each a - /// `PtrHasher` hash plus a hashbrown insert, mirrored by two removals at - /// death and two rekeys per evacuation. + /// Address-keyed side-table inserts performed per construction. This was + /// two (`REGEX_POINTERS` plus the source table) before the header's string + /// slots became traced edges; only `REGEX_POINTERS` remains. pub new_side_table_inserts: u64, + /// Split of the above by table. The source counters are retained as zeroed + /// before/after controls for the #9908 measurement; `REGEX_POINTERS` is + /// still the registry the copied-minor finaliser enumerates. + pub pointer_table_inserts: u64, + pub source_table_inserts: u64, + /// The death side. `source_table_removals` is the zeroed after-control; + /// `regex_header_clear_dead_for_gc` now removes only `REGEX_POINTERS`. + pub pointer_table_removals: u64, + pub source_table_removals: u64, + /// Evacuation rekeys of the remaining pointer registry. + pub side_table_rekeys: u64, /// Constructions answered from the LITERAL-SITE table — identity by the /// compiler-emitted site global's address, so neither the pattern's /// fingerprint nor its byte compare ran. `site_hit` counts the @@ -326,7 +336,8 @@ impl RegexDiag { match={} replace={} replace_matches={} split={} flags_alloc={} \ desc_regexp_probes={} desc_regexp_meta_negative={} \ barrier_taken={} barrier_gated={} header_bytes={} site_verify_bytes={} \ - side_table_inserts={} site_key_hit={}", + side_table_inserts={} site_key_hit={} ptr_ins={} src_ins={} \ + ptr_rm={} src_rm={} rekeys={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -356,6 +367,11 @@ impl RegexDiag { self.new_site_verify_bytes, self.new_side_table_inserts, self.new_site_key_hit, + self.pointer_table_inserts, + self.source_table_inserts, + self.pointer_table_removals, + self.source_table_removals, + self.side_table_rekeys, ); // Merge by content (prefix, len, flags): distinct literal sites with // the same pattern are one row. diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 2a7c5461e0..5d8aa2db2f 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -145,21 +145,6 @@ crate::perry_thread_local! { /// relocate or die. Header magic remains the primary identity check. static REGEX_POINTERS: RefCell> = RefCell::new(crate::fast_hash::new_ptr_hash_set()); - /// Issue #637: Owned copies of pattern and flags strings keyed by - /// the RegExpHeader pointer. The header's `pattern_ptr` / `flags_ptr` - /// fields hold raw `*const StringHeader` pointers to the input - /// strings — when those inputs are temporaries (e.g. the result of - /// a template-literal expression `\`^${p}\``), the GC frees them - /// after the function call returns and subsequent `.source` / - /// `.flags` reads dereference dangling memory. We side-table an - /// owned `String` copy at construction time; readers prefer this - /// over `pattern_ptr` whenever an entry exists. - /// - /// The copies are `Arc` shared with `regex::site_cache`: every - /// header built from the same literal text bumps two refcounts instead - /// of copying the pattern (12 KB for emoji-class patterns, once per - /// evaluation of the literal). - static REGEX_SOURCE_TABLE: RefCell, Arc)>> = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } /// Check whether `ptr` is a RegExpHeader pointer that was allocated in @@ -206,29 +191,33 @@ pub(crate) fn regex_header_moved_for_gc(old_addr: usize, new_addr: usize) { if old_addr == new_addr { return; } + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_counters(|d| d.side_table_rekeys += 1); + } REGEX_POINTERS.with(|table| { let mut table = table.borrow_mut(); if table.remove(&old_addr) { table.insert(new_addr); } }); - REGEX_SOURCE_TABLE.with(|table| { - let mut table = table.borrow_mut(); - if let Some(source) = table.remove(&old_addr) { - table.insert(new_addr, source); - } - }); crate::object::exotic_expando::exotic_expando_owner_moved(old_addr, new_addr); } /// Remove address-owned RegExp metadata when the cell is proven dead. pub(crate) fn regex_header_clear_dead_for_gc(addr: usize) { + // Counted, not timed: this runs inside a collection, so a probe here must + // allocate nothing and must not dump. `regex_counters` does neither, and + // `regex_on`'s one-time env read cannot first happen here — a header can + // only die after `js_regexp_new` created it, and that path arms the + // instrument first. + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_counters(|d| { + d.pointer_table_removals += 1; + }); + } REGEX_POINTERS.with(|table| { table.borrow_mut().remove(&addr); }); - REGEX_SOURCE_TABLE.with(|table| { - table.borrow_mut().remove(&addr); - }); crate::object::exotic_expando::exotic_expando_owner_clear_dead(addr); } @@ -271,7 +260,7 @@ pub(crate) unsafe fn regex_header_finalize_for_gc(re: *mut RegExpHeader) { /// /// The copying minor's from-space flip runs no per-object finalize hooks, so /// a nursery header that was neither evacuated nor pinned would otherwise keep -/// its `Arc` programs and its `REGEX_POINTERS` / `REGEX_SOURCE_TABLE` / expando +/// its `Arc` programs and its `REGEX_POINTERS` / expando /// entries forever. Same shape as `map::finalize_dead_copied_minor_from_space_maps`: /// walk the registry after the flip, collect the provably-dead addresses, then /// finalize each (the finalizer removes its own registry entries, which is why @@ -386,11 +375,6 @@ pub(crate) fn test_regex_pointer_entry_exists(addr: usize) -> bool { REGEX_POINTERS.with(|table| table.borrow().contains(&addr)) } -#[cfg(test)] -pub(crate) fn test_regex_source_entry_exists(addr: usize) -> bool { - REGEX_SOURCE_TABLE.with(|table| table.borrow().contains_key(&addr)) -} - /// Build a minimal nursery-resident RegExp payload for the copying collector's /// relocation contract test. Production construction currently chooses the /// malloc-backed arm of `ArenaOrMalloc`; this exercises the same registered GC @@ -398,6 +382,9 @@ pub(crate) fn test_regex_source_entry_exists(addr: usize) -> bool { /// strand the address-owned tables. #[cfg(all(test, feature = "regex-engine"))] pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> *mut RegExpHeader { + let scope = crate::gc::RuntimeHandleScope::new(); + let pattern = scope.root_string_ptr(js_string_from_str(source)); + let flags_string = scope.root_string_ptr(js_string_from_str(flags)); unsafe { let ptr = crate::arena::arena_alloc_gc( std::mem::size_of::(), @@ -408,8 +395,8 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * // must be set explicitly or the GC follows a garbage pointer. (*ptr).meta = std::ptr::null_mut(); (*ptr).regex_ptr = std::ptr::null_mut(); - (*ptr).pattern_ptr = std::ptr::null(); - (*ptr).flags_ptr = std::ptr::null(); + (*ptr).pattern_ptr = pattern.get_raw_const_ptr::(); + (*ptr).flags_ptr = flags_string.get_raw_const_ptr::(); (*ptr).case_insensitive = flags.contains('i'); (*ptr).global = flags.contains('g'); (*ptr).multiline = flags.contains('m'); @@ -426,11 +413,6 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * REGEX_POINTERS.with(|table| { table.borrow_mut().insert(ptr as usize); }); - REGEX_SOURCE_TABLE.with(|table| { - table - .borrow_mut() - .insert(ptr as usize, (Arc::from(source), Arc::from(flags))); - }); ptr } } @@ -811,7 +793,7 @@ fn js_regexp_new_impl( // A `site_key` of 0 (every dynamic construction, and every runtime caller) // misses by construction and takes the content-keyed path below unchanged. let site_entry = site_key::lookup(site_key, raw_flags_str); - let (owned_pattern, owned_flags, programs, bits, shared_flags_root) = match site_entry { + let (programs, bits, shared_flags_root) = match site_entry { Some(hit) => { // The site's own flags literal, so this is the same sharing // decision the first construction at this site made (#9819). @@ -845,13 +827,7 @@ fn js_regexp_new_impl( picked } }; - ( - hit.pattern, - hit.flags, - programs, - hit.bits, - shared_flags_root, - ) + (programs, hit.bits, shared_flags_root) } None => { let pattern_str = if is_valid_ptr(pattern) { @@ -1038,10 +1014,10 @@ fn js_regexp_new_impl( // ★ Last use of the borrowed pattern text before this function allocates. // `pattern_str` borrows the GC string; the two allocations below can move - // it, and everything after this point reads the pattern from `owned_pattern` - // (a shared `Arc`, which relocation cannot invalidate) or from - // `pattern_root` (a runtime handle the collector rewrites). Nothing below - // may use `pattern_str` or the incoming `pattern` argument again. + // it. The site/content cache snapshots it into `owned_pattern`, and + // the header store below re-reads it from `pattern_root` (a runtime + // handle the collector rewrites). Nothing below may use `pattern_str` + // or the incoming `pattern` argument again. let (owned_pattern, owned_flags, programs) = match site_hit { Some(hit) => (hit.pattern, hit.flags, hit.programs), None => { @@ -1069,19 +1045,13 @@ fn js_regexp_new_impl( site_key::record( site_key, raw_flags_owned, - owned_pattern.clone(), - owned_flags.clone(), + owned_pattern, + owned_flags, flags_are_canonical, bits, programs.clone(), ); - ( - owned_pattern, - owned_flags, - programs, - bits, - shared_flags_root, - ) + (programs, bits, shared_flags_root) } }; let site_key::FlagBits { @@ -1110,7 +1080,7 @@ fn js_regexp_new_impl( // old-generation prices to do it. // // `GC_TYPE_REGEXP` has been movable (`GcMoveHookKind::RegExpSideTables` - // rekeys `REGEX_POINTERS`, `REGEX_SOURCE_TABLE` and the expando owner + // rekeys `REGEX_POINTERS` and the expando owner // after evacuation; `GcLayoutSlotKind::RegExpFields` traces the two string // edges and `meta`) since the copying collector landed, and // `test_movable_regexp_evacuation_migrates_all_address_owned_state` has @@ -1125,8 +1095,8 @@ fn js_regexp_new_impl( // old-generation sweep's ordinary `gc_type_finalize_unmarked_payload`. let header_size = std::mem::size_of::(); // `flags_ptr` must hold the CANONICAL form, so that `flags_ptr`-keyed - // lookups (FANCY_CACHE, lookup_fancy_regex) and the GC-survivable source - // table all agree. When the caller's string already is that text it is + // lookups (FANCY_CACHE, lookup_fancy_regex) agree. When the caller's + // string already is that text it is // shared (rooted above); only a non-canonical spelling (`/x/ig` → `"gi"`, // or a computed `new RegExp(p, f)`) still has to materialize one. The // counter makes the removal provable rather than asserted. @@ -1292,21 +1262,16 @@ fn js_regexp_new_impl( s.borrow_mut().insert(ptr as usize); }); if crate::hot_diag::regex_on() { - // Two address-keyed inserts per construction (this one and - // `REGEX_SOURCE_TABLE` below), each a `PtrHasher` hash plus a - // hashbrown insert, mirrored by two removals at death and two - // rekeys per evacuation. Counted so the pair is a number rather - // than a reading of the profile. - crate::hot_diag::regex_counters(|d| d.new_side_table_inserts += 2); + // One address-keyed insert per construction. `REGEX_POINTERS` + // remains because the copied-minor finaliser enumerates it; the + // former source table became redundant when #9845 made the + // header's two string slots traced GC edges. + crate::hot_diag::regex_counters(|d| { + d.new_side_table_inserts += 1; + d.pointer_table_inserts += 1; + }); } - // Issue #637: side-table owned copies of pattern + flags so - // `.source` / `.flags` survive GC of the input StringHeaders. - REGEX_SOURCE_TABLE.with(|t| { - t.borrow_mut() - .insert(ptr as usize, (owned_pattern, owned_flags)); - }); - ptr } } @@ -1335,10 +1300,18 @@ pub extern "C" fn js_regexp_construct(pattern: f64, flags: f64) -> *mut RegExpHe let (source_string, inherited_flags) = if pattern_is_regex { let re = pv.as_pointer::(); - let entry = REGEX_SOURCE_TABLE.with(|t| t.borrow().get(&(re as usize)).cloned()); - match entry { - Some((pat, fl)) => (pat.to_string(), Some(fl.to_string())), - None => (String::new(), Some(String::new())), + unsafe { + let source = if is_valid_ptr((*re).pattern_ptr) { + string_as_str((*re).pattern_ptr).to_string() + } else { + String::new() + }; + let inherited = if is_valid_ptr((*re).flags_ptr) { + string_as_str((*re).flags_ptr).to_string() + } else { + String::new() + }; + (source, Some(inherited)) } } else if pv.is_undefined() { (String::new(), None) @@ -1810,18 +1783,10 @@ pub extern "C" fn js_regexp_get_source(re: *const RegExpHeader) -> *mut StringHe if !is_valid_regex_ptr(re) { return js_string_from_str("(?:)"); } - // Issue #637: prefer the side-tabled owned copy so we survive GC - // of the input StringHeader (e.g. template-literal temporary). - if let Some(pat) = - REGEX_SOURCE_TABLE.with(|t| t.borrow().get(&(re as usize)).map(|(p, _)| p.clone())) - { - return js_string_from_str(&escape_regexp_source(&pat)); - } unsafe { if is_valid_ptr((*re).pattern_ptr) { - // Return a copy of the pattern string - let pattern_str = string_as_str((*re).pattern_ptr); - js_string_from_str(&escape_regexp_source(pattern_str)) + let escaped = escape_regexp_source(string_as_bytes((*re).pattern_ptr)); + crate::string::js_string_from_wtf8_bytes(escaped.as_ptr(), escaped.len() as u32) } else { js_string_from_str("(?:)") } @@ -1841,12 +1806,6 @@ pub extern "C" fn js_regexp_get_flags(re: *const RegExpHeader) -> *mut StringHea if !is_valid_regex_ptr(re) { return js_string_from_str(""); } - // Issue #637: prefer the side-tabled owned copy. - if let Some(flags) = - REGEX_SOURCE_TABLE.with(|t| t.borrow().get(&(re as usize)).map(|(_, f)| f.clone())) - { - return js_string_from_str(&flags); - } unsafe { if is_valid_ptr((*re).flags_ptr) { let flags_str = string_as_str((*re).flags_ptr); diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index f8472f50d6..865fdfd02b 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -152,8 +152,7 @@ pub extern "C" fn js_regexp_compile_value( // the one the receiver was constructed with. // `RegExp.prototype.compile` re-initialises an existing receiver — once per // call from user code, not per object — so materialising the shared key - // here costs nothing measurable, and the same `Arc`s go into the source - // table below. + // here costs nothing measurable. let pattern_key: std::sync::Arc = std::sync::Arc::from(pattern_str); let flags_key: std::sync::Arc = std::sync::Arc::from(flags_str); let arc = get_or_compile_regex(&pattern_key, &flags_key); @@ -203,6 +202,20 @@ pub extern "C" fn js_regexp_compile_value( } (*re).pattern_ptr = pattern_ptr; (*re).flags_ptr = canonical_flags_ptr; + // These are traced header edges. Unlike construction, `compile` can + // rewrite a tenured receiver with newly allocated nursery strings, so + // both stores need the ordinary runtime barrier. + let parent = re as usize; + crate::gc::runtime_write_barrier_gc_slot( + parent, + std::ptr::addr_of!((*re).pattern_ptr) as usize, + crate::value::js_nanbox_string(pattern_ptr as i64).to_bits(), + ); + crate::gc::runtime_write_barrier_gc_slot( + parent, + std::ptr::addr_of!((*re).flags_ptr) as usize, + crate::value::js_nanbox_string(canonical_flags_ptr as i64).to_bits(), + ); (*re).case_insensitive = flags_str.contains('i'); (*re).global = flags_str.contains('g'); (*re).multiline = flags_str.contains('m'); @@ -210,10 +223,6 @@ pub extern "C" fn js_regexp_compile_value( (*re).dot_all = flags_str.contains('s'); (*re).unicode = flags_str.contains('u') || flags_str.contains('v'); (*re).has_indices = flags_str.contains('d'); - super::REGEX_SOURCE_TABLE.with(|t| { - t.borrow_mut() - .insert(re as usize, (Arc::from(pattern_str), Arc::from(flags_str))); - }); } // Spec RegExpInitialize step 12: `Set(obj, "lastIndex", 0, true)` runs LAST, // with the *Throw* flag. A user-frozen `lastIndex` diff --git a/crates/perry-runtime/src/regex/escape.rs b/crates/perry-runtime/src/regex/escape.rs index 0a3b88acc4..6f1cfc2417 100644 --- a/crates/perry-runtime/src/regex/escape.rs +++ b/crates/perry-runtime/src/regex/escape.rs @@ -143,13 +143,8 @@ pub extern "C" fn js_regexp_escape(input: f64) -> f64 { #[used] static KEEP_REGEXP_ESCAPE: extern "C" fn(f64) -> f64 = js_regexp_escape; -/// ECMA-262 22.2.6.10 EscapeRegExpPattern: produce a string that, placed -/// between two `/` characters, parses as the same pattern. An empty pattern -/// becomes `"(?:)"`; an unescaped `/` outside a character class becomes `\/`; -/// the four LineTerminators become their `\n`/`\r`/`
`/`
` escapes -/// (even inside a character class). A backslash escapes the following code -/// point, which is copied verbatim. -pub(super) fn escape_regexp_source(pattern: &str) -> String { +/// ECMA-262 22.2.6.10 EscapeRegExpPattern for a valid UTF-8 pattern. +fn escape_regexp_source_utf8(pattern: &str) -> String { if pattern.is_empty() { return "(?:)".to_string(); } @@ -183,3 +178,73 @@ pub(super) fn escape_regexp_source(pattern: &str) -> String { } out } + +/// ECMA-262 22.2.6.10 EscapeRegExpPattern: produce WTF-8 bytes that, placed +/// between two `/` characters, parse as the same pattern. JavaScript strings +/// may contain lone UTF-16 surrogates, represented by Perry as WTF-8; those +/// bytes must round-trip rather than pass through Rust's `str::chars()`. +pub(super) fn escape_regexp_source(pattern: &[u8]) -> Vec { + if let Ok(pattern) = std::str::from_utf8(pattern) { + return escape_regexp_source_utf8(pattern).into_bytes(); + } + if pattern.is_empty() { + return b"(?:)".to_vec(); + } + + let mut out = Vec::with_capacity(pattern.len() + 2); + let mut in_class = false; + let mut i = 0; + while i < pattern.len() { + match pattern[i] { + b'\\' => { + out.push(b'\\'); + i += 1; + if i < pattern.len() { + let (advance, _, _) = crate::string::wtf8_step(pattern, i); + let end = i.saturating_add(advance).min(pattern.len()); + out.extend_from_slice(&pattern[i..end]); + i = end; + } + } + b'[' if !in_class => { + in_class = true; + out.push(b'['); + i += 1; + } + b']' if in_class => { + in_class = false; + out.push(b']'); + i += 1; + } + b'/' if !in_class => { + out.extend_from_slice(b"\\/"); + i += 1; + } + b'\n' => { + out.extend_from_slice(b"\\n"); + i += 1; + } + b'\r' => { + out.extend_from_slice(b"\\r"); + i += 1; + } + 0xE2 if pattern.get(i..i + 3) == Some(&[0xE2, 0x80, 0xA8]) + || pattern.get(i..i + 3) == Some(&[0xE2, 0x80, 0xA9]) => + { + out.extend_from_slice(if pattern[i + 2] == 0xA8 { + b"\\u2028" + } else { + b"\\u2029" + }); + i += 3; + } + _ => { + let (advance, _, _) = crate::string::wtf8_step(pattern, i); + let end = i.saturating_add(advance).min(pattern.len()); + out.extend_from_slice(&pattern[i..end]); + i = end; + } + } + } + out +} diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index f5492c317f..eba7213bbd 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -37,7 +37,7 @@ //! lookbehind/backreferences still decides, and still throws when both //! engines refuse); //! * `.source` / `.flags` / `.global` / `.sticky` / `lastIndex` are header -//! and side-table reads that never touched the compiled program; +//! reads that never touched the compiled program; //! * identity is untouched — `js_regexp_new` still allocates a fresh header //! per evaluation. //! @@ -53,8 +53,7 @@ use regex::Regex; use super::grammar::{collapse_redos_guard_quantifiers, js_regex_to_rust_with_flags}; use super::{ evict_regex_cache_if_full, get_or_compile_regex, is_valid_ptr, is_valid_regex_ptr, - string_as_str, RegExpHeader, FANCY_CACHE, REGEX_SOURCE_TABLE, REPEAT_MATCHER_CACHE, - VALIDATED_PATTERNS, + string_as_str, RegExpHeader, FANCY_CACHE, REPEAT_MATCHER_CACHE, VALIDATED_PATTERNS, }; /// The exact string `build_std_regex` is handed for `(pattern, flags)`: the @@ -160,15 +159,10 @@ pub(super) fn mark_pattern_validated(pattern: &str, flags: &str) { /// The `(source, flags)` a header was built from. /// -/// Prefers the GC-survivable side table (issue #637) and falls back to the -/// header's own string payloads, which — unlike the thread-local table — are -/// readable from a second statically-linked copy of the runtime (Wall 18). +/// Since #9845 the header's string slots are traced GC edges, so the payloads +/// are both collection-safe and readable from a second statically-linked copy +/// of the runtime (Wall 18). pub(super) fn source_and_flags(re: *const RegExpHeader) -> (Arc, Arc) { - if let Some(source) = - REGEX_SOURCE_TABLE.with(|table| table.borrow().get(&(re as usize)).cloned()) - { - return source; - } unsafe { let pattern: Arc = if is_valid_ptr((*re).pattern_ptr) { Arc::from(string_as_str((*re).pattern_ptr)) diff --git a/crates/perry-runtime/src/regex/program_key.rs b/crates/perry-runtime/src/regex/program_key.rs index e753fe0f69..45852319cb 100644 --- a/crates/perry-runtime/src/regex/program_key.rs +++ b/crates/perry-runtime/src/regex/program_key.rs @@ -29,9 +29,8 @@ pub(crate) const NEVER_MATCH_PATTERN: &str = r"[^\s\S]"; /// 1,984 MB), which is what `.to_string()` on an `Arc` lowers to. /// /// Keying by `Arc` makes a probe two refcount increments and no -/// allocation: every caller that matters already holds those `Arc`s, because -/// `REGEX_SOURCE_TABLE` and `regex::site_cache` share one allocation of a -/// literal's text with every header built from it. Hashing still walks the +/// allocation: every caller that matters already holds those `Arc`s through +/// `regex::site_cache`. Hashing still walks the /// pattern bytes — the allocation is what the census measured, and what this /// removes. #[cfg(feature = "regex-engine")] diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index bf0d3c1897..17788e277b 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -8,8 +8,8 @@ //! `/…/g` on every call, once per text segment per layout pass, and //! `ansi-regex` builds the same `new RegExp(parts.join("|"), "g")` per call. //! Each construction used to copy the pattern three times (the -//! `VALIDATED_PATTERNS` probe key, `owned_pattern`, the `REGEX_SOURCE_TABLE` -//! entry) and SipHash all of it once; the first operation on each header then +//! `VALIDATED_PATTERNS` probe key and `owned_pattern`) and SipHash all of it +//! once; the first operation on each header then //! did the same three more times — `build_and_install_programs` probes the //! three `(String, String)`-keyed program caches — and, for the common //! no-fallback pattern, `lookup_fancy_regex` / `lookup_repeat_matcher` @@ -24,9 +24,8 @@ //! full byte compare — identity never depends on an address, so nothing is //! rekeyed on a GC move and a dynamic `new RegExp(sameText)` hits too; a hit //! costs one `memcmp` instead of a hash plus three copies. An entry owns the -//! pattern and canonical flags as `Arc` (shared into -//! `REGEX_SOURCE_TABLE`, so a header costs two refcount bumps instead of two -//! `String`s) and, once the first header built from it has been executed, the +//! pattern and canonical flags as `Arc` and, once the first header built +//! from it has been executed, the //! compiled programs: a later construction installs those eagerly, so the //! header is born built and never touches the `(pattern, flags)` caches. //! diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index 3ea07b9671..ec6b577f66 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -151,6 +151,66 @@ fn construction_defers_the_program_build_until_first_use() { ); } +/// Removing the address-keyed source table must not turn the RegExp-pattern +/// constructor arm into an empty-pattern fallback. This calls the exported +/// constructor entry point, so deleting its direct header read fails both the +/// source and inherited-flags assertions. +#[test] +fn regexp_construct_reads_source_and_flags_from_the_pattern_header() { + let original = js_regexp_new(make_string("left/right"), make_string("ig")); + let pattern = crate::value::js_nanbox_pointer(original as i64); + let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); + + let copy = js_regexp_construct(pattern, undefined); + assert_ne!( + copy, original, + "construction must still allocate a fresh object" + ); + assert_eq!(string_payload(js_regexp_get_source(copy)), b"left\\/right"); + assert_eq!(string_payload(js_regexp_get_flags(copy)), b"gi"); + + let override_flags = crate::value::js_nanbox_string(make_string("m") as i64); + let overridden = js_regexp_construct(pattern, override_flags); + assert_eq!( + string_payload(js_regexp_get_source(overridden)), + b"left\\/right" + ); + assert_eq!(string_payload(js_regexp_get_flags(overridden)), b"m"); +} + +/// `RegExp.prototype.compile` rewrites the header in place. The source table +/// used to mask a stale header slot here; after its removal both observable +/// strings must come from the newly stored, traced edges. +#[test] +fn regexp_compile_replaces_the_header_source_and_flags() { + let receiver = js_regexp_new(make_string("old"), make_string("m")); + js_regexp_set_last_index(receiver, 9.0); + let pattern = crate::value::js_nanbox_string(make_string("new/source") as i64); + let flags = crate::value::js_nanbox_string(make_string("ig") as i64); + let result = js_regexp_compile_value(receiver, pattern, flags); + let receiver = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); + + assert_eq!( + string_payload(js_regexp_get_source(receiver)), + b"new\\/source" + ); + assert_eq!(string_payload(js_regexp_get_flags(receiver)), b"gi"); + assert_eq!(js_regexp_get_last_index(receiver), 0.0); + assert_eq!(js_regexp_test(receiver, make_string("NEW/source")), 1); +} + +/// Perry stores lone JavaScript surrogates as WTF-8. `.source` must copy those +/// exact bytes from the traced pattern slot; routing them through Rust's UTF-8 +/// scalar iterator either replaces the surrogate or invokes undefined +/// behaviour. +#[test] +fn regexp_source_round_trips_wtf8_lone_surrogates_from_the_header() { + let lone_high = [b'a', 0xED, 0xA0, 0x80, b'/', b'b']; + let re = js_regexp_new(make_wtf8(&lone_high), make_string("")); + let expected = [b'a', 0xED, 0xA0, 0x80, b'\\', b'/', b'b']; + assert_eq!(string_payload(js_regexp_get_source(re)), expected); +} + /// The deferred build installs the fancy-regex and RepeatMatcher programs too, /// not just the linear one — they live on the same publish point, so a header /// whose pattern needs one must still get it on first use. @@ -633,16 +693,6 @@ fn site_cache_reconstruction_is_born_built() { std::ptr::eq(unsafe { (*re1).regex_ptr }, unsafe { (*re2).regex_ptr }), "both headers share one compiled program" ); - // The owned source copies are shared too (two refcount bumps per header, - // not two `String`s). - let (p1, p2) = REGEX_SOURCE_TABLE.with(|t| { - let t = t.borrow(); - ( - t.get(&(re1 as usize)).map(|(p, _)| p.clone()).unwrap(), - t.get(&(re2 as usize)).map(|(p, _)| p.clone()).unwrap(), - ) - }); - assert!(Arc::ptr_eq(&p1, &p2), "source text is shared, not copied"); assert_eq!(js_regexp_test(re2, make_string("born7built")), 1); assert_eq!(js_regexp_test(re2, make_string("nothing")), 0); // Different flags are a different entry. From e2b0a905482fd3440714b1299a8c10aa7aa742d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:21:49 +0200 Subject: [PATCH 2/8] perf(regex): share one program-set handle per header --- .../gc/tests/copying/survival_and_malloc.rs | 4 +- crates/perry-runtime/src/regex.rs | 158 +++++------------- crates/perry-runtime/src/regex/compile.rs | 51 ++---- .../perry-runtime/src/regex/compile_cache.rs | 4 +- crates/perry-runtime/src/regex/lazy.rs | 54 ++---- .../perry-runtime/src/regex/replace_expand.rs | 2 +- crates/perry-runtime/src/regex/site_cache.rs | 16 +- crates/perry-runtime/src/regex/site_key.rs | 72 +++----- crates/perry-runtime/src/regex/tests.rs | 49 ++++-- crates/perry-runtime/src/regex/tests_part2.rs | 46 ++--- 10 files changed, 173 insertions(+), 283 deletions(-) diff --git a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs index 5765446f41..fba293deca 100644 --- a/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs +++ b/crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs @@ -1035,7 +1035,7 @@ fn nursery_regexp_that_dies_young_is_finalized_by_the_copied_minor() { ); assert!(crate::regex::test_regex_pointer_entry_exists(dead_addr)); // Both headers share one program through the site cache. - let count_before = crate::regex::test_regexp_std_program_strong_count(live); + let count_before = crate::regex::test_regexp_program_set_strong_count(live); assert!(count_before >= 2); // Only `live` is rooted; `dead` is garbage. @@ -1053,7 +1053,7 @@ fn nursery_regexp_that_dies_young_is_finalized_by_the_copied_minor() { "a nursery RegExp that died must be removed from REGEX_POINTERS by the copied minor" ); assert_eq!( - crate::regex::test_regexp_std_program_strong_count(live_new as *const _), + crate::regex::test_regexp_program_set_strong_count(live_new as *const _), count_before - 1, "the dead header's Arc clone of the shared program must have been dropped" ); diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 5d8aa2db2f..527e0c99b4 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -23,14 +23,14 @@ use crate::value::js_nanbox_string; use crate::object::ObjectHeader; -/// The compiled standard-engine regex type. When the regex engine is gated -/// off, `RegExpHeader::regex_ptr` is typed `*mut ()` (a never-dereferenced -/// dangling field) so the identity/display layer keeps the same struct -/// layout without pulling in the `regex` crate. +/// The shared compiled-program set. When the regex engine is gated off, +/// `RegExpHeader::programs_ptr` is typed `*const ()` (a never-dereferenced +/// field) so the identity/display layer keeps the same struct layout without +/// pulling in the matcher crates. #[cfg(feature = "regex-engine")] -type CompiledRegex = regex::Regex; +type CompiledPrograms = site_cache::Programs; #[cfg(not(feature = "regex-engine"))] -type CompiledRegex = (); +type CompiledPrograms = (); #[cfg(feature = "regex-engine")] mod class_range_validate; @@ -224,7 +224,7 @@ pub(crate) fn regex_header_clear_dead_for_gc(addr: usize) { /// Release the compiled programs owned by a dead `RegExpHeader`, then remove /// its address-owned metadata. /// -/// The program pointers are raw `Arc` references installed by +/// The program pointer is a raw `Arc` reference installed by /// `lazy::build_and_install_programs` or `RegExp.prototype.compile`. Null them /// before reconstructing the `Arc`s because arena cleanup can visit the /// metadata and finalizer paths for the same dead cell. @@ -234,23 +234,11 @@ pub(crate) unsafe fn regex_header_finalize_for_gc(re: *mut RegExpHeader) { } #[cfg(feature = "regex-engine")] { - let regex_ptr = (*re).regex_ptr; - let fancy_ptr = (*re).fancy_ptr; - let repeat_matcher_ptr = (*re).repeat_matcher_ptr; - (*re).regex_ptr = ptr::null_mut(); - (*re).fancy_ptr = ptr::null(); - (*re).repeat_matcher_ptr = ptr::null(); - - if !regex_ptr.is_null() { - drop(Arc::from_raw(regex_ptr as *const Regex)); - } - if !fancy_ptr.is_null() { - drop(Arc::from_raw(fancy_ptr as *const fancy_regex::Regex)); - } - if !repeat_matcher_ptr.is_null() { - drop(Arc::from_raw( - repeat_matcher_ptr as *const repeat_matcher::RepeatMatcherRegex, - )); + let programs_ptr = (*re).programs_ptr; + (*re).programs_ptr = ptr::null(); + + if !programs_ptr.is_null() { + drop(Arc::from_raw(programs_ptr)); } } regex_header_clear_dead_for_gc(re as usize); @@ -260,7 +248,7 @@ pub(crate) unsafe fn regex_header_finalize_for_gc(re: *mut RegExpHeader) { /// /// The copying minor's from-space flip runs no per-object finalize hooks, so /// a nursery header that was neither evacuated nor pinned would otherwise keep -/// its `Arc` programs and its `REGEX_POINTERS` / expando +/// its program-set `Arc` and its `REGEX_POINTERS` / expando /// entries forever. Same shape as `map::finalize_dead_copied_minor_from_space_maps`: /// walk the registry after the flip, collect the provably-dead addresses, then /// finalize each (the finalizer removes its own registry entries, which is why @@ -359,14 +347,14 @@ pub(crate) fn test_construct_regexp_and_exec_once(pattern: &str, flags: &str) -> /// Test support: strong count of the standard program a header holds (the /// observer clone taken here is released before returning). #[cfg(all(test, feature = "regex-engine"))] -pub(crate) fn test_regexp_std_program_strong_count(re: *const RegExpHeader) -> usize { +pub(crate) fn test_regexp_program_set_strong_count(re: *const RegExpHeader) -> usize { unsafe { - let raw = (*re).regex_ptr as *const Regex; - assert!(!raw.is_null(), "program must be installed"); - let arc = Arc::from_raw(raw); - let n = Arc::strong_count(&arc); + let programs = (*re).programs_ptr; + assert!(!programs.is_null(), "program must be installed"); + let arc = Arc::from_raw(programs); + let count = Arc::strong_count(&arc); std::mem::forget(arc); - n + count } } @@ -394,7 +382,7 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * // Neither `gc_malloc` nor the arena zeroes reused memory, so this // must be set explicitly or the GC follows a garbage pointer. (*ptr).meta = std::ptr::null_mut(); - (*ptr).regex_ptr = std::ptr::null_mut(); + (*ptr).programs_ptr = std::ptr::null(); (*ptr).pattern_ptr = pattern.get_raw_const_ptr::(); (*ptr).flags_ptr = flags_string.get_raw_const_ptr::(); (*ptr).case_insensitive = flags.contains('i'); @@ -406,8 +394,6 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * (*ptr).has_indices = flags.contains('d'); (*ptr).last_index = crate::value::JSValue::number(0.0).bits(); (*ptr).magic = REGEXP_MAGIC; - (*ptr).fancy_ptr = std::ptr::null(); - (*ptr).repeat_matcher_ptr = std::ptr::null(); REGEX_EVER_REGISTERED.arm(); REGEX_POINTERS.with(|table| { @@ -456,7 +442,7 @@ pub(crate) fn regex_header_has_magic(re: *const RegExpHeader) -> bool { /// * `flags_ptr` — the flags `StringHeader`, /// * `last_index` — a writable JSValue (`re.lastIndex = …`) that may be a /// NaN-boxed heap pointer. -/// The compiled matcher pointers point to OFF-heap leaked Rust allocations and the +/// The compiled-program pointer points to an OFF-heap Rust allocation and the /// bool/`magic` fields are never heap refs, so they must NOT be scanned. /// /// `pattern_ptr` and `flags_ptr` are consecutive equal-width fields, so under @@ -505,11 +491,12 @@ pub(crate) use compile_cache::*; /// Header for heap-allocated RegExp objects #[repr(C)] pub struct RegExpHeader { - /// Pointer to the compiled Regex object (boxed). Typed via the - /// `CompiledRegex` alias so the struct layout is identical whether or not - /// the regex engine is linked (it's `*mut ()` when gated off and never - /// dereferenced — all dereferencing sites are themselves engine-gated). - regex_ptr: *mut CompiledRegex, + /// Header-owned `Arc` raw pointer, or null until first use. + /// The program set contains the standard engine and optional fancy/repeat + /// matchers once per pattern instead of repeating three pointers in every + /// RegExp object. Typed through `CompiledPrograms` so the layout is stable + /// when the regex engine is gated off. + programs_ptr: *const CompiledPrograms, /// Original pattern string (for debugging/serialization) pattern_ptr: *const StringHeader, /// Flags string (e.g., "gi" for global+ignoreCase) @@ -548,15 +535,6 @@ pub struct RegExpHeader { /// identity + fancy-fallback resolution independent of WHICH runtime copy's /// thread-locals are live. Set to `REGEXP_MAGIC` by `js_regexp_new`. pub magic: u64, - /// Leaked `Arc` (as a raw pointer) for patterns the - /// `regex` crate can't compile (lookahead/lookbehind/backrefs), or null. - /// Header-resident twin of the `FANCY_CACHE` thread-local so the fancy - /// fallback survives the duplicate-runtime split described above. - pub fancy_ptr: *const (), - /// Header-owned `Arc` for quantified capture groups, - /// or null for the ordinary linear/fancy paths. Like `fancy_ptr`, this - /// survives cache eviction and duplicate statically-linked runtime copies. - pub repeat_matcher_ptr: *const (), /// #6759 phase 1 (header unification): per-object metadata record, or /// null. Appended LAST so `regex_gc_slot_ptrs`' adjacency assertion on /// `pattern_ptr`/`flags_ptr` and every other offset are undisturbed. @@ -706,8 +684,8 @@ fn newborn_barrier_gate_enabled() -> bool { /// /// Validates the pattern and allocates the header; it does NOT build the /// compiled program. That happens on the first operation that needs a matcher -/// — see `regex::lazy`, and the `regex_ptr`/`fancy_ptr`/`repeat_matcher_ptr` -/// fields, which are null until then. A fresh header per call is required: +/// — see `regex::lazy`; `programs_ptr` is null until then. A fresh header per +/// call is required: /// ECMA-262 evaluates a regex literal to a NEW object every time, and the /// distinction is observable through `===`, expandos and `lastIndex`. #[cfg(feature = "regex-engine")] @@ -1008,7 +986,7 @@ fn js_regexp_new_impl( // established that the pattern is legal, and a bundle evaluates hundreds // of module-level literals it never matches with — building each one's // NFA at construction is what put ~14% of a claude-code `--help` run - // inside `regex_syntax`/`regex_automata`. `regex_ptr` stays null (the + // inside `regex_syntax`/`regex_automata`. `programs_ptr` stays null (the // "not built yet" state) and `lazy::ensure_regex_compiled` installs the // owned `Arc`s on the first operation that needs a matcher. @@ -1087,7 +1065,7 @@ fn js_regexp_new_impl( // exercised the arena arm all along. What kept production on malloc was // finalization: the copying minor's from-space flip runs no per-object // finalize hooks (`gc::copying`), so a nursery header that dies young - // would leak its three `Arc` programs and its registry entries. That is + // would leak its program-set `Arc` and its registry entries. That is // now handled the way Map/Set/Error handle theirs — // `finalize_dead_copied_minor_from_space_regexps` after a copied minor and // `collect_dead_registered_regexps_post_trace` at sweep entry for the @@ -1152,7 +1130,7 @@ fn js_regexp_new_impl( // must be set explicitly or the GC follows a garbage pointer. (*ptr).meta = std::ptr::null_mut(); // Null = not compiled yet; see `lazy::ensure_regex_compiled`. - (*ptr).regex_ptr = std::ptr::null_mut(); + (*ptr).programs_ptr = std::ptr::null(); (*ptr).pattern_ptr = pattern; (*ptr).flags_ptr = canonical_flags_ptr; // `pattern_ptr` / `flags_ptr` are GC-managed StringHeaders — the GC scans @@ -1232,26 +1210,11 @@ fn js_regexp_new_impl( // Wall 18: self-identifying marker so identity checks survive a // duplicate-runtime thread-local split. (*ptr).magic = REGEXP_MAGIC; - // The header-resident fancy-regex fallback (lookahead/lookbehind/ - // backrefs) and the ECMAScript backtracking matcher are installed - // alongside `regex_ptr` by `lazy::ensure_regex_compiled`, from the - // same caches, on the first operation that needs a matcher. Keeping - // all three on one publish point is what makes `regex_ptr.is_null()` - // a sound built/not-built flag. - (*ptr).fancy_ptr = std::ptr::null(); - (*ptr).repeat_matcher_ptr = std::ptr::null(); - // Born built: the site cache already holds the programs the first - // execution of this text compiled. Install the same three owned - // references `lazy::build_and_install_programs` would, publishing - // `regex_ptr` last for the same reason it does. + // Born built: the site cache already holds the shared program set the + // first execution of this text compiled. Install one owned reference; + // null remains the sound not-built state. if let Some(programs) = programs { - (*ptr).fancy_ptr = programs - .fancy - .map_or(std::ptr::null(), |arc| Arc::into_raw(arc) as *const ()); - (*ptr).repeat_matcher_ptr = programs - .repeat - .map_or(std::ptr::null(), |arc| Arc::into_raw(arc) as *const ()); - (*ptr).regex_ptr = Arc::into_raw(programs.std) as *mut Regex; + (*ptr).programs_ptr = Arc::into_raw(programs); } // Record the pointer so that js_string_split can detect @@ -1538,32 +1501,14 @@ pub(super) fn diag_note_op(re: *const RegExpHeader, op: crate::hot_diag::RegexOp /// pattern (backreferences, lookbehind, etc.). #[cfg(feature = "regex-engine")] pub(crate) fn lookup_fancy_regex(re: *const RegExpHeader) -> Option> { - // The header's programs are built on first use; `fancy_ptr` is null until - // then, and a null there is indistinguishable from "this pattern has no - // fancy fallback" — so build before reading it. + // The header's shared program set is built on first use. lazy::ensure_regex_compiled(re); unsafe { - // Wall 18: header-resident fancy Arc first (duplicate-runtime - // thread-local resilient). `fancy_ptr` is a leaked `Arc` raw pointer; to - // hand back an owned `Arc` clone WITHOUT consuming the header's - // reference, reconstruct, clone, then `mem::forget` the reconstructed - // one so the header's strong count is preserved. + // Wall 18: header-resident program set first (duplicate-runtime + // thread-local resilient). if regex_header_has_magic(re) { - if (*re).fancy_ptr.is_null() { - // Built (see `ensure_regex_compiled` above) with no fancy - // fallback: every install path (`lazy`, `compile`, the site - // cache) publishes all three program pointers together, so a - // null here is the answer, not "not looked up yet". Falling - // through to the cache probe re-hashed the whole pattern on - // EVERY exec of every ordinary regex (#keystroke profile: - // 514 samples under this function alone). - return None; - } - let raw = (*re).fancy_ptr as *const fancy_regex::Regex; - let arc = Arc::from_raw(raw); - let cloned = arc.clone(); - std::mem::forget(arc); - return Some(cloned); + let programs = &*(*re).programs_ptr; + return programs.fancy.clone(); } let pat = string_as_str((*re).pattern_ptr); let flags_str = string_as_str((*re).flags_ptr); @@ -1611,11 +1556,11 @@ pub(crate) fn lookup_fancy_regex(re: *const RegExpHeader) -> Option bool { unsafe { - let program = (*re).regex_ptr; - if program.is_null() { + let programs = (*re).programs_ptr; + if programs.is_null() { return false; } - let program: &Regex = &*program; + let program: &Regex = &(*programs).std; if program.as_str() == NEVER_MATCH_PATTERN { // The `regex` crate refused this pattern (lookaround / // backreference); it has no opinion about the subject. @@ -1649,22 +1594,11 @@ fn lookup_repeat_matcher_for( fn lookup_repeat_matcher( re: *const RegExpHeader, ) -> Option> { - // Same first-use build as `lookup_fancy_regex`: a null - // `repeat_matcher_ptr` means "not built yet" before it can mean "this - // pattern needs no backtracking matcher". lazy::ensure_regex_compiled(re); unsafe { if regex_header_has_magic(re) { - if (*re).repeat_matcher_ptr.is_null() { - // Same reasoning as `lookup_fancy_regex`: a built header with - // a null pointer has no backtracking matcher. - return None; - } - let raw = (*re).repeat_matcher_ptr as *const repeat_matcher::RepeatMatcherRegex; - let arc = Arc::from_raw(raw); - let cloned = arc.clone(); - std::mem::forget(arc); - return Some(cloned); + let programs = &*(*re).programs_ptr; + return programs.repeat.clone(); } let pat = string_as_str((*re).pattern_ptr); let flags_str = string_as_str((*re).flags_ptr); diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index 865fdfd02b..c569ad8bb3 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -4,8 +4,6 @@ use std::sync::Arc; -use regex::Regex; - use super::class_range_validate::has_out_of_order_double_dash_class_range; use super::grammar::{ has_invalid_repeated_quantifier, has_unicode_forbidden_legacy_escape, @@ -145,33 +143,30 @@ pub extern "C" fn js_regexp_compile_value( )); } - // The header OWNS raw `Arc` references to its compiled program(s) + // The header OWNS one raw `Arc` reference to its compiled program set // (mirrors `js_regexp_new`), so the capped `REGEX_CACHE`/`FANCY_CACHE` // (see `REGEX_CACHE_MAX_ENTRIES`) can evict without invalidating this - // receiver. Refresh `fancy_ptr` too — it must track the NEW pattern, not - // the one the receiver was constructed with. + // receiver. Refresh the whole program set so it tracks the NEW pattern, + // not the one the receiver was constructed with. // `RegExp.prototype.compile` re-initialises an existing receiver — once per // call from user code, not per object — so materialising the shared key // here costs nothing measurable. let pattern_key: std::sync::Arc = std::sync::Arc::from(pattern_str); let flags_key: std::sync::Arc = std::sync::Arc::from(flags_str); - let arc = get_or_compile_regex(&pattern_key, &flags_key); - let regex_ptr = Arc::into_raw(arc) as *mut Regex; - let fancy_ptr: *const () = super::FANCY_CACHE.with(|fc| { - match fc.borrow().get(&(pattern_key.clone(), flags_key.clone())) { - Some(arc) => Arc::into_raw(arc.clone()) as *const (), - None => std::ptr::null(), - } + let std = get_or_compile_regex(&pattern_key, &flags_key); + let fancy = super::FANCY_CACHE.with(|fc| { + fc.borrow() + .get(&(pattern_key.clone(), flags_key.clone())) + .cloned() }); - let repeat_matcher_ptr: *const () = super::REPEAT_MATCHER_CACHE.with(|cache| { - match cache + let repeat = super::REPEAT_MATCHER_CACHE.with(|cache| { + cache .borrow() .get(&(pattern_key.clone(), flags_key.clone())) - { - Some(arc) => Arc::into_raw(arc.clone()) as *const (), - None => std::ptr::null(), - } + .cloned() }); + let programs = Arc::new(super::site_cache::Programs { std, fancy, repeat }); + let programs_ptr = Arc::into_raw(programs); let (canonical_flags_ptr, _) = re_handle.across_mut::(|| js_string_from_str(flags_str)); let canonical_flags_handle = scope.root_string_ptr(canonical_flags_ptr); @@ -180,25 +175,13 @@ pub extern "C" fn js_regexp_compile_value( .across_const::(|| js_string_from_str(pattern_str)) }); unsafe { - let old_regex_ptr = (*re).regex_ptr; - let old_fancy_ptr = (*re).fancy_ptr; - let old_repeat_matcher_ptr = (*re).repeat_matcher_ptr; - (*re).regex_ptr = regex_ptr; - (*re).fancy_ptr = fancy_ptr; - (*re).repeat_matcher_ptr = repeat_matcher_ptr; + let old_programs_ptr = (*re).programs_ptr; + (*re).programs_ptr = programs_ptr; // Release the receiver's PREVIOUS owned references now that the new // ones are installed (recompiling the same pattern is fine: the fresh // `into_raw` reference above keeps the shared program alive). - if !old_regex_ptr.is_null() { - drop(Arc::from_raw(old_regex_ptr as *const Regex)); - } - if !old_fancy_ptr.is_null() { - drop(Arc::from_raw(old_fancy_ptr as *const fancy_regex::Regex)); - } - if !old_repeat_matcher_ptr.is_null() { - drop(Arc::from_raw( - old_repeat_matcher_ptr as *const super::repeat_matcher::RepeatMatcherRegex, - )); + if !old_programs_ptr.is_null() { + drop(Arc::from_raw(old_programs_ptr)); } (*re).pattern_ptr = pattern_ptr; (*re).flags_ptr = canonical_flags_ptr; diff --git a/crates/perry-runtime/src/regex/compile_cache.rs b/crates/perry-runtime/src/regex/compile_cache.rs index bde70bc77f..c08835f960 100644 --- a/crates/perry-runtime/src/regex/compile_cache.rs +++ b/crates/perry-runtime/src/regex/compile_cache.rs @@ -131,7 +131,7 @@ pub(crate) fn evict_regex_cache_if_full(cache: &mut HashMap) { /// One shared never-match program per thread. /// /// Only used by the `PERRY_REGEX_ENGINE=regress` measurement path, where every -/// pattern needs a value in `regex_ptr` (the built/not-built flag) but no NFA: +/// pattern needs a value in `programs_ptr` (the built/not-built flag) but no NFA: /// building a fresh one per pattern would be exactly the compile cost the /// experiment exists to remove from the measurement. #[cfg(feature = "regex-engine")] @@ -173,7 +173,7 @@ pub(crate) fn compile_and_cache_regex_checked(pattern: &Arc, flags: &Arc (Arc, Arc) /// Build this header's compiled program(s) if it has none yet. /// -/// `regex_ptr == null` is the "not built yet" state. It is published LAST so -/// a header is never observable as built while `fancy_ptr` / -/// `repeat_matcher_ptr` are still stale — every reader that consults those -/// two goes through [`lookup_fancy_regex`](super::lookup_fancy_regex) / -/// `lookup_repeat_matcher`, which call this first. +/// `programs_ptr == null` is the "not built yet" state. The one-pointer +/// publication keeps the three engines coherent. /// -/// The header OWNS a leaked `Arc` reference to each program (mirroring what -/// `js_regexp_new` used to do inline), so the capped `REGEX_CACHE` / -/// `FANCY_CACHE` / `REPEAT_MATCHER_CACHE` can evict without invalidating a -/// live receiver. +/// The header OWNS one leaked `Arc` to the complete program set, so the capped +/// `REGEX_CACHE` / `FANCY_CACHE` / `REPEAT_MATCHER_CACHE` can evict without +/// invalidating a live receiver. /// /// Contains no JS allocation and cannot re-enter the interpreter, so it is /// safe to call from inside a phase that holds a borrow of a GC string. @@ -209,7 +204,7 @@ pub(crate) fn ensure_regex_compiled(re: *const RegExpHeader) { if !is_valid_ptr(re) { return; } - if unsafe { !(*re).regex_ptr.is_null() } { + if unsafe { !(*re).programs_ptr.is_null() } { return; } build_and_install_programs(re); @@ -248,9 +243,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { }); // ── 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 + // A built header is treated as AUTHORITATIVE, 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 @@ -300,32 +293,21 @@ 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(), - }, - ); - 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 ()); - let repeat_matcher_ptr: *const () = - repeat_arc.map_or(std::ptr::null(), |arc| Arc::into_raw(arc) as *const ()); + let programs = Arc::new(super::site_cache::Programs { + std: std_arc.clone(), + fancy: fancy_arc.clone(), + repeat: repeat_arc.clone(), + }); + super::site_cache::install_programs(&pattern, &flags, programs.clone()); unsafe { let re = re as *mut RegExpHeader; - (*re).fancy_ptr = fancy_ptr; - (*re).repeat_matcher_ptr = repeat_matcher_ptr; - // Publish last: `regex_ptr` is the built/not-built flag. - (*re).regex_ptr = regex_ptr; + (*re).programs_ptr = Arc::into_raw(programs); } } /// The header's standard-engine program, building it on first use. /// -/// Every `&*(*re).regex_ptr` in the tree goes through here — the field is +/// Every standard-program borrow in the tree goes through here — the field is /// null until something needs a matcher. /// /// # Safety @@ -334,5 +316,5 @@ fn build_and_install_programs(re: *const RegExpHeader) { /// header owns until its GC finalizer runs. pub(crate) unsafe fn header_std_regex<'a>(re: *const RegExpHeader) -> &'a Regex { ensure_regex_compiled(re); - &*(*re).regex_ptr + &(*(*re).programs_ptr).std } diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index 70b1931f61..89391be8aa 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -374,7 +374,7 @@ pub extern "C" fn js_string_replace_regex_fn( // If the `regex` crate couldn't compile this pattern (lookahead, // backreferences, …), `get_or_compile_regex` stashed a never-match - // placeholder in `(*re).regex_ptr` and the real pattern in + // placeholder in the header's standard program and the real pattern in // `FANCY_CACHE`. Route the callback-replace through fancy-regex so the // callback actually fires — otherwise `captures_iter` below would // silently match nothing and return the input unchanged. (get-intrinsic's diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index 17788e277b..459ad1f1de 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -47,28 +47,18 @@ pub(super) struct Programs { pub(super) repeat: Option>, } -impl Clone for Programs { - fn clone(&self) -> Self { - Self { - std: self.std.clone(), - fancy: self.fancy.clone(), - repeat: self.repeat.clone(), - } - } -} - /// What a construction gets back on a hit. pub(super) struct Hit { pub(super) pattern: Arc, pub(super) flags: Arc, - pub(super) programs: Option, + pub(super) programs: Option>, } struct Entry { fp: u64, pattern: Arc, flags: Arc, - programs: Option, + programs: Option>, } /// Direct-mapped slots (2-way: a fingerprint may live in `slot` or @@ -204,7 +194,7 @@ pub(super) fn insert(pattern: &str, flags: &str) -> (Arc, Arc) { /// Attach the programs the first execution built to the entry for /// `(pattern, canonical flags)`, so every later construction of the same /// text is born built. Inserts the entry if it was evicted meanwhile. -pub(super) fn install_programs(pattern: &str, flags: &str, programs: Programs) { +pub(super) fn install_programs(pattern: &str, flags: &str, programs: Arc) { if !enabled() { return; } diff --git a/crates/perry-runtime/src/regex/site_key.rs b/crates/perry-runtime/src/regex/site_key.rs index aff7f4f028..842faa7072 100644 --- a/crates/perry-runtime/src/regex/site_key.rs +++ b/crates/perry-runtime/src/regex/site_key.rs @@ -65,37 +65,20 @@ use super::site_cache::Programs; /// site entry whose programs have been dropped simply reports "not built /// yet", and the next construction re-picks them up from the content cache — /// the same path the site's very first construction takes. -struct WeakPrograms { - std: Weak<::regex::Regex>, - fancy: Option>, - repeat: Option>, -} +struct WeakPrograms(Weak); impl WeakPrograms { - fn downgrade(programs: &Programs) -> Self { - Self { - std: Arc::downgrade(&programs.std), - fancy: programs.fancy.as_ref().map(Arc::downgrade), - repeat: programs.repeat.as_ref().map(Arc::downgrade), - } + fn downgrade(programs: &Arc) -> Self { + Self(Arc::downgrade(programs)) } /// ALL-OR-NOTHING. A header must carry **every** program its pattern needs /// — that is #9801's coherence rule, and a partial upgrade is exactly the /// incoherent triple it fixed: a standard program installed beside a /// missing fancy fallback silently never-matches instead of falling back. - /// So a single dead reference makes the whole entry report unbuilt. - fn upgrade(&self) -> Option { - let std = self.std.upgrade()?; - let fancy = match &self.fancy { - None => None, - Some(weak) => Some(weak.upgrade()?), - }; - let repeat = match &self.repeat { - None => None, - Some(weak) => Some(weak.upgrade()?), - }; - Some(Programs { std, fancy, repeat }) + /// One weak pointer to the bundle makes partial upgrade unrepresentable. + fn upgrade(&self) -> Option> { + self.0.upgrade() } } @@ -137,7 +120,7 @@ pub(super) struct SiteHit { pub(super) flags: Arc, pub(super) flags_are_canonical: bool, pub(super) bits: FlagBits, - pub(super) programs: Option, + pub(super) programs: Option>, } /// Direct-mapped, 2-way (a key may live in `slot` or `slot ^ 1`). A bundle's @@ -204,7 +187,7 @@ pub(super) fn record( flags: Arc, flags_are_canonical: bool, bits: FlagBits, - programs: Option, + programs: Option>, ) { if !enabled() || key == 0 { return; @@ -260,7 +243,7 @@ pub(super) fn record( /// Attach the programs the first execution built, so later constructions at /// this site are born built. A no-op when the site was evicted meanwhile. -pub(super) fn install_programs(key: usize, programs: Programs) { +pub(super) fn install_programs(key: usize, programs: Arc) { if !enabled() || key == 0 { return; } @@ -323,42 +306,38 @@ mod tests { /// #9801 fixed an incoherent triple — a standard program memoized beside a /// missing fancy fallback — which does not error: it silently never /// matches. Holding the site entry's programs weakly reintroduces exactly - /// that shape unless a dead reference invalidates the WHOLE entry, because - /// the three `Arc`s have independent lifetimes and the fancy fallback is - /// the one a pattern the linear engine refused depends on. - /// - /// A sabotage that upgrades each field independently — the natural way to - /// write it — returns `Some(Programs { std, fancy: None, .. })` here and - /// fails on the second assertion. + /// that shape if it weakens each matcher separately. A single weak pointer + /// to the program bundle makes an incoherent partial upgrade impossible. #[test] - fn one_dead_reference_invalidates_the_whole_entry() { + fn the_program_set_weak_reference_expires_atomically() { let std_program = Arc::new(::regex::Regex::new("a(b)c").expect("linear program")); let fancy_program = Arc::new(::fancy_regex::Regex::new("a(?=b)c").expect("fancy program")); - let programs = Programs { + let programs = Arc::new(Programs { std: std_program.clone(), fancy: Some(fancy_program.clone()), repeat: None, - }; + }); let weak = WeakPrograms::downgrade(&programs); - drop(programs); let upgraded = weak .upgrade() - .expect("both strong references are still held here"); + .expect("the shared program set is still held here"); assert!( upgraded.fancy.is_some(), "the fancy fallback must survive the round trip while its Arc is alive" ); drop(upgraded); - // Only the FANCY program dies. The standard one is still strongly held. - drop(fancy_program); + // Individual matcher Arcs do not keep the SET alive. Once the bundle + // is gone the weak entry expires all three lanes together, which is + // the coherence property a triple of independent Weak pointers had + // to implement manually. + drop(programs); assert!( weak.upgrade().is_none(), - "one dead reference must invalidate the whole entry — handing back a header with a \ - standard program and no fancy fallback is #9801's incoherent triple, which never \ - matches instead of failing" + "the site entry must never upgrade only part of a program set" ); + drop(fancy_program); drop(std_program); assert!(weak.upgrade().is_none()); } @@ -371,11 +350,11 @@ mod tests { test_reset(); let key = 0x5171_E000_usize; let std_program = Arc::new(::regex::Regex::new("keepalive").expect("linear program")); - let programs = Programs { + let programs = Arc::new(Programs { std: std_program.clone(), fancy: None, repeat: None, - }; + }); record( key, Arc::from("g"), @@ -391,7 +370,7 @@ mod tests { unicode: false, has_indices: false, }, - Some(programs), + Some(programs.clone()), ); assert!( lookup(key, "g") @@ -401,6 +380,7 @@ mod tests { "precondition: the entry answers with its programs while they are alive" ); + drop(programs); drop(std_program); let hit = lookup(key, "g").expect("the entry itself survives"); assert!( diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index 11a6347e77..7d72ffff98 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -15,6 +15,18 @@ pub(super) fn string_payload(s: *const StringHeader) -> Vec { } } +pub(super) fn regex_is_built(re: *const RegExpHeader) -> bool { + !unsafe { (*re).programs_ptr.is_null() } +} + +pub(super) fn regex_has_fancy_program(re: *const RegExpHeader) -> bool { + regex_is_built(re) && unsafe { (*(*re).programs_ptr).fancy.is_some() } +} + +pub(super) fn regex_has_repeat_program(re: *const RegExpHeader) -> bool { + regex_is_built(re) && unsafe { (*(*re).programs_ptr).repeat.is_some() } +} + #[test] fn regexp_has_dedicated_gc_kind_and_is_not_a_shaped_object() { let _lock = crate::gc::global_side_table_test_lock(); @@ -32,7 +44,17 @@ fn regexp_has_dedicated_gc_kind_and_is_not_a_shaped_object() { } #[test] -fn malloc_finalize_clears_regexp_address_owned_tables() { +#[cfg(target_pointer_width = "64")] +fn regexp_header_is_one_56_byte_per_object_record() { + assert_eq!( + std::mem::size_of::(), + 56, + "the three per-program matcher pointers must stay collapsed into one handle" + ); +} + +#[test] +fn malloc_finalize_clears_regexp_address_owned_state() { let _lock = crate::gc::global_side_table_test_lock(); let scope = crate::gc::RuntimeHandleScope::new(); let pattern = scope.root_string_ptr(make_string("finalize")); @@ -42,7 +64,6 @@ fn malloc_finalize_clears_regexp_address_owned_tables() { }); let addr = re as usize; assert!(test_regex_pointer_entry_exists(addr)); - assert!(test_regex_source_entry_exists(addr)); crate::object::exotic_expando::test_seed_exotic_expando_entry( addr, "owned", @@ -55,7 +76,6 @@ fn malloc_finalize_clears_regexp_address_owned_tables() { } assert!(!test_regex_pointer_entry_exists(addr)); - assert!(!test_regex_source_entry_exists(addr)); assert!(!crate::object::exotic_expando::test_exotic_expando_entry_exists(addr)); } @@ -78,10 +98,10 @@ fn regexp_finalize_releases_all_header_owned_programs() { re } - // Every compiled header owns the standard-engine program, including the - // never-match placeholder used by fancy-regex patterns. + // Every compiled header owns one shared program bundle, including the + // never-match placeholder and any fallback matcher. let standard = compile(r"needle\d+", "needle42"); - let standard_raw = unsafe { (*standard).regex_ptr as *const Regex }; + let standard_raw = unsafe { (*standard).programs_ptr }; assert!(!standard_raw.is_null()); let standard_observer = clone_raw_arc(standard_raw); let standard_before = std::sync::Arc::strong_count(&standard_observer); @@ -95,11 +115,11 @@ fn regexp_finalize_releases_all_header_owned_programs() { std::sync::Arc::strong_count(&standard_observer) + 1, standard_before ); - assert!(unsafe { (*standard).regex_ptr.is_null() }); + assert!(!regex_is_built(standard)); let fancy = compile(r"(?<=pre)\d+", "pre77"); - let fancy_raw = unsafe { (*fancy).fancy_ptr as *const fancy_regex::Regex }; - assert!(!fancy_raw.is_null()); + let fancy_raw = unsafe { (*fancy).programs_ptr }; + assert!(regex_has_fancy_program(fancy)); let fancy_observer = clone_raw_arc(fancy_raw); let fancy_before = std::sync::Arc::strong_count(&fancy_observer); unsafe { @@ -109,12 +129,11 @@ fn regexp_finalize_releases_all_header_owned_programs() { std::sync::Arc::strong_count(&fancy_observer) + 1, fancy_before ); - assert!(unsafe { (*fancy).fancy_ptr.is_null() }); + assert!(!regex_is_built(fancy)); let repeat = compile(r"(a?b??)*", "ab"); - let repeat_raw = - unsafe { (*repeat).repeat_matcher_ptr as *const repeat_matcher::RepeatMatcherRegex }; - assert!(!repeat_raw.is_null()); + let repeat_raw = unsafe { (*repeat).programs_ptr }; + assert!(regex_has_repeat_program(repeat)); let repeat_observer = clone_raw_arc(repeat_raw); let repeat_before = std::sync::Arc::strong_count(&repeat_observer); unsafe { @@ -125,7 +144,7 @@ fn regexp_finalize_releases_all_header_owned_programs() { } let repeat_after = std::sync::Arc::strong_count(&repeat_observer); assert_eq!(repeat_after + 1, repeat_before); - assert!(unsafe { (*repeat).repeat_matcher_ptr.is_null() }); + assert!(!regex_is_built(repeat)); // Arena overflow cleanup and finalization can overlap. A second finalizer // must observe null pointers rather than release an owned reference twice. @@ -953,7 +972,7 @@ fn regex_cache_capped_and_prior_headers_survive_eviction() { assert!( js_regexp_test(fancy, make_string("pre77")) != 0, "fancy-fallback header must keep matching after cache eviction \ - (header-resident fancy_ptr, not the cleared FANCY_CACHE)" + (header-resident program set, not the cleared FANCY_CACHE)" ); assert!( js_regexp_test(fancy, make_string("nope77")) == 0, diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index ec6b577f66..c6ba025437 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -2,7 +2,10 @@ //! A sibling child of `regex`, so `use super::*` resolves exactly as it does //! in `tests.rs`; the shared fixtures come from there. -use super::tests::{make_string, match_capture_text, string_payload}; +use super::tests::{ + make_string, match_capture_text, regex_has_fancy_program, regex_has_repeat_program, + regex_is_built, string_payload, +}; use super::*; #[test] @@ -121,7 +124,7 @@ fn syntax_check_agrees_with_full_build() { /// fixture whose 200 literals cost 73 ms to construct before and ~0 after. A /// regression here (something re-introducing an eager build) would not fail any /// behavioural test, only make every program slower, so assert the state -/// directly: `regex_ptr` is the built/not-built flag. +/// directly: `programs_ptr` is the built/not-built flag. #[test] fn construction_defers_the_program_build_until_first_use() { let re = js_regexp_new( @@ -129,7 +132,7 @@ fn construction_defers_the_program_build_until_first_use() { make_string("i"), ); assert!( - unsafe { (*re).regex_ptr.is_null() }, + !regex_is_built(re), "constructing a RegExp must not build its program" ); // Everything observable without matching stays available. @@ -140,13 +143,13 @@ fn construction_defers_the_program_build_until_first_use() { assert_eq!(string_payload(js_regexp_get_flags(re)), b"i".to_vec()); assert!(unsafe { (*re).case_insensitive }); assert!( - unsafe { (*re).regex_ptr.is_null() }, + !regex_is_built(re), "reading .source/.flags must not build the program either" ); assert!(js_regexp_test(re, make_string("XFOO12")) != 0); assert!( - !unsafe { (*re).regex_ptr.is_null() }, + regex_is_built(re), "the first match must build and install the program" ); } @@ -217,19 +220,19 @@ fn regexp_source_round_trips_wtf8_lone_surrogates_from_the_header() { #[test] fn deferred_build_installs_the_fancy_and_repeat_matcher_fallbacks() { let fancy = js_regexp_new(make_string(r"(?<=pre)\d+"), make_string("")); - assert!(unsafe { (*fancy).fancy_ptr.is_null() }); + assert!(!regex_is_built(fancy)); assert!(js_regexp_test(fancy, make_string("pre77")) != 0); assert!( - !unsafe { (*fancy).fancy_ptr.is_null() }, + regex_has_fancy_program(fancy), "first use must install the fancy-regex fallback" ); assert!(js_regexp_test(fancy, make_string("nope77")) == 0); let repeat = js_regexp_new(make_string(r"(a?b??)*"), make_string("")); - assert!(unsafe { (*repeat).repeat_matcher_ptr.is_null() }); + assert!(!regex_is_built(repeat)); assert!(js_regexp_test(repeat, make_string("ab")) != 0); assert!( - !unsafe { (*repeat).repeat_matcher_ptr.is_null() }, + regex_has_repeat_program(repeat), "first use must install the ECMAScript RepeatMatcher" ); } @@ -669,10 +672,7 @@ fn site_cache_reconstruction_is_born_built() { let _lock = crate::gc::global_side_table_test_lock(); site_cache::test_reset(); let re1 = js_regexp_new(make_string("born[0-9]+built"), make_string("g")); - assert!( - unsafe { (*re1).regex_ptr.is_null() }, - "construction stays lazy" - ); + assert!(!regex_is_built(re1), "construction stays lazy"); assert_eq!( site_cache::test_has_programs("born[0-9]+built", "g"), Some(false), @@ -686,18 +686,20 @@ fn site_cache_reconstruction_is_born_built() { ); let re2 = js_regexp_new(make_string("born[0-9]+built"), make_string("g")); assert!( - !unsafe { (*re2).regex_ptr.is_null() }, + regex_is_built(re2), "the second construction installs the programs eagerly" ); assert!( - std::ptr::eq(unsafe { (*re1).regex_ptr }, unsafe { (*re2).regex_ptr }), + std::ptr::eq(unsafe { (*re1).programs_ptr }, unsafe { + (*re2).programs_ptr + }), "both headers share one compiled program" ); assert_eq!(js_regexp_test(re2, make_string("born7built")), 1); assert_eq!(js_regexp_test(re2, make_string("nothing")), 0); // Different flags are a different entry. let re3 = js_regexp_new(make_string("born[0-9]+built"), make_string("i")); - assert!(unsafe { (*re3).regex_ptr.is_null() }); + assert!(!regex_is_built(re3)); } /// `test` on a global/sticky receiver advances `lastIndex` exactly like @@ -818,9 +820,9 @@ fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { unsafe { lazy::ensure_regex_compiled(cold); assert!( - !(*cold).fancy_ptr.is_null(), + regex_has_fancy_program(cold), "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 fancy program here is memoized by site_cache::install_programs and makes \ the breakage permanent for this literal" ); } @@ -1064,7 +1066,7 @@ fn a_dynamic_construction_records_nothing_in_the_site_table() { /// A site hit must be born built: the second construction at a site whose /// first header has already executed installs the compiled programs eagerly, -/// so `regex_ptr` is non-null before any match runs. +/// so `programs_ptr` is non-null before any match runs. /// /// This is what makes the fast path complete — a hit that skipped the content /// cache but arrived unbuilt would push the pattern's hash back onto the first @@ -1077,19 +1079,19 @@ fn a_site_hit_after_the_first_execution_is_born_built() { let first = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); assert!( - unsafe { (*first).regex_ptr }.is_null(), + !regex_is_built(first), "construction must not build the program (that is #5777's deferred build)" ); assert!(js_regexp_test(first, make_string("boorn")) != 0); assert!( - !unsafe { (*first).regex_ptr }.is_null(), + regex_is_built(first), "the first execution installs the programs" ); // Second construction at the SAME site. let second = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); assert!( - !unsafe { (*second).regex_ptr }.is_null(), + regex_is_built(second), "a site hit must install the programs the site already compiled, so the header is born \ built and the first match pays no lookup" ); From 7a44e5948a2e51dca5f6de9eb76d639bf34de811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:23:56 +0200 Subject: [PATCH 3/8] perf(regex): tag the selected matcher on each header --- crates/perry-runtime/src/regex.rs | 53 +++++++++++++++---- crates/perry-runtime/src/regex/compile.rs | 2 + crates/perry-runtime/src/regex/lazy.rs | 1 + crates/perry-runtime/src/regex/site_cache.rs | 12 +++++ crates/perry-runtime/src/regex/tests_part2.rs | 32 +++++++++++ 5 files changed, 89 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 527e0c99b4..1350926d40 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -392,6 +392,7 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * (*ptr).dot_all = flags.contains('s'); (*ptr).unicode = flags.contains('u') || flags.contains('v'); (*ptr).has_indices = flags.contains('d'); + (*ptr).matcher_kind = MatcherKind::Unbuilt; (*ptr).last_index = crate::value::JSValue::number(0.0).bits(); (*ptr).magic = REGEXP_MAGIC; @@ -488,6 +489,15 @@ mod compile_cache; #[cfg(feature = "regex-engine")] pub(crate) use compile_cache::*; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[repr(u8)] +pub(super) enum MatcherKind { + Unbuilt, + Standard, + Fancy, + Repeat, +} + /// Header for heap-allocated RegExp objects #[repr(C)] pub struct RegExpHeader { @@ -512,6 +522,9 @@ pub struct RegExpHeader { pub dot_all: bool, pub unicode: bool, pub has_indices: bool, + /// Selected engine after the first build. This occupies the byte that was + /// padding before `last_index`, so it does not grow the 56-byte header. + matcher_kind: MatcherKind, /// `lastIndex` is a writable data property holding an *arbitrary* JSValue /// (spec: `Set(R, "lastIndex", v)` with no coercion on write). Stored as the /// raw NaN-boxed bits; `exec`/`test` apply `ToLength` on read to derive the @@ -531,8 +544,8 @@ pub struct RegExpHeader { /// string pattern → never matches → get-intrinsic's `stringToPath` returns /// `[]` → `intrinsic %% does not exist!` → express adapter load `exit(1)`. /// - /// Storing the marker (and the fancy-regex Arc) ON the heap header makes - /// identity + fancy-fallback resolution independent of WHICH runtime copy's + /// Storing the marker and program-set handle ON the heap header makes + /// identity + fallback resolution independent of WHICH runtime copy's /// thread-locals are live. Set to `REGEXP_MAGIC` by `js_regexp_new`. pub magic: u64, /// #6759 phase 1 (header unification): per-object metadata record, or @@ -1206,6 +1219,7 @@ fn js_regexp_new_impl( (*ptr).dot_all = dot_all; (*ptr).unicode = unicode; (*ptr).has_indices = has_indices; + (*ptr).matcher_kind = MatcherKind::Unbuilt; (*ptr).last_index = crate::value::JSValue::number(0.0).bits(); // Wall 18: self-identifying marker so identity checks survive a // duplicate-runtime thread-local split. @@ -1214,6 +1228,7 @@ fn js_regexp_new_impl( // first execution of this text compiled. Install one owned reference; // null remains the sound not-built state. if let Some(programs) = programs { + (*ptr).matcher_kind = programs.matcher_kind(); (*ptr).programs_ptr = Arc::into_raw(programs); } @@ -1405,16 +1420,32 @@ pub(crate) fn regexp_test_str_bounded(re: *const RegExpHeader, hay: &str) -> Opt if crate::hot_diag::regex_on() { diag_note_op(re, crate::hot_diag::RegexOp::Test); } - if let Some(repeat_matcher) = lookup_repeat_matcher(re) { - return Some(repeat_matcher.regex.find(hay).is_some()); - } - if let Some(fre) = lookup_fancy_regex(re) { - return match fre.is_match(hay) { - Ok(v) => Some(v), - Err(_) => None, - }; + lazy::ensure_regex_compiled(re); + let programs = &*(*re).programs_ptr; + match (*re).matcher_kind { + MatcherKind::Repeat => { + let repeat = programs + .repeat + .as_ref() + .expect("repeat matcher tag must name a repeat program"); + Some(repeat.regex.find(hay).is_some()) + } + MatcherKind::Fancy => { + let fancy = programs + .fancy + .as_ref() + .expect("fancy matcher tag must name a fancy program"); + match fancy.is_match(hay) { + Ok(v) => Some(v), + Err(_) => None, + } + } + MatcherKind::Standard => Some(programs.std.is_match(hay)), + MatcherKind::Unbuilt => { + debug_assert!(false, "compiled header kept the unbuilt matcher tag"); + Some(programs.std.is_match(hay)) + } } - Some(lazy::header_std_regex(re).is_match(hay)) } } diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index c569ad8bb3..cb0aae6e43 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -166,6 +166,7 @@ pub extern "C" fn js_regexp_compile_value( .cloned() }); let programs = Arc::new(super::site_cache::Programs { std, fancy, repeat }); + let matcher_kind = programs.matcher_kind(); let programs_ptr = Arc::into_raw(programs); let (canonical_flags_ptr, _) = re_handle.across_mut::(|| js_string_from_str(flags_str)); @@ -176,6 +177,7 @@ pub extern "C" fn js_regexp_compile_value( }); unsafe { let old_programs_ptr = (*re).programs_ptr; + (*re).matcher_kind = matcher_kind; (*re).programs_ptr = programs_ptr; // Release the receiver's PREVIOUS owned references now that the new // ones are installed (recompiling the same pattern is fine: the fresh diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index e912e944cd..845f1fb20b 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -301,6 +301,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { super::site_cache::install_programs(&pattern, &flags, programs.clone()); unsafe { let re = re as *mut RegExpHeader; + (*re).matcher_kind = programs.matcher_kind(); (*re).programs_ptr = Arc::into_raw(programs); } } diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index 459ad1f1de..675fe657df 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -47,6 +47,18 @@ pub(super) struct Programs { pub(super) repeat: Option>, } +impl Programs { + pub(super) fn matcher_kind(&self) -> super::MatcherKind { + if self.repeat.is_some() { + super::MatcherKind::Repeat + } else if self.fancy.is_some() { + super::MatcherKind::Fancy + } else { + super::MatcherKind::Standard + } + } +} + /// What a construction gets back on a hit. pub(super) struct Hit { pub(super) pattern: Arc, diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index c6ba025437..bf801c218f 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -237,6 +237,38 @@ fn deferred_build_installs_the_fancy_and_repeat_matcher_fallbacks() { ); } +/// Sabotage for the bounded Segmenter lane: this pattern's standard program +/// is the never-match placeholder, so a wrong `Standard` tag returns false. +/// Exercise all three installation routes that must publish the tag beside the +/// program handle: lazy build, born-built cache hit, and `compile`. +#[test] +fn bounded_test_matcher_tag_routes_fancy_patterns_to_fancy_regex() { + let _lock = crate::gc::global_side_table_test_lock(); + site_cache::test_reset(); + let pattern = r"(?<=left)right"; + + let cold = js_regexp_new(make_string(pattern), make_string("")); + assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Unbuilt); + assert_eq!(regexp_test_str_bounded(cold, "leftright"), Some(true)); + assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Fancy); + + let born_built = js_regexp_new(make_string(pattern), make_string("")); + assert!(regex_is_built(born_built)); + assert_eq!(unsafe { (*born_built).matcher_kind }, MatcherKind::Fancy); + assert_eq!( + regexp_test_str_bounded(born_built, "leftwrong"), + Some(false) + ); + + let compiled = js_regexp_new(make_string("plain"), make_string("")); + let pattern_value = crate::value::js_nanbox_string(make_string(pattern) as i64); + let flags_value = crate::value::js_nanbox_string(make_string("") as i64); + let result = js_regexp_compile_value(compiled, pattern_value, flags_value); + let compiled = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); + assert_eq!(unsafe { (*compiled).matcher_kind }, MatcherKind::Fancy); + assert_eq!(regexp_test_str_bounded(compiled, "leftright"), Some(true)); +} + /// Two evaluations of the same pattern are still distinct objects with /// independent `lastIndex`, and deferring the build does not let them share a /// header (ECMA-262 requires a fresh object per evaluation — the same From 6ea7ad9ebe3a01caa56e8dc33e10afd3d574f585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:27:18 +0200 Subject: [PATCH 4/8] test(regex): isolate WTF-8 source from matcher parsing --- crates/perry-runtime/src/regex/tests_part2.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index bf801c218f..65086f719f 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -209,7 +209,11 @@ fn regexp_compile_replaces_the_header_source_and_flags() { #[test] fn regexp_source_round_trips_wtf8_lone_surrogates_from_the_header() { let lone_high = [b'a', 0xED, 0xA0, 0x80, b'/', b'b']; - let re = js_regexp_new(make_wtf8(&lone_high), make_string("")); + let re = js_regexp_new(make_string("placeholder"), make_string("")); + let pattern = make_wtf8(&lone_high); + unsafe { + (*re).pattern_ptr = pattern; + } let expected = [b'a', 0xED, 0xA0, 0x80, b'\\', b'/', b'b']; assert_eq!(string_payload(js_regexp_get_source(re)), expected); } From c217a231c0e0133885ce105aa66de313f6c70be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:36:31 +0200 Subject: [PATCH 5/8] refactor(regex): split header properties and tests --- crates/perry-runtime/src/regex.rs | 80 +---- crates/perry-runtime/src/regex/properties.rs | 78 +++++ .../perry-runtime/src/regex/tests_header.rs | 281 ++++++++++++++++++ crates/perry-runtime/src/regex/tests_part2.rs | 254 ---------------- scripts/gc_runtime_root_holders.json | 7 - 5 files changed, 366 insertions(+), 334 deletions(-) create mode 100644 crates/perry-runtime/src/regex/properties.rs create mode 100644 crates/perry-runtime/src/regex/tests_header.rs diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 1350926d40..2c2614473b 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -62,6 +62,7 @@ mod grammar; mod lazy; #[cfg(feature = "regex-engine")] mod match_all; +mod properties; #[cfg(feature = "regex-engine")] mod repeat_matcher; #[cfg(feature = "regex-engine")] @@ -83,7 +84,6 @@ mod utf16; use class_range_validate::has_out_of_order_double_dash_class_range; #[cfg(feature = "regex-engine")] pub use compile::js_regexp_compile_value; -use escape::escape_regexp_source; pub use escape::js_regexp_escape; #[cfg(feature = "regex-engine")] use exec_array::{ @@ -105,6 +105,10 @@ pub(crate) use match_all::dispatch_regexp_string_iterator_method_builtin; pub use match_all::{ dispatch_regexp_string_iterator_method, js_string_match_all, js_string_match_all_value, }; +pub use properties::{ + js_regexp_empty_source, js_regexp_get_flags, js_regexp_get_last_index, js_regexp_get_source, + js_regexp_set_last_index, js_regexp_to_string, +}; /// Class id for `RegExp String Iterator` exotic objects. Referenced by the /// always-linked iterator-prototype dispatch, so it stays ungated even when @@ -1742,79 +1746,9 @@ pub(crate) fn test_last_exec_groups() -> usize { LAST_EXEC_GROUPS.with(|g| *g.borrow() as usize) } -/// Get regex.source — returns the pattern string -#[no_mangle] -pub extern "C" fn js_regexp_get_source(re: *const RegExpHeader) -> *mut StringHeader { - if !is_valid_regex_ptr(re) { - return js_string_from_str("(?:)"); - } - unsafe { - if is_valid_ptr((*re).pattern_ptr) { - let escaped = escape_regexp_source(string_as_bytes((*re).pattern_ptr)); - crate::string::js_string_from_wtf8_bytes(escaped.as_ptr(), escaped.len() as u32) - } else { - js_string_from_str("(?:)") - } - } -} - -/// `RegExp.prototype.source` for the prototype object itself (no -/// `[[OriginalSource]]`) returns the canonical empty source `"(?:)"`. -#[no_mangle] -pub extern "C" fn js_regexp_empty_source() -> *mut StringHeader { - js_string_from_str("(?:)") -} - -/// Get regex.flags — returns the flags string -#[no_mangle] -pub extern "C" fn js_regexp_get_flags(re: *const RegExpHeader) -> *mut StringHeader { - if !is_valid_regex_ptr(re) { - return js_string_from_str(""); - } - unsafe { - if is_valid_ptr((*re).flags_ptr) { - let flags_str = string_as_str((*re).flags_ptr); - js_string_from_str(flags_str) - } else { - js_string_from_str("") - } - } -} - -/// `RegExp.prototype.toString()` — `/source/flags`. Used by both the -/// `regex.toString()` method dispatch and ToString coercion (`String(re)`, -/// template literals). Node never produces `"[object Object]"` for a RegExp. -#[no_mangle] -pub extern "C" fn js_regexp_to_string(re: *const RegExpHeader) -> *mut StringHeader { - let src = js_regexp_get_source(re); - let flg = js_regexp_get_flags(re); - let out = format!("/{}/{}", string_as_str(src), string_as_str(flg)); - js_string_from_str(&out) -} - -/// Get regex.lastIndex — returns the stored value (NaN-boxed JSValue bits as -/// f64). Usually a number, but `re.lastIndex = obj` round-trips the object. -#[no_mangle] -pub extern "C" fn js_regexp_get_last_index(re: *const RegExpHeader) -> f64 { - if !is_valid_regex_ptr(re) { - return 0.0; - } - unsafe { f64::from_bits((*re).last_index) } -} - -/// Set regex.lastIndex — stores the value verbatim (no coercion on write, per -/// spec `Set(R, "lastIndex", v)`). -#[no_mangle] -pub extern "C" fn js_regexp_set_last_index(re: *mut RegExpHeader, value: f64) { - if !is_valid_regex_ptr(re) { - return; - } - unsafe { - (*re).last_index = value.to_bits(); - } -} - #[cfg(all(test, feature = "regex-engine"))] mod tests; #[cfg(all(test, feature = "regex-engine"))] mod tests_part2; +#[cfg(all(test, feature = "regex-engine"))] +mod tests_header; diff --git a/crates/perry-runtime/src/regex/properties.rs b/crates/perry-runtime/src/regex/properties.rs new file mode 100644 index 0000000000..e4f0a53f3e --- /dev/null +++ b/crates/perry-runtime/src/regex/properties.rs @@ -0,0 +1,78 @@ +//! Observable RegExp data properties and stringification. + +use super::escape::escape_regexp_source; +use super::RegExpHeader; +use super::{is_valid_ptr, is_valid_regex_ptr, js_string_from_str, string_as_bytes, string_as_str}; +use crate::string::StringHeader; + +/// Get regex.source — returns the pattern string. +#[no_mangle] +pub extern "C" fn js_regexp_get_source(re: *const RegExpHeader) -> *mut StringHeader { + if !is_valid_regex_ptr(re) { + return js_string_from_str("(?:)"); + } + unsafe { + if is_valid_ptr((*re).pattern_ptr) { + let escaped = escape_regexp_source(string_as_bytes((*re).pattern_ptr)); + crate::string::js_string_from_wtf8_bytes(escaped.as_ptr(), escaped.len() as u32) + } else { + js_string_from_str("(?:)") + } + } +} + +/// `RegExp.prototype.source` for the prototype object itself (no +/// `[[OriginalSource]]`) returns the canonical empty source `"(?:)"`. +#[no_mangle] +pub extern "C" fn js_regexp_empty_source() -> *mut StringHeader { + js_string_from_str("(?:)") +} + +/// Get regex.flags — returns the flags string. +#[no_mangle] +pub extern "C" fn js_regexp_get_flags(re: *const RegExpHeader) -> *mut StringHeader { + if !is_valid_regex_ptr(re) { + return js_string_from_str(""); + } + unsafe { + if is_valid_ptr((*re).flags_ptr) { + let flags_str = string_as_str((*re).flags_ptr); + js_string_from_str(flags_str) + } else { + js_string_from_str("") + } + } +} + +/// `RegExp.prototype.toString()` — `/source/flags`. Used by both the +/// `regex.toString()` method dispatch and ToString coercion (`String(re)`, +/// template literals). Node never produces `"[object Object]"` for a RegExp. +#[no_mangle] +pub extern "C" fn js_regexp_to_string(re: *const RegExpHeader) -> *mut StringHeader { + let src = js_regexp_get_source(re); + let flg = js_regexp_get_flags(re); + let out = format!("/{}/{}", string_as_str(src), string_as_str(flg)); + js_string_from_str(&out) +} + +/// Get regex.lastIndex — returns the stored value (NaN-boxed JSValue bits as +/// f64). Usually a number, but `re.lastIndex = obj` round-trips the object. +#[no_mangle] +pub extern "C" fn js_regexp_get_last_index(re: *const RegExpHeader) -> f64 { + if !is_valid_regex_ptr(re) { + return 0.0; + } + unsafe { f64::from_bits((*re).last_index) } +} + +/// Set regex.lastIndex — stores the value verbatim (no coercion on write, per +/// spec `Set(R, "lastIndex", v)`). +#[no_mangle] +pub extern "C" fn js_regexp_set_last_index(re: *mut RegExpHeader, value: f64) { + if !is_valid_regex_ptr(re) { + return; + } + unsafe { + (*re).last_index = value.to_bits(); + } +} diff --git a/crates/perry-runtime/src/regex/tests_header.rs b/crates/perry-runtime/src/regex/tests_header.rs new file mode 100644 index 0000000000..dcf8eacc17 --- /dev/null +++ b/crates/perry-runtime/src/regex/tests_header.rs @@ -0,0 +1,281 @@ +use super::*; + +fn make_string(s: &str) -> *mut StringHeader { + crate::string::js_string_from_bytes(s.as_ptr(), s.len() as u32) +} + +fn make_wtf8(bytes: &[u8]) -> *mut StringHeader { + crate::string::js_string_from_wtf8_bytes(bytes.as_ptr(), bytes.len() as u32) +} + +fn string_payload(s: *const StringHeader) -> Vec { + unsafe { + std::slice::from_raw_parts(crate::string::string_data(s), (*s).byte_len as usize).to_vec() + } +} + +fn regex_is_built(re: *const RegExpHeader) -> bool { + !unsafe { (*re).programs_ptr.is_null() } +} + +fn regex_has_fancy_program(re: *const RegExpHeader) -> bool { + regex_is_built(re) && unsafe { (*(*re).programs_ptr).fancy.is_some() } +} + +fn regex_has_repeat_program(re: *const RegExpHeader) -> bool { + regex_is_built(re) && unsafe { (*(*re).programs_ptr).repeat.is_some() } +} + +/// Construction must NOT build the automaton; the first operation that needs a +/// matcher must. +/// +/// This is the structural half of the perf fix — the wall-clock half is a +/// fixture whose 200 literals cost 73 ms to construct before and ~0 after. A +/// regression here (something re-introducing an eager build) would not fail any +/// behavioural test, only make every program slower, so assert the state +/// directly: `programs_ptr` is the built/not-built flag. +#[test] +fn construction_defers_the_program_build_until_first_use() { + let re = js_regexp_new( + make_string("[A-Za-z]+(?:foo|bar)[0-9]{1,4}"), + make_string("i"), + ); + assert!( + !regex_is_built(re), + "constructing a RegExp must not build its program" + ); + // Everything observable without matching stays available. + assert_eq!( + string_payload(js_regexp_get_source(re)), + b"[A-Za-z]+(?:foo|bar)[0-9]{1,4}".to_vec() + ); + assert_eq!(string_payload(js_regexp_get_flags(re)), b"i".to_vec()); + assert!(unsafe { (*re).case_insensitive }); + assert!( + !regex_is_built(re), + "reading .source/.flags must not build the program either" + ); + + assert!(js_regexp_test(re, make_string("XFOO12")) != 0); + assert!( + regex_is_built(re), + "the first match must build and install the program" + ); +} + +/// Removing the address-keyed source table must not turn the RegExp-pattern +/// constructor arm into an empty-pattern fallback. This calls the exported +/// constructor entry point, so deleting its direct header read fails both the +/// source and inherited-flags assertions. +#[test] +fn regexp_construct_reads_source_and_flags_from_the_pattern_header() { + let original = js_regexp_new(make_string("left/right"), make_string("ig")); + let pattern = crate::value::js_nanbox_pointer(original as i64); + let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); + + let copy = js_regexp_construct(pattern, undefined); + assert_ne!( + copy, original, + "construction must still allocate a fresh object" + ); + assert_eq!(string_payload(js_regexp_get_source(copy)), b"left\\/right"); + assert_eq!(string_payload(js_regexp_get_flags(copy)), b"gi"); + + let override_flags = crate::value::js_nanbox_string(make_string("m") as i64); + let overridden = js_regexp_construct(pattern, override_flags); + assert_eq!( + string_payload(js_regexp_get_source(overridden)), + b"left\\/right" + ); + assert_eq!(string_payload(js_regexp_get_flags(overridden)), b"m"); +} + +/// `RegExp.prototype.compile` rewrites the header in place. The source table +/// used to mask a stale header slot here; after its removal both observable +/// strings must come from the newly stored, traced edges. +#[test] +fn regexp_compile_replaces_the_header_source_and_flags() { + let receiver = js_regexp_new(make_string("old"), make_string("m")); + js_regexp_set_last_index(receiver, 9.0); + let pattern = crate::value::js_nanbox_string(make_string("new/source") as i64); + let flags = crate::value::js_nanbox_string(make_string("ig") as i64); + let result = js_regexp_compile_value(receiver, pattern, flags); + let receiver = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); + + assert_eq!( + string_payload(js_regexp_get_source(receiver)), + b"new\\/source" + ); + assert_eq!(string_payload(js_regexp_get_flags(receiver)), b"gi"); + assert_eq!(js_regexp_get_last_index(receiver), 0.0); + assert_eq!(js_regexp_test(receiver, make_string("NEW/source")), 1); +} + +/// Perry stores lone JavaScript surrogates as WTF-8. `.source` must copy those +/// exact bytes from the traced pattern slot; routing them through Rust's UTF-8 +/// scalar iterator either replaces the surrogate or invokes undefined +/// behaviour. +#[test] +fn regexp_source_round_trips_wtf8_lone_surrogates_from_the_header() { + let lone_high = [b'a', 0xED, 0xA0, 0x80, b'/', b'b']; + let re = js_regexp_new(make_string("placeholder"), make_string("")); + let pattern = make_wtf8(&lone_high); + unsafe { + (*re).pattern_ptr = pattern; + } + let expected = [b'a', 0xED, 0xA0, 0x80, b'\\', b'/', b'b']; + assert_eq!(string_payload(js_regexp_get_source(re)), expected); +} + +/// The deferred build installs the fancy-regex and RepeatMatcher programs too, +/// not just the linear one — they live on the same publish point, so a header +/// whose pattern needs one must still get it on first use. +#[test] +fn deferred_build_installs_the_fancy_and_repeat_matcher_fallbacks() { + let fancy = js_regexp_new(make_string(r"(?<=pre)\d+"), make_string("")); + assert!(!regex_is_built(fancy)); + assert!(js_regexp_test(fancy, make_string("pre77")) != 0); + assert!( + regex_has_fancy_program(fancy), + "first use must install the fancy-regex fallback" + ); + assert!(js_regexp_test(fancy, make_string("nope77")) == 0); + + let repeat = js_regexp_new(make_string(r"(a?b??)*"), make_string("")); + assert!(!regex_is_built(repeat)); + assert!(js_regexp_test(repeat, make_string("ab")) != 0); + assert!( + regex_has_repeat_program(repeat), + "first use must install the ECMAScript RepeatMatcher" + ); +} + +/// Sabotage for the bounded Segmenter lane: this pattern's standard program +/// is the never-match placeholder, so a wrong `Standard` tag returns false. +/// Exercise all three installation routes that must publish the tag beside the +/// program handle: lazy build, born-built cache hit, and `compile`. +#[test] +fn bounded_test_matcher_tag_routes_fancy_patterns_to_fancy_regex() { + let _lock = crate::gc::global_side_table_test_lock(); + site_cache::test_reset(); + let pattern = r"(?<=left)right"; + + let cold = js_regexp_new(make_string(pattern), make_string("")); + assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Unbuilt); + assert_eq!(regexp_test_str_bounded(cold, "leftright"), Some(true)); + assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Fancy); + + let born_built = js_regexp_new(make_string(pattern), make_string("")); + assert!(regex_is_built(born_built)); + assert_eq!(unsafe { (*born_built).matcher_kind }, MatcherKind::Fancy); + assert_eq!( + regexp_test_str_bounded(born_built, "leftwrong"), + Some(false) + ); + + let compiled = js_regexp_new(make_string("plain"), make_string("")); + let pattern_value = crate::value::js_nanbox_string(make_string(pattern) as i64); + let flags_value = crate::value::js_nanbox_string(make_string("") as i64); + let result = js_regexp_compile_value(compiled, pattern_value, flags_value); + let compiled = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); + assert_eq!(unsafe { (*compiled).matcher_kind }, MatcherKind::Fancy); + assert_eq!(regexp_test_str_bounded(compiled, "leftright"), Some(true)); +} + +/// Two evaluations of the same pattern are still distinct objects with +/// independent `lastIndex`, and deferring the build does not let them share a +/// header (ECMA-262 requires a fresh object per evaluation — the same +/// invariant the closure-literal singleton fix restored for functions). +#[test] +fn deferred_build_keeps_per_object_identity_and_last_index() { + let a = js_regexp_new(make_string("x"), make_string("g")); + let b = js_regexp_new(make_string("x"), make_string("g")); + assert_ne!( + a as usize, b as usize, + "each evaluation is a distinct object" + ); + assert!(!js_regexp_exec(a, make_string("xx")).is_null()); + assert_eq!(regex_last_index_offset(a), 1); + assert_eq!( + regex_last_index_offset(b), + 0, + "a sibling regex must not inherit lastIndex through the shared program" + ); +} + +/// The validated-pattern set is capped like the program caches: it holds owned +/// pattern text (`emoji-regex` is ~12,807 chars) and is fed by `new +/// RegExp(userInput)`, so an uncapped one would be the same attacker-driven +/// growth the compiled-program caches were capped for. +#[test] +fn validated_pattern_set_is_capped() { + for i in 0..(REGEX_CACHE_MAX_ENTRIES * 2 + 10) { + lazy::mark_pattern_validated(&format!("validfill{i}[a-z]+"), ""); + } + let len = VALIDATED_PATTERNS.with(|c| c.borrow().len()); + assert!( + len <= REGEX_CACHE_MAX_ENTRIES, + "VALIDATED_PATTERNS must stay capped at {REGEX_CACHE_MAX_ENTRIES} entries, got {len}" + ); +} + +/// The `[\s\S]` → `(?s:.)` rewrite must not move a single match result. +/// +/// The rewrite exists purely to dodge a 1.1-million-iteration case fold in +/// `regex_syntax` (see `grammar::push_any_char`), so the only thing that may +/// change is how long construction takes. Everything a program can observe — +/// what matches, what a capture group holds, which group number it is, and +/// that the NEGATED forms still match nothing — is pinned here, because a +/// silently widened character class produces no error anywhere: only a wrong +/// answer, on inputs a syntax test never looks at. +#[test] +fn any_char_rewrite_preserves_match_behaviour() { + // Matches every code point, newlines included, with and without `i`. + for pattern in ["[\\s\\S]", "[^]", "[\\d\\D]", "[\\w\\W]", "[\\S\\s]"] { + for flags in ["", "i", "u", "iu", "m"] { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + for subject in ["a", "\n", " ", "\u{1F600}", "Ω", "\r"] { + assert!( + js_regexp_test(re, make_string(subject)) != 0, + "/{pattern}/{flags} must match {subject:?}" + ); + } + } + } + + // The negated forms are the exact opposite and must still match NOTHING. + for pattern in ["[^\\s\\S]", "[^\\w\\W]", "[]"] { + let re = js_regexp_new(make_string(pattern), make_string("i")); + for subject in ["a", "\n", "Ω"] { + assert!( + js_regexp_test(re, make_string(subject)) == 0, + "/{pattern}/i must not match {subject:?}" + ); + } + } + + // A class that is NOT a complementary pair keeps its narrow meaning. + let narrow = js_regexp_new(make_string("[\\d\\s]"), make_string("i")); + assert!(js_regexp_test(narrow, make_string("7")) != 0); + assert!(js_regexp_test(narrow, make_string("a")) == 0); + + // The rewrite emits a NON-capturing group, so group numbering is + // unchanged: `$1` is still `b`, not the any-char. + let re = js_regexp_new(make_string("a[\\s\\S](b)"), make_string("")); + let m = js_regexp_exec(re, make_string("a\nb")); + assert!(!m.is_null(), "a[\\s\\S](b) must match \"a\\nb\""); + + // Quantifiers still bind to the any-char, lazily and greedily. + let lazy = js_regexp_new(make_string("([\\s\\S]*?)"), make_string("i")); + assert!(js_regexp_test(lazy, make_string("one\ntwo")) != 0); + let greedy = js_regexp_new(make_string("^[\\s\\S]{3}$"), make_string("")); + assert!(js_regexp_test(greedy, make_string("a\nb")) != 0); + assert!(js_regexp_test(greedy, make_string("a\nbc")) == 0); + + // `.source` still reports what the author wrote, not the translation. + let re = js_regexp_new(make_string("[\\s\\S]+"), make_string("gi")); + assert_eq!( + string_payload(js_regexp_get_source(re)), + b"[\\s\\S]+".to_vec() + ); +} diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index 65086f719f..e8cda486be 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -117,260 +117,6 @@ fn syntax_check_agrees_with_full_build() { } } -/// Construction must NOT build the automaton; the first operation that needs a -/// matcher must. -/// -/// This is the structural half of the perf fix — the wall-clock half is a -/// fixture whose 200 literals cost 73 ms to construct before and ~0 after. A -/// regression here (something re-introducing an eager build) would not fail any -/// behavioural test, only make every program slower, so assert the state -/// directly: `programs_ptr` is the built/not-built flag. -#[test] -fn construction_defers_the_program_build_until_first_use() { - let re = js_regexp_new( - make_string("[A-Za-z]+(?:foo|bar)[0-9]{1,4}"), - make_string("i"), - ); - assert!( - !regex_is_built(re), - "constructing a RegExp must not build its program" - ); - // Everything observable without matching stays available. - assert_eq!( - string_payload(js_regexp_get_source(re)), - b"[A-Za-z]+(?:foo|bar)[0-9]{1,4}".to_vec() - ); - assert_eq!(string_payload(js_regexp_get_flags(re)), b"i".to_vec()); - assert!(unsafe { (*re).case_insensitive }); - assert!( - !regex_is_built(re), - "reading .source/.flags must not build the program either" - ); - - assert!(js_regexp_test(re, make_string("XFOO12")) != 0); - assert!( - regex_is_built(re), - "the first match must build and install the program" - ); -} - -/// Removing the address-keyed source table must not turn the RegExp-pattern -/// constructor arm into an empty-pattern fallback. This calls the exported -/// constructor entry point, so deleting its direct header read fails both the -/// source and inherited-flags assertions. -#[test] -fn regexp_construct_reads_source_and_flags_from_the_pattern_header() { - let original = js_regexp_new(make_string("left/right"), make_string("ig")); - let pattern = crate::value::js_nanbox_pointer(original as i64); - let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); - - let copy = js_regexp_construct(pattern, undefined); - assert_ne!( - copy, original, - "construction must still allocate a fresh object" - ); - assert_eq!(string_payload(js_regexp_get_source(copy)), b"left\\/right"); - assert_eq!(string_payload(js_regexp_get_flags(copy)), b"gi"); - - let override_flags = crate::value::js_nanbox_string(make_string("m") as i64); - let overridden = js_regexp_construct(pattern, override_flags); - assert_eq!( - string_payload(js_regexp_get_source(overridden)), - b"left\\/right" - ); - assert_eq!(string_payload(js_regexp_get_flags(overridden)), b"m"); -} - -/// `RegExp.prototype.compile` rewrites the header in place. The source table -/// used to mask a stale header slot here; after its removal both observable -/// strings must come from the newly stored, traced edges. -#[test] -fn regexp_compile_replaces_the_header_source_and_flags() { - let receiver = js_regexp_new(make_string("old"), make_string("m")); - js_regexp_set_last_index(receiver, 9.0); - let pattern = crate::value::js_nanbox_string(make_string("new/source") as i64); - let flags = crate::value::js_nanbox_string(make_string("ig") as i64); - let result = js_regexp_compile_value(receiver, pattern, flags); - let receiver = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); - - assert_eq!( - string_payload(js_regexp_get_source(receiver)), - b"new\\/source" - ); - assert_eq!(string_payload(js_regexp_get_flags(receiver)), b"gi"); - assert_eq!(js_regexp_get_last_index(receiver), 0.0); - assert_eq!(js_regexp_test(receiver, make_string("NEW/source")), 1); -} - -/// Perry stores lone JavaScript surrogates as WTF-8. `.source` must copy those -/// exact bytes from the traced pattern slot; routing them through Rust's UTF-8 -/// scalar iterator either replaces the surrogate or invokes undefined -/// behaviour. -#[test] -fn regexp_source_round_trips_wtf8_lone_surrogates_from_the_header() { - let lone_high = [b'a', 0xED, 0xA0, 0x80, b'/', b'b']; - let re = js_regexp_new(make_string("placeholder"), make_string("")); - let pattern = make_wtf8(&lone_high); - unsafe { - (*re).pattern_ptr = pattern; - } - let expected = [b'a', 0xED, 0xA0, 0x80, b'\\', b'/', b'b']; - assert_eq!(string_payload(js_regexp_get_source(re)), expected); -} - -/// The deferred build installs the fancy-regex and RepeatMatcher programs too, -/// not just the linear one — they live on the same publish point, so a header -/// whose pattern needs one must still get it on first use. -#[test] -fn deferred_build_installs_the_fancy_and_repeat_matcher_fallbacks() { - let fancy = js_regexp_new(make_string(r"(?<=pre)\d+"), make_string("")); - assert!(!regex_is_built(fancy)); - assert!(js_regexp_test(fancy, make_string("pre77")) != 0); - assert!( - regex_has_fancy_program(fancy), - "first use must install the fancy-regex fallback" - ); - assert!(js_regexp_test(fancy, make_string("nope77")) == 0); - - let repeat = js_regexp_new(make_string(r"(a?b??)*"), make_string("")); - assert!(!regex_is_built(repeat)); - assert!(js_regexp_test(repeat, make_string("ab")) != 0); - assert!( - regex_has_repeat_program(repeat), - "first use must install the ECMAScript RepeatMatcher" - ); -} - -/// Sabotage for the bounded Segmenter lane: this pattern's standard program -/// is the never-match placeholder, so a wrong `Standard` tag returns false. -/// Exercise all three installation routes that must publish the tag beside the -/// program handle: lazy build, born-built cache hit, and `compile`. -#[test] -fn bounded_test_matcher_tag_routes_fancy_patterns_to_fancy_regex() { - let _lock = crate::gc::global_side_table_test_lock(); - site_cache::test_reset(); - let pattern = r"(?<=left)right"; - - let cold = js_regexp_new(make_string(pattern), make_string("")); - assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Unbuilt); - assert_eq!(regexp_test_str_bounded(cold, "leftright"), Some(true)); - assert_eq!(unsafe { (*cold).matcher_kind }, MatcherKind::Fancy); - - let born_built = js_regexp_new(make_string(pattern), make_string("")); - assert!(regex_is_built(born_built)); - assert_eq!(unsafe { (*born_built).matcher_kind }, MatcherKind::Fancy); - assert_eq!( - regexp_test_str_bounded(born_built, "leftwrong"), - Some(false) - ); - - let compiled = js_regexp_new(make_string("plain"), make_string("")); - let pattern_value = crate::value::js_nanbox_string(make_string(pattern) as i64); - let flags_value = crate::value::js_nanbox_string(make_string("") as i64); - let result = js_regexp_compile_value(compiled, pattern_value, flags_value); - let compiled = crate::value::JSValue::from_bits(result.to_bits()).as_pointer::(); - assert_eq!(unsafe { (*compiled).matcher_kind }, MatcherKind::Fancy); - assert_eq!(regexp_test_str_bounded(compiled, "leftright"), Some(true)); -} - -/// Two evaluations of the same pattern are still distinct objects with -/// independent `lastIndex`, and deferring the build does not let them share a -/// header (ECMA-262 requires a fresh object per evaluation — the same -/// invariant the closure-literal singleton fix restored for functions). -#[test] -fn deferred_build_keeps_per_object_identity_and_last_index() { - let a = js_regexp_new(make_string("x"), make_string("g")); - let b = js_regexp_new(make_string("x"), make_string("g")); - assert_ne!( - a as usize, b as usize, - "each evaluation is a distinct object" - ); - assert!(!js_regexp_exec(a, make_string("xx")).is_null()); - assert_eq!(regex_last_index_offset(a), 1); - assert_eq!( - regex_last_index_offset(b), - 0, - "a sibling regex must not inherit lastIndex through the shared program" - ); -} - -/// The validated-pattern set is capped like the program caches: it holds owned -/// pattern text (`emoji-regex` is ~12,807 chars) and is fed by `new -/// RegExp(userInput)`, so an uncapped one would be the same attacker-driven -/// growth the compiled-program caches were capped for. -#[test] -fn validated_pattern_set_is_capped() { - for i in 0..(REGEX_CACHE_MAX_ENTRIES * 2 + 10) { - lazy::mark_pattern_validated(&format!("validfill{i}[a-z]+"), ""); - } - let len = VALIDATED_PATTERNS.with(|c| c.borrow().len()); - assert!( - len <= REGEX_CACHE_MAX_ENTRIES, - "VALIDATED_PATTERNS must stay capped at {REGEX_CACHE_MAX_ENTRIES} entries, got {len}" - ); -} - -/// The `[\s\S]` → `(?s:.)` rewrite must not move a single match result. -/// -/// The rewrite exists purely to dodge a 1.1-million-iteration case fold in -/// `regex_syntax` (see `grammar::push_any_char`), so the only thing that may -/// change is how long construction takes. Everything a program can observe — -/// what matches, what a capture group holds, which group number it is, and -/// that the NEGATED forms still match nothing — is pinned here, because a -/// silently widened character class produces no error anywhere: only a wrong -/// answer, on inputs a syntax test never looks at. -#[test] -fn any_char_rewrite_preserves_match_behaviour() { - // Matches every code point, newlines included, with and without `i`. - for pattern in ["[\\s\\S]", "[^]", "[\\d\\D]", "[\\w\\W]", "[\\S\\s]"] { - for flags in ["", "i", "u", "iu", "m"] { - let re = js_regexp_new(make_string(pattern), make_string(flags)); - for subject in ["a", "\n", " ", "\u{1F600}", "Ω", "\r"] { - assert!( - js_regexp_test(re, make_string(subject)) != 0, - "/{pattern}/{flags} must match {subject:?}" - ); - } - } - } - - // The negated forms are the exact opposite and must still match NOTHING. - for pattern in ["[^\\s\\S]", "[^\\w\\W]", "[]"] { - let re = js_regexp_new(make_string(pattern), make_string("i")); - for subject in ["a", "\n", "Ω"] { - assert!( - js_regexp_test(re, make_string(subject)) == 0, - "/{pattern}/i must not match {subject:?}" - ); - } - } - - // A class that is NOT a complementary pair keeps its narrow meaning. - let narrow = js_regexp_new(make_string("[\\d\\s]"), make_string("i")); - assert!(js_regexp_test(narrow, make_string("7")) != 0); - assert!(js_regexp_test(narrow, make_string("a")) == 0); - - // The rewrite emits a NON-capturing group, so group numbering is - // unchanged: `$1` is still `b`, not the any-char. - let re = js_regexp_new(make_string("a[\\s\\S](b)"), make_string("")); - let m = js_regexp_exec(re, make_string("a\nb")); - assert!(!m.is_null(), "a[\\s\\S](b) must match \"a\\nb\""); - - // Quantifiers still bind to the any-char, lazily and greedily. - let lazy = js_regexp_new(make_string("([\\s\\S]*?)"), make_string("i")); - assert!(js_regexp_test(lazy, make_string("one\ntwo")) != 0); - let greedy = js_regexp_new(make_string("^[\\s\\S]{3}$"), make_string("")); - assert!(js_regexp_test(greedy, make_string("a\nb")) != 0); - assert!(js_regexp_test(greedy, make_string("a\nbc")) == 0); - - // `.source` still reports what the author wrote, not the translation. - let re = js_regexp_new(make_string("[\\s\\S]+"), make_string("gi")); - assert_eq!( - string_payload(js_regexp_get_source(re)), - b"[\\s\\S]+".to_vec() - ); -} - /// #9305 fallout: the translator spells ECMAScript's ASCII `\b`/`\B` as /// `(?-iu:\b)`, which fancy-regex's parser rejects (`NonUnicodeUnsupported`). /// Any lookaround/backreference pattern containing a word boundary therefore diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index f1582b60c2..e3171ccd11 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -704,13 +704,6 @@ "scanner": "regex::regex_header_moved_for_gc / regex_header_clear_dead_for_gc (regex.rs), the RegExp move/death hooks the copying minor and sweep invoke", "why": "Address-KEYED owner set, not a root: the key is rekeyed when the RegExpHeader moves and removed when it dies; it never keeps the header alive. Reached from GC hooks, not from a registered scanner, so the walk misses it." }, - { - "file": "crates/perry-runtime/src/regex.rs", - "name": "REGEX_SOURCE_TABLE", - "verdict": "covered_elsewhere", - "scanner": "regex::regex_header_moved_for_gc / regex_header_clear_dead_for_gc (regex.rs)", - "why": "Address-KEYED owner table (owned String copies of pattern/flags); rekeyed on move, cleared on death, same hooks as REGEX_POINTERS." - }, { "file": "crates/perry-runtime/src/regex.rs", "name": "VALIDATED_PATTERNS", From 883d334a68f1159903922476090549e1097bd346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:39:57 +0200 Subject: [PATCH 6/8] fix(regex): retain canonical flags through allocation --- crates/perry-runtime/src/gc/types.rs | 6 +++--- crates/perry-runtime/src/regex.rs | 8 ++++---- crates/perry-runtime/src/regex/match_all.rs | 2 +- crates/perry-runtime/src/regex/replace_expand.rs | 2 +- crates/perry-runtime/src/regex/replace_expand_fancy.rs | 4 ++-- crates/perry-runtime/src/string/split.rs | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/perry-runtime/src/gc/types.rs b/crates/perry-runtime/src/gc/types.rs index 08f9832cd5..97129c6088 100644 --- a/crates/perry-runtime/src/gc/types.rs +++ b/crates/perry-runtime/src/gc/types.rs @@ -224,9 +224,9 @@ pub(crate) enum GcMoveHookKind { /// live on the Error's traced `ObjectMeta` edge and need no side-table /// rekeying. ErrorSideTables, - /// Rekey RegExp identity/source registries plus its exotic expando owner - /// entry. `GC_TYPE_REGEXP` is movable, and all three tables use the - /// payload address as their key. + /// Rekey the RegExp identity registry plus its exotic expando owner entry. + /// `GC_TYPE_REGEXP` is movable, and both tables use the payload address as + /// their key. RegExpSideTables, } diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 2c2614473b..3170cdd277 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -788,7 +788,7 @@ fn js_regexp_new_impl( // A `site_key` of 0 (every dynamic construction, and every runtime caller) // misses by construction and takes the content-keyed path below unchanged. let site_entry = site_key::lookup(site_key, raw_flags_str); - let (programs, bits, shared_flags_root) = match site_entry { + let (programs, bits, shared_flags_root, owned_flags) = match site_entry { Some(hit) => { // The site's own flags literal, so this is the same sharing // decision the first construction at this site made (#9819). @@ -822,7 +822,7 @@ fn js_regexp_new_impl( picked } }; - (programs, hit.bits, shared_flags_root) + (programs, hit.bits, shared_flags_root, hit.flags) } None => { let pattern_str = if is_valid_ptr(pattern) { @@ -1041,12 +1041,12 @@ fn js_regexp_new_impl( site_key, raw_flags_owned, owned_pattern, - owned_flags, + owned_flags.clone(), flags_are_canonical, bits, programs.clone(), ); - (programs, bits, shared_flags_root) + (programs, bits, shared_flags_root, owned_flags) } }; let site_key::FlagBits { diff --git a/crates/perry-runtime/src/regex/match_all.rs b/crates/perry-runtime/src/regex/match_all.rs index 998e2ad83f..f533dd4955 100644 --- a/crates/perry-runtime/src/regex/match_all.rs +++ b/crates/perry-runtime/src/regex/match_all.rs @@ -87,7 +87,7 @@ unsafe fn materialize_match_all_results( // Phase 1 (borrowing, no JS allocation): snapshot every match into owned // Rust data. The fancy-regex fallback (lookbehind/backreferences) is - // needed because the never-match placeholder in `regex_ptr` would yield + // needed because the never-match standard program would yield // an empty iterator otherwise. // The scan starts AT `search_start` inside the whole subject — never on a // `&str_data[search_start..]` slice, which would strip the context every diff --git a/crates/perry-runtime/src/regex/replace_expand.rs b/crates/perry-runtime/src/regex/replace_expand.rs index 89391be8aa..19214cce1a 100644 --- a/crates/perry-runtime/src/regex/replace_expand.rs +++ b/crates/perry-runtime/src/regex/replace_expand.rs @@ -485,7 +485,7 @@ pub extern "C" fn js_string_replace_regex_named( // Fancy-regex fallback (lookbehind/backreferences): expand `$` // and friends against the fancy captures instead of the never-match - // placeholder stored in `regex_ptr`. + // placeholder stored as the standard program. if let Some(fre) = lookup_fancy_regex(re) { return replace_regex_str_fancy(str_data, &fre, (*re).global, repl_str); } diff --git a/crates/perry-runtime/src/regex/replace_expand_fancy.rs b/crates/perry-runtime/src/regex/replace_expand_fancy.rs index 22e78cee6a..9106ade261 100644 --- a/crates/perry-runtime/src/regex/replace_expand_fancy.rs +++ b/crates/perry-runtime/src/regex/replace_expand_fancy.rs @@ -197,7 +197,7 @@ pub extern "C" fn js_string_replace_regex( // Pattern the `regex` crate couldn't compile (lookbehind/backreferences) // → drive the replacement through fancy-regex. Otherwise the never-match - // placeholder in `regex_ptr` would leave the input unchanged. + // placeholder standard program would leave the input unchanged. if let Some(fre) = lookup_fancy_regex(re) { return replace_regex_str_fancy(str_data, &fre, (*re).global, repl_str); } @@ -369,7 +369,7 @@ pub extern "C" fn js_string_search_regex(s: *const StringHeader, re: *const RegE } // Fancy-regex fallback (lookbehind/backreferences): the never-match - // placeholder in `regex_ptr` would always report -1 otherwise. + // placeholder standard program would always report -1 otherwise. if let Some(fre) = lookup_fancy_regex(re) { return match fre.find(str_data) { Ok(Some(m)) => byte_index_to_utf16_index(str_data, m.start()) as i32, diff --git a/crates/perry-runtime/src/string/split.rs b/crates/perry-runtime/src/string/split.rs index addeb94e55..286fccf888 100644 --- a/crates/perry-runtime/src/string/split.rs +++ b/crates/perry-runtime/src/string/split.rs @@ -460,7 +460,7 @@ pub extern "C" fn js_string_split_n( // both. Detect regex delimiters by checking whether the pointer was // recorded by `js_regexp_new` and delegate to `js_string_split_regex` // on a match. Otherwise the regex header would be read as a - // StringHeader and segfault on the first byte of its `regex_ptr`. + // StringHeader and segfault on the first byte of its program-set pointer. #[cfg(feature = "regex-engine")] if crate::regex::is_regex_pointer(delimiter as *const u8) { return crate::regex::js_string_split_regex_n( From ce9e12801e8d83fae471e06cc85429257ac10854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:29:14 +0200 Subject: [PATCH 7/8] perf(regex): preserve live literal programs on eviction Replace whole-map overflow clears with one-entry eviction, and keep content-cache entries pinned while a recorded literal site refers to them. Only dynamic or displaced-site entries can leave the bounded table. Add a sabotage test that crosses both cache bounds, collects dead nursery headers, and proves the recorded literal does not rebuild. --- changelog.d/9918-regex-cache-eviction.md | 1 + crates/perry-runtime/src/hot_diag.rs | 37 ++- crates/perry-runtime/src/regex.rs | 8 +- .../perry-runtime/src/regex/compile_cache.rs | 29 +-- crates/perry-runtime/src/regex/lazy.rs | 14 +- crates/perry-runtime/src/regex/site_cache.rs | 214 +++++++++--------- crates/perry-runtime/src/regex/site_key.rs | 37 ++- crates/perry-runtime/src/regex/tests.rs | 2 +- crates/perry-runtime/src/regex/tests_cache.rs | 139 ++++++++++++ 9 files changed, 351 insertions(+), 130 deletions(-) create mode 100644 changelog.d/9918-regex-cache-eviction.md create mode 100644 crates/perry-runtime/src/regex/tests_cache.rs diff --git a/changelog.d/9918-regex-cache-eviction.md b/changelog.d/9918-regex-cache-eviction.md new file mode 100644 index 0000000000..29fd1168f7 --- /dev/null +++ b/changelog.d/9918-regex-cache-eviction.md @@ -0,0 +1 @@ +Keep compiled programs for recorded regular-expression literal sites across bounded cache eviction, and replace whole-cache overflow clears with one-entry eviction. diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index c397b88370..2b984e17ae 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -126,6 +126,9 @@ pub struct RegexDiag { pub compiles_std: u64, pub compiles_fancy: u64, pub compiles_repeat: u64, + /// One-entry evictions after a regex cache reaches its bound. The former + /// wholesale-clear counter remains as a zeroed regression control. + pub cache_evictions: u64, pub cache_clears: u64, /// `lazy::build_and_install_programs` runs (one per header that is /// executed at least once). @@ -190,6 +193,10 @@ pub struct RegexDiag { /// CONTENT-keyed cache; a site hit never reaches it, so the two are /// disjoint and `site_key_hit + site_hit <= new`. pub new_site_key_hit: u64, + #[cfg(test)] + test_program_builds: u64, + #[cfg(test)] + test_cache_evictions: u64, per_pattern: HashMap, } @@ -252,6 +259,33 @@ pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) { }); } +#[cfg(test)] +pub(crate) fn test_reset_regex_builds_and_evictions() { + REGEX_DIAG.with(|diag| { + let mut diag = diag.borrow_mut(); + diag.test_program_builds = 0; + diag.test_cache_evictions = 0; + }); +} + +#[cfg(test)] +pub(crate) fn test_note_regex_program_build() { + REGEX_DIAG.with(|diag| diag.borrow_mut().test_program_builds += 1); +} + +#[cfg(test)] +pub(crate) fn test_note_regex_cache_eviction() { + REGEX_DIAG.with(|diag| diag.borrow_mut().test_cache_evictions += 1); +} + +#[cfg(test)] +pub(crate) fn test_regex_builds_and_evictions() -> (u64, u64) { + REGEX_DIAG.with(|diag| { + let diag = diag.borrow(); + (diag.test_program_builds, diag.test_cache_evictions) + }) +} + impl RegexDiag { fn pat(&mut self, pattern_addr: usize, pattern: &[u8], flags: &str) -> &mut PatStat { let entry = self.per_pattern.entry(pattern_addr).or_default(); @@ -331,7 +365,7 @@ impl RegexDiag { let _ = writeln!( out, "[regex-diag] t={secs:.1}s new={} validated_hit={} site_hit={} pattern_bytes={} \ - compiles std={} fancy={} repeat={} cache_clears={} lazy_builds={} lazy_cache_hits={} \ + compiles std={} fancy={} repeat={} cache_clears={} evictions={} lazy_builds={} lazy_cache_hits={} \ exec={} exec_matched={} capture_slots={} capture_bytes={} test={} test_global={} \ match={} replace={} replace_matches={} split={} flags_alloc={} \ desc_regexp_probes={} desc_regexp_meta_negative={} \ @@ -346,6 +380,7 @@ impl RegexDiag { self.compiles_fancy, self.compiles_repeat, self.cache_clears, + self.cache_evictions, self.lazy_builds, self.lazy_cache_hits, self.exec_calls, diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 3170cdd277..a88a815048 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -481,9 +481,9 @@ crate::perry_thread_local! { /// validation. Validity is a pure function of the pair, so the answer is /// worth remembering; `js_regexp_new` used to get this from a /// `REGEX_CACHE` hit, which stopped being a proxy once the compiled - /// program became lazy (see `regex::lazy`). Same cap and - /// clear-on-overflow policy as the program caches — the cost of a clear - /// is a repeated parse, never a wrong verdict. The unit value keeps + /// program became lazy (see `regex::lazy`). Same cap and one-entry + /// eviction policy as the program caches — eviction can repeat one parse, + /// never change a verdict. The unit value keeps /// `evict_regex_cache_if_full` shared with the three program caches. static VALIDATED_PATTERNS: RefCell> = RefCell::new(HashMap::new()); } @@ -1751,4 +1751,6 @@ mod tests; #[cfg(all(test, feature = "regex-engine"))] mod tests_part2; #[cfg(all(test, feature = "regex-engine"))] +mod tests_cache; +#[cfg(all(test, feature = "regex-engine"))] mod tests_header; diff --git a/crates/perry-runtime/src/regex/compile_cache.rs b/crates/perry-runtime/src/regex/compile_cache.rs index c08835f960..4a4270aeb6 100644 --- a/crates/perry-runtime/src/regex/compile_cache.rs +++ b/crates/perry-runtime/src/regex/compile_cache.rs @@ -86,26 +86,27 @@ pub(crate) fn build_fancy_regex(pattern: &str) -> Result(cache: &mut HashMap) { +pub(crate) fn evict_regex_cache_if_full( + cache: &mut HashMap, +) { if cache.len() >= REGEX_CACHE_MAX_ENTRIES { - cache.clear(); + let victim = cache.keys().next().cloned(); + if let Some(victim) = victim { + cache.remove(&victim); + } + #[cfg(test)] + super::tests_cache::note_cache_eviction(); if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| d.cache_clears += 1); + crate::hot_diag::regex_counters(|d| d.cache_evictions += 1); } } } diff --git a/crates/perry-runtime/src/regex/lazy.rs b/crates/perry-runtime/src/regex/lazy.rs index 845f1fb20b..fbc583eb19 100644 --- a/crates/perry-runtime/src/regex/lazy.rs +++ b/crates/perry-runtime/src/regex/lazy.rs @@ -217,6 +217,8 @@ fn build_and_install_programs(re: *const RegExpHeader) { if !is_valid_regex_ptr(re) { return; } + #[cfg(test)] + crate::hot_diag::test_note_regex_program_build(); let (pattern, flags) = source_and_flags(re); if crate::hot_diag::regex_on() { let cache_hit = super::REGEX_CACHE.with(|cache| { @@ -248,7 +250,7 @@ fn build_and_install_programs(re: *const RegExpHeader) { // 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 + // and each can evict a different entry, 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 @@ -306,6 +308,16 @@ fn build_and_install_programs(re: *const RegExpHeader) { } } +#[cfg(test)] +pub(super) fn test_reset_program_builds() { + crate::hot_diag::test_reset_regex_builds_and_evictions(); +} + +#[cfg(test)] +pub(super) fn test_program_builds() -> u64 { + crate::hot_diag::test_regex_builds_and_evictions().0 +} + /// The header's standard-engine program, building it on first use. /// /// Every standard-program borrow in the tree goes through here — the field is diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index 675fe657df..5f6533cfd5 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -9,29 +9,24 @@ //! `ansi-regex` builds the same `new RegExp(parts.join("|"), "g")` per call. //! Each construction used to copy the pattern three times (the //! `VALIDATED_PATTERNS` probe key and `owned_pattern`) and SipHash all of it -//! once; the first operation on each header then -//! did the same three more times — `build_and_install_programs` probes the -//! three `(String, String)`-keyed program caches — and, for the common -//! no-fallback pattern, `lookup_fancy_regex` / `lookup_repeat_matcher` -//! re-probed two of them on EVERY exec. On the claude-code keystroke profile -//! SipHash over pattern text was 31 % of the post-turn window (regex 38 % -//! inclusive), all of it under these five functions. +//! once; the first operation on each header then did the same three more times. +//! On the claude-code keystroke profile SipHash over pattern text was 31 % of +//! the post-turn window (regex 38 % inclusive). //! //! # What //! -//! A direct-mapped, thread-local table keyed by a cheap CONTENT fingerprint -//! (length, first / middle / last 8 bytes, canonical flags) and verified by a -//! full byte compare — identity never depends on an address, so nothing is -//! rekeyed on a GC move and a dynamic `new RegExp(sameText)` hits too; a hit -//! costs one `memcmp` instead of a hash plus three copies. An entry owns the -//! pattern and canonical flags as `Arc` and, once the first header built -//! from it has been executed, the -//! compiled programs: a later construction installs those eagerly, so the -//! header is born built and never touches the `(pattern, flags)` caches. +//! A bounded, thread-local table keyed by a cheap CONTENT fingerprint (length, +//! first / middle / last 8 bytes, canonical flags) and verified by a full byte +//! compare. Identity never depends on an address, so nothing is rekeyed on a GC +//! move and a dynamic `new RegExp(sameText)` hits too; a hit costs one short +//! integer hash plus one `memcmp` instead of hashing all pattern bytes. //! -//! Validity is a pure function of `(pattern, flags)`, so a hit legitimately -//! skips validation: an entry is only ever written on the validated path, and -//! the programs it hands out were built for exactly this text. +//! An entry owns the pattern and canonical flags as `Arc` and, once the +//! first header built from it has executed, the compiled programs: a later +//! construction is born built. At capacity, entries referenced by a recorded +//! literal site are pinned and only dynamic or displaced-site entries are +//! evictable. Thus a live literal cannot rebuild, while programs for dead sites +//! can still leave the fixed-size table. //! //! Kill switch: `PERRY_REGEX_SITE_CACHE=0` (lookups miss, nothing is stored). @@ -40,6 +35,8 @@ use std::sync::Arc; use regex::Regex; +type ContentMap = crate::fast_hash::PtrHashMap>; + /// The compiled programs a header owns, in the form `lazy` installs them. pub(super) struct Programs { pub(super) std: Arc, @@ -73,13 +70,12 @@ struct Entry { programs: Option>, } -/// Direct-mapped slots (2-way: a fingerprint may live in `slot` or -/// `slot ^ 1`). Sized for a bundle's live literal working set; the -/// claude-code TUI cycles through a few dozen per render. -const SLOTS: usize = 1024; +/// Sized for the recorded literal-site table. The table never exceeds this +/// bound: if all entries are pinned, a dynamic miss remains uncached. +pub(super) const MAX_ENTRIES: usize = 1024; crate::perry_thread_local! { - static SITE_CACHE: RefCell>> = RefCell::new(Vec::new()); + static SITE_CACHE: RefCell = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } fn enabled() -> bool { @@ -92,8 +88,8 @@ fn enabled() -> bool { } /// Cheap content fingerprint: length, three 8-byte windows of the pattern, -/// the (≤ 8 byte) canonical flags. Collisions are harmless — every hit is -/// verified by a full compare — they only cost the verify and a re-insert. +/// the (≤ 8 byte) canonical flags. Collisions are harmless because every hit +/// is verified by a full compare and colliding entries share one small bucket. fn fingerprint(pattern: &[u8], flags: &[u8]) -> u64 { #[inline] fn window(bytes: &[u8], at: usize) -> u64 { @@ -117,46 +113,69 @@ fn fingerprint(pattern: &[u8], flags: &[u8]) -> u64 { h } -#[inline] -fn slot_of(fp: u64) -> usize { - (fp as usize) & (SLOTS - 1) -} - fn entry_matches(entry: &Entry, fp: u64, pattern: &str, flags: &str) -> bool { entry.fp == fp && &*entry.flags == flags && &*entry.pattern == pattern } +fn entry_count(cache: &ContentMap) -> usize { + cache.values().map(Vec::len).sum() +} + +/// Remove one entry that has no recorded literal site. The scan happens only +/// on a distinct-content miss at capacity; literal-site hits never reach it. +fn evict_one_dynamic(cache: &mut ContentMap) -> bool { + let victim = cache.iter().find_map(|(&fp, bucket)| { + bucket + .iter() + .position(|entry| !super::site_key::references_content(&entry.pattern, &entry.flags)) + .map(|index| (fp, index)) + }); + let Some((fp, index)) = victim else { + return false; + }; + let bucket = cache.get_mut(&fp).expect("the selected bucket exists"); + bucket.swap_remove(index); + if bucket.is_empty() { + cache.remove(&fp); + } + true +} + +fn make_room(cache: &mut ContentMap) -> bool { + if entry_count(cache) < MAX_ENTRIES { + return true; + } + if evict_one_dynamic(cache) { + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_counters(|d| d.cache_evictions += 1); + } + return true; + } + false +} + /// Find the verified entry for `(pattern, canonical flags)`. pub(super) fn lookup(pattern: &str, flags: &str) -> Option { if !enabled() { return None; } let fp = fingerprint(pattern.as_bytes(), flags.as_bytes()); - let slot = slot_of(fp); SITE_CACHE.with(|cache| { let cache = cache.borrow(); - if cache.is_empty() { - return None; - } - for s in [slot, slot ^ 1] { - if let Some(entry) = &cache[s] { - if entry_matches(entry, fp, pattern, flags) { - // The verify is a FULL byte compare, so its cost is - // linear in the pattern and this counter — not - // `pattern_bytes`, which counts every construction - // whether it probed or not — is the `memcmp` volume. - // Counted at the construction probe only; `insert` and - // `install_programs` verify too and are not counted here. - if crate::hot_diag::regex_on() { - let n = pattern.len() as u64; - crate::hot_diag::regex_counters(|d| d.new_site_verify_bytes += n); - } - return Some(Hit { - pattern: entry.pattern.clone(), - flags: entry.flags.clone(), - programs: entry.programs.clone(), + for entry in cache.get(&fp)? { + if entry_matches(entry, fp, pattern, flags) { + // Count only the construction probe's full byte compare, not + // the cold insert/install verification. + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_counters(|d| { + d.new_site_verify_bytes += pattern.len() as u64 }); } + return Some(Hit { + pattern: entry.pattern.clone(), + flags: entry.flags.clone(), + programs: entry.programs.clone(), + }); } } None @@ -164,83 +183,68 @@ pub(super) fn lookup(pattern: &str, flags: &str) -> Option { } /// Record a validated `(pattern, canonical flags)`, returning the shared -/// owned copies a header should keep. An existing verified entry is reused -/// (its programs are kept); otherwise the fresh entry has none yet. +/// owned copies a header should keep. An existing verified entry is reused. pub(super) fn insert(pattern: &str, flags: &str) -> (Arc, Arc) { if !enabled() { return (Arc::from(pattern), Arc::from(flags)); } let fp = fingerprint(pattern.as_bytes(), flags.as_bytes()); - let slot = slot_of(fp); SITE_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); - if cache.is_empty() { - cache.resize_with(SLOTS, || None); - } - for s in [slot, slot ^ 1] { - if let Some(entry) = &cache[s] { + if let Some(bucket) = cache.get(&fp) { + for entry in bucket { if entry_matches(entry, fp, pattern, flags) { return (entry.pattern.clone(), entry.flags.clone()); } } } - let victim = if cache[slot].is_none() { - slot - } else if cache[slot ^ 1].is_none() { - slot ^ 1 - } else { - slot ^ ((fp >> 11) as usize & 1) - }; let pattern: Arc = Arc::from(pattern); let flags: Arc = Arc::from(flags); - cache[victim] = Some(Entry { - fp, - pattern: pattern.clone(), - flags: flags.clone(), - programs: None, - }); + if make_room(&mut cache) { + cache.entry(fp).or_default().push(Entry { + fp, + pattern: pattern.clone(), + flags: flags.clone(), + programs: None, + }); + } (pattern, flags) }) } -/// Attach the programs the first execution built to the entry for -/// `(pattern, canonical flags)`, so every later construction of the same -/// text is born built. Inserts the entry if it was evicted meanwhile. +/// Attach the programs the first execution built to the content entry and +/// publish a weak view to every recorded literal site for this exact content. pub(super) fn install_programs(pattern: &str, flags: &str, programs: Arc) { if !enabled() { return; } let fp = fingerprint(pattern.as_bytes(), flags.as_bytes()); - let slot = slot_of(fp); - SITE_CACHE.with(|cache| { + let content_owned = SITE_CACHE.with(|cache| { let mut cache = cache.borrow_mut(); - if cache.is_empty() { - cache.resize_with(SLOTS, || None); - } - for s in [slot, slot ^ 1] { - if let Some(entry) = &mut cache[s] { + if let Some(bucket) = cache.get_mut(&fp) { + for entry in bucket { if entry_matches(entry, fp, pattern, flags) { if entry.programs.is_none() { - entry.programs = Some(programs); + entry.programs = Some(programs.clone()); } - return; + return true; } } } - let victim = if cache[slot].is_none() { - slot - } else if cache[slot ^ 1].is_none() { - slot ^ 1 - } else { - slot ^ ((fp >> 11) as usize & 1) - }; - cache[victim] = Some(Entry { + if !make_room(&mut cache) { + return false; + } + cache.entry(fp).or_default().push(Entry { fp, pattern: Arc::from(pattern), flags: Arc::from(flags), - programs: Some(programs), + programs: Some(programs.clone()), }); + true }); + if content_owned { + super::site_key::install_programs_for_content(pattern, flags, &programs); + } } #[cfg(test)] @@ -251,19 +255,23 @@ pub(super) fn test_reset() { #[cfg(test)] pub(super) fn test_has_programs(pattern: &str, flags: &str) -> Option { let fp = fingerprint(pattern.as_bytes(), flags.as_bytes()); - let slot = slot_of(fp); SITE_CACHE.with(|cache| { let cache = cache.borrow(); - if cache.is_empty() { - return None; - } - for s in [slot, slot ^ 1] { - if let Some(entry) = &cache[s] { - if entry_matches(entry, fp, pattern, flags) { - return Some(entry.programs.is_some()); - } + for entry in cache.get(&fp)? { + if entry_matches(entry, fp, pattern, flags) { + return Some(entry.programs.is_some()); } } None }) } + +#[cfg(test)] +pub(super) fn test_len() -> usize { + SITE_CACHE.with(|cache| entry_count(&cache.borrow())) +} + +#[cfg(test)] +pub(super) fn test_try_evict_one_dynamic() -> bool { + SITE_CACHE.with(|cache| evict_one_dynamic(&mut cache.borrow_mut())) +} diff --git a/crates/perry-runtime/src/regex/site_key.rs b/crates/perry-runtime/src/regex/site_key.rs index 842faa7072..f9737f7ef6 100644 --- a/crates/perry-runtime/src/regex/site_key.rs +++ b/crates/perry-runtime/src/regex/site_key.rs @@ -50,8 +50,8 @@ use std::sync::{Arc, Weak}; use super::site_cache::Programs; -/// The site entry's view of a pattern's compiled programs: **weak**, so the -/// table can hand them out but can never be the reason they stay alive. +/// The site entry's view of a pattern's compiled programs: **weak**, because +/// the pinned content-cache entry owns the bundle. /// /// Measured cost of holding them strongly (cc, one 3300-char reply): settled /// footprint 478/474 MB → 500/527 MB and idle CPU 2.37 → 2.68 s. The site @@ -60,11 +60,9 @@ use super::site_cache::Programs; /// wants. The campaign's directive is both metrics together, and a CPU win /// bought with resident memory does not land. /// -/// Strong references remain where they belong: the `(pattern, flags)` program -/// caches, and every live header that installed them via `Arc::into_raw`. A -/// site entry whose programs have been dropped simply reports "not built -/// yet", and the next construction re-picks them up from the content cache — -/// the same path the site's very first construction takes. +/// Strong references remain where they belong: the content cache and every +/// live header that installed them via `Arc::into_raw`. When this bounded site +/// entry is displaced, the content entry becomes eligible for eviction. struct WeakPrograms(Weak); impl WeakPrograms { @@ -270,6 +268,31 @@ pub(super) fn install_programs(key: usize, programs: Arc) { }); } +/// Whether the bounded literal-site table still records this content. The +/// content cache consults this only on a collision miss, never on a site hit. +pub(super) fn references_content(pattern: &str, flags: &str) -> bool { + SITE_KEY_TABLE.with(|table| { + table + .borrow() + .iter() + .flatten() + .any(|entry| &*entry.pattern == pattern && &*entry.flags == flags) + }) +} + +/// Publish a freshly built bundle to every literal site for this content. The +/// content cache owns it; sites observe the complete bundle through one weak +/// reference, preserving the all-or-nothing matcher rule. +pub(super) fn install_programs_for_content(pattern: &str, flags: &str, programs: &Arc) { + SITE_KEY_TABLE.with(|table| { + for entry in table.borrow_mut().iter_mut().flatten() { + if &*entry.pattern == pattern && &*entry.flags == flags { + entry.programs = Some(WeakPrograms::downgrade(programs)); + } + } + }); +} + #[cfg(test)] pub(super) fn test_reset() { SITE_KEY_TABLE.with(|table| table.borrow_mut().clear()); diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index 7d72ffff98..af67a400ea 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -904,7 +904,7 @@ fn unicode17_scripts_expand_to_codepoint_ranges() { /// 2026-07-09 GC audit (wave 2 batch A): the compiled-regex caches were /// unbounded — one entry per distinct `(pattern, flags)` ever compiled, up to /// 64 MiB each — so `new RegExp(userInput)` was an attacker-driven OOM. The -/// caches are now capped (clear-on-overflow) and every `RegExpHeader` OWNS a +/// caches are now capped (one-entry eviction) and every `RegExpHeader` OWNS a /// leaked Arc reference to its compiled program(s), so a header created /// before an eviction keeps matching afterwards. #[test] diff --git a/crates/perry-runtime/src/regex/tests_cache.rs b/crates/perry-runtime/src/regex/tests_cache.rs new file mode 100644 index 0000000000..297198c31d --- /dev/null +++ b/crates/perry-runtime/src/regex/tests_cache.rs @@ -0,0 +1,139 @@ +use super::*; +use std::collections::HashSet; + +pub(super) fn note_cache_eviction() { + crate::hot_diag::test_note_regex_cache_eviction(); +} + +fn make_string(text: &str) -> *mut StringHeader { + js_string_from_str(text) +} + +fn cache_test_key(slot: usize) -> i64 { + (0x6000_0000usize + (slot << 3)) as i64 +} + +/// Sabotage: remove the literal pin in `site_cache::evict_one_dynamic`, or put +/// back the whole-map clear in `evict_regex_cache_if_full`. +/// +/// The first sabotage lets a content-capacity eviction discard the target's +/// sole `Arc` owner. The second removes 512 answers instead of one. +/// After an explicit young collection finalizes both target headers, either +/// regression makes the next evaluation run the lazy builder again. +#[test] +fn literal_site_program_is_not_rebuilt_after_cache_overflow_and_young_collection() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + site_cache::test_reset(); + REGEX_CACHE.with(|cache| cache.borrow_mut().clear()); + FANCY_CACHE.with(|cache| cache.borrow_mut().clear()); + REPEAT_MATCHER_CACHE.with(|cache| cache.borrow_mut().clear()); + VALIDATED_PATTERNS.with(|cache| cache.borrow_mut().clear()); + lazy::test_reset_program_builds(); + + let target = "literal-site-overflow-target-[0-9]+"; + let target_key = cache_test_key(0); + let first = js_regexp_new_site(make_string(target), make_string(""), target_key); + assert_eq!( + js_regexp_test(first, make_string("literal-site-overflow-target-42")), + 1 + ); + assert_eq!( + lazy::test_program_builds(), + 1, + "the target builds exactly once" + ); + + // A second construction installs the content cache's program bundle into + // the site's weak lane. Both headers must then die so that only the cache + // policy, not a surviving receiver, can keep that weak reference live. + let second = js_regexp_new_site(make_string(target), make_string(""), target_key); + assert!(!unsafe { (*second).programs_ptr.is_null() }); + let first_addr = first as usize; + let second_addr = second as usize; + assert!( + !site_cache::test_try_evict_one_dynamic(), + "the only content entry is site-referenced, so no dynamic victim exists" + ); + + // Overflow the bounded content table with dynamic patterns. The target is + // the only recorded literal among them, so every capacity eviction must + // choose one of the dynamic entries and leave its program bundle intact. + for i in 0..site_cache::MAX_ENTRIES { + let _ = js_regexp_new( + make_string(&format!("content-overflow-{i}[a-z]")), + make_string(""), + ); + } + assert_eq!(site_cache::test_len(), site_cache::MAX_ENTRIES); + assert_eq!( + site_cache::test_has_programs(target, ""), + Some(true), + "a recorded literal's content entry must survive capacity eviction" + ); + + // 513 compiled literals (the target plus this 512-pattern flood) cross the + // former 512-entry wholesale-clear boundary. Snapshot at capacity so the + // assertion proves the overflow path preserved 511 old answers. + for i in 0..(REGEX_CACHE_MAX_ENTRIES - 1) { + let pattern = format!("overflow-literal-{i}$"); + let re = js_regexp_new_site( + make_string(&pattern), + make_string(""), + cache_test_key(100 + i), + ); + assert_eq!( + js_regexp_test(re, make_string(&format!("overflow-literal-{i}"))), + 1 + ); + } + let before: HashSet<_> = REGEX_CACHE.with(|cache| cache.borrow().keys().cloned().collect()); + assert_eq!(before.len(), REGEX_CACHE_MAX_ENTRIES); + + let last_i = REGEX_CACHE_MAX_ENTRIES - 1; + let last_pattern = format!("overflow-literal-{last_i}$"); + let last = js_regexp_new_site( + make_string(&last_pattern), + make_string(""), + cache_test_key(100 + last_i), + ); + assert_eq!( + js_regexp_test(last, make_string(&format!("overflow-literal-{last_i}"))), + 1 + ); + let survivors = REGEX_CACHE.with(|cache| { + cache + .borrow() + .keys() + .filter(|key| before.contains(*key)) + .count() + }); + assert_eq!(survivors, REGEX_CACHE_MAX_ENTRIES - 1); + assert!( + crate::hot_diag::test_regex_builds_and_evictions().1 > 0, + "the capacity eviction path must have executed" + ); + + let builds_before_gc = lazy::test_program_builds(); + let _ = crate::gc::gc_collect_minor(); + assert!( + !test_regex_pointer_entry_exists(first_addr) + && !test_regex_pointer_entry_exists(second_addr), + "the explicit young collection must finalize both unrooted target headers" + ); + + let rebuilt = js_regexp_new_site(make_string(target), make_string(""), target_key); + assert!( + !unsafe { (*rebuilt).programs_ptr.is_null() }, + "the still-recorded site must be born built after cache overflow and young GC" + ); + assert_eq!( + js_regexp_test(rebuilt, make_string("literal-site-overflow-target-7")), + 1 + ); + assert_eq!( + lazy::test_program_builds(), + builds_before_gc, + "the target site's compiled program must not be rebuilt" + ); +} From dd1c5242d2ce87139d33436f347adb7245fe754d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 16:41:18 +0200 Subject: [PATCH 8/8] fix(regex): clear branch-owned CI failures Use scoped handle access in the nursery relocation fixture, gate the Arc import to the matcher feature, and document why matcher kinds are dead in the feature-off layout-only build. Remove the unused test import and unsafe block, and apply rustfmt's module ordering. Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo --- crates/perry-runtime/src/regex.rs | 20 +++++++++++++++---- crates/perry-runtime/src/regex/tests_part2.rs | 19 ++++++++---------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index a88a815048..12c0ef67b4 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -13,6 +13,7 @@ use std::cell::RefCell; #[cfg(feature = "regex-engine")] use std::collections::HashMap; use std::ptr; +#[cfg(feature = "regex-engine")] use std::sync::Arc; #[cfg(feature = "regex-engine")] @@ -387,8 +388,12 @@ pub(crate) fn test_alloc_nursery_regexp_for_move(source: &str, flags: &str) -> * // must be set explicitly or the GC follows a garbage pointer. (*ptr).meta = std::ptr::null_mut(); (*ptr).programs_ptr = std::ptr::null(); - (*ptr).pattern_ptr = pattern.get_raw_const_ptr::(); - (*ptr).flags_ptr = flags_string.get_raw_const_ptr::(); + pattern.with_const_ptr::(|pattern| { + (*ptr).pattern_ptr = pattern; + }); + flags_string.with_const_ptr::(|flags| { + (*ptr).flags_ptr = flags; + }); (*ptr).case_insensitive = flags.contains('i'); (*ptr).global = flags.contains('g'); (*ptr).multiline = flags.contains('m'); @@ -495,6 +500,13 @@ pub(crate) use compile_cache::*; #[derive(Clone, Copy, Debug, Eq, PartialEq)] #[repr(u8)] +#[cfg_attr( + not(feature = "regex-engine"), + allow( + dead_code, + reason = "the feature-off runtime preserves RegExpHeader layout but constructs no matchers" + ) +)] pub(super) enum MatcherKind { Unbuilt, Standard, @@ -1749,8 +1761,8 @@ pub(crate) fn test_last_exec_groups() -> usize { #[cfg(all(test, feature = "regex-engine"))] mod tests; #[cfg(all(test, feature = "regex-engine"))] -mod tests_part2; -#[cfg(all(test, feature = "regex-engine"))] mod tests_cache; #[cfg(all(test, feature = "regex-engine"))] mod tests_header; +#[cfg(all(test, feature = "regex-engine"))] +mod tests_part2; diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs index e8cda486be..5896ff2c94 100644 --- a/crates/perry-runtime/src/regex/tests_part2.rs +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -3,8 +3,7 @@ //! in `tests.rs`; the shared fixtures come from there. use super::tests::{ - make_string, match_capture_text, regex_has_fancy_program, regex_has_repeat_program, - regex_is_built, string_payload, + make_string, match_capture_text, regex_has_fancy_program, regex_is_built, string_payload, }; use super::*; @@ -599,15 +598,13 @@ fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { site_cache::test_reset(); let cold = build(); - unsafe { - lazy::ensure_regex_compiled(cold); - assert!( - regex_has_fancy_program(cold), - "a built header must carry every program its pattern needs — a null \ - the fancy program here is memoized by site_cache::install_programs and makes \ - the breakage permanent for this literal" - ); - } + lazy::ensure_regex_compiled(cold); + assert!( + regex_has_fancy_program(cold), + "a built header must carry every program its pattern needs — a null \ + the fancy program 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,