diff --git a/crates/perry-runtime/src/intl/segments_view.rs b/crates/perry-runtime/src/intl/segments_view.rs index f33489004b..fbadacf6a9 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,13 +113,41 @@ 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. +/// +/// **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)) } @@ -713,6 +735,85 @@ 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_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); + 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/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 db5dba4de6..c11ff4a563 100644 --- a/crates/perry-runtime/src/object/regex_proto_thunks.rs +++ b/crates/perry-runtime/src/object/regex_proto_thunks.rs @@ -316,41 +316,162 @@ fn regex_instance_or_throw(method: &str) -> *const crate::regex::RegExpHeader { )) } +crate::perry_thread_local! { + /// 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_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 +/// 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); + /// 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 proto_ptr = REGEXP_PROTOTYPE_PTR.load(std::sync::atomic::Ordering::Acquire); + let canonical = REGEXP_PROTOTYPE_TEST_CLOSURE.load(std::sync::atomic::Ordering::Acquire); + 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 { + 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; + } + let addr = proto_obj as i64; + 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` /// 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 +482,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}`).