Skip to content

Commit a997276

Browse files
author
Ralph Küpper
committed
fix(codegen): harden packed loop invalidation
1 parent 8571570 commit a997276

12 files changed

Lines changed: 138 additions & 139 deletions

File tree

crates/perry-codegen/src/collectors/proven_this_routing_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ fn single_arm_method_shape_guard_is_inlined_with_the_runtime_contract() {
738738
&& probe.contains("getelementptr i8, ptr")
739739
&& probe.contains("i64 -8")
740740
&& probe.contains("and i32")
741-
&& probe.contains(", 134250751")
741+
&& probe.contains(", 142639359")
742742
&& probe.contains("icmp eq i32")
743743
&& probe.contains(", 2")
744744
&& probe.contains("load i64, ptr")
@@ -751,7 +751,7 @@ fn single_arm_method_shape_guard_is_inlined_with_the_runtime_contract() {
751751
&& probe.contains(", -2147483648")
752752
&& probe.contains("icmp ult i32")
753753
&& probe.contains(", 1073741824"),
754-
"the packed header block must check the GC type, forwarding flag, own-descriptor bit, exact class/ShapeId pair, and ShapeId domain:\n{probe}"
754+
"the packed header block must check the GC type, forwarding flag, own-descriptor/packed-proof bits, exact class/ShapeId pair, and ShapeId domain:\n{probe}"
755755
);
756756
}
757757

crates/perry-codegen/src/expr/class_field_barrier_tests.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,19 @@ pub(super) fn ir() -> String {
232232
.expect("LLVM IR should be UTF-8")
233233
}
234234

235+
/// #8690: pointer-free tagged writes (SSO and booleans) skip the shared
236+
/// value-is-pointer bookkeeping arm. The receiver precheck must therefore
237+
/// reject the packed-numeric authority bit before entering the inline store.
238+
#[test]
239+
fn class_field_set_precheck_blocks_packed_numeric_proof_receivers() {
240+
let ir = ir();
241+
assert!(
242+
ir.lines()
243+
.any(|line| line.contains("and i16") && line.trim_end().ends_with(", 128")),
244+
"class-field set admission must mask OBJ_FLAG_PACKED_NUMERIC_PROOF (0x80)\n{ir}"
245+
);
246+
}
247+
235248
/// The `br i1 %cond, label %class_field_set.barrier.N, label %...` line, plus
236249
/// the body of the block that contains it.
237250
///

crates/perry-codegen/src/expr/class_field_inline_guard.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ const GC_FLAG_FORWARDED_I8: &str = "-128"; // 0x80 as i8
4848
const TYPED_LAYOUT_INTACT_BIT: &str = "4096"; // GC_OBJ_TYPED_LAYOUT_INTACT (0x1000)
4949
const OBJ_FLAG_FROZEN_BIT: &str = "1"; // OBJ_FLAG_FROZEN (0x01)
5050
const OBJ_FLAG_HAS_DESCRIPTORS_BIT: &str = "2048"; // OBJ_FLAG_HAS_DESCRIPTORS (0x800)
51-
/// `OBJ_FLAG_FROZEN | OBJ_FLAG_HAS_DESCRIPTORS` — both live in the same
52-
/// `GcHeader::_reserved` i16, so one mask tests both.
53-
const OBJ_FLAG_FROZEN_OR_DESCRIPTORS: &str = "2049";
51+
const OBJ_FLAG_PACKED_NUMERIC_PROOF_BIT: &str = "128"; // OBJ_FLAG_PACKED_NUMERIC_PROOF (0x080)
52+
/// `OBJ_FLAG_FROZEN | OBJ_FLAG_HAS_DESCRIPTORS | OBJ_FLAG_PACKED_NUMERIC_PROOF`
53+
/// — all live in the same `GcHeader::_reserved` i16, so one mask tests them.
54+
const OBJ_FLAG_WRITE_FAST_PATH_BLOCKED: &str = "2177";
5455
const F64_EXP_MASK: &str = "9218868437227405312"; // 0x7FF0_0000_0000_0000
5556

