From 46a3c647d1e481e2d9e81f153edb6f7aa7188d32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:14:28 +0200 Subject: [PATCH 01/19] diag(regex): the site cache's byte-compare volume, and a diag file that cannot fail silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PERRY_REGEX_DIAG` gains four per-construction work counters — `barrier_taken` / `barrier_gated` (whose sum must equal `new`), `header_bytes`, `site_verify_bytes` and `side_table_inserts` — so what `js_regexp_new` costs per call is a number rather than a reading of a profile. Writers for the first group arrive with the change they measure; `site_verify_bytes` is written here. `site_verify_bytes` is deliberately NOT `pattern_bytes`: the latter counts every construction's pattern length whether the site-cache probe hit or missed, while the full byte compare that verifies a fingerprint match is the part that is linear in the pattern — what makes a 12 KB emoji pattern expensive and a 60-byte one free. Counted at the construction probe only; `insert` and `install_programs` verify too and are not counted here. Two reliability fixes, both of the same shape as the campaign's missing exit-line trap — an absent output that greps identically to an instrument that was never built: * a file sink that cannot write now reports the path and the error on stderr once and keeps writing there, instead of swallowing the error; * the first snapshot is written at the first tick rather than one full DUMP_INTERVAL_MS later, so a run shorter than a second produces output. (cherry picked from commit df202bf8fbc7955ff1db85009c6369a36cca0275) --- crates/perry-runtime/src/hot_diag.rs | 54 ++++++++++++++++++-- crates/perry-runtime/src/regex/site_cache.rs | 10 ++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index ca92f9f93b..2836715f5e 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -37,13 +37,25 @@ fn sink_from_env(name: &str) -> Option { } } +/// A failed file write used to be swallowed (`if ... .is_ok()`), so an +/// unwritable path — a directory that does not exist, a read-only mount, a +/// sandbox — produced *no file and no message*, which greps identically to +/// "this instrument was never built". That is the campaign's own +/// missing-exit-line trap in a second form, and it cost a lane a measurement +/// run. Report the first failure on stderr, naming the path and the error, +/// and keep writing there. fn write_sink(sink: &Sink, text: &str) { match sink { Sink::Stderr => eprint!("{text}"), Sink::File(path) => { let tmp = format!("{path}.tmp"); - if std::fs::write(&tmp, text).is_ok() { - let _ = std::fs::rename(&tmp, path); + let wrote = std::fs::write(&tmp, text).and_then(|()| std::fs::rename(&tmp, path)); + if let Err(err) = wrote { + static WARNED: AtomicBool = AtomicBool::new(false); + if !WARNED.swap(true, Ordering::Relaxed) { + eprintln!("[hot-diag] cannot write {path}: {err} — falling back to stderr"); + } + eprint!("{text}"); } } } @@ -140,6 +152,28 @@ pub struct RegexDiag { /// meta edge was wired for RegExp this was 0 by construction: the filter /// answered "maybe" for every one of them. pub desc_regexp_meta_negative: u64, + /// Constructions whose two header string stores took the full write + /// barrier pair (`GC_FLAG_TENURED` set on the freshly allocated header, + /// or an incremental cycle live anywhere). + pub new_barrier_taken: u64, + /// Constructions the newborn-parent gate proved owe the remembered set + /// nothing, so neither barrier call ran. `taken + gated == new_calls` + /// is the invariant: a run where `gated` is 0 did not exercise the gate. + pub new_barrier_gated: u64, + /// Bytes of `RegExpHeader` allocated by `js_regexp_new`. Load-independent + /// and directly comparable with a probe's allocation-per-grapheme reading. + pub new_header_bytes: u64, + /// Bytes the literal-site cache byte-compared to VERIFY a fingerprint + /// match (`site_cache::entry_matches`). Distinct from `pattern_bytes`, + /// which counts every construction's pattern length whether the probe hit + /// 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. + pub new_side_table_inserts: u64, per_pattern: HashMap, } @@ -154,14 +188,17 @@ pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) { let mut d = d.borrow_mut(); if d.started.is_none() { d.started = Some(Instant::now()); - d.last_dump = d.started; + // `last_dump` stays None so the FIRST tick dumps immediately: a + // run shorter than `DUMP_INTERVAL_MS` used to write nothing at + // all, which is indistinguishable from a dead instrument. + d.last_dump = None; } f(&mut d); d.events = d.events.wrapping_add(1); if d.events % TICK_EVERY == 0 { let due = d .last_dump - .is_some_and(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS); + .is_none_or(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS); if due { d.last_dump = Some(Instant::now()); if let Some(sink) = regex_sink() { @@ -254,7 +291,9 @@ impl RegexDiag { compiles std={} fancy={} repeat={} cache_clears={} 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={}", + desc_regexp_probes={} desc_regexp_meta_negative={} \ + barrier_taken={} barrier_gated={} header_bytes={} site_verify_bytes={} \ + side_table_inserts={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -278,6 +317,11 @@ impl RegexDiag { self.new_flags_allocated, self.desc_regexp_probes, self.desc_regexp_meta_negative, + self.new_barrier_taken, + self.new_barrier_gated, + self.new_header_bytes, + self.new_site_verify_bytes, + self.new_side_table_inserts, ); // Merge by content (prefix, len, flags): distinct literal sites with // the same pattern are one row. diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index b8e68af0da..3ba9ace469 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -140,6 +140,16 @@ pub(super) fn lookup(pattern: &str, flags: &str) -> Option { 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_with(|d| d.new_site_verify_bytes += n); + } return Some(Hit { pattern: entry.pattern.clone(), flags: entry.flags.clone(), From 6953d05ee968f6d24e09ac209c6270a72a18fb89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:14:28 +0200 Subject: [PATCH 02/19] perf(regex): construction skips the barrier's parent classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #9845 the `RegExpHeader` is a nursery allocation, so its two string field stores cannot owe the remembered set anything — and they were still taking the full barrier twice to discover that: four page-map classifications, two dirty-page-cache probes and two child classifications per construction, every one of them ending at `ParentNotOldSkips`. The gate is the runtime twin of the one the compiler already emits in front of every one of its own stores (`emit_parent_may_need_remembering_check`, #7511): `GC_FLAG_TENURED` clear on the parent's LIVE header, and a globally idle incremental mark barrier. The first clause answers the generational question; the second is what makes it legal to skip the SATB/insertion shading as well, and dropping either one is a live child swept. Both are read live, so a header a collection promoted between `arena_alloc_gc` and the store, and `RegExp.prototype.compile` reassigning a tenured receiver, still take the full path. `gc::tests::inline_generation_gate_contract` already pins those two clauses for the emitted gate against a stranded-child witness; it now pins the runtime twin to the same codegen predicate clause by clause, and a third test asserts on the header `js_regexp_new` actually returns — so the skip arm is proven REACHED, not merely available. Measured motivation (segment-loop probe, region B, 60,000 reps, `sample`, main thread, leaf sum = thread header exactly): one `RegExp` per grapheme from a literal inside a function body, and the barrier subtree under `js_regexp_new` is 739 of 14,628 main-thread samples — 32 % of that function's own subtree. `PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores the unconditional pair. With the gate off nothing else changes, so the OFF arm is the pre-change code path exactly rather than a control still carrying the bookkeeping. (cherry picked from commit 3b5f5efd692f970ed9f95be24df9f53def3ca967) --- .../9885-regex-newborn-barrier-gate.md | 43 ++++++ crates/perry-runtime/src/gc/barrier_store.rs | 57 ++++++++ .../tests/inline_generation_gate_contract.rs | 138 ++++++++++++++++++ crates/perry-runtime/src/regex.rs | 82 +++++++++-- 4 files changed, 308 insertions(+), 12 deletions(-) create mode 100644 changelog.d/9885-regex-newborn-barrier-gate.md diff --git a/changelog.d/9885-regex-newborn-barrier-gate.md b/changelog.d/9885-regex-newborn-barrier-gate.md new file mode 100644 index 0000000000..751904067f --- /dev/null +++ b/changelog.d/9885-regex-newborn-barrier-gate.md @@ -0,0 +1,43 @@ +**A `RegExp` literal's construction no longer pays the write barrier's parent +classification.** Since #9845 the `RegExpHeader` is a nursery allocation, so +its two string field stores — `pattern_ptr` and `flags_ptr` — cannot owe the +remembered set anything; they were still taking the full barrier and +discovering that fact, twice, at a cost of four page-map classifications, two +dirty-page-cache probes and two child classifications per construction, all +ending at `ParentNotOldSkips`. + +The fix is the runtime twin of a gate the compiler already emits in front of +every one of its own stores (`emit_parent_may_need_remembering_check`, #7511): +`GC_FLAG_TENURED` clear on the parent's live header **and** a globally idle +incremental mark barrier ⇒ neither the remembered set nor the SATB shading has +anything to record. Both clauses are read live, so a header a collection +promoted between `arena_alloc_gc` and the store, or a +`RegExp.prototype.compile` reassigning a tenured receiver, still takes the +full path. + +Why the two clauses and not one: the tenured bit answers the generational +question, and the incremental count is what makes it legal to skip the +insertion shading as well — dropping either is a live child swept, which is +what `gc::tests::inline_generation_gate_contract` already pins for the emitted +gate and now pins for the runtime twin, clause by clause, against the same +codegen predicate. A third test asserts on the header `js_regexp_new` actually +returns, so the skip arm is proven reached rather than merely available. + +Measured motivation (segment-loop probe, region B, 60,000 reps, `sample`, main +thread, leaf sum = thread header exactly): the probe constructs one `RegExp` +per grapheme from a literal inside a function body, and the barrier subtree +under `js_regexp_new` is 739 of 14,628 main-thread samples — 32 % of that +function's own subtree. + +`PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores the unconditional pair; with the +gate off nothing else changes, so the OFF arm is the pre-change code path +exactly rather than a handicapped control. + +`PERRY_REGEX_DIAG` gains the counters that make the claim checkable rather +than argued: `barrier_taken` / `barrier_gated` (whose sum must equal `new`), +`header_bytes`, `site_verify_bytes` (the site cache's byte-compare volume, +which `pattern_bytes` does not isolate) and `side_table_inserts`. Two +reliability fixes ride along: a diag file the process cannot write now says so +on stderr and falls back there instead of vanishing silently, and the first +snapshot is written at the first tick rather than after a full second, so a +short run can no longer look like a dead instrument. diff --git a/crates/perry-runtime/src/gc/barrier_store.rs b/crates/perry-runtime/src/gc/barrier_store.rs index c77a3eb4bd..2b43c80240 100644 --- a/crates/perry-runtime/src/gc/barrier_store.rs +++ b/crates/perry-runtime/src/gc/barrier_store.rs @@ -319,3 +319,60 @@ pub(super) fn barrier_remembering_active() -> bool { bump_write_barrier_trace_counter(BarrierTraceCounter::UnarmedSkips); false } + +/// The runtime twin of codegen's `emit_parent_may_need_remembering_check` +/// (#7511): may a store into a **freshly allocated** GC parent be skipped +/// outright? +/// +/// # Why this exists on the runtime side too +/// +/// Every store emitted by the compiler is already gated this way — the +/// generated code reads the parent's `gc_flags` and, when `GC_FLAG_TENURED` +/// is clear *and* no incremental cycle is live anywhere, jumps over the +/// barrier call entirely. Runtime-Rust construction paths call +/// [`runtime_write_barrier_gc_slot`] unconditionally instead, so a native +/// header born in the nursery pays, per pointer slot: a page-map +/// classification for the malloc-parent probe, `barrier_child_prologue`, the +/// armed check, the dereferenceability test, the dirty-page-cache probe and a +/// second page-map classification in `barrier_parent_needs_remembering` — +/// all of which end at `ParentNotOldSkips` because the parent is young. +/// +/// Measured on the segment-loop probe (region B, 60,000 reps, `sample`, main +/// thread): `js_regexp_new` constructs one `RegExp` per grapheme and its +/// barrier subtree — `runtime_write_barrier_gc_slot` / +/// `write_barrier_slot_decoded` / `write_barrier_decoded_parent` / +/// `mark_dirty_external_slot_page` / `remembered_child_needs_tracking` / +/// `classify_heap_generation_uncached` — is 739 of the 14,628 main-thread +/// samples, 32 % of that function's own subtree. +/// +/// # Why it is sound +/// +/// Exactly the two clauses the emitted gate uses, for exactly the two reasons +/// its doc comment gives: +/// +/// * **`GC_FLAG_TENURED` clear** ⇒ the parent is not in the old generation, +/// so no old→young remembered-set entry can be owed. The flag is read +/// LIVE at the store, not claimed statically, because promotion can move +/// an object under any static proof (#7501) — a header that a collection +/// promoted between its allocation and this store reads TENURED here and +/// takes the full barrier. +/// * **`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT == 0`** ⇒ no thread has +/// an incremental mark barrier installed, which is what makes it legal to +/// skip the SATB/insertion shading as well +/// ([`incremental_mark_barrier_globally_idle`]). Shading is not a +/// generational question and must never be dropped while a cycle is live, +/// so a non-zero count sends the store down the ordinary path. +/// +/// # Safety +/// +/// Dereferences `parent_addr - GC_HEADER_SIZE`. The caller must pass a live, +/// non-forwarded GC user pointer it has just allocated (or otherwise +/// validated) — the same contract `emit_parent_may_need_remembering_check` +/// places on its caller. +#[inline] +pub(crate) unsafe fn newborn_parent_needs_barrier(parent_addr: usize) -> bool { + if !super::barrier::incremental_mark_barrier_globally_idle() { + return true; + } + (*super::layout::header_from_user_ptr(parent_addr as *const u8)).gc_flags & GC_FLAG_TENURED != 0 +} diff --git a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs index e4df96234c..04eade291e 100644 --- a/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs +++ b/crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs @@ -262,3 +262,141 @@ fn the_incremental_clause_forces_the_call_for_a_young_parent() { store skips its insertion barrier and a live object is swept" ); } + +// --------------------------------------------------------------------------- +// The RUNTIME twin of the same gate. +// +// Runtime-Rust construction paths (`js_regexp_new` and friends) call the +// barrier unconditionally, so a native header born in the nursery pays the +// full parent classification on every field it initialises while generated +// code, storing into the very same kind of object, skips it. These tests pin +// `gc::newborn_parent_needs_barrier` to the emitted predicate CLAUSE FOR +// CLAUSE, so the two can only drift by failing here. +// --------------------------------------------------------------------------- + +/// Clause 1 (`GC_FLAG_TENURED`): the runtime twin must answer exactly what the +/// emitted gate answers for the same live header. +/// +/// Sabotage that this catches: a twin that only consults the incremental +/// count answers "skip" for the tenured parent and fails the second assert — +/// which is the stranded-child bug of +/// `sabotaged_parent_gate_strands_a_young_child_the_shipped_gate_keeps`, +/// reached from the runtime side instead of the emitted side. +#[test] +fn the_runtime_twin_reads_the_tenured_clause_from_the_live_header() { + let _guard = GcTestIsolationGuard::new(); + assert!( + crate::gc::incremental_mark_barrier_globally_idle(), + "this test isolates clause 1, so no cycle may be live" + ); + + let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize; + assert_eq!( + header_flags(young) & GC_FLAG_TENURED, + 0, + "a fresh nursery allocation must not be TENURED — otherwise this test exercises nothing" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "a nursery parent with no cycle live is exactly the case the gate exists to skip" + ); + assert_eq!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + codegen_parent_may_need_remembering(header_flags(young), 0), + "the runtime twin and the emitted gate must agree on a nursery parent" + ); + + // The SAME address, now carrying the bit a promotion would have stamped. + // Read live, so this models a header a collection promoted between its + // allocation and the store that follows it. + unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags |= GC_FLAG_TENURED }; + assert!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "a TENURED parent owes the remembered set an entry and must take the full barrier" + ); + assert_eq!( + unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + codegen_parent_may_need_remembering(header_flags(young), 0), + "the runtime twin and the emitted gate must agree on a tenured parent" + ); + unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags &= !GC_FLAG_TENURED }; +} + +/// Clause 2 (the incremental count): with a cycle live anywhere, a nursery +/// parent must still take the call, because the skipped work includes the +/// SATB/insertion shading and that is not a generational question. +/// +/// Sabotage that this catches: a twin that only reads the header's flags +/// answers "skip" while a cycle is marking, and a child linked in during that +/// window is never shaded. +#[test] +fn the_runtime_twin_forces_the_barrier_while_an_incremental_cycle_is_live() { + let _guard = GcTestIsolationGuard::new(); + + let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize; + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "precondition: with no cycle live this parent is skipped" + ); + + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_add(1, Ordering::Relaxed); + let forced = unsafe { crate::gc::newborn_parent_needs_barrier(young) }; + let codegen_answer = codegen_parent_may_need_remembering( + header_flags(young), + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed), + ); + crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_sub(1, Ordering::Relaxed); + + assert!( + forced, + "a live incremental cycle must force the call even for a nursery parent — dropping this \ + clause skips the insertion shading and sweeps a live child" + ); + assert_eq!( + forced, codegen_answer, + "the runtime twin and the emitted gate must agree while a cycle is live" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(young) }, + "the count is back to zero, so the same parent is skippable again" + ); +} + +/// **Did this code run?** The gate is only worth anything if the real +/// `js_regexp_new` header reaches its skip arm — a fast path that is available +/// but never taken is the campaign's "measured flat" failure in advance. +/// +/// Asserts on the header `js_regexp_new` actually produced, not on a synthetic +/// fixture: since #9845 it is a nursery allocation, so with no cycle live the +/// two `pattern_ptr` / `flags_ptr` stores skip the barrier entirely, and the +/// SAME header answers "take the barrier" the moment it carries the bit a +/// promotion would stamp. +#[cfg(feature = "regex-engine")] +#[test] +fn a_freshly_constructed_regexp_header_reaches_the_skip_arm() { + let _guard = GcTestIsolationGuard::new(); + + let pattern = crate::string::js_string_from_bytes(b"a(b)c".as_ptr(), 5); + let flags = crate::string::js_string_from_bytes(b"g".as_ptr(), 1); + let re = crate::regex::js_regexp_new(pattern, flags) as usize; + + assert_eq!( + crate::arena::classify_heap_generation(re), + crate::arena::HeapGeneration::Nursery, + "#9845 allocates the RegExp header in the NURSERY; if that changes, the construction path \ + stops being the case this gate is written for" + ); + assert!( + !unsafe { crate::gc::newborn_parent_needs_barrier(re) }, + "the construction path must actually TAKE the skip arm — an available-but-unreached fast \ + path is indistinguishable from no fast path in a measurement" + ); + + unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags |= GC_FLAG_TENURED }; + assert!( + unsafe { crate::gc::newborn_parent_needs_barrier(re) }, + "the same header, promoted, must take the full barrier — this is the \ + `RegExp.prototype.compile`-on-a-tenured-receiver case" + ); + unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags &= !GC_FLAG_TENURED }; +} diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 73dd933c05..24c33fd88c 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -943,6 +943,24 @@ pub(super) fn throw_regexp_syntax_error(message: &str) -> ! { crate::exception::js_throw(crate::value::js_nanbox_pointer(err as i64)) } +/// Kill switch for the newborn-parent barrier gate below +/// (`PERRY_REGEX_NEWBORN_BARRIER_GATE=0` ⇒ the two header stores take the +/// unconditional barrier pair, i.e. the pre-gate code path exactly). One +/// relaxed load of a `OnceLock` per construction, resolved once per process, +/// mirroring `regex::site_cache::enabled`. +#[cfg(feature = "regex-engine")] +#[inline] +fn newborn_barrier_gate_enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + crate::gc::env_default_on_from_value( + std::env::var("PERRY_REGEX_NEWBORN_BARRIER_GATE") + .ok() + .as_deref(), + ) + }) +} + /// Create a new RegExp from pattern and flags strings /// Returns a pointer to RegExpHeader /// @@ -1275,20 +1293,52 @@ pub extern "C" fn js_regexp_new( // `runtime_write_barrier_gc_slot` classifies the parent and only // remembers genuinely-young children, so an already-old/interned // `pattern` is a harmless no-op. + // + // ★ Gated by the same live header test the COMPILER emits in front of + // every one of its own stores (`emit_parent_may_need_remembering_check`, + // #7511): a parent whose `GC_FLAG_TENURED` is clear owes the + // remembered set nothing, and a globally idle incremental barrier + // makes the SATB shading skippable too. Both clauses are read live — + // a header a collection promoted between `arena_alloc_gc` above and + // this store reads TENURED here and takes the full path, as does + // `RegExp.prototype.compile` reassigning a tenured header. + // + // Since #9845 the header is a NURSERY allocation, so on the common + // path both clauses are false and the pair of barrier calls — four + // page-map classifications, two dirty-page-cache probes and two child + // classifications, all ending at `ParentNotOldSkips` — collapses to + // one relaxed load of a static and one byte read of the header this + // function just wrote. `PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores + // the unconditional pair; nothing else changes with the gate off, so + // the OFF arm is the pre-change code path exactly. let regexp_parent_addr = ptr as usize; - if !pattern.is_null() { - crate::gc::runtime_write_barrier_gc_slot( - regexp_parent_addr, - std::ptr::addr_of!((*ptr).pattern_ptr) as usize, - js_nanbox_string(pattern as i64).to_bits(), - ); + let needs_barrier = !newborn_barrier_gate_enabled() + || crate::gc::newborn_parent_needs_barrier(regexp_parent_addr); + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| { + if needs_barrier { + d.new_barrier_taken += 1; + } else { + d.new_barrier_gated += 1; + } + d.new_header_bytes += header_size as u64; + }); } - if !canonical_flags_ptr.is_null() { - crate::gc::runtime_write_barrier_gc_slot( - regexp_parent_addr, - std::ptr::addr_of!((*ptr).flags_ptr) as usize, - js_nanbox_string(canonical_flags_ptr as i64).to_bits(), - ); + if needs_barrier { + if !pattern.is_null() { + crate::gc::runtime_write_barrier_gc_slot( + regexp_parent_addr, + std::ptr::addr_of!((*ptr).pattern_ptr) as usize, + js_nanbox_string(pattern as i64).to_bits(), + ); + } + if !canonical_flags_ptr.is_null() { + crate::gc::runtime_write_barrier_gc_slot( + regexp_parent_addr, + std::ptr::addr_of!((*ptr).flags_ptr) as usize, + js_nanbox_string(canonical_flags_ptr as i64).to_bits(), + ); + } } (*ptr).case_insensitive = case_insensitive; (*ptr).global = global; @@ -1330,6 +1380,14 @@ pub extern "C" fn js_regexp_new( REGEX_POINTERS.with(|s| { 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_with(|d| d.new_side_table_inserts += 2); + } // Issue #637: side-table owned copies of pattern + flags so // `.source` / `.flags` survive GC of the input StringHeaders. From b2777c47905969bfe391cb5ffe845d59a84f6a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:28:29 +0200 Subject: [PATCH 03/19] perf(regex): identify a literal by its source site, not by its pattern text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `regex::site_cache` answers "have I seen this pattern TEXT before?" — the right question for a dynamic `new RegExp(s)`, and the wrong one for a literal, which is one source site whose pattern and flags are fixed at compile time. Because a content fingerprint can collide, every hit is verified by a full byte compare of the pattern, and a literal constructs a fresh object every time it is reached: on claude-code that verify is ~2.0 GB of `memcmp` per 400-character reply and 39.6 % of `js_regexp_new`'s own profile subtree. `Expr::RegExp` now emits an 8-byte private global per literal site and passes its ADDRESS to a new `js_regexp_new_site(pattern, flags, site_key)`. The address is unique by construction, immortal and never moves — the three properties a `StringHeader` address lacks, which is why the earlier analysis concluded no sound string identity was available and left the compare in place. A hit compares one word plus the site's <= 8-byte flags text and then reads nothing about the pattern: no fingerprint, no memcmp, no validation (validity is a pure function of the pair and the site's first construction established it), no flag canonicalization (the seven bits are a property of the site), and the programs the site already compiled are installed eagerly. `site_key = 0` is "no site": every dynamic construction keeps the two-argument entry and never touches the table. Kill switch `PERRY_REGEX_SITE_KEY=0`. Tests: two sites whose patterns have EQUAL LENGTH and different text, each constructed twice — the sabotage of keying the table by pattern length hands the second site the first's entry and fails on `.source` and on `test`; four dynamic constructions leave the table empty while one site-keyed construction fills it; a second construction at an executed site is born built; and the new symbol's declaration is asserted by name AND arity, because a missing declare fails only at the LLVM parse and a wrong arity miscompiles silently. Measurement is owed on the cc rig, where the 12,807-character pattern lives — the segment-loop probe's literal is ~60 characters and its memcmp is 0.16 % of the thread, so the probe cannot show this change. (cherry picked from commit 7cbadc1c18683f18b399af879f8d66bae157c0ea) --- changelog.d/9886-regex-literal-site-key.md | 55 ++ .../src/expr/logical_collections.rs | 68 ++- crates/perry-codegen/src/runtime_decls/mod.rs | 53 ++ .../src/runtime_decls/strings.rs | 7 + crates/perry-runtime/src/hot_diag.rs | 9 +- crates/perry-runtime/src/regex.rs | 502 ++++++++++++------ crates/perry-runtime/src/regex/site_key.rs | 249 +++++++++ crates/perry-runtime/src/regex/tests.rs | 171 ++++++ 8 files changed, 939 insertions(+), 175 deletions(-) create mode 100644 changelog.d/9886-regex-literal-site-key.md create mode 100644 crates/perry-runtime/src/regex/site_key.rs diff --git a/changelog.d/9886-regex-literal-site-key.md b/changelog.d/9886-regex-literal-site-key.md new file mode 100644 index 0000000000..91ee033086 --- /dev/null +++ b/changelog.d/9886-regex-literal-site-key.md @@ -0,0 +1,55 @@ +**A regex literal is now identified by its SOURCE SITE, not by its text**, so +constructing one costs a single word compare instead of a content fingerprint +plus a full byte compare of the pattern. + +A regex literal evaluates to a fresh object every time it is reached +(ECMA-262), and TUI code reaches them inside hot functions: `string-width`'s +`emojiRegex()` returns a fresh ~12,807-character `/…/g` on every call, once per +grapheme in claude-code's layout pass. The runtime therefore re-derived "which +pattern is this?" from the text on every construction — `regex::site_cache` +keys on a cheap fingerprint and, because a fingerprint can collide, verifies +every hit with `&*entry.pattern == pattern`. That verify is linear in the +pattern: `PERRY_REGEX_DIAG` measured **2.0 GB of `memcmp` per 400-character +reply**, and a `sample` of the segment loop put `_platform_memcmp` at **39.6 % +of `js_regexp_new`'s own subtree**. + +The compiler knew the answer all along; the lowering just had no way to say it. +`Expr::RegExp` now emits an 8-byte private global per literal site and passes +its **address** as a third argument to a new `js_regexp_new_site(pattern, +flags, site_key)`. That address is unique by construction, immortal, and never +moves — which is exactly what a `StringHeader` address is not, and why the +earlier analysis of this problem concluded no sound string identity existed and +left the byte compare in place: string headers are GC-managed, so an address is +freed and reused, and a moving collector relocates them. + +A hit verifies with one word plus the site's ≤ 8-byte flags text (two spellings +of one canonical form must not answer for each other) and then reads nothing +about the pattern at all: no fingerprint, no `memcmp`, no validation — validity +is a pure function of `(pattern, flags)` and the site's first construction +established it — and no flag canonicalization, since the seven flag bits are a +property of the site. Once the site's first header has executed, later +constructions are born built. + +`site_key = 0` means "no site" and behaves exactly as before, so every dynamic +construction (`new RegExp(s)`, `js_regexp_construct`, +`RegExp.prototype.compile`, the runtime's own callers) keeps the two-argument +entry point and never touches the site table — pinned by a test that asserts +the table is still empty after four dynamic constructions, and non-empty after +one site-keyed one, so the zero is a property of the entry point rather than of +a table that never works. + +The named sabotage is a table keyed by anything weaker than the site address: +two literals at two sites, same flags, **same pattern length**, different text. +Under a length- or prefix-keyed table the second site inherits the first's +entry, `.source` reports a pattern the literal never contained and `test` +matches the wrong language. Each site is constructed twice, because a first +construction always misses and would pass under every sabotage. + +Kill switch: `PERRY_REGEX_SITE_KEY=0` — the probe misses and nothing is +recorded, so the OFF arm is the content-keyed path exactly rather than a +control still paying the bookkeeping. + +The new runtime symbol is declared in `runtime_decls/strings.rs` with a test +asserting its **name and arity**: a missing `declare` is invisible to every +HIR-level test and fails only at the in-process LLVM parse (`use of undefined +value`), and a wrong arity parses and miscompiles. diff --git a/crates/perry-codegen/src/expr/logical_collections.rs b/crates/perry-codegen/src/expr/logical_collections.rs index 1073cdd045..83b5a40695 100644 --- a/crates/perry-codegen/src/expr/logical_collections.rs +++ b/crates/perry-codegen/src/expr/logical_collections.rs @@ -1283,15 +1283,79 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let flags_idx = ctx.strings.intern(flags); let pattern_global = format!("@{}", ctx.strings.entry(pattern_idx).handle_global); let flags_global = format!("@{}", ctx.strings.entry(flags_idx).handle_global); + // ★ A literal's SITE IDENTITY, as an immortal address. + // + // A regex literal evaluates to a fresh object every time it is + // reached (ECMA-262), and TUI code reaches them inside hot + // functions — `string-width`'s `emojiRegex()` returns a fresh + // ~12,807-character `/…/g` per call. The runtime therefore + // re-derives "which pattern is this?" per construction from the + // TEXT: a content fingerprint plus, on every hit, a full byte + // compare to verify it (`regex::site_cache::entry_matches`). On + // claude-code that verify is ~2.0 GB of `memcmp` per 400-character + // reply, and it is 39.6 % of `js_regexp_new`'s own profile subtree. + // + // The compiler knows the answer statically: this literal is one + // source site whose pattern and flags can never change. What the + // runtime was missing is the key, because the lowering passed only + // the two string handles. This emits an 8-byte private global per + // literal site and passes its ADDRESS — unique by construction + // (distinct globals have distinct addresses), immortal (it is not + // GC memory, so it can never be freed and reused under a stale + // cache entry, which is why the string handles themselves cannot + // serve), and stable for the process. The runtime's site table + // then verifies a hit by comparing that one word, and never looks + // at the pattern at all. + // + // The slot is zero-initialised so it lands in `__bss` and costs + // nothing until the linker lays it out (#9610's lesson about + // zero-initialised globals applies: `private global i64 0`, not a + // non-zero initialiser). Naming carries the module prefix for the + // same reason `inline_cache_global_name` does — codegen-unit + // splitting can promote a private global for cross-unit use. + // + // FAILURE MODE, stated so it is diagnosable rather than surprising: + // `typed_parse_rodata` is drained into the module by all five + // lowering entry points, but `codegen/method.rs`'s + // "parent class has no callable constructor symbol" bail-out + // DISCARDS it (along with `ic_globals` and `pending_declares`) + // after having lowered the body. A regex literal inside such a + // constructor would therefore reference a global that is never + // defined. That is a LOUD compile-time failure — the in-process + // LLVM parse rejects it with `use of undefined value`, exactly as + // it rejected #9859's undeclared externs — and never a wrong + // answer at runtime. The same bail-out already discards IC globals + // and pending declares, so the path is either unreachable for + // non-trivial constructors or already broken for them; this note + // is here so the next person to see that message knows where to + // look. + let site_id = ctx.ic_site_counter; + ctx.ic_site_counter += 1; + let slot_name = { + let prefix = ctx.strings.module_prefix(); + if prefix.is_empty() { + format!("perry_regexp_site_{site_id}") + } else { + format!("perry_regexp_site_{prefix}__{site_id}") + } + }; + ctx.typed_parse_rodata + .push(format!("@{slot_name} = private global i64 0")); + let slot_ref = format!("@{slot_name}"); let blk = ctx.block(); let pattern_box = blk.load(DOUBLE, &pattern_global); let flags_box = blk.load(DOUBLE, &flags_global); let pattern_handle = unbox_to_i64(blk, &pattern_box); let flags_handle = unbox_to_i64(blk, &flags_box); + let site_key = blk.ptrtoint(&slot_ref, I64); let result = blk.call( I64, - "js_regexp_new", - &[(I64, &pattern_handle), (I64, &flags_handle)], + "js_regexp_new_site", + &[ + (I64, &pattern_handle), + (I64, &flags_handle), + (I64, &site_key), + ], ); Ok(nanbox_pointer_inline(blk, &result)) } diff --git a/crates/perry-codegen/src/runtime_decls/mod.rs b/crates/perry-codegen/src/runtime_decls/mod.rs index ec14cd207e..fab34b46eb 100644 --- a/crates/perry-codegen/src/runtime_decls/mod.rs +++ b/crates/perry-codegen/src/runtime_decls/mod.rs @@ -213,3 +213,56 @@ pub fn declare_phase_a_strings(module: &mut LlModule) { // function once they grow. declare_phase_b_strings(module); } + +#[cfg(test)] +mod tests { + use super::*; + + /// A lowering that introduces a new runtime call needs one test that + /// reaches the DECLARATION, not just the HIR. + /// + /// #9859 emitted five `js_segments_view_*` calls whose symbols were never + /// declared in the LLVM module: twelve HIR-level unit tests passed and the + /// first real compile died at the in-process LLVM parse with `use of + /// undefined value`. The arity half matters just as much and fails more + /// quietly — a wrong arity PARSES and miscompiles, handing the runtime a + /// garbage argument. + /// + /// `Expr::RegExp` lowers to `js_regexp_new_site(pattern, flags, site_key)` + /// (`expr/logical_collections.rs`), so the declaration must be exactly + /// three `i64` parameters returning `i64`. + #[test] + fn the_literal_site_regexp_entry_is_declared_with_its_exact_arity() { + let mut module = crate::module::LlModule::new("arm64-apple-macosx"); + declare_phase_b_strings(&mut module); + + let line = module + .declaration_lines() + .find(|(name, _)| *name == "js_regexp_new_site") + .map(|(_, line)| line.to_string()) + .expect( + "`Expr::RegExp` emits a call to `js_regexp_new_site`; without a `declare` the \ + module fails the in-process LLVM parse with `use of undefined value`, which no \ + HIR-level test can see", + ); + assert!( + line.starts_with("declare i64 @js_regexp_new_site(i64, i64, i64)"), + "the site-keyed entry takes (pattern handle, flags handle, site key) and returns a \ + RegExpHeader handle — a wrong arity parses and miscompiles instead of failing. Got: \ + {line}" + ); + + // The two-argument form stays, because every non-literal construction + // (`new RegExp(str)`, `js_regexp_construct`, the runtime's own + // callers) uses it and must never reach the site table. + let plain = module + .declaration_lines() + .find(|(name, _)| *name == "js_regexp_new") + .map(|(_, line)| line.to_string()) + .expect("the dynamic form must remain declared"); + assert!( + plain.starts_with("declare i64 @js_regexp_new(i64, i64)"), + "got: {plain}" + ); + } +} diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 765fe9c8e7..911e318a47 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1312,6 +1312,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { &[DOUBLE, DOUBLE, DOUBLE, I32, DOUBLE], ); module.declare_function("js_regexp_new", I64, &[I64, I64]); + // The literal-site form (`Expr::RegExp` lowering). A missing `declare` + // here is invisible to every HIR-level test and fails only at the + // in-process LLVM parse with `use of undefined value` — which is exactly + // how #9859's five segment-view externs were caught, after twelve passing + // unit tests. `runtime_decls::tests` asserts the name AND the arity: a + // wrong arity parses and miscompiles. + module.declare_function("js_regexp_new_site", I64, &[I64, I64, I64]); // Full ECMAScript RegExp constructor: NaN-boxed pattern + flags in, handles // RegExp/undefined/object patterns and ToString-coerced flags. module.declare_function("js_regexp_construct", I64, &[DOUBLE, DOUBLE]); diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 2836715f5e..3ca20c3cae 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -174,6 +174,12 @@ pub struct RegexDiag { /// `PtrHasher` hash plus a hashbrown insert, mirrored by two removals at /// death and two rekeys per evacuation. pub new_side_table_inserts: 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 + /// 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, per_pattern: HashMap, } @@ -293,7 +299,7 @@ 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={}", + side_table_inserts={} site_key_hit={}", self.new_calls, self.new_validated_hit, self.new_site_hit, @@ -322,6 +328,7 @@ impl RegexDiag { self.new_header_bytes, self.new_site_verify_bytes, self.new_side_table_inserts, + self.new_site_key_hit, ); // 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 24c33fd88c..648682f21c 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -69,6 +69,11 @@ mod replace_expand; mod replace_fn; #[cfg(feature = "regex-engine")] mod site_cache; +/// Literal-site keyed construction cache — identity by an immortal address +/// emitted per regex literal, so a hit costs one word compare instead of a +/// fingerprint plus a full byte compare of the pattern. +#[cfg(feature = "regex-engine")] +mod site_key; #[cfg(feature = "regex-engine")] mod unicode17; #[cfg(feature = "regex-engine")] @@ -975,6 +980,49 @@ fn newborn_barrier_gate_enabled() -> bool { pub extern "C" fn js_regexp_new( pattern: *const StringHeader, flags: *const StringHeader, +) -> *mut RegExpHeader { + js_regexp_new_impl(pattern, flags, 0) +} + +/// [`js_regexp_new`] for a **regex literal**, which the compiler can identify +/// by its source site instead of by its text. +/// +/// `site_key` is the address of an 8-byte private global the `Expr::RegExp` +/// lowering emits once per literal (`expr/logical_collections.rs`). It is +/// unique by construction, immortal, and never moves, which is what makes it a +/// sound identity where a `StringHeader` address is not: string headers are +/// GC-managed, so an address is freed and reused and a moving collector +/// relocates them, and a pointer-keyed cache over them would answer for a +/// different pattern. +/// +/// A hit therefore verifies with ONE word compare (plus the site's ≤ 8-byte +/// flags text) and never reads the pattern at all — no fingerprint, no +/// `memcmp`, no validation, no flag canonicalization. On claude-code the +/// segment loop constructs `string-width`'s ~12,807-character `/…/g` once per +/// grapheme, and the content cache's exactness verify alone is ~2.0 GB of +/// `memcmp` per 400-character reply. +/// +/// A `site_key` of 0 means "no site" and behaves exactly like +/// [`js_regexp_new`]; every dynamic construction (`new RegExp(s)`, +/// [`js_regexp_construct`], the runtime's own callers) keeps the two-argument +/// form and never touches the site table. +/// +/// Kill switch: `PERRY_REGEX_SITE_KEY=0`. +#[cfg(feature = "regex-engine")] +#[no_mangle] +pub extern "C" fn js_regexp_new_site( + pattern: *const StringHeader, + flags: *const StringHeader, + site_key: i64, +) -> *mut RegExpHeader { + js_regexp_new_impl(pattern, flags, site_key as usize) +} + +#[cfg(feature = "regex-engine")] +fn js_regexp_new_impl( + pattern: *const StringHeader, + flags: *const StringHeader, + site_key: usize, ) -> *mut RegExpHeader { // ★ `pattern` is a raw `StringHeader*` in a Rust local, and this function // allocates twice below (`js_string_from_str` for the canonical flags, then @@ -991,151 +1039,215 @@ pub extern "C" fn js_regexp_new( // in `js_regexp_new` itself, on BOTH sides of an unrelated codegen change. let scope = crate::gc::RuntimeHandleScope::new(); let pattern_root = scope.root_string_ptr(pattern); - let pattern_str = if is_valid_ptr(pattern) { - string_as_str(pattern) - } else { - "" - }; let raw_flags_str = if is_valid_ptr(flags) { string_as_str(flags) } else { "" }; - // #2829: reject duplicate/unknown flags (SyntaxError) and store the - // canonical sorted form so `.flags` reflects Node's ordering. - let canonical_flags = validate_and_canonicalize_flags(raw_flags_str); - let flags_str = canonical_flags.as_str(); - - // ★ Share the caller's flags string when it is ALREADY the canonical text. + // ★ LITERAL-SITE FAST PATH — identity by an immortal address. // - // `flags_ptr` used to be a fresh `js_string_from_str` on every - // construction. A JS regex literal evaluates to a fresh RegExp object - // every time it is reached, so that is one 32-byte GC string per - // evaluation: `PERRY_REGEX_DIAG` counts 161,897 constructions per - // 400-character claude-code reply, ~5.2 MB of identical one- and two-byte - // strings, and ~44 MB on a 3300-character reply. + // `site_key` is the address of a private global the compiler emits once + // per regex literal, so a match on it proves this is the SAME SOURCE SITE + // that recorded the entry, whose pattern and flags are fixed at compile + // time. Nothing about the pattern text is read: no fingerprint, no + // `memcmp`, no validation, no flag canonicalization. The flags text IS + // compared, because it is at most eight bytes and because two spellings of + // one canonical form (`/x/ig`, `/x/gi`) must not answer for each other. // - // JS strings are immutable and have no identity semantics, and a literal's - // flags text is written by the author in spec order (`/x/gi`, not - // `/x/ig`), so the caller's string usually IS the canonical text and can - // simply be shared. Nothing downstream depends on the pointer being fresh: - // `flags_ptr`-keyed lookups (`FANCY_CACHE`, `lookup_fancy_regex`) read it - // through `string_as_str` and compare CONTENT, and the header keeping a - // pointer to it is what keeps it alive. - // - // This comparison must happen HERE, before the validation block below, - // because `raw_flags_str` borrows the caller's GC string and that block - // can allocate. The root is taken here for the same reason: the raw - // `flags` argument may name from-space after any allocation, exactly as - // the ★ note on `pattern_root` says, and this one is stored into the - // header too. - let shared_flags_root = - (is_valid_ptr(flags) && raw_flags_str == flags_str).then(|| scope.root_string_ptr(flags)); - - let case_insensitive = flags_str.contains('i'); - let global = flags_str.contains('g'); - let multiline = flags_str.contains('m'); - let sticky = flags_str.contains('y'); - let dot_all = flags_str.contains('s'); - let unicode = flags_str.contains('u') || flags_str.contains('v'); - let has_indices = flags_str.contains('d'); - - // Content-keyed construction cache (`regex::site_cache`): a verified hit - // means this exact `(pattern, canonical flags)` already cleared the - // validation below — validity is a pure function of the pair — and hands - // back the shared owned copies plus, once some header built from this - // text has been executed, its compiled programs. The probe is one - // fingerprint and one byte compare; everything below it that copies or - // hashes the pattern is skipped. - let site_hit = site_cache::lookup(pattern_str, flags_str); - let validated_hit = - site_hit.is_some() || lazy::pattern_already_validated(pattern_str, flags_str); - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| { - d.note_new( - pattern as usize, - pattern_str.as_bytes(), - flags_str, - validated_hit && site_hit.is_none(), - site_hit.is_some(), + // 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 { + Some(hit) => { + // The site's own flags literal, so this is the same sharing + // decision the first construction at this site made (#9819). + let shared_flags_root = (hit.flags_are_canonical && is_valid_ptr(flags)) + .then(|| scope.root_string_ptr(flags)); + debug_assert!( + !is_valid_ptr(pattern) || string_as_str(pattern) == &*hit.pattern, + "a site key names ONE source literal, whose pattern text cannot change; a \ + caller that reuses a key for different text would silently take another \ + site's program" + ); + if crate::hot_diag::regex_on() { + let bytes: &[u8] = hit.pattern.as_bytes(); + let flags_text: &str = &hit.flags; + crate::hot_diag::regex_with(|d| { + d.new_site_key_hit += 1; + d.note_new(pattern as usize, bytes, flags_text, false, true); + }); + } + // Until the site's first execution installs the compiled programs, + // pick them up from the content cache — one probe per construction, + // and in a loop that matches immediately that is exactly one. + let programs = match hit.programs { + Some(programs) => Some(programs), + None => { + let picked = + site_cache::lookup(&hit.pattern, &hit.flags).and_then(|h| h.programs); + if let Some(programs) = picked.clone() { + site_key::install_programs(site_key, programs); + } + picked + } + }; + ( + hit.pattern, + hit.flags, + programs, + hit.bits, + shared_flags_root, ) - }); - } + } + None => { + let pattern_str = if is_valid_ptr(pattern) { + string_as_str(pattern) + } else { + "" + }; - // #2829: reject invalid pattern syntax with a SyntaxError. A pattern the - // `regex` crate rejects is only a real error if `fancy-regex` (which - // covers the full JS feature set: lookbehind/lookahead/backreferences) - // ALSO rejects it — otherwise it is a valid JS pattern we route through - // the fancy fallback. `get_or_compile_regex` populates FANCY_CACHE when - // the regex crate fails but fancy-regex succeeds; check both here. - // - // PERF (#5777 follow-up): the ENTIRE validation block runs at most once - // per (pattern, flags). Regex validity is a pure function of the pair, so - // a pattern that has already cleared it can never fail it later; the - // cheap JS-syntax checks are not actually cheap - // (`has_invalid_repeated_quantifier` does a - // `pattern.chars().collect::>()` — a ~51 KB allocation for a - // 12,807-char pattern — plus an O(n) scan on EVERY `new RegExp(...)`), - // and the common `string-width`/`emoji-regex` npm packages construct a - // fresh ~12,807-char `/…/g` literal on every measurement, which a layout - // pass calls thousands of times. #5777 keyed that skip off a REGEX_CACHE - // hit, which worked only because construction also COMPILED; with the - // build deferred, the fact is recorded directly in `VALIDATED_PATTERNS`. - { - if !validated_hit { - if has_invalid_repeated_quantifier(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // `--` is the real ClassSetExpression subtraction operator under - // the `v` flag (UTS #51) — `[a--z]` there means "a minus z", not - // a malformed range — so only legacy/`u`-mode patterns are - // subject to the doubled-hyphen range-order check. - if !flags_str.contains('v') && has_out_of_order_double_dash_class_range(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // Annex B.1.4 legacy escapes (`\1` non-backref octal, `\0DD`, `\8`/`\9`, - // `\c` without a control letter) are accepted in sloppy patterns but are - // a hard SyntaxError under the `/u` (and `/v`) flag — `js_regex_to_rust` - // would otherwise silently relax them. (test262 RegExp/ - // unicode_restricted_octal_escape + unicode_restricted_identity_escape_c) - if unicode && has_unicode_forbidden_legacy_escape(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); - } - // The remaining Annex B.1.4 leniencies (lone `]`/`}`, incomplete `{` - // quantifiers, `\d`-style range endpoints, quantified lookarounds, and - // forbidden IdentityEscapes) are likewise hard errors under `/u`. Gated - // on `u` specifically — `/v`'s ClassSetExpression grammar differs. - if flags_str.contains('u') && has_unicode_forbidden_pattern(pattern_str) { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); + // #2829: reject duplicate/unknown flags (SyntaxError) and store the + // canonical sorted form so `.flags` reflects Node's ordering. + let canonical_flags = validate_and_canonicalize_flags(raw_flags_str); + let flags_str = canonical_flags.as_str(); + + // ★ Share the caller's flags string when it is ALREADY the canonical text. + // + // `flags_ptr` used to be a fresh `js_string_from_str` on every + // construction. A JS regex literal evaluates to a fresh RegExp object + // every time it is reached, so that is one 32-byte GC string per + // evaluation: `PERRY_REGEX_DIAG` counts 161,897 constructions per + // 400-character claude-code reply, ~5.2 MB of identical one- and two-byte + // strings, and ~44 MB on a 3300-character reply. + // + // JS strings are immutable and have no identity semantics, and a literal's + // flags text is written by the author in spec order (`/x/gi`, not + // `/x/ig`), so the caller's string usually IS the canonical text and can + // simply be shared. Nothing downstream depends on the pointer being fresh: + // `flags_ptr`-keyed lookups (`FANCY_CACHE`, `lookup_fancy_regex`) read it + // through `string_as_str` and compare CONTENT, and the header keeping a + // pointer to it is what keeps it alive. + // + // This comparison must happen HERE, before the validation block below, + // because `raw_flags_str` borrows the caller's GC string and that block + // can allocate. The root is taken here for the same reason: the raw + // `flags` argument may name from-space after any allocation, exactly as + // the ★ note on `pattern_root` says, and this one is stored into the + // header too. + let flags_are_canonical = raw_flags_str == flags_str; + let shared_flags_root = + (is_valid_ptr(flags) && flags_are_canonical).then(|| scope.root_string_ptr(flags)); + // Materialized HERE, while `raw_flags_str`'s borrow of the caller's + // GC string is still guaranteed live: the validation block below + // can allocate, and the site record is written after it. + let raw_flags_owned: Arc = Arc::from(raw_flags_str); + + let case_insensitive = flags_str.contains('i'); + let global = flags_str.contains('g'); + let multiline = flags_str.contains('m'); + let sticky = flags_str.contains('y'); + let dot_all = flags_str.contains('s'); + let unicode = flags_str.contains('u') || flags_str.contains('v'); + let has_indices = flags_str.contains('d'); + + // Content-keyed construction cache (`regex::site_cache`): a verified hit + // means this exact `(pattern, canonical flags)` already cleared the + // validation below — validity is a pure function of the pair — and hands + // back the shared owned copies plus, once some header built from this + // text has been executed, its compiled programs. The probe is one + // fingerprint and one byte compare; everything below it that copies or + // hashes the pattern is skipped. + let site_hit = site_cache::lookup(pattern_str, flags_str); + let validated_hit = + site_hit.is_some() || lazy::pattern_already_validated(pattern_str, flags_str); + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| { + d.note_new( + pattern as usize, + pattern_str.as_bytes(), + flags_str, + validated_hit && site_hit.is_none(), + site_hit.is_some(), + ) + }); } - // The remaining question — "is this a SyntaxError?" — used to be - // answered by BUILDING the pattern, which is why constructing a - // regex cost an NFA. Ask the standard engine's PARSER instead - // (`lazy::std_engine_syntax_ok`, the same `regex_syntax` parse - // `build_std_regex` performs, on the same string): 17.8x cheaper, - // and it agrees with the full build on every one of the 2,378 - // regex literals in the claude-code bundle (asserted over a - // corpus by `tests::syntax_check_agrees_with_full_build`). + + // #2829: reject invalid pattern syntax with a SyntaxError. A pattern the + // `regex` crate rejects is only a real error if `fancy-regex` (which + // covers the full JS feature set: lookbehind/lookahead/backreferences) + // ALSO rejects it — otherwise it is a valid JS pattern we route through + // the fancy fallback. `get_or_compile_regex` populates FANCY_CACHE when + // the regex crate fails but fancy-regex succeeds; check both here. // - // A parser rejection is NOT a verdict: every lookbehind / - // backreference pattern is rejected by the linear engine too. Fall - // through to the unchanged both-engines path, which owns the - // SyntaxError decision and populates the caches for the fancy - // fallback. - if !lazy::std_engine_syntax_ok(pattern_str, flags_str) + // PERF (#5777 follow-up): the ENTIRE validation block runs at most once + // per (pattern, flags). Regex validity is a pure function of the pair, so + // a pattern that has already cleared it can never fail it later; the + // cheap JS-syntax checks are not actually cheap + // (`has_invalid_repeated_quantifier` does a + // `pattern.chars().collect::>()` — a ~51 KB allocation for a + // 12,807-char pattern — plus an O(n) scan on EVERY `new RegExp(...)`), + // and the common `string-width`/`emoji-regex` npm packages construct a + // fresh ~12,807-char `/…/g` literal on every measurement, which a layout + // pass calls thousands of times. #5777 keyed that skip off a REGEX_CACHE + // hit, which worked only because construction also COMPILED; with the + // build deferred, the fact is recorded directly in `VALIDATED_PATTERNS`. + { + if !validated_hit { + if has_invalid_repeated_quantifier(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // `--` is the real ClassSetExpression subtraction operator under + // the `v` flag (UTS #51) — `[a--z]` there means "a minus z", not + // a malformed range — so only legacy/`u`-mode patterns are + // subject to the doubled-hyphen range-order check. + if !flags_str.contains('v') + && has_out_of_order_double_dash_class_range(pattern_str) + { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // Annex B.1.4 legacy escapes (`\1` non-backref octal, `\0DD`, `\8`/`\9`, + // `\c` without a control letter) are accepted in sloppy patterns but are + // a hard SyntaxError under the `/u` (and `/v`) flag — `js_regex_to_rust` + // would otherwise silently relax them. (test262 RegExp/ + // unicode_restricted_octal_escape + unicode_restricted_identity_escape_c) + if unicode && has_unicode_forbidden_legacy_escape(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // The remaining Annex B.1.4 leniencies (lone `]`/`}`, incomplete `{` + // quantifiers, `\d`-style range endpoints, quantified lookarounds, and + // forbidden IdentityEscapes) are likewise hard errors under `/u`. Gated + // on `u` specifically — `/v`'s ClassSetExpression grammar differs. + if flags_str.contains('u') && has_unicode_forbidden_pattern(pattern_str) { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + // The remaining question — "is this a SyntaxError?" — used to be + // answered by BUILDING the pattern, which is why constructing a + // regex cost an NFA. Ask the standard engine's PARSER instead + // (`lazy::std_engine_syntax_ok`, the same `regex_syntax` parse + // `build_std_regex` performs, on the same string): 17.8x cheaper, + // and it agrees with the full build on every one of the 2,378 + // regex literals in the claude-code bundle (asserted over a + // corpus by `tests::syntax_check_agrees_with_full_build`). + // + // A parser rejection is NOT a verdict: every lookbehind / + // backreference pattern is rejected by the linear engine too. Fall + // through to the unchanged both-engines path, which owns the + // SyntaxError decision and populates the caches for the fancy + // fallback. + if !lazy::std_engine_syntax_ok(pattern_str, flags_str) // Cold: the linear engine's parser refused, so only a BUILD // can tell a fancy-regex pattern from a SyntaxError. // Materialising the `Arc` key happens once per distinct @@ -1143,49 +1255,91 @@ pub extern "C" fn js_regexp_new( && !compile_and_cache_regex_checked( &Arc::from(pattern_str), &Arc::from(flags_str), - ) - { - // Preserve the historical edge: validation used to test the - // BARE translated pattern (no `(?ims)` prefix). A pattern that - // compiles bare but blows the size limit with the flag prefix - // must stay a silent never-match (matching prior behavior), - // not a SyntaxError. - let translated = js_regex_to_rust(pattern_str); - if build_std_regex(&translated).is_err() && build_fancy_regex(&translated).is_err() - { - throw_regexp_syntax_error(&format!( - "Invalid regular expression: /{}/: invalid pattern", - pattern_str - )); + ) { + // Preserve the historical edge: validation used to test the + // BARE translated pattern (no `(?ims)` prefix). A pattern that + // compiles bare but blows the size limit with the flag prefix + // must stay a silent never-match (matching prior behavior), + // not a SyntaxError. + let translated = js_regex_to_rust(pattern_str); + if build_std_regex(&translated).is_err() + && build_fancy_regex(&translated).is_err() + { + throw_regexp_syntax_error(&format!( + "Invalid regular expression: /{}/: invalid pattern", + pattern_str + )); + } + } + lazy::mark_pattern_validated(pattern_str, flags_str); } } - lazy::mark_pattern_validated(pattern_str, flags_str); - } - } - // The compiled program is NOT built here. Validation above has already - // 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 - // "not built yet" state) and `lazy::ensure_regex_compiled` installs the - // owned `Arc`s on the first operation that needs a matcher. - - // ★ 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. - let (owned_pattern, owned_flags, programs) = match site_hit { - Some(hit) => (hit.pattern, hit.flags, hit.programs), - None => { - let (p, f) = site_cache::insert(pattern_str, flags_str); - (p, f, None) + // The compiled program is NOT built here. Validation above has already + // 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 + // "not built yet" state) and `lazy::ensure_regex_compiled` installs the + // owned `Arc`s on the first operation that needs a matcher. + + // ★ 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. + let (owned_pattern, owned_flags, programs) = match site_hit { + Some(hit) => (hit.pattern, hit.flags, hit.programs), + None => { + let (p, f) = site_cache::insert(pattern_str, flags_str); + (p, f, None) + } + }; + #[allow(unused_variables)] + let pattern_str: () = (); + + // Record what this construction established, so every later + // evaluation of this literal answers from the site key. Only ever + // written on the path that has already validated the pair — a + // hit legitimately skips validation because validity is a pure + // function of `(pattern, flags)`. + let bits = site_key::FlagBits { + case_insensitive, + global, + multiline, + sticky, + dot_all, + unicode, + has_indices, + }; + site_key::record( + site_key, + raw_flags_owned, + owned_pattern.clone(), + owned_flags.clone(), + flags_are_canonical, + bits, + programs.clone(), + ); + ( + owned_pattern, + owned_flags, + programs, + bits, + shared_flags_root, + ) } }; - #[allow(unused_variables)] - let pattern_str: () = (); + let site_key::FlagBits { + case_insensitive, + global, + multiline, + sticky, + dot_all, + unicode, + has_indices, + } = bits; // ★ The header is NURSERY-allocated, like an ordinary object. // @@ -1229,7 +1383,11 @@ pub extern "C" fn js_regexp_new( if crate::hot_diag::regex_on() { crate::hot_diag::regex_with(|d| d.new_flags_allocated += 1); } - scope.root_string_ptr(js_string_from_str(flags_str)) + // `owned_flags` IS the canonical text (the shared `Arc` the + // site or content cache handed back), and unlike `flags_str` it + // does not borrow the caller's GC string, so it is still valid + // here after the analysis above. + scope.root_string_ptr(js_string_from_str(&owned_flags)) } }; // ★ #7341: root the canonical flags string too. The header allocation below diff --git a/crates/perry-runtime/src/regex/site_key.rs b/crates/perry-runtime/src/regex/site_key.rs new file mode 100644 index 0000000000..7a60ba88d1 --- /dev/null +++ b/crates/perry-runtime/src/regex/site_key.rs @@ -0,0 +1,249 @@ +//! Literal-site keyed construction cache for `RegExp` — O(1), no hashing, no +//! byte compare. +//! +//! # Why this exists next to `site_cache` +//! +//! [`super::site_cache`] answers "have I seen this pattern TEXT before?" and +//! is what a dynamic `new RegExp(s)` needs. It is keyed by a content +//! fingerprint and, because a fingerprint can collide, every hit is verified +//! by a **full byte compare of the pattern**. That verify is linear in the +//! pattern, and a regex literal evaluates to a fresh object every time it is +//! reached: on claude-code the segment loop constructs `string-width`'s +//! ~12,807-character `/…/g` once per grapheme, so the verify alone is ~2.0 GB +//! of `memcmp` per 400-character reply and 39.6 % of `js_regexp_new`'s own +//! profile subtree. +//! +//! A literal does not need to be identified by its text. It is one source +//! site, and its pattern and flags are fixed at compile time. The compiler now +//! says so: `Expr::RegExp` emits an 8-byte private global per literal site and +//! passes its ADDRESS as `site_key` (`expr/logical_collections.rs`), and +//! [`js_regexp_new_site`](super::js_regexp_new_site) probes this table with +//! it. +//! +//! # Why the key is sound, and why the string handles are not +//! +//! Identity by address is only sound while the address cannot be reused for +//! something else. A `StringHeader` address fails that twice over — headers +//! are GC-managed, so an address is freed and reused, and a moving collector +//! relocates them — which is why the earlier analysis of this problem +//! concluded no sound string identity was available and left the content +//! compare in place. +//! +//! A per-site global has neither problem: it is emitted by the compiler into +//! the image, never freed, never moved, and distinct sites are distinct +//! globals and therefore distinct addresses. So an entry is verified by +//! comparing ONE WORD, and the pattern is never read at all — not hashed, not +//! fingerprinted, not compared. +//! +//! What that leaves per construction on a hit: two `Arc` refcount bumps for +//! the shared `(pattern, flags)` text, the program handles if the site has +//! been executed once, and the header allocation itself. No validation (the +//! first construction at this site did it, and validity is a pure function of +//! the pair), no flag canonicalization, no fingerprint, no `memcmp`. +//! +//! Kill switch: `PERRY_REGEX_SITE_KEY=0` (every probe misses and nothing is +//! recorded, so the construction falls through to the content-keyed path +//! exactly as before this existed). + +use std::cell::RefCell; +use std::sync::Arc; + +use super::site_cache::Programs; + +/// The flag bits `js_regexp_new` derives from the canonical flags text. They +/// are a pure function of the site's flags literal, so a hit reads them +/// instead of re-scanning the string seven times. +#[derive(Clone, Copy)] +pub(super) struct FlagBits { + pub(super) case_insensitive: bool, + pub(super) global: bool, + pub(super) multiline: bool, + pub(super) sticky: bool, + pub(super) dot_all: bool, + pub(super) unicode: bool, + pub(super) has_indices: bool, +} + +struct Entry { + key: usize, + /// The caller's flags text VERBATIM, as the site spells it. Compared on + /// every probe: flags are at most eight bytes, so the check is free, and + /// it makes the entry exact for a caller that is not the emitted lowering + /// (`/x/ig` and `/x/gi` are two spellings of one canonical form and must + /// not answer for each other's `flags_are_canonical`). + raw_flags: Arc, + pattern: Arc, + flags: Arc, + /// The caller's flags string already IS the canonical text, so the header + /// can share it instead of materializing a GC string (#9819). A property + /// of the site: the author wrote `/x/gi` or `/x/ig` once. + flags_are_canonical: bool, + bits: FlagBits, + programs: Option, +} + +/// What a construction gets back on a site hit. +pub(super) struct SiteHit { + pub(super) pattern: Arc, + pub(super) flags: Arc, + pub(super) flags_are_canonical: bool, + pub(super) bits: FlagBits, + pub(super) programs: Option, +} + +/// Direct-mapped, 2-way (a key may live in `slot` or `slot ^ 1`). A bundle's +/// live literal working set is small — claude-code holds 2,935 distinct +/// patterns across ~2,378 literal sites and a render cycles through a few +/// dozen. +const SLOTS: usize = 1024; + +crate::perry_thread_local! { + static SITE_KEY_TABLE: RefCell>> = RefCell::new(Vec::new()); +} + +fn enabled() -> bool { + static ON: std::sync::OnceLock = std::sync::OnceLock::new(); + *ON.get_or_init(|| { + crate::gc::env_default_on_from_value(std::env::var("PERRY_REGEX_SITE_KEY").ok().as_deref()) + }) +} + +/// The site global is 8-byte aligned, so the low three bits carry no +/// information; shift them out before masking. No hash — the key is already a +/// unique identity, and hashing it would be the cost this table exists to +/// remove. +#[inline] +fn slot_of(key: usize) -> usize { + (key >> 3) & (SLOTS - 1) +} + +/// The entry recorded for `key`, or `None`. +pub(super) fn lookup(key: usize, raw_flags: &str) -> Option { + if !enabled() || key == 0 { + return None; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let table = table.borrow(); + if table.is_empty() { + return None; + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &table[s] { + if entry.key == key && &*entry.raw_flags == raw_flags { + return Some(SiteHit { + pattern: entry.pattern.clone(), + flags: entry.flags.clone(), + flags_are_canonical: entry.flags_are_canonical, + bits: entry.bits, + programs: entry.programs.clone(), + }); + } + } + } + None + }) +} + +/// Record what the first construction at `key` established. Callers must pass +/// the validated, canonical values — an entry is only ever written on the path +/// that has already validated the pair. +pub(super) fn record( + key: usize, + raw_flags: Arc, + pattern: Arc, + flags: Arc, + flags_are_canonical: bool, + bits: FlagBits, + programs: Option, +) { + if !enabled() || key == 0 { + return; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let mut table = table.borrow_mut(); + if table.is_empty() { + table.resize_with(SLOTS, || None); + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &mut table[s] { + if entry.key == key && entry.raw_flags == raw_flags { + if entry.programs.is_none() { + entry.programs = programs; + } + return; + } + } + } + let victim = if table[slot].is_none() { + slot + } else if table[slot ^ 1].is_none() { + slot ^ 1 + } else { + // Both ways taken by other sites: evict the primary. A site whose + // entry is evicted simply falls back to the content-keyed path, + // which is correct and merely slower. + slot + }; + table[victim] = Some(Entry { + key, + raw_flags, + pattern, + flags, + flags_are_canonical, + bits, + programs, + }); + }); +} + +/// 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) { + if !enabled() || key == 0 { + return; + } + let slot = slot_of(key); + SITE_KEY_TABLE.with(|table| { + let mut table = table.borrow_mut(); + if table.is_empty() { + return; + } + for s in [slot, slot ^ 1] { + if let Some(entry) = &mut table[s] { + if entry.key == key && entry.programs.is_none() { + entry.programs = Some(programs); + return; + } + } + } + }); +} + +#[cfg(test)] +pub(super) fn test_reset() { + SITE_KEY_TABLE.with(|table| table.borrow_mut().clear()); +} + +/// The pattern text this site is recorded under, or `None`. Test-only: the +/// probe that lets a sabotage of `slot_of`/the key comparison be caught by a +/// test that constructs two different literals at two colliding sites. +/// +/// Takes the key in the **emitted lowering's type** (`i64`, what +/// `Expr::RegExp`'s `ptrtoint` produces and what the `js_regexp_new_site` +/// extern declares) and narrows it here, so a test holds exactly the value the +/// compiler passes and crosses the same `as usize` boundary the product entry +/// point does. The table itself is keyed by `usize` because the key IS an +/// address; the two spellings meet at the FFI edge and nowhere else. +#[cfg(test)] +pub(super) fn test_recorded_pattern(key: i64, raw_flags: &str) -> Option { + lookup(key as usize, raw_flags).map(|hit| hit.pattern.to_string()) +} + +/// How many slots hold an entry. Test-only: proves a dynamic +/// `new RegExp(str)` did NOT record anything. +#[cfg(test)] +pub(super) fn test_occupied_slots() -> usize { + SITE_KEY_TABLE.with(|table| table.borrow().iter().filter(|e| e.is_some()).count()) +} diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index cc5908b41e..76aad4a12d 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1948,3 +1948,174 @@ fn a_regexp_with_a_non_writable_lastindex_is_still_found_by_the_probe() { "an unrelated key on the same RegExp must still take the fast negative" ); } + +// --------------------------------------------------------------------------- +// Literal-site keyed construction (`js_regexp_new_site` + `regex::site_key`). +// +// The site key is the address of a private global the compiler emits once per +// regex literal. These statics stand in for two such globals: distinct +// addresses, 8-byte aligned, immortal — the same three properties the emitted +// ones have, and the reason the key is a sound identity where a `StringHeader` +// address is not. +// --------------------------------------------------------------------------- + +// Distinct initialisers so nothing may merge them: the test uses only their +// ADDRESSES, and identical zero-valued statics are exactly the shape a +// constant-merging pass is allowed to collapse. `assert_ne!` on the two keys +// keeps that from being a silent assumption. +static SITE_SLOT_A: u64 = 0xA; +static SITE_SLOT_B: u64 = 0xB; +static SITE_SLOT_C: u64 = 0xC; + +fn site_key_of(slot: &'static u64) -> i64 { + slot as *const u64 as i64 +} + +/// **The sabotage the coordinator named: key the table by pattern length.** +/// +/// Two literals at two sites, same flags, same pattern LENGTH, different text. +/// A table that verifies a probe by anything weaker than the site key — a +/// length, a prefix, a fingerprint without the exactness check — hands the +/// second site the first site's entry, and `.source` then reports a pattern +/// this literal never contained while `test` matches the wrong language. +/// +/// Each site is constructed twice: the first construction records, the second +/// is the one that must come back from the table, which is the case a weak key +/// breaks. Asserting only on a first construction would pass under every +/// sabotage, because a miss always takes the content-keyed path. +#[test] +fn two_literal_sites_with_equal_length_patterns_never_answer_for_each_other() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + let key_a = site_key_of(&SITE_SLOT_A); + let key_b = site_key_of(&SITE_SLOT_B); + assert_ne!(key_a, key_b, "two sites must have two addresses"); + + for round in 0..2 { + let a = js_regexp_new_site(make_string("a.c"), make_string("g"), key_a); + let b = js_regexp_new_site(make_string("x.z"), make_string("g"), key_b); + assert_eq!( + string_payload(js_regexp_get_source(a)), + b"a.c".to_vec(), + "round {round}: site A must report its own pattern" + ); + assert_eq!( + string_payload(js_regexp_get_source(b)), + b"x.z".to_vec(), + "round {round}: site B must report its own pattern — a table keyed by anything \ + weaker than the site address hands B the entry A recorded, and both patterns are \ + three bytes long" + ); + assert!( + js_regexp_test(a, make_string("abc")) != 0 + && js_regexp_test(a, make_string("xyz")) == 0, + "round {round}: site A must match its own language" + ); + assert!( + js_regexp_test(b, make_string("xyz")) != 0 + && js_regexp_test(b, make_string("abc")) == 0, + "round {round}: site B must match its own language" + ); + } + + assert_eq!( + site_key::test_recorded_pattern(key_a, "g").as_deref(), + Some("a.c") + ); + assert_eq!( + site_key::test_recorded_pattern(key_b, "g").as_deref(), + Some("x.z") + ); +} + +/// A dynamic `new RegExp(str)` must never reach the site table. +/// +/// The two-argument entry point is what every non-literal construction uses — +/// `js_regexp_construct`, `RegExp.prototype.compile`, the runtime's own +/// callers — and it has no site to be keyed by. If it recorded under some +/// stand-in key, a later literal whose key collided would inherit a pattern +/// that a *variable* produced, which is the one thing the site key's +/// compile-time-constant argument is supposed to guarantee against. +#[test] +fn a_dynamic_construction_records_nothing_in_the_site_table() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + for _ in 0..4 { + let re = js_regexp_new(make_string("dyn(amic)"), make_string("g")); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + } + assert_eq!( + site_key::test_occupied_slots(), + 0, + "the two-argument entry point has no site key and must record nothing" + ); + + // The same TEXT through the site entry does record — so the zero above is + // a property of the entry point, not of a table that never works. + let key = site_key_of(&SITE_SLOT_C); + let re = js_regexp_new_site(make_string("dyn(amic)"), make_string("g"), key); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + assert_eq!( + site_key::test_occupied_slots(), + 1, + "the site entry point must record — otherwise the assertion above proves nothing" + ); + assert_eq!( + site_key::test_recorded_pattern(key, "g").as_deref(), + Some("dyn(amic)") + ); +} + +/// 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. +/// +/// 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 +/// `test()` and give the site key nothing. +#[test] +fn a_site_hit_after_the_first_execution_is_born_built() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + let key = site_key_of(&SITE_SLOT_A); + + let first = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); + assert!( + unsafe { (*first).regex_ptr }.is_null(), + "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(), + "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(), + "a site hit must install the programs the site already compiled, so the header is born \ + built and the first match pays no lookup" + ); + assert_ne!(first, second, "each evaluation is still a distinct object"); + assert!(js_regexp_test(second, make_string("born")) != 0); +} + +/// The kill switch has to remove the lane, not just its answers: with +/// `PERRY_REGEX_SITE_KEY=0` the table records nothing, so the OFF arm is the +/// content-keyed path exactly rather than a control still paying the +/// bookkeeping. +/// +/// Read once per process through a `OnceLock`, so this asserts the DEFAULT is +/// on rather than flipping the variable mid-run (which would only test the +/// cache of the first read). +#[test] +fn the_site_key_lane_is_on_by_default() { + assert!( + crate::gc::env_default_on_from_value(None), + "the site-key lane defaults ON; `PERRY_REGEX_SITE_KEY=0` is the kill switch" + ); + assert!(!crate::gc::env_default_on_from_value(Some("0"))); +} From 7ef06502404a5a5d9b2cf8e3d92f436d4070ca17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 21:54:15 +0200 Subject: [PATCH 04/19] perf(regex): hold the site table's programs weakly, and stop the added counters moving the dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections from the I6 cc arm, both found by reading the instrument back rather than by argument. **1. The site table must never be the reason a program stays alive.** Measured on one 3300-char reply: settled footprint 478/474 MB -> 500/527 MB and idle CPU 2.37 -> 2.68 s against main. 1,024 entries at ~19 KB per compiled program is that order, and the campaign's directive is both metrics together — a CPU win bought with resident memory does not land. The entry now holds `Weak` / `Weak` / `Weak`; strong references stay where they belong, in the `(pattern, flags)` program caches and in every live header that installed them with `Arc::into_raw`. An entry whose programs have expired reports "not built yet" and the next construction re-picks them up from the content cache — the same path the site's first construction takes, so the lane self-heals. The upgrade is ALL-OR-NOTHING. #9801 fixed an incoherent triple — a standard program memoized beside a missing fancy fallback — which does not error, it silently never matches; three independent `Arc` lifetimes reintroduce exactly that shape unless one dead reference invalidates the whole entry. Pinned by a test that drops ONLY the fancy program and asserts the entry reports unbuilt, which the natural per-field upgrade fails. **2. An added counter moved the instrument's own sampling.** `regex_with` counts every call as an event and dumps every `TICK_EVERY` events after a second has passed, so a second probe on an already-instrumented path doubles that path's event rate and moves the snapshot a SIGKILLed process leaves behind. On the I6 pair that showed up as `new / t` 206 k/s vs 173 k/s between two arms whose per-call ratios agree to 0.13 %, i.e. the two files describe different windows of the same workload. `regex_counters` accumulates without ticking the dump clock, and the three counters that ride along on already instrumented paths (barrier gate outcome, side-table inserts, site-verify bytes) now use it. (cherry picked from commit 78e8b9df7d9d660f1d8ab94ede504807b8f2da5f) --- crates/perry-runtime/src/hot_diag.rs | 27 +++ crates/perry-runtime/src/regex.rs | 4 +- crates/perry-runtime/src/regex/site_cache.rs | 2 +- crates/perry-runtime/src/regex/site_key.rs | 184 ++++++++++++++++++- 4 files changed, 206 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index 3ca20c3cae..51615783ca 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -187,6 +187,33 @@ crate::perry_thread_local! { static REGEX_DIAG: RefCell = RefCell::new(RegexDiag::default()); } +/// Accumulate into the thread's regex counters WITHOUT ticking the dump clock. +/// +/// `regex_with` counts every call as an "event" and dumps every `TICK_EVERY` +/// events once a second has passed, so the snapshot a `SIGKILL`ed process +/// leaves behind lands wherever the event stream happened to be. Adding a +/// second probe to a path that already had one therefore does not just add a +/// counter — it **doubles that path's event rate and moves the last snapshot**, +/// which makes two arms' absolute counts describe different windows of the +/// same workload. +/// +/// Measured, on the I6 cc arm: the extra per-construction probes took +/// `new / t` from 206 k/s to 173 k/s between two arms whose per-call ratios +/// agree to 0.13 %. Counters that ride along on an already-instrumented path +/// use this entry point so the cadence stays the pre-change one and the +/// windows stay comparable. +#[inline] +pub fn regex_counters(f: impl FnOnce(&mut RegexDiag)) { + REGEX_DIAG.with(|d| { + let mut d = d.borrow_mut(); + if d.started.is_none() { + d.started = Some(Instant::now()); + d.last_dump = None; + } + f(&mut d); + }); +} + /// Run `f` against the thread's regex counters, then maybe dump. #[inline] pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) { diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index 648682f21c..d8a1ccc1ef 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -1473,7 +1473,7 @@ fn js_regexp_new_impl( let needs_barrier = !newborn_barrier_gate_enabled() || crate::gc::newborn_parent_needs_barrier(regexp_parent_addr); if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| { + crate::hot_diag::regex_counters(|d| { if needs_barrier { d.new_barrier_taken += 1; } else { @@ -1544,7 +1544,7 @@ fn js_regexp_new_impl( // 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_with(|d| d.new_side_table_inserts += 2); + crate::hot_diag::regex_counters(|d| d.new_side_table_inserts += 2); } // Issue #637: side-table owned copies of pattern + flags so diff --git a/crates/perry-runtime/src/regex/site_cache.rs b/crates/perry-runtime/src/regex/site_cache.rs index 3ba9ace469..bf0d3c1897 100644 --- a/crates/perry-runtime/src/regex/site_cache.rs +++ b/crates/perry-runtime/src/regex/site_cache.rs @@ -148,7 +148,7 @@ pub(super) fn lookup(pattern: &str, flags: &str) -> Option { // `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_with(|d| d.new_site_verify_bytes += n); + crate::hot_diag::regex_counters(|d| d.new_site_verify_bytes += n); } return Some(Hit { pattern: entry.pattern.clone(), diff --git a/crates/perry-runtime/src/regex/site_key.rs b/crates/perry-runtime/src/regex/site_key.rs index 7a60ba88d1..aff7f4f028 100644 --- a/crates/perry-runtime/src/regex/site_key.rs +++ b/crates/perry-runtime/src/regex/site_key.rs @@ -46,10 +46,59 @@ //! exactly as before this existed). use std::cell::RefCell; -use std::sync::Arc; +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. +/// +/// 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 +/// table is 1,024 entries and a compiled program is ~19 KB, so a table that +/// outlives the content cache's own eviction retains programs nothing else +/// 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. +struct WeakPrograms { + std: Weak<::regex::Regex>, + fancy: Option>, + repeat: Option>, +} + +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), + } + } + + /// 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 }) + } +} + /// The flag bits `js_regexp_new` derives from the canonical flags text. They /// are a pure function of the site's flags literal, so a hit reads them /// instead of re-scanning the string seven times. @@ -79,7 +128,7 @@ struct Entry { /// of the site: the author wrote `/x/gi` or `/x/ig` once. flags_are_canonical: bool, bits: FlagBits, - programs: Option, + programs: Option, } /// What a construction gets back on a site hit. @@ -136,7 +185,7 @@ pub(super) fn lookup(key: usize, raw_flags: &str) -> Option { flags: entry.flags.clone(), flags_are_canonical: entry.flags_are_canonical, bits: entry.bits, - programs: entry.programs.clone(), + programs: entry.programs.as_ref().and_then(WeakPrograms::upgrade), }); } } @@ -169,8 +218,19 @@ pub(super) fn record( for s in [slot, slot ^ 1] { if let Some(entry) = &mut table[s] { if entry.key == key && entry.raw_flags == raw_flags { - if entry.programs.is_none() { - entry.programs = programs; + // Refresh a reference whose programs have been dropped, + // rather than only filling an empty one: a dead weak and + // an absent entry mean the same thing here, and the + // former must be able to heal. + if let Some(programs) = &programs { + if entry + .programs + .as_ref() + .and_then(WeakPrograms::upgrade) + .is_none() + { + entry.programs = Some(WeakPrograms::downgrade(programs)); + } } return; } @@ -193,7 +253,7 @@ pub(super) fn record( flags, flags_are_canonical, bits, - programs, + programs: programs.as_ref().map(WeakPrograms::downgrade), }); }); } @@ -212,8 +272,14 @@ pub(super) fn install_programs(key: usize, programs: Programs) { } for s in [slot, slot ^ 1] { if let Some(entry) = &mut table[s] { - if entry.key == key && entry.programs.is_none() { - entry.programs = Some(programs); + if entry.key == key + && entry + .programs + .as_ref() + .and_then(WeakPrograms::upgrade) + .is_none() + { + entry.programs = Some(WeakPrograms::downgrade(&programs)); return; } } @@ -247,3 +313,105 @@ pub(super) fn test_recorded_pattern(key: i64, raw_flags: &str) -> Option pub(super) fn test_occupied_slots() -> usize { SITE_KEY_TABLE.with(|table| table.borrow().iter().filter(|e| e.is_some()).count()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// **The all-or-nothing rule, made able to fail.** + /// + /// #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. + #[test] + fn one_dead_reference_invalidates_the_whole_entry() { + 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 { + 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"); + 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); + 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" + ); + drop(std_program); + assert!(weak.upgrade().is_none()); + } + + /// The table must not be the reason a program stays alive: once nothing + /// else holds it, a recorded entry reports "not built yet" and the next + /// construction re-picks it up from the content cache. + #[test] + fn the_site_table_does_not_keep_a_program_alive() { + test_reset(); + let key = 0x5171_E000_usize; + let std_program = Arc::new(::regex::Regex::new("keepalive").expect("linear program")); + let programs = Programs { + std: std_program.clone(), + fancy: None, + repeat: None, + }; + record( + key, + Arc::from("g"), + Arc::from("keepalive"), + Arc::from("g"), + true, + FlagBits { + case_insensitive: false, + global: true, + multiline: false, + sticky: false, + dot_all: false, + unicode: false, + has_indices: false, + }, + Some(programs), + ); + assert!( + lookup(key, "g") + .expect("the entry was just recorded") + .programs + .is_some(), + "precondition: the entry answers with its programs while they are alive" + ); + + drop(std_program); + let hit = lookup(key, "g").expect("the entry itself survives"); + assert!( + hit.programs.is_none(), + "the site table holds programs WEAKLY: with every other reference gone the entry must \ + report unbuilt rather than keeping ~19 KB per slot alive on its own" + ); + assert_eq!( + &*hit.pattern, "keepalive", + "the entry's identity is unaffected — only its programs expire" + ); + test_reset(); + } +} From 93c2e64cdc0fb06b8f34515c9f551d5cacf7c7e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 21:54:15 +0200 Subject: [PATCH 05/19] docs(codegen): state the site-slot publish obligation now that #9890 is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment at the `Expr::RegExp` lowering described the artifact-discarding bail-out in `codegen/method.rs` in the present tense. #9896 fixed it: every return there now goes through `publish_lowered_fn_artifacts`, which drains all three collections and restores `llmod.ic_counter`, closing the duplicate site-id half as well. Rewritten as the obligation rather than the bug — every lowering exit must PUBLISH `typed_parse_rodata`, and a future early return that drops it breaks this site loudly at the in-process LLVM parse. A comment describing a hazard that no longer exists is a false lead, which is the thing it was written to prevent. (cherry picked from commit 91a77917f0848de87aa62f8330a2b6eafa4b8d8a) --- .../src/expr/logical_collections.rs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/crates/perry-codegen/src/expr/logical_collections.rs b/crates/perry-codegen/src/expr/logical_collections.rs index 83b5a40695..68b3be1a6e 100644 --- a/crates/perry-codegen/src/expr/logical_collections.rs +++ b/crates/perry-codegen/src/expr/logical_collections.rs @@ -1314,21 +1314,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // same reason `inline_cache_global_name` does — codegen-unit // splitting can promote a private global for cross-unit use. // - // FAILURE MODE, stated so it is diagnosable rather than surprising: - // `typed_parse_rodata` is drained into the module by all five - // lowering entry points, but `codegen/method.rs`'s - // "parent class has no callable constructor symbol" bail-out - // DISCARDS it (along with `ic_globals` and `pending_declares`) - // after having lowered the body. A regex literal inside such a - // constructor would therefore reference a global that is never - // defined. That is a LOUD compile-time failure — the in-process - // LLVM parse rejects it with `use of undefined value`, exactly as - // it rejected #9859's undeclared externs — and never a wrong - // answer at runtime. The same bail-out already discards IC globals - // and pending declares, so the path is either unreachable for - // non-trivial constructors or already broken for them; this note - // is here so the next person to see that message knows where to - // look. + // The slot must reach the module, so every lowering exit has to + // PUBLISH `typed_parse_rodata` rather than drop it. That was not + // true when this landed: `codegen/method.rs`'s "parent class has + // no callable constructor symbol" bail-out lowered the body and + // then discarded the three artifact collections, so a regex + // literal inside such a constructor would have referenced a + // global that is never defined (#9890, fixed by #9896 — every + // return now goes through `publish_lowered_fn_artifacts`, which + // also restores `llmod.ic_counter` and so closes the duplicate + // site-id half). Kept as a note because the obligation is real + // and unenforced: a future early return that drops the artifacts + // breaks this site, loudly, at the in-process LLVM parse (`use of + // undefined value`) rather than at runtime. let site_id = ctx.ic_site_counter; ctx.ic_site_counter += 1; let slot_name = { From 9e84f335a190ec646c1404b74e64971eda55c42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:58:53 +0200 Subject: [PATCH 06/19] perf(intl): answer the view mode's canonicality proof in loads, not a walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_segments_view_regexp_test` asks "is `RegExp.prototype.test` still the builtin?" twice per grapheme, and the proof cost more than the match it guards. Symbolised on the view arm of the string-width probe, the proof was ~13 % of the thread — `get_field_by_name_object_tail` 3.6, `js_object_get_field_by_name` 3.5, `get_accessor_descriptor` 2.1, `closure_get_dynamic_prop` 1.75, `RandomState::hash_one<&str>` 1.4 (it hashed the key string on every call), `js_object_get_prototype_of` 1.3 — against 0.8 % for `regexp_test_str_bounded`, the actual matching. The property belongs to `RegExp.prototype`, not to the call, so it is recorded once when the prototype's methods are installed: the prototype pointer, the FIELD INDEX of its own `test`, and the canonical closure value. A call reads that slot by index and compares — three loads — plus the per-key accessor Bloom bit off the meta record. Everything it can get wrong, it gets wrong in the declining direction: a replaced or deleted `test` no longer matches the recorded closure; a reshaped prototype makes the index hold something else, which also does not match; `defineProperty(proto,"test",{get})` leaves the data slot alone and is caught by the accessor bit; a reparented receiver is caught by `object_static_prototype`, which answers from the object's own meta record or an atomic "nothing was ever recorded" latch — no mutex, no chain walk. Deliberately NOT a flag invalidated from the property-set path: that design makes every property store in the program pay for this one question and adds an invalidation surface that fails silently. This one hooks no shared write path. `REGEXP_PROTOTYPE_TEST_WALKS` counts by-name walks. The fast path does none, so it counts realms rather than calls, and the tests pin the property that matters: 50 accepted calls, plus a second cursor and a second regex, add ZERO walks; and patching `RegExp.prototype.test` after the site is recorded makes the very next call decline, so the caller materialises and runs the user's function. Also removes `report_segview_counters`, which had no caller. The counters now have one — the test suite — and a comment says to wire a runtime-side printer when a rig run needs the numbers, not before. `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed, 0 failed. (cherry picked from commit 638b8327f9f254ddb33d9e40820d3e52c1581263) --- .../perry-runtime/src/intl/segments_view.rs | 115 ++++++++++--- .../src/object/regex_proto_thunks.rs | 154 ++++++++++++++++-- 2 files changed, 231 insertions(+), 38 deletions(-) diff --git a/crates/perry-runtime/src/intl/segments_view.rs b/crates/perry-runtime/src/intl/segments_view.rs index f33489004b..d76dea493d 100644 --- a/crates/perry-runtime/src/intl/segments_view.rs +++ b/crates/perry-runtime/src/intl/segments_view.rs @@ -52,6 +52,11 @@ static REGEXP_TEST_ACCEPTED: AtomicU64 = AtomicU64::new(0); static REGEXP_TEST_DECLINED: AtomicU64 = AtomicU64::new(0); fn diag_on() -> bool { + // Always on under test: a counter nothing increments cannot be asserted on, + // and the decline counters exist so a decline names its cause. + if cfg!(test) { + return true; + } static ON: std::sync::OnceLock = std::sync::OnceLock::new(); *ON.get_or_init(|| std::env::var("PERRY_SEGVIEW_DIAG").is_ok()) } @@ -63,29 +68,18 @@ fn bump(c: &AtomicU64) { } } -/// One line, on demand. A decline that names its own reason is the difference -/// between "the tier did not fire" and "the tier fired and found nothing". -pub fn report_segview_counters() { - if !diag_on() { - return; - } - eprintln!( - "[segview] opens={} declines: not_segmenter={} not_grapheme={} segment_patched={} \ - not_string={} not_utf8={} empty={} | nexts={} code_point_at={} materialise_segment={} \ - regexp_test: accepted={} declined={}", +/// The counters' consumer today is the test suite, which is why there is no +/// `report_*` function: a printer with no caller is dead code, and the tally +/// the campaign reads is the COMPILER-side `PERRY_SEGVIEW_DIAG` one. Wire a +/// runtime-side printer when a rig run needs these numbers, not before. +#[cfg(test)] +fn counters() -> [u64; 4] { + [ OPENS.load(Ordering::Relaxed), - DECLINE_NOT_SEGMENTER.load(Ordering::Relaxed), - DECLINE_NOT_GRAPHEME.load(Ordering::Relaxed), - DECLINE_SEGMENT_PATCHED.load(Ordering::Relaxed), DECLINE_NOT_STRING.load(Ordering::Relaxed), - DECLINE_NOT_UTF8.load(Ordering::Relaxed), - DECLINE_EMPTY.load(Ordering::Relaxed), NEXTS.load(Ordering::Relaxed), - CODE_POINT_ATS.load(Ordering::Relaxed), - MATERIALISE_SEGMENT.load(Ordering::Relaxed), - REGEXP_TEST_ACCEPTED.load(Ordering::Relaxed), - REGEXP_TEST_DECLINED.load(Ordering::Relaxed), - ); + crate::object::regex_proto_thunks::REGEXP_PROTOTYPE_TEST_WALKS.load(Ordering::Relaxed), + ] } // --- cursor plumbing -------------------------------------------------------- @@ -119,6 +113,7 @@ fn set_num_field(obj: *mut ObjectHeader, index: u32, value: usize) { /// dropped at the end of the call; a short (SSO) string is decoded into the /// caller's stack buffer, so neither case allocates and neither case leaks an /// address. +/// #[inline] fn with_input(cursor: *mut ObjectHeader, f: impl FnOnce(&str) -> R) -> Option { let value = crate::object::js_object_get_field(cursor, F_INPUT); @@ -713,6 +708,86 @@ mod view_mode_tests { ); } + /// The canonicality proof must be a LOAD, not a walk: the only by-name + /// lookup is the one-time recording at install. If this ever climbs with + /// the number of calls, the fast path is not the path being taken — which + /// is exactly the failure the counter exists to catch. + #[cfg(feature = "regex-engine")] + #[test] + fn canonicality_proof_walks_once_per_realm_not_once_per_call() { + let cursor = js_segments_view_open(grapheme_segmenter(), js_string("abcdef")); + assert!(cursor != 0.0); + let re = crate::regex::js_regexp_construct(js_string("[a-z]"), js_string("")); + let re_v = f64::from_bits(JSValue::pointer(re as *const u8).bits()); + assert_eq!(js_segments_view_next(cursor), 1.0); + let before = counters()[3]; + for _ in 0..50 { + let v = js_segments_view_regexp_test(cursor, re_v); + assert!(!is_undefined(v), "a plain regex must keep being accepted"); + } + assert_eq!( + counters()[3], + before, + "50 accepted calls must add ZERO by-name walks" + ); + // A second cursor and a second regex must not add one either: the + // recorded site belongs to the realm, not to the call or the receiver. + let cursor2 = js_segments_view_open(grapheme_segmenter(), js_string("xy")); + assert_eq!(js_segments_view_next(cursor2), 1.0); + let re2 = crate::regex::js_regexp_construct(js_string("[x-z]"), js_string("")); + let re2_v = f64::from_bits(JSValue::pointer(re2 as *const u8).bits()); + assert!(!is_undefined(js_segments_view_regexp_test(cursor2, re2_v))); + assert_eq!( + counters()[3], + before, + "a second cursor and regex must add no walks either" + ); + // NOT asserted: an absolute bound like `walks <= 1`. The unit harness + // resets arenas between tests and builds the prototype tower more than + // once in a process, so the per-process total counts REALMS, not calls. + // The property that matters — and the one that fails if the fast path + // stops being taken — is the delta above. + } + + /// SABOTAGE-SHAPED, kept as a test: patching `RegExp.prototype.test` AFTER + /// the site is recorded must make the very next call decline. + #[cfg(feature = "regex-engine")] + #[test] + fn a_patched_prototype_test_declines_on_the_next_call() { + let cursor = js_segments_view_open(grapheme_segmenter(), js_string("ab")); + assert_eq!(js_segments_view_next(cursor), 1.0); + let re = crate::regex::js_regexp_construct(js_string("[a-z]"), js_string("")); + let re_v = f64::from_bits(JSValue::pointer(re as *const u8).bits()); + assert!( + !is_undefined(js_segments_view_regexp_test(cursor, re_v)), + "accepted before the patch" + ); + + // Replace `RegExp.prototype.test` the way a program would. + let proto_ptr = crate::object::regex_proto_thunks::REGEXP_PROTOTYPE_TEST_SITE + .load(Ordering::Acquire) + & ((1i64 << 48) - 1); + assert!(proto_ptr != 0, "the site must have been recorded"); + let proto = proto_ptr as *mut ObjectHeader; + let key = crate::string::js_string_from_bytes(b"test".as_ptr(), 4); + let replacement = crate::closure::js_closure_alloc(patched_test_thunk as *const u8, 0); + crate::closure::js_register_closure_arity(patched_test_thunk as *const u8, 0); + crate::object::js_object_set_field_by_name( + proto, + key, + crate::value::js_nanbox_pointer(replacement as i64), + ); + assert!( + is_undefined(js_segments_view_regexp_test(cursor, re_v)), + "a replaced `RegExp.prototype.test` must make the view DECLINE, so \ + the caller materialises and runs the user's function" + ); + } + + extern "C" fn patched_test_thunk(_c: *const crate::closure::ClosureHeader) -> f64 { + f64::from_bits(JSValue::bool(true).bits()) + } + /// `_regexp_test` answers the same as the materialised call for a plain /// regex, and DECLINES (three-valued `undefined`) for a global one, whose /// `test` is stateful in `lastIndex`. diff --git a/crates/perry-runtime/src/object/regex_proto_thunks.rs b/crates/perry-runtime/src/object/regex_proto_thunks.rs index db5dba4de6..8b7a7415a4 100644 --- a/crates/perry-runtime/src/object/regex_proto_thunks.rs +++ b/crates/perry-runtime/src/object/regex_proto_thunks.rs @@ -316,41 +316,157 @@ fn regex_instance_or_throw(method: &str) -> *const crate::regex::RegExpHeader { )) } +crate::perry_thread_local! { + /// The realm's `RegExp.prototype`, and the field index its own `test` + /// occupies, packed as `(index << 48) | ptr`. Recorded once when the + /// prototype's methods are installed. + static REGEXP_PROTOTYPE_TEST_SITE_SLOT: std::sync::atomic::AtomicI64 = + const { std::sync::atomic::AtomicI64::new(0) }; + /// The canonical `test` closure as a NaN-boxed value, for an + /// identity compare against whatever the slot holds now. + static REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT: std::sync::atomic::AtomicI64 = + const { std::sync::atomic::AtomicI64::new(0) }; +} + +pub(crate) static REGEXP_PROTOTYPE_TEST_SITE: super::RealmAtomicI64 = + super::RealmAtomicI64::new(®EXP_PROTOTYPE_TEST_SITE_SLOT); +pub(crate) static REGEXP_PROTOTYPE_TEST_CLOSURE: super::RealmAtomicI64 = + super::RealmAtomicI64::new(®EXP_PROTOTYPE_TEST_CLOSURE_SLOT); + +/// How many by-name walks the canonicality proof has done in this process. +/// The fast path does none: the only walk is the one-time recording below, so +/// this must read **1 per realm**, not one per call. It is the counter that +/// says the fast path is actually the path being taken. +pub(crate) static REGEXP_PROTOTYPE_TEST_WALKS: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); + +const PROTO_PTR_MASK: i64 = (1i64 << 48) - 1; + /// Is `RegExp.prototype.test` still the builtin, for the regex `value`? /// /// The `Intl.Segmenter` view mode answers `regex.test(segment)` without /// materialising the segment, so it must not silently bypass a user -/// replacement. Same allocation-free proof as -/// `iterator_prototypes::prototype_next_is_canonical`: the prototype's OWN -/// `test` slot still holds a closure whose native entry is this module's -/// thunk, AND no accessor descriptor is recorded for `"test"` (a -/// `defineProperty(proto, "test", {get})` leaves the old closure in the data -/// slot). Any other state returns `false` and the caller declines. +/// replacement — and it asks this question TWICE PER GRAPHEME, so the question +/// has to be answered in loads. +/// +/// It used to be answered by `js_object_get_prototype_of` (the general spec +/// entry: proxy trap, Temporal cell, primitive-wrapper resolution by name) plus +/// a by-name own-field lookup that hashes `"test"` on every call. Symbolised, +/// that proof was **~13 % of the loop's thread** — +/// `get_field_by_name_object_tail` 3.6, `js_object_get_field_by_name` 3.5, +/// `get_accessor_descriptor` 2.1, `closure_get_dynamic_prop` 1.75, +/// `RandomState::hash_one<&str>` 1.4, `js_object_get_prototype_of` 1.3 — +/// against 0.8 % for the match it was guarding. +/// +/// The property being tested belongs to `RegExp.prototype`, not to the call, so +/// it is recorded once at install time: the prototype pointer, the FIELD INDEX +/// its `test` occupies, and the canonical closure value. A call then reads that +/// one slot by index and compares. Everything this can get wrong, it gets wrong +/// in the declining direction: +/// +/// * `test` replaced or deleted -> the slot no longer holds the recorded +/// closure -> decline; +/// * the prototype reshaped so the index means a different key -> the slot does +/// not hold the recorded closure -> decline; +/// * an accessor installed with `defineProperty(proto,"test",{get})`, which +/// leaves the old closure in the data slot -> the per-key accessor Bloom bit +/// catches it, read straight off the meta record; +/// * the receiver reparented, so the `test` it would resolve is not this one -> +/// `object_static_prototype` says a prototype was recorded -> decline. +/// +/// No invalidation hook on any shared write path, which is the alternative +/// design and the one that would make every property store in the program pay +/// for this. #[cfg(feature = "regex-engine")] pub(crate) fn regexp_prototype_test_is_canonical(value: f64) -> bool { - let proto = super::js_object_get_prototype_of(value); - let jv = crate::value::JSValue::from_bits(proto.to_bits()); - if !jv.is_pointer() { + let jv_recv = crate::value::JSValue::from_bits(value.to_bits()); + if !jv_recv.is_pointer() { return false; } - let proto_obj = jv.as_pointer::() as *mut ObjectHeader; - if proto_obj.is_null() { + let recv_addr = jv_recv.as_pointer::() as usize; + if recv_addr == 0 { return false; } - let own = super::js_object_get_own_field_or_undef(proto, b"test".as_ptr(), 4); - let own_jv = crate::value::JSValue::from_bits(own.to_bits()); - if !own_jv.is_pointer() { + // A regex with no recorded prototype still has its class default, which is + // the object recorded below. `object_static_prototype` answers from the + // object's own meta record, or from an atomic "nothing was ever recorded" + // latch — no mutex, no chain walk. + if super::prototype_chain::object_static_prototype(recv_addr).is_some() { return false; } - let closure = own_jv.as_pointer::(); - if closure.is_null() - || crate::closure::get_valid_func_ptr(closure) != regex_proto_test_thunk as *const u8 - { + let site = REGEXP_PROTOTYPE_TEST_SITE.load(std::sync::atomic::Ordering::Acquire); + let canonical = REGEXP_PROTOTYPE_TEST_CLOSURE.load(std::sync::atomic::Ordering::Acquire); + if site == 0 || canonical == 0 { return false; } + let proto_obj = (site & PROTO_PTR_MASK) as *mut ObjectHeader; + let index = (site >> 48) as u32; + if proto_obj.is_null() { + return false; + } + let current = crate::object::js_object_get_field(proto_obj, index); + if current.bits() != canonical as u64 { + return false; + } + // `defineProperty(proto, "test", { get })` leaves the data slot alone and + // records the accessor, so the identity compare above cannot see it. !super::descriptor_state::may_have_descriptor_entry(proto_obj as usize, "test", true) } +/// Record the prototype, the index of its own `test`, and the canonical +/// closure. Called once, from the installer below. +#[cfg(feature = "regex-engine")] +fn record_canonical_test_site(proto_obj: *mut ObjectHeader) { + REGEXP_PROTOTYPE_TEST_WALKS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let proto_value = crate::value::js_nanbox_pointer(proto_obj as i64); + let own = super::js_object_get_own_field_or_undef(proto_value, b"test".as_ptr(), 4); + let jv = crate::value::JSValue::from_bits(own.to_bits()); + if !jv.is_pointer() { + return; + } + // The index of the KEY `"test"` in the prototype's keys array IS its field + // index. Done once, at install, with the ordinary accessors. + let keys = unsafe { super::object_keys_array(proto_obj) }; + if keys.is_null() { + return; + } + let count = crate::array::js_array_length(keys); + let mut found: Option = None; + for i in 0..count { + let key = crate::array::js_array_get_f64(keys, i); + let matches = unsafe { + crate::string::js_string_key_matches_bytes( + crate::value::JSValue::from_bits(key.to_bits()), + b"test", + ) + }; + if matches { + found = Some(i as u32); + break; + } + } + let Some(index) = found else { + return; + }; + // The recorded index must actually hold the closure we just read, or the + // per-call load would compare the wrong slot. + if crate::object::js_object_get_field(proto_obj, index).bits() != own.to_bits() { + return; + } + if index as i64 > (i64::MAX >> 48) { + return; + } + let addr = proto_obj as i64; + if addr & !PROTO_PTR_MASK != 0 { + return; // an address that does not fit the packing: stay on the slow path + } + REGEXP_PROTOTYPE_TEST_CLOSURE.store(own.to_bits() as i64, std::sync::atomic::Ordering::Release); + REGEXP_PROTOTYPE_TEST_SITE.store( + ((index as i64) << 48) | addr, + std::sync::atomic::Ordering::Release, + ); +} + /// Install the real (brand-checking) `exec`/`test`/`toString`/`compile` /// prototype methods. `compile` is only installed here when the `regex-engine` /// feature is on; the fallback no-op (for builds without an engine) is installed @@ -361,6 +477,8 @@ pub(super) fn install_regex_proto_methods(proto_obj: *mut ObjectHeader) { ipm(proto_obj, "exec", regex_proto_exec_thunk as *const u8, 1); #[cfg(feature = "regex-engine")] ipm(proto_obj, "test", regex_proto_test_thunk as *const u8, 1); + #[cfg(feature = "regex-engine")] + record_canonical_test_site(proto_obj); // Annex B `compile` re-initializes the receiver in place. It needs a real // brand check so `RegExp.prototype.compile.call(non-regexp)` throws a // `TypeError` (test262 annexB `.../compile/this-{not-object,obj-not-regexp}`). From b2011ee2a794e832fc125a79fa92105e3bdfc48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 16:59:08 +0200 Subject: [PATCH 07/19] perf(intl): validate the view cursor's input once, not on every entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every view entry point re-derives the input `&str` from the cursor's traced slot per call — that is the §9a rooting contract and it stays — but each derivation also re-ran `std::str::from_utf8` over the WHOLE input. On the symbolised view arm of the string-width probe that was the top self symbol at 6.2 %. `open` already validates: it refuses an input that is not already a string primitive and runs `from_utf8` on its bytes before allocating the cursor. So the per-call validation re-establishes something the slot's only writer guaranteed. The borrow now uses `from_utf8_unchecked`, with the invariant written out where the `unsafe` is, in four checkable parts: `F_INPUT` is written exactly once, by `open`, and never reassigned; `open` validated that value; a collection MOVES the string but never rewrites its bytes, and the traced slot is updated to the new address; the SSO path decodes the same value into the stack buffer. A `debug_assert` re-checks it in debug builds, which is where a future second writer to the slot would be caught. `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed, 0 failed. (cherry picked from commit ea54f0a3deb3702126541ecd30150af952f07335) --- .../perry-runtime/src/intl/segments_view.rs | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/intl/segments_view.rs b/crates/perry-runtime/src/intl/segments_view.rs index d76dea493d..6abf3bc5fa 100644 --- a/crates/perry-runtime/src/intl/segments_view.rs +++ b/crates/perry-runtime/src/intl/segments_view.rs @@ -114,13 +114,40 @@ fn set_num_field(obj: *mut ObjectHeader, index: u32, value: usize) { /// caller's stack buffer, so neither case allocates and neither case leaks an /// address. /// +/// **The UTF-8 validation is done ONCE, in `open`, and is not repeated here.** +/// Re-validating cost 6.2 % of the loop's thread as `core::str::from_utf8` +/// self, because every entry point re-derives the borrow per call (the §9a +/// rooting contract) and each derivation walked the whole input again. +/// +/// The invariant that makes `from_utf8_unchecked` sound here, stated so it can +/// be checked rather than trusted: +/// +/// 1. `F_INPUT` is written exactly once, by `js_segments_view_open`, and never +/// reassigned — no entry point below stores into it. +/// 2. `open` refuses any input that is not already a string primitive and runs +/// `std::str::from_utf8` on its bytes before allocating the cursor, so the +/// value in that slot has been validated. +/// 3. A collection may MOVE that string but never rewrites its bytes, and the +/// traced slot is updated to the new address, so the bytes reachable here +/// are the same bytes `open` validated. +/// 4. The SSO path decodes the same value into the stack buffer, so it carries +/// the same guarantee. +/// +/// A `debug_assert` re-checks it in debug builds, which is where a future +/// fourth writer to the slot would be caught. #[inline] fn with_input(cursor: *mut ObjectHeader, f: impl FnOnce(&str) -> R) -> Option { let value = crate::object::js_object_get_field(cursor, F_INPUT); let mut sso = [0u8; crate::value::SHORT_STRING_MAX_LEN]; let bytes = unsafe { crate::string::js_string_key_bytes(JSValue::from_bits(value.bits()), &mut sso) }?; - let text = std::str::from_utf8(bytes).ok()?; + debug_assert!( + std::str::from_utf8(bytes).is_ok(), + "the cursor's input slot is written once, by `open`, from a value it \ + validated — a failure here means a second writer appeared" + ); + // SAFETY: invariants 1-4 above. + let text = unsafe { std::str::from_utf8_unchecked(bytes) }; Some(f(text)) } From 46eae8a5808fed50ee26a2f8ecd3b273f44602f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:21:19 +0200 Subject: [PATCH 08/19] fix(intl): scan the recorded RegExp.prototype site as GC roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonicality fast path records the prototype address and the canonical `test` closure and reads them on every call — and neither was scanned. An address held across a collection without being visited is stale the first time the collector moves the object, which is the #9539 / #9445 shape and exactly what this campaign keeps finding. Nothing had failed yet because a realm prototype is long-lived and rarely moves; that is luck, not a design. The packed `(index << 48) | ptr` word is split so each part can be handled correctly: * `REGEXP_PROTOTYPE_PTR` — a raw address, visited by `scan_object_cache_roots_mut` with `visit_atomic_i64_slot` beside the iterator-prototype towers, so a move rewrites it; * `REGEXP_PROTOTYPE_TEST_CLOSURE` — a NaN-boxed word, visited with `visit_atomic_nanbox_u64_slot` and stored through `runtime_store_root_atomic_nanbox_u64` with the GC_STORE_AUDIT(ROOT) note the other mutable roots carry, so the pointer inside it is rewritten too; * the field index is not an address and stays an ordinary atomic. The per-call cost is unchanged — three loads and the accessor Bloom bit — and the identity compare stays an identity compare across a move, because both sides are now maintained by the collector. `cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,239 passed, 0 failed. (cherry picked from commit f076656e5e2c5e9b43ad7ffe638cd79ddfead352) --- .../perry-runtime/src/intl/segments_view.rs | 5 +- crates/perry-runtime/src/object/mod.rs | 12 ++++ .../src/object/regex_proto_thunks.rs | 71 ++++++++++--------- 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/crates/perry-runtime/src/intl/segments_view.rs b/crates/perry-runtime/src/intl/segments_view.rs index 6abf3bc5fa..fbadacf6a9 100644 --- a/crates/perry-runtime/src/intl/segments_view.rs +++ b/crates/perry-runtime/src/intl/segments_view.rs @@ -791,9 +791,8 @@ mod view_mode_tests { ); // Replace `RegExp.prototype.test` the way a program would. - let proto_ptr = crate::object::regex_proto_thunks::REGEXP_PROTOTYPE_TEST_SITE - .load(Ordering::Acquire) - & ((1i64 << 48) - 1); + let proto_ptr = + crate::object::regex_proto_thunks::REGEXP_PROTOTYPE_PTR.load(Ordering::Acquire); assert!(proto_ptr != 0, "the site must have been recorded"); let proto = proto_ptr as *mut ObjectHeader; let key = crate::string::js_string_from_bytes(b"test".as_ptr(), 4); diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 914f7e0989..77a02c25a4 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -1295,11 +1295,23 @@ pub fn scan_object_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<' &iterator_prototypes::STRING_ITERATOR_PROTOTYPE_PTR, &iterator_prototypes::REGEXP_STRING_ITERATOR_PROTOTYPE_PTR, &iterator_prototypes::ITERATOR_HELPER_PROTOTYPE_PTR, + // The realm's `RegExp.prototype`, recorded by `regex_proto_thunks` so + // the view mode's canonicality proof is three loads instead of a walk. + // A recorded address MUST be scanned: unscanned, it is a stale pointer + // the first time the collector moves the prototype. + #[cfg(feature = "regex-engine")] + ®ex_proto_thunks::REGEXP_PROTOTYPE_PTR, ] { slot.with_slot(|slot| { visitor.visit_atomic_i64_slot(slot, Ordering::Acquire, Ordering::Release); }); } + // The canonical `test` closure is a NaN-boxed word, not a bare address, so + // it is visited as one — the collector rewrites the pointer inside it. + #[cfg(feature = "regex-engine")] + regex_proto_thunks::REGEXP_PROTOTYPE_TEST_CLOSURE.with_slot(|slot| { + visitor.visit_atomic_nanbox_u64_slot(slot, Ordering::Acquire, Ordering::Release); + }); } /// Drive the PRODUCTION shape-cache writer from a test. Deliberately nothing diff --git a/crates/perry-runtime/src/object/regex_proto_thunks.rs b/crates/perry-runtime/src/object/regex_proto_thunks.rs index 8b7a7415a4..c11ff4a563 100644 --- a/crates/perry-runtime/src/object/regex_proto_thunks.rs +++ b/crates/perry-runtime/src/object/regex_proto_thunks.rs @@ -317,21 +317,26 @@ fn regex_instance_or_throw(method: &str) -> *const crate::regex::RegExpHeader { } crate::perry_thread_local! { - /// The realm's `RegExp.prototype`, and the field index its own `test` - /// occupies, packed as `(index << 48) | ptr`. Recorded once when the - /// prototype's methods are installed. - static REGEXP_PROTOTYPE_TEST_SITE_SLOT: std::sync::atomic::AtomicI64 = - const { std::sync::atomic::AtomicI64::new(0) }; - /// The canonical `test` closure as a NaN-boxed value, for an - /// identity compare against whatever the slot holds now. - static REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT: std::sync::atomic::AtomicI64 = + /// The realm's `RegExp.prototype`. A raw heap address, so it is a GC ROOT: + /// visited in `scan_object_cache_roots_mut` beside the iterator-prototype + /// towers, which both marks it and rewrites it when the collector moves the + /// object. A recorded address that is not scanned is a stale pointer the + /// first time the prototype moves — the #9539/#9445 shape. + static REGEXP_PROTOTYPE_PTR_SLOT: std::sync::atomic::AtomicI64 = const { std::sync::atomic::AtomicI64::new(0) }; + /// The canonical `test` closure, NaN-boxed. Also a root, visited as a + /// nanbox word so the collector rewrites the pointer inside it. + static REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT: std::sync::atomic::AtomicU64 = + const { std::sync::atomic::AtomicU64::new(0) }; + /// The field index its own `test` occupies. Not an address, so not a root. + static REGEXP_PROTOTYPE_TEST_INDEX_SLOT: std::sync::atomic::AtomicU32 = + const { std::sync::atomic::AtomicU32::new(u32::MAX) }; } -pub(crate) static REGEXP_PROTOTYPE_TEST_SITE: super::RealmAtomicI64 = - super::RealmAtomicI64::new(®EXP_PROTOTYPE_TEST_SITE_SLOT); -pub(crate) static REGEXP_PROTOTYPE_TEST_CLOSURE: super::RealmAtomicI64 = - super::RealmAtomicI64::new(®EXP_PROTOTYPE_TEST_CLOSURE_SLOT); +pub(crate) static REGEXP_PROTOTYPE_PTR: super::RealmAtomicI64 = + super::RealmAtomicI64::new(®EXP_PROTOTYPE_PTR_SLOT); +pub(crate) static REGEXP_PROTOTYPE_TEST_CLOSURE: super::RealmAtomicU64 = + super::RealmAtomicU64::new(®EXP_PROTOTYPE_TEST_CLOSURE_SLOT); /// How many by-name walks the canonicality proof has done in this process. /// The fast path does none: the only walk is the one-time recording below, so @@ -340,8 +345,6 @@ pub(crate) static REGEXP_PROTOTYPE_TEST_CLOSURE: super::RealmAtomicI64 = pub(crate) static REGEXP_PROTOTYPE_TEST_WALKS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); -const PROTO_PTR_MASK: i64 = (1i64 << 48) - 1; - /// Is `RegExp.prototype.test` still the builtin, for the regex `value`? /// /// The `Intl.Segmenter` view mode answers `regex.test(segment)` without @@ -394,18 +397,19 @@ pub(crate) fn regexp_prototype_test_is_canonical(value: f64) -> bool { if super::prototype_chain::object_static_prototype(recv_addr).is_some() { return false; } - let site = REGEXP_PROTOTYPE_TEST_SITE.load(std::sync::atomic::Ordering::Acquire); + let proto_ptr = REGEXP_PROTOTYPE_PTR.load(std::sync::atomic::Ordering::Acquire); let canonical = REGEXP_PROTOTYPE_TEST_CLOSURE.load(std::sync::atomic::Ordering::Acquire); - if site == 0 || canonical == 0 { - return false; - } - let proto_obj = (site & PROTO_PTR_MASK) as *mut ObjectHeader; - let index = (site >> 48) as u32; - if proto_obj.is_null() { + let index = REGEXP_PROTOTYPE_TEST_INDEX_SLOT + .with(|slot| slot.load(std::sync::atomic::Ordering::Acquire)); + if proto_ptr == 0 || canonical == 0 || index == u32::MAX { return false; } + let proto_obj = proto_ptr as *mut ObjectHeader; + // Both reads below are of values the collector maintains: the prototype + // address is a scanned root, and the recorded closure is a scanned nanbox + // word, so a move rewrites both and this compare stays an identity compare. let current = crate::object::js_object_get_field(proto_obj, index); - if current.bits() != canonical as u64 { + if current.bits() != canonical { return false; } // `defineProperty(proto, "test", { get })` leaves the data slot alone and @@ -453,18 +457,19 @@ fn record_canonical_test_site(proto_obj: *mut ObjectHeader) { if crate::object::js_object_get_field(proto_obj, index).bits() != own.to_bits() { return; } - if index as i64 > (i64::MAX >> 48) { - return; - } let addr = proto_obj as i64; - if addr & !PROTO_PTR_MASK != 0 { - return; // an address that does not fit the packing: stay on the slow path - } - REGEXP_PROTOTYPE_TEST_CLOSURE.store(own.to_bits() as i64, std::sync::atomic::Ordering::Release); - REGEXP_PROTOTYPE_TEST_SITE.store( - ((index as i64) << 48) | addr, - std::sync::atomic::Ordering::Release, - ); + REGEXP_PROTOTYPE_TEST_INDEX_SLOT + .with(|slot| slot.store(index, std::sync::atomic::Ordering::Release)); + // GC_STORE_AUDIT(ROOT): REGEXP_PROTOTYPE_TEST_CLOSURE is a mutable nanbox + // root visited by scan_object_cache_roots_mut. + REGEXP_PROTOTYPE_TEST_CLOSURE.with_slot(|slot| { + crate::gc::runtime_store_root_atomic_nanbox_u64( + slot, + own.to_bits(), + std::sync::atomic::Ordering::Release, + ); + }); + REGEXP_PROTOTYPE_PTR.store(addr, std::sync::atomic::Ordering::Release); } /// Install the real (brand-checking) `exec`/`test`/`toString`/`compile` From 219427a3a3ceff17579293e9399da4e372d22dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 18:55:27 +0200 Subject: [PATCH 09/19] fix(hir): scope native assignment instances (cherry picked from commit f2a0f3e777af7d2a09832bace252667f0607a649) --- .../9904-native-instance-assignment-scope.md | 8 ++ crates/perry-hir/src/lower/context.rs | 6 +- crates/perry-hir/src/lower/expr_assign.rs | 65 +++++++++------- .../tests/native_instance_binding_scope.rs | 76 +++++++++++++++++++ 4 files changed, 123 insertions(+), 32 deletions(-) create mode 100644 changelog.d/9904-native-instance-assignment-scope.md diff --git a/changelog.d/9904-native-instance-assignment-scope.md b/changelog.d/9904-native-instance-assignment-scope.md new file mode 100644 index 0000000000..42d6a7cdf2 --- /dev/null +++ b/changelog.d/9904-native-instance-assignment-scope.md @@ -0,0 +1,8 @@ +### Fixed + +- Native instances assigned with `target = new NativeClass(...)` or propagated + with `target = source` are now tracked by the resolved binding rather than by + identifier text across the whole module. A native handle named `O` can no + longer make unrelated bindings named `O` dispatch ordinary methods through + that native class, while module-level handles and unresolved global fallbacks + retain their existing cross-function behavior. diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 0a5826a26d..1305c29fa1 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -1641,7 +1641,7 @@ impl LoweringContext { .filter(|(_, module, class)| !exposes_plain_object_fields(module, class)) .map(|(_, module, class)| (module.as_str(), class.as_str())) .or_else(|| { - // #9847: a bare assignment (`O = cp.spawn(...)`) tags the + // #9847/#9858: assignment-derived native tags use the // RESOLVED binding, not the spelling. Consulted before the // name-keyed module-wide table below, so a same-named binding // in another function is simply a different binding and cannot @@ -1721,8 +1721,8 @@ impl LoweringContext { /// #9847: tag the RESOLVED binding `id` as holding a native instance. /// - /// Used by the bare-assignment path (`O = cp.spawn(...)`) in place of - /// `push_module_native_instance`, whose name key was module-wide: in a + /// Used by native-instance assignment paths in place of name-keyed + /// module-wide registration: in a /// minified single-module bundle a single native handle poisoned every /// homonym in the program. Keyed on the `LocalId` the target resolves to, /// this keeps the cross-function reach the module-wide table was there to diff --git a/crates/perry-hir/src/lower/expr_assign.rs b/crates/perry-hir/src/lower/expr_assign.rs index 123f562e05..a947b6cdf9 100644 --- a/crates/perry-hir/src/lower/expr_assign.rs +++ b/crates/perry-hir/src/lower/expr_assign.rs @@ -165,6 +165,26 @@ fn lower_logical_assignment( Ok(Expr::Logical { op, left, right }) } +fn register_assignment_native_instance( + ctx: &mut LoweringContext, + var_name: String, + module_name: String, + class_name: String, + register_scoped_fallback: bool, +) { + if let Some(local_id) = ctx.lookup_local(&var_name) { + ctx.register_local_id_native_instance(local_id, module_name, class_name); + return; + } + + // An unresolvable assignment target has no LocalId to key on. Preserve + // the former name-keyed registrations for that global fallback path. + if register_scoped_fallback { + ctx.register_native_instance(var_name.clone(), module_name.clone(), class_name.clone()); + } + ctx.push_module_native_instance((var_name, module_name, class_name)); +} + pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) -> Result { // Detect assignments from native module calls and register for cross-function tracking. // e.g., `mongoClient = await MongoClient.connect(uri)` registers mongoClient as a mongodb instance. @@ -221,22 +241,13 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) // key on and keeps the old name-keyed // registration; see the matching arm in // `lookup_native_instance`. - match ctx.lookup_local(&var_name) { - Some(local_id) => { - ctx.register_local_id_native_instance( - local_id, - module_name.to_string(), - class_name.to_string(), - ); - } - None => { - ctx.push_module_native_instance(( - var_name.clone(), - module_name.to_string(), - class_name.to_string(), - )); - } - } + register_assignment_native_instance( + ctx, + var_name.clone(), + module_name.to_string(), + class_name.to_string(), + false, + ); } } } @@ -252,16 +263,13 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) .lookup_native_module(class_name_str) .map(|(m, _)| m.to_string()); if let Some(module_name) = native_info { - ctx.register_native_instance( - var_name.clone(), - module_name.clone(), - class_name_str.to_string(), - ); - ctx.push_module_native_instance(( + register_assignment_native_instance( + ctx, var_name.clone(), module_name, class_name_str.to_string(), - )); + true, + ); } } } @@ -269,12 +277,11 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr) // e.g., `mongoClient = client` where client was tracked from MongoClient.connect(). if let ast::Expr::Ident(rhs_ident) = inner_rhs { let rhs_name = rhs_ident.sym.as_ref(); - if let Some((module, class)) = ctx.lookup_native_instance(rhs_name) { - ctx.push_module_native_instance(( - var_name, - module.to_string(), - class.to_string(), - )); + let native_info = ctx + .lookup_native_instance(rhs_name) + .map(|(module, class)| (module.to_string(), class.to_string())); + if let Some((module, class)) = native_info { + register_assignment_native_instance(ctx, var_name, module, class, false); } } } diff --git a/crates/perry-hir/tests/native_instance_binding_scope.rs b/crates/perry-hir/tests/native_instance_binding_scope.rs index 1d9fe3731d..b756ddc609 100644 --- a/crates/perry-hir/tests/native_instance_binding_scope.rs +++ b/crates/perry-hir/tests/native_instance_binding_scope.rs @@ -208,3 +208,79 @@ export function widthLike(q: any): number { {arm_b}" ); } + +const NEW_ASSIGNMENT_FIXTURE: &str = r#" +import { BlockList } from "net"; + +export function maker(): any { + let O: any; + O = new BlockList(); + O.addSubnet("10.0.0.0", 8); + return O; +} + +export function widthLike(q: any): number { + let Y = 0; + for (let { segment: O } of q) { + Y += O.codePointAt(0) >= 4352 ? 2 : 1; + } + return Y; +} +"#; + +const PROPAGATED_ASSIGNMENT_FIXTURE: &str = r#" +import * as cp from "child_process"; + +export function copier(): any { + let source: any; + source = cp.spawn("true", []); + let O: any; + O = source; + O.kill(); + return O; +} + +export function widthLike(q: any): number { + let Y = 0; + for (let { segment: O } of q) { + Y += O.codePointAt(0) >= 4352 ? 2 : 1; + } + return Y; +} +"#; + +fn assert_unrelated_width_binding_is_ordinary(module: &perry_hir::Module) { + let width_like = body_of(module, "widthLike"); + assert!( + !width_like.contains("method: \"codePointAt\""), + "the unrelated for-of binding must not inherit a native-instance tag: \ + {width_like}" + ); + assert!( + width_like.contains("property: \"codePointAt\""), + "the string binding's method should remain an ordinary property call: \ + {width_like}" + ); +} + +#[test] +fn a_native_constructor_assignment_does_not_tag_an_unrelated_homonym() { + let module = lower(NEW_ASSIGNMENT_FIXTURE); + let maker = body_of(&module, "maker"); + assert!( + maker.contains("module: \"net\"") && maker.contains("method: \"addSubnet\""), + "the assigned BlockList binding must retain native dispatch: {maker}" + ); + assert_unrelated_width_binding_is_ordinary(&module); +} + +#[test] +fn a_propagated_native_assignment_does_not_tag_an_unrelated_homonym() { + let module = lower(PROPAGATED_ASSIGNMENT_FIXTURE); + let copier = body_of(&module, "copier"); + assert!( + copier.contains("module: \"child_process\"") && copier.contains("method: \"kill\""), + "the propagated handle binding must retain native dispatch: {copier}" + ); + assert_unrelated_width_binding_is_ordinary(&module); +} From aab391ed9f0a260dbf4457f3ed30bd0bceac1196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 19:32:29 +0200 Subject: [PATCH 10/19] fix(runtime): link cluster default prototype (cherry picked from commit 4f49591c523a346cb639463f1e31785aa9d16b30) --- changelog.d/9905-cluster-default-prototype.md | 5 ++ crates/perry-runtime/src/cluster.rs | 46 +++++++++++++++++-- .../perry-runtime/src/object/native_module.rs | 2 +- 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 changelog.d/9905-cluster-default-prototype.md diff --git a/changelog.d/9905-cluster-default-prototype.md b/changelog.d/9905-cluster-default-prototype.md new file mode 100644 index 0000000000..79e6c468fb --- /dev/null +++ b/changelog.d/9905-cluster-default-prototype.md @@ -0,0 +1,5 @@ +### Fixed + +- The `node:cluster` default export now inherits from the canonical + `EventEmitter.prototype`, so reflective prototype checks agree with Node while + preserving the cached singleton used by cluster event methods. diff --git a/crates/perry-runtime/src/cluster.rs b/crates/perry-runtime/src/cluster.rs index 4dfcc26d8e..2798e8a4cc 100644 --- a/crates/perry-runtime/src/cluster.rs +++ b/crates/perry-runtime/src/cluster.rs @@ -241,11 +241,51 @@ thread_local! { /// `import cluster from "node:cluster"`. Cached (see /// `should_cache_native_module_namespace`), so EventEmitter methods can return /// it for `cluster.on(...) === cluster` chaining. -fn cluster_default_value() -> f64 { - crate::object::js_create_native_module_namespace( +pub(crate) fn cluster_default_value() -> f64 { + let scope = crate::gc::RuntimeHandleScope::new(); + let cluster = scope.root_nanbox_f64(crate::object::js_create_native_module_namespace( b"cluster.default".as_ptr(), "cluster.default".len(), - ) + )); + let event_emitter = scope.root_nanbox_f64(crate::object::bound_native_callable_export_value( + "events", + "EventEmitter", + )); + if let Some(event_emitter_proto) = + crate::object::ordinary_function_prototype_value_for_read(event_emitter.get_nanbox_f64()) + { + let event_emitter_proto = scope.root_nanbox_f64(event_emitter_proto); + let cluster_addr = (cluster.get_nanbox_u64() & crate::value::POINTER_MASK) as usize; + let proto_bits = event_emitter_proto.get_nanbox_u64(); + if crate::object::prototype_chain::object_static_prototype(cluster_addr) != Some(proto_bits) + { + crate::object::prototype_chain::object_set_static_prototype(cluster_addr, proto_bits); + } + } + cluster.get_nanbox_f64() +} + +#[cfg(test)] +mod cluster_default_prototype_tests { + use super::*; + + #[test] + fn default_export_inherits_from_event_emitter() { + let scope = crate::gc::RuntimeHandleScope::new(); + let cluster = scope.root_nanbox_f64(cluster_default_value()); + let event_emitter = scope.root_nanbox_f64( + crate::object::bound_native_callable_export_value("events", "EventEmitter"), + ); + let expected = crate::object::ordinary_function_prototype_value_for_read( + event_emitter.get_nanbox_f64(), + ) + .expect("EventEmitter must expose a prototype object"); + + assert_eq!( + crate::object::js_object_get_prototype_of(cluster.get_nanbox_f64()).to_bits(), + expected.to_bits(), + ); + } } fn cluster_emitter_event_name(event: f64) -> Option { diff --git a/crates/perry-runtime/src/object/native_module.rs b/crates/perry-runtime/src/object/native_module.rs index b7440e4687..d427223761 100644 --- a/crates/perry-runtime/src/object/native_module.rs +++ b/crates/perry-runtime/src/object/native_module.rs @@ -694,7 +694,7 @@ pub(crate) fn cjs_default_export_value(module_name: &str) -> Option { // #3687: `node:cluster` default import is a distinct EventEmitter-shaped // `cluster.default` namespace (its `on`/`emit`/… reads diverge from the // bare `import * as` namespace). - "cluster" => create_cjs_default_namespace("cluster"), + "cluster" => Some(crate::cluster::cluster_default_value()), // #3693: `node:dgram` default === the module namespace (CJS // `module.exports`); a cached singleton makes `dgram === ns.default`. "dgram" => Some(js_create_native_module_namespace( From b5dbdfe2bb5102fa164483ebff34a1fcda526eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 19:50:52 +0200 Subject: [PATCH 11/19] fix(stream): wait for both finished sides (cherry picked from commit 23f0e503ee99b18a9b7b882f63acd65db0d184d9) --- changelog.d/9906-stream-finished-duplex.md | 5 ++++ crates/perry-runtime/src/node_stream.rs | 25 ++++++++++++++++ .../src/node_stream_constructors/pipeline.rs | 2 +- .../perry-runtime/src/node_stream_dispatch.rs | 1 + crates/perry-runtime/src/node_stream_tests.rs | 29 +++++++++++++++++++ 5 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9906-stream-finished-duplex.md diff --git a/changelog.d/9906-stream-finished-duplex.md b/changelog.d/9906-stream-finished-duplex.md new file mode 100644 index 0000000000..6931f15687 --- /dev/null +++ b/changelog.d/9906-stream-finished-duplex.md @@ -0,0 +1,5 @@ +### Fixed + +- Callback-form `stream.finished()` now waits for both sides of a duplex stream, + so ending an unread `PassThrough` does not report completion before its + readable side emits `end`. diff --git a/crates/perry-runtime/src/node_stream.rs b/crates/perry-runtime/src/node_stream.rs index 722648d1a6..dc13ddf430 100644 --- a/crates/perry-runtime/src/node_stream.rs +++ b/crates/perry-runtime/src/node_stream.rs @@ -373,6 +373,31 @@ extern "C" fn ns_finished_error_false_close(closure: *const ClosureHeader) -> f6 f64::from_bits(TAG_UNDEFINED) } +extern "C" fn ns_finished_default_completion(closure: *const ClosureHeader) -> f64 { + if closure.is_null() || js_closure_get_capture_f64(closure, 2).to_bits() == TAG_TRUE { + return f64::from_bits(TAG_UNDEFINED); + } + let stream = js_closure_get_capture_f64(closure, 0); + let readable_done = !js_node_stream_has_readable_side(stream) + || has_truthy_hidden(stream, hidden_end_emitted_key()); + let writable_done = !js_node_stream_has_writable_side(stream) + || has_truthy_hidden(stream, hidden_finish_emitted_key()); + let closed = has_truthy_hidden(stream, hidden_key(b"closed")); + let error = readable_hidden_error(stream); + if error.is_none() && !closed && !(readable_done && writable_done) { + return f64::from_bits(TAG_UNDEFINED); + } + + js_closure_set_capture_f64(closure as *mut ClosureHeader, 2, f64::from_bits(TAG_TRUE)); + let callback = js_closure_get_capture_f64(closure, 1); + if let Some(error) = error { + call_listener_args(stream, callback, &[error]); + } else { + call_listener_args(stream, callback, &[]); + } + f64::from_bits(TAG_UNDEFINED) +} + extern "C" fn ns_finished_signal_abort(closure: *const ClosureHeader) -> f64 { if closure.is_null() { return f64::from_bits(TAG_UNDEFINED); diff --git a/crates/perry-runtime/src/node_stream_constructors/pipeline.rs b/crates/perry-runtime/src/node_stream_constructors/pipeline.rs index 27ab7fa395..39c95c9388 100644 --- a/crates/perry-runtime/src/node_stream_constructors/pipeline.rs +++ b/crates/perry-runtime/src/node_stream_constructors/pipeline.rs @@ -182,7 +182,7 @@ pub(super) fn add_finished_signal_abort_listener(stream: f64, signal: f64, callb } pub(super) fn add_finished_cleanup_completion_listener(stream: f64, callback: f64) { - let listener = js_closure_alloc(ns_finished_error_false_close as *const u8, 3); + let listener = js_closure_alloc(ns_finished_default_completion as *const u8, 3); js_closure_set_capture_f64(listener, 0, stream); js_closure_set_capture_f64(listener, 1, callback); js_closure_set_capture_f64(listener, 2, f64::from_bits(TAG_FALSE)); diff --git a/crates/perry-runtime/src/node_stream_dispatch.rs b/crates/perry-runtime/src/node_stream_dispatch.rs index 7fc0cc5ef2..7bdc6e3c8e 100644 --- a/crates/perry-runtime/src/node_stream_dispatch.rs +++ b/crates/perry-runtime/src/node_stream_dispatch.rs @@ -604,6 +604,7 @@ pub(super) fn register_stub_arities() { 1, ); register(ns_finished_error_false_close as *const u8, 0); + register(ns_finished_default_completion as *const u8, 0); register(ns_finished_signal_abort as *const u8, 0); register(ns_iter_to_array as *const u8, 1); register(ns_iter_map as *const u8, 2); diff --git a/crates/perry-runtime/src/node_stream_tests.rs b/crates/perry-runtime/src/node_stream_tests.rs index 19aa3f717a..1ba98c5c90 100644 --- a/crates/perry-runtime/src/node_stream_tests.rs +++ b/crates/perry-runtime/src/node_stream_tests.rs @@ -28,12 +28,41 @@ thread_local! { static TRANSFORM_THIS_HAS_STREAM_STATE: RefCell> = const { RefCell::new(Vec::new()) }; static TRANSFORM_FLUSH_COUNT: RefCell = const { RefCell::new(0) }; static UNCAUGHT_STREAM_ERROR_COUNT: RefCell = const { RefCell::new(0) }; + static FINISHED_CALLBACK_COUNT: RefCell = const { RefCell::new(0) }; } fn catches_runtime_throw(f: impl FnOnce()) -> bool { crate::exception::catch_js_throw(f).is_err() } +extern "C" fn capture_finished_callback(_closure: *const ClosureHeader) -> f64 { + FINISHED_CALLBACK_COUNT.with(|count| *count.borrow_mut() += 1); + f64::from_bits(TAG_UNDEFINED) +} + +#[test] +fn finished_waits_for_both_passthrough_sides() { + FINISHED_CALLBACK_COUNT.with(|count| *count.borrow_mut() = 0); + crate::closure::js_register_closure_arity(capture_finished_callback as *const u8, 0); + + let stream = js_node_stream_passthrough_new(f64::from_bits(TAG_UNDEFINED)); + let callback = + box_pointer(js_closure_alloc(capture_finished_callback as *const u8, 0) as *const u8); + let mut args = crate::array::js_array_alloc(2); + args = crate::array::js_array_push_f64(args, stream); + args = crate::array::js_array_push_f64(args, callback); + js_node_stream_finished(args); + + let handle = raw_ptr_from_value(stream) as i64; + js_node_stream_method_end(handle, string_value("done")); + let _ = crate::promise::js_promise_run_microtasks(); + FINISHED_CALLBACK_COUNT.with(|count| assert_eq!(*count.borrow(), 0)); + + js_node_stream_method_resume(handle); + let _ = crate::promise::js_promise_run_microtasks(); + FINISHED_CALLBACK_COUNT.with(|count| assert_eq!(*count.borrow(), 1)); +} + pub(super) fn string_value(s: &str) -> f64 { let ptr = crate::string::js_string_from_bytes(s.as_ptr(), s.len() as u32); box_string(ptr) From 64ea0a5290faf2821e95998ce6b73576f20c4fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 20:11:06 +0200 Subject: [PATCH 12/19] fix(sqlite): keep statement iterators exhausted (cherry picked from commit 48cf508bbbdebac67d36fe8b705ccb085a401164) --- changelog.d/9909-sqlite-iterate-exhaustion.md | 1 + crates/perry-runtime/src/array/iter_object.rs | 64 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 changelog.d/9909-sqlite-iterate-exhaustion.md diff --git a/changelog.d/9909-sqlite-iterate-exhaustion.md b/changelog.d/9909-sqlite-iterate-exhaustion.md new file mode 100644 index 0000000000..f5a6cdd675 --- /dev/null +++ b/changelog.d/9909-sqlite-iterate-exhaustion.md @@ -0,0 +1 @@ +Fix `DatabaseSync` statement iterators so they remain exhausted after a `for...of` loop. A later `.next()` on the same iterator now returns `{ done: true, value: null }` instead of restarting from the first row. diff --git a/crates/perry-runtime/src/array/iter_object.rs b/crates/perry-runtime/src/array/iter_object.rs index 3276f932b8..d5c08b37e1 100644 --- a/crates/perry-runtime/src/array/iter_object.rs +++ b/crates/perry-runtime/src/array/iter_object.rs @@ -734,8 +734,9 @@ unsafe fn dispatch_array_iterator_method_inner( // Field 0: backing array pointer (NaN-boxed). let backing_field = js_object_get_field(iter_obj(), 0); let backing_f64 = f64::from_bits(backing_field.bits()); - // Array iterators clear their backing array at exhaustion. SQLite's - // statement iterator restarts a completed execution on the next call. + // Iterators clear their backing array at exhaustion. A completed + // SQLite statement iterator is also permanently closed; calling + // `StatementSync::iterate()` again creates a separate iterator. if JSValue::from_bits(backing_f64.to_bits()).is_undefined() { return crate::iter_result::emit_iter_result_cached( &scope, @@ -763,12 +764,7 @@ unsafe fn dispatch_array_iterator_method_inner( }; if idx >= len { - if kind == KIND_VALUES_NULL_DONE { - // SQLite's statement iterator restarts on the next call. - js_object_set_field(iter_obj(), 1, JSValue::number(0.0)); - } else { - js_object_set_field(iter_obj(), 0, JSValue::undefined()); - } + js_object_set_field(iter_obj(), 0, JSValue::undefined()); return crate::iter_result::emit_iter_result_cached( &scope, &iter_h, @@ -851,3 +847,55 @@ unsafe fn dispatch_array_iterator_method_inner( _ => f64::from_bits(TAG_UNDEFINED), } } + +#[cfg(test)] +mod sqlite_iterator_tests { + use super::*; + use std::sync::atomic::AtomicU64; + + unsafe fn result_fields(result: f64) -> (u64, u64) { + let result = js_nanbox_get_pointer(result) as *mut ObjectHeader; + ( + js_object_get_field(result, 0).bits(), + js_object_get_field(result, 1).bits(), + ) + } + + #[test] + fn sqlite_iterator_stays_exhausted_after_fused_for_of_drain() { + let _serialized = crate::array::test_serialize(); + let epoch = AtomicU64::new(0); + let rows = crate::array::js_array_push_f64(crate::array::js_array_alloc(1), 7.0); + let scope = crate::gc::RuntimeHandleScope::new(); + let iter_h = scope.root_nanbox_f64(array_values_iter_null_done( + js_nanbox_pointer(rows as i64), + &epoch, + 0, + )); + let iter = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; + + unsafe { + // Model the optimized `for...of` driver: one yielded row followed + // by its terminal advance. + let first = dispatch_array_iterator_method_emit(iter(), "next", true, true); + assert_eq!( + result_fields(first), + (crate::value::TAG_FALSE, 7.0f64.to_bits()) + ); + let done = dispatch_array_iterator_method_emit(iter(), "next", true, true); + assert_eq!( + result_fields(done), + (crate::value::TAG_TRUE, crate::value::TAG_NULL) + ); + + // The same iterator must remain closed when user code calls + // `.next()` after the loop. Resetting its cursor used to return + // the first row again here. + let after = dispatch_array_iterator_method(iter(), "next"); + assert_eq!( + result_fields(after), + (crate::value::TAG_TRUE, crate::value::TAG_NULL) + ); + } + } +} From 1c0075842cfb9d0dc3c194ef6db6653e3f332ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 19:37:19 +0200 Subject: [PATCH 13/19] fix(compile): exclude PERRY_SEGVIEW from the build cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PERRY_SEGVIEW` is not a diagnostic — it changes the emitted code — and it is in neither the build-cache fingerprint nor any object-cache key. So a cached build can return a binary compiled with the OTHER setting: compile a source with the tier on, compile it again with the tier off, and the second can be served from the first. That is precisely the shape every A/B in this campaign uses — one compiler binary, two compiles of one source differing only in this variable — so the failure mode is not a broken build, it is two arms that are secretly the same binary and a measured difference of zero, or two arms swapped. Silent, and it would look like a result. `PERRY_SEGVIEW_DIAG` was already excluded for the weaker reason that a cached build prints no report. The switch that changes codegen was not, which is the worse omission of the two and mine. Excluded rather than keyed because the tier is experimental and default OFF. A cache key is the right fix when it ships on by default; exclusion is correct now and cannot produce a stale entry that becomes the measurement. (cherry picked from commit a423a1b635bd19b5431bf4a302ac23766ca4b579) --- crates/perry/src/commands/compile/build_cache.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 2e50446cf8..43d9730a4f 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -866,6 +866,19 @@ fn eligibility(args: &CompileArgs, project_root: &Path) -> Result<(), String> { if std::env::var("PERRY_SEGVIEW_DIAG").is_ok() { return Err("segview-diag".to_string()); } + // #9843: `PERRY_SEGVIEW` is NOT a diagnostic — it changes the emitted + // code. It is not part of the build-cache fingerprint or any object-cache + // key, so without this a cached build can hand back a binary compiled with + // the OTHER setting: compile a file with the tier on, compile it again + // with the tier off, and the second can be served from the first. The + // A/B rig's whole shape is "one compiler binary, two compiles of one + // source differing only in this variable", which is exactly the collision. + // Excluded rather than keyed because the tier is experimental and default + // OFF; a cache key is the right fix when it ships on, and then a stale + // entry cannot silently become the measurement. + if std::env::var("PERRY_SEGVIEW").is_ok() { + return Err("segview-lowering".to_string()); + } if args.verify_native_regions || args.emit_attest || args.emit_sandbox { return Err("sidecar-or-verify".to_string()); } From b5f0fc6b2a1596d30dd468d14c7c946f6dbe4016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 17:34:42 +0200 Subject: [PATCH 14/19] fix(codegen): clear the segment-view cursor at loop exit The cursor local is declared in the ENCLOSING statement list, not inside the loop: let __segview_recv = let __segview_input = let __segview_cursor = js_segments_view_open(recv, inp) let __segview_iter = cur != 0 ? undefined : GetIterator(...) For { ... } <- last read of the cursor so without a clear its slot stays a live GC root until the function returns. The cursor holds the input string in a traced slot, so a cursor promoted during the loop drags that string into the old generation, and leaving the slot rooted afterwards keeps a DEAD cursor doing it for the rest of the function. `string-width` is entered thousands of times per reply. That is a candidate mechanism for the idle behaviour measured on cc: I4 settles 45-65 MB ABOVE I3 at 3300 and 15-20 MB at 400 after 120 s, despite winning 20-50 MB of PEAK RSS in 12/12 paired runs. Lower peak with a higher floor is not "less garbage"; it is something being retained. One unconditional `LocalSet(cursor, undefined)` after the loop covers both paths: on the declined path the local holds `0.0`, a number, so the clear is a no-op. `break` reaches it; `return` inside the body pops the frame, which is equally fine. WHAT THIS DOES NOT DO, stated so the commit is not read as a cure: it does not prevent promotion DURING the loop, and nothing in the compiler can, because the cursor is genuinely live there. It removes only the post-loop rooting of a dead cursor. If the idle delta comes from cursors promoted mid-loop, this will not move it. perrymaster's old-gen census after idle, counting class id 0xFFFF_000E on I5-spec / I5-view / I7-view, decides that independently. The test is structural rather than string-matched: it locates the rewritten `For`, reads the cursor's LocalId out of the loop's own guard, and requires the next statement to be `LocalSet(that id, Undefined)`. It fails if the clear is removed, clears the wrong local, or is emitted before the loop. 17/17 segview tests. (cherry picked from commit 15ea8eb62679cfd667094d2f28c90763b7a66415) --- .../perry-codegen/src/collectors/segview.rs | 21 ++++++++- .../src/collectors/segview_tests.rs | 43 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/crates/perry-codegen/src/collectors/segview.rs b/crates/perry-codegen/src/collectors/segview.rs index 90da00a26f..990f31b646 100644 --- a/crates/perry-codegen/src/collectors/segview.rs +++ b/crates/perry-codegen/src/collectors/segview.rs @@ -1221,7 +1221,26 @@ fn rewrite_site(list: &mut Vec, i: usize, site: &SegmentForOfSite, fresh: pick(cur, Expr::Undefined, decline_iter), ), ); - 3 + // Clear the cursor at loop exit. The cursor local is declared in the + // ENCLOSING statement list, not inside the loop, so without this its slot + // stays a live GC root until the function returns. A cursor that spans a + // minor while the loop runs is promoted, and because it holds the input + // string in a traced slot it drags that string into the old generation + // with it — one per `open`, and `string-width` is entered thousands of + // times per reply. That is a candidate mechanism for I4 settling 45-65 MB + // ABOVE I3 after idle despite winning 20-50 MB of peak. + // + // One unconditional clear covers both paths: on the declined path the + // local holds `0.0`, a number, so clearing it is a no-op. `break` reaches + // this statement; `return` inside the body pops the frame, which is + // equally fine. It does not prevent promotion DURING the loop — nothing + // in the compiler can, since the cursor is genuinely live there — it stops + // the slot from keeping a dead cursor rooted for the rest of the function. + list.insert( + i + 5, + Stmt::Expr(Expr::LocalSet(cur, Box::new(Expr::Undefined))), + ); + 4 } fn unwrap_for_mut(s: &mut Stmt) -> &mut Stmt { diff --git a/crates/perry-codegen/src/collectors/segview_tests.rs b/crates/perry-codegen/src/collectors/segview_tests.rs index 51b7ae71d5..9783faf640 100644 --- a/crates/perry-codegen/src/collectors/segview_tests.rs +++ b/crates/perry-codegen/src/collectors/segview_tests.rs @@ -509,3 +509,46 @@ fn a_site_with_an_unanswerable_use_stays_on_v1() { "v1 does not rewrite uses: {out}" ); } + +/// The cursor local is declared in the enclosing statement list, so its slot is +/// a live GC root until the function returns unless the lowering clears it. A +/// cursor promoted during the loop holds the input string in a traced slot and +/// drags it into the old generation; leaving the slot rooted afterwards keeps a +/// DEAD cursor doing that for the rest of the function. +#[test] +fn the_cursor_is_cleared_at_loop_exit() { + let mut m = module_with(region(cc_body())); + assert_eq!(segview_rewrite_module(&mut m), 1); + + // Structural, not string-matched: the statement AFTER the `For` must be a + // `LocalSet(, Undefined)`, and the cursor is the local the `For`'s + // condition tests against zero. + let for_idx = m + .init + .iter() + .position(|s| matches!(s, Stmt::For { .. })) + .expect("the rewritten loop"); + let cursor_id = match &m.init[for_idx] { + Stmt::For { + condition: Some(Expr::Conditional { condition, .. }), + .. + } => match condition.as_ref() { + Expr::Compare { left, .. } => match left.as_ref() { + Expr::LocalGet(id) => *id, + other => panic!("expected the cursor guard, got {other:?}"), + }, + other => panic!("expected a compare, got {other:?}"), + }, + _ => unreachable!(), + }; + match m.init.get(for_idx + 1) { + Some(Stmt::Expr(Expr::LocalSet(id, v))) => { + assert_eq!(*id, cursor_id, "the cleared local must be the cursor"); + assert!( + matches!(v.as_ref(), Expr::Undefined), + "the cursor slot must be cleared to undefined, got {v:?}" + ); + } + other => panic!("no cursor clear after the loop: {other:?}"), + } +} From cdc40c582e1eefac7c7be22f615f0c5b499ad70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 20:41:27 +0200 Subject: [PATCH 15/19] fix(tls): preserve dynamic peer certificates (cherry picked from commit c0bb08aff3ed2e7c3e405b2b17bc52d8af335cbd) --- changelog.d/9911-tls-peer-certificate-dispatch.md | 1 + crates/perry-ext-net/src/dispatch.rs | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 changelog.d/9911-tls-peer-certificate-dispatch.md diff --git a/changelog.d/9911-tls-peer-certificate-dispatch.md b/changelog.d/9911-tls-peer-certificate-dispatch.md new file mode 100644 index 0000000000..80900b1a43 --- /dev/null +++ b/changelog.d/9911-tls-peer-certificate-dispatch.md @@ -0,0 +1 @@ +Fix dynamic `TLSSocket.getPeerCertificate()` calls from the native net extension so they return the full negotiated certificate. Certificate inspection now preserves the peer identity across server secure-context rotation. diff --git a/crates/perry-ext-net/src/dispatch.rs b/crates/perry-ext-net/src/dispatch.rs index 127a572476..07c8e3fc84 100644 --- a/crates/perry-ext-net/src/dispatch.rs +++ b/crates/perry-ext-net/src/dispatch.rs @@ -180,7 +180,11 @@ fn socket_method_name(prop: &str) -> Option<&'static [u8]> { "upgradeToTLS" => Some(b"upgradeToTLS"), "getSession" => Some(b"getSession"), "isSessionReused" => Some(b"isSessionReused"), - "getPeerCertificate" => Some(b"getPeerCertificate"), + // The primary stdlib dispatcher owns `getPeerCertificate`: it builds + // the complete legacy certificate object from the DER recorded at the + // handshake. Claiming it here returned the extension's reduced JSON + // facade, whose optional CN is populated only for adopted HTTPS + // sockets, so direct `tls.connect()` handles produced `{}`. "setDefaultEncoding" => Some(b"setDefaultEncoding"), "cork" => Some(b"cork"), "uncork" => Some(b"uncork"), @@ -281,9 +285,6 @@ unsafe fn socket_method(handle: i64, method: &str, args: &[f64]) -> Option } "getSession" => nanbox_ptr(crate::js_ext_net_socket_tls_session(handle)), "isSessionReused" => crate::js_ext_net_socket_tls_session_reused(handle), - "getPeerCertificate" => { - json_str_to_value(crate::js_ext_net_socket_peer_certificate_json(handle)) - } "once" if args.len() >= 2 => { crate::js_net_socket_once(handle, unbox_to_i64(args[0]), unbox_to_i64(args[1])); nanbox_handle(handle) From a2956d7a55455eaafc6051568a9b6918b3d90c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 21:05:08 +0200 Subject: [PATCH 16/19] fix(runtime): mock declared prototype methods (cherry picked from commit 1b5ee0d6668044aca0d4c406b2be62d82dbc95a5) --- changelog.d/9913-test-mock-prototype.md | 1 + .../perry-runtime/src/node_submodules/test.rs | 20 +++++++++++++++++-- .../test/mock-fn/prototype-method.ts | 5 ++++- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 changelog.d/9913-test-mock-prototype.md diff --git a/changelog.d/9913-test-mock-prototype.md b/changelog.d/9913-test-mock-prototype.md new file mode 100644 index 0000000000..04063d419e --- /dev/null +++ b/changelog.d/9913-test-mock-prototype.md @@ -0,0 +1 @@ +`node:test`'s `mock.method()` now replaces declared class prototype methods for instance dispatch and restores their original behavior, while recording calls and receiver identity like Node. diff --git a/crates/perry-runtime/src/node_submodules/test.rs b/crates/perry-runtime/src/node_submodules/test.rs index cb70c0b994..15acb0c498 100644 --- a/crates/perry-runtime/src/node_submodules/test.rs +++ b/crates/perry-runtime/src/node_submodules/test.rs @@ -367,6 +367,22 @@ fn set_property_value(target: f64, property: &str, value: f64) { } } +fn set_method_property_value(target: f64, property: &str, value: f64) { + let raw = raw_ptr_from_value(target); + if let Some(class_id) = crate::object::class_id_for_decl_prototype_object(raw) { + unsafe { + crate::object::js_register_prototype_method( + class_id, + property.as_ptr(), + property.len(), + value, + ); + } + } else { + set_property_value(target, property, value); + } +} + fn get_property_value(target: f64, property: &str) -> f64 { let raw = raw_ptr_from_value(target); if raw >= 0x10000 && crate::closure::is_closure_ptr(raw) { @@ -691,7 +707,7 @@ fn restore_mock_state(id: i64) { target, property, original, - }) => set_property_value(target, &property, original), + }) => set_method_property_value(target, &property, original), Some(MockRestoreTarget::ObjectAccessor { target, property, @@ -1036,7 +1052,7 @@ extern "C" fn mock_method_thunk( original, }, ); - set_property_value(target.get_nanbox_f64(), &property_name, function); + set_method_property_value(target.get_nanbox_f64(), &property_name, function); function } diff --git a/test-parity/node-suite/test/mock-fn/prototype-method.ts b/test-parity/node-suite/test/mock-fn/prototype-method.ts index c6e2756b69..bcf7138aeb 100644 --- a/test-parity/node-suite/test/mock-fn/prototype-method.ts +++ b/test-parity/node-suite/test/mock-fn/prototype-method.ts @@ -1,7 +1,10 @@ import { mock } from "node:test"; class Counter { - constructor(public value: number) {} + value: number; + constructor(value: number) { + this.value = value; + } read() { return this.value; } From f22100967d4362604936df97239e8e74135bc1c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 21:32:55 +0200 Subject: [PATCH 17/19] fix(test): render reporter directives (cherry picked from commit c8663cfb087617fda0d3977e00a0d56a17d79767) --- changelog.d/9914-test-reporter-directives.md | 2 + .../src/node_submodules/test_reporters.rs | 57 ++++++++++++++++++- .../node-suite/test/reporters/directives.ts | 16 +++++- 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 changelog.d/9914-test-reporter-directives.md diff --git a/changelog.d/9914-test-reporter-directives.md b/changelog.d/9914-test-reporter-directives.md new file mode 100644 index 0000000000..665b8e5770 --- /dev/null +++ b/changelog.d/9914-test-reporter-directives.md @@ -0,0 +1,2 @@ +Render `skip` and `todo` directives in the `node:test` spec and TAP reporters, +including Node-compatible markers and optional directive reasons. diff --git a/crates/perry-runtime/src/node_submodules/test_reporters.rs b/crates/perry-runtime/src/node_submodules/test_reporters.rs index 93f8f86a9e..3cab2b3b85 100644 --- a/crates/perry-runtime/src/node_submodules/test_reporters.rs +++ b/crates/perry-runtime/src/node_submodules/test_reporters.rs @@ -88,6 +88,48 @@ fn event_data(event: f64) -> f64 { object_property(event, b"data").unwrap_or(undefined_value()) } +fn reporter_directive(data: f64) -> Option<(&'static str, String)> { + for (key, label) in [(b"skip".as_slice(), "SKIP"), (b"todo".as_slice(), "TODO")] { + let Some(value) = object_property(data, key) else { + continue; + }; + if crate::value::js_is_truthy(value) == 0 { + continue; + } + let reason = if JSValue::from_bits(value.to_bits()).is_any_string() { + value_to_string(value).unwrap_or_default() + } else { + String::new() + }; + return Some((label, reason)); + } + None +} + +fn directive_suffix(data: f64) -> String { + reporter_directive(data) + .map(|(label, reason)| { + if reason.is_empty() { + format!(" # {label}") + } else { + format!(" # {label} {reason}") + } + }) + .unwrap_or_default() +} + +fn spec_directive_suffix(data: f64) -> String { + reporter_directive(data) + .map(|(label, reason)| { + if reason.is_empty() { + format!(" # {label}") + } else { + format!(" # {reason}") + } + }) + .unwrap_or_default() +} + fn format_reporter_events(kind: i32, events: &[f64]) -> String { if kind == REPORTER_LCOV { return String::new(); @@ -118,7 +160,15 @@ fn format_reporter_event(kind: i32, event: f64) -> String { match kind { REPORTER_SPEC => match typ.as_str() { "test:pass" => object_string(data, b"name") - .map(|name| format!("✔ {name}\n")) + .map(|name| { + let marker = + if reporter_directive(data).is_some_and(|(label, _)| label == "SKIP") { + "﹣" + } else { + "✔" + }; + format!("{marker} {name}{}\n", spec_directive_suffix(data)) + }) .unwrap_or_default(), "test:diagnostic" => object_string(data, b"message") .map(|message| format!("ℹ {message}\n")) @@ -134,7 +184,10 @@ fn format_reporter_event(kind: i32, event: f64) -> String { let detail_type = object_property(data, b"details") .and_then(|details| object_string(details, b"type")) .unwrap_or_else(|| "test".to_string()); - format!("ok undefined - {name}\n ---\n type: '{detail_type}'\n ...\n") + format!( + "ok undefined - {name}{}\n ---\n type: '{detail_type}'\n ...\n", + directive_suffix(data) + ) } "test:diagnostic" => object_string(data, b"message") .map(|message| format!("# {message}\n")) diff --git a/test-parity/node-suite/test/reporters/directives.ts b/test-parity/node-suite/test/reporters/directives.ts index a08580b76a..468260f370 100644 --- a/test-parity/node-suite/test/reporters/directives.ts +++ b/test-parity/node-suite/test/reporters/directives.ts @@ -21,9 +21,21 @@ const events = [ }, ]; -async function collect(name: string, reporter: any) { +async function collect(name: string, reporter: any): Promise { let output = ""; - for await (const chunk of reporter(Readable.from(events))) output += String(chunk); + const result = reporter(Readable.from(events)); + if (typeof result.write === "function") { + const transform = reporter(); + transform.on("data", (chunk: unknown) => { + output += String(chunk); + }); + await new Promise((resolve) => { + transform.on("end", resolve); + Readable.from(events).pipe(transform); + }); + } else { + for await (const chunk of result) output += String(chunk); + } console.log(`${name}:`, JSON.stringify(output)); } From 91d8a267a54849e8450f81d92e8a653c2d6dbcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:21:43 +0200 Subject: [PATCH 18/19] fix(train): split regex.rs and its tests for the file cap; classify the new roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two files went over 2000 lines: - regex.rs -> regex/compile_cache.rs takes the program-compilation and cache group (size limit, std/fancy builders, eviction, the checked compile-and-cache entry). `js_regexp_new_impl` was the obvious bigger extraction and is deliberately NOT the one taken: it carries the two #7341 raw-handle debt sites, and raw_handle_debt.py's --no-raise-vs arm refuses a ceiling on a file absent at the merge base, so moving it would have forced either surgery on the allocation path two perf PRs are tuning, or a with_const_ptr(|p| p) that games the ratchet without scoping anything. - regex/tests.rs -> regex/tests_part2.rs, split at a test boundary. All 70 #[test] items are accounted for across the two files, and part2 carries the same cfg(all(test, feature)) gate as its sibling. #9893's new roots are classified: REGEXP_PROTOTYPE_PTR_SLOT and REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT are covered_elsewhere, naming scan_object_cache_roots_mut, which really does visit them (as an i64 slot and a nanbox word respectively) and is registered via reg_scanner!. The test index and the walk counter are not_a_gc_pointer. NEVER_MATCH moved with the compile-cache group, so its inventory entry is retargeted. #9906's new test thread_local is recorded cold — it is a test file, and the other 23 there are recorded the same way. --- crates/perry-runtime/src/regex.rs | 253 +--- .../perry-runtime/src/regex/compile_cache.rs | 258 ++++ crates/perry-runtime/src/regex/tests.rs | 1065 +---------------- crates/perry-runtime/src/regex/tests_part2.rs | 1065 +++++++++++++++++ scripts/gc_runtime_root_holders.json | 50 +- scripts/thread_local_cold_allowlist.json | 8 +- 6 files changed, 1366 insertions(+), 1333 deletions(-) create mode 100644 crates/perry-runtime/src/regex/compile_cache.rs create mode 100644 crates/perry-runtime/src/regex/tests_part2.rs diff --git a/crates/perry-runtime/src/regex.rs b/crates/perry-runtime/src/regex.rs index d8a1ccc1ef..2a7c5461e0 100644 --- a/crates/perry-runtime/src/regex.rs +++ b/crates/perry-runtime/src/regex.rs @@ -515,257 +515,10 @@ crate::perry_thread_local! { static VALIDATED_PATTERNS: RefCell> = RefCell::new(HashMap::new()); } -/// Compiled-program size budget handed to both regex engines. -/// -/// The `regex` crate (and the `regex-automata` backend `fancy-regex` -/// delegates to) caps a compiled program at 10 MiB by default and rejects -/// anything larger with `CompiledTooBig` / `ExceededSizeLimit` — which our -/// callers surface as a bogus `SyntaxError: invalid pattern`. JS itself has -/// no such limit, so a *valid* pattern with large bounded repetitions is -/// wrongly rejected. semver's ReDoS-hardened `safeRe` rewrites (`\s{0,1}`, -/// `\d{1,256}`, `[…]{0,250}`, …) blow well past 10 MiB; raise the budget so -/// these legitimate patterns compile. 64 MiB comfortably fits semver's full -/// range regex while still bounding pathological input. -#[cfg(feature = "regex-engine")] -const REGEX_SIZE_LIMIT: usize = 64 * 1024 * 1024; - -/// Build a `regex` crate `Regex` with the raised [`REGEX_SIZE_LIMIT`] so that -/// large-but-valid bounded-quantifier patterns aren't rejected as -/// `CompiledTooBig`. Drop-in replacement for `regex::Regex::new`. -#[cfg(feature = "regex-engine")] -pub(crate) fn build_std_regex(pattern: &str) -> Result { - // Collapse ReDoS-guard bounded quantifiers (`{m,N}`, large N) to unbounded before - // compiling. The linear `regex` engine expands `x{0,N}` into N states, so the semver - // package's `\d{0,256}` patterns became 8–16 MB automata each (~183 MB in a large - // bundle). This engine can't ReDoS, so the bound is safely removable here. See - // `grammar::collapse_redos_guard_quantifiers`. - let collapsed = collapse_redos_guard_quantifiers(pattern); - regex::RegexBuilder::new(&collapsed) - .size_limit(REGEX_SIZE_LIMIT) - .build() -} - -/// The ASCII word atom the boundary spellings below share. `(?-i:…)` keeps -/// the class exact under an outer `(?i)` — ECMAScript's non-Unicode word set -/// is pure ASCII even case-insensitively (no LONG S / KELVIN SIGN), and the -/// class is already case-closed so disabling the fold changes nothing else. -#[cfg(feature = "regex-engine")] -const FANCY_ASCII_WORD: &str = r"(?-i:[0-9A-Za-z_])"; - -/// Rewrite the translator's ASCII word-boundary markers into a form -/// `fancy-regex` parses (#9305 fallout, unmasked by the transport fix). -/// -/// `js_regex_to_rust` spells ECMAScript's ASCII `\b`/`\B` as `(?-iu:\b)` / -/// `(?-iu:\B)` (#9263). The `regex` crate accepts that scoped flag group, -/// but `fancy-regex`'s own parser rejects the `u` flag outright -/// (`NonUnicodeUnsupported`) — so every pattern that must run on this -/// engine (lookarounds, backreferences) and also contains a word boundary -/// failed to compile as a `SyntaxError`. cli.js's `marked` html-block -/// regex is exactly that shape, which is the throw-in-a-microtask that -/// #9305's setjmp miscompile turned into a segfault. -/// -/// The markers can only come from our own translator — `(?-iu:` is itself -/// a SyntaxError in a JS pattern, so no user input survives translation -/// with that byte sequence outside a character class — making a textual -/// substitution exact. The replacement spells the boundary with -/// one-code-point lookarounds, the same technique -/// `push_unicode_ignore_case_word_boundary` already relies on fancy-regex -/// for: a boundary is "exactly one side is a word char", a non-boundary -/// "both sides agree". -#[cfg(feature = "regex-engine")] -fn fancy_compatible_word_boundaries(pattern: &str) -> String { - if !pattern.contains("(?-iu:") { - return pattern.to_string(); - } - let w = FANCY_ASCII_WORD; - let boundary = format!("(?:(?<={w})(?!{w})|(? Result { - let pattern = fancy_compatible_word_boundaries(pattern); - fancy_regex::RegexBuilder::new(&pattern) - .delegate_size_limit(REGEX_SIZE_LIMIT) - .build() -} - -/// Entry cap for the compiled-regex caches (2026-07-09 GC audit: one entry -/// per distinct `(pattern, flags)` ever compiled, no cap of any kind, entries -/// up to [`REGEX_SIZE_LIMIT`] — `new RegExp(userInput)` was an attacker-driven -/// OOM). When an insert would exceed the cap the whole map is cleared — the -/// `PARSE_KEY_CACHE` precedent: cheap, no LRU bookkeeping, recompilation is -/// the fallback. Live `RegExpHeader`s are unaffected: each header OWNS a raw -/// `Arc` reference to its compiled program(s), released by its GC finalizer, -/// so dropping the cache's references cannot free a program still in use. +mod compile_cache; #[cfg(feature = "regex-engine")] -const REGEX_CACHE_MAX_ENTRIES: usize = 512; - -/// Clear-on-overflow guard shared by the compiled-program caches and the -/// validated-pattern set: make room for one more entry, wiping the map when it -/// is at capacity. -#[cfg(feature = "regex-engine")] -fn evict_regex_cache_if_full(cache: &mut HashMap) { - if cache.len() >= REGEX_CACHE_MAX_ENTRIES { - cache.clear(); - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| d.cache_clears += 1); - } - } -} - -/// Compile `(pattern, flags)` into the caches if absent, reporting whether -/// SOME engine accepted the flag-prefixed pattern. One NFA build total. -/// -/// This is the expensive path — the emoji-regex class of pattern costs -/// milliseconds per build. It no longer runs at construction: `js_regexp_new` -/// validates with the parser alone and `regex::lazy` calls this (through -/// `get_or_compile_regex`) on the first operation that needs a matcher. It is -/// still reached from construction for the patterns the linear engine's parser -/// rejects, where only a build can tell a fancy-regex pattern from a -/// `SyntaxError`. -/// -/// Returns `true` when the pattern is usable: compiled by the `regex` crate -/// (cached in `REGEX_CACHE`), or by `fancy-regex` (cached in `FANCY_CACHE`, -/// with the never-match placeholder in `REGEX_CACHE` so non-fancy callers -/// don't crash — the fancy fallback is handled in `js_regexp_exec_fancy`). -/// Returns `false` when BOTH engines reject it — nothing is cached and the -/// caller decides whether that is a SyntaxError (see `js_regexp_new`'s -/// bare-pattern fallback for the flag-prefix size edge). -/// 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: -/// building a fresh one per pattern would be exactly the compile cost the -/// experiment exists to remove from the measurement. -#[cfg(feature = "regex-engine")] -fn shared_never_match_program() -> Arc { - crate::perry_thread_local! { - static NEVER_MATCH: RefCell>> = const { RefCell::new(None) }; - } - NEVER_MATCH.with(|slot| { - slot.borrow_mut() - .get_or_insert_with(|| Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap())) - .clone() - }) -} - -#[cfg(feature = "regex-engine")] -fn compile_and_cache_regex_checked(pattern: &Arc, flags: &Arc) -> bool { - let already = REGEX_CACHE.with(|cache| { - cache - .borrow() - .contains_key(&(pattern.clone(), flags.clone())) - }); - if already { - return true; - } - let regress_covers = if let Some(repeat_matcher) = repeat_matcher::compile(pattern, flags) { - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| d.compiles_repeat += 1); - } - REPEAT_MATCHER_CACHE.with(|cache| { - let mut cache = cache.borrow_mut(); - evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.clone(), flags.clone()), Arc::new(repeat_matcher)); - }); - true - } else { - false - }; - // `PERRY_REGEX_ENGINE=regress` (measurement only — see - // `repeat_matcher::regress_first`): the ECMAScript backtracker is the - // primary engine, so stop here. Every exec-family entry point consults the - // repeat matcher first, and the shared never-match placeholder gives the - // header's `regex_ptr` built-flag a value WITHOUT building an NFA — which - // is the whole point of the experiment (the linear engine's program is - // ~12.5 KB median against regress's 512 B, measured over 4,463 literals - // from seven real bundles). - if regress_covers && repeat_matcher::regress_first() { - REGEX_CACHE.with(|cache| { - let mut cache = cache.borrow_mut(); - evict_regex_cache_if_full(&mut cache); - cache.insert( - (pattern.clone(), flags.clone()), - shared_never_match_program(), - ); - }); - return true; - } - // Translate JS regex to Rust-compatible pattern, with the inline mode - // prefix the flags imply. Shared with `lazy::std_engine_syntax_ok` so the - // eager syntax check and this build can never inspect different strings. - let regex_pattern = lazy::flag_prefixed_pattern(pattern, flags); - let regex = match build_std_regex(®ex_pattern) { - Ok(re) => re, - Err(_) => { - // Pattern has features regex crate doesn't support - // (lookbehind, lookahead). Try fancy-regex which supports - // the full JS regex feature set, and if it compiles, wrap - // the result via a find-and-replace approach at the exec - // call sites. Store a never-matching pattern so existing - // callers don't crash. - let fancy_ok = FANCY_CACHE.with(|fc| { - if let Ok(fre) = build_fancy_regex(®ex_pattern) { - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| d.compiles_fancy += 1); - } - let mut fc = fc.borrow_mut(); - evict_regex_cache_if_full(&mut fc); - fc.insert((pattern.clone(), flags.clone()), std::sync::Arc::new(fre)); - true - } else { - false - } - }); - if !fancy_ok { - return false; - } - Regex::new(NEVER_MATCH_PATTERN).unwrap() - } - }; - if crate::hot_diag::regex_on() { - crate::hot_diag::regex_with(|d| d.compiles_std += 1); - } - REGEX_CACHE.with(|cache| { - let mut cache = cache.borrow_mut(); - evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.clone(), flags.clone()), Arc::new(regex)); - }); - true -} - -#[cfg(feature = "regex-engine")] -fn get_or_compile_regex(pattern: &Arc, flags: &Arc) -> Arc { - let hit = REGEX_CACHE.with(|cache| { - cache - .borrow() - .get(&(pattern.clone(), flags.clone())) - .cloned() - }); - if let Some(re) = hit { - return re; - } - let _ = compile_and_cache_regex_checked(pattern, flags); - REGEX_CACHE.with(|cache| { - let mut cache = cache.borrow_mut(); - if let Some(re) = cache.get(&(pattern.clone(), flags.clone())) { - return re.clone(); - } - // Both engines rejected it (validation normally throws before this - // point) — keep the historical behavior: cache + return never-match. - let arc = Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap()); - evict_regex_cache_if_full(&mut cache); - cache.insert((pattern.clone(), flags.clone()), arc.clone()); - arc - }) -} +pub(crate) use compile_cache::*; /// Header for heap-allocated RegExp objects #[repr(C)] @@ -2139,3 +1892,5 @@ pub extern "C" fn js_regexp_set_last_index(re: *mut RegExpHeader, value: f64) { #[cfg(all(test, feature = "regex-engine"))] mod tests; +#[cfg(all(test, feature = "regex-engine"))] +mod tests_part2; diff --git a/crates/perry-runtime/src/regex/compile_cache.rs b/crates/perry-runtime/src/regex/compile_cache.rs new file mode 100644 index 0000000000..bde70bc77f --- /dev/null +++ b/crates/perry-runtime/src/regex/compile_cache.rs @@ -0,0 +1,258 @@ +//! Program compilation and the program caches, split out of `regex.rs` +//! for the 2000-line file cap: size limit, std/fancy builders, cache +//! eviction, and the checked compile-and-cache entry point. +//! +//! A child module, so `use super::*` reaches the parent's private items. + +use super::*; + +/// Compiled-program size budget handed to both regex engines. +/// +/// The `regex` crate (and the `regex-automata` backend `fancy-regex` +/// delegates to) caps a compiled program at 10 MiB by default and rejects +/// anything larger with `CompiledTooBig` / `ExceededSizeLimit` — which our +/// callers surface as a bogus `SyntaxError: invalid pattern`. JS itself has +/// no such limit, so a *valid* pattern with large bounded repetitions is +/// wrongly rejected. semver's ReDoS-hardened `safeRe` rewrites (`\s{0,1}`, +/// `\d{1,256}`, `[…]{0,250}`, …) blow well past 10 MiB; raise the budget so +/// these legitimate patterns compile. 64 MiB comfortably fits semver's full +/// range regex while still bounding pathological input. +pub(crate) const REGEX_SIZE_LIMIT: usize = 64 * 1024 * 1024; + +/// Build a `regex` crate `Regex` with the raised [`REGEX_SIZE_LIMIT`] so that +/// large-but-valid bounded-quantifier patterns aren't rejected as +/// `CompiledTooBig`. Drop-in replacement for `regex::Regex::new`. +#[cfg(feature = "regex-engine")] +pub(crate) fn build_std_regex(pattern: &str) -> Result { + // Collapse ReDoS-guard bounded quantifiers (`{m,N}`, large N) to unbounded before + // compiling. The linear `regex` engine expands `x{0,N}` into N states, so the semver + // package's `\d{0,256}` patterns became 8–16 MB automata each (~183 MB in a large + // bundle). This engine can't ReDoS, so the bound is safely removable here. See + // `grammar::collapse_redos_guard_quantifiers`. + let collapsed = collapse_redos_guard_quantifiers(pattern); + regex::RegexBuilder::new(&collapsed) + .size_limit(REGEX_SIZE_LIMIT) + .build() +} + +/// The ASCII word atom the boundary spellings below share. `(?-i:…)` keeps +/// the class exact under an outer `(?i)` — ECMAScript's non-Unicode word set +/// is pure ASCII even case-insensitively (no LONG S / KELVIN SIGN), and the +/// class is already case-closed so disabling the fold changes nothing else. +#[cfg(feature = "regex-engine")] +pub(crate) const FANCY_ASCII_WORD: &str = r"(?-i:[0-9A-Za-z_])"; + +/// Rewrite the translator's ASCII word-boundary markers into a form +/// `fancy-regex` parses (#9305 fallout, unmasked by the transport fix). +/// +/// `js_regex_to_rust` spells ECMAScript's ASCII `\b`/`\B` as `(?-iu:\b)` / +/// `(?-iu:\B)` (#9263). The `regex` crate accepts that scoped flag group, +/// but `fancy-regex`'s own parser rejects the `u` flag outright +/// (`NonUnicodeUnsupported`) — so every pattern that must run on this +/// engine (lookarounds, backreferences) and also contains a word boundary +/// failed to compile as a `SyntaxError`. cli.js's `marked` html-block +/// regex is exactly that shape, which is the throw-in-a-microtask that +/// #9305's setjmp miscompile turned into a segfault. +/// +/// The markers can only come from our own translator — `(?-iu:` is itself +/// a SyntaxError in a JS pattern, so no user input survives translation +/// with that byte sequence outside a character class — making a textual +/// substitution exact. The replacement spells the boundary with +/// one-code-point lookarounds, the same technique +/// `push_unicode_ignore_case_word_boundary` already relies on fancy-regex +/// for: a boundary is "exactly one side is a word char", a non-boundary +/// "both sides agree". +#[cfg(feature = "regex-engine")] +pub(crate) fn fancy_compatible_word_boundaries(pattern: &str) -> String { + if !pattern.contains("(?-iu:") { + return pattern.to_string(); + } + let w = FANCY_ASCII_WORD; + let boundary = format!("(?:(?<={w})(?!{w})|(? Result { + let pattern = fancy_compatible_word_boundaries(pattern); + fancy_regex::RegexBuilder::new(&pattern) + .delegate_size_limit(REGEX_SIZE_LIMIT) + .build() +} + +/// Entry cap for the compiled-regex caches (2026-07-09 GC audit: one entry +/// per distinct `(pattern, flags)` ever compiled, no cap of any kind, entries +/// up to [`REGEX_SIZE_LIMIT`] — `new RegExp(userInput)` was an attacker-driven +/// OOM). When an insert would exceed the cap the whole map is cleared — the +/// `PARSE_KEY_CACHE` precedent: cheap, no LRU bookkeeping, recompilation is +/// the fallback. Live `RegExpHeader`s are unaffected: each header OWNS a raw +/// `Arc` reference to its compiled program(s), released by its GC finalizer, +/// so dropping the cache's references cannot free a program still in use. +#[cfg(feature = "regex-engine")] +pub(crate) const REGEX_CACHE_MAX_ENTRIES: usize = 512; + +/// Clear-on-overflow guard shared by the compiled-program caches and the +/// validated-pattern set: make room for one more entry, wiping the map when it +/// is at capacity. +#[cfg(feature = "regex-engine")] +pub(crate) fn evict_regex_cache_if_full(cache: &mut HashMap) { + if cache.len() >= REGEX_CACHE_MAX_ENTRIES { + cache.clear(); + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.cache_clears += 1); + } + } +} + +/// Compile `(pattern, flags)` into the caches if absent, reporting whether +/// SOME engine accepted the flag-prefixed pattern. One NFA build total. +/// +/// This is the expensive path — the emoji-regex class of pattern costs +/// milliseconds per build. It no longer runs at construction: `js_regexp_new` +/// validates with the parser alone and `regex::lazy` calls this (through +/// `get_or_compile_regex`) on the first operation that needs a matcher. It is +/// still reached from construction for the patterns the linear engine's parser +/// rejects, where only a build can tell a fancy-regex pattern from a +/// `SyntaxError`. +/// +/// Returns `true` when the pattern is usable: compiled by the `regex` crate +/// (cached in `REGEX_CACHE`), or by `fancy-regex` (cached in `FANCY_CACHE`, +/// with the never-match placeholder in `REGEX_CACHE` so non-fancy callers +/// don't crash — the fancy fallback is handled in `js_regexp_exec_fancy`). +/// Returns `false` when BOTH engines reject it — nothing is cached and the +/// caller decides whether that is a SyntaxError (see `js_regexp_new`'s +/// bare-pattern fallback for the flag-prefix size edge). +/// 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: +/// building a fresh one per pattern would be exactly the compile cost the +/// experiment exists to remove from the measurement. +#[cfg(feature = "regex-engine")] +pub(crate) fn shared_never_match_program() -> Arc { + crate::perry_thread_local! { + static NEVER_MATCH: RefCell>> = const { RefCell::new(None) }; + } + NEVER_MATCH.with(|slot| { + slot.borrow_mut() + .get_or_insert_with(|| Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap())) + .clone() + }) +} + +#[cfg(feature = "regex-engine")] +pub(crate) fn compile_and_cache_regex_checked(pattern: &Arc, flags: &Arc) -> bool { + let already = REGEX_CACHE.with(|cache| { + cache + .borrow() + .contains_key(&(pattern.clone(), flags.clone())) + }); + if already { + return true; + } + let regress_covers = if let Some(repeat_matcher) = repeat_matcher::compile(pattern, flags) { + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.compiles_repeat += 1); + } + REPEAT_MATCHER_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert((pattern.clone(), flags.clone()), Arc::new(repeat_matcher)); + }); + true + } else { + false + }; + // `PERRY_REGEX_ENGINE=regress` (measurement only — see + // `repeat_matcher::regress_first`): the ECMAScript backtracker is the + // primary engine, so stop here. Every exec-family entry point consults the + // repeat matcher first, and the shared never-match placeholder gives the + // header's `regex_ptr` built-flag a value WITHOUT building an NFA — which + // is the whole point of the experiment (the linear engine's program is + // ~12.5 KB median against regress's 512 B, measured over 4,463 literals + // from seven real bundles). + if regress_covers && repeat_matcher::regress_first() { + REGEX_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert( + (pattern.clone(), flags.clone()), + shared_never_match_program(), + ); + }); + return true; + } + // Translate JS regex to Rust-compatible pattern, with the inline mode + // prefix the flags imply. Shared with `lazy::std_engine_syntax_ok` so the + // eager syntax check and this build can never inspect different strings. + let regex_pattern = lazy::flag_prefixed_pattern(pattern, flags); + let regex = match build_std_regex(®ex_pattern) { + Ok(re) => re, + Err(_) => { + // Pattern has features regex crate doesn't support + // (lookbehind, lookahead). Try fancy-regex which supports + // the full JS regex feature set, and if it compiles, wrap + // the result via a find-and-replace approach at the exec + // call sites. Store a never-matching pattern so existing + // callers don't crash. + let fancy_ok = FANCY_CACHE.with(|fc| { + if let Ok(fre) = build_fancy_regex(®ex_pattern) { + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.compiles_fancy += 1); + } + let mut fc = fc.borrow_mut(); + evict_regex_cache_if_full(&mut fc); + fc.insert((pattern.clone(), flags.clone()), std::sync::Arc::new(fre)); + true + } else { + false + } + }); + if !fancy_ok { + return false; + } + Regex::new(NEVER_MATCH_PATTERN).unwrap() + } + }; + if crate::hot_diag::regex_on() { + crate::hot_diag::regex_with(|d| d.compiles_std += 1); + } + REGEX_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + evict_regex_cache_if_full(&mut cache); + cache.insert((pattern.clone(), flags.clone()), Arc::new(regex)); + }); + true +} + +#[cfg(feature = "regex-engine")] +pub(crate) fn get_or_compile_regex(pattern: &Arc, flags: &Arc) -> Arc { + let hit = REGEX_CACHE.with(|cache| { + cache + .borrow() + .get(&(pattern.clone(), flags.clone())) + .cloned() + }); + if let Some(re) = hit { + return re; + } + let _ = compile_and_cache_regex_checked(pattern, flags); + REGEX_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + if let Some(re) = cache.get(&(pattern.clone(), flags.clone())) { + return re.clone(); + } + // Both engines rejected it (validation normally throws before this + // point) — keep the historical behavior: cache + return never-match. + let arc = Arc::new(Regex::new(NEVER_MATCH_PATTERN).unwrap()); + evict_regex_cache_if_full(&mut cache); + cache.insert((pattern.clone(), flags.clone()), arc.clone()); + arc + }) +} diff --git a/crates/perry-runtime/src/regex/tests.rs b/crates/perry-runtime/src/regex/tests.rs index 76aad4a12d..11a6347e77 100644 --- a/crates/perry-runtime/src/regex/tests.rs +++ b/crates/perry-runtime/src/regex/tests.rs @@ -1,7 +1,7 @@ use super::*; use crate::string::js_string_from_bytes; -fn make_string(s: &str) -> *mut StringHeader { +pub(super) fn make_string(s: &str) -> *mut StringHeader { js_string_from_bytes(s.as_ptr(), s.len() as u32) } @@ -9,7 +9,7 @@ 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 { +pub(super) 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() } @@ -364,7 +364,7 @@ fn fancy_lookbehind_exec_index() { } } -fn match_capture_text(arr: *const ArrayHeader, index: u32) -> Option { +pub(super) fn match_capture_text(arr: *const ArrayHeader, index: u32) -> Option { let value = crate::array::js_array_get_f64(arr, index); if crate::value::JSValue::from_bits(value.to_bits()).is_undefined() { return None; @@ -1060,1062 +1060,3 @@ fn global_exec_walks_astral_string_by_code_units() { assert!(js_regexp_exec(re, make_string(subject)).is_null()); assert_eq!(regex_last_index_offset(re), 0); } - -#[test] -fn search_returns_utf16_index() { - // `"𝌆x".search(/x/)` is 2 (the astral scalar occupies indices 0 and 1), - // matching `"𝌆x".indexOf("x")`. - let re = js_regexp_new(make_string("x"), make_string("")); - assert_eq!(js_string_search_regex(make_string("𝌆x"), re), 2); -} - -/// The eager syntax check must accept EXACTLY what the full build accepts. -/// -/// `js_regexp_new` no longer answers "is this a `SyntaxError`?" by building the -/// automaton — it asks the standard engine's parser alone -/// (`lazy::std_engine_syntax_ok`) and only falls through to the both-engines -/// path when the parser refuses. That is sound only while parser-acceptance and -/// builder-acceptance agree; if a future `regex` release moves a diagnostic out -/// of the parser and into the NFA build, a pattern would silently stop throwing -/// at construction. This is the gate for that: it disagrees loudly rather than -/// letting the divergence ship. -/// -/// Both directions matter, so the corpus deliberately contains patterns the -/// linear engine ACCEPTS, ones it rejects for lack of a feature (lookbehind, -/// backreferences — the fancy-regex fallback's territory) and ones that are -/// genuinely malformed. -#[test] -fn syntax_check_agrees_with_full_build() { - let corpus: &[(&str, &str)] = &[ - // Ordinary shapes. - ("abc", ""), - ("^v?(\\d+)\\.(\\d+)\\.(\\d+)$", ""), - ("[A-Za-z0-9_.+-]+@[\\w-]+\\.[\\w.-]+", "i"), - ("(?:https?|ftp)://[^\\s]+", "gi"), - ("\\s+", "gm"), - ("a.b", "s"), - ("(foo|bar|baz){2,4}", "i"), - ("x{0,250}", ""), - ("\\d{1,256}", ""), - // Unicode classes / properties / astral — the case-folding shapes. - ("[A-Za-zÀ-ɏ]+", "i"), - ("[Ѐ-ӿͰ-Ͽ]*", "giu"), - ("\\p{L}+", "u"), - ("\\p{Script=Greek}", "u"), - ("[\\u{1F600}-\\u{1F64F}]", "u"), - ("[←-⇿☀-⛿]", "u"), - ("\\w+\\b", "iu"), - // Fancy-only (the linear engine refuses; fancy-regex accepts). - ("(?<=pre)\\d+", ""), - ("(?([\\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 -/// raised a bogus SyntaxError — cli.js's `marked` html-block regex among -/// them, whose throw-in-a-microtask the setjmp miscompile then turned into -/// a segfault. `build_fancy_regex` now rewrites the marker into one-char -/// lookarounds. -#[test] -fn fancy_engine_accepts_ascii_word_boundary_markers() { - // Lookahead + \b: std engine refuses (lookaround), fancy must accept. - let translated = js_regex_to_rust(r"(?!foo\b)\w+"); - let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy build"); - assert_eq!( - fancy.find("foobar").unwrap().map(|m| m.as_str()), - Some("foobar") - ); - assert!(fancy.find("foo bar").unwrap().map(|m| m.as_str()) != Some("foo")); - - // \B variant. - let translated = js_regex_to_rust(r"(?=x)x\Ba"); - let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy \\B build"); - assert!(fancy.is_match("xa").unwrap()); - - // Boundary semantics stay ASCII on the fancy engine: é is NOT a word - // char, so /(?=.)\bé/ must treat the position before é as a boundary - // only when the preceding char is a word char... spec: \b before é - // (non-word) requires previous to be word. - let translated = js_regex_to_rust(r"(?=.)a\b\u00e9"); - let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy ascii build"); - assert!(fancy.is_match("a\u{e9}").unwrap()); - - // The real-world shape: marked's html-block regex from cli_2.1.112.js. - let marked = concat!( - r"^ *(?:|$)) *(?:\n|\s*$)", - r"|<((?!(?:a|em|strong|small|s|cite|q|dfn|abbr|data|time|code|var|samp|kbd", - r"|sub|sup|i|b|u|mark|ruby|rt|rp|bdi|bdo|span|br|wbr|ins|del|img)\b)", - r"\w+(?!:|[^\w\s@]*@)\b)[\s\S]+? *(?:\n{2,}|\s*$)", - r"|<(?!(?:a|em|strong|small|s|cite|q|dfn|abbr|data|time|code|var|samp|kbd", - r"|sub|sup|i|b|u|mark|ruby|rt|rp|bdi|bdo|span|br|wbr|ins|del|img)\b)", - r"\w+(?!:|[^\w\s@]*@)\b(?:\x22[^\x22]*\x22|'[^']*'|\s[^'\x22/>\s]*)*?/?> *(?:\n{2,}|\s*$))", - ); - let translated = js_regex_to_rust(marked); - let fancy = crate::regex::build_fancy_regex(&translated).expect("marked html regex build"); - assert_eq!( - fancy - .find("
\nhello\n
\n\n") - .unwrap() - .map(|m| m.as_str()), - Some("
\nhello\n
\n\n") - ); -} - -// ---- #9429: exec/test at a non-zero lastIndex see the WHOLE subject ------ - -/// One `exec` at `last_index`, as `(matched text, .index, lastIndex after)`. -/// `None` also asserts the spec's reset-to-0 on a failed stateful exec, so a -/// row that stops matching cannot quietly leave `lastIndex` behind. -fn exec_from( - pattern: &str, - flags: &str, - subject: &str, - last_index: usize, -) -> Option<(String, f64, usize)> { - let re = js_regexp_new(make_string(pattern), make_string(flags)); - store_last_index_number(re, last_index); - let arr = js_regexp_exec(re, make_string(subject)); - if arr.is_null() { - assert_eq!( - regex_last_index_offset(re), - 0, - "{pattern}/{flags} @{last_index}: a failed stateful exec resets lastIndex" - ); - return None; - } - let text = match_capture_text(arr, 0).expect("capture zero always participates"); - Some(( - text, - js_regexp_exec_get_index(), - regex_last_index_offset(re), - )) -} - -fn hit(text: &str, index: f64, last_index: usize) -> Option<(String, f64, usize)> { - Some((text.to_string(), index, last_index)) -} - -#[test] -fn exec_at_last_index_holds_anchors_against_the_subject_not_a_slice() { - // Every row is a position where the SLICE and the SUBJECT disagree. - // `^` is start-of-subject: at lastIndex 1 of "ab" it must not hold, even - // though it would hold at offset 0 of the slice "b". - assert_eq!(exec_from("^b", "g", "ab", 1), None); - assert_eq!(exec_from("^b", "g", "ab", 0), None); - assert_eq!(exec_from("^a", "g", "ab", 0), hit("a", 0.0, 1)); - assert_eq!(exec_from("^a", "g", "ab", 1), None); - // Under `m` it holds after a LineTerminator IN THE SUBJECT — index 2 of - // "a\nb" regardless of where the scan was told to start. - assert_eq!(exec_from("^b", "gm", "a\nb", 0), hit("b", 2.0, 3)); - assert_eq!(exec_from("^b", "gm", "a\nb", 1), hit("b", 2.0, 3)); - assert_eq!(exec_from("^b", "gm", "a\nb", 2), hit("b", 2.0, 3)); - // `\b`/`\B` read the character BEFORE the start position. - assert_eq!(exec_from(r"\bb", "g", "ab", 1), None); - assert_eq!(exec_from(r"\Bb", "g", "ab", 1), hit("b", 1.0, 2)); - assert_eq!(exec_from(r"\bb", "g", "a b", 1), hit("b", 2.0, 3)); - assert_eq!(exec_from(r"\Bb", "g", "a b", 1), None); - // `$` at the very end still matches the empty string there. - assert_eq!(exec_from("$", "g", "ab", 2), hit("", 2.0, 2)); -} - -#[test] -fn exec_at_last_index_keeps_lookaround_context() { - // The `regex` crate has no lookaround, so these run on the fancy-regex - // fallback — assert the lane, or the rows below could pass on a different - // engine than the one this fix touches. - let looky = js_regexp_new(make_string("(?<=a)b"), make_string("g")); - assert!( - lookup_fancy_regex(looky).is_some(), - "lookbehind must select the fancy-regex lane" - ); - - // Lookbehind is destroyed by a slice: the `a` is to the LEFT of the start. - assert_eq!(exec_from("(?<=a)b", "g", "ab", 0), hit("b", 1.0, 2)); - assert_eq!(exec_from("(?<=a)b", "g", "ab", 1), hit("b", 1.0, 2)); - assert_eq!(exec_from("(?<=a)b", "g", "ab", 2), None); - assert_eq!(exec_from("(?<=ab)c", "g", "abc", 2), hit("c", 2.0, 3)); - // …and a NEGATIVE lookbehind is wrong the other way: a slice makes it hold. - assert_eq!(exec_from("(? Vec { - let re = js_regexp_new(make_string(pattern), make_string(flags)); - let arr = js_string_match(make_string(subject), re); - if arr.is_null() { - return Vec::new(); - } - let len = unsafe { (*arr).length }; - (0..len) - .map(|index| match_capture_text(arr, index).expect("a match list holds only strings")) - .collect() -} - -fn replace_all_with(pattern: &str, flags: &str, subject: &str, repl: &str) -> String { - let re = js_regexp_new(make_string(pattern), make_string(flags)); - let out = js_string_replace_regex(make_string(subject), re, make_string(repl)); - string_as_str(out).to_string() -} - -#[test] -fn ecmascript_scan_keeps_an_empty_match_where_the_previous_one_ended() { - // The scan loop's contract, pinned without an engine: an empty match at - // the previous match's end is KEPT, and the cursor then advances one - // position — Rust's iterators drop it and advance instead. - // - // The finder below is `/a*/` over "aXa" written out by hand. - let subject = "aXa"; - let seen = super::global_scan::scan(subject, 0, |cursor| { - // `a*` matches the empty string anywhere, so its leftmost match from - // `cursor` always STARTS at `cursor` and runs over the `a`s there. - let mut end = cursor; - while subject.as_bytes().get(end) == Some(&b'a') { - end += 1; - } - Some((cursor, end, (cursor, end))) - }); - assert_eq!(seen, vec![(0, 1), (1, 1), (2, 3), (3, 3)]); - - // The bound is what terminates the walk: without `cursor > len` ending it, - // the trailing empty match would repeat forever. - let empties = super::global_scan::scan("ab", 0, |cursor| Some((cursor, cursor, cursor))); - assert_eq!(empties, vec![0, 1, 2]); - - // A zero-width step never lands inside a scalar. - assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 0), 1); - assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 1), 5); - assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 5), 6); - assert_eq!(super::global_scan::advance_past_empty("ab", 2), 3); -} - -#[test] -fn global_match_keeps_the_trailing_and_interior_empty_matches() { - // The linear `regex` lane. - let plain = js_regexp_new(make_string("a*"), make_string("g")); - assert!( - lookup_fancy_regex(plain).is_none() && lookup_repeat_matcher(plain).is_none(), - "`a*` must stay on the linear engine" - ); - assert_eq!(global_match_list("a*", "g", "a"), vec!["a", ""]); - assert_eq!(global_match_list("a*", "g", "aa"), vec!["aa", ""]); - assert_eq!(global_match_list("b*", "g", "ab"), vec!["", "b", ""]); - // Not only the trailing one: the empty match at index 1 is interior. - assert_eq!(global_match_list("a*", "g", "aXa"), vec!["a", "", "a", ""]); - assert_eq!(global_match_list("x*", "g", "abc"), vec!["", "", "", ""]); - assert_eq!(global_match_list("a*", "g", ""), vec![""]); - // A pattern that cannot match empty is unchanged. - assert_eq!(global_match_list("a+", "g", "aXa"), vec!["a", "a"]); -} - -#[test] -fn global_match_keeps_empty_matches_on_the_fancy_lane() { - // A possibly-empty pattern the linear engine cannot compile. - let looky = js_regexp_new(make_string("a*(?!x)"), make_string("g")); - assert!( - lookup_fancy_regex(looky).is_some(), - "a lookahead must select the fancy-regex lane" - ); - assert_eq!(global_match_list("a*(?!x)", "g", "a"), vec!["a", ""]); - assert_eq!( - global_match_list("a*(?!x)", "g", "aXa"), - vec!["a", "", "a", ""] - ); - assert_eq!(global_match_list("(?<=,)", "g", "a,b,"), vec!["", ""]); -} - -#[test] -fn global_match_on_the_regress_lane_is_unchanged() { - // `regress`'s iterator already implements the ECMAScript rule; this is the - // control that says so, and that nothing routed it elsewhere. - let quantified = js_regexp_new(make_string("(a)*"), make_string("g")); - assert!( - lookup_repeat_matcher(quantified).is_some(), - "a quantified capture must select the regress lane" - ); - assert_eq!(global_match_list("(a)*", "g", "a"), vec!["a", ""]); - assert_eq!( - global_match_list("(a)*", "g", "aXa"), - vec!["a", "", "a", ""] - ); -} - -#[test] -fn global_replace_substitutes_at_every_empty_match() { - assert_eq!(replace_all_with("a*", "g", "a", "<>"), "<><>"); - assert_eq!(replace_all_with("a*", "g", "aXa", "-"), "--X--"); - assert_eq!(replace_all_with("b*", "g", "ab", "-"), "-a--"); - assert_eq!(replace_all_with("x*", "g", "abc", "-"), "-a-b-c-"); - assert_eq!(replace_all_with("a*", "g", "aXa", "[$&]"), "[a][]X[a][]"); - // The non-global form still replaces exactly one match. - assert_eq!(replace_all_with("a*", "", "aXa", "-"), "-Xa"); - // Fancy lane. - assert_eq!(replace_all_with("a*(?!x)", "g", "a", "<>"), "<><>"); - assert_eq!(replace_all_with("(?<=a)", "g", "aba", "!"), "a!ba!"); - // Named-group substitution takes its own scan path. - let named = js_regexp_new(make_string("(?a)*"), make_string("g")); - let out = js_string_replace_regex_named(make_string("a"), named, make_string("[$]")); - assert_eq!(string_as_str(out), "[a][]"); -} - -/// The construction cache (`regex::site_cache`): once a header built from -/// some `(pattern, flags)` has been executed, the next construction of the -/// same text is born built — it shares the executed header's program and -/// never runs the lazy build. Fails on a runtime without the cache (the -/// second header stays lazy). -#[test] -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_eq!( - site_cache::test_has_programs("born[0-9]+built", "g"), - Some(false), - "construction records the validated text without programs" - ); - assert!(js_regexp_test(re1, make_string("xx born42built")) != 0); - assert_eq!( - site_cache::test_has_programs("born[0-9]+built", "g"), - Some(true), - "the first execution's build is remembered against the text" - ); - let re2 = js_regexp_new(make_string("born[0-9]+built"), make_string("g")); - assert!( - !unsafe { (*re2).regex_ptr.is_null() }, - "the second construction installs the programs eagerly" - ); - assert!( - 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. - let re3 = js_regexp_new(make_string("born[0-9]+built"), make_string("i")); - assert!(unsafe { (*re3).regex_ptr.is_null() }); -} - -/// `test` on a global/sticky receiver advances `lastIndex` exactly like -/// `exec` and resets it on failure, through the find-only engine phase (no -/// exec array). Pinned against node for every branch of that bookkeeping. -#[test] -fn global_test_advances_and_resets_last_index() { - let _lock = crate::gc::global_side_table_test_lock(); - let re = js_regexp_new(make_string("a"), make_string("g")); - let s = make_string("aXa"); - assert_eq!(js_regexp_test(re, s), 1); - assert_eq!(js_regexp_get_last_index(re), 1.0); - assert_eq!(js_regexp_test(re, s), 1); - assert_eq!(js_regexp_get_last_index(re), 3.0); - assert_eq!(js_regexp_test(re, s), 0); - assert_eq!(js_regexp_get_last_index(re), 0.0); - - // `lastIndex > length` is "no match" and resets. - js_regexp_set_last_index(re, 10.0); - assert_eq!(js_regexp_test(re, s), 0); - assert_eq!(js_regexp_get_last_index(re), 0.0); - - // sticky anchors at lastIndex. - let sticky = js_regexp_new(make_string("a"), make_string("y")); - let t = make_string("ba"); - assert_eq!(js_regexp_test(sticky, t), 0); - assert_eq!(js_regexp_get_last_index(sticky), 0.0); - js_regexp_set_last_index(sticky, 1.0); - assert_eq!(js_regexp_test(sticky, t), 1); - assert_eq!(js_regexp_get_last_index(sticky), 2.0); - - // lastIndex counts UTF-16 code units, not bytes. - let astral = js_regexp_new(make_string("b"), make_string("g")); - let u = make_string("😀b😀b"); - assert_eq!(js_regexp_test(astral, u), 1); - assert_eq!(js_regexp_get_last_index(astral), 3.0); - assert_eq!(js_regexp_test(astral, u), 1); - assert_eq!(js_regexp_get_last_index(astral), 6.0); - assert_eq!(js_regexp_test(astral, u), 0); - - // The fancy-regex fallback (lookbehind) takes the same path. - let fancy = js_regexp_new(make_string("(?<=x)a"), make_string("g")); - let f = make_string("xa xa a"); - assert_eq!(js_regexp_test(fancy, f), 1); - assert_eq!(js_regexp_get_last_index(fancy), 2.0); - assert_eq!(js_regexp_test(fancy, f), 1); - assert_eq!(js_regexp_get_last_index(fancy), 5.0); - assert_eq!(js_regexp_test(fancy, f), 0); - assert_eq!(js_regexp_get_last_index(fancy), 0.0); - - // The backtracking matcher (quantified capture) likewise. - let repeat = js_regexp_new(make_string("(a?b??)*c"), make_string("g")); - let r = make_string("abc c"); - assert_eq!(js_regexp_test(repeat, r), 1); - assert_eq!(js_regexp_get_last_index(repeat), 3.0); - assert_eq!(js_regexp_test(repeat, r), 1); - assert_eq!(js_regexp_get_last_index(repeat), 5.0); - assert_eq!(js_regexp_test(repeat, r), 0); -} - -/// A capacity event in one compiled-program cache must not leave a pattern -/// whose real program lives in ANOTHER of them permanently non-matching. -/// -/// `compile_and_cache_regex_checked` returns early when `REGEX_CACHE` already -/// holds the pattern, so it never re-runs the fancy build; for a lookbehind -/// pattern that `REGEX_CACHE` entry is the never-match placeholder and the -/// real program is the one in `FANCY_CACHE`. Clear `FANCY_CACHE` on its own — -/// which is exactly what its independent 512-entry overflow used to do — and -/// `get_or_compile_regex` hands back a program that matches nothing while -/// nothing rebuilds the fallback. Since `lookup_fancy_regex` now treats a -/// built header as authoritative and `site_cache::install_programs` memoizes -/// the triple against the pattern text, that is not one bad header: every -/// later construction of the same literal is born with it. -/// -/// The fix is that `lazy::build_and_install_programs` REPAIRS the header -/// before publishing it and before memoizing the triple: a standard program -/// that is the never-match placeholder with no fancy program beside it means -/// the fancy program is missing, so it is rebuilt. (Clearing the three caches -/// as a group was built first and dropped: it closes the route into the bad -/// state but cannot repair a header already in it, so this test still failed.) -#[test] -fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { - let _lock = crate::gc::global_side_table_test_lock(); - let source = "(?<=foo)bar"; - let scope = crate::gc::RuntimeHandleScope::new(); - site_cache::test_reset(); - - let build = || { - let pattern = scope.root_string_ptr(make_string(source)); - let flags = scope.root_string_ptr(make_string("")); - pattern.with_mut_ptr::(|pattern| { - flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) - }) - }; - let subject = scope.root_string_ptr(make_string("foobar")); - - let warm = build(); - assert_eq!( - subject.with_const_ptr::(|s| js_regexp_test(warm, s)), - 1, - "a lookbehind pattern must match through the fancy fallback" - ); - - // The state a `FANCY_CACHE` overflow produces: its programs are gone, the - // never-match placeholder for this pattern survives in `REGEX_CACHE`. - FANCY_CACHE.with(|fc| fc.borrow_mut().clear()); - assert!( - REGEX_CACHE.with(|c| c - .borrow() - .contains_key(&(std::sync::Arc::from(source), std::sync::Arc::from("")))), - "the placeholder must survive, or this test exercises nothing" - ); - // A fresh literal site, so the construction cache cannot answer from the - // programs the first header built. - site_cache::test_reset(); - - let cold = build(); - unsafe { - lazy::ensure_regex_compiled(cold); - assert!( - !(*cold).fancy_ptr.is_null(), - "a built header must carry every program its pattern needs — a null \ - fancy_ptr here is memoized by site_cache::install_programs and makes \ - the breakage permanent for this literal" - ); - } - assert_eq!( - subject.with_const_ptr::(|s| js_regexp_test(cold, s)), - 1, - "the literal must still match after an unrelated cache reached capacity" - ); -} - -/// The backtracking cliff: a capture group under a quantifier takes a pattern -/// off the linear engine, and the ECMAScript backtracker has no step budget. -/// `/^(a+)+$/.test("a"*28 + "!")` measured 16.5 s against 4.8 s for node and -/// 0 ms for the identical-language `/^(?:a+)+$/`. -/// -/// The linear program proves the answer in O(n) — the two engines accept the -/// same language and disagree only about capture ASSIGNMENT — so the -/// backtracker must not be entered for a subject the linear engine has already -/// ruled out. This test would take minutes without that gate. -#[test] -fn quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject() { - let _lock = crate::gc::global_side_table_test_lock(); - let scope = crate::gc::RuntimeHandleScope::new(); - let pattern = scope.root_string_ptr(make_string("^(a+)+$")); - let flags = scope.root_string_ptr(make_string("")); - let re = pattern.with_mut_ptr::(|pattern| { - flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) - }); - // The pattern really is on the backtracker — that is the premise. - assert!( - lookup_repeat_matcher(re).is_some(), - "a capture under a quantifier must route to the ECMAScript matcher" - ); - - let hay = format!("{}!", "a".repeat(40)); - let subject = scope.root_string_ptr(make_string(&hay)); - let started = std::time::Instant::now(); - assert_eq!( - subject.with_const_ptr::(|s| js_regexp_test(re, s)), - 0, - "no match: the subject ends in '!'" - ); - assert!( - started.elapsed() < std::time::Duration::from_secs(2), - "a non-matching subject must not be handed to the backtracker \ - (took {:?} for 40 characters)", - started.elapsed() - ); - - // A subject that DOES match still goes through the backtracker and still - // reports the spec's captures. - let good = scope.root_string_ptr(make_string("aaaa")); - assert_eq!( - good.with_const_ptr::(|s| js_regexp_test(re, s)), - 1 - ); -} - -/// #6759 phase 1 follow-up: a `RegExp` receiver can now answer the -/// descriptor-summary probe. Before the meta edge was wired for -/// `GC_TYPE_REGEXP`, `may_have_descriptor_entry` answered the conservative -/// `true` for every RegExp, so `set_last_index_throwing` built a `String` and -/// SipHashed `(usize, String)` on every global/sticky `test()`/`exec()`. -#[test] -fn a_fresh_regexp_proves_lastindex_absent_without_probing_the_tables() { - let _lock = crate::gc::global_side_table_test_lock(); - let scope = crate::gc::RuntimeHandleScope::new(); - let pattern = scope.root_string_ptr(make_string("x")); - let flags = scope.root_string_ptr(make_string("g")); - let re = pattern.with_mut_ptr::(|pattern| { - flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) - }); - // Premise: this really is the dedicated RegExp cell, not a shaped object - // that would have answered through the ordinary `GC_TYPE_OBJECT` path. - let gc = unsafe { crate::value::addr_class::try_read_gc_header(re as usize) } - .expect("RegExp must be a GC allocation"); - assert_eq!(gc.obj_type, crate::gc::GC_TYPE_REGEXP); - - assert!( - !crate::object::test_may_have_descriptor_entry(re as usize, "lastIndex", false), - "a fresh RegExp has no descriptors, so the meta summary must prove \ - `lastIndex` absent instead of sending the caller to the table" - ); - assert!( - crate::object::get_property_attrs(re as usize, "lastIndex").is_none(), - "and the answer the fast path skips must be the same one" - ); -} - -/// The other half, and the one that makes the fast negative safe: an owner -/// that DOES have a descriptor must still be found. Install and probe share -/// one predicate, so a probe widened without its install would answer -/// "proven absent" here and `set_last_index_throwing` would silently stop -/// throwing (test262 prototype/{exec,test}/y-fail-lastindex-no-write). -#[test] -fn a_regexp_with_a_non_writable_lastindex_is_still_found_by_the_probe() { - let _lock = crate::gc::global_side_table_test_lock(); - let scope = crate::gc::RuntimeHandleScope::new(); - let pattern = scope.root_string_ptr(make_string("x")); - let flags = scope.root_string_ptr(make_string("g")); - let re = pattern.with_mut_ptr::(|pattern| { - flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) - }); - let attrs = crate::object::PropertyAttrs::new(false, true, true); - crate::object::set_property_attrs(re as usize, "lastIndex".to_string(), attrs); - - assert!( - crate::object::test_may_have_descriptor_entry(re as usize, "lastIndex", false), - "the install set the key bit, so the probe must send the caller to the table" - ); - let found = crate::object::get_property_attrs(re as usize, "lastIndex") - .expect("the descriptor the test installed must be readable back"); - assert!(!found.writable(), "and it must still read as non-writable"); - - // A DIFFERENT key on the same owner stays proven-absent: the summary is - // per key, not per owner, so widening it must not blunt it. - assert!( - !crate::object::test_may_have_descriptor_entry(re as usize, "source", false), - "an unrelated key on the same RegExp must still take the fast negative" - ); -} - -// --------------------------------------------------------------------------- -// Literal-site keyed construction (`js_regexp_new_site` + `regex::site_key`). -// -// The site key is the address of a private global the compiler emits once per -// regex literal. These statics stand in for two such globals: distinct -// addresses, 8-byte aligned, immortal — the same three properties the emitted -// ones have, and the reason the key is a sound identity where a `StringHeader` -// address is not. -// --------------------------------------------------------------------------- - -// Distinct initialisers so nothing may merge them: the test uses only their -// ADDRESSES, and identical zero-valued statics are exactly the shape a -// constant-merging pass is allowed to collapse. `assert_ne!` on the two keys -// keeps that from being a silent assumption. -static SITE_SLOT_A: u64 = 0xA; -static SITE_SLOT_B: u64 = 0xB; -static SITE_SLOT_C: u64 = 0xC; - -fn site_key_of(slot: &'static u64) -> i64 { - slot as *const u64 as i64 -} - -/// **The sabotage the coordinator named: key the table by pattern length.** -/// -/// Two literals at two sites, same flags, same pattern LENGTH, different text. -/// A table that verifies a probe by anything weaker than the site key — a -/// length, a prefix, a fingerprint without the exactness check — hands the -/// second site the first site's entry, and `.source` then reports a pattern -/// this literal never contained while `test` matches the wrong language. -/// -/// Each site is constructed twice: the first construction records, the second -/// is the one that must come back from the table, which is the case a weak key -/// breaks. Asserting only on a first construction would pass under every -/// sabotage, because a miss always takes the content-keyed path. -#[test] -fn two_literal_sites_with_equal_length_patterns_never_answer_for_each_other() { - let _lock = crate::gc::global_side_table_test_lock(); - site_key::test_reset(); - - let key_a = site_key_of(&SITE_SLOT_A); - let key_b = site_key_of(&SITE_SLOT_B); - assert_ne!(key_a, key_b, "two sites must have two addresses"); - - for round in 0..2 { - let a = js_regexp_new_site(make_string("a.c"), make_string("g"), key_a); - let b = js_regexp_new_site(make_string("x.z"), make_string("g"), key_b); - assert_eq!( - string_payload(js_regexp_get_source(a)), - b"a.c".to_vec(), - "round {round}: site A must report its own pattern" - ); - assert_eq!( - string_payload(js_regexp_get_source(b)), - b"x.z".to_vec(), - "round {round}: site B must report its own pattern — a table keyed by anything \ - weaker than the site address hands B the entry A recorded, and both patterns are \ - three bytes long" - ); - assert!( - js_regexp_test(a, make_string("abc")) != 0 - && js_regexp_test(a, make_string("xyz")) == 0, - "round {round}: site A must match its own language" - ); - assert!( - js_regexp_test(b, make_string("xyz")) != 0 - && js_regexp_test(b, make_string("abc")) == 0, - "round {round}: site B must match its own language" - ); - } - - assert_eq!( - site_key::test_recorded_pattern(key_a, "g").as_deref(), - Some("a.c") - ); - assert_eq!( - site_key::test_recorded_pattern(key_b, "g").as_deref(), - Some("x.z") - ); -} - -/// A dynamic `new RegExp(str)` must never reach the site table. -/// -/// The two-argument entry point is what every non-literal construction uses — -/// `js_regexp_construct`, `RegExp.prototype.compile`, the runtime's own -/// callers — and it has no site to be keyed by. If it recorded under some -/// stand-in key, a later literal whose key collided would inherit a pattern -/// that a *variable* produced, which is the one thing the site key's -/// compile-time-constant argument is supposed to guarantee against. -#[test] -fn a_dynamic_construction_records_nothing_in_the_site_table() { - let _lock = crate::gc::global_side_table_test_lock(); - site_key::test_reset(); - - for _ in 0..4 { - let re = js_regexp_new(make_string("dyn(amic)"), make_string("g")); - assert!(js_regexp_test(re, make_string("dynamic")) != 0); - } - assert_eq!( - site_key::test_occupied_slots(), - 0, - "the two-argument entry point has no site key and must record nothing" - ); - - // The same TEXT through the site entry does record — so the zero above is - // a property of the entry point, not of a table that never works. - let key = site_key_of(&SITE_SLOT_C); - let re = js_regexp_new_site(make_string("dyn(amic)"), make_string("g"), key); - assert!(js_regexp_test(re, make_string("dynamic")) != 0); - assert_eq!( - site_key::test_occupied_slots(), - 1, - "the site entry point must record — otherwise the assertion above proves nothing" - ); - assert_eq!( - site_key::test_recorded_pattern(key, "g").as_deref(), - Some("dyn(amic)") - ); -} - -/// 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. -/// -/// 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 -/// `test()` and give the site key nothing. -#[test] -fn a_site_hit_after_the_first_execution_is_born_built() { - let _lock = crate::gc::global_side_table_test_lock(); - site_key::test_reset(); - let key = site_key_of(&SITE_SLOT_A); - - let first = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); - assert!( - unsafe { (*first).regex_ptr }.is_null(), - "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(), - "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(), - "a site hit must install the programs the site already compiled, so the header is born \ - built and the first match pays no lookup" - ); - assert_ne!(first, second, "each evaluation is still a distinct object"); - assert!(js_regexp_test(second, make_string("born")) != 0); -} - -/// The kill switch has to remove the lane, not just its answers: with -/// `PERRY_REGEX_SITE_KEY=0` the table records nothing, so the OFF arm is the -/// content-keyed path exactly rather than a control still paying the -/// bookkeeping. -/// -/// Read once per process through a `OnceLock`, so this asserts the DEFAULT is -/// on rather than flipping the variable mid-run (which would only test the -/// cache of the first read). -#[test] -fn the_site_key_lane_is_on_by_default() { - assert!( - crate::gc::env_default_on_from_value(None), - "the site-key lane defaults ON; `PERRY_REGEX_SITE_KEY=0` is the kill switch" - ); - assert!(!crate::gc::env_default_on_from_value(Some("0"))); -} diff --git a/crates/perry-runtime/src/regex/tests_part2.rs b/crates/perry-runtime/src/regex/tests_part2.rs new file mode 100644 index 0000000000..3ea07b9671 --- /dev/null +++ b/crates/perry-runtime/src/regex/tests_part2.rs @@ -0,0 +1,1065 @@ +//! Second half of the `regex` test module, split for the 2000-line file cap. +//! 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::*; + +#[test] +fn search_returns_utf16_index() { + // `"𝌆x".search(/x/)` is 2 (the astral scalar occupies indices 0 and 1), + // matching `"𝌆x".indexOf("x")`. + let re = js_regexp_new(make_string("x"), make_string("")); + assert_eq!(js_string_search_regex(make_string("𝌆x"), re), 2); +} + +/// The eager syntax check must accept EXACTLY what the full build accepts. +/// +/// `js_regexp_new` no longer answers "is this a `SyntaxError`?" by building the +/// automaton — it asks the standard engine's parser alone +/// (`lazy::std_engine_syntax_ok`) and only falls through to the both-engines +/// path when the parser refuses. That is sound only while parser-acceptance and +/// builder-acceptance agree; if a future `regex` release moves a diagnostic out +/// of the parser and into the NFA build, a pattern would silently stop throwing +/// at construction. This is the gate for that: it disagrees loudly rather than +/// letting the divergence ship. +/// +/// Both directions matter, so the corpus deliberately contains patterns the +/// linear engine ACCEPTS, ones it rejects for lack of a feature (lookbehind, +/// backreferences — the fancy-regex fallback's territory) and ones that are +/// genuinely malformed. +#[test] +fn syntax_check_agrees_with_full_build() { + let corpus: &[(&str, &str)] = &[ + // Ordinary shapes. + ("abc", ""), + ("^v?(\\d+)\\.(\\d+)\\.(\\d+)$", ""), + ("[A-Za-z0-9_.+-]+@[\\w-]+\\.[\\w.-]+", "i"), + ("(?:https?|ftp)://[^\\s]+", "gi"), + ("\\s+", "gm"), + ("a.b", "s"), + ("(foo|bar|baz){2,4}", "i"), + ("x{0,250}", ""), + ("\\d{1,256}", ""), + // Unicode classes / properties / astral — the case-folding shapes. + ("[A-Za-zÀ-ɏ]+", "i"), + ("[Ѐ-ӿͰ-Ͽ]*", "giu"), + ("\\p{L}+", "u"), + ("\\p{Script=Greek}", "u"), + ("[\\u{1F600}-\\u{1F64F}]", "u"), + ("[←-⇿☀-⛿]", "u"), + ("\\w+\\b", "iu"), + // Fancy-only (the linear engine refuses; fancy-regex accepts). + ("(?<=pre)\\d+", ""), + ("(?([\\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 +/// raised a bogus SyntaxError — cli.js's `marked` html-block regex among +/// them, whose throw-in-a-microtask the setjmp miscompile then turned into +/// a segfault. `build_fancy_regex` now rewrites the marker into one-char +/// lookarounds. +#[test] +fn fancy_engine_accepts_ascii_word_boundary_markers() { + // Lookahead + \b: std engine refuses (lookaround), fancy must accept. + let translated = js_regex_to_rust(r"(?!foo\b)\w+"); + let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy build"); + assert_eq!( + fancy.find("foobar").unwrap().map(|m| m.as_str()), + Some("foobar") + ); + assert!(fancy.find("foo bar").unwrap().map(|m| m.as_str()) != Some("foo")); + + // \B variant. + let translated = js_regex_to_rust(r"(?=x)x\Ba"); + let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy \\B build"); + assert!(fancy.is_match("xa").unwrap()); + + // Boundary semantics stay ASCII on the fancy engine: é is NOT a word + // char, so /(?=.)\bé/ must treat the position before é as a boundary + // only when the preceding char is a word char... spec: \b before é + // (non-word) requires previous to be word. + let translated = js_regex_to_rust(r"(?=.)a\b\u00e9"); + let fancy = crate::regex::build_fancy_regex(&translated).expect("fancy ascii build"); + assert!(fancy.is_match("a\u{e9}").unwrap()); + + // The real-world shape: marked's html-block regex from cli_2.1.112.js. + let marked = concat!( + r"^ *(?:|$)) *(?:\n|\s*$)", + r"|<((?!(?:a|em|strong|small|s|cite|q|dfn|abbr|data|time|code|var|samp|kbd", + r"|sub|sup|i|b|u|mark|ruby|rt|rp|bdi|bdo|span|br|wbr|ins|del|img)\b)", + r"\w+(?!:|[^\w\s@]*@)\b)[\s\S]+? *(?:\n{2,}|\s*$)", + r"|<(?!(?:a|em|strong|small|s|cite|q|dfn|abbr|data|time|code|var|samp|kbd", + r"|sub|sup|i|b|u|mark|ruby|rt|rp|bdi|bdo|span|br|wbr|ins|del|img)\b)", + r"\w+(?!:|[^\w\s@]*@)\b(?:\x22[^\x22]*\x22|'[^']*'|\s[^'\x22/>\s]*)*?/?> *(?:\n{2,}|\s*$))", + ); + let translated = js_regex_to_rust(marked); + let fancy = crate::regex::build_fancy_regex(&translated).expect("marked html regex build"); + assert_eq!( + fancy + .find("
\nhello\n
\n\n") + .unwrap() + .map(|m| m.as_str()), + Some("
\nhello\n
\n\n") + ); +} + +// ---- #9429: exec/test at a non-zero lastIndex see the WHOLE subject ------ + +/// One `exec` at `last_index`, as `(matched text, .index, lastIndex after)`. +/// `None` also asserts the spec's reset-to-0 on a failed stateful exec, so a +/// row that stops matching cannot quietly leave `lastIndex` behind. +fn exec_from( + pattern: &str, + flags: &str, + subject: &str, + last_index: usize, +) -> Option<(String, f64, usize)> { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + store_last_index_number(re, last_index); + let arr = js_regexp_exec(re, make_string(subject)); + if arr.is_null() { + assert_eq!( + regex_last_index_offset(re), + 0, + "{pattern}/{flags} @{last_index}: a failed stateful exec resets lastIndex" + ); + return None; + } + let text = match_capture_text(arr, 0).expect("capture zero always participates"); + Some(( + text, + js_regexp_exec_get_index(), + regex_last_index_offset(re), + )) +} + +fn hit(text: &str, index: f64, last_index: usize) -> Option<(String, f64, usize)> { + Some((text.to_string(), index, last_index)) +} + +#[test] +fn exec_at_last_index_holds_anchors_against_the_subject_not_a_slice() { + // Every row is a position where the SLICE and the SUBJECT disagree. + // `^` is start-of-subject: at lastIndex 1 of "ab" it must not hold, even + // though it would hold at offset 0 of the slice "b". + assert_eq!(exec_from("^b", "g", "ab", 1), None); + assert_eq!(exec_from("^b", "g", "ab", 0), None); + assert_eq!(exec_from("^a", "g", "ab", 0), hit("a", 0.0, 1)); + assert_eq!(exec_from("^a", "g", "ab", 1), None); + // Under `m` it holds after a LineTerminator IN THE SUBJECT — index 2 of + // "a\nb" regardless of where the scan was told to start. + assert_eq!(exec_from("^b", "gm", "a\nb", 0), hit("b", 2.0, 3)); + assert_eq!(exec_from("^b", "gm", "a\nb", 1), hit("b", 2.0, 3)); + assert_eq!(exec_from("^b", "gm", "a\nb", 2), hit("b", 2.0, 3)); + // `\b`/`\B` read the character BEFORE the start position. + assert_eq!(exec_from(r"\bb", "g", "ab", 1), None); + assert_eq!(exec_from(r"\Bb", "g", "ab", 1), hit("b", 1.0, 2)); + assert_eq!(exec_from(r"\bb", "g", "a b", 1), hit("b", 2.0, 3)); + assert_eq!(exec_from(r"\Bb", "g", "a b", 1), None); + // `$` at the very end still matches the empty string there. + assert_eq!(exec_from("$", "g", "ab", 2), hit("", 2.0, 2)); +} + +#[test] +fn exec_at_last_index_keeps_lookaround_context() { + // The `regex` crate has no lookaround, so these run on the fancy-regex + // fallback — assert the lane, or the rows below could pass on a different + // engine than the one this fix touches. + let looky = js_regexp_new(make_string("(?<=a)b"), make_string("g")); + assert!( + lookup_fancy_regex(looky).is_some(), + "lookbehind must select the fancy-regex lane" + ); + + // Lookbehind is destroyed by a slice: the `a` is to the LEFT of the start. + assert_eq!(exec_from("(?<=a)b", "g", "ab", 0), hit("b", 1.0, 2)); + assert_eq!(exec_from("(?<=a)b", "g", "ab", 1), hit("b", 1.0, 2)); + assert_eq!(exec_from("(?<=a)b", "g", "ab", 2), None); + assert_eq!(exec_from("(?<=ab)c", "g", "abc", 2), hit("c", 2.0, 3)); + // …and a NEGATIVE lookbehind is wrong the other way: a slice makes it hold. + assert_eq!(exec_from("(? Vec { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + let arr = js_string_match(make_string(subject), re); + if arr.is_null() { + return Vec::new(); + } + let len = unsafe { (*arr).length }; + (0..len) + .map(|index| match_capture_text(arr, index).expect("a match list holds only strings")) + .collect() +} + +fn replace_all_with(pattern: &str, flags: &str, subject: &str, repl: &str) -> String { + let re = js_regexp_new(make_string(pattern), make_string(flags)); + let out = js_string_replace_regex(make_string(subject), re, make_string(repl)); + string_as_str(out).to_string() +} + +#[test] +fn ecmascript_scan_keeps_an_empty_match_where_the_previous_one_ended() { + // The scan loop's contract, pinned without an engine: an empty match at + // the previous match's end is KEPT, and the cursor then advances one + // position — Rust's iterators drop it and advance instead. + // + // The finder below is `/a*/` over "aXa" written out by hand. + let subject = "aXa"; + let seen = super::global_scan::scan(subject, 0, |cursor| { + // `a*` matches the empty string anywhere, so its leftmost match from + // `cursor` always STARTS at `cursor` and runs over the `a`s there. + let mut end = cursor; + while subject.as_bytes().get(end) == Some(&b'a') { + end += 1; + } + Some((cursor, end, (cursor, end))) + }); + assert_eq!(seen, vec![(0, 1), (1, 1), (2, 3), (3, 3)]); + + // The bound is what terminates the walk: without `cursor > len` ending it, + // the trailing empty match would repeat forever. + let empties = super::global_scan::scan("ab", 0, |cursor| Some((cursor, cursor, cursor))); + assert_eq!(empties, vec![0, 1, 2]); + + // A zero-width step never lands inside a scalar. + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 0), 1); + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 1), 5); + assert_eq!(super::global_scan::advance_past_empty("a𝌆b", 5), 6); + assert_eq!(super::global_scan::advance_past_empty("ab", 2), 3); +} + +#[test] +fn global_match_keeps_the_trailing_and_interior_empty_matches() { + // The linear `regex` lane. + let plain = js_regexp_new(make_string("a*"), make_string("g")); + assert!( + lookup_fancy_regex(plain).is_none() && lookup_repeat_matcher(plain).is_none(), + "`a*` must stay on the linear engine" + ); + assert_eq!(global_match_list("a*", "g", "a"), vec!["a", ""]); + assert_eq!(global_match_list("a*", "g", "aa"), vec!["aa", ""]); + assert_eq!(global_match_list("b*", "g", "ab"), vec!["", "b", ""]); + // Not only the trailing one: the empty match at index 1 is interior. + assert_eq!(global_match_list("a*", "g", "aXa"), vec!["a", "", "a", ""]); + assert_eq!(global_match_list("x*", "g", "abc"), vec!["", "", "", ""]); + assert_eq!(global_match_list("a*", "g", ""), vec![""]); + // A pattern that cannot match empty is unchanged. + assert_eq!(global_match_list("a+", "g", "aXa"), vec!["a", "a"]); +} + +#[test] +fn global_match_keeps_empty_matches_on_the_fancy_lane() { + // A possibly-empty pattern the linear engine cannot compile. + let looky = js_regexp_new(make_string("a*(?!x)"), make_string("g")); + assert!( + lookup_fancy_regex(looky).is_some(), + "a lookahead must select the fancy-regex lane" + ); + assert_eq!(global_match_list("a*(?!x)", "g", "a"), vec!["a", ""]); + assert_eq!( + global_match_list("a*(?!x)", "g", "aXa"), + vec!["a", "", "a", ""] + ); + assert_eq!(global_match_list("(?<=,)", "g", "a,b,"), vec!["", ""]); +} + +#[test] +fn global_match_on_the_regress_lane_is_unchanged() { + // `regress`'s iterator already implements the ECMAScript rule; this is the + // control that says so, and that nothing routed it elsewhere. + let quantified = js_regexp_new(make_string("(a)*"), make_string("g")); + assert!( + lookup_repeat_matcher(quantified).is_some(), + "a quantified capture must select the regress lane" + ); + assert_eq!(global_match_list("(a)*", "g", "a"), vec!["a", ""]); + assert_eq!( + global_match_list("(a)*", "g", "aXa"), + vec!["a", "", "a", ""] + ); +} + +#[test] +fn global_replace_substitutes_at_every_empty_match() { + assert_eq!(replace_all_with("a*", "g", "a", "<>"), "<><>"); + assert_eq!(replace_all_with("a*", "g", "aXa", "-"), "--X--"); + assert_eq!(replace_all_with("b*", "g", "ab", "-"), "-a--"); + assert_eq!(replace_all_with("x*", "g", "abc", "-"), "-a-b-c-"); + assert_eq!(replace_all_with("a*", "g", "aXa", "[$&]"), "[a][]X[a][]"); + // The non-global form still replaces exactly one match. + assert_eq!(replace_all_with("a*", "", "aXa", "-"), "-Xa"); + // Fancy lane. + assert_eq!(replace_all_with("a*(?!x)", "g", "a", "<>"), "<><>"); + assert_eq!(replace_all_with("(?<=a)", "g", "aba", "!"), "a!ba!"); + // Named-group substitution takes its own scan path. + let named = js_regexp_new(make_string("(?a)*"), make_string("g")); + let out = js_string_replace_regex_named(make_string("a"), named, make_string("[$]")); + assert_eq!(string_as_str(out), "[a][]"); +} + +/// The construction cache (`regex::site_cache`): once a header built from +/// some `(pattern, flags)` has been executed, the next construction of the +/// same text is born built — it shares the executed header's program and +/// never runs the lazy build. Fails on a runtime without the cache (the +/// second header stays lazy). +#[test] +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_eq!( + site_cache::test_has_programs("born[0-9]+built", "g"), + Some(false), + "construction records the validated text without programs" + ); + assert!(js_regexp_test(re1, make_string("xx born42built")) != 0); + assert_eq!( + site_cache::test_has_programs("born[0-9]+built", "g"), + Some(true), + "the first execution's build is remembered against the text" + ); + let re2 = js_regexp_new(make_string("born[0-9]+built"), make_string("g")); + assert!( + !unsafe { (*re2).regex_ptr.is_null() }, + "the second construction installs the programs eagerly" + ); + assert!( + 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. + let re3 = js_regexp_new(make_string("born[0-9]+built"), make_string("i")); + assert!(unsafe { (*re3).regex_ptr.is_null() }); +} + +/// `test` on a global/sticky receiver advances `lastIndex` exactly like +/// `exec` and resets it on failure, through the find-only engine phase (no +/// exec array). Pinned against node for every branch of that bookkeeping. +#[test] +fn global_test_advances_and_resets_last_index() { + let _lock = crate::gc::global_side_table_test_lock(); + let re = js_regexp_new(make_string("a"), make_string("g")); + let s = make_string("aXa"); + assert_eq!(js_regexp_test(re, s), 1); + assert_eq!(js_regexp_get_last_index(re), 1.0); + assert_eq!(js_regexp_test(re, s), 1); + assert_eq!(js_regexp_get_last_index(re), 3.0); + assert_eq!(js_regexp_test(re, s), 0); + assert_eq!(js_regexp_get_last_index(re), 0.0); + + // `lastIndex > length` is "no match" and resets. + js_regexp_set_last_index(re, 10.0); + assert_eq!(js_regexp_test(re, s), 0); + assert_eq!(js_regexp_get_last_index(re), 0.0); + + // sticky anchors at lastIndex. + let sticky = js_regexp_new(make_string("a"), make_string("y")); + let t = make_string("ba"); + assert_eq!(js_regexp_test(sticky, t), 0); + assert_eq!(js_regexp_get_last_index(sticky), 0.0); + js_regexp_set_last_index(sticky, 1.0); + assert_eq!(js_regexp_test(sticky, t), 1); + assert_eq!(js_regexp_get_last_index(sticky), 2.0); + + // lastIndex counts UTF-16 code units, not bytes. + let astral = js_regexp_new(make_string("b"), make_string("g")); + let u = make_string("😀b😀b"); + assert_eq!(js_regexp_test(astral, u), 1); + assert_eq!(js_regexp_get_last_index(astral), 3.0); + assert_eq!(js_regexp_test(astral, u), 1); + assert_eq!(js_regexp_get_last_index(astral), 6.0); + assert_eq!(js_regexp_test(astral, u), 0); + + // The fancy-regex fallback (lookbehind) takes the same path. + let fancy = js_regexp_new(make_string("(?<=x)a"), make_string("g")); + let f = make_string("xa xa a"); + assert_eq!(js_regexp_test(fancy, f), 1); + assert_eq!(js_regexp_get_last_index(fancy), 2.0); + assert_eq!(js_regexp_test(fancy, f), 1); + assert_eq!(js_regexp_get_last_index(fancy), 5.0); + assert_eq!(js_regexp_test(fancy, f), 0); + assert_eq!(js_regexp_get_last_index(fancy), 0.0); + + // The backtracking matcher (quantified capture) likewise. + let repeat = js_regexp_new(make_string("(a?b??)*c"), make_string("g")); + let r = make_string("abc c"); + assert_eq!(js_regexp_test(repeat, r), 1); + assert_eq!(js_regexp_get_last_index(repeat), 3.0); + assert_eq!(js_regexp_test(repeat, r), 1); + assert_eq!(js_regexp_get_last_index(repeat), 5.0); + assert_eq!(js_regexp_test(repeat, r), 0); +} + +/// A capacity event in one compiled-program cache must not leave a pattern +/// whose real program lives in ANOTHER of them permanently non-matching. +/// +/// `compile_and_cache_regex_checked` returns early when `REGEX_CACHE` already +/// holds the pattern, so it never re-runs the fancy build; for a lookbehind +/// pattern that `REGEX_CACHE` entry is the never-match placeholder and the +/// real program is the one in `FANCY_CACHE`. Clear `FANCY_CACHE` on its own — +/// which is exactly what its independent 512-entry overflow used to do — and +/// `get_or_compile_regex` hands back a program that matches nothing while +/// nothing rebuilds the fallback. Since `lookup_fancy_regex` now treats a +/// built header as authoritative and `site_cache::install_programs` memoizes +/// the triple against the pattern text, that is not one bad header: every +/// later construction of the same literal is born with it. +/// +/// The fix is that `lazy::build_and_install_programs` REPAIRS the header +/// before publishing it and before memoizing the triple: a standard program +/// that is the never-match placeholder with no fancy program beside it means +/// the fancy program is missing, so it is rebuilt. (Clearing the three caches +/// as a group was built first and dropped: it closes the route into the bad +/// state but cannot repair a header already in it, so this test still failed.) +#[test] +fn a_single_program_cache_clear_cannot_disarm_a_lookbehind_literal() { + let _lock = crate::gc::global_side_table_test_lock(); + let source = "(?<=foo)bar"; + let scope = crate::gc::RuntimeHandleScope::new(); + site_cache::test_reset(); + + let build = || { + let pattern = scope.root_string_ptr(make_string(source)); + let flags = scope.root_string_ptr(make_string("")); + pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }) + }; + let subject = scope.root_string_ptr(make_string("foobar")); + + let warm = build(); + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(warm, s)), + 1, + "a lookbehind pattern must match through the fancy fallback" + ); + + // The state a `FANCY_CACHE` overflow produces: its programs are gone, the + // never-match placeholder for this pattern survives in `REGEX_CACHE`. + FANCY_CACHE.with(|fc| fc.borrow_mut().clear()); + assert!( + REGEX_CACHE.with(|c| c + .borrow() + .contains_key(&(std::sync::Arc::from(source), std::sync::Arc::from("")))), + "the placeholder must survive, or this test exercises nothing" + ); + // A fresh literal site, so the construction cache cannot answer from the + // programs the first header built. + site_cache::test_reset(); + + let cold = build(); + unsafe { + lazy::ensure_regex_compiled(cold); + assert!( + !(*cold).fancy_ptr.is_null(), + "a built header must carry every program its pattern needs — a null \ + fancy_ptr here is memoized by site_cache::install_programs and makes \ + the breakage permanent for this literal" + ); + } + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(cold, s)), + 1, + "the literal must still match after an unrelated cache reached capacity" + ); +} + +/// The backtracking cliff: a capture group under a quantifier takes a pattern +/// off the linear engine, and the ECMAScript backtracker has no step budget. +/// `/^(a+)+$/.test("a"*28 + "!")` measured 16.5 s against 4.8 s for node and +/// 0 ms for the identical-language `/^(?:a+)+$/`. +/// +/// The linear program proves the answer in O(n) — the two engines accept the +/// same language and disagree only about capture ASSIGNMENT — so the +/// backtracker must not be entered for a subject the linear engine has already +/// ruled out. This test would take minutes without that gate. +#[test] +fn quantified_capture_pattern_does_not_backtrack_on_a_non_matching_subject() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let pattern = scope.root_string_ptr(make_string("^(a+)+$")); + let flags = scope.root_string_ptr(make_string("")); + let re = pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }); + // The pattern really is on the backtracker — that is the premise. + assert!( + lookup_repeat_matcher(re).is_some(), + "a capture under a quantifier must route to the ECMAScript matcher" + ); + + let hay = format!("{}!", "a".repeat(40)); + let subject = scope.root_string_ptr(make_string(&hay)); + let started = std::time::Instant::now(); + assert_eq!( + subject.with_const_ptr::(|s| js_regexp_test(re, s)), + 0, + "no match: the subject ends in '!'" + ); + assert!( + started.elapsed() < std::time::Duration::from_secs(2), + "a non-matching subject must not be handed to the backtracker \ + (took {:?} for 40 characters)", + started.elapsed() + ); + + // A subject that DOES match still goes through the backtracker and still + // reports the spec's captures. + let good = scope.root_string_ptr(make_string("aaaa")); + assert_eq!( + good.with_const_ptr::(|s| js_regexp_test(re, s)), + 1 + ); +} + +/// #6759 phase 1 follow-up: a `RegExp` receiver can now answer the +/// descriptor-summary probe. Before the meta edge was wired for +/// `GC_TYPE_REGEXP`, `may_have_descriptor_entry` answered the conservative +/// `true` for every RegExp, so `set_last_index_throwing` built a `String` and +/// SipHashed `(usize, String)` on every global/sticky `test()`/`exec()`. +#[test] +fn a_fresh_regexp_proves_lastindex_absent_without_probing_the_tables() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let pattern = scope.root_string_ptr(make_string("x")); + let flags = scope.root_string_ptr(make_string("g")); + let re = pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }); + // Premise: this really is the dedicated RegExp cell, not a shaped object + // that would have answered through the ordinary `GC_TYPE_OBJECT` path. + let gc = unsafe { crate::value::addr_class::try_read_gc_header(re as usize) } + .expect("RegExp must be a GC allocation"); + assert_eq!(gc.obj_type, crate::gc::GC_TYPE_REGEXP); + + assert!( + !crate::object::test_may_have_descriptor_entry(re as usize, "lastIndex", false), + "a fresh RegExp has no descriptors, so the meta summary must prove \ + `lastIndex` absent instead of sending the caller to the table" + ); + assert!( + crate::object::get_property_attrs(re as usize, "lastIndex").is_none(), + "and the answer the fast path skips must be the same one" + ); +} + +/// The other half, and the one that makes the fast negative safe: an owner +/// that DOES have a descriptor must still be found. Install and probe share +/// one predicate, so a probe widened without its install would answer +/// "proven absent" here and `set_last_index_throwing` would silently stop +/// throwing (test262 prototype/{exec,test}/y-fail-lastindex-no-write). +#[test] +fn a_regexp_with_a_non_writable_lastindex_is_still_found_by_the_probe() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let pattern = scope.root_string_ptr(make_string("x")); + let flags = scope.root_string_ptr(make_string("g")); + let re = pattern.with_mut_ptr::(|pattern| { + flags.with_mut_ptr::(|flags| js_regexp_new(pattern, flags)) + }); + let attrs = crate::object::PropertyAttrs::new(false, true, true); + crate::object::set_property_attrs(re as usize, "lastIndex".to_string(), attrs); + + assert!( + crate::object::test_may_have_descriptor_entry(re as usize, "lastIndex", false), + "the install set the key bit, so the probe must send the caller to the table" + ); + let found = crate::object::get_property_attrs(re as usize, "lastIndex") + .expect("the descriptor the test installed must be readable back"); + assert!(!found.writable(), "and it must still read as non-writable"); + + // A DIFFERENT key on the same owner stays proven-absent: the summary is + // per key, not per owner, so widening it must not blunt it. + assert!( + !crate::object::test_may_have_descriptor_entry(re as usize, "source", false), + "an unrelated key on the same RegExp must still take the fast negative" + ); +} + +// --------------------------------------------------------------------------- +// Literal-site keyed construction (`js_regexp_new_site` + `regex::site_key`). +// +// The site key is the address of a private global the compiler emits once per +// regex literal. These statics stand in for two such globals: distinct +// addresses, 8-byte aligned, immortal — the same three properties the emitted +// ones have, and the reason the key is a sound identity where a `StringHeader` +// address is not. +// --------------------------------------------------------------------------- + +// Distinct initialisers so nothing may merge them: the test uses only their +// ADDRESSES, and identical zero-valued statics are exactly the shape a +// constant-merging pass is allowed to collapse. `assert_ne!` on the two keys +// keeps that from being a silent assumption. +static SITE_SLOT_A: u64 = 0xA; +static SITE_SLOT_B: u64 = 0xB; +static SITE_SLOT_C: u64 = 0xC; + +fn site_key_of(slot: &'static u64) -> i64 { + slot as *const u64 as i64 +} + +/// **The sabotage the coordinator named: key the table by pattern length.** +/// +/// Two literals at two sites, same flags, same pattern LENGTH, different text. +/// A table that verifies a probe by anything weaker than the site key — a +/// length, a prefix, a fingerprint without the exactness check — hands the +/// second site the first site's entry, and `.source` then reports a pattern +/// this literal never contained while `test` matches the wrong language. +/// +/// Each site is constructed twice: the first construction records, the second +/// is the one that must come back from the table, which is the case a weak key +/// breaks. Asserting only on a first construction would pass under every +/// sabotage, because a miss always takes the content-keyed path. +#[test] +fn two_literal_sites_with_equal_length_patterns_never_answer_for_each_other() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + let key_a = site_key_of(&SITE_SLOT_A); + let key_b = site_key_of(&SITE_SLOT_B); + assert_ne!(key_a, key_b, "two sites must have two addresses"); + + for round in 0..2 { + let a = js_regexp_new_site(make_string("a.c"), make_string("g"), key_a); + let b = js_regexp_new_site(make_string("x.z"), make_string("g"), key_b); + assert_eq!( + string_payload(js_regexp_get_source(a)), + b"a.c".to_vec(), + "round {round}: site A must report its own pattern" + ); + assert_eq!( + string_payload(js_regexp_get_source(b)), + b"x.z".to_vec(), + "round {round}: site B must report its own pattern — a table keyed by anything \ + weaker than the site address hands B the entry A recorded, and both patterns are \ + three bytes long" + ); + assert!( + js_regexp_test(a, make_string("abc")) != 0 + && js_regexp_test(a, make_string("xyz")) == 0, + "round {round}: site A must match its own language" + ); + assert!( + js_regexp_test(b, make_string("xyz")) != 0 + && js_regexp_test(b, make_string("abc")) == 0, + "round {round}: site B must match its own language" + ); + } + + assert_eq!( + site_key::test_recorded_pattern(key_a, "g").as_deref(), + Some("a.c") + ); + assert_eq!( + site_key::test_recorded_pattern(key_b, "g").as_deref(), + Some("x.z") + ); +} + +/// A dynamic `new RegExp(str)` must never reach the site table. +/// +/// The two-argument entry point is what every non-literal construction uses — +/// `js_regexp_construct`, `RegExp.prototype.compile`, the runtime's own +/// callers — and it has no site to be keyed by. If it recorded under some +/// stand-in key, a later literal whose key collided would inherit a pattern +/// that a *variable* produced, which is the one thing the site key's +/// compile-time-constant argument is supposed to guarantee against. +#[test] +fn a_dynamic_construction_records_nothing_in_the_site_table() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + + for _ in 0..4 { + let re = js_regexp_new(make_string("dyn(amic)"), make_string("g")); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + } + assert_eq!( + site_key::test_occupied_slots(), + 0, + "the two-argument entry point has no site key and must record nothing" + ); + + // The same TEXT through the site entry does record — so the zero above is + // a property of the entry point, not of a table that never works. + let key = site_key_of(&SITE_SLOT_C); + let re = js_regexp_new_site(make_string("dyn(amic)"), make_string("g"), key); + assert!(js_regexp_test(re, make_string("dynamic")) != 0); + assert_eq!( + site_key::test_occupied_slots(), + 1, + "the site entry point must record — otherwise the assertion above proves nothing" + ); + assert_eq!( + site_key::test_recorded_pattern(key, "g").as_deref(), + Some("dyn(amic)") + ); +} + +/// 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. +/// +/// 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 +/// `test()` and give the site key nothing. +#[test] +fn a_site_hit_after_the_first_execution_is_born_built() { + let _lock = crate::gc::global_side_table_test_lock(); + site_key::test_reset(); + let key = site_key_of(&SITE_SLOT_A); + + let first = js_regexp_new_site(make_string("bo+rn"), make_string(""), key); + assert!( + unsafe { (*first).regex_ptr }.is_null(), + "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(), + "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(), + "a site hit must install the programs the site already compiled, so the header is born \ + built and the first match pays no lookup" + ); + assert_ne!(first, second, "each evaluation is still a distinct object"); + assert!(js_regexp_test(second, make_string("born")) != 0); +} + +/// The kill switch has to remove the lane, not just its answers: with +/// `PERRY_REGEX_SITE_KEY=0` the table records nothing, so the OFF arm is the +/// content-keyed path exactly rather than a control still paying the +/// bookkeeping. +/// +/// Read once per process through a `OnceLock`, so this asserts the DEFAULT is +/// on rather than flipping the variable mid-run (which would only test the +/// cache of the first read). +#[test] +fn the_site_key_lane_is_on_by_default() { + assert!( + crate::gc::env_default_on_from_value(None), + "the site-key lane defaults ON; `PERRY_REGEX_SITE_KEY=0` is the kill switch" + ); + assert!(!crate::gc::env_default_on_from_value(Some("0"))); +} diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 13909495cd..f1582b60c2 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -407,12 +407,6 @@ "verdict": "not_a_gc_pointer", "why": "#9870: `PERRY_SEGVIEW_DIAG` tally for the Intl.Segmenter view mode \u2014 how often the fast path opened, and which check declined it. A plain `AtomicU64` written only through `bump()`'s `fetch_add(1, Relaxed)` and read only by the diagnostic dump: it holds a COUNT, never an address or a NaN-boxed value, so there is no slot for the collector to mark or rewrite." }, - { - "file": "crates/perry-runtime/src/intl/segments_view.rs", - "name": "DECLINE_NOT_STRING", - "verdict": "not_a_gc_pointer", - "why": "#9870: `PERRY_SEGVIEW_DIAG` tally for the Intl.Segmenter view mode \u2014 how often the fast path opened, and which check declined it. A plain `AtomicU64` written only through `bump()`'s `fetch_add(1, Relaxed)` and read only by the diagnostic dump: it holds a COUNT, never an address or a NaN-boxed value, so there is no slot for the collector to mark or rewrite." - }, { "file": "crates/perry-runtime/src/intl/segments_view.rs", "name": "DECLINE_NOT_UTF8", @@ -431,12 +425,6 @@ "verdict": "not_a_gc_pointer", "why": "#9870: `PERRY_SEGVIEW_DIAG` tally for the Intl.Segmenter view mode \u2014 how often the fast path opened, and which check declined it. A plain `AtomicU64` written only through `bump()`'s `fetch_add(1, Relaxed)` and read only by the diagnostic dump: it holds a COUNT, never an address or a NaN-boxed value, so there is no slot for the collector to mark or rewrite." }, - { - "file": "crates/perry-runtime/src/intl/segments_view.rs", - "name": "OPENS", - "verdict": "not_a_gc_pointer", - "why": "#9870: `PERRY_SEGVIEW_DIAG` tally for the Intl.Segmenter view mode \u2014 how often the fast path opened, and which check declined it. A plain `AtomicU64` written only through `bump()`'s `fetch_add(1, Relaxed)` and read only by the diagnostic dump: it holds a COUNT, never an address or a NaN-boxed value, so there is no slot for the collector to mark or rewrite." - }, { "file": "crates/perry-runtime/src/map.rs", "name": "MAP_COMPACTION_LOG", @@ -607,6 +595,32 @@ "verdict": "not_a_gc_pointer", "why": "Megamorphic property-read stub cache, the read twin of WRITE_STUB: 2-way ways of (shape_token, key_bits, slot) plain u64s. The token is a shape id and the slot an index; key_bits are content-derived by construction, because read_stub_key_bits returns short_ascii_sso_bits(key) \u2014 the key's characters packed inline \u2014 and yields None for any key that would otherwise be stored under a pointer. No way holds a heap address, so nothing here keeps an object alive, and a stale entry cannot hit: receiver_shape_token returns None for a receiver with no live shape, and the token identifies the exact key set and order, so a shape change yields a different token." }, + { + "file": "crates/perry-runtime/src/object/regex_proto_thunks.rs", + "name": "REGEXP_PROTOTYPE_PTR_SLOT", + "verdict": "covered_elsewhere", + "why": "#9893: the realm's `RegExp.prototype` address, recorded so the view mode's canonicality proof is three loads instead of a by-name walk. It IS a root and it IS scanned: `object::scan_object_cache_roots_mut` (registered via `reg_scanner!` in gc/mod.rs) visits it with `visit_atomic_i64_slot` beside the iterator-prototype towers, which marks it and rewrites it when the collector moves the prototype. The walk does not reach it because access goes through the `RealmAtomicI64` wrapper's `with_slot`, not a direct static read.", + "scanner": "object::scan_object_cache_roots_mut (crates/perry-runtime/src/object/mod.rs), registered by reg_scanner! in crates/perry-runtime/src/gc/mod.rs" + }, + { + "file": "crates/perry-runtime/src/object/regex_proto_thunks.rs", + "name": "REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT", + "verdict": "covered_elsewhere", + "why": "#9893: the canonical `RegExp.prototype.test` closure, NaN-BOXED rather than a bare address, and visited as such \u2014 `scan_object_cache_roots_mut` uses `visit_atomic_nanbox_u64_slot` so the collector rewrites the pointer inside the word. Same wrapper indirection as `REGEXP_PROTOTYPE_PTR_SLOT` above.", + "scanner": "object::scan_object_cache_roots_mut (crates/perry-runtime/src/object/mod.rs), registered by reg_scanner! in crates/perry-runtime/src/gc/mod.rs" + }, + { + "file": "crates/perry-runtime/src/object/regex_proto_thunks.rs", + "name": "REGEXP_PROTOTYPE_TEST_INDEX_SLOT", + "verdict": "not_a_gc_pointer", + "why": "#9893: the field INDEX `test` occupies on the prototype \u2014 a `u32` ordinal, not an address, sentinel `u32::MAX`. Nothing for the collector to mark or rewrite." + }, + { + "file": "crates/perry-runtime/src/object/regex_proto_thunks.rs", + "name": "REGEXP_PROTOTYPE_TEST_WALKS", + "verdict": "not_a_gc_pointer", + "why": "#9893: how many by-name canonicality walks this process has done \u2014 a plain `u64` count whose whole purpose is to read 1 per realm and thereby prove the fast path is the path being taken. A NUMBER, never an address." + }, { "file": "crates/perry-runtime/src/object/shapes_store.rs", "name": "ID_LIST_OP_STATS", @@ -683,12 +697,6 @@ "verdict": "not_a_gc_pointer", "why": "Atomic count of live PTY handles that currently keep the event loop active. It stores only a scalar count; PTY JS values live in PTY_LIVE and are visited by pty_reactor_scan_roots_mut." }, - { - "file": "crates/perry-runtime/src/regex.rs", - "name": "NEVER_MATCH", - "verdict": "not_a_gc_pointer", - "why": "#9796: the memoized never-match placeholder program installed for a pattern only `fancy-regex` accepts. `Arc` is a Rust-allocator compiled program \u2014 the collector neither traces nor moves it \u2014 and the `Arc` keeps it alive independently." - }, { "file": "crates/perry-runtime/src/regex.rs", "name": "REGEX_POINTERS", @@ -711,6 +719,12 @@ "verdict": "not_a_gc_pointer", "why": "Memoises which (pattern, flags) pairs have already passed the eager syntax check, so a repeated literal validates once (#9178). Keys are owned Rust `String`s copied out of the JS strings, and the value is `()` \u2014 nothing in the map is or points to a JS heap object, so there is no slot for the collector to mark or rewrite. The JS string a pattern came from is rooted by its own RegExp header, independently of this table." }, + { + "file": "crates/perry-runtime/src/regex/compile_cache.rs", + "name": "NEVER_MATCH", + "verdict": "not_a_gc_pointer", + "why": "#9796: the memoized never-match placeholder program installed for a pattern only `fancy-regex` accepts. `Arc` is a Rust-allocator compiled program \u2014 the collector neither traces nor moves it \u2014 and the `Arc` keeps it alive independently." + }, { "file": "crates/perry-runtime/src/set.rs", "name": "SET_COMPACTION_LOG", diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index 5884403015..b27978e5db 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -1,9 +1,10 @@ { "_comment": "Files still declaring raw `thread_local!`. The count is the number of DECLARATIONS that survive into a shipping build \u2014 each one pays `_tlv_get_addr` per read on Darwin \u2014 and it is a ratchet, so adding a `static` to an already-listed file fails whether or not it opens a new block. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", - "_hot_declarations": 385, + "_hot_declarations": 405, "files": { "crates/perry-runtime/src/agent.rs": 1, "crates/perry-runtime/src/arena/block.rs": 5, + "crates/perry-runtime/src/arena/page_meta/mod.rs": 2, "crates/perry-runtime/src/async_context.rs": 3, "crates/perry-runtime/src/async_hooks.rs": 7, "crates/perry-runtime/src/builtins/console.rs": 4, @@ -54,7 +55,7 @@ "crates/perry-runtime/src/node_inspector.rs": 2, "crates/perry-runtime/src/node_repl.rs": 1, "crates/perry-runtime/src/node_stream_constructors.rs": 3, - "crates/perry-runtime/src/node_stream_tests.rs": 23, + "crates/perry-runtime/src/node_stream_tests.rs": 24, "crates/perry-runtime/src/node_submodules/blob.rs": 4, "crates/perry-runtime/src/node_submodules/diagnostics.rs": 9, "crates/perry-runtime/src/node_submodules/diagnostics_tail.rs": 2, @@ -83,7 +84,6 @@ "crates/perry-runtime/src/v8.rs": 2, "crates/perry-runtime/src/wasi.rs": 1, "crates/perry-runtime/src/weakref.rs": 2, - "crates/perry-runtime/src/web_storage.rs": 2, - "crates/perry-runtime/src/arena/page_meta/mod.rs": 2 + "crates/perry-runtime/src/web_storage.rs": 2 } } From e046bca1feaf570bb02ecfae004e5ee01dc2888d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 22:55:38 +0200 Subject: [PATCH 19/19] fix(train): retarget the RegExp birth census, audit the prototype-ptr store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - shape_descriptor_census asserted the dedicated GC birth kind inside `js_regexp_new`. #9892 split construction into a thin js_regexp_new / js_regexp_new_site pair over a shared js_regexp_new_impl, which is where the allocation now lives, so the census read a wrapper with no birth site. It follows the birth site instead. Verified the retargeted gate still fails when the kind is blunted inside js_regexp_new_impl. - gc_store_site_inventory wanted a marker on `REGEXP_PROTOTYPE_PTR.store`. The store is already correct — the sibling closure store spells out its barrier only because it uses with_slot and bypasses the wrapper, while RealmAtomicI64::store routes through runtime_store_root_atomic_raw_i64 itself. Marker records that. --- crates/perry-runtime/src/object/regex_proto_thunks.rs | 5 +++++ scripts/shape_descriptor_census.py | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/object/regex_proto_thunks.rs b/crates/perry-runtime/src/object/regex_proto_thunks.rs index c11ff4a563..389499db5d 100644 --- a/crates/perry-runtime/src/object/regex_proto_thunks.rs +++ b/crates/perry-runtime/src/object/regex_proto_thunks.rs @@ -469,6 +469,11 @@ fn record_canonical_test_site(proto_obj: *mut ObjectHeader) { std::sync::atomic::Ordering::Release, ); }); + // GC_STORE_AUDIT(ROOT): REGEXP_PROTOTYPE_PTR is a mutable raw-address root + // visited by scan_object_cache_roots_mut. `RealmAtomicI64::store` routes + // through `runtime_store_root_atomic_raw_i64`, so the heap-word barrier + // runs here too — the sibling closure store spells that out only because + // it goes through `with_slot` and bypasses the wrapper. REGEXP_PROTOTYPE_PTR.store(addr, std::sync::atomic::Ordering::Release); } diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 0f03ff37b4..104ec61dff 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -574,7 +574,10 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: # point of the check: whichever allocator RegExp is born from, it is born # with its OWN GcHeader kind, never as a generic object that something later # has to re-identify by payload magic. - regexp_alloc = function_body(regex_runtime, "js_regexp_new") + # #9892 split construction into a thin `js_regexp_new` / `js_regexp_new_site` + # pair over a shared `js_regexp_new_impl`, which is where the allocation now + # lives. Follow the birth site rather than the entry point's name. + regexp_alloc = function_body(regex_runtime, "js_regexp_new_impl") require_code( regexp_alloc, r"(?:gc_malloc|arena_alloc_gc)\s*\([^;]*crate::gc::GC_TYPE_REGEXP",