Skip to content

Commit

Permalink
Remove bounds checks from array_get.
Browse files Browse the repository at this point in the history
  • Loading branch information
ilyalesokhin-starkware committed Nov 12, 2024
1 parent f67829f commit e2b3cc6
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 85 deletions.
15 changes: 7 additions & 8 deletions corelib/src/array.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ extern fn array_snapshot_multi_pop_front<PoppedT, impl Info: FixedSizedArrayInfo
extern fn array_snapshot_multi_pop_back<PoppedT, impl Info: FixedSizedArrayInfo<PoppedT>>(
ref arr: @Array<Info::Element>
) -> Option<@Box<PoppedT>> implicits(RangeCheck) nopanic;
#[panic_with('Index out of bounds', array_at)]
extern fn array_get<T>(
arr: @Array<T>, index: usize
) -> Option<Box<@T>> implicits(RangeCheck) nopanic;
) -> Box<@T> nopanic;
extern fn array_slice<T>(
arr: @Array<T>, start: usize, length: usize
) -> Option<@Array<T>> implicits(RangeCheck) nopanic;
Expand Down Expand Up @@ -121,7 +120,7 @@ pub impl ArrayImpl<T> of ArrayTrait<T> {
/// ```
#[inline(always)]
fn get(self: @Array<T>, index: usize) -> Option<Box<@T>> {
array_get(self, index)
Option::Some(array_get(self, index))
}
/// Returns a snapshot of the value at the given 'index'.
///
Expand All @@ -132,7 +131,7 @@ pub impl ArrayImpl<T> of ArrayTrait<T> {
/// assert!(arr.at(1) == @4);
/// ```
fn at(self: @Array<T>, index: usize) -> @T {
array_at(self, index).unbox()
array_get(self, index).unbox()
}
/// Returns the length of the array.
///
Expand Down Expand Up @@ -183,7 +182,7 @@ impl ArrayDefault<T> of Default<Array<T>> {

impl ArrayIndex<T> of IndexView<Array<T>, usize, @T> {
fn index(self: @Array<T>, index: usize) -> @T {
array_at(self, index).unbox()
array_get(self, index).unbox()
}
}

Expand Down Expand Up @@ -347,7 +346,7 @@ pub impl SpanImpl<T> of SpanTrait<T> {
/// ```
#[inline(always)]
fn get(self: Span<T>, index: usize) -> Option<Box<@T>> {
array_get(self.snapshot, index)
Option::Some(array_get(self.snapshot, index))
}
/// Returns a snapshot of the value at the given 'index'.
///
Expand All @@ -358,7 +357,7 @@ pub impl SpanImpl<T> of SpanTrait<T> {
/// ```
#[inline(always)]
fn at(self: Span<T>, index: usize) -> @T {
array_at(self.snapshot, index).unbox()
array_get(self.snapshot, index).unbox()
}
/// Returns a span containing values from the 'start' index, with
/// amount equal to 'length'.
Expand Down Expand Up @@ -406,7 +405,7 @@ pub impl SpanImpl<T> of SpanTrait<T> {
pub impl SpanIndex<T> of IndexView<Span<T>, usize, @T> {
#[inline(always)]
fn index(self: @Span<T>, index: usize) -> @T {
array_at(*self.snapshot, index).unbox()
array_get(*self.snapshot, index).unbox()
}
}

Expand Down
26 changes: 6 additions & 20 deletions crates/cairo-lang-lowering/src/test_data/match
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ test_function_lowering

//! > function
fn foo(a: @Array::<felt252>) -> Option<Box<@felt252>> {
core::array::array_get(a, 0_u32)
Option::Some(core::array::array_get(a, 0_u32))
}

//! > function_name
Expand All @@ -177,28 +177,14 @@ foo
//! > lowering_diagnostics

//! > lowering_flat
Parameters: v0: core::RangeCheck, v1: @core::array::Array::<core::felt252>
Parameters: v0: @core::array::Array::<core::felt252>
blk0 (root):
Statements:
(v2: core::integer::u32) <- 0
(v1: core::integer::u32) <- 0
(v2: core::box::Box::<@core::felt252>) <- core::array::array_get::<core::felt252>(v0, v1)
(v3: core::option::Option::<core::box::Box::<@core::felt252>>) <- Option::Some(v2)
End:
Match(match core::array::array_get::<core::felt252>(v0, v1, v2) {
Option::Some(v3, v4) => blk1,
Option::None(v5) => blk2,
})

blk1:
Statements:
(v6: core::option::Option::<core::box::Box::<@core::felt252>>) <- Option::Some(v4)
End:
Return(v3, v6)

blk2:
Statements:
(v7: ()) <- struct_construct()
(v8: core::option::Option::<core::box::Box::<@core::felt252>>) <- Option::None(v7)
End:
Return(v5, v8)
Return(v3)

//! > ==========================================================================

Expand Down
47 changes: 4 additions & 43 deletions crates/cairo-lang-sierra-to-casm/src/invocations/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,8 @@ fn build_array_get(
elem_ty: &ConcreteTypeId,
builder: CompiledInvocationBuilder<'_>,
) -> Result<CompiledInvocation, InvocationError> {
let [expr_range_check, expr_arr, expr_index] = builder.try_get_refs()?;
let range_check = expr_range_check.try_unpack_single()?;
let [arr_start, arr_end] = expr_arr.try_unpack()?;
let [expr_arr, expr_index] = builder.try_get_refs()?;
let [arr_start, _arr_end] = expr_arr.try_unpack()?;
let index = expr_index.try_unpack_single()?;

let element_size = builder.program_info.type_sizes[elem_ty];
Expand All @@ -224,54 +223,16 @@ fn build_array_get(
add_input_variables! {casm_builder,
deref_or_immediate index;
deref arr_start;
deref arr_end;
buffer(1) range_check;
};
casm_build_extend! {casm_builder,
const element_size = element_size;
let orig_range_check = range_check;
// Compute the length of the array (in cells).
tempvar array_length_in_cells = arr_end - arr_start;
// Compute the offset of the element (in cells).
maybe_tempvar element_offset_in_cells = index * element_size;
// Check that offset is in range.
// Note that the offset may be as large as `(2^15 - 1) * (2^32 - 1)`.
tempvar is_in_range;
hint TestLessThan {lhs: element_offset_in_cells, rhs: array_length_in_cells} into {dst: is_in_range};
jump InRange if is_in_range != 0;
// Index out of bounds. Compute offset - length.
tempvar offset_length_diff = element_offset_in_cells - array_length_in_cells;
// Assert offset - length >= 0. Note that offset_length_diff is smaller than 2^128 as the index type is u32.
assert offset_length_diff = *(range_check++);
jump FailureHandle;

InRange:
// Assert offset < length, or that length - (offset + 1) is in [0, 2^128).
// Compute offset + 1.
const one = 1;
tempvar element_offset_in_cells_plus_1 = element_offset_in_cells + one;
// Compute length - (offset + 1).
tempvar offset_length_diff = array_length_in_cells - element_offset_in_cells_plus_1;
// Assert length - (offset + 1) is in [0, 2^128).
assert offset_length_diff = *(range_check++);
// The start address of target cells.
let target_cell = arr_start + element_offset_in_cells;
};
let failure_handle = get_non_fallthrough_statement_id(&builder);
Ok(builder.build_from_casm_builder(
casm_builder,
[
("Fallthrough", &[&[range_check], &[target_cell]], None),
("FailureHandle", &[&[range_check]], Some(failure_handle)),
],
CostValidationInfo {
builtin_infos: vec![BuiltinInfo {
cost_token_ty: CostTokenType::RangeCheck,
start: orig_range_check,
end: range_check,
}],
extra_costs: None,
},
[("Fallthrough", &[&[target_cell]], None)],
CostValidationInfo { builtin_infos: vec![], extra_costs: None },
))
}

Expand Down
18 changes: 4 additions & 14 deletions crates/cairo-lang-sierra/src/extensions/modules/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,29 +334,19 @@ impl SignatureAndTypeGenericLibfunc for ArrayGetLibfuncWrapped {
ty: ConcreteTypeId,
) -> Result<LibfuncSignature, SpecializationError> {
let arr_type = context.get_wrapped_concrete_type(ArrayType::id(), ty.clone())?;
let range_check_type = context.get_concrete_type(RangeCheckType::id(), &[])?;
let index_type = context.get_concrete_type(ArrayIndexType::id(), &[])?;
let param_signatures = vec![
ParamSignature::new(range_check_type.clone()).with_allow_add_const(),
ParamSignature::new(snapshot_ty(context, arr_type)?),
ParamSignature::new(index_type),
];
let rc_output_info = OutputVarInfo::new_builtin(range_check_type, 0);
let branch_signatures = vec![
// First (success) branch returns rc, array and element; failure branch does not return
// an element.
BranchSignature {
vars: vec![
rc_output_info.clone(),
OutputVarInfo {
ty: box_ty(context, snapshot_ty(context, ty)?)?,
ref_info: OutputVarReferenceInfo::Deferred(DeferredOutputKind::Generic),
},
],
ap_change: SierraApChange::Known { new_vars_only: false },
},
BranchSignature {
vars: vec![rc_output_info],
vars: vec![OutputVarInfo {
ty: box_ty(context, snapshot_ty(context, ty)?)?,
ref_info: OutputVarReferenceInfo::Deferred(DeferredOutputKind::Generic),
}],
ap_change: SierraApChange::Known { new_vars_only: false },
},
];
Expand Down

0 comments on commit e2b3cc6

Please sign in to comment.