diff --git a/changelog.d/8672-method-name-prototype-guards.md b/changelog.d/8672-method-name-prototype-guards.md new file mode 100644 index 0000000000..ae0bd97cc7 --- /dev/null +++ b/changelog.d/8672-method-name-prototype-guards.md @@ -0,0 +1,9 @@ +--- +category: Performance +title: Restore method-scoped prototype guards +--- + +Prototype mutation now invalidates direct-call guards by method-name slot +instead of permanently disabling every method guard in the process. Hash +collisions remain conservative, and dynamic prototype replacement retains a +global fail-closed escape hatch. diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index f0f113833d..a6bb8193fc 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -692,11 +692,11 @@ fn guarded_pshape_call_site_is_preceded_by_a_shape_id_guard() { } /// The single-pair shape-only arm is small enough to inline at the call site. -/// Pin the complete safety gate: acquire the prototype-mutation latch, accept -/// both the boxed-pointer and internal raw-pointer ABIs, reject addresses -/// outside the target heap range before dereference, reject own descriptors, -/// then compare the exact class/ShapeId pair. The out-of-line guard must be -/// absent from this caller. +/// Pin the complete safety gate: acquire both the all-method escape latch and +/// the FNV-indexed method-name latch, accept both the boxed-pointer and +/// internal raw-pointer ABIs, reject addresses outside the target heap range +/// before dereference, reject own descriptors, then compare the exact +/// class/ShapeId pair. The out-of-line guard must be absent from this caller. #[test] fn single_arm_method_shape_guard_is_inlined_with_the_runtime_contract() { let ir = emit(&guarded_site_module(), false); @@ -707,6 +707,12 @@ fn single_arm_method_shape_guard_is_inlined_with_the_runtime_contract() { ), "the inline guard must acquire the runtime's release-published sticky latch:\n{probe}" ); + assert!( + probe.contains( + "getelementptr i8, ptr @PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD", + ) && probe.matches("load atomic i8").count() >= 2, + "the inline guard must acquire its method-name invalidation byte:\n{probe}" + ); assert!( !probe.contains("call i32 @js_method_direct_shape_guard("), "a monomorphic shape-only site must not retain the out-of-line guard call:\n{probe}" diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index bdea25d439..8cbf5922c4 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -37,15 +37,17 @@ const SHAPE_ID_RANGE_LEN: &str = "1073741824"; // 0x4000_0000 /// The first block proves that the value is a tagged heap pointer or the raw /// object-address form used by internal method ABIs before any dereference. /// The second block reproduces the runtime helper's production contract: the -/// class-prototype invalidation latch is clear, the receiver is a non-forwarded -/// ordinary object without own descriptors, and its exact `(class_id, ShapeId)` -/// pair still matches the compiler-published pair. Any failed proof takes the -/// unchanged dynamic method fallback. +/// all-method escape latch and this method name's invalidation byte are clear, +/// the receiver is a non-forwarded ordinary object without own descriptors, +/// and its exact `(class_id, ShapeId)` pair still matches the +/// compiler-published pair. Any failed proof takes the unchanged dynamic +/// method fallback. fn emit_inline_direct_method_shape_guard( ctx: &mut FnCtx<'_>, recv_box: &str, expected_class_id: &str, expected_shape_id: &str, + method_guard_slot: &str, fast_label: &str, fallback_label: &str, ) { @@ -60,7 +62,15 @@ fn emit_inline_direct_method_shape_guard( let blk = ctx.block(); let invalidated = blk.load_atomic_acquire(I8, "@PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED", 1); - let prototype_ok = blk.icmp_eq(I8, &invalidated, "0"); + let all_methods_ok = blk.icmp_eq(I8, &invalidated, "0"); + let method_slot_ptr = blk.gep( + I8, + "@PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD", + &[(I64, method_guard_slot)], + ); + let method_invalidated = blk.load_atomic_acquire(I8, &method_slot_ptr, 1); + let method_ok = blk.icmp_eq(I8, &method_invalidated, "0"); + let prototype_ok = blk.and(I1, &all_methods_ok, &method_ok); let recv_bits = blk.bitcast_double_to_i64(recv_box); let recv_handle = blk.and(I64, &recv_bits, crate::nanbox::POINTER_MASK_I64); let tag = blk.lshr(I64, &recv_bits, "48"); @@ -395,6 +405,7 @@ pub(super) fn emit_guarded_direct_method_call( let entry = ctx.strings.entry(key_idx); let bytes_global = format!("@{}", entry.bytes_global); let name_len_str = entry.byte_len.to_string(); + let method_guard_slot_str = (entry.dispatch_hash & 0xffff).to_string(); let dispatch_global = ctx.strings.static_dispatch_global(key_idx); let site_id = if shape_only_guard { None @@ -452,7 +463,11 @@ pub(super) fn emit_guarded_direct_method_call( let cid = ctx.block().call( I32, "js_method_direct_shape_class", - &[(DOUBLE, recv_box), (crate::types::PTR, &shape_slot)], + &[ + (DOUBLE, recv_box), + (crate::types::PTR, &shape_slot), + (I32, &method_guard_slot_str), + ], ); let shape_id = ctx.block().load(I32, &shape_slot); { @@ -486,6 +501,7 @@ pub(super) fn emit_guarded_direct_method_call( recv_box, &expected_class_id_str, &expected_shape_id, + &method_guard_slot_str, &fast_label, &fallback_label, ); @@ -504,6 +520,7 @@ pub(super) fn emit_guarded_direct_method_call( (DOUBLE, recv_box), (I32, &expected_class_id_str), (I32, &expected_shape_id), + (I32, &method_guard_slot_str), ], ) } else { diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index b7f903579b..6f0d7a6e40 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -498,6 +498,7 @@ pub(crate) fn try_lower_instance_method_call( let probe_entry = ctx.strings.entry(key_idx_probe); let probe_bytes_global = format!("@{}", probe_entry.bytes_global); let probe_name_len_str = probe_entry.byte_len.to_string(); + let method_guard_slot_str = (probe_entry.dispatch_hash & 0xffff).to_string(); let probe_override_idx = ctx.new_block("idisp.override"); let probe_dispatch_idx = ctx.new_block("idisp.dispatch"); let probe_outer_merge_idx = ctx.new_block("idisp.outer_merge"); @@ -536,7 +537,11 @@ pub(crate) fn try_lower_instance_method_call( let cid = ctx.block().call( I32, "js_method_direct_shape_class", - &[(DOUBLE, &recv_box), (crate::types::PTR, &shape_slot)], + &[ + (DOUBLE, &recv_box), + (crate::types::PTR, &shape_slot), + (I32, &method_guard_slot_str), + ], ); let shape_id = ctx.block().load(I32, &shape_slot); shape_probe_cid = Some(cid.clone()); diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index 0faf427979..707e24780d 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -33,6 +33,12 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { // Direct-method lowering reads it with acquire ordering before touching a // receiver header; prototype mutation stores 1 with release ordering. module.add_external_global("PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED", I8); + // Per-method sticky invalidation table indexed by low FNV-1a bits. A + // collision is conservative: it only disables another direct guard. + module.add_external_global( + "PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD", + "[65536 x i8]", + ); // #7834/#7873: process-global count of threads with per-object records. // `0` proves both per-object side tables are empty everywhere, so a // construction site can skip `js_gc_forget_object_layout` outright. @@ -214,8 +220,12 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { I32, &[I64, DOUBLE, I32, I32, PTR, I64, PTR], ); - module.declare_function("js_method_direct_shape_guard", I32, &[DOUBLE, I32, I32]); - module.declare_function("js_method_direct_shape_class", I32, &[DOUBLE, PTR]); + module.declare_function( + "js_method_direct_shape_guard", + I32, + &[DOUBLE, I32, I32, I32], + ); + module.declare_function("js_method_direct_shape_class", I32, &[DOUBLE, PTR, I32]); module.declare_function( "js_typed_feedback_closure_direct_call_guard", I32, diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 07f843298d..f318f14910 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -65,9 +65,9 @@ pub(crate) use state::{ class_parent_closure, class_parent_closure_root_store, class_prototype_method_is_enumerable, class_prototype_method_set_enumerable, class_prototype_method_value_cache_root_store, class_prototype_object_root_store, class_static_defined_attrs, class_static_set_defined_attrs, - global_object_prototype_bits, is_bound_native_constructor_closure_value, - is_non_constructable_builtin_function_value, parent_closure_in_chain, - throw_non_constructable_builtin_function, + class_unmark_key_deleted, global_object_prototype_bits, + is_bound_native_constructor_closure_value, is_non_constructable_builtin_function_value, + parent_closure_in_chain, throw_non_constructable_builtin_function, }; pub use state::{ ClassVTable, VTableMethodEntry, CLASS_DECL_PROTOTYPE_OBJECTS, CLASS_DYNAMIC_PARENT_VALUE, @@ -104,12 +104,15 @@ pub(crate) use class_meta::{ CLASS_ID_TEXT_ENCODER_STREAM, }; #[cfg(test)] -pub(crate) use prototype_methods::CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED; +pub(crate) use prototype_methods::{ + class_prototype_fast_guards_invalidated, class_prototype_method_guard_slot, + CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED, CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD, +}; // ── prototype_methods.rs ──────────────────────────────────────────────────── pub(crate) use prototype_methods::{ - class_prototype_fast_guards_invalidated, class_prototype_method_root_store, - invalidate_class_prototype_fast_guards, mirror_prototype_method_on_object, + class_prototype_fast_guard_invalidated_for_method, class_prototype_method_root_store, + invalidate_class_prototype_fast_guards_for_method, mirror_prototype_method_on_object, synthetic_class_id_for_function, }; pub use prototype_methods::{ @@ -120,9 +123,9 @@ pub use prototype_methods::{ // ── construct.rs / vm_brand.rs ────────────────────────────────────────────── pub(crate) use construct::{ extends_target_must_throw, is_callable_function_value, js_value_is_constructor, - lookup_prototype_method, nm_ctor_child_process, nm_ctor_cluster, nm_ctor_fs, nm_ctor_readline, - nm_ctor_repl, nm_ctor_stream, nm_ctor_tls, nm_ctor_tty, nm_ctor_vm, nm_ctor_wasi, - promise_parent_in_chain, + lookup_own_prototype_method, lookup_prototype_method, nm_ctor_child_process, nm_ctor_cluster, + nm_ctor_fs, nm_ctor_readline, nm_ctor_repl, nm_ctor_stream, nm_ctor_tls, nm_ctor_tty, + nm_ctor_vm, nm_ctor_wasi, promise_parent_in_chain, }; pub use construct::{ js_ctor_return_override, js_new_function_construct, js_new_function_construct_apply, diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index d7a5940205..f66cd9dc43 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -1862,6 +1862,17 @@ pub(super) fn is_arrow_function_value(value: f64) -> bool { /// `(class_id, name)`, or None if no assignment matched. Walks the /// parent-class chain so methods registered on a base class are found /// via subclass instances. +pub(crate) fn lookup_own_prototype_method(class_id: u32, name: &str) -> Option { + if class_is_key_deleted(class_id, name) { + return None; + } + CLASS_PROTOTYPE_METHODS.with(|table| { + let guard = table.read().ok()?; + let bits = guard.as_ref()?.get(&class_id)?.get(name)?; + Some(f64::from_bits(*bits)) + }) +} + pub(crate) fn lookup_prototype_method(class_id: u32, name: &str) -> Option { CLASS_PROTOTYPE_METHODS.with(|table| { let guard = table.read().ok()?; @@ -1869,9 +1880,11 @@ pub(crate) fn lookup_prototype_method(class_id: u32, name: &str) -> Option let mut cid = class_id; let mut depth = 0usize; while depth < 32 { - if let Some(per_class) = map.get(&cid) { - if let Some(&bits) = per_class.get(name) { - return Some(f64::from_bits(bits)); + if !class_is_key_deleted(cid, name) { + if let Some(per_class) = map.get(&cid) { + if let Some(&bits) = per_class.get(name) { + return Some(f64::from_bits(bits)); + } } } match crate::object::class_generic_origin(cid).or_else(|| get_parent_class_id(cid)) { 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 9cc418b72b..f514a8bf27 100644 --- a/crates/perry-runtime/src/object/class_registry/gc_roots.rs +++ b/crates/perry-runtime/src/object/class_registry/gc_roots.rs @@ -622,6 +622,10 @@ pub(crate) fn test_clear_class_side_table_roots() { } }); CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED.store(false, std::sync::atomic::Ordering::Release); + CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD + .write() + .unwrap() + .clear(); FUNCTION_CLASS_IDS.with(|table| { if let Ok(mut guard) = table.write() { *guard = None; 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 d09c74d746..919bbca739 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs @@ -100,19 +100,36 @@ crate::perry_thread_local! { RwLock::new(None); } -// Production codegen reads this byte directly before entering a guarded -// direct-method arm. Keep the test build per-thread so one mutation test cannot -// poison unrelated tests running in parallel; generated programs link the -// non-test symbol below. +// Production codegen reads this fail-closed all-method byte and the scoped +// table below before entering a guarded direct-method arm. Keep the test state +// per-thread so one mutation test cannot poison unrelated tests running in +// parallel; generated programs link the non-test symbols below. #[cfg(not(test))] #[no_mangle] pub static PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED: std::sync::atomic::AtomicU8 = std::sync::atomic::AtomicU8::new(0); +/// Sticky per-method invalidation bytes for compiler-emitted direct-method +/// guards. Prototype writes always have a property name, so they only need to +/// retire guards for that name. The low 16 bits of the name's FNV-1a hash +/// select a byte; collisions conservatively retire additional names. +pub(crate) const CLASS_PROTOTYPE_METHOD_GUARD_SLOT_COUNT: usize = 1 << 16; +pub(crate) const CLASS_PROTOTYPE_METHOD_GUARD_SLOT_MASK: u64 = + (CLASS_PROTOTYPE_METHOD_GUARD_SLOT_COUNT - 1) as u64; + +#[cfg(not(test))] +#[no_mangle] +pub static PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD: [std::sync::atomic::AtomicU8; + CLASS_PROTOTYPE_METHOD_GUARD_SLOT_COUNT] = + [const { std::sync::atomic::AtomicU8::new(0) }; CLASS_PROTOTYPE_METHOD_GUARD_SLOT_COUNT]; + #[cfg(test)] per_test_global! { pub(crate) static CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + pub(crate) static CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD: + std::sync::RwLock> = + std::sync::RwLock::new(std::collections::HashSet::new()); } pub(crate) fn class_prototype_fast_guards_invalidated() -> bool { @@ -127,30 +144,68 @@ pub(crate) fn class_prototype_fast_guards_invalidated() -> bool { } } +#[inline] +pub(crate) fn class_prototype_method_guard_slot(name: &str) -> u32 { + (super::super::key_bytes_hash(name.as_ptr(), name.len()) + & CLASS_PROTOTYPE_METHOD_GUARD_SLOT_MASK) as u32 +} + +#[inline] +pub(crate) fn class_prototype_fast_guard_invalidated_for_method(slot: u32) -> bool { + if class_prototype_fast_guards_invalidated() { + return true; + } + let slot = (slot as usize) & (CLASS_PROTOTYPE_METHOD_GUARD_SLOT_COUNT - 1); + #[cfg(not(test))] + { + PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD[slot] + .load(std::sync::atomic::Ordering::Acquire) + != 0 + } + #[cfg(test)] + { + CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD + .read() + .unwrap() + .contains(&(slot as u16)) + } +} + +#[inline] +fn retire_prototype_dependent_caches() { + // #7480: prototype surgery retires element-shape proofs. + crate::array::invalidate_all_element_shapes(); + // #7769: method-dispatch caches are keyed by VTABLE_GEN. + VTABLE_GEN.fetch_add(1, std::sync::atomic::Ordering::Release); +} + +pub(crate) fn invalidate_class_prototype_fast_guards_for_method(name: &str) { + let slot = class_prototype_method_guard_slot(name) as usize; + #[cfg(not(test))] + PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD[slot] + .store(1, std::sync::atomic::Ordering::Release); + #[cfg(test)] + CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD + .write() + .unwrap() + .insert(slot as u16); + retire_prototype_dependent_caches(); +} + +#[allow(dead_code)] // Fail-closed escape hatch for a future keyless mutation path. pub(crate) fn invalidate_class_prototype_fast_guards() { #[cfg(not(test))] PERRY_CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED.store(1, std::sync::atomic::Ordering::Release); #[cfg(test)] CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED.store(true, std::sync::atomic::Ordering::Release); - // #7480: prototype surgery is the one event that retires an element-shape - // proof without touching any array — the class's shape stopped being a - // reliable description of its instances. This is the existing single - // latch all three prototype-write entry points funnel through - // (`js_register_prototype_method`, `class_prototype_method_root_store`, - // and the class-registry state path), so one generation bump here retires - // every outstanding record at O(1). - crate::array::invalidate_all_element_shapes(); - // #7769: prototype surgery can change which member a `recv.m()` resolves - // to, and the method-dispatch caches (`vtable_ic`, `obj_dispatch_ic`) key - // their entries on `VTABLE_GEN`. Those caches were only retired by class - // REGISTRATION, so a `Class.prototype.m = fn` after first dispatch left - // them serving the pre-surgery answer. Retire them here, at the one latch - // all three prototype-write entry points funnel through — the same O(1) - // argument as the element-shape invalidation above. - VTABLE_GEN.fetch_add(1, std::sync::atomic::Ordering::Release); + // Unknown-key prototype surgery cannot use a scoped slot. Retire every + // direct-method guard, then perform the common cache invalidations. + retire_prototype_dependent_caches(); } pub(crate) fn class_prototype_method_root_store(class_id: u32, name: String, value_bits: u64) { + // Assignment after `delete C.prototype.m` creates the property again. + class_unmark_key_deleted(class_id, &name); CLASS_PROTOTYPE_METHODS.with(|table| { let mut guard = table.write().unwrap(); if guard.is_none() { @@ -163,7 +218,7 @@ pub(crate) fn class_prototype_method_root_store(class_id: u32, name: String, val .or_default() .insert(name.clone(), value_bits); }); - invalidate_class_prototype_fast_guards(); + invalidate_class_prototype_fast_guards_for_method(&name); crate::gc::runtime_write_barrier_root_nanbox(value_bits); // #5024: the side table makes the method dispatchable, but own-key // enumeration on the prototype OBJECT (Object.keys / getOwnPropertyNames / @@ -250,7 +305,6 @@ pub unsafe extern "C" fn js_register_prototype_method( name_len: usize, value: f64, ) { - invalidate_class_prototype_fast_guards(); if class_id == 0 || name_ptr.is_null() || name_len == 0 { return; } diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index bebfa9038b..7eedac7a0d 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -43,6 +43,14 @@ pub(crate) fn class_is_key_deleted(class_id: u32, key: &str) -> bool { }) } +pub(crate) fn class_unmark_key_deleted(class_id: u32, key: &str) { + CLASS_DELETED_KEYS.with(|m| { + if let Some(keys) = m.borrow_mut().get_mut(&class_id) { + keys.remove(key); + } + }); +} + /// Record `C. = value` in the class-ref side table that dynamic reads /// (`const K: any = C; K.name`, `Object.keys(C)`, `getOwnPropertyDescriptor`) /// consult, and shade the stored value for the incremental marker. @@ -648,18 +656,9 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { return f64::from_bits(crate::value::TAG_UNDEFINED); } // #7769 follow-up: materializing a declared class's prototype object is - // not prototype SURGERY, and it used to invalidate the fast guards as if - // it were. - // - // `invalidate_class_prototype_fast_guards()` trips a process-global, - // MONOTONIC latch. It disables every `js_method_direct_shape_guard` / - // `js_typed_feedback_method_direct_call_guard` in the program, retires - // every outstanding element-shape record (`invalidate_all_element_shapes`) - // and bumps `VTABLE_GEN`, retiring the `vtable_ic` / `obj_dispatch_ic` - // dispatch caches. It exists for the one event that can change which - // member `recv.m()` resolves to: a WRITE to a prototype - // (`Class.prototype.m = fn`), which is what the two call sites in - // `prototype_methods.rs` cover. + // not prototype surgery. A real keyed prototype write invalidates only + // the matching method-name guard slot, retires element-shape records, and + // bumps `VTABLE_GEN` so generic dispatch observes the replacement. // // Reaching this line changes none of that. The object being created is // fresh and unobserved; the writes immediately below install diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 4b7015e949..0efd8fc607 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -237,6 +237,12 @@ pub extern "C" fn js_object_delete_field( return 0; } } + // Deleting an accessor from a class/Object prototype changes + // method resolution for this key just like installing it. + super::descriptor_state::disable_inline_guards_for_descriptor_target( + obj as usize, + name, + ); super::clear_accessor_descriptor(obj as usize, name); super::clear_property_attrs(obj as usize, name); // defineProperty may ALSO have planted a keys_array @@ -308,6 +314,12 @@ pub extern "C" fn js_object_delete_field( return 0; } } + // A configurable data method on a class/Object prototype is about + // to disappear. Retire only this name's direct-method guards. + super::descriptor_state::disable_inline_guards_for_descriptor_target( + obj as usize, + name, + ); } // Proper delete: shift remaining keys + values down by one, then diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 53b5c1d234..462f08099f 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -200,11 +200,11 @@ pub(crate) fn disable_inline_guards_for_descriptor_target(obj: usize, key: &str) || class_registry::is_registered_class_prototype_object(obj) || class_registry::class_id_for_decl_prototype_object(obj).is_some(); if is_prototype_target { - // Direct method guards are not key-aware. Conservatively retire them - // after any user descriptor/accessor install on a prototype that can - // affect a class instance. Own-instance installs are rejected by the - // receiver's `OBJ_FLAG_HAS_DESCRIPTORS` header bit instead. - class_registry::invalidate_class_prototype_fast_guards(); + // A prototype descriptor can only change resolution for this key. + // Retire the matching method-name guard slot across all classes; + // own-instance installs are still rejected by the receiver's + // `OBJ_FLAG_HAS_DESCRIPTORS` header bit. + class_registry::invalidate_class_prototype_fast_guards_for_method(key); let hash = super::key_bytes_hash(key.as_ptr(), key.len()); note_proto_descriptor_key_hash(hash); if declared_field_name_hash_exists(hash) { @@ -1187,8 +1187,18 @@ mod c5a_tests { the inline class-field fast path" ); assert!( - class_registry::class_prototype_fast_guards_invalidated(), - "a prototype descriptor must retire unkeyed direct-method guards" + !class_registry::class_prototype_fast_guards_invalidated(), + "a keyed prototype descriptor must not retire every method guard" + ); + let render_slot = class_registry::class_prototype_method_guard_slot("c5a_render_method"); + assert!( + class_registry::class_prototype_fast_guard_invalidated_for_method(render_slot), + "a prototype descriptor must retire its matching method guard" + ); + let other_slot = class_registry::class_prototype_method_guard_slot("c5a_other_method"); + assert!( + !class_registry::class_prototype_fast_guard_invalidated_for_method(other_slot), + "an unrelated method guard must remain valid" ); // Field-style install: key declared by a registered class. diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 8848bb91da..7d3eddd4dc 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -2059,7 +2059,7 @@ pub unsafe extern "C-unwind" fn js_native_call_method( // Vtable lookup: check if this class has a registered method in the vtable let class_id = (*obj).class_id; - if class_id != 0 { + if class_id != 0 && !class_is_key_deleted(class_id, method_name) { if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() { if let Some(ref reg) = *registry { if let Some(vtable) = reg.get(&class_id) { diff --git a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs index d4c150ae9c..aec8c9e67f 100644 --- a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs @@ -1017,45 +1017,65 @@ pub(super) unsafe fn dispatch_handle( let mut cur_cid = class_id; let mut depth = 0u32; while depth < 32 { - if let Some(vtable) = reg.get(&cur_cid) { - if let Some(entry) = vtable.methods.get(method_name) { - vtable_ic_insert( - class_id, - method_name_ptr as usize, - entry.func_ptr, - entry.param_count, - entry.has_synthetic_arguments, - entry.has_rest, - ); - // #7769: this walk — not the tail vtable - // arm of `js_native_call_method` — is where - // an INHERITED method resolves, and - // inherited methods are the common case in - // any real hierarchy (`class Square extends - // Rect` calling `Rect`'s `area`). Recording - // the outcome here is what lets the - // top-of-tower fast path serve them; the - // helper re-checks the receiver-shape - // predicate before storing anything. - super::note_class_vtable_resolution( - f64::from_bits(jsval.bits()), - method_name, - entry.func_ptr, - entry.param_count, - entry.has_synthetic_arguments, - entry.has_rest, - ); - resolved_method = Some(ResolvedMethod::Vtable { - func_ptr: entry.func_ptr, - param_count: entry.param_count, - has_synthetic_arguments: entry.has_synthetic_arguments, - has_rest: entry.has_rest, - this_i64: jsval.as_pointer::() as i64, + let deleted = class_is_key_deleted(cur_cid, method_name); + // `C.prototype.m = fn` replaces a declared `m` on + // this exact prototype object, so the assignment + // side table must win before the original vtable. + if !deleted { + if let Some(method_value) = + lookup_own_prototype_method(cur_cid, method_name) + { + resolved_method = Some(ResolvedMethod::ProtoClosure { + field_bits: method_value.to_bits(), }); break; } } - let proto_obj = class_prototype_object(cur_cid); + if !deleted { + if let Some(vtable) = reg.get(&cur_cid) { + if let Some(entry) = vtable.methods.get(method_name) { + vtable_ic_insert( + class_id, + method_name_ptr as usize, + entry.func_ptr, + entry.param_count, + entry.has_synthetic_arguments, + entry.has_rest, + ); + // #7769: this walk — not the tail vtable + // arm of `js_native_call_method` — is where + // an INHERITED method resolves, and + // inherited methods are the common case in + // any real hierarchy (`class Square extends + // Rect` calling `Rect`'s `area`). Recording + // the outcome here is what lets the + // top-of-tower fast path serve them; the + // helper re-checks the receiver-shape + // predicate before storing anything. + super::note_class_vtable_resolution( + f64::from_bits(jsval.bits()), + method_name, + entry.func_ptr, + entry.param_count, + entry.has_synthetic_arguments, + entry.has_rest, + ); + resolved_method = Some(ResolvedMethod::Vtable { + func_ptr: entry.func_ptr, + param_count: entry.param_count, + has_synthetic_arguments: entry.has_synthetic_arguments, + has_rest: entry.has_rest, + this_i64: jsval.as_pointer::() as i64, + }); + break; + } + } + } + let proto_obj = if deleted { + std::ptr::null_mut() + } else { + class_prototype_object(cur_cid) + }; if !proto_obj.is_null() { let method_key = crate::string::js_string_from_bytes( method_name.as_ptr(), diff --git a/crates/perry-runtime/src/typed_feedback/guards.rs b/crates/perry-runtime/src/typed_feedback/guards.rs index c9045638ff..e5e459f858 100644 --- a/crates/perry-runtime/src/typed_feedback/guards.rs +++ b/crates/perry-runtime/src/typed_feedback/guards.rs @@ -1036,14 +1036,15 @@ pub unsafe extern "C" fn js_typed_feedback_method_direct_call_guard( /// /// Descriptor invalidation is deliberately scoped rather than process-wide: /// an own descriptor sets `OBJ_FLAG_HAS_DESCRIPTORS` on this receiver, while a -/// user descriptor on a registered class/Object prototype flips the same -/// sticky prototype latch checked below. A descriptor on an unrelated object -/// can affect neither method resolution nor this exact ShapeId proof and must -/// not poison every direct-method site in the process. +/// user descriptor on a registered class/Object prototype flips the matching +/// method-name slot checked below. A descriptor on an unrelated object or for +/// an unrelated key can affect neither this method's resolution nor this exact +/// ShapeId proof and must not poison every direct-method site in the process. #[no_mangle] pub unsafe extern "C" fn js_method_direct_shape_class( receiver: f64, out_shape_id: *mut u32, + method_guard_slot: u32, ) -> u32 { if !out_shape_id.is_null() { *out_shape_id = 0; @@ -1058,7 +1059,7 @@ pub unsafe extern "C" fn js_method_direct_shape_class( if (*gc_header).obj_type != crate::gc::GC_TYPE_OBJECT || (*gc_header).gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 || (*gc_header)._reserved & crate::gc::OBJ_FLAG_HAS_DESCRIPTORS != 0 - || crate::object::class_prototype_fast_guards_invalidated() + || crate::object::class_prototype_fast_guard_invalidated_for_method(method_guard_slot) { return 0; } @@ -1089,12 +1090,13 @@ pub unsafe extern "C" fn js_method_direct_shape_guard( receiver: f64, expected_class_id: u32, expected_shape_id: u32, + method_guard_slot: u32, ) -> i32 { if expected_class_id == 0 || !crate::object::shapes::is_shape_id(expected_shape_id) { return 0; } let mut shape_id = 0; - let class_id = js_method_direct_shape_class(receiver, &mut shape_id); + let class_id = js_method_direct_shape_class(receiver, &mut shape_id, method_guard_slot); (class_id == expected_class_id && shape_id == expected_shape_id) as i32 } @@ -1185,7 +1187,7 @@ mod keep_guard_symbols { #[cfg(feature = "keepalive-anchors")] #[used] static G3: extern "C" fn(u64, f64, *const u8, u32, u32) -> i32 = js_typed_feedback_closure_direct_call_guard; #[cfg(feature = "keepalive-anchors")] - #[used] static G4: unsafe extern "C" fn(f64, u32, u32) -> i32 = js_method_direct_shape_guard; + #[used] static G4: unsafe extern "C" fn(f64, u32, u32, u32) -> i32 = js_method_direct_shape_guard; #[cfg(feature = "keepalive-anchors")] - #[used] static G4B: unsafe extern "C" fn(f64, *mut u32) -> u32 = js_method_direct_shape_class; + #[used] static G4B: unsafe extern "C" fn(f64, *mut u32, u32) -> u32 = js_method_direct_shape_class; } diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index 75feab5563..61a6a39c25 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -1277,7 +1277,7 @@ fn representation_lowering_helpers_have_lto_keepalive_anchors() { ( guards, "static G4", - "static G4: unsafe extern \"C\" fn(f64, u32, u32) -> i32", + "static G4: unsafe extern \"C\" fn(f64, u32, u32, u32) -> i32", "js_method_direct_shape_guard", ), ( @@ -1981,10 +1981,17 @@ fn method_direct_shape_guard_requires_the_exact_compiler_pair() { let class_id = 0x7EED_1061; let (obj, _, _, receiver) = class_instance(class_id, b"x"); let expected_shape_id = shape_id(obj); + let method_name = "direct_shape_target_1061"; + let method_slot = crate::object::class_prototype_method_guard_slot(method_name); assert_eq!( unsafe { - super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id) + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ) }, 1 ); @@ -1994,6 +2001,7 @@ fn method_direct_shape_guard_requires_the_exact_compiler_pair() { receiver, class_id.wrapping_add(1), expected_shape_id, + method_slot, ) }, 0 @@ -2011,7 +2019,12 @@ fn method_direct_shape_guard_requires_the_exact_compiler_pair() { ); assert_eq!( unsafe { - super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id) + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ) }, 1 ); @@ -2023,7 +2036,12 @@ fn method_direct_shape_guard_requires_the_exact_compiler_pair() { let original_reserved = (*gc)._reserved; (*gc)._reserved |= crate::gc::OBJ_FLAG_HAS_DESCRIPTORS; assert_eq!( - super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id), + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ), 0 ); (*gc)._reserved = original_reserved; @@ -2037,13 +2055,54 @@ fn method_direct_shape_guard_requires_the_exact_compiler_pair() { } assert_eq!( unsafe { - super::guards::js_method_direct_shape_guard(receiver, class_id, expected_shape_id) + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ) }, 0 ); unsafe { (*obj).parent_class_id = expected_shape_id; } + + crate::object::class_prototype_method_root_store( + class_id.wrapping_add(10), + "direct_shape_unrelated_1061".to_string(), + crate::value::TAG_UNDEFINED, + ); + assert_eq!( + unsafe { + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ) + }, + 1, + "a different method name must not poison this direct guard", + ); + + crate::object::class_prototype_method_root_store( + class_id.wrapping_add(11), + method_name.to_string(), + crate::value::TAG_UNDEFINED, + ); + assert_eq!( + unsafe { + super::guards::js_method_direct_shape_guard( + receiver, + class_id, + expected_shape_id, + method_slot, + ) + }, + 0, + "the same method name must retire guards across the class hierarchy", + ); } #[test] diff --git a/test-files/test_method_guard_name_invalidation.ts b/test-files/test_method_guard_name_invalidation.ts new file mode 100644 index 0000000000..93564f9b4c --- /dev/null +++ b/test-files/test_method_guard_name_invalidation.ts @@ -0,0 +1,67 @@ +// Direct-method guards are invalidated by method name. Unrelated prototype +// writes must leave a hot guard usable, while writes for the guarded name must +// fall back and observe the replacement across an inheritance chain. + +class MethodGuardBase { + value: number; + + constructor(value: number) { + this.value = value; + } + + hot(): string { + return "base:" + this.value; + } +} + +class MethodGuardChild extends MethodGuardBase {} + +class MethodGuardOther { + cold(): string { + return "cold"; + } +} + +function callHot(receiver: MethodGuardBase): string { + return receiver.hot(); +} + +const receiver: MethodGuardBase = new MethodGuardChild(7); +console.log(callHot(receiver)); + +class MethodGuardDelete { + gone(): string { + return "present"; + } +} + +function callGone(receiver: MethodGuardDelete): string { + return receiver.gone(); +} + +const deleted = new MethodGuardDelete(); +console.log(callGone(deleted)); +delete (MethodGuardDelete.prototype as any).gone; +try { + console.log(callGone(deleted)); +} catch (_error) { + console.log("deleted"); +} + +(MethodGuardOther.prototype as any).cold = function (): string { + return "patched-cold"; +}; +console.log(callHot(receiver)); + +// A same-name write on any class conservatively retires the hash slot. It +// must not change this receiver's answer, but subsequent direct guards may no +// longer assume that `hot` is untouched. +(MethodGuardOther.prototype as any).hot = function (): string { + return "other-hot"; +}; +console.log(callHot(receiver)); + +(MethodGuardBase.prototype as any).hot = function (this: MethodGuardBase): string { + return "patched:" + this.value; +}; +console.log(callHot(receiver));