5657
/// A widening arm for the class-field shape check: one concrete subclass whose
@@ -308,9 +309,9 @@ pub(crate) fn emit_class_field_loop_preheader_check(
308309
}
309310

310311
if require_not_frozen {
311-
let frozen = blk.and(I16, &reserved, OBJ_FLAG_FROZEN_BIT);
312-
let not_frozen = blk.icmp_eq(I16, &frozen, "0");
313-
acc = blk.and(I1, &acc, &not_frozen);
312+
let blocked = blk.and(I16, &reserved, OBJ_FLAG_WRITE_FAST_PATH_BLOCKED);
313+
let write_fast_path_ok = blk.icmp_eq(I16, &blocked, "0");
314+
acc = blk.and(I1, &acc, &write_fast_path_ok);
314315
}
315316

316317
// No terminator: the caller branches after verifying the fast clone.
@@ -394,7 +395,7 @@ pub(crate) fn emit_proven_shape_recheck(
394395

395396
let res_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-6")]);
396397
let reserved = blk.load(I16, &res_ptr);
397-
let latched = blk.and(I16, &reserved, OBJ_FLAG_FROZEN_OR_DESCRIPTORS);
398+
let latched = blk.and(I16, &reserved, OBJ_FLAG_WRITE_FAST_PATH_BLOCKED);
398399
let unlatched = blk.icmp_eq(I16, &latched, "0");
399400

400401
// `class_id` @0 was already matched by the tower. ShapeId @4 proves the
@@ -554,6 +555,13 @@ pub(crate) fn emit_class_field_inline_precheck(
554555
let not_frozen = blk.icmp_eq(I16, &frozen, "0");
555556
acc = blk.and(I1, &acc, &not_frozen);
556557

558+
// #8690: an inline field write can overlap an Array-subclass
559+
// numeric prefix. Route proof-authoritative receivers through the
560+
// runtime setter so pointer-free SSO/boolean stores retire it too.
561+
let numeric_proof = blk.and(I16, &reserved, OBJ_FLAG_PACKED_NUMERIC_PROOF_BIT);
562+
let no_numeric_proof = blk.icmp_eq(I16, &numeric_proof, "0");
563+
acc = blk.and(I1, &acc, &no_numeric_proof);
564+
557565
if require_raw_f64 {
558566
// Only a plain finite number may be stored raw. Non-finite
559567
// (exponent all-ones: ±Inf/NaN — rare) and every NaN-boxed tag

crates/perry-codegen/src/expr/literals_vars.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::lower_string_concat::{
1414
};
1515
use crate::nanbox::double_literal;
1616
use crate::native_value::ExpectedNativeRep;
17-
use crate::type_analysis::{is_map_expr, is_numeric_expr, is_set_expr, receiver_class_name};
17+
use crate::type_analysis::{is_map_expr, is_set_expr, receiver_class_name};
1818
use crate::types::{DOUBLE, I32, I64};
1919

2020
use super::{
@@ -637,20 +637,20 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
637637
// still see the current value.
638638
if let Some(i32_slot) = ctx.i32_counter_slots.get(id).cloned() {
639639
let structurally_i32 = can_lower_expr_as_i32_in_current_region(ctx, value);
640-
// When `x` is proven numeric, `x | 0` may feed the canonical
640+
// When `x` is a canonical raw Number, `x | 0` may feed the canonical
641641
// i32 slot directly: materializing a double here only to
642642
// convert it back would duplicate the spec ToInt32. Keep the
643-
// numeric gate explicit — an untyped `x` can be a String, and
644-
// its `+` expression must preserve concatenation before `|0`.
643+
// canonical gate explicit — declared Number types are erased,
644+
// so a local can still hold a String or BigInt at runtime.
645645
let explicit_numeric_toint32 = matches!(
646-
value.as_ref(),
647-
Expr::Binary {
648-
op: BinaryOp::BitOr,
649-
left,
650-
right,
651-
..
652-
} if matches!(right.as_ref(), Expr::Integer(0))
653-
&& is_numeric_expr(ctx, left)
646+
value.as_ref(),
647+
Expr::Binary {
648+
op: BinaryOp::BitOr,
649+
left,
650+
right,
651+
..
652+
} if matches!(right.as_ref(), Expr::Integer(0))
653+
&& crate::type_analysis::expr_produces_canonical_raw_f64(ctx, left)
654654
);
655655
if !ctx.closure_captures.contains_key(id)
656656
&& !(ctx.boxed_vars.contains(id) && !ctx.module_globals.contains_key(id))

crates/perry-codegen/src/expr/mod.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,10 +1616,6 @@ pub(crate) struct StablePackedLoopFact {
16161616
/// an untagged IEEE Number. This is requested only when the indexed value
16171617
/// appears below a numeric operator in the cloned body.
16181618
pub numeric_elements: bool,
1619-
/// The clone was emitted against a preheader-cached receiver. Its entry is
1620-
/// enabled only after an emitted-IR scan proves the whole clone call-free,
1621-
/// so no allocation, collection, or proof-revoking runtime funnel can run.
1622-
pub preheader_stable: bool,
16231619
/// Preheader-derived numeric storage bases. Admission proved the complete
16241620
/// range is raw f64 and the call-free clone keeps these addresses stable.
16251621
pub numeric_access: Option<StablePackedNumericAccess>,

crates/perry-codegen/src/lower_call/method_override.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ const GC_TYPE_OBJECT: &str = "2";
2121
//
2222
// gtype == GC_TYPE_OBJECT
2323
// flags & GC_FLAG_FORWARDED == 0
24-
// reserved & OBJ_FLAG_HAS_DESCRIPTORS == 0
24+
// reserved & (OBJ_FLAG_HAS_DESCRIPTORS | OBJ_FLAG_PACKED_NUMERIC_PROOF) == 0
2525
//
26-
// Mask: 0x0800_0000 (descriptor bit) | 0x0000_8000 (forwarded bit) |
27-
// 0x0000_00ff (the complete gtype byte).
28-
const GC_OBJECT_METHOD_GUARD_MASK_I32: &str = "134250751"; // 0x0800_80ff
26+
// Mask: 0x0800_0000 (descriptor bit) | 0x0080_0000 (packed proof bit) |
27+
// 0x0000_8000 (forwarded bit) | 0x0000_00ff (the complete gtype byte).
28+
const GC_OBJECT_METHOD_GUARD_MASK_I32: &str = "142639359"; // 0x0880_80ff
2929
const SHAPE_ID_BASE_NEG_I32: &str = "-2147483648"; // subtract 0x8000_0000
3030
const SHAPE_ID_RANGE_LEN: &str = "1073741824"; // 0x4000_0000
3131

@@ -136,13 +136,15 @@ mod packed_guard_tests {
136136
let obj_type_mask = 0x0000_00ffu32;
137137
let forwarded = u32::from(0x80u8) << 8;
138138
let has_descriptors = 0x0800u32 << 16;
139-
let mask = obj_type_mask | forwarded | has_descriptors;
139+
let packed_numeric_proof = 0x0080u32 << 16;
140+
let mask = obj_type_mask | forwarded | has_descriptors | packed_numeric_proof;
140141
let expected = u32::from(2u8);
141142

142143
assert_eq!(GC_OBJECT_METHOD_GUARD_MASK_I32, mask.to_string());
143144
assert_eq!(expected & mask, expected);
144145
assert_ne!((expected | forwarded) & mask, expected);
145146
assert_ne!((expected | has_descriptors) & mask, expected);
147+
assert_ne!((expected | packed_numeric_proof) & mask, expected);
146148
assert_ne!((expected ^ 1) & mask, expected);
147149
}
148150

crates/perry-codegen/src/stmt/loops.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4965,11 +4965,11 @@ pub(super) fn lower_for_after_init_with_i32_bound(
49654965
// Saves ~25-30% on `for (let i = 0; i < arr.length; i++) arr[i] = i`
49664966
// and `for (let i = 0; i < arr.length; i++) for (let j = 0; j <
49674967
// arr.length; j++) ...` patterns.
4968-
let raw_hoist_classification: Option<LengthHoist> = if precomputed_i32_bound.is_some() {
4969-
None
4970-
} else {
4971-
condition.and_then(|cond| classify_for_length_hoist(ctx, cond, update, body))
4972-
};
4968+
// A precomputed bound replaces only the emitted length LOAD. Keep the
4969+
// structural classification: bounded-index facts, buffer-width facts and
4970+
// the counter's i32 slot are independent proofs consumed inside clones.
4971+
let raw_hoist_classification: Option<LengthHoist> =
4972+
condition.and_then(|cond| classify_for_length_hoist(ctx, cond, update, body));
49734973
let hoist_rejection = if raw_hoist_classification.is_none() && precomputed_i32_bound.is_none() {
49744974
condition.and_then(|cond| classify_for_length_hoist_rejection(ctx, cond, update, body))
49754975
} else {
@@ -5024,8 +5024,10 @@ pub(super) fn lower_for_after_init_with_i32_bound(
50245024
// i32 counter slot below are proofs and storage, not emitted work, and the
50255025
// clone's other lowering may depend on them; suppressing those too would
50265026
// trade one silent loss for another.
5027-
let in_call_free_clone =
5028-
!ctx.element_shape_loop_facts.is_empty() || !ctx.class_field_loop_facts.is_empty();
5027+
let in_call_free_clone = !ctx.element_shape_loop_facts.is_empty()
5028+
|| !ctx.class_field_loop_facts.is_empty()
5029+
|| !ctx.stable_packed_loop_facts.is_empty()
5030+
|| precomputed_i32_bound.is_some();
50295031
let hoisted_length_slot: Option<String> = if let Some(hoist) = hoist_classification {
50305032
let hoisted_slot = if in_call_free_clone {
50315033
None
@@ -5405,7 +5407,6 @@ pub(super) fn lower_for_after_init_with_i32_bound(
54055407

54065408
// Body block.
54075409
ctx.current_block = body_idx;
5408-
super::stable_packed_loop::emit_iteration_guard(ctx)?;
54095410
super::versioned_indexed_loop::emit_iteration_guard(ctx);
54105411
if let Some(cond) = condition {
54115412
let mut guarded =
@@ -5632,10 +5633,7 @@ pub(crate) fn emit_gc_loop_safepoint(
56325633
// scope is popped — keeps its poll either way.
56335634
if !ctx.element_shape_loop_facts.is_empty()
56345635
|| !ctx.class_field_loop_facts.is_empty()
5635-
|| ctx
5636-
.stable_packed_loop_facts
5637-
.last()
5638-
.is_some_and(|fact| fact.preheader_stable)
5636+
|| !ctx.stable_packed_loop_facts.is_empty()
56395637
{
56405638
return;
56415639
}

crates/perry-codegen/src/stmt/stable_packed_loop.rs

Lines changed: 5 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
//! Guarded loop versions for counted Array and Array-subclass iteration.
22
//!
33
//! A one-time runtime admission publishes scalar layout facts. The fast copy
4-
//! reloads the receiver root at iteration entry, compares compact header words,
5-
//! and performs the indexed load directly. Any failed proof resumes the
6-
//! unchanged generic loop at the current counter.
4+
//! is entered only after its emitted blocks are proven call-free, so its
5+
//! preheader-cached receiver and storage bases stay valid for the whole copy.
6+
//! Failed admission runs the unchanged generic loop from the current counter.
77
88
use anyhow::Result;
99
use perry_hir::{CompareOp, Expr, Stmt, UpdateOp};
1010

1111
use crate::expr::{FnCtx, StablePackedLoopFact, StablePackedNumericAccess};
1212
use crate::native_value::{BoundsState, BufferAccessMode, LoweredValue, MaterializationReason};
13-
use crate::types::{DOUBLE, I1, I32, I64, I8, PTR};
13+
use crate::types::{DOUBLE, I1, I32, I64, PTR};
1414

1515
#[derive(Clone, Copy)]
1616
enum LoopBound {
@@ -291,88 +291,6 @@ fn record_artifacts(ctx: &mut FnCtx<'_>, array_id: u32, receiver: &str) {
291291
);
292292
}
293293

294-
pub(super) fn emit_iteration_guard(ctx: &mut FnCtx<'_>) -> Result<bool> {
295-
let Some(fact) = ctx.stable_packed_loop_facts.last().cloned() else {
296-
return Ok(false);
297-
};
298-
if fact.preheader_stable {
299-
return Ok(true);
300-
}
301-
let receiver = crate::expr::lower_expr(ctx, &Expr::LocalGet(fact.array_local_id))?;
302-
let bits = ctx.block().bitcast_double_to_i64(&receiver);
303-
let raw = ctx.block().and(I64, &bits, crate::nanbox::POINTER_MASK_I64);
304-
let tag = ctx.block().lshr(I64, &bits, "48");
305-
let is_pointer = ctx.block().icmp_eq(I64, &tag, "32765");
306-
let floor =
307-
crate::target_layout::heap_addr_lower_bound_inclusive(ctx.target_triple).to_string();
308-
let ceiling =
309-
crate::target_layout::heap_addr_upper_bound_exclusive(ctx.target_triple).to_string();
310-
let above = ctx.block().icmp_uge(I64, &raw, &floor);
311-
let below = ctx.block().icmp_ult(I64, &raw, &ceiling);
312-
let in_heap = ctx.block().and(I1, &above, &below);
313-
let safe = ctx.block().and(I1, &is_pointer, &in_heap);
314-
315-
let deref_idx = ctx.new_block("stable_packed.iteration.deref");
316-
let plain_idx = ctx.new_block("stable_packed.iteration.plain");
317-
let object_idx = ctx.new_block("stable_packed.iteration.object");
318-
let fast_idx = ctx.new_block("stable_packed.iteration.fast");
319-
let deref_label = ctx.block_label(deref_idx);
320-
let plain_label = ctx.block_label(plain_idx);
321-
let object_label = ctx.block_label(object_idx);
322-
let fast_label = ctx.block_label(fast_idx);
323-
ctx.block()
324-
.cond_br(&safe, &deref_label, &fact.side_exit_label);
325-
326-
ctx.current_block = deref_idx;
327-
let gc_addr = ctx.block().sub(I64, &raw, "8");
328-
let gc_ptr = ctx.block().inttoptr(I64, &gc_addr);
329-
let live_gc = ctx.block().load_aligned(I64, &gc_ptr, 8);
330-
let receiver_ptr = ctx.block().inttoptr(I64, &raw);
331-
let live_header = ctx.block().load(I64, &receiver_ptr);
332-
let expected_gc = descriptor_word(ctx, &fact.descriptor, 1);
333-
let expected_header = descriptor_word(ctx, &fact.descriptor, 2);
334-
let gc_ok = ctx.block().icmp_eq(I64, &live_gc, &expected_gc);
335-
let header_ok = ctx.block().icmp_eq(I64, &live_header, &expected_header);
336-
let header_words_ok = ctx.block().and(I1, &gc_ok, &header_ok);
337-
let kind = descriptor_word(ctx, &fact.descriptor, 0);
338-
let is_plain = ctx.block().icmp_eq(I64, &kind, "1");
339-
let header_and_plain = ctx.block().and(I1, &header_words_ok, &is_plain);
340-
ctx.block()
341-
.cond_br(&header_and_plain, &plain_label, &object_label);
342-
343-
ctx.current_block = plain_idx;
344-
let invalidated = ctx
345-
.block()
346-
.load_volatile(I8, "@PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED");
347-
let prototype_ok = ctx.block().icmp_eq(I8, &invalidated, "0");
348-
ctx.block()
349-
.cond_br(&prototype_ok, &fast_label, &fact.side_exit_label);
350-
351-
ctx.current_block = object_idx;
352-
let is_object = ctx.block().icmp_eq(I64, &kind, "2");
353-
let mut object_ok = ctx.block().and(I1, &header_words_ok, &is_object);
354-
let length_slot = descriptor_word(ctx, &fact.descriptor, 3);
355-
let object_header_size =
356-
crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string();
357-
let length_bytes = ctx.block().shl(I64, &length_slot, "3");
358-
let length_offset = ctx.block().add(I64, &length_bytes, &object_header_size);
359-
let length_addr = ctx.block().add(I64, &raw, &length_offset);
360-
let length_ptr = ctx.block().inttoptr(I64, &length_addr);
361-
let live_length = ctx.block().load(DOUBLE, &length_ptr);
362-
let bound64 = descriptor_word(ctx, &fact.descriptor, 6);
363-
let bound = ctx.block().uitofp(I64, &bound64, DOUBLE);
364-
let length_covers_bound = ctx.block().fcmp("oge", &live_length, &bound);
365-
object_ok = ctx.block().and(I1, &object_ok, &length_covers_bound);
366-
ctx.block()
367-
.cond_br(&object_ok, &fast_label, &fact.side_exit_label);
368-
369-
ctx.current_block = fast_idx;
370-
if let Some(active) = ctx.stable_packed_loop_facts.last_mut() {
371-
active.live_receiver_handle = Some(raw);
372-
}
373-
Ok(true)
374-
}
375-
376294
pub(crate) fn try_lower_index_get(
377295
ctx: &mut FnCtx<'_>,
378296
object: &Expr,
@@ -540,8 +458,7 @@ pub(crate) fn has_numeric_index_fact(ctx: &FnCtx<'_>, expr: &Expr) -> bool {
540458
return false;
541459
};
542460
ctx.stable_packed_loop_facts.iter().rev().any(|fact| {
543-
fact.preheader_stable
544-
&& fact.numeric_elements
461+
fact.numeric_elements
545462
&& fact.array_local_id == *array_id
546463
&& fact.counter_local_id == *counter_id
547464
})
@@ -708,7 +625,6 @@ pub(super) fn lower(
708625
descriptor,
709626
live_receiver_handle: Some(fast_raw),
710627
numeric_elements: candidate.numeric_elements,
711-
preheader_stable: true,
712628
numeric_access,
713629
});
714630
super::loops::lower_for_after_init_with_i32_bound(

crates/perry-codegen/src/stmt/versioned_indexed_loop.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ pub(super) fn emit_iteration_guard(ctx: &mut FnCtx<'_>) -> bool {
351351
let object_ptr = ctx.block().inttoptr(I64, &this_handle);
352352
let gc_header_ptr = ctx.block().gep(I8, &object_ptr, &[(I64, "-8")]);
353353
let gc_header = ctx.block().load(I32, &gc_header_ptr);
354-
let guarded_gc_bits = ctx.block().and(I32, &gc_header, "134250751");
354+
let guarded_gc_bits = ctx.block().and(I32, &gc_header, "142639359");
355355
let gc_ok = ctx.block().icmp_eq(I32, &guarded_gc_bits, "2");
356356
let class_shape = ctx.block().load(I64, &object_ptr);
357357
let expected_shape_i64 = ctx.block().zext(I32, &fact.method.expected_shape_id, I64);
@@ -439,11 +439,9 @@ pub(super) fn lower(
439439
.get(position + 1)
440440
.map(String::as_str)
441441
.unwrap_or(method_entry_label.as_str());
442-
let Some((local_slot, expected_fingerprint)) =
442+
let (local_slot, expected_fingerprint) =
443443
emit_array_admission(ctx, local_id, &bound_i32, next, &slow_pre_label)
444-
else {
445-
return Ok(false);
446-
};
444+
.expect("matched array local has storage");
447445
array_facts.push(VersionedIndexedArrayFact {
448446
local_id,
449447
local_slot,

0 commit comments

Comments
 (0)