diff --git a/crates/perry-runtime/src/array/alloc.rs b/crates/perry-runtime/src/array/alloc.rs index 87320f7648..1fe0048aa9 100644 --- a/crates/perry-runtime/src/array/alloc.rs +++ b/crates/perry-runtime/src/array/alloc.rs @@ -46,6 +46,10 @@ pub extern "C" fn js_array_alloc(capacity: u32) -> *mut ArrayHeader { unsafe { // Initialize header (*ptr).length = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "ArrayHeader::capacity", + actual_capacity, + ); (*ptr).capacity = actual_capacity; // HOLE-initialize the whole capacity so the unused [length, capacity) // slack never holds stale arena bits that the whole-heap from-space @@ -88,6 +92,10 @@ pub(crate) fn js_array_alloc_named_props_reserved( ) as *mut ArrayHeader; unsafe { (*ptr).length = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "ArrayHeader::capacity", + actual_capacity, + ); (*ptr).capacity = actual_capacity; let front = ptr.add(1) as *mut u64; // GC_STORE_AUDIT(INIT): the header word is an INT32 box on a @@ -129,6 +137,10 @@ pub(crate) fn js_array_alloc_pointer_elements(capacity: u32) -> *mut ArrayHeader unsafe { (*ptr).length = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "ArrayHeader::capacity", + actual_capacity, + ); (*ptr).capacity = actual_capacity; // Arena slots can be reused after a raw-f64 array. The all-pointer // layout owns the same header, so clear numeric representation flags @@ -173,6 +185,10 @@ pub extern "C" fn js_array_alloc_with_length(capacity: u32) -> *mut ArrayHeader unsafe { (*ptr).length = capacity; // Set length = requested capacity + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "ArrayHeader::capacity", + actual_capacity, + ); (*ptr).capacity = actual_capacity; let elements_ptr = crate::array::array_elements_ptr(ptr as *const ArrayHeader) as *mut u64; for i in 0..capacity as usize { diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 54cab10a38..0c07dd48b9 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -195,6 +195,10 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu let arr = arr_handle.get_raw_mut_ptr::(); let shifted = array_front_offset(arr) != reserve; (*new_ptr).length = (*arr).length; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "ArrayHeader::capacity (grow)", + new_capacity, + ); (*new_ptr).capacity = new_capacity; // GC_STORE_AUDIT(BARRIERED): growth normalizes the logical backing // range, transfers its layout, and replays its write barriers below. diff --git a/crates/perry-runtime/src/buffer/header.rs b/crates/perry-runtime/src/buffer/header.rs index 804bb30531..43b9a3bf49 100644 --- a/crates/perry-runtime/src/buffer/header.rs +++ b/crates/perry-runtime/src/buffer/header.rs @@ -989,6 +989,10 @@ pub fn buffer_alloc(capacity: u32) -> *mut BufferHeader { let header = (ptr as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; (*header).gc_flags |= crate::gc::GC_FLAG_TENURED; (*ptr).length = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "BufferHeader::capacity", + capacity, + ); (*ptr).capacity = capacity; } register_buffer(ptr); diff --git a/crates/perry-runtime/src/gc/tests/alloc.rs b/crates/perry-runtime/src/gc/tests/alloc.rs index f44da8248f..17741cf3d7 100644 --- a/crates/perry-runtime/src/gc/tests/alloc.rs +++ b/crates/perry-runtime/src/gc/tests/alloc.rs @@ -644,6 +644,7 @@ fn alloc_malloc_kind_test_object(obj_type: u8) -> *mut u8 { ptr as *mut crate::promise::Promise, crate::promise::Promise { native_pinned: 0, + _shape_word_pad: [0; 6], state: crate::promise::PromiseState::Pending, value: 0.0, reason: 0.0, diff --git a/crates/perry-runtime/src/gc/tests/support.rs b/crates/perry-runtime/src/gc/tests/support.rs index bfd3a3ddd1..e187ce488e 100644 --- a/crates/perry-runtime/src/gc/tests/support.rs +++ b/crates/perry-runtime/src/gc/tests/support.rs @@ -75,6 +75,7 @@ pub(super) unsafe fn alloc_old_test_promise() -> *mut crate::promise::Promise { ptr, crate::promise::Promise { native_pinned: 0, + _shape_word_pad: [0; 6], state: crate::promise::PromiseState::Pending, value: 0.0, reason: 0.0, @@ -806,6 +807,7 @@ pub(super) fn allocate_dead_malloc_churn_headers(per_type: usize) -> Vec ptr, crate::promise::Promise { native_pinned: 0, + _shape_word_pad: [0; 6], state: crate::promise::PromiseState::Pending, value: 0.0, reason: 0.0, diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index a2b4ab8be0..dccf016cd2 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -862,6 +862,8 @@ struct SiteStat { /// cache held the right answer in a way and the read came back to the miss /// handler regardless. See [`IcDiag::prime_in_ways`]. prime_in_ways: u64, + /// Per-site hit count; see [`IcDiag::hits`]. + hits: u64, } #[derive(Default)] @@ -908,12 +910,66 @@ pub struct IcDiag { /// * `new_token` + not `in_ways` — a shape neither the MRU entry nor the /// ways had (genuine polymorphism, or a first sighting). pub prime_in_ways: u64, + /// The HIT side of the property-get IC, so a hit rate can be quoted + /// instead of a miss count. + /// + /// Everything else in this group is recorded from the runtime miss + /// handler, because that is the only part of a property read the runtime + /// sees: a hit is served entirely by the emitted diamond and never calls + /// in. Counting hits therefore needs an emitted probe, which is what + /// [`js_ic_diag_note_hit`] is — one `#[cold]`-guarded call on the hit + /// edge, behind the same `PERRY_IC_DIAG` arming as the misses, so an + /// unarmed run pays one relaxed load. + /// + /// This matters for the single-path object-model work: a fast path that + /// stops being taken is correct-but-slow and invisible in program output. + /// `hits` next to `misses` is the only way to tell "the guard was removed + /// and reads now hit" from "the guard was removed and every read falls to + /// the miss handler, still producing the right answer". + pub hits: u64, + /// Hits served from the MRU packed word versus one of the four ways. + pub hits_in_ways: u64, } crate::perry_thread_local! { static IC_DIAG: RefCell = RefCell::new(IcDiag::default()); } +/// Record one property-get IC HIT. See [`IcDiag::hits`]. +/// +/// Callers must gate on [`ic_on`] first — emitted code should load the +/// arming byte and branch, never call unconditionally. +pub fn ic_note_hit(site: usize, in_ways: bool) { + IC_DIAG.with(|d| { + let mut d = d.borrow_mut(); + if d.started.is_none() { + d.started = Some(Instant::now()); + d.last_dump = d.started; + } + d.hits += 1; + if in_ways { + d.hits_in_ways += 1; + } + let s = d.sites.entry(site).or_default(); + s.hits += 1; + }); +} + +/// Emitted-code entry point for [`ic_note_hit`]: `site` is the per-site cache +/// address the hit path already holds, `in_ways` is 0 for an MRU hit and 1 for +/// a polymorphic-way hit. +/// +/// Diagnostic only — nothing may branch on the counters, and the call must sit +/// behind an `ic_on()` test in the emitted code so an unarmed run never +/// reaches it. +#[no_mangle] +pub extern "C" fn js_ic_diag_note_hit(site: i64, in_ways: i32) { + if !ic_on() { + return; + } + ic_note_hit(site as usize, in_ways != 0); +} + /// Record one `pic_prime_get`, splitting it by whether the token the site is /// being primed with is one it already held — in the MRU entry (`same`) or in /// one of the ways (`in_ways`). See [`IcDiag::prime_same_token`] and @@ -997,10 +1053,19 @@ impl IcDiag { use std::fmt::Write as _; let mut out = String::with_capacity(4096); let secs = self.started.map_or(0.0, |t| t.elapsed().as_secs_f64()); + let reads = self.hits + self.misses; let _ = write!( out, - "[ic-diag] t={secs:.1}s misses={} sites={}", + "[ic-diag] t={secs:.1}s misses={} hits={} ({:.1} % of {reads} reads, \ + {} via ways) sites={}", self.misses, + self.hits, + if reads == 0 { + 0.0 + } else { + 100.0 * self.hits as f64 / reads as f64 + }, + self.hits_in_ways, self.sites.len() ); for (i, name) in IC_REASON_NAMES.iter().enumerate() { @@ -1034,7 +1099,7 @@ impl IcDiag { rows.sort_by_key(|s| std::cmp::Reverse(s.misses)); let _ = writeln!( out, - " misses same/new/inways fresh/armed/mega key reasons" + " misses hits same/new/inways fresh/armed/mega key reasons" ); for s in rows.iter().take(40) { let mut reasons = String::new(); @@ -1047,8 +1112,9 @@ impl IcDiag { } let _ = writeln!( out, - " {:6} {:>8}/{}/{:<8} {:>7}/{}/{:<8} {:<24}{reasons}", + " {:6} {:6} {:>8}/{}/{:<8} {:>7}/{}/{:<8} {:<24}{reasons}", s.misses, + s.hits, s.prime_same_token, s.prime_new_token, s.prime_in_ways, diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 7236eebea4..691f93a188 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1572,6 +1572,7 @@ pub extern "C" fn js_map_alloc(capacity: u32) -> *mut MapHeader { // Initialize header (*ptr).size = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word("MapHeader::capacity", cap); (*ptr).capacity = cap; // GC_STORE_AUDIT(INIT): map entries buffer is external storage; element stores are barriered separately. (*ptr).entries = entries; @@ -2048,6 +2049,10 @@ unsafe fn ensure_capacity(map: *mut MapHeader) -> bool { // GC_STORE_AUDIT(INIT): map external buffer pointer moves; live entry slots are dirtied by caller. (*map).entries = new_entries; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "MapHeader::capacity (grow)", + new_capacity, + ); (*map).capacity = new_capacity; MAP_REGISTRY.with(|registry| { let mut registry = registry.borrow_mut(); diff --git a/crates/perry-runtime/src/native_arena.rs b/crates/perry-runtime/src/native_arena.rs index 567b719a50..89229d6eeb 100644 --- a/crates/perry-runtime/src/native_arena.rs +++ b/crates/perry-runtime/src/native_arena.rs @@ -323,6 +323,10 @@ pub extern "C" fn js_native_arena_view( crate::gc::GC_TYPE_NATIVE_TYPED_VIEW, ) as *mut NativeTypedViewHeader; (*view).length = length as u32; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "NativeTypedViewHeader::capacity", + length as u32, + ); (*view).capacity = length as u32; (*view).kind = kind; (*view).elem_size = elem_size as u8; diff --git a/crates/perry-runtime/src/object/array_object_ops.rs b/crates/perry-runtime/src/object/array_object_ops.rs index 98d7e4fa95..b530103f1d 100644 --- a/crates/perry-runtime/src/object/array_object_ops.rs +++ b/crates/perry-runtime/src/object/array_object_ops.rs @@ -678,12 +678,11 @@ pub(crate) unsafe fn define_array_property( } // Redefining an index that was previously an accessor back to a data - // property: drop the stale accessor entry. - crate::state::state() - .descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(obj as usize, key_name.to_string())); + // property: drop the stale accessor entry. Through the funnel, so the + // owner index and (for a shaped receiver) the ShapeId follow — a raw + // table `remove` leaves `accessor_descriptor_keys_for_obj` reporting + // the key and every shape-keyed cache still claiming an accessor. + clear_accessor_descriptor(obj as usize, key_name); // [[DefineOwnProperty]] writes the slot directly — clear any stale // attrs first so the extend helper's [[Set]]-side writability check // (added for ordinary `arr[i] = v` writes) can't reject this store. @@ -862,11 +861,7 @@ pub(crate) unsafe fn define_array_property( // Redefining a former accessor back to a data property drops the stale // accessor entry (the non-configurable case already threw above). if cur_accessor.is_some() { - crate::state::state() - .descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(obj as usize, key_name.to_string())); + clear_accessor_descriptor(obj as usize, key_name); } // Write the value: an explicit `value` wins; a NEW property with no value diff --git a/crates/perry-runtime/src/object/class_meta_registry.rs b/crates/perry-runtime/src/object/class_meta_registry.rs index d7731b5f13..5dc4eb95cd 100644 --- a/crates/perry-runtime/src/object/class_meta_registry.rs +++ b/crates/perry-runtime/src/object/class_meta_registry.rs @@ -454,6 +454,12 @@ mod dense_parent_tests { /// cached store plan in the program. #[test] fn re_registering_the_same_edge_flushes_nothing() { + // The epoch this asserts on is PROCESS-global, so any other test + // thread installing a descriptor or registering a class inside the + // window below moves it and fails this test for reasons that have + // nothing to do with re-registration. Serialize against the tests + // that do (they all take this lock). + let _lock = crate::gc::global_side_table_test_lock(); const CHILD: u32 = 60_020; const PARENT: u32 = 60_021; crate::object::class_registry::register_class(CHILD, PARENT); diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 96da731562..9e9c6f8c9c 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -105,14 +105,16 @@ pub use state::{ // ── prototype_objects.rs ──────────────────────────────────────────────────── pub(crate) use prototype_objects::{ - class_prototype_object, ensure_function_prototype_object, function_class_id, - function_value_for_class_id, proto_chain_symbol_slot, resolve_proto_chain_field, - resolve_proto_chain_field_with_receiver, resolve_proto_chain_symbol, - synthetic_class_prototype_object, + alloc_synthetic_class_id, class_prototype_object, ensure_function_prototype_object, + function_class_id, function_value_for_class_id, proto_chain_symbol_slot, + resolve_proto_chain_field, resolve_proto_chain_field_with_receiver, resolve_proto_chain_symbol, + synthetic_class_prototype_object, SYNTHETIC_CLASS_ID_BASE, }; pub use prototype_objects::{ js_set_function_prototype, js_set_prototype_property, NEXT_SYNTHETIC_CLASS_ID, }; +#[cfg(test)] +pub(crate) use prototype_objects::{test_alloc_synthetic_class_id, SYNTHETIC_CLASS_ID_END}; // ── class_meta.rs ─────────────────────────────────────────────────────────── #[cfg(test)] diff --git a/crates/perry-runtime/src/object/class_registry/gc_roots.rs b/crates/perry-runtime/src/object/class_registry/gc_roots.rs index 4e344f998b..2d5178493a 100644 --- a/crates/perry-runtime/src/object/class_registry/gc_roots.rs +++ b/crates/perry-runtime/src/object/class_registry/gc_roots.rs @@ -718,7 +718,10 @@ pub(crate) fn test_clear_class_side_table_roots() { // it lives in the calling thread's class image (`object/class_image.rs`) // rather than a `per_test_global!`, where a clear from a guard on another // libtest thread would be the #7672 hazard this helper exists to avoid. - NEXT_SYNTHETIC_CLASS_ID.store(0x8000_0000, std::sync::atomic::Ordering::Relaxed); + NEXT_SYNTHETIC_CLASS_ID.store( + super::prototype_objects::SYNTHETIC_CLASS_ID_BASE, + std::sync::atomic::Ordering::Relaxed, + ); } #[cfg(test)] diff --git a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs index bd64ffc44c..8667bb0c8e 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs @@ -523,7 +523,7 @@ pub(crate) fn synthetic_class_id_for_function(func_value: f64) -> u32 { if let Some(existing) = existing { return existing; } - let new_cid = NEXT_SYNTHETIC_CLASS_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let new_cid = super::prototype_objects::alloc_synthetic_class_id(); FUNCTION_CLASS_IDS.with(|table| { let mut write = table.write().unwrap(); if write.is_none() { diff --git a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs index 9a689eba69..235da8406e 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_objects.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_objects.rs @@ -147,18 +147,82 @@ pub(crate) fn ensure_function_prototype_object( } /// Floor of the synthetic class-id range (see [`NEXT_SYNTHETIC_CLASS_ID`]). -pub(crate) const SYNTHETIC_CLASS_ID_BASE: u32 = 0x8000_0000; +/// +/// RULE 2 (single-path object model): a class id must never land inside the +/// ShapeId range `[SHAPE_ID_BASE, SHAPE_ID_END)`. The read path tells a +/// STAMPED object from an UNSTAMPED one by range alone — the `+4` word holds +/// `parent_class_id` until a shape is stamped over it, and +/// `shapes::object_shape_stamp` answers with that word exactly when +/// `shapes::is_shape_id` accepts it. This counter used to start at +/// `0x8000_0000`, the ShapeId floor, so synthetic id *n* and ShapeId *n* were +/// the same u32: an instance born with a synthetic PARENT id +/// (`class X extends someFunction`, resolved through +/// `dynamic_value_class_id` -> `function_class_id`, then written to `+4` by +/// `js_object_alloc_class_dynamic_parent`) claimed a live, unrelated shape, +/// whose ordered keys and live-inline-slot bound then described its layout. +/// The band above the ShapeId range is unused: codegen ids start at 1, the +/// builtin bands are `0x7FFF_FF00..=0x7FFF_FFFF` and `0xFFFF_0000..`. +pub(crate) const SYNTHETIC_CLASS_ID_BASE: u32 = 0xC000_0000; +/// Exclusive end of the synthetic range: the first reserved builtin id. +pub(crate) const SYNTHETIC_CLASS_ID_END: u32 = 0xFFFF_0000; + +const _: () = assert!( + SYNTHETIC_CLASS_ID_BASE >= crate::object::shapes::SHAPE_ID_END, + "rule 2: synthetic class ids must not overlap the ShapeId range" +); +const _: () = assert!( + SYNTHETIC_CLASS_ID_END > SYNTHETIC_CLASS_ID_BASE, + "the synthetic class-id range must be non-empty" +); per_test_global! { - /// Synthetic class id allocator for prototype-object classes. High bit - /// set (0x8000_0000+) to keep them separate from codegen-assigned ids - /// (which start from 1 and grow by module). u32 wraparound is not a - /// concern in practice — would require ~2 billion `Function.prototype = X` - /// statements at module init. + /// Synthetic class id allocator for prototype-object classes. Allocated + /// from `[SYNTHETIC_CLASS_ID_BASE, SYNTHETIC_CLASS_ID_END)` so the ids + /// stay separate from codegen-assigned ids (which start from 1 and grow + /// by module), from the reserved builtin bands, AND from the ShapeId + /// range. Mint through [`alloc_synthetic_class_id`], never by a bare + /// `fetch_add` — the bound is the invariant. pub static NEXT_SYNTHETIC_CLASS_ID: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(SYNTHETIC_CLASS_ID_BASE); } +/// Mint the next synthetic class id, parked at the end of the range rather +/// than wrapped. +/// +/// A bare `fetch_add` could walk out of the range in two ways, both of which +/// break a rule the read path depends on: past `SYNTHETIC_CLASS_ID_END` it +/// collides with the reserved builtin ids (`0xFFFF_0000..`), and on u32 wrap +/// it lands back in — among others — the ShapeId range. Exhaustion is +/// unreachable in practice (2^30 ids, one per distinct +/// `Object.create(proto)` / `F.prototype = X` FUNCTION, not per call), so +/// saturating is the conservative answer: `0` means "no synthetic id", which +/// every caller already handles as "stays parentless". +pub(crate) fn alloc_synthetic_class_id() -> u32 { + use std::sync::atomic::Ordering; + loop { + let id = NEXT_SYNTHETIC_CLASS_ID.load(Ordering::Relaxed); + if id >= SYNTHETIC_CLASS_ID_END { + NEXT_SYNTHETIC_CLASS_ID.store(SYNTHETIC_CLASS_ID_END, Ordering::Relaxed); + return 0; + } + if NEXT_SYNTHETIC_CLASS_ID + .compare_exchange_weak(id, id + 1, Ordering::Relaxed, Ordering::Relaxed) + .is_ok() + { + debug_assert!( + !crate::object::shapes::is_shape_id(id), + "rule 2: a synthetic class id must never be readable as a ShapeId" + ); + return id; + } + } +} + +#[cfg(test)] +pub(crate) fn test_alloc_synthetic_class_id() -> u32 { + alloc_synthetic_class_id() +} + /// The `[[Prototype]]` object recorded for a SYNTHETIC class id — one of the /// ids `Object.create(proto)` (#809) and `F.prototype = obj` (#711) allocate /// from [`NEXT_SYNTHETIC_CLASS_ID`]. That link is the authoritative prototype @@ -360,7 +424,7 @@ pub extern "C" fn js_set_function_prototype(func: f64, proto: f64) -> u32 { crate::typed_feedback::invalidate_method_change(existing); return existing; } - let new_cid = NEXT_SYNTHETIC_CLASS_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let new_cid = alloc_synthetic_class_id(); FUNCTION_CLASS_IDS.with(|table| { let mut write = table.write().unwrap(); if write.is_none() { diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 45f2b6b671..ac7456c5fb 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -584,13 +584,6 @@ pub(crate) unsafe fn class_instance_set_may_intercept( /// `GLOBAL_DESCRIPTORS_IN_USE`, neither is poisoned by the runtime /// installing attrs on unrelated builtins (RegExp prototype etc.), so the /// dynamic-write fast path stays precise. -/// #6710: set once a native HANDLE-band owner (small id, not a heap object) -/// gets a property-attr / accessor descriptor. Heap owners record this on their -/// GC header (`OBJ_FLAG_HAS_DESCRIPTORS`) but a handle id has no header, so -/// `clear_object_descriptors` uses this flag to skip the O(N) `retain` scans on -/// the common path where no handle was ever `defineProperty`'d. -static HANDLE_HAS_DESCRIPTORS: AtomicBool = AtomicBool::new(false); - pub(crate) fn note_descriptor_target(obj: usize) { note_descriptor_target_keyed(obj, None); } @@ -623,10 +616,31 @@ pub(crate) fn note_accessor_descriptor_target(obj: usize, key: &str, acc: &Acces note_descriptor_target_keyed(obj, Some((key.as_bytes(), shape))); } +/// The ONE funnel every descriptor install goes through, and therefore the +/// one place RULE 1 ("every descriptor change changes the ShapeId of an +/// ordinary object") is implemented. +/// +/// What it covers, and what it deliberately does not — this is the exact +/// scope any shape-only read guard inherits: +/// +/// * `GC_TYPE_OBJECT`: sets `OBJ_FLAG_HAS_DESCRIPTORS` **and**, when the +/// receiver is shaped, transitions the shape. A cache entry keyed on the +/// old ShapeId can no longer match, so for these receivers the shape +/// compare subsumes the flag test. +/// * **typed arrays**: early return, before either. A small typed array is +/// plain-alloc'd without a `GcHeader`, so there is no flag bit to set and +/// no `ObjectHeader` to stamp. +/// * **every other cell kind** (array, closure, Map/Set, RegExp, Error, +/// Promise, native handles, handle-band ids): the `obj_type` test below +/// rejects them, so they get neither the flag nor a shape transition. +/// +/// Consequence for the emitted read path: for a non-`GC_TYPE_OBJECT` +/// receiver the descriptor flag is *never set*, so dropping the flag test +/// loses nothing — but the SHAPE is equally uninformative, so such receivers +/// must still be rejected by KIND. Removing the GC-header load from the read +/// path needs their descriptor state carried in the shape word first +/// (`rule1_funnel_does_not_cover_non_object_receivers` pins this). fn note_descriptor_target_keyed(obj: usize, data_install: Option<(&[u8], u8)>) { - if crate::value::addr_class::is_handle_band(obj) { - HANDLE_HAS_DESCRIPTORS.store(true, Ordering::Relaxed); - } if crate::array::object_prototype_addr_matches(obj) { OBJECT_PROTO_DESCRIPTORS.store(true, Ordering::Relaxed); } @@ -1857,31 +1871,48 @@ fn remove_descriptor_owner_entries(st: &crate::state::RuntimeState, owner: usize /// only reaps entries whose owner is a dead *heap* object, so a recycled handle /// id's descriptors survive into the next owner. Called from /// `handle_expando_clear` when perry-ffi hands a freed handle id back out. +/// +/// RULE 1: this is a BULK descriptor removal — after it every key of the +/// receiver is an ordinary `{writable, enumerable, configurable}` data +/// property again — so a shaped ordinary receiver must transition, exactly as +/// the per-key [`clear_property_attrs`] / [`clear_accessor_descriptor`] do. +/// It used to make no shape call at all, so a cache primed while the object +/// was frozen kept serving the frozen answer. Today's only production caller +/// passes a handle-band id, which has no header and no shape (the transition +/// is then skipped by `object_is_shaped`), but nothing in the signature says +/// so and the function is reachable from anywhere in the crate. +/// +/// The removal itself goes through the owner index rather than two O(table) +/// `retain` scans. That index is authoritative — every insert in this file +/// calls `owner_index_add`/`owner_index_push_proven_new` first, and +/// `gc_scan.rs` keeps it in step across evacuation — so it is both complete +/// and O(this owner's keys). It also removes the need for the +/// `HANDLE_HAS_DESCRIPTORS` latch that existed only to skip those scans, and +/// with it the silent no-op this function performed for a HEAP owner while no +/// handle had ever taken a descriptor. pub(crate) fn clear_object_descriptors(obj: usize) { - // Fast path: if no handle-band owner ever received a descriptor, these - // tables hold only heap owners — none of whose keys can match `obj` (a - // handle id) — so the O(N) `retain` scans would remove nothing. Skip them. - if !HANDLE_HAS_DESCRIPTORS.load(Ordering::Relaxed) { - return; - } let st = state(); - { - let mut m = st.descriptors.property_descriptors.borrow_mut(); - if !m.is_empty() { - m.retain(|(owner, _), _| *owner != obj); - } + let owned_any = st + .descriptors + .attr_keys_by_owner + .borrow() + .contains_key(&obj) + || st + .descriptors + .accessor_keys_by_owner + .borrow() + .contains_key(&obj); + if !owned_any { + return; } - { - let mut m = st.descriptors.accessor_descriptors.borrow_mut(); - if !m.is_empty() { - m.retain(|(owner, _), _| *owner != obj); + remove_descriptor_owner_entries(st, obj); + super::prop_plan::prop_plan_epoch_bump(); + unsafe { + let object = obj as *mut crate::object::ObjectHeader; + if crate::object::object_is_shaped(object) { + crate::object::shapes::transition_object_shape_semantics(object); } } - st.descriptors.attr_keys_by_owner.borrow_mut().remove(&obj); - st.descriptors - .accessor_keys_by_owner - .borrow_mut() - .remove(&obj); } /// Move string-keyed descriptor ownership when `ArrayHeader` growth replaces @@ -1889,6 +1920,27 @@ pub(crate) fn clear_object_descriptors(obj: usize) { /// the metadata-rewrite scanner below does not run; without this explicit /// transfer, descriptors installed before a later grow remain keyed to the /// forwarding stub and disappear from reads through the canonical array head. +/// +/// RULE 1: this makes NO shape call, and that is correct. It is not a +/// descriptor CHANGE — it is a RE-KEY of one logical object whose allocation +/// moved. The descriptor set, the attributes and the accessors are all +/// identical either side; only the address the tables index them under +/// changes. A shape transition here would be actively wrong: it would retire +/// every cache entry for a shape the object still has. +/// +/// The contract that makes it correct is the CALLER's: the replacement cell +/// must already carry the original's identity — `js_array_grow` copies the +/// `GcHeader` (`_reserved`, hence `OBJ_FLAG_HAS_DESCRIPTORS`) verbatim before +/// calling here, and an array never carries the flag in the first place +/// (`note_descriptor_target_keyed` only sets it for `GC_TYPE_OBJECT`). +/// +/// Where that contract is NOT met — any caller handing the entries to a cell +/// that is not already the same object — the moved descriptors would land on +/// a receiver whose header and shape both still say "no descriptors", which +/// is precisely what rule 1 forbids. Repair it rather than assert it: route +/// the new owner through the funnel, which sets the flag and transitions a +/// shaped ordinary receiver. Free on the array-growth path, where the +/// condition is false. pub(crate) fn transfer_descriptor_owner(old_owner: usize, new_owner: usize) { if old_owner == new_owner { return; @@ -1964,6 +2016,13 @@ pub(crate) fn transfer_descriptor_owner(old_owner: usize, new_owner: usize) { for key in &moved_acc { note_meta_descriptor_key(new_owner, key, true); } + + // Rule 1 repair (see the doc comment): a new owner that did not already + // carry the source's descriptor state must not silently acquire + // descriptors behind an unchanged header and shape. + if object_has_descriptors(old_owner) && !object_has_descriptors(new_owner) { + note_descriptor_target(new_owner); + } } /// Rewrite a descriptor table's owner ADDRESS during the GC metadata-rewrite diff --git a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs index 48471ca672..b71c5afa9d 100644 --- a/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs +++ b/crates/perry-runtime/src/object/descriptor_state/gc_scan.rs @@ -356,10 +356,6 @@ mod owner_index_tests { fn clear_object_descriptors_empties_the_index_too() { let _lock = crate::gc::global_side_table_test_lock(); let obj = crate::object::js_object_alloc(0, 0) as usize; - // `clear_object_descriptors` early-returns unless a handle-band owner - // has ever taken a descriptor; set the latch so the body actually runs. - HANDLE_HAS_DESCRIPTORS.store(true, Ordering::Relaxed); - set_property_attrs(obj, "p".to_string(), PropertyAttrs::new(true, true, true)); set_accessor_descriptor(obj, "acc".to_string(), AccessorDescriptor::default()); assert_mirrors(obj, "before clear"); diff --git a/crates/perry-runtime/src/object/handle_expando.rs b/crates/perry-runtime/src/object/handle_expando.rs index 731b36bbba..fe241abe1e 100644 --- a/crates/perry-runtime/src/object/handle_expando.rs +++ b/crates/perry-runtime/src/object/handle_expando.rs @@ -228,15 +228,11 @@ pub(crate) fn handle_expando_delete(handle: i64, name: &str) -> bool { props.retain(|(k, _)| k != name); } }); - let st = crate::state::state(); - st.descriptors - .property_descriptors - .borrow_mut() - .remove(&(handle as usize, name.to_string())); - st.descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(handle as usize, name.to_string())); + // Through the funnel: a raw table `remove` left the owner index naming a + // key the tables no longer hold, so `accessor_descriptor_keys_for_obj` + // kept reporting a deleted expando accessor for the handle. + super::descriptor_state::clear_property_attrs(handle as usize, name); + super::descriptor_state::clear_accessor_descriptor(handle as usize, name); true } @@ -436,10 +432,6 @@ mod tests { HANDLE_EXPANDO_PROPS.with(|cell| { cell.borrow_mut().remove(&h); }); - crate::state::state() - .descriptors - .property_descriptors - .borrow_mut() - .remove(&(h as usize, "hid".to_string())); + crate::object::descriptor_state::clear_property_attrs(h as usize, "hid"); } } diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 8d2b99c02d..1474ee9879 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -1854,6 +1854,9 @@ pub(super) unsafe fn mark_object_dynamic_shape_unknown(obj: *mut ObjectHeader) { mod own_key_probe_tests; #[cfg(test)] mod restricted_function_store_tests; +pub(crate) mod shape_rule3; +#[cfg(test)] +mod shape_rules_tests; #[cfg(test)] mod test_root_accessors; #[cfg(test)] diff --git a/crates/perry-runtime/src/object/native_get.rs b/crates/perry-runtime/src/object/native_get.rs index b3315dd4d3..f9556eae6a 100644 --- a/crates/perry-runtime/src/object/native_get.rs +++ b/crates/perry-runtime/src/object/native_get.rs @@ -88,7 +88,7 @@ pub(crate) unsafe fn try_data_get_bytes(receiver: JSValue, key: &[u8]) -> Option // Positive membership, rather than a blacklist of native class ids. // Synthetic function/Object.create ids occupy the allocated prefix // of this counter's range; reserved native ids are outside it. - let synthetic = class_id >= 0x8000_0000 + let synthetic = class_id >= super::class_registry::SYNTHETIC_CLASS_ID_BASE && class_id < super::NEXT_SYNTHETIC_CLASS_ID.load(std::sync::atomic::Ordering::Relaxed); if class_id != 0 && !synthetic && !super::is_anon_shape_class_id(class_id) { return None; diff --git a/crates/perry-runtime/src/object/object_ops/define_property.rs b/crates/perry-runtime/src/object/object_ops/define_property.rs index c7ea7e6dde..ab452920bf 100644 --- a/crates/perry-runtime/src/object/object_ops/define_property.rs +++ b/crates/perry-runtime/src/object/object_ops/define_property.rs @@ -241,11 +241,7 @@ unsafe fn define_property_on_handle( // (`{ enumerable: true }` alone). Drop any accessor that used to occupy // the key so the store can't fire a stale setter. if had_accessor { - crate::state::state() - .descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(hid as usize, key.clone())); + clear_accessor_descriptor(hid as usize, &key); } // [[Value]] is the descriptor's when present; otherwise it defaults to // `undefined` for a BRAND-NEW key or an accessor→data conversion (neither @@ -1133,11 +1129,7 @@ pub extern "C" fn js_object_define_property( let value_key = crate::string::js_string_from_bytes(b"value".as_ptr(), 5); let value_field = js_object_get_field_by_name(desc_ptr as *const ObjectHeader, value_key); - crate::state::state() - .descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(closure_ptr, key_rust.clone())); + clear_accessor_descriptor(closure_ptr, &key_rust); if !value_field.is_undefined() { crate::closure::closure_set_dynamic_prop( closure_ptr, @@ -1667,11 +1659,7 @@ pub extern "C" fn js_object_define_property( // `writable: false` doesn't reject the forced store below. The // final attributes are (re)applied a few lines down. if let Some(ref k) = key_rust { - crate::state::state() - .descriptors - .accessor_descriptors - .borrow_mut() - .remove(&(obj as usize, k.clone())); + clear_accessor_descriptor(obj as usize, k); clear_property_attrs(obj as usize, k); } // Ensure the key exists; store the (possibly `undefined`) value diff --git a/crates/perry-runtime/src/object/object_ops/prototype.rs b/crates/perry-runtime/src/object/object_ops/prototype.rs index a4e0bcfb59..f1941d17cc 100644 --- a/crates/perry-runtime/src/object/object_ops/prototype.rs +++ b/crates/perry-runtime/src/object/object_ops/prototype.rs @@ -118,8 +118,7 @@ pub extern "C" fn js_object_create(proto_value: f64) -> f64 { || crate::regex::is_regex_pointer(proto_ptr as *const u8)); let valid = modellable && is_valid_obj_ptr(proto_ptr as *const u8); if valid { - let cid = - NEXT_SYNTHETIC_CLASS_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let cid = crate::object::class_registry::alloc_synthetic_class_id(); class_prototype_object_root_store(cid, proto_ptr); unsafe { js_register_class_id(cid) }; // #1805: link the synthetic class_id into the original class's diff --git a/crates/perry-runtime/src/object/shape_rule3.rs b/crates/perry-runtime/src/object/shape_rule3.rs new file mode 100644 index 0000000000..33c6796457 --- /dev/null +++ b/crates/perry-runtime/src/object/shape_rule3.rs @@ -0,0 +1,323 @@ +//! Rule 3 of the single-path object model: **no pointer-tagged non-object +//! cell may hold a value in the ShapeId range at payload offset `+4`.** +//! +//! The emitted read path derives its shape token from a raw 32-bit load at +//! `receiver + 4` and compares it against the site's cached ShapeId +//! (`perry-codegen/src/expr/property_get/generic_dispatch.rs`). Today that +//! compare is fenced by a GC-header load proving `obj_type == GC_TYPE_OBJECT`. +//! The point of the rule is to make that fence removable: if no other cell +//! kind can produce a `+4` word inside `[SHAPE_ID_BASE, SHAPE_ID_END)`, a +//! shape compare that succeeds has already proved the receiver is a shaped +//! ordinary object. +//! +//! Every kind's `+4` word, and why it is (or is not) safe, is enumerated by +//! [`RULE3_KINDS`] below — a table the tests walk, so a new `GC_TYPE_*` +//! cannot be added without classifying it. + +#[cfg(test)] +use crate::gc; + +/// What the 32-bit word at payload `+4` of a cell of this kind holds, and +/// whether a value in the ShapeId range is reachable. +#[cfg(test)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum Rule3Word { + /// This IS the ShapeId word. `GC_TYPE_OBJECT` only. + TheShapeId, + /// Bounded below `SHAPE_ID_BASE` at every allocation site. + BoundedBelowRange, + /// Structurally small (enum, magic constant, pointer high half on a + /// 47-bit address space, a length with its own much smaller cap). + StructurallySmall, + /// Can reach the range, but values of this kind never travel under + /// `POINTER_TAG`, so no read path can load `+4` on one. + NotPointerTagged, + /// Can reach the range and IS pointer-tagged: the GC-kind fence is still + /// required for this kind. Each variant carries the reason. + RangeReachable(&'static str), +} + +/// Every GC kind, its `+4` word, and its rule-3 verdict. +/// +/// Sourced from the struct definitions, not from comments: `ArrayHeader` +/// (`array/header.rs`), `ObjectHeader` (`object/mod.rs`), `StringHeader` +/// (`string/mod.rs`), `ClosureHeader` (`closure/alloc.rs`), `Promise` +/// (`promise/mod.rs`), `BigIntHeader` (`bigint/mod.rs`), `ErrorHeader` +/// (`error.rs`), `MapHeader` (`map.rs`), `LazyArrayHeader` (`json_tape.rs`), +/// `BufferHeader` (`buffer/header.rs`), `TypedArrayHeader` +/// (`typedarray/mod.rs`), `SetHeader` (`set.rs`), the three `native_arena.rs` +/// headers, `NativeHandleHeader` (`native_handle.rs`), `DateCell` (`date.rs`), +/// `TemporalCell` (`temporal/mod.rs`), `ObjectMeta` (`object/mod.rs`), +/// `RegExpHeader` (`regex.rs`), `ProgramCell` (`regex/perex_owner.rs`). +#[cfg(test)] +pub(crate) const RULE3_KINDS: &[(u8, &str, &str, Rule3Word)] = &[ + ( + gc::GC_TYPE_ARRAY, + "ArrayHeader", + "capacity: u32", + Rule3Word::RangeReachable( + "a 2^31-element capacity needs a 16 GiB backing block; growth is \ + old*2 on u32 and is debug-asserted at array/push_pop.rs", + ), + ), + ( + gc::GC_TYPE_OBJECT, + "ObjectHeader", + "parent_class_id / ShapeId", + Rule3Word::TheShapeId, + ), + ( + gc::GC_TYPE_STRING, + "StringHeader", + "byte_len: u32", + // MAX_STRING_LENGTH is 536_870_888 UTF-16 units, so byte_len tops out + // at 1_610_612_664 — below SHAPE_ID_BASE. (A SymbolHeader shares the + // kind and holds `registered: u32`, 0 or 1.) + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_CLOSURE, + "ClosureHeader", + "high half of func_ptr", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_PROMISE, + "Promise", + "explicit zero pad", + Rule3Word::BoundedBelowRange, + ), + ( + gc::GC_TYPE_BIGINT, + "BigIntHeader", + "high half of limbs[0]", + Rule3Word::NotPointerTagged, + ), + ( + gc::GC_TYPE_ERROR, + "ErrorHeader", + "error_kind: u32", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_MAP, + "MapHeader", + "capacity: u32", + Rule3Word::RangeReachable("needs a 32 GiB entry buffer; debug-asserted at map.rs"), + ), + ( + gc::GC_TYPE_LAZY_ARRAY, + "LazyArrayHeader", + "magic = LAZY_ARRAY_MAGIC", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_BUFFER, + "BufferHeader", + "capacity: u32 (bytes)", + Rule3Word::RangeReachable( + "every user-facing constructor caps at i32::MAX; Buffer.from(arrayLike) \ + and Buffer.concat still clamp at u32::MAX", + ), + ), + ( + gc::GC_TYPE_TYPED_ARRAY, + "TypedArrayHeader", + "capacity: u32 (elements)", + Rule3Word::BoundedBelowRange, + ), + ( + gc::GC_TYPE_SET, + "SetHeader", + "capacity: u32", + Rule3Word::RangeReachable("needs a 16 GiB entry buffer; debug-asserted at set.rs"), + ), + ( + gc::GC_TYPE_NATIVE_ARENA_OWNER, + "NativeArenaOwnerHeader", + "high half of byte_length: u64", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_NATIVE_TYPED_VIEW, + "NativeTypedViewHeader", + "capacity: u32 (elements)", + Rule3Word::RangeReachable("debug-asserted at native_arena.rs"), + ), + ( + gc::GC_TYPE_NATIVE_HANDLE, + "NativeHandleHeader", + "high half of magic", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_NATIVE_POD_VIEW, + "NativePodViewHeader", + "high half of owner pointer", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_DATE_CELL, + "DateCell", + "high 32 bits of ts: f64", + Rule3Word::RangeReachable( + "new Date(-1) gives 0xBFF0_0000 — in range, but that is shape index \ + 1_072_693_248, three orders past anything a program mints", + ), + ), + ( + gc::GC_TYPE_TEMPORAL, + "TemporalCell", + "high half of Box", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_OBJECT_META, + "ObjectMeta", + "high 32 of prototype NaN-box", + Rule3Word::NotPointerTagged, + ), + ( + gc::GC_TYPE_REGEXP, + "RegExpHeader", + "high half of pattern_ptr", + Rule3Word::StructurallySmall, + ), + ( + gc::GC_TYPE_REGEX_PROGRAM, + "ProgramCell", + "high half of word_count (< 2^30)", + Rule3Word::NotPointerTagged, + ), +]; + +/// Rule 3, checked where the word is written. +/// +/// Free in release (`debug_assert!`), and in debug it names the kind whose +/// allocation would hand the emitted read path a word it cannot tell from a +/// live ShapeId. Call it beside every store of a `capacity`-shaped u32 into a +/// pointer-tagged cell's `+4`. +#[inline] +pub(crate) fn debug_assert_not_shape_id_word(kind: &'static str, word: u32) { + debug_assert!( + !crate::object::shapes::is_shape_id(word), + "rule 3: {kind} stored {word:#x} at payload +4, which the emitted read \ + path cannot distinguish from a live ShapeId" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::object::shapes::{is_shape_id, SHAPE_ID_BASE, SHAPE_ID_END}; + + /// The table must classify EVERY kind the collector knows about. Adding a + /// `GC_TYPE_*` without a rule-3 verdict is the failure this catches — the + /// new kind would otherwise silently inherit "nobody looked". + #[test] + fn rule3_table_covers_every_gc_kind() { + for kind in 1..=gc::GC_TYPE_MAX { + assert!( + RULE3_KINDS.iter().any(|(k, ..)| *k == kind), + "GC kind {kind} has no rule-3 verdict: what does it store at payload +4, \ + and can that value land in [{SHAPE_ID_BASE:#x}, {SHAPE_ID_END:#x})?" + ); + } + // And nothing stale: every row names a live kind. + for (kind, name, ..) in RULE3_KINDS { + assert!( + (1..=gc::GC_TYPE_MAX).contains(kind), + "{name} names GC kind {kind}, which no longer exists" + ); + } + } + + /// Exactly one kind may legitimately hold a ShapeId at +4. + #[test] + fn rule3_only_ordinary_objects_own_the_shape_word() { + let owners: Vec<&str> = RULE3_KINDS + .iter() + .filter(|(.., verdict)| *verdict == Rule3Word::TheShapeId) + .map(|(_, name, ..)| *name) + .collect(); + assert_eq!(owners, vec!["ObjectHeader"]); + } + + /// The live consequence for the emitted read path: as long as any kind is + /// `RangeReachable`, a shape compare on its own does NOT prove the + /// receiver is an ordinary object, so the GC-kind load cannot be dropped. + /// + /// This test does not demand the list be empty — it pins WHICH kinds keep + /// the fence alive, so shrinking the list is a visible change rather than + /// a silent one. + #[test] + fn rule3_kinds_that_still_require_the_gc_kind_fence() { + let mut still_reachable: Vec<&str> = RULE3_KINDS + .iter() + .filter(|(.., verdict)| matches!(verdict, Rule3Word::RangeReachable(_))) + .map(|(_, name, ..)| *name) + .collect(); + still_reachable.sort_unstable(); + assert_eq!( + still_reachable, + vec![ + "ArrayHeader", + "BufferHeader", + "DateCell", + "MapHeader", + "NativeTypedViewHeader", + "SetHeader", + ], + "the set of kinds whose +4 word can alias a ShapeId changed — if it \ + shrank, the emitted read path's GC-kind load is closer to removable; \ + if it grew, something new can silently alias a live shape" + ); + } + + /// Cells that really exist, checked rather than argued. Each is allocated + /// through its production path and its `+4` word read back. + #[test] + fn rule3_real_cells_do_not_carry_a_shape_id_at_plus_four() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let word_at = |addr: usize| -> u32 { *((addr + 4) as *const u32) }; + + let arr = crate::array::js_array_alloc(8) as usize; + assert!(!is_shape_id(word_at(arr)), "array capacity"); + + let buf = crate::buffer::js_buffer_alloc(64, 0) as usize; + assert!(!is_shape_id(word_at(buf)), "buffer capacity"); + + let map = crate::map::js_map_alloc(8) as usize; + assert!(!is_shape_id(word_at(map)), "map capacity"); + + let set = crate::set::js_set_alloc(8) as usize; + assert!(!is_shape_id(word_at(set)), "set capacity"); + + // A Promise's +4 is struct padding: `arena_alloc_gc` hands back + // reused, non-zeroed memory, so without an explicit zero field a + // Promise born at a dead shaped object's address inherits its + // LIVE ShapeId. + let promise = crate::promise::js_promise_new() as usize; + assert_eq!( + word_at(promise), + 0, + "a Promise's +4 padding must be written, not inherited from \ + whatever cell previously occupied the address" + ); + } + } + + /// The typed-array length cap: `new Int8Array(2**31)` used to be admitted + /// (the check was `u32::MAX`, while its Uint8Array / ArrayBuffer siblings + /// stop at `i32::MAX`), and `capacity = 0x8000_0000` is the FIRST ShapeId + /// the process ever mints. + #[test] + fn rule3_typed_array_length_cap_is_below_the_shape_id_range() { + assert!( + (i32::MAX as u32) < SHAPE_ID_BASE, + "the typed-array element cap must stay below the ShapeId floor" + ); + assert!(!is_shape_id(i32::MAX as u32)); + } +} diff --git a/crates/perry-runtime/src/object/shape_rules_tests.rs b/crates/perry-runtime/src/object/shape_rules_tests.rs new file mode 100644 index 0000000000..61704a990f --- /dev/null +++ b/crates/perry-runtime/src/object/shape_rules_tests.rs @@ -0,0 +1,412 @@ +//! Lane 1 of the single-path object model: the invariants the emitted read +//! path is allowed to depend on, asserted as runtime tests. +//! +//! Three rules, one file, because they are one argument: +//! +//! * **Rule 1** — every descriptor change changes the ShapeId of an ordinary +//! object. A read that has compared the shape word has therefore also proved +//! "no accessor, no non-default attribute" for every key of that shape, and +//! needs no separate `OBJ_FLAG_HAS_DESCRIPTORS` test. +//! * **Rule 2** — ShapeIds are never reused, AND no other producer mints a u32 +//! inside the ShapeId range. The second half is what makes an UNSTAMPED +//! object safe: its `+4` word holds `parent_class_id`, and the read path +//! distinguishes the two by range alone ([`shapes::is_shape_id`]). +//! * **Rule 3** — no pointer-tagged non-object cell holds a value in the +//! ShapeId range at payload `+4`, so a shape compare on a mis-typed receiver +//! fails instead of aliasing a live shape. (Enforced at the allocation sites; +//! the enumerating test lives in [`super::shape_rule3_tests`].) +//! +//! Each test states what it would catch, and asserts a FAILURE mode that the +//! code under test can actually produce — a shape compare that stays equal is +//! not "slow", it is a wrong value silently returned. + +use super::descriptor_state::{ + clear_accessor_descriptor, clear_object_descriptors, clear_property_attrs, + install_fresh_accessor_property, object_has_descriptors, set_accessor_descriptor, + set_builtin_accessor_descriptor, set_builtin_property_attrs, set_property_attrs, + set_property_attrs_batch, transfer_descriptor_owner, AccessorDescriptor, PropertyAttrs, +}; +use super::{js_object_alloc, js_object_set_field_by_name, shapes, ObjectHeader}; + +/// An ordinary, shaped, stamped object carrying `keys` as plain data +/// properties — the receiver every rule-1 assertion is about. +unsafe fn shaped_object(keys: &[&str]) -> *mut ObjectHeader { + let obj = js_object_alloc(0, 0); + for (i, name) in keys.iter().enumerate() { + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + js_object_set_field_by_name(obj, key, i as f64); + } + assert!( + shapes::is_shape_id(shapes::object_shape_stamp(obj)), + "test premise: a plain object with data properties must be stamped" + ); + obj +} + +/// Run `mutate` on a freshly built receiver and assert the ShapeId moved. +/// +/// The `_ne` is the whole point: an install that leaves the shape equal is +/// invisible in program output (the descriptor still works, through the side +/// table) and is exactly the case that makes a shape-only read return the raw +/// slot for what is now an accessor. +fn assert_shape_moves(what: &str, mutate: impl FnOnce(usize)) { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = shaped_object(&["alpha", "beta"]); + let before = shapes::object_shape_stamp(obj); + mutate(obj as usize); + let after = shapes::object_shape_stamp(obj); + assert!( + shapes::is_shape_id(after), + "{what}: the receiver must still carry a real ShapeId, got {after:#x}" + ); + assert_ne!( + before, after, + "{what}: a descriptor change left the ShapeId unchanged — a read that \ + only compares the shape word would serve the pre-descriptor answer" + ); + } +} + +fn accessor() -> AccessorDescriptor { + // Non-zero halves so the #10287 keyed encoding sees a get+set accessor. + // The bits are never dereferenced by any of these paths. + AccessorDescriptor { + get: crate::value::JSValue::pointer(0x1000 as *const u8).bits(), + set: 0, + } +} + +const DEFAULT_ATTRS: PropertyAttrs = PropertyAttrs::new(true, true, true); +const FROZEN_ATTRS: PropertyAttrs = PropertyAttrs::new(false, true, false); + +#[test] +fn rule1_set_property_attrs_transitions() { + assert_shape_moves("set_property_attrs", |addr| { + set_property_attrs(addr, "alpha".to_string(), FROZEN_ATTRS); + }); +} + +#[test] +fn rule1_set_property_attrs_batch_transitions() { + assert_shape_moves("set_property_attrs_batch", |addr| { + set_property_attrs_batch(addr, &[("alpha", FROZEN_ATTRS), ("beta", FROZEN_ATTRS)]); + }); +} + +#[test] +fn rule1_clear_property_attrs_transitions() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = shaped_object(&["alpha"]); + let addr = obj as usize; + set_property_attrs(addr, "alpha".to_string(), FROZEN_ATTRS); + let before = shapes::object_shape_stamp(obj); + clear_property_attrs(addr, "alpha"); + assert_ne!( + before, + shapes::object_shape_stamp(obj), + "clear_property_attrs: removing a descriptor restores writability, \ + so it must retire every cache keyed on the old shape" + ); + } +} + +#[test] +fn rule1_set_accessor_descriptor_transitions() { + assert_shape_moves("set_accessor_descriptor", |addr| { + set_accessor_descriptor(addr, "alpha".to_string(), accessor()); + }); +} + +#[test] +fn rule1_install_fresh_accessor_property_transitions() { + assert_shape_moves("install_fresh_accessor_property", |addr| { + install_fresh_accessor_property(addr, "gamma".to_string(), accessor(), DEFAULT_ATTRS); + }); +} + +#[test] +fn rule1_clear_accessor_descriptor_transitions() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = shaped_object(&["alpha"]); + let addr = obj as usize; + set_accessor_descriptor(addr, "alpha".to_string(), accessor()); + let before = shapes::object_shape_stamp(obj); + clear_accessor_descriptor(addr, "alpha"); + assert_ne!( + before, + shapes::object_shape_stamp(obj), + "clear_accessor_descriptor: the key becomes a plain data slot again" + ); + } +} + +#[test] +fn rule1_set_builtin_accessor_descriptor_transitions() { + assert_shape_moves("set_builtin_accessor_descriptor", |addr| { + set_builtin_accessor_descriptor(addr, "alpha".to_string(), accessor(), DEFAULT_ATTRS); + }); +} + +#[test] +fn rule1_set_builtin_property_attrs_transitions() { + assert_shape_moves("set_builtin_property_attrs", |addr| { + set_builtin_property_attrs(addr, "alpha".to_string(), FROZEN_ATTRS); + }); +} + +/// `clear_object_descriptors` is a BULK removal: after it, every key of the +/// receiver is an ordinary writable/enumerable/configurable data property +/// again. Before this lane it made no shape call at all, so a cache primed +/// while the object was frozen kept serving the frozen answer. +/// +/// Its only production caller hands it a handle-band id (see +/// [`rule1_clear_object_descriptors_on_a_handle_id_is_a_no_op`]), which has no +/// shape — but nothing in the signature says so, and the function is reachable +/// from anywhere in the crate. +#[test] +fn rule1_clear_object_descriptors_transitions() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = shaped_object(&["alpha", "beta"]); + let addr = obj as usize; + set_property_attrs(addr, "alpha".to_string(), FROZEN_ATTRS); + set_accessor_descriptor(addr, "beta".to_string(), accessor()); + let before = shapes::object_shape_stamp(obj); + clear_object_descriptors(addr); + assert_ne!( + before, + shapes::object_shape_stamp(obj), + "clear_object_descriptors: a bulk clear is a descriptor change" + ); + assert!( + super::descriptor_state::get_accessor_descriptor(addr, "beta").is_none(), + "clear_object_descriptors must actually drop the entries it transitions for" + ); + assert!( + super::descriptor_state::get_property_attrs(addr, "alpha").is_none(), + "clear_object_descriptors must actually drop the entries it transitions for" + ); + } +} + +/// The handle path this function exists for: a recycled perry-ffi handle id. +/// It is not a heap cell, so there is nothing to transition — and the clear +/// must still empty the tables. +#[test] +fn rule1_clear_object_descriptors_on_a_handle_id_is_a_no_op() { + let _lock = crate::gc::global_side_table_test_lock(); + // A handle-band id: small integer, no GcHeader, never a shaped object. + let handle: usize = 0x41; + assert!( + crate::value::addr_class::is_handle_band(handle), + "test premise: the id used by handle_expando_clear is handle-band" + ); + set_property_attrs(handle, "hid".to_string(), FROZEN_ATTRS); + assert!(super::descriptor_state::get_property_attrs(handle, "hid").is_some()); + clear_object_descriptors(handle); + assert!( + super::descriptor_state::get_property_attrs(handle, "hid").is_none(), + "a recycled handle id must not inherit the previous tenant's descriptors" + ); +} + +/// `transfer_descriptor_owner` is NOT a descriptor change: it re-keys one +/// logical object's entries after `js_array_grow` replaced its allocation. +/// The identity, the descriptor set and (for a shaped receiver) the ShapeId +/// all stay the same, so making no shape call is correct — but only while the +/// caller really has copied the header. Assert that contract rather than +/// documenting it. +#[test] +fn rule1_transfer_descriptor_owner_preserves_the_shape() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let old = shaped_object(&["alpha"]); + let new = shaped_object(&["alpha"]); + set_property_attrs(old as usize, "alpha".to_string(), FROZEN_ATTRS); + // The two receivers took the same keyed transition (#10287), so they + // share a ShapeId — which is exactly what an address change must not + // disturb. + let old_shape = shapes::object_shape_stamp(old); + set_property_attrs(new as usize, "alpha".to_string(), FROZEN_ATTRS); + assert_eq!( + old_shape, + shapes::object_shape_stamp(new), + "test premise: an identical keyed descriptor install is shared" + ); + transfer_descriptor_owner(old as usize, new as usize); + assert_eq!( + old_shape, + shapes::object_shape_stamp(new), + "transfer is a re-key of the same logical object: the ShapeId is a \ + fact about the object, not about its address" + ); + assert!( + super::descriptor_state::get_property_attrs(new as usize, "alpha").is_some(), + "the moved entry must be readable through the new address" + ); + } +} + +/// The funnel (`note_descriptor_target_keyed`) sets +/// `OBJ_FLAG_HAS_DESCRIPTORS` and transitions the shape for +/// `GC_TYPE_OBJECT` ONLY. A typed array returns early before either; an array +/// or closure fails the `obj_type` test inside. +/// +/// This is load-bearing for the emitted read path in two opposite directions: +/// +/// * the descriptor FLAG is worthless as a guard for these receivers (it is +/// never set), so dropping the flag test loses nothing for them; +/// * the SHAPE is equally worthless (never transitioned), so a shape-only +/// guard must keep rejecting them by KIND. The GC-kind load cannot be +/// removed from the read path for non-`GC_TYPE_OBJECT` receivers until +/// their descriptor state is shape-carried too. +#[test] +fn rule1_funnel_does_not_cover_non_object_receivers() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let arr = crate::array::js_array_alloc(4); + let addr = arr as usize; + let header = crate::value::addr_class::try_read_gc_header(addr) + .expect("a fresh array carries a GcHeader"); + assert_eq!( + header.obj_type, + crate::gc::GC_TYPE_ARRAY, + "test premise: js_array_alloc produces GC_TYPE_ARRAY" + ); + set_property_attrs(addr, "0".to_string(), FROZEN_ATTRS); + assert!( + !object_has_descriptors(addr), + "the funnel only flags GC_TYPE_OBJECT — if this ever starts passing, \ + the read path's flag test became meaningful for arrays and this \ + test's conclusion below must be re-derived" + ); + assert!( + super::descriptor_state::get_property_attrs(addr, "0").is_some(), + "the descriptor is still installed and observable: it is only the \ + shape-and-flag record of it that is missing" + ); + } +} + +/// Rule 2, the half the design doc left as "TO CONFIRM": a class id must +/// never fall inside the ShapeId range. +/// +/// It matters because the read path tells a stamped object from an unstamped +/// one by RANGE alone — [`shapes::object_shape_stamp`] returns +/// `parent_class_id` when `is_shape_id` accepts it, and 0 otherwise. A class +/// id inside the range makes an unstamped object claim a live, unrelated +/// shape: `shape_record_by_id` then answers with someone else's ordered keys +/// and live-inline-slot bound. +#[test] +fn rule2_class_id_producers_stay_out_of_the_shape_id_range() { + use super::class_registry::{SYNTHETIC_CLASS_ID_BASE, SYNTHETIC_CLASS_ID_END}; + // Compile-time: the two ranges are disjoint by construction. + const _: () = assert!( + SYNTHETIC_CLASS_ID_BASE >= shapes::SHAPE_ID_END + || SYNTHETIC_CLASS_ID_END <= shapes::SHAPE_ID_BASE, + "synthetic class ids and ShapeIds must occupy disjoint u32 ranges" + ); + assert!(!shapes::is_shape_id(SYNTHETIC_CLASS_ID_BASE)); + assert!(!shapes::is_shape_id(SYNTHETIC_CLASS_ID_END - 1)); + // Every other class-id producer: codegen ids start at 1 and grow per + // module; the builtin bands are 0x7FFF_FF00..=0x7FFF_FFFF and 0xFFFF_0000+. + for id in [ + 0u32, + 1, + 0x0077_8655, + shapes::SHAPE_ID_BASE - 1, + 0x7FFF_FF30, + 0xFFFF_0000, + 0xFFFF_00D4, + crate::object::NATIVE_MODULE_CLASS_ID, + ] { + assert!( + !shapes::is_shape_id(id), + "class id {id:#x} would be read back as a ShapeId" + ); + } +} + +/// The live allocator, not just the constants: minting a synthetic class id +/// must never produce a value an object's `+4` word would read back as a +/// shape. Sabotage-resistant because it mints from the real counter. +#[test] +fn rule2_minted_synthetic_class_ids_are_never_shape_ids() { + let _lock = crate::gc::global_side_table_test_lock(); + let mut minted = Vec::new(); + for _ in 0..4 { + let id = super::class_registry::test_alloc_synthetic_class_id(); + assert!( + !shapes::is_shape_id(id), + "a freshly minted synthetic class id {id:#x} lands in the ShapeId \ + range: an instance born with it as parent_class_id would alias a \ + live shape" + ); + minted.push(id); + } + minted.dedup(); + assert_eq!(minted.len(), 4, "synthetic class ids must be unique"); +} + +/// Condition (ii) of the #10287 soundness argument, restated without the +/// descriptor flag: an unstamped object can never match a cached shape token. +/// +/// It is a corollary of rule 2 and nothing else — which is why rule 2 is a +/// prerequisite for dropping the flag guard, not an independent nicety. +/// +/// The `+4` word is written with a raw class id by five allocators +/// (`object/alloc.rs:190, 257, 315, 546, 720`) before any stamp lands on it, +/// and `js_object_alloc_class_dynamic_parent` then reads the PREDECESSOR +/// shape back out of that same word (`set_object_keys_array_with_live` -> +/// `object_shape_descriptor`). So the state exercised here is not +/// hypothetical: it is the window every dynamically-parented instance passes +/// through. +#[test] +fn rule2_unstamped_object_never_matches_a_cached_shape() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = js_object_alloc(0, 0); + // Put the object back into the pre-stamp state the allocators leave + // it in, with a real parent class id in the word. + const PARENT: u32 = 0x0051_7A1D; + (*obj).parent_class_id = PARENT; + assert_eq!( + shapes::object_shape_stamp(obj), + 0, + "an unstamped object must report NO shape, so no cached token can match" + ); + assert!( + shapes::object_shape_descriptor(obj).is_none(), + "and must resolve to no shape descriptor at all" + ); + } +} + +/// The producer end of the same invariant, on the live path: a synthetic +/// class id registered as a PARENT is what `js_object_alloc_class_dynamic_parent` +/// writes into `+4` of every instance of a `class X extends someFunction`. +/// While synthetic ids were minted from `0x8000_0000` — the ShapeId floor — +/// that word aliased a live shape, and the alias was not rare: both counters +/// started at the same value, so synthetic id #1 was shape #1. +#[test] +fn rule2_a_synthetic_parent_class_id_is_not_a_shape_token() { + let _lock = crate::gc::global_side_table_test_lock(); + const CHILD: u32 = 0x0051_7A20; + let synthetic = super::class_registry::test_alloc_synthetic_class_id(); + super::class_registry::register_class(CHILD, synthetic); + let parent = super::class_meta_registry::get_parent_class_id(CHILD) + .expect("the registry must hold the edge just registered"); + assert_eq!(parent, synthetic); + assert!( + !shapes::is_shape_id(parent), + "the parent class id {parent:#x} that the dynamic-parent allocator \ + writes to +4 would be read back as a live ShapeId" + ); + assert!( + shapes::shape_record_by_id(parent).is_none(), + "and it must resolve to no shape record" + ); +} diff --git a/crates/perry-runtime/src/promise/mod.rs b/crates/perry-runtime/src/promise/mod.rs index 75fca14189..c05ececa10 100644 --- a/crates/perry-runtime/src/promise/mod.rs +++ b/crates/perry-runtime/src/promise/mod.rs @@ -548,6 +548,14 @@ pub struct Promise { /// paths (`js_promise_resolve` / `js_promise_reject`) and by /// `remove_token_from_registry` for a token dropped without settling. pub(crate) native_pinned: u8, + /// RULE 3 (single-path object model): the six bytes between + /// `native_pinned` and `value` are struct padding, and payload `+4` — the + /// word the emitted read path loads as a ShapeId — falls inside them. + /// Neither `gc_malloc` nor the arena zeroes reused memory and Rust does + /// not guarantee writing implicit padding, so a Promise born at a dead + /// shaped object's address inherited that object's LIVE ShapeId there. + /// Naming the padding makes `ptr::write(p, Promise::new())` zero it. + pub(crate) _shape_word_pad: [u8; 6], /// The resolved value (if fulfilled) pub(crate) value: f64, /// The rejection reason (if rejected) @@ -582,6 +590,7 @@ impl Promise { Promise { state: PromiseState::Pending, native_pinned: 0, + _shape_word_pad: [0; 6], value: 0.0, reason: 0.0, on_fulfilled: ptr::null(), diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 63892278a1..a52b0a346b 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -1203,6 +1203,10 @@ unsafe fn ensure_capacity(set: *mut SetHeader) -> bool { // GC_STORE_AUDIT(INIT): set external buffer pointer moves; live slots are dirtied by caller. (*set).elements = new_elements; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "SetHeader::capacity (grow)", + new_capacity, + ); (*set).capacity = new_capacity; SET_REGISTRY.with(|registry| { let mut registry = registry.borrow_mut(); @@ -1238,6 +1242,7 @@ pub extern "C" fn js_set_alloc(capacity: u32) -> *mut SetHeader { // Initialize header (*ptr).size = 0; + crate::object::shape_rule3::debug_assert_not_shape_id_word("SetHeader::capacity", cap); (*ptr).capacity = cap; // GC_STORE_AUDIT(INIT): set elements buffer is external storage; element stores are barriered separately. (*ptr).elements = elements; diff --git a/crates/perry-runtime/src/typedarray/construct.rs b/crates/perry-runtime/src/typedarray/construct.rs index 7187406b47..7492afcfa6 100644 --- a/crates/perry-runtime/src/typedarray/construct.rs +++ b/crates/perry-runtime/src/typedarray/construct.rs @@ -246,7 +246,10 @@ unsafe fn typed_array_plain_object_values(val: f64) -> Vec { }; // AllocateTypedArrayBuffer implementation limit (Node throws RangeError // for lengths past the max typed-array size). - if len > u32::MAX as f64 { + // RULE 3: see `typed_array_length_or_throw` — the element count lands in + // `TypedArrayHeader::capacity` at payload `+4`, which must stay outside + // the ShapeId range. + if len > i32::MAX as f64 { throw_range_error(format!("Invalid typed array length: {}", len as u64).as_bytes()); } let len = len as u32; diff --git a/crates/perry-runtime/src/typedarray/mod.rs b/crates/perry-runtime/src/typedarray/mod.rs index a687f003f5..c5cea9d70d 100644 --- a/crates/perry-runtime/src/typedarray/mod.rs +++ b/crates/perry-runtime/src/typedarray/mod.rs @@ -851,13 +851,23 @@ pub(crate) fn typed_array_length_or_throw(val: f64) -> u32 { throw_range_error(format!("Invalid typed array length: {shown}").as_bytes()); } // #5067 — Perry stores the element count in a `u32` capacity field, so a - // length above `u32::MAX` cannot be represented (and the backing block - // could never be allocated anyway). Node passes the `<= 2**53-1` length - // check for these and then fails the actual allocation, so match its + // length above the cap cannot be represented (and the backing block could + // never be allocated anyway). Node passes the `<= 2**53-1` length check + // for these and then fails the actual allocation, so match its // `RangeError: Array buffer allocation failed` rather than silently - // saturating the cast to `u32::MAX` (which produced a wrong-size array - // or aborted the process in the allocator). - if integer > u32::MAX as f64 { + // saturating the cast (which produced a wrong-size array or aborted the + // process in the allocator). + // + // RULE 3 (single-path object model): the cap is `i32::MAX`, not + // `u32::MAX`, because `TypedArrayHeader::capacity` sits at payload `+4` — + // the word the emitted read path loads as a ShapeId. `new Int8Array(2**31)` + // wrote `capacity = 0x8000_0000`, which is not merely inside the ShapeId + // range but is the FIRST id the process ever mints. This lowers no + // documented maximum: `new Uint8Array(n)`, `new ArrayBuffer(n)` and + // `Buffer.alloc(n)` already stop at `i32::MAX` + // (`buffer/from.rs`), so this only makes the remaining element types + // agree with their siblings. + if integer > i32::MAX as f64 { throw_range_error(b"Array buffer allocation failed"); } integer as u32 @@ -1027,6 +1037,10 @@ pub fn typed_array_alloc(kind: u8, length: u32) -> *mut TypedArrayHeader { let header = (p as *mut u8).sub(crate::gc::GC_HEADER_SIZE) as *mut crate::gc::GcHeader; (*header).gc_flags |= crate::gc::GC_FLAG_TENURED; (*p).length = length; + crate::object::shape_rule3::debug_assert_not_shape_id_word( + "TypedArrayHeader::capacity", + capacity, + ); (*p).capacity = capacity; (*p).kind = kind; (*p).elem_size = elem_size as u8;