Skip to content

Commit 0f0d3ef

Browse files
proggeramlugRalph Küpper
andauthored
fix(codegen): verify cached typed-array pointer lifetime (#7849)
* fix(codegen): verify cached typed-array pointer lifetime * docs(changelog): record buffer-view verifier fix * fix(codegen): close buffer-view pointer review gaps * fix(codegen): gate cached-view native access * fix(codegen): invalidate named typed-array gets --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 72d0cee commit 0f0d3ef

23 files changed

Lines changed: 600 additions & 46 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
## Fixed
2+
3+
- Native-region verification now tracks cached buffer-view pointer lifetime
4+
independently of bounds and alias facts. Accessing a typed array's `.buffer`
5+
directly or through a computed or named accessor invalidates every copied
6+
view alias, runtime fallbacks retain that evidence, and checked or unchecked
7+
native access through the invalidated pointer is rejected. Statically proven
8+
canonical numeric string keys remain on the non-invalidating element path.
9+
Scalar accesses through cached views must carry the same pointer-lifetime
10+
evidence (#7220).
11+
12+
Targeted verifier and native-proof regressions cover direct, computed,
13+
copied-alias, scalar, and bulk-memory access after pointer invalidation.

crates/perry-codegen/src/codegen/function.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use perry_hir::Function;
99

1010
use crate::expr::FnCtx;
1111
use crate::module::LlModule;
12-
use crate::native_value::{AliasState, BufferElem, BufferIndexUnit, BufferViewSlot, LengthSource};
12+
use crate::native_value::{
13+
AliasState, BufferElem, BufferIndexUnit, BufferViewPointerState, BufferViewSlot, LengthSource,
14+
};
1315
use crate::stmt;
1416
use crate::strings::StringPool;
1517
use crate::types::{LlvmType, DOUBLE, I1, I32, I64, I8, PTR};
@@ -957,6 +959,7 @@ pub(super) fn compile_function(
957959
alias: AliasState::Unknown,
958960
length_source: Some(LengthSource::Unknown),
959961
native_owned: None,
962+
pointer_state: BufferViewPointerState::Stable,
960963
// Declared-type hoist only — the construction form is unknown,
961964
// so no inline-storage proof.
962965
storage_inline_proven: false,
@@ -1026,6 +1029,7 @@ pub(super) fn compile_function(
10261029
None => LengthSource::Unknown,
10271030
}),
10281031
native_owned: None,
1032+
pointer_state: BufferViewPointerState::Stable,
10291033
storage_inline_proven: true,
10301034
},
10311035
);

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::native_value::{
88
use crate::types::{DOUBLE, F32, I16, I32, I8, PTR};
99

1010
use super::{
11-
attach_native_owned_view_fact, bounds_for_buffer_access_width, buffer_alias_metadata_suffix,
11+
attach_buffer_view_facts, bounds_for_buffer_access_width, buffer_alias_metadata_suffix,
1212
buffer_view_lowered_value, can_lower_expr_as_i32, effective_alias_state_for_access,
1313
int_range_expr, is_numeric_expr, lower_expr_native, FnCtx,
1414
};
@@ -248,6 +248,9 @@ pub(crate) fn lower_buffer_access_proof(
248248
},
249249
_ => return Ok(None),
250250
};
251+
if !view.pointer_state.is_stable() {
252+
return Ok(None);
253+
}
251254

252255
// A closure-captured buffer local is hazardous even before any escape
253256
// walk stamped `buffer_hazard_reasons` — the closure may mutate/realloc
@@ -386,7 +389,7 @@ fn record_buffer_view(
386389
proof.may_emit_noalias,
387390
vec![format!("elem={:?}", proof.view.elem)],
388391
);
389-
attach_native_owned_view_fact(ctx, &proof.view);
392+
attach_buffer_view_facts(ctx, &proof.view);
390393
}
391394

392395
pub(crate) fn lower_buffer_load(
@@ -423,7 +426,7 @@ pub(crate) fn lower_buffer_load(
423426
proof.may_emit_noalias,
424427
vec![format!("zext_to={}", result_i32)],
425428
);
426-
attach_native_owned_view_fact(ctx, &proof.view);
429+
attach_buffer_view_facts(ctx, &proof.view);
427430
let result = LoweredValue::i32(result_i32);
428431
if let Some(consumer) = spec.result_consumer {
429432
let facts = access_facts_for_spec(spec, &proof.view, Some(&emission.len_i32));
@@ -442,7 +445,7 @@ pub(crate) fn lower_buffer_load(
442445
false,
443446
Vec::new(),
444447
);
445-
attach_native_owned_view_fact(ctx, &proof.view);
448+
attach_buffer_view_facts(ctx, &proof.view);
446449
}
447450
Ok(Some(result))
448451
}
@@ -482,7 +485,7 @@ pub(crate) fn lower_buffer_store(
482485
proof.may_emit_noalias,
483486
vec![format!("source_i32={}", val_i32)],
484487
);
485-
attach_native_owned_view_fact(ctx, &proof.view);
488+
attach_buffer_view_facts(ctx, &proof.view);
486489
let result = LoweredValue::i32(val_i32.clone());
487490
Ok(Some(StoreResult { result }))
488491
}
@@ -612,7 +615,7 @@ pub(crate) fn lower_typed_array_load(
612615
proof.may_emit_noalias,
613616
vec![format!("elem={:?}", proof.view.elem)],
614617
);
615-
attach_native_owned_view_fact(ctx, &proof.view);
618+
attach_buffer_view_facts(ctx, &proof.view);
616619
Ok(Some(result))
617620
}
618621

@@ -765,6 +768,6 @@ pub(crate) fn lower_typed_array_store(
765768
proof.may_emit_noalias,
766769
vec![format!("elem={:?}", proof.view.elem)],
767770
);
768-
attach_native_owned_view_fact(ctx, &proof.view);
771+
attach_buffer_view_facts(ctx, &proof.view);
769772
Ok(Some(StoreResult { result }))
770773
}

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

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use perry_hir::{walker::walk_expr_children, Expr};
22

33
use crate::native_value::{
4-
AliasState, BoundsState, BufferElem, BufferIndexUnit, BufferViewSlot, LengthSource,
5-
LoweredValue, MaterializationReason, NativeOwnedViewFact,
4+
AliasState, BoundsState, BufferElem, BufferIndexUnit, BufferViewPointerState, BufferViewSlot,
5+
LengthSource, LoweredValue, MaterializationReason, NativeOwnedViewFact,
66
};
77
use crate::types::{I32, I64, I8, PTR};
88

@@ -111,6 +111,36 @@ pub(crate) fn downgrade_buffer_alias(ctx: &mut FnCtx<'_>, id: u32, reason: Mater
111111
);
112112
}
113113

114+
/// Mark a cached data pointer as unusable after an operation changes which
115+
/// storage the receiver aliases. Alias state alone cannot express this: the
116+
/// pointer is stale, not merely shared.
117+
pub(crate) fn invalidate_buffer_view_pointer(
118+
ctx: &mut FnCtx<'_>,
119+
id: u32,
120+
reason: MaterializationReason,
121+
) {
122+
let affected_ids = if let Some(data_slot) = ctx
123+
.buffer_view_slots
124+
.get(&id)
125+
.map(|view| view.data_slot.clone())
126+
{
127+
ctx.buffer_view_slots
128+
.iter()
129+
.filter_map(|(view_id, view)| (view.data_slot == data_slot).then_some(*view_id))
130+
.collect::<Vec<_>>()
131+
} else {
132+
vec![id]
133+
};
134+
for affected_id in affected_ids {
135+
if let Some(view) = ctx.buffer_view_slots.get_mut(&affected_id) {
136+
view.pointer_state = BufferViewPointerState::Invalidated {
137+
reason: reason.clone(),
138+
};
139+
}
140+
downgrade_buffer_alias(ctx, affected_id, reason.clone());
141+
}
142+
}
143+
114144
fn owner_alias_invalidation_reason(reason: &MaterializationReason) -> MaterializationReason {
115145
match reason {
116146
MaterializationReason::UnknownCallEscape => MaterializationReason::MissingOwnerRoot,
@@ -233,12 +263,27 @@ pub(crate) fn native_owned_fact_for_view(view: &BufferViewSlot) -> Option<Native
233263
.map(|native| native.fact(view.element_width_bytes, alias_group))
234264
}
235265

236-
pub(crate) fn attach_native_owned_view_fact(ctx: &mut FnCtx<'_>, view: &BufferViewSlot) {
237-
let Some(fact) = native_owned_fact_for_view(view) else {
266+
pub(crate) fn attach_buffer_view_facts(ctx: &mut FnCtx<'_>, view: &BufferViewSlot) {
267+
if let Some(record) = ctx.native_rep_records.last_mut() {
268+
record.buffer_view_pointer_state = Some(view.pointer_state.clone());
269+
record.native_owned_view = native_owned_fact_for_view(view);
270+
}
271+
}
272+
273+
pub(crate) fn attach_buffer_view_pointer_state_for_expr(ctx: &mut FnCtx<'_>, expr: &Expr) {
274+
let Expr::LocalGet(id) = expr else {
275+
return;
276+
};
277+
let Some(state) = ctx
278+
.buffer_view_slots
279+
.get(id)
280+
.map(|view| view.pointer_state.clone())
281+
else {
238282
return;
239283
};
240284
if let Some(record) = ctx.native_rep_records.last_mut() {
241-
record.native_owned_view = Some(fact);
285+
record.local_id = Some(*id);
286+
record.buffer_view_pointer_state = Some(state);
242287
}
243288
}
244289

@@ -285,6 +330,7 @@ pub(crate) fn update_buffer_view_for_assignment(
285330
alias: AliasState::MayAlias,
286331
length_source: Some(LengthSource::Unknown),
287332
native_owned: None,
333+
pointer_state: BufferViewPointerState::Stable,
288334
// Reassignment refresh: `Uint8ArrayNew` with a non-literal arg
289335
// can be the view form (`new Uint8Array(buffer)`), so the
290336
// inline-storage proof is not re-established here.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,8 @@ fn ta_int_elem_load_is_i32_provable(ctx: &FnCtx<'_>, object: &Expr, index: &Expr
583583
let Some(view) = ctx.buffer_view_slots.get(id) else {
584584
return false;
585585
};
586-
if view.index_unit != BufferIndexUnit::Element
586+
if !view.pointer_state.is_stable()
587+
|| view.index_unit != BufferIndexUnit::Element
587588
|| !view.alias.allows_noalias()
588589
|| view.scope_idx.is_none()
589590
{

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

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,13 @@ use crate::type_analysis::{is_array_expr, is_numeric_expr, is_string_expr, recei
3131
use crate::types::{DOUBLE, I1, I16, I32, I64, I8};
3232

3333
use super::{
34-
array_kind_fact, buffer_access_materialization_reason, emit_typed_feedback_register_site,
35-
expr_has_numeric_pointer_free_array_layout, int_range_expr, lower_buffer_load, lower_expr,
36-
lower_expr_as_i32, lower_typed_array_load, materialize_js_value, raw_f64_layout_fact,
37-
try_lower_flat_const_index_get, typed_feedback_emission_enabled, unbox_str_handle,
38-
unbox_to_i64, BufferAccessSpec, FnCtx, PackedF64LoopFact, TypedFeedbackContract,
39-
TypedFeedbackKind,
34+
array_kind_fact, attach_buffer_view_pointer_state_for_expr,
35+
buffer_access_materialization_reason, emit_typed_feedback_register_site,
36+
expr_has_numeric_pointer_free_array_layout, int_range_expr, invalidate_buffer_view_pointer,
37+
lower_buffer_load, lower_expr, lower_expr_as_i32, lower_typed_array_load, materialize_js_value,
38+
raw_f64_layout_fact, try_lower_flat_const_index_get, typed_feedback_emission_enabled,
39+
unbox_str_handle, unbox_to_i64, BufferAccessSpec, FnCtx, PackedF64LoopFact,
40+
TypedFeedbackContract, TypedFeedbackKind,
4041
};
4142

4243
mod guarded_array;
@@ -217,6 +218,40 @@ fn typed_array_index_needs_runtime_key(ctx: &FnCtx<'_>, object: &Expr, index: &E
217218
&& !numeric_index_has_loop_array_index_proof(ctx, object, index)
218219
}
219220

221+
fn is_proven_canonical_numeric_string_literal(key: &[u8]) -> bool {
222+
if matches!(key, b"-0" | b"NaN" | b"Infinity" | b"-Infinity") {
223+
return true;
224+
}
225+
226+
let digits = key.strip_prefix(b"-").unwrap_or(key);
227+
if digits.is_empty()
228+
|| (digits.len() > 1 && digits[0] == b'0')
229+
|| !digits.iter().all(u8::is_ascii_digit)
230+
{
231+
return false;
232+
}
233+
234+
// Decimal integers through Number.MAX_SAFE_INTEGER are exact, and this
235+
// range is below the threshold where JS Number#toString switches to
236+
// exponent notation. Their source spelling therefore proves
237+
// CanonicalNumericIndexString without invoking runtime conversion.
238+
digits
239+
.iter()
240+
.try_fold(0_u64, |value, digit| {
241+
value.checked_mul(10)?.checked_add(u64::from(digit - b'0'))
242+
})
243+
.is_some_and(|value| value <= 9_007_199_254_740_991)
244+
}
245+
246+
fn runtime_key_may_expose_typed_array_backing_buffer(index: &Expr) -> bool {
247+
match index {
248+
Expr::String(key) => !is_proven_canonical_numeric_string_literal(key.as_bytes()),
249+
Expr::WtfString(key) => !is_proven_canonical_numeric_string_literal(key),
250+
Expr::Integer(_) | Expr::Number(_) => false,
251+
_ => true,
252+
}
253+
}
254+
220255
fn lower_array_index_get_via_runtime_key(
221256
ctx: &mut FnCtx<'_>,
222257
arr_box: &str,
@@ -825,6 +860,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
825860
return Ok(v);
826861
}
827862
if typed_array_index_needs_runtime_key(ctx, object.as_ref(), index.as_ref()) {
863+
if runtime_key_may_expose_typed_array_backing_buffer(index) {
864+
if let Expr::LocalGet(id) = object.as_ref() {
865+
if ctx.buffer_view_slots.contains_key(id) {
866+
invalidate_buffer_view_pointer(
867+
ctx,
868+
*id,
869+
MaterializationReason::MutableAlias,
870+
);
871+
}
872+
}
873+
}
828874
let arr_box = lower_expr(ctx, object)?;
829875
let key_box = lower_expr(ctx, index)?;
830876
let blk = ctx.block();
@@ -849,6 +895,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
849895
false,
850896
vec!["typed_array_fallback=untracked_or_unproven".to_string()],
851897
);
898+
attach_buffer_view_pointer_state_for_expr(ctx, object);
852899
return Ok(result);
853900
}
854901

@@ -893,6 +940,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
893940
false,
894941
vec!["typed_array_fallback=untracked_or_unproven".to_string()],
895942
);
943+
attach_buffer_view_pointer_state_for_expr(ctx, object);
896944
return Ok(result);
897945
}
898946
if is_uint8array_receiver(ctx, object) && is_numeric_expr(ctx, index) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ use crate::types::{DOUBLE, I32, I64};
4242
use super::index_set_typed_array::lower_inline_dyn_typed_array_set;
4343
use super::{
4444
array_kind_fact, array_store_needs_layout_note, array_store_needs_write_barrier,
45-
buffer_access_materialization_reason, emit_array_numeric_write_note_on_block,
46-
emit_jsvalue_slot_store_on_block, emit_root_nanbox_store_on_block,
47-
emit_typed_feedback_register_site, emit_write_barrier,
45+
attach_buffer_view_pointer_state_for_expr, buffer_access_materialization_reason,
46+
emit_array_numeric_write_note_on_block, emit_jsvalue_slot_store_on_block,
47+
emit_root_nanbox_store_on_block, emit_typed_feedback_register_site, emit_write_barrier,
4848
expr_has_numeric_pointer_free_array_layout, int_range_expr, lower_buffer_store, lower_expr,
4949
lower_expr_as_i32, lower_expr_native, lower_index_set_fast, lower_typed_array_store,
5050
materialize_js_value, nanbox_pointer_inline, raw_f64_layout_fact, unbox_str_handle,
@@ -881,6 +881,7 @@ pub(crate) fn lower(
881881
false,
882882
vec!["typed_array_fallback=untracked_or_unproven".to_string()],
883883
);
884+
attach_buffer_view_pointer_state_for_expr(ctx, object);
884885
return Ok(result);
885886
}
886887

@@ -911,6 +912,7 @@ pub(crate) fn lower(
911912
false,
912913
vec!["typed_array_fallback=untracked_or_unproven".to_string()],
913914
);
915+
attach_buffer_view_pointer_state_for_expr(ctx, object);
914916
return Ok(val_double);
915917
}
916918
if is_uint8array_receiver(ctx, object) && is_numeric_expr(ctx, index) {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ pub(crate) use buffer_access::{
6363
lower_typed_array_store, BufferAccessSpec,
6464
};
6565
pub(crate) use buffer_views::{
66-
alias_buffer_view_slot, attach_native_owned_view_fact, buffer_access_materialization_reason,
67-
buffer_view_lowered_value, downgrade_buffer_alias, downgrade_buffer_aliases_in_expr,
66+
alias_buffer_view_slot, attach_buffer_view_facts, attach_buffer_view_pointer_state_for_expr,
67+
buffer_access_materialization_reason, buffer_view_lowered_value, downgrade_buffer_alias,
68+
downgrade_buffer_aliases_in_expr, invalidate_buffer_view_pointer,
6869
invalidate_native_owned_views_for_dispose, native_arena_canonical_owner_id,
6970
record_native_arena_owner_assignment, update_buffer_view_for_assignment,
7071
};

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::native_value::{
99
use crate::types::{DOUBLE, I1, I32, I64, I8, PTR};
1010

1111
use super::{
12-
attach_native_owned_view_fact, buffer_access_materialization_reason, buffer_view_lowered_value,
12+
attach_buffer_view_facts, buffer_access_materialization_reason, buffer_view_lowered_value,
1313
effective_alias_state_for_access, lower_expr, lower_expr_native, unbox_to_i64, FnCtx,
1414
};
1515

@@ -180,6 +180,9 @@ fn proven_view(
180180
return None;
181181
}
182182
let slot = ctx.buffer_view_slots.get(local_id)?.clone();
183+
if !slot.pointer_state.is_stable() {
184+
return None;
185+
}
183186
if slot.index_unit != BufferIndexUnit::Element {
184187
return None;
185188
}
@@ -290,7 +293,7 @@ fn record_bulk_view(
290293
false,
291294
Vec::new(),
292295
);
293-
attach_native_owned_view_fact(ctx, view);
296+
attach_buffer_view_facts(ctx, view);
294297
}
295298

296299
fn record_runtime_fallback(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
9595
if property == "buffer" {
9696
if let Expr::LocalGet(id) = object.as_ref() {
9797
if ctx.buffer_view_slots.contains_key(id) {
98-
super::downgrade_buffer_alias(
98+
super::invalidate_buffer_view_pointer(
9999
ctx,
100100
*id,
101101
crate::native_value::MaterializationReason::MutableAlias,

0 commit comments

Comments
 (0)