diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3cd21240da..30bf439b9a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -586,6 +586,24 @@ jobs: python3 scripts/check_node_version_consistency.py --self-test python3 scripts/check_node_version_consistency.py + # perry-codegen does NOT depend on perry-runtime, yet it bakes the + # collector's header layout into emitted code: the inline `new` path + # stores a packed GcHeader word as a compile-time constant (#8122) and + # every class-field / element-shape guard masks that word against a + # literal. Both sides carried their own copy of GC_TYPE_OBJECT, + # GC_FLAG_FORWARDED, GC_OBJ_TYPED_LAYOUT_INTACT and friends, held together + # only by a comment -- the `debug_assert_eq!` that looks like it checks + # them compares codegen's constant to a string literal (a tautology) and + # is compiled out of release and perry-dev anyway. A renumbering in the + # runtime therefore compiled clean, passed every suite, and shipped a + # binary whose objects the collector misreads. Build-free, so it belongs + # in `lint`, which IS a required context. + - name: GC header constant consistency + if: ${{ !cancelled() }} + run: | + python3 scripts/check_gc_header_constants.py --self-test + python3 scripts/check_gc_header_constants.py + # #7341 layer 3. A RuntimeHandleScope gives an object liveness; it does # nothing for a raw pointer already read out of the slot. Every rooting bug # in the quarantine sweep had rooting ALREADY -- what was missing was diff --git a/changelog.d/10646-retire-class-field-latch.md b/changelog.d/10646-retire-class-field-latch.md new file mode 100644 index 0000000000..f93d0625dc --- /dev/null +++ b/changelog.d/10646-retire-class-field-latch.md @@ -0,0 +1,59 @@ +### Static-key class-field reads drop the per-access latch (−18% on the guard's fast path) + +Every static-key class-field read gated its fast path on +`@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED`. That is an `external global`, so on +arm64 reading it cost `adrp` + a GOT `ldr` + a dependent `ldrb` through it + a +compare — four instructions and two dependent loads, before the guard had +looked at the receiver at all. It could not be hoisted: the runtime flips it +mid-execution when a descriptor or accessor lands on a class prototype, so the +load was `volatile` by necessity. + +That authority now rides on a value the guard already had to load. Each class +gains a `@perry_class_guard_shape_*` expectation, seeded at module init with +the class ShapeId and registered with the runtime; +`disable_class_field_inline_guard` poisons every registered slot with +`u32::MAX`. ShapeIds are allocated from `[0x8000_0000, 0xC000_0000)` and never +reused, so a poisoned expectation can never match a live object — every guard +misses and routes to the IC, exactly what the latch bought. + +It is deliberately a SEPARATE global from `@perry_class_shape_id_*`: that one is +stamped into every new instance by `js_object_alloc_class_inline_keys_stamped`, +so poisoning it would brand live objects with a bogus ShapeId rather than close +a fast path. Subclass arms and imported-class stubs carry the poisonable +expectation too, and an imported-stub rewrite that lands after a disable +re-poisons rather than resurrects. + +Measured on arm64 (`-Os` + `llc -O2 -mcpu=apple-m1`): one `o.a` on a typed +receiver goes from 28 to 23 executed fast-path instructions (−18%) with one +fewer dependent load; a probe making 16 reads on one receiver goes from 595.2 +to 563.0 executed instructions per call (−5.4%, `/usr/bin/time -l` instructions +retired, differenced over iteration count). The per-read marginal is −2 rather +than −5 because LLVM already hoisted the latch's GOT base register across +accesses within a function. The latch is gone from `$generic`, `$spec_b` and +the copy the inliner leaves in the caller — the last of which is the code that +actually executes. + +### The compiler's copy of the GC header layout is now gated + +`perry-codegen` does not depend on `perry-runtime`, yet it bakes the collector's +header layout into emitted code: the inline `new` path stores a packed +`GcHeader` word as a compile-time constant, and every class-field / +element-shape / method-probe guard masks that word against a literal. Thirty-six +restatements across ten files, with the agreement held by a code comment — the +`debug_assert_eq!` that looked like enforcement compared codegen's constant to a +string literal, a tautology that never referenced the runtime and is compiled +out of `release` and `perry-dev` anyway. A flag renumbered in the runtime +compiled clean, passed every suite, and shipped a compiler whose allocator baked +one bit layout while the collector read another. + +`scripts/check_gc_header_constants.py` (in `lint`) re-derives every restatement +from the runtime constant it quotes, including composites and the fused 32-bit +masks. A registered constant that stops existing fails, so a fix deletes its own +entry, and a new header-shaped `const` in a watched file must be registered or +exempted with a reason. Writing the registry found five restatements a +module-scope grep misses, because they are declared inside function bodies. + +`shape_descriptor_census` gains a matching requirement: the class-field +precheck must read its expectation VOLATILE from the poisonable global, since a +lowering that hoisted that load would reopen a fast path the runtime has closed +and would still satisfy a shape-only assertion. diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 925c42f4cd..a082a823c0 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1196,6 +1196,14 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> I32, "0", ); + // The poisonable twin of the ShapeId global: same value, same linkage, + // but only ever COMPARED against — see + // `typed_shape::guard_shape_global_name_from_keys_global`. + llmod.add_global( + &crate::typed_shape::guard_shape_global_name_from_keys_global(&global_name), + I32, + "0", + ); // #8122: the inline-`new` header image, composed at module init // (`string_pool.rs`) for the classes `class_header_images` admits. llmod.add_internal_global( @@ -1389,6 +1397,11 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> I32, "0", ); + llmod.add_internal_global( + &crate::typed_shape::guard_shape_global_name_from_keys_global(&global_name), + I32, + "0", + ); // #8122: the inline-`new` header image, composed at module init // (`string_pool.rs`) for the classes `class_header_images` admits. llmod.add_internal_global( diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index 6f68e99303..cd54dba1dc 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -647,6 +647,18 @@ pub(super) fn emit_string_pool( ); blk.store(I32, &shape_id, &shape_global); + // Seed the guard expectation with the same ShapeId and hand the + // runtime its address, so `disable_class_field_inline_guard` can poison + // it. Registration happens AFTER the seed, and the runtime poisons on + // the spot if the latch already flipped — so a module initialised late + // cannot reopen a fast path the process has closed. + let guard_global = format!( + "@{}", + crate::typed_shape::guard_shape_global_name_from_keys_global(global_name) + ); + blk.store(I32, &shape_id, &guard_global); + blk.call_void("js_register_class_guard_shape", &[(PTR, &guard_global)]); + // #8122: compose the class's inline-`new` header image — // `[packed GcHeader word | class_id | ShapeId << 32]` — beside the // ShapeId it consumes, ONCE. Every inline allocation of this class @@ -703,6 +715,7 @@ pub(super) fn emit_string_pool( (PTR, &global_ref), (PTR, &shape_global), (PTR, &image_ref), + (PTR, &guard_global), ], ); } diff --git a/crates/perry-codegen/src/expr/class_field_inline_guard.rs b/crates/perry-codegen/src/expr/class_field_inline_guard.rs index 1ffdd4a3b6..b680c2d4cf 100644 --- a/crates/perry-codegen/src/expr/class_field_inline_guard.rs +++ b/crates/perry-codegen/src/expr/class_field_inline_guard.rs @@ -155,7 +155,7 @@ pub(crate) fn class_field_subclass_arms( seen_ids.push(sub_id); arms.push(ClassFieldSubclassArm { class_id: sub_id, - shape_id_global: crate::typed_shape::shape_id_global_name_from_keys_global( + shape_id_global: crate::typed_shape::guard_shape_global_name_from_keys_global( &keys_global, ), }); @@ -441,12 +441,14 @@ pub(crate) fn emit_class_field_inline_precheck( obj_bits: &str, obj_handle: &str, expected_class_id: &str, - expected_shape_id: &str, require_raw_f64: bool, set_value_bits: Option<&str>, fast_label: &str, subclass_arms: &[ClassFieldSubclassArm], + keys_global_name: &str, ) -> String { + let guard_shape_global = + crate::typed_shape::guard_shape_global_name_from_keys_global(keys_global_name); let deref_idx = ctx.new_block("class_field_inline.deref"); let guardcall_idx = ctx.new_block("class_field_inline.guardcall"); let deref_label = ctx.block_label(deref_idx); @@ -467,14 +469,11 @@ pub(crate) fn emit_class_field_inline_precheck( // relaxed-atomic read the guard itself performs. { let blk = ctx.block(); - let flag = blk.load_volatile(I8, "@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED"); - let flag_ok = blk.icmp_eq(I8, &flag, "0"); let tag = blk.lshr(I64, obj_bits, "48"); let is_ptr = blk.icmp_eq(I64, &tag, POINTER_TAG_HI16); let above_band = blk.icmp_ugt(I64, obj_handle, HANDLE_BAND_TOP); let ptr_safe = blk.and(I1, &is_ptr, &above_band); - let can_inline = blk.and(I1, &ptr_safe, &flag_ok); - blk.cond_br(&can_inline, &deref_label, &guardcall_label); + blk.cond_br(&ptr_safe, &deref_label, &guardcall_label); } ctx.current_block = deref_idx; @@ -522,13 +521,20 @@ pub(crate) fn emit_class_field_inline_precheck( // ObjectHeader word 0 is class_id @0 and the authoritative ShapeId @4 // (#8113): one 64-bit compare against `(shape << 32) | class_id`. let identity = blk.load(I64, &obj_ptr); - let declared = expected_class_identity(blk, expected_class_id, expected_shape_id); + // The displaced latch's authority lives here now: this expectation is + // what `disable_class_field_inline_guard` poisons, so the compare the + // guard already had to make now also answers "is the inline path still + // open?". VOLATILE for exactly the reason the latch load was — the + // runtime flips it mid-execution and a cached expectation would take a + // fast path the process has closed. + let live_shape = blk.load_volatile(I32, &format!("@{guard_shape_global}")); + let declared = expected_class_identity(blk, expected_class_id, &live_shape); let mut shape_ok = blk.icmp_eq(I64, &identity, &declared); // The declared class's own (class id, ShapeId) pair, OR any subclass // arm's. Each arm is a full pair — matching a class id without its // canonical descriptor would accept a diverged layout. for arm in subclass_arms { - let arm_shape = blk.load(I32, &format!("@{}", arm.shape_id_global)); + let arm_shape = blk.load_volatile(I32, &format!("@{}", arm.shape_id_global)); let arm_expected = expected_class_identity(blk, &arm.class_id.to_string(), &arm_shape); let arm_ok = blk.icmp_eq(I64, &identity, &arm_expected); shape_ok = blk.or(I1, &shape_ok, &arm_ok); diff --git a/crates/perry-codegen/src/expr/hit_path_access_tests.rs b/crates/perry-codegen/src/expr/hit_path_access_tests.rs index 269e4067e0..1ad24a4850 100644 --- a/crates/perry-codegen/src/expr/hit_path_access_tests.rs +++ b/crates/perry-codegen/src/expr/hit_path_access_tests.rs @@ -272,6 +272,16 @@ fn point_class() -> Class { /// `probe(p: Point) { return p.x }` — the inline class-field guard tests the /// GcHeader with one masked 32-bit compare and the class/shape identity with /// one 64-bit compare, instead of five separate header loads. +/// +/// Three loads now, not two: the third is the poisonable +/// `@perry_class_guard_shape_*` expectation, which carries the authority the +/// `@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch used to. That is a +/// REDUCTION, not an addition — the latch it displaced was an `external +/// global`, so reading it cost a GOT load plus a dependent `ldrb` through it +/// plus a compare, in the gate block, on every access. Net per access: one +/// fewer machine instruction pair and one fewer dependent load. The assertions +/// below pin both halves, because "three loads" alone would also be satisfied +/// by a lowering that kept the latch and added the expectation. #[test] fn class_field_inline_guard_uses_two_fused_loads() { let mut m = module( @@ -290,8 +300,20 @@ fn class_field_inline_guard_uses_two_fused_loads() { let loads: Vec<&str> = deref.lines().filter(|l| l.contains(" = load ")).collect(); assert_eq!( loads.len(), - 2, - "the guard must load the header word and the identity word only:\n{deref}" + 3, + "the guard must load the header word, the identity word and the live \ + expectation, and nothing else:\n{deref}" + ); + assert!( + !ir.contains("@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED"), + "the per-access latch must be GONE from the class-field guard — its \ + authority moved onto the expectation this guard already loads:\n{ir}" + ); + assert!( + deref.contains("load volatile i32, ptr @perry_class_guard_shape_"), + "the expectation must be read VOLATILE per access: the runtime poisons \ + it mid-execution and a cached copy would reopen a closed fast \ + path:\n{deref}" ); assert!( loads.iter().any(|l| l.contains("load i32")) diff --git a/crates/perry-codegen/src/expr/property_get.rs b/crates/perry-codegen/src/expr/property_get.rs index c568fed651..cb89f4d244 100644 --- a/crates/perry-codegen/src/expr/property_get.rs +++ b/crates/perry-codegen/src/expr/property_get.rs @@ -1712,11 +1712,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, requires_raw_f64, None, &fast_label, &subclass_arms, + &keys_global_name, ); // ONE EXIT. Everything the pre-check could not prove — // the guard call, the guard-PASS slot load, the nullish @@ -1757,6 +1757,19 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let key_bits = blk.bitcast_double_to_i64(&key_box); blk.and(I64, &key_bits, POINTER_MASK_I64) }; + // Loaded HERE, not through the function-entry cache + // `load_class_shape_id` keeps: since the inline + // precheck moved to the poisonable + // `@perry_class_guard_shape_*` expectation, the + // truthful ShapeId is a cold-arm-only operand, and an + // entry-block load of it is two instructions the fast + // path pays and never reads. + let ic_shape_id = { + let global = crate::typed_shape::shape_id_global_name_from_keys_global( + &keys_global_name, + ); + ctx.block().load(I32, &format!("@{global}")) + }; let val_ic = ctx.block().call( DOUBLE, "js_class_field_get_ic", @@ -1764,7 +1777,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (I64, &site_id), (DOUBLE, &recv_box), (I32, &expected_class_id_str), - (I32, &expected_shape_id), + (I32, &ic_shape_id), (I64, &key_raw), (I32, &field_idx_str), (I32, requires_raw_f64_str), diff --git a/crates/perry-codegen/src/expr/property_get/helpers.rs b/crates/perry-codegen/src/expr/property_get/helpers.rs index 302fedd4b8..2ab78b7db8 100644 --- a/crates/perry-codegen/src/expr/property_get/helpers.rs +++ b/crates/perry-codegen/src/expr/property_get/helpers.rs @@ -765,11 +765,11 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context( &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, true, None, &fast_label, &subclass_arms, + &keys_global_name, ); let guard_ok = ctx.block().call( I32, diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index 4882eb2919..17cce2c26a 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -1261,11 +1261,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr, assignment_strict: bool) - &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, requires_raw_f64, Some(&val_bits), &fast_label, &subclass_arms, + &keys_global_name, ); let guard_ok = ctx.block().call( I32, diff --git a/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs b/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs index f942638e3f..1827274fad 100644 --- a/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs +++ b/crates/perry-codegen/src/expr/property_set/sloppy_class_field.rs @@ -158,8 +158,6 @@ pub(crate) fn try_lower_sloppy_class_field_store( let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let field_idx_str = field_index.to_string(); let expected_class_id_str = expected_class_id.to_string(); - let expected_shape_id = - crate::typed_shape::load_class_shape_id(ctx, &class_name, &keys_global_name); let (obj_bits, obj_handle, key_box, val_bits) = { let blk = ctx.block(); @@ -189,11 +187,11 @@ pub(crate) fn try_lower_sloppy_class_field_store( &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, true, Some(&val_bits), &fast_label, &subclass_arms, + &keys_global_name, ); // Miss: the strict-aware runtime with `strict = 0`, so a rejected write @@ -291,8 +289,6 @@ fn try_lower_sloppy_class_field_boxed_store( let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let field_idx_str = field_index.to_string(); let expected_class_id_str = expected_class_id.to_string(); - let expected_shape_id = - crate::typed_shape::load_class_shape_id(ctx, class_name, keys_global_name); let (obj_bits, obj_handle, key_box, val_bits) = { let blk = ctx.block(); @@ -322,11 +318,11 @@ fn try_lower_sloppy_class_field_boxed_store( &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, false, Some(&val_bits), &fast_label, &subclass_arms, + &keys_global_name, ); { diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 52e3540630..ca5809784d 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -1203,11 +1203,11 @@ pub(super) fn emit_guarded_direct_method_call( &obj_bits, &obj_handle, &expected_class_id_str, - &expected_shape_id, true, None, &proven_label, &[], + &keys_global_name, ); // Created after the precheck's own blocks so the merge (and the // typed/generic branch it feeds) follows the per-field guard diff --git a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs index fead87a8c6..c261864d57 100644 --- a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs +++ b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs @@ -683,13 +683,22 @@ fn imported_stub_registers_its_shape_slots_for_the_defining_modules_typed_id() { && call.contains("i32 55,"), "registration must name the stub's keys and ShapeId globals and its class id:\n{call}" ); - let shape_store = ir[..register_at] - .rfind("store i32 ") - .expect("the stub's own ShapeId store"); + // The stub seeds TWO globals before registering — its ShapeId and the + // poisonable guard expectation twinned with it — so look for the ShapeId + // store by name rather than taking whichever `store i32` happens to be + // last. assert!( - ir[shape_store..register_at].contains("@perry_class_shape_id_"), + ir[..register_at] + .rfind("@perry_class_shape_id_") + .is_some_and(|at| ir[at..register_at].contains("store i32 ") + || ir[..at].rfind("store i32 ").is_some()), "the slot is registered after this module stored its own id:\n{ir}" ); + assert!( + ir[..register_at].contains("@perry_class_guard_shape_"), + "the guard expectation must be seeded before registration too, or an \ + imported class guards against a stale value forever:\n{ir}" + ); let mut dylib = ir_opts(); dylib.output_type = "dylib".to_string(); diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index a73cd55079..8f0d3a3744 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1164,6 +1164,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { ); module.declare_function("js_build_class_keys_array", I64, &[I32, I32, PTR, I32]); module.declare_function("js_object_shape_id_for_keys", I32, &[I64, I32]); + module.declare_function("js_register_class_guard_shape", VOID, &[PTR]); // #10123: (shape_id, NaN-boxed key) -> inline slot index, or -1. The // element-shape loop clone's shape-keyed preheader resolves each tracked // property once against the shape the runtime just proved. @@ -1176,7 +1177,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function( "js_register_imported_class_shape_slot", VOID, - &[I32, I32, PTR, PTR, PTR], + &[I32, I32, PTR, PTR, PTR, PTR], ); // Inline bump-allocator state accessor + slow path. Ordinary allocation // kernels cache `js_inline_arena_state` at function entry. Self-recursive diff --git a/crates/perry-codegen/src/typed_shape.rs b/crates/perry-codegen/src/typed_shape.rs index d45b6b2cfa..0406d56338 100644 --- a/crates/perry-codegen/src/typed_shape.rs +++ b/crates/perry-codegen/src/typed_shape.rs @@ -380,6 +380,23 @@ pub(crate) fn raw_f64_mask_global_name_from_keys_global(keys_global_name: &str) /// minted once, immediately after `js_build_class_keys_array`, and loaded by /// every compiled construction path so class instances arrive birth-stamped /// instead of waiting for their first by-name lookup (#6759 C3 rung 2). +/// The per-class GUARD EXPECTATION paired with one canonical class keys array. +/// +/// Seeded at module init with the same ShapeId as +/// [`shape_id_global_name_from_keys_global`], and registered with the runtime +/// so `disable_class_field_inline_guard` can poison it. It exists as a SEPARATE +/// global for one reason: `js_object_alloc_class_inline_keys_stamped` stamps +/// every newly allocated instance with the value read out of the ShapeId +/// global (`lower_call/new_alloc.rs`), so poisoning that one would brand live +/// objects with a bogus ShapeId. The expectation is only ever compared against, +/// never stamped, so it is safe to poison. +pub(crate) fn guard_shape_global_name_from_keys_global(keys_global_name: &str) -> String { + keys_global_name + .strip_prefix("perry_class_keys_") + .map(|suffix| format!("perry_class_guard_shape_{}", suffix)) + .unwrap_or_else(|| format!("perry_class_guard_shape_{}", keys_global_name)) +} + pub(crate) fn shape_id_global_name_from_keys_global(keys_global_name: &str) -> String { keys_global_name .strip_prefix("perry_class_keys_") diff --git a/crates/perry-runtime/src/gc/layout/typed_shape.rs b/crates/perry-runtime/src/gc/layout/typed_shape.rs index 5056959022..785625611a 100644 --- a/crates/perry-runtime/src/gc/layout/typed_shape.rs +++ b/crates/perry-runtime/src/gc/layout/typed_shape.rs @@ -109,6 +109,11 @@ struct ImportedShapeSlot { keys_slot: usize, shape_slot: usize, image_slot: usize, + /// The importing module's `@perry_class_guard_shape_*` twin of + /// `shape_slot`. It must follow the same rewrite, or an imported class's + /// inline guard would compare against a stale expectation and miss for the + /// life of the process. 0 when the stub predates the guard global. + guard_slot: usize, } static REGISTERED_TYPED_SHAPES: std::sync::LazyLock> = @@ -259,6 +264,18 @@ unsafe fn rewrite_imported_shape_slot(slot: ImportedShapeSlot, slot_count: u32, return; } std::ptr::write(slot.shape_slot as *mut u32, shape_id); + if slot.guard_slot != 0 { + // The guard expectation follows the ShapeId — unless the inline path + // has already been disabled, in which case it must stay poisoned. A + // stub registered before the flip and rewritten after it would + // otherwise reopen a fast path the process has closed. + let value = if crate::object::class_field_inline_guard_enabled() { + shape_id + } else { + crate::object::CLASS_GUARD_SHAPE_POISON + }; + std::ptr::write(slot.guard_slot as *mut u32, value); + } if slot.image_slot != 0 { let word = (slot.image_slot as *mut u64).add(1); let class_id_bits = std::ptr::read(word) & 0xFFFF_FFFF; @@ -281,6 +298,7 @@ pub extern "C" fn js_register_imported_class_shape_slot( keys_slot: *const u64, shape_slot: *mut u32, image_slot: *mut u64, + guard_slot: *mut u32, ) { if class_id == 0 || keys_slot.is_null() || shape_slot.is_null() || slot_count >= 16_000_000 { return; @@ -289,7 +307,15 @@ pub extern "C" fn js_register_imported_class_shape_slot( keys_slot: keys_slot as usize, shape_slot: shape_slot as usize, image_slot: image_slot as usize, + guard_slot: guard_slot as usize, }; + // Also enrol the guard expectation for process-wide poisoning, so a LATER + // `disable_class_field_inline_guard` reaches an imported class's slot too. + if !guard_slot.is_null() { + // SAFETY: a compiled `@perry_class_guard_shape_*` global, static and + // writable for the life of the image (the caller's contract above). + unsafe { crate::object::js_register_class_guard_shape(guard_slot) }; + } let mut registered = registered_typed_shapes(); match registered .typed_by_class @@ -313,6 +339,7 @@ static KEEP_JS_REGISTER_IMPORTED_CLASS_SHAPE_SLOT: extern "C" fn( *const u64, *mut u32, *mut u64, + *mut u32, ) = js_register_imported_class_shape_slot; #[allow(clippy::too_many_arguments)] @@ -642,6 +669,10 @@ mod imported_shape_slot_tests { keys: Box, shape: Box, image: Box<[u64; 2]>, + /// The poisonable guard expectation twinned with `shape`. It must + /// follow every rewrite `shape` gets, or an imported class's inline + /// field guard compares against a stale value for the whole process. + guard: Box, } fn slots(class_id: u32) -> Slots { @@ -651,6 +682,7 @@ mod imported_shape_slot_tests { keys: Box::new(keys), shape: Box::new(ordinary), image: Box::new([0x1234_5678, ((ordinary as u64) << 32) | class_id as u64]), + guard: Box::new(ordinary), } } @@ -661,6 +693,7 @@ mod imported_shape_slot_tests { &*s.keys as *const u64, &mut *s.shape as *mut u32, s.image.as_mut_ptr(), + &mut *s.guard as *mut u32, ); } @@ -685,6 +718,14 @@ mod imported_shape_slot_tests { "the consumer's packed word is untouched" ); assert_eq!(s.image[1], ((typed as u64) << 32) | class_id as u64); + // Both rewrite paths must carry the guard expectation with them. If + // this drifts, an imported class's inline field guard compares a live + // object against the stub's ORDINARY id forever — it never goes wrong, + // it just silently never hits, which no correctness test would catch. + assert_eq!( + *s.guard, typed, + "the guard expectation follows the ShapeId slot" + ); } /// The consumer registers first (its string pool ran before the defining @@ -711,6 +752,29 @@ mod imported_shape_slot_tests { assert_published(class_id, &s, typed); } + /// Disabling the inline path poisons a registered expectation, and a + /// LATER rewrite must not resurrect it. + #[test] + fn a_disabled_inline_path_keeps_imported_expectations_poisoned() { + let class_id = 0x0B1_1005; + let mut s = slots(class_id); + register(class_id, &mut s); + crate::object::disable_class_field_inline_guard(); + assert_eq!( + *s.guard, + crate::object::CLASS_GUARD_SHAPE_POISON, + "disabling must poison an already-registered expectation" + ); + let typed = mint(class_id, *s.keys); + assert_eq!(*s.shape, typed, "the ShapeId slot still follows the mint"); + assert_eq!( + *s.guard, + crate::object::CLASS_GUARD_SHAPE_POISON, + "a rewrite after the disable must NOT reopen the fast path" + ); + crate::object::test_reset_class_field_inline_guard(); + } + /// A slot whose keys global does not hold the typed id's keys array, or /// whose slot count differs, is never rewritten. #[test] @@ -725,6 +789,7 @@ mod imported_shape_slot_tests { &*foreign.keys as *const u64, &mut *foreign.shape as *mut u32, foreign.image.as_mut_ptr(), + &mut *foreign.guard as *mut u32, ); let mut narrow = slots(class_id); let narrow_ordinary = *narrow.shape; @@ -734,6 +799,7 @@ mod imported_shape_slot_tests { &*narrow.keys as *const u64, &mut *narrow.shape as *mut u32, std::ptr::null_mut(), + &mut *narrow.guard as *mut u32, ); let _typed = mint(class_id, *narrow.keys); assert_eq!( diff --git a/crates/perry-runtime/src/object/class_guard_shape.rs b/crates/perry-runtime/src/object/class_guard_shape.rs new file mode 100644 index 0000000000..9cf4369ce8 --- /dev/null +++ b/crates/perry-runtime/src/object/class_guard_shape.rs @@ -0,0 +1,103 @@ +//! The per-class guard expectation: the poisonable carrier that replaced the +//! `@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch on the per-access +//! class-field guard. +//! +//! # Why a second global, beside the ShapeId +//! +//! The obvious move is to poison `@perry_class_shape_id_*` itself — the guard +//! already loads it, so disabling the fast path would cost nothing. That is +//! UNSOUND: `js_object_alloc_class_inline_keys_stamped` stamps every newly +//! allocated instance with the value read out of that global +//! (`perry-codegen/src/lower_call/new_alloc.rs`), so poisoning it would brand +//! live objects with a bogus ShapeId rather than close a fast path. +//! +//! `@perry_class_guard_shape_*` is seeded with the same ShapeId at module init +//! and is only ever COMPARED against, never stamped — so it is safe to poison, +//! and the guard gets the latch's authority out of a load it was making +//! anyway. Net on the emitted fast path of a static-key read: 28 -> 23 ARM64 +//! instructions, and one fewer dependent load (the latch was an `external +//! global`, so reading it cost a GOT load plus an `ldrb` through it). + +use super::descriptor_state::PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED; +use std::sync::atomic::Ordering; + +/// The value written into every registered `@perry_class_guard_shape_*` slot +/// when the inline fast path is disabled. +/// +/// ShapeIds are allocated from `[SHAPE_ID_BASE, SHAPE_ID_END)` = +/// `[0x8000_0000, 0xC000_0000)` and are never reused (`object/shapes.rs`), so +/// `u32::MAX` can never be a live ShapeId. A guard comparing an object's +/// `class_id | ShapeId << 32` word against a poisoned expectation therefore +/// misses for EVERY receiver, which is exactly what the latch bought — at no +/// per-access cost, because the guard already had to load its expectation. +pub const CLASS_GUARD_SHAPE_POISON: u32 = u32::MAX; + +/// Addresses of the per-class guard-expectation slots compiled code emits. +/// +/// Held as `(usize, u32)` — address plus the ShapeId it was seeded with — and +/// never as `*mut u32`: these point into the program's own data segment, never +/// into the Perry heap, so this table is deliberately NOT a GC root holder and +/// must not be registered with `gc_register_mutable_root_scanner`. The seeded +/// value is kept so `test_reset_class_field_inline_guard` can restore it; +/// production never unpoisons (the decision is monotonic). +static CLASS_GUARD_SHAPE_SLOTS: std::sync::Mutex> = + std::sync::Mutex::new(Vec::new()); + +/// Register a compiled module's per-class guard-expectation slot. +/// +/// Called once per class from module init, right after the slot is seeded with +/// the class's freshly minted ShapeId. A module initialised AFTER the latch +/// already flipped is poisoned on the spot, so late `require`/`dlopen` arrivals +/// cannot reopen a fast path the process has already closed. +/// +/// # Safety +/// `slot` must be a valid, writable, 4-byte-aligned `u32` with static lifetime +/// — i.e. a `@perry_class_guard_shape_*` global emitted by perry-codegen. +#[no_mangle] +pub unsafe extern "C" fn js_register_class_guard_shape(slot: *mut u32) { + if slot.is_null() { + return; + } + if let Ok(mut slots) = CLASS_GUARD_SHAPE_SLOTS.lock() { + // SAFETY: caller contract above. + let seeded = unsafe { slot.read() }; + slots.push((slot as usize, seeded)); + if PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED.load(Ordering::Relaxed) != 0 { + // GC_STORE_AUDIT(POINTER_FREE): a `u32` ShapeId in the program's own + // data segment, never a heap edge — the collector neither scans nor + // rewrites it. + // SAFETY: caller contract above. + unsafe { slot.write(CLASS_GUARD_SHAPE_POISON) }; + } + } +} + +pub(super) fn poison_class_guard_shapes() { + if let Ok(slots) = CLASS_GUARD_SHAPE_SLOTS.lock() { + for &(addr, _) in slots.iter() { + // GC_STORE_AUDIT(POINTER_FREE): a `u32` ShapeId in the program's own + // data segment, never a heap edge. + // SAFETY: every entry was registered through + // `js_register_class_guard_shape`, whose contract requires a valid + // writable static `u32`. + unsafe { (addr as *mut u32).write(CLASS_GUARD_SHAPE_POISON) }; + } + } +} + +/// Restore every registered expectation to the ShapeId it was seeded with. +/// +/// Production never does this — the disable decision is monotonic — but a test +/// that flips the latch must not leave later tests guarding against +/// [`CLASS_GUARD_SHAPE_POISON`]. +#[cfg(test)] +pub(super) fn restore_class_guard_shapes_for_test() { + if let Ok(slots) = CLASS_GUARD_SHAPE_SLOTS.lock() { + for &(addr, seeded) in slots.iter() { + // GC_STORE_AUDIT(POINTER_FREE): a `u32` ShapeId in the program's own + // data segment, never a heap edge. + // SAFETY: registered through `js_register_class_guard_shape`. + unsafe { (addr as *mut u32).write(seeded) }; + } + } +} diff --git a/crates/perry-runtime/src/object/descriptor_state.rs b/crates/perry-runtime/src/object/descriptor_state.rs index 7f2e23ed8b..45f2b6b671 100644 --- a/crates/perry-runtime/src/object/descriptor_state.rs +++ b/crates/perry-runtime/src/object/descriptor_state.rs @@ -290,7 +290,15 @@ pub static PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED: AtomicU8 = AtomicU8::new(0); /// Disable the codegen-inlined class-field fast path process-wide (see /// [`PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED`]). Idempotent. +/// +/// Sets the latch AND poisons every registered guard-expectation slot. The two +/// are one decision with two carriers: sites that still read the latch keep +/// working unchanged, while `emit_class_field_inline_precheck` — the per-access +/// guard on every static-key read — gets the same authority for free out of the +/// expectation it already loads. Poison first, so no thread can observe a set +/// latch beside a live expectation. pub(crate) fn disable_class_field_inline_guard() { + super::class_guard_shape::poison_class_guard_shapes(); PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED.store(1, Ordering::Relaxed); } @@ -302,6 +310,11 @@ pub(crate) fn class_field_inline_guard_enabled() -> bool { #[cfg(test)] pub(crate) fn test_reset_class_field_inline_guard() { PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED.store(0, Ordering::Relaxed); + // Unpoison every registered expectation back to the ShapeId it was seeded + // with. Production never does this — the disable decision is monotonic — + // but a test that flips the latch must not leave later tests guarding + // against `CLASS_GUARD_SHAPE_POISON`. + super::class_guard_shape::restore_class_guard_shapes_for_test(); // Also clear the C5a per-key vetting sets (production-monotonic, so // without this a key name reused across tests in one process would // inherit an earlier test's declared-field / installed-key state and diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index d23eab11e6..441ac96acc 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -200,6 +200,7 @@ mod websocket_global; mod with_env; // Issue #1103 follow-up: behavior-preserving split of the residual top-level // helpers that lived directly in `object/mod.rs`. +mod class_guard_shape; mod class_meta_registry; pub(crate) mod descriptor_state; mod this_binding; @@ -265,6 +266,7 @@ pub use with_env::*; // Re-exports for the residual-helper split (issue #1103 follow-up). Explicit // named re-exports keep existing `crate::object::X` / bare-name call sites in // the object submodules resolving unchanged. +pub use class_guard_shape::{js_register_class_guard_shape, CLASS_GUARD_SHAPE_POISON}; pub(crate) use class_meta_registry::{ builtin_error_prototype_name, class_generic_origin, extends_builtin_error, fetch_parent_kind, lookup_has_instance_hook, lookup_to_string_tag_hook, register_fetch_parent_kind, @@ -276,6 +278,8 @@ pub use class_meta_registry::{ }; #[cfg(test)] pub(crate) use descriptor_state::test_may_have_descriptor_entry; +#[cfg(test)] +pub(crate) use descriptor_state::test_reset_class_field_inline_guard; pub use descriptor_state::PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED; pub(crate) use descriptor_state::{ accessor_descriptor_keys_for_obj, class_field_inline_guard_enabled, diff --git a/scripts/check_gc_header_constants.py b/scripts/check_gc_header_constants.py new file mode 100755 index 0000000000..5fc08d736c --- /dev/null +++ b/scripts/check_gc_header_constants.py @@ -0,0 +1,402 @@ +#!/usr/bin/env python3 +"""Hold every compiler-side restatement of a GC header bit to the runtime's own. + +WHY THIS EXISTS +--------------- +`perry-codegen` does NOT depend on `perry-runtime` — its dependencies are +perry-hir, perry-dispatch and perry-api-manifest. Yet the compiler bakes the +collector's header layout into emitted code in two load-bearing ways: + +* the inline `new` path stores a packed `GcHeader` word as a COMPILE-TIME + constant (`target_layout::inline_alloc_gc_packed`, #8122), pre-composed per + class into `@perry_class_header_image_*`; and +* every class-field / element-shape guard masks that word against a literal + and compares it to a literal (`expr/class_field_inline_guard.rs`, + `expr/element_shape_guard.rs`). + +Both sides therefore carry their own copy of `GC_TYPE_OBJECT`, +`GC_FLAG_FORWARDED`, `OBJ_FLAG_HAS_DESCRIPTORS`, `GC_OBJ_TYPED_LAYOUT_INTACT` +and friends, and until this checker the agreement was held by a code comment +("Runtime-side name: `gc::layout::GC_OBJ_TYPED_LAYOUT_INTACT`"). + +The things that LOOK like they enforce it do not: + + // expr/class_field_inline_guard.rs + const GC_FLAG_FORWARDED_I8: &str = "-128"; + ... + debug_assert_eq!(GC_FLAG_FORWARDED_I8, "-128"); + +That compares codegen's constant to a string literal — a tautology. It is a +useful "you edited the const, now fix the mask arithmetic below" pin, but it +never references the runtime, so it cannot detect a renumbering there; and per +CLAUDE.md's profile note it is compiled out of `release` AND `perry-dev` +anyway. Every codegen test naming these bits asserts codegen's own constant +appears in the emitted IR, so they pin codegen to itself and would all stay +green. + +So, before this checker, renumbering a flag in `perry-runtime` compiled clean, +passed every suite, and shipped a compiler whose inline allocator baked one bit +layout while the collector read another — objects born with flags the GC +misreads. CLAUDE.md describes that class of bug as surfacing cycles later as +`TypeError: value is not a function`, nowhere near the cause. + +WHAT THIS DOES NOT CATCH (stated plainly, per CLAUDE.md's gate rules) +-------------------------------------------------------------------- +* Whether a bit ASSIGNMENT is the right one. This only proves the two sides + agree, never that the value is well chosen. +* Restatements that are not a `const` declaration — a bare literal inline in + an expression is invisible here. The registry below is the defence against + that: a checked constant must be DECLARED, so review has one place to look. +* Layout contracts other than the 32-bit GC header word. String, Map and array + header offsets/sizes are duplicated the same way and are enumerated in + `OUT_OF_SCOPE` rather than silently ignored — they belong to different + subsystems and want their own derivation, not a second-guessing one here. +* Field OFFSETS within `GcHeader` (obj_type @-8, gc_flags @-7, _reserved @-6). + Those are asserted structurally by the runtime's own layout tests. + +Usage: + python3 scripts/check_gc_header_constants.py # check + python3 scripts/check_gc_header_constants.py --list # describe + python3 scripts/check_gc_header_constants.py --self-test # prove it can fail +""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent + +# --------------------------------------------------------------------------- +# The authoritative side: where each runtime constant is DEFINED. +# --------------------------------------------------------------------------- +RUNTIME_SOURCES = [ + "crates/perry-runtime/src/gc/types.rs", + "crates/perry-runtime/src/gc/layout.rs", +] + +# Runtime constants this checker resolves. Anything a codegen restatement +# derives from must be listed here, so a rename on the runtime side fails loudly +# instead of leaving a restatement unanchored. +RUNTIME_WANTED = { + "GC_TYPE_ARRAY", + "GC_TYPE_OBJECT", + "GC_TYPE_MAP", + "GC_FLAG_ARENA", + "GC_FLAG_FORWARDED", + "OBJ_FLAG_FROZEN", + "OBJ_FLAG_PACKED_NUMERIC_PROOF", + "OBJ_FLAG_PLAIN_ORDINARY", + "OBJ_FLAG_ARRAY_DESCRIPTORS", + "OBJ_FLAG_STABLE_TOMBSTONES", + "OBJ_FLAG_HAS_DESCRIPTORS", + "GC_LAYOUT_POINTER_FREE", + "GC_LAYOUT_SIDE_MASK", + "GC_OBJ_TYPED_LAYOUT_INTACT", +} + +# --------------------------------------------------------------------------- +# The registry: every codegen-side restatement, and how to re-derive it. +# +# `expr` is evaluated with the runtime constants in scope. A restatement whose +# declaration has vanished FAILS — a fix must delete its entry, so this list +# cannot rot into a description of a tree that no longer exists. +# +# Byte positions inside the 32-bit header word (little-endian): +# bits 0..7 obj_type | bits 8..15 gc_flags | bits 16..31 _reserved +# --------------------------------------------------------------------------- +Restatement = tuple[str, str, str, str] # (file, const, expr, why) + +REGISTRY: list[Restatement] = [ + # --- the packed GcHeader word the inline `new` path bakes (#8122) -------- + ("crates/perry-codegen/src/target_layout.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "byte 0 of the baked header word"), + ("crates/perry-codegen/src/target_layout.rs", "GC_FLAG_ARENA", + "GC_FLAG_ARENA", "byte 1 of the baked header word"), + ("crates/perry-codegen/src/target_layout.rs", "GC_LAYOUT_POINTER_FREE", + "GC_LAYOUT_POINTER_FREE", "_reserved half of the baked header word"), + ("crates/perry-codegen/src/target_layout.rs", "GC_LAYOUT_SIDE_MASK", + "GC_LAYOUT_SIDE_MASK", "_reserved half of the baked header word"), + ("crates/perry-codegen/src/target_layout.rs", "GC_OBJ_TYPED_LAYOUT_INTACT", + "GC_OBJ_TYPED_LAYOUT_INTACT", "_reserved half of the baked header word"), + # `new_alloc.rs` re-derives the same word at the allocation site and + # cross-checks it against the per-class table; both copies are pinned. + ("crates/perry-codegen/src/lower_call/new_alloc.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "allocation-site copy of the baked header word"), + ("crates/perry-codegen/src/lower_call/new_alloc.rs", "GC_FLAG_ARENA", + "GC_FLAG_ARENA", "allocation-site copy of the baked header word"), + ("crates/perry-codegen/src/lower_call/new_alloc.rs", "GC_LAYOUT_POINTER_FREE", + "GC_LAYOUT_POINTER_FREE", "allocation-site copy of the baked header word"), + ("crates/perry-codegen/src/lower_call/new_alloc.rs", "GC_OBJ_TYPED_LAYOUT_INTACT", + "GC_OBJ_TYPED_LAYOUT_INTACT", "allocation-site copy of the baked header word"), + + # --- the class-field inline guard --------------------------------------- + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "guard: obj_type byte"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", "GC_FLAG_FORWARDED_I8", + "GC_FLAG_FORWARDED - 256", "guard: gc_flags 0x80 spelled as a signed i8"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", "TYPED_LAYOUT_INTACT_BIT", + "GC_OBJ_TYPED_LAYOUT_INTACT", "guard: raw-f64 slots need the intact bit"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", "OBJ_FLAG_FROZEN_BIT", + "OBJ_FLAG_FROZEN", "guard: a frozen receiver must route through the setter"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", + "OBJ_FLAG_PACKED_NUMERIC_PROOF_BIT", "OBJ_FLAG_PACKED_NUMERIC_PROOF", + "guard: #8690 Array-subclass numeric-prefix proof"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", + "OBJ_FLAG_READ_FAST_PATH_BLOCKED", + "OBJ_FLAG_ARRAY_DESCRIPTORS | OBJ_FLAG_HAS_DESCRIPTORS", + "guard: #5654 per-receiver descriptor veto (a COMPOSITE of two flags)"), + ("crates/perry-codegen/src/expr/class_field_inline_guard.rs", + "OBJ_FLAG_WRITE_FAST_PATH_BLOCKED", + "OBJ_FLAG_ARRAY_DESCRIPTORS | OBJ_FLAG_HAS_DESCRIPTORS" + " | OBJ_FLAG_PACKED_NUMERIC_PROOF | OBJ_FLAG_FROZEN", + "guard: the write side adds frozen + the packed-numeric proof"), + + # --- the element-shape guard's fused masks ------------------------------- + ("crates/perry-codegen/src/expr/element_shape_guard.rs", "GC_TYPE_ARRAY", + "GC_TYPE_ARRAY", "element guard: obj_type byte"), + ("crates/perry-codegen/src/expr/element_shape_guard.rs", "ELEM_HEADER_MASK", + "0xFF | (GC_FLAG_FORWARDED << 8)" + " | ((GC_OBJ_TYPED_LAYOUT_INTACT | OBJ_FLAG_HAS_DESCRIPTORS) << 16)", + "element guard: one fused 32-bit mask over all three header bytes"), + ("crates/perry-codegen/src/expr/element_shape_guard.rs", "ELEM_HEADER_EXPECT", + "GC_TYPE_OBJECT | (GC_OBJ_TYPED_LAYOUT_INTACT << 16)", + "element guard: the value ELEM_HEADER_MASK must produce"), + ("crates/perry-codegen/src/expr/element_shape_guard.rs", "ELEM_HEADER_SHAPE_MASK", + "0xFF | (GC_FLAG_FORWARDED << 8) | (OBJ_FLAG_HAS_DESCRIPTORS << 16)", + "element guard: shape-keyed arm drops the intact conjunct"), + ("crates/perry-codegen/src/expr/element_shape_guard.rs", "ELEM_HEADER_SHAPE_EXPECT", + "GC_TYPE_OBJECT", "element guard: shape-keyed arm's expected value"), + + # --- the array-literal inline allocator's baked header word ------------- + ("crates/perry-codegen/src/expr/array_literal.rs", "GC_TYPE_ARRAY", + "GC_TYPE_ARRAY", "array literal: obj_type byte of the baked header word"), + ("crates/perry-codegen/src/expr/array_literal.rs", "GC_FLAG_ARENA", + "GC_FLAG_ARENA", "array literal: gc_flags byte of the baked header word"), + ("crates/perry-codegen/src/expr/array_literal.rs", "GC_LAYOUT_POINTER_FREE", + "GC_LAYOUT_POINTER_FREE", "array literal: _reserved half of the baked header word"), + + # --- the typed-f64 receiver method probe's fused mask ------------------- + ("crates/perry-codegen/src/lower_call/method_override.rs", + "GC_OBJECT_METHOD_GUARD_MASK_I32", + "0xFF | (GC_FLAG_FORWARDED << 8)" + " | ((OBJ_FLAG_HAS_DESCRIPTORS | OBJ_FLAG_PACKED_NUMERIC_PROOF) << 16)", + "method probe: one fused 32-bit mask over all three header bytes"), + ("crates/perry-codegen/src/lower_call/property_get/imported_object.rs", + "GC_OBJECT_METHOD_GUARD_MASK_I32", + "0xFF | (GC_FLAG_FORWARDED << 8)" + " | ((OBJ_FLAG_HAS_DESCRIPTORS | OBJ_FLAG_PACKED_NUMERIC_PROOF) << 16)", + "imported-object probe: second copy of the same fused mask"), + + # --- other single-bit restatements -------------------------------------- + ("crates/perry-codegen/src/expr/in_presence_ic.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "`in` presence IC: obj_type byte"), + ("crates/perry-codegen/src/expr/in_presence_ic.rs", "GC_FLAG_FORWARDED", + "GC_FLAG_FORWARDED", "`in` presence IC: not-forwarded"), + ("crates/perry-codegen/src/expr/arrays_finds.rs", "GC_TYPE_MAP", + "GC_TYPE_MAP", "Map find fast path: obj_type byte"), + ("crates/perry-codegen/src/expr/arrays_finds.rs", "GC_FLAG_FORWARDED", + "GC_FLAG_FORWARDED", "Map find fast path: not-forwarded"), + ("crates/perry-codegen/src/lower_call/method_override.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "typed-f64 receiver method probe: obj_type byte"), + ("crates/perry-codegen/src/lower_call/property_get/imported_object.rs", "GC_TYPE_OBJECT", + "GC_TYPE_OBJECT", "imported-object property get: obj_type byte"), + ("crates/perry-codegen/src/expr/proxy_reflect.rs", "PLAIN_ORDINARY_OBJ_FLAG", + "OBJ_FLAG_PLAIN_ORDINARY", "proxy/reflect: plain-ordinary veto"), + ("crates/perry-codegen/src/expr/proxy_reflect_write_ic.rs", "STABLE_TOMBSTONES_OBJ_FLAG", + "OBJ_FLAG_STABLE_TOMBSTONES", "proxy write IC: stable-tombstones veto"), + ("crates/perry-codegen/src/codegen/string_pool.rs", "GC_LAYOUT_AND_INTACT_MASK", + "GC_LAYOUT_POINTER_FREE | GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT", + "module init: the header-image layout bits it may rewrite"), + ("crates/perry-codegen/src/codegen/string_pool.rs", "GC_SIDE_MASK_AND_INTACT", + "GC_LAYOUT_SIDE_MASK | GC_OBJ_TYPED_LAYOUT_INTACT", + "module init: the side-mask + intact pair it writes"), +] + +# Declared constants this checker deliberately does not anchor, each with the +# reason. Enumerated rather than ignored: a reader should be able to see the +# whole duplicated surface in one place, including the parts out of scope. +OUT_OF_SCOPE = { + ("crates/perry-codegen/src/target_layout.rs", "GC_HEADER_SIZE_BYTES"): + "a STRUCT SIZE, not a bit assignment; the runtime asserts it structurally", + ("crates/perry-codegen/src/lower_call/new_alloc.rs", "GC_HEADER_SIZE"): + "same struct size, re-derived at the allocation site", + ("crates/perry-codegen/src/expr/array_literal.rs", "GC_HEADER_SIZE"): + "same struct size, re-derived at the array-literal site", + ("crates/perry-codegen/src/gc_map.rs", "GC_MAP_MAGIC"): + "`.perry_gcmap` section format, not the object header", + ("crates/perry-codegen/src/gc_map.rs", "GC_MAP_VERSION"): + "`.perry_gcmap` section format, not the object header", + ("crates/perry-codegen/src/gc_map.rs", "GC_MAP_LABEL"): + "`.perry_gcmap` section format, not the object header", +} + +# Prefixes that make a codegen `const` look like a header restatement. A new +# declaration matching one of these must be registered above or exempted in +# OUT_OF_SCOPE, so the next one cannot arrive silently — the rule +# `check_node_version_consistency.py` uses for `node-version:` literals. +WATCHED = re.compile(r"^(GC_|OBJ_|TYPED_LAYOUT|PLAIN_ORDINARY_OBJ|STABLE_TOMBSTONES_OBJ|ELEM_HEADER)") + +CONST_RE = re.compile( + r"^\s*(?:pub(?:\([^)]*\))?\s+)?const\s+([A-Za-z_][A-Za-z0-9_]*)\s*:\s*[^=]+=\s*([^;]+);" +) + + +def parse_consts(path: Path) -> dict[str, str]: + out: dict[str, str] = {} + for line in path.read_text().splitlines(): + m = CONST_RE.match(line) + if m: + out.setdefault(m.group(1), m.group(2).strip()) + return out + + +def literal_value(raw: str) -> int | None: + """The integer a declaration's right-hand side denotes, or None.""" + text = raw.strip().strip('"').strip() + text = re.sub(r"_", "", text) + try: + return int(text, 0) + except ValueError: + return None + + +def runtime_values(root: Path) -> tuple[dict[str, int], list[str]]: + values: dict[str, int] = {} + problems: list[str] = [] + for rel in RUNTIME_SOURCES: + path = root / rel + if not path.exists(): + problems.append(f"runtime source missing: {rel}") + continue + for name, raw in parse_consts(path).items(): + if name in RUNTIME_WANTED and name not in values: + v = literal_value(raw) + if v is not None: + values[name] = v + for name in sorted(RUNTIME_WANTED - values.keys()): + problems.append( + f"runtime constant {name} not found in {' or '.join(RUNTIME_SOURCES)} — " + "it was renamed or moved; update RUNTIME_SOURCES/RUNTIME_WANTED so the " + "restatements that derive from it stay anchored" + ) + return values, problems + + +def check(root: Path) -> list[str]: + values, problems = runtime_values(root) + if problems: + return problems + + declared: dict[tuple[str, str], str] = {} + for rel in sorted({entry[0] for entry in REGISTRY} | {k[0] for k in OUT_OF_SCOPE}): + path = root / rel + if not path.exists(): + problems.append(f"registered file missing: {rel}") + continue + for name, raw in parse_consts(path).items(): + declared[(rel, name)] = raw + + for rel, const, expr, why in REGISTRY: + raw = declared.get((rel, const)) + if raw is None: + problems.append( + f"{rel}: registered constant {const} no longer declared " + f"({why}) — delete its REGISTRY entry in the same commit" + ) + continue + got = literal_value(raw) + if got is None: + problems.append(f"{rel}:{const} = {raw!r} is not an integer literal") + continue + want = eval(expr, {"__builtins__": {}}, dict(values)) # noqa: S307 - fixed table + if got != want: + problems.append( + f"{rel}:{const} = {got} (0x{got:X}) but the runtime says " + f"{want} (0x{want:X})\n" + f" derivation: {expr}\n" + f" role: {why}\n" + f" The compiler bakes this into emitted code and does NOT link " + f"perry-runtime, so a mismatch ships a binary whose objects the " + f"collector misreads. Fix the compiler side, or update the " + f"derivation if the runtime deliberately moved the bit." + ) + + # Nothing header-shaped may arrive unregistered. + watched_files = {entry[0] for entry in REGISTRY} | {k[0] for k in OUT_OF_SCOPE} + known = {(rel, const) for rel, const, _, _ in REGISTRY} | set(OUT_OF_SCOPE) + for (rel, const) in sorted(declared): + if rel in watched_files and WATCHED.match(const) and (rel, const) not in known: + problems.append( + f"{rel}: {const} looks like a GC header restatement but is not " + "registered. Add it to REGISTRY with the expression that " + "re-derives it from perry-runtime, or to OUT_OF_SCOPE with a reason." + ) + return problems + + +def self_test(root: Path) -> int: + """Prove the checker can fail: perturb one runtime value and expect a report.""" + values, _ = runtime_values(root) + saved = values["GC_OBJ_TYPED_LAYOUT_INTACT"] + rel, const, expr, why = next( + e for e in REGISTRY if e[1] == "TYPED_LAYOUT_INTACT_BIT" + ) + raw = parse_consts(root / rel).get(const) + got = literal_value(raw) + perturbed = dict(values) + perturbed["GC_OBJ_TYPED_LAYOUT_INTACT"] = saved << 1 + want = eval(expr, {"__builtins__": {}}, perturbed) # noqa: S307 + if got == want: + print("self-test FAILED: a moved intact bit was not detected", file=sys.stderr) + return 1 + if check(root): + print("self-test FAILED: the tree is already red", file=sys.stderr) + return 1 + print( + "check_gc_header_constants self-test: OK — a one-bit move of " + "GC_OBJ_TYPED_LAYOUT_INTACT is detected, and the tree is currently clean" + ) + return 0 + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--list", action="store_true", help="describe what is pinned") + ap.add_argument("--self-test", action="store_true", help="prove the checker can fail") + args = ap.parse_args() + + if args.self_test: + return self_test(REPO) + + if args.list: + values, _ = runtime_values(REPO) + print("Authoritative runtime values:") + for name in sorted(values): + print(f" {name:32s} = {values[name]} (0x{values[name]:X})") + print(f"\nCompiler-side restatements pinned ({len(REGISTRY)}):") + for rel, const, expr, why in REGISTRY: + print(f" {rel}\n {const} = {expr}\n {why}") + print(f"\nDeclared but out of scope ({len(OUT_OF_SCOPE)}):") + for (rel, const), reason in sorted(OUT_OF_SCOPE.items()): + print(f" {rel}:{const} — {reason}") + return 0 + + problems = check(REPO) + if problems: + print("check_gc_header_constants: FAILED\n", file=sys.stderr) + for p in problems: + print(f" - {p}\n", file=sys.stderr) + return 1 + print( + f"check_gc_header_constants: OK — {len(REGISTRY)} compiler-side header " + f"restatements agree with perry-runtime " + f"({len(OUT_OF_SCOPE)} declared constants out of scope, listed with reasons)" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/shape_descriptor_census.py b/scripts/shape_descriptor_census.py index 910f1ed5a1..1361cf1497 100644 --- a/scripts/shape_descriptor_census.py +++ b/scripts/shape_descriptor_census.py @@ -676,6 +676,16 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: # and compares it with `expected_class_identity`, which puts # the ShapeId in the high 32 bits. All three halves are # required, so dropping the ShapeId from the compare fails. + # + # The expectation is the POISONABLE `@perry_class_guard_shape_*` + # twin, not `@perry_class_shape_id_*`, and it is read VOLATILE + # per access. That is load-bearing, not incidental: this compare + # now carries the authority the + # `@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch used to, so a + # lowering that hoisted the load, or read the ShapeId global + # instead, would take a fast path the runtime has already closed + # — and would still pass a shape-only assertion. Both halves are + # required here for that reason. require_code( body, r"load\s*\(\s*I64\s*,\s*&obj_ptr\s*\)", @@ -683,8 +693,13 @@ def assert_authority_surfaces(sources: dict[str, str]) -> None: ) require_code( body, - r"expected_class_identity\s*\(\s*blk\s*,\s*expected_class_id\s*,\s*expected_shape_id\s*\)", - f"{name} compares the identity word with the expected ShapeId", + r"expected_class_identity\s*\(\s*blk\s*,\s*expected_class_id\s*,\s*&live_shape\s*\)", + f"{name} compares the identity word with the live expectation", + ) + require_code( + body, + r"load_volatile\s*\(\s*I32\s*,\s*&format!\(\s*\"@\{guard_shape_global\}\"", + f"{name} reads the poisonable expectation VOLATILE, per access", ) require_code( function_body(raw_class_guard, "expected_class_identity"),