From 9f8cd38a809098461f9d7943f7c6181a8fdd1f32 Mon Sep 17 00:00:00 2001 From: Eytan Singher Date: Sun, 12 Jul 2026 16:03:02 +0300 Subject: [PATCH] (bug fix): prune usage after change promotion in loop usage finalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A loop/closure that reads an ancestor as a snapshot (`@a.b`) while mutating a nested field (`a.b.c`) silently dropped the mutation, returning a stale value (e.g. 1 instead of 4). In `Usage::finalize_as_scope`, when a changed child has an ancestor that is read as a snapshot, the ancestor is promoted into `usage`/`changes` so the loop reads and writes back the whole ancestor. The usage-pruning pass ran *before* the promotion, so the promoted ancestor's descendants survived in `usage`, violating the antichain invariant. Downstream the loop signature then listed both the ancestor and its descendants, and the reconstruction of the returned ancestor overwrote the mutated child with the stale snapshot input. Fix: reorder the finalization passes — snap_usage, then changes (promotion), then usage — so the usage pass prunes descendants subsumed by any promoted ancestor. Adds an e2e sanity repro. --- crates/cairo-lang-semantic/src/usage/mod.rs | 36 +-- tests/e2e_test_data/libfuncs/casm_run_sanity | 265 +++++++++++++++++++ 2 files changed, 285 insertions(+), 16 deletions(-) diff --git a/crates/cairo-lang-semantic/src/usage/mod.rs b/crates/cairo-lang-semantic/src/usage/mod.rs index 03cba61a2bd..466a8e5202a 100644 --- a/crates/cairo-lang-semantic/src/usage/mod.rs +++ b/crates/cairo-lang-semantic/src/usage/mod.rs @@ -79,21 +79,8 @@ impl<'db> Usage<'db> { /// Removes usage that was introduced current block and usage that is already covered /// by containing variables. pub fn finalize_as_scope(&mut self) { - // Prune introductions from usages. - for member_path in prune_and_get_candidates(&mut self.usage, |k| { - self.introductions.contains(&k.base_var()) - }) { - // Prune usages that are members of other usages. - let mut current_path = &member_path; - while let MemberPath::Member { parent, .. } = current_path { - current_path = parent.as_ref(); - if self.usage.contains_key(current_path) { - self.usage.swap_remove(&member_path); - break; - } - } - } - // Prune usages and introdictions from snap_usage. + // Prune usages and introdictions from snap_usage. Runs before the `changes` pass so that + // the promotion below always promotes the top-most snapshotted ancestor. for member_path in prune_and_get_candidates(&mut self.snap_usage, |k| { self.usage.contains_key(k) || self.introductions.contains(&k.base_var()) }) { @@ -114,7 +101,9 @@ impl<'db> Usage<'db> { self.introductions.contains(&k.base_var()) }) { // Prune changes that are members of other changes. - // Also if a child is changed and its parent is used, then we change the parent. + // Also if a child is changed and an ancestor is used as a snapshot, promote the + // ancestor into `usage`/`changes` so the whole ancestor is read and written back, + // instead of returning the child of a stale snapshot. // TODO(TomerStarkware): Deconstruct the parent, and snap_use other members. let mut current_path = &member_path; while let MemberPath::Member { parent, .. } = current_path { @@ -133,6 +122,21 @@ impl<'db> Usage<'db> { } } } + // Prune introductions from usages. Runs after the `changes` pass so that members of an + // ancestor promoted above are pruned as well. + for member_path in prune_and_get_candidates(&mut self.usage, |k| { + self.introductions.contains(&k.base_var()) + }) { + // Prune usages that are members of other usages. + let mut current_path = &member_path; + while let MemberPath::Member { parent, .. } = current_path { + current_path = parent.as_ref(); + if self.usage.contains_key(current_path) { + self.usage.swap_remove(&member_path); + break; + } + } + } } } diff --git a/tests/e2e_test_data/libfuncs/casm_run_sanity b/tests/e2e_test_data/libfuncs/casm_run_sanity index bee5fa0d5ff..b05c3ff1b51 100644 --- a/tests/e2e_test_data/libfuncs/casm_run_sanity +++ b/tests/e2e_test_data/libfuncs/casm_run_sanity @@ -448,3 +448,268 @@ test::fib@F0([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252) -> (RangeCheck, Gas test::fib[116-219]@F1([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252, [3]: felt252, [4]: felt252, [5]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(core::felt252, core::felt252, core::felt252, ())>); core::panic_with_const_felt252::<375233589013918064796019>@F2() -> (Tuple>); core::panic_with_felt252@F3([0]: felt252) -> (Tuple>); + +//! > ========================================================================== + +//! > Loop that reads an ancestor as a snapshot (@a.b) while also mutating a nested field (a.b.c) + +//! > must preserve the mutation. When `finalize_as_scope` promotes the snapped ancestor into + +//! > usage/changes, the later usage-pruning pass subsumes its descendants so the loop reads and writes back the whole + +//! > ancestor. For x=2 the result is 1*2*2=4 (0x4). + +//! > test_runner_name +SmallE2ETestRunner(test_data_function: loop_snap_mut, test_data_input: 2, test_data_output: 4) + +//! > cairo_code +#[derive(Drop)] +struct Inner { + c: felt252, +} +#[derive(Drop)] +struct Outer { + b: Inner, +} + +fn loop_snap_mut(x: felt252) -> felt252 { + let mut a = Outer { b: Inner { c: 1 } }; + let mut i: felt252 = 0; + loop { + if i == x { + break; + } + let _snap = @a.b; + a.b.c = a.b.c * 2; + i = i + 1; + } + a.b.c +} + +//! > casm +[ap + 0] = [fp + -5], ap++; +[ap + 0] = [fp + -4], ap++; +[ap + 0] = 1, ap++; +[ap + 0] = 0, ap++; +[ap + 0] = [fp + -3], ap++; +call rel 19; +jmp rel 10 if [ap + -3] != 0; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = 0, ap++; +[ap + 0] = 0, ap++; +[ap + 0] = [ap + -6], ap++; +ret; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = 1, ap++; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = [ap + -5], ap++; +ret; +%{ memory[ap + 0] = 1270 <= memory[fp + -6] %} +jmp rel 7 if [ap + 0] != 0, ap++; +[ap + 0] = [fp + -6] + 340282366920938463463374607431768210186, ap++; +[ap + -1] = [[fp + -7] + 0]; +jmp rel 28; +[fp + -6] = [ap + 0] + 1270, ap++; +[ap + -1] = [[fp + -7] + 0]; +[fp + -4] = [ap + 0] + [fp + -3], ap++; +jmp rel 11 if [ap + -1] != 0; +[ap + 0] = [fp + -7] + 1, ap++; +[ap + 0] = [ap + -3] + 2070, ap++; +[ap + 0] = 0, ap++; +[ap + 0] = [fp + -5], ap++; +[ap + 0] = [fp + -4], ap++; +ret; +[ap + 0] = [fp + -7] + 1, ap++; +[ap + 0] = [ap + -3], ap++; +[ap + 0] = [fp + -5] * 2, ap++; +[ap + 0] = [fp + -4] + 1, ap++; +[ap + 0] = [fp + -3], ap++; +call rel -30; +ret; +call rel 10; +[ap + 0] = [fp + -7] + 1, ap++; +[ap + 0] = [fp + -6], ap++; +[ap + 0] = 1, ap++; +[ap + 0] = [ap + -5], ap++; +[ap + 0] = [ap + -5], ap++; +ret; +[ap + 0] = 375233589013918064796019, ap++; +call rel 3; +ret; +%{ memory[ap + 0] = segments.add() %} +ap += 1; +[fp + -3] = [[ap + -1] + 0]; +[ap + 0] = [ap + -1], ap++; +[ap + 0] = [ap + -2] + 1, ap++; +ret; + +//! > function_costs +test::loop_snap_mut: SmallOrderedMap({Const: 3170}) +test::loop_snap_mut[116-254]: SmallOrderedMap({Const: 1870}) +core::panic_with_const_felt252::<375233589013918064796019>: SmallOrderedMap({Const: 700}) +core::panic_with_felt252: SmallOrderedMap({Const: 400}) + +//! > sierra_code +type felt252 = felt252 [storable: true, drop: true, dup: true, zero_sized: false]; +type core::panics::Panic = Struct [storable: true, drop: true, dup: true, zero_sized: true]; +type Array = Array [storable: true, drop: true, dup: false, zero_sized: false]; +type Const = Const [storable: false, drop: false, dup: false, zero_sized: false]; +type Const = Const [storable: false, drop: false, dup: false, zero_sized: false]; +type NonZero = NonZero [storable: true, drop: true, dup: true, zero_sized: false]; +type Tuple = Struct [storable: true, drop: true, dup: true, zero_sized: false]; +type Tuple> = Struct> [storable: true, drop: true, dup: false, zero_sized: false]; +type core::panics::PanicResult::<(core::felt252,)> = Enum, Tuple, Tuple>> [storable: true, drop: true, dup: false, zero_sized: false]; +type Unit = Struct [storable: true, drop: true, dup: true, zero_sized: true]; +type test::Inner = Struct [storable: true, drop: true, dup: true, zero_sized: false]; +type Tuple = Struct [storable: true, drop: true, dup: true, zero_sized: false]; +type core::panics::PanicResult::<(test::Inner, core::felt252, ())> = Enum, Tuple, Tuple>> [storable: true, drop: true, dup: false, zero_sized: false]; +type GasBuiltin = GasBuiltin [storable: true, drop: false, dup: false, zero_sized: false]; +type RangeCheck = RangeCheck [storable: true, drop: false, dup: false, zero_sized: false]; +type Const = Const [storable: false, drop: false, dup: false, zero_sized: false]; +type Const = Const [storable: false, drop: false, dup: false, zero_sized: false]; + +libfunc disable_ap_tracking = disable_ap_tracking; +libfunc const_as_immediate> = const_as_immediate>; +libfunc const_as_immediate> = const_as_immediate>; +libfunc snapshot_take = snapshot_take; +libfunc drop = drop; +libfunc struct_construct = struct_construct; +libfunc store_temp = store_temp; +libfunc store_temp = store_temp; +libfunc store_temp = store_temp; +libfunc store_temp = store_temp; +libfunc function_call = function_call; +libfunc enum_match> = enum_match>; +libfunc branch_align = branch_align; +libfunc redeposit_gas = redeposit_gas; +libfunc struct_deconstruct> = struct_deconstruct>; +libfunc drop = drop; +libfunc struct_deconstruct = struct_deconstruct; +libfunc struct_construct> = struct_construct>; +libfunc enum_init, 0> = enum_init, 0>; +libfunc store_temp> = store_temp>; +libfunc enum_init, 1> = enum_init, 1>; +libfunc withdraw_gas = withdraw_gas; +libfunc dup = dup; +libfunc rename = rename; +libfunc felt252_sub = felt252_sub; +libfunc felt252_is_zero = felt252_is_zero; +libfunc struct_construct = struct_construct; +libfunc struct_construct> = struct_construct>; +libfunc enum_init, 0> = enum_init, 0>; +libfunc store_temp> = store_temp>; +libfunc drop> = drop>; +libfunc const_as_immediate> = const_as_immediate>; +libfunc felt252_mul = felt252_mul; +libfunc felt252_add = felt252_add; +libfunc drop = drop; +libfunc function_call> = function_call>; +libfunc enum_init, 1> = enum_init, 1>; +libfunc const_as_immediate> = const_as_immediate>; +libfunc function_call = function_call; +libfunc array_new = array_new; +libfunc array_append = array_append; +libfunc struct_construct = struct_construct; +libfunc struct_construct>> = struct_construct>>; +libfunc store_temp>> = store_temp>>; + +F0: +disable_ap_tracking() -> (); +const_as_immediate>() -> ([3]); +const_as_immediate>() -> ([4]); +snapshot_take([2]) -> ([5], [6]); +drop([5]) -> (); +struct_construct([3]) -> ([7]); +store_temp([0]) -> ([0]); +store_temp([1]) -> ([1]); +store_temp([7]) -> ([7]); +store_temp([4]) -> ([4]); +store_temp([6]) -> ([6]); +function_call([0], [1], [7], [4], [6]) -> ([8], [9], [10]); +enum_match>([10]) { fallthrough([11]) F0_B0([12]) }; +branch_align() -> (); +redeposit_gas([9]) -> ([13]); +struct_deconstruct>([11]) -> ([14], [15], [16]); +drop([15]) -> (); +drop([16]) -> (); +struct_deconstruct([14]) -> ([17]); +struct_construct>([17]) -> ([18]); +enum_init, 0>([18]) -> ([19]); +store_temp([8]) -> ([8]); +store_temp([13]) -> ([13]); +store_temp>([19]) -> ([19]); +return([8], [13], [19]); +F0_B0: +branch_align() -> (); +enum_init, 1>([12]) -> ([20]); +store_temp([8]) -> ([8]); +store_temp([9]) -> ([9]); +store_temp>([20]) -> ([20]); +return([8], [9], [20]); +F1: +disable_ap_tracking() -> (); +withdraw_gas([0], [1]) { fallthrough([5], [6]) F1_B1([7], [8]) }; +branch_align() -> (); +dup([4]) -> ([4], [9]); +rename([9]) -> ([10]); +dup([3]) -> ([3], [11]); +felt252_sub([11], [10]) -> ([12]); +store_temp([12]) -> ([12]); +felt252_is_zero([12]) { fallthrough() F1_B0([13]) }; +branch_align() -> (); +drop([4]) -> (); +redeposit_gas([6]) -> ([14]); +struct_construct() -> ([15]); +struct_construct>([2], [3], [15]) -> ([16]); +enum_init, 0>([16]) -> ([17]); +store_temp([5]) -> ([5]); +store_temp([14]) -> ([14]); +store_temp>([17]) -> ([17]); +return([5], [14], [17]); +F1_B0: +branch_align() -> (); +drop>([13]) -> (); +redeposit_gas([6]) -> ([18]); +struct_deconstruct([2]) -> ([19]); +const_as_immediate>() -> ([20]); +felt252_mul([19], [20]) -> ([21]); +const_as_immediate>() -> ([22]); +felt252_add([3], [22]) -> ([23]); +struct_construct([21]) -> ([24]); +store_temp([5]) -> ([5]); +store_temp([18]) -> ([18]); +store_temp([24]) -> ([24]); +store_temp([23]) -> ([23]); +store_temp([4]) -> ([4]); +function_call([5], [18], [24], [23], [4]) -> ([25], [26], [27]); +return([25], [26], [27]); +F1_B1: +branch_align() -> (); +drop([4]) -> (); +drop([3]) -> (); +drop([2]) -> (); +function_call>() -> ([28]); +enum_init, 1>([28]) -> ([29]); +store_temp([7]) -> ([7]); +store_temp([8]) -> ([8]); +store_temp>([29]) -> ([29]); +return([7], [8], [29]); +F2: +const_as_immediate>() -> ([0]); +store_temp([0]) -> ([0]); +function_call([0]) -> ([1]); +return([1]); +F3: +array_new() -> ([1]); +array_append([1], [0]) -> ([2]); +struct_construct() -> ([3]); +struct_construct>>([3], [2]) -> ([4]); +store_temp>>([4]) -> ([4]); +return([4]); + +test::loop_snap_mut@F0([0]: RangeCheck, [1]: GasBuiltin, [2]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(core::felt252,)>); +test::loop_snap_mut[116-254]@F1([0]: RangeCheck, [1]: GasBuiltin, [2]: test::Inner, [3]: felt252, [4]: felt252) -> (RangeCheck, GasBuiltin, core::panics::PanicResult::<(test::Inner, core::felt252, ())>); +core::panic_with_const_felt252::<375233589013918064796019>@F2() -> (Tuple>); +core::panic_with_felt252@F3([0]: felt252) -> (Tuple>);