From 638b8327f9f254ddb33d9e40820d3e52c1581263 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 1/3] 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. --- .../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 ea54f0a3deb3702126541ecd30150af952f07335 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 2/3] 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. --- .../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 f076656e5e2c5e9b43ad7ffe638cd79ddfead352 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 3/3] 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. --- .../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`