Skip to content

Commit

Permalink
feat: Add brillig array index check (#4127)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Adds a runtime length check to avoid out of bounds accesses, to emulate
the behavior with ACIR dynamic array accesses.

## Summary\*



## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
sirasistant authored Jan 23, 2024
1 parent 09d1d5c commit c29f85f
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 0 deletions.
33 changes: 33 additions & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ impl<'block> BrilligBlock<'block> {
};

let index_register = self.convert_ssa_register_value(*index, dfg);
self.validate_array_index(array_variable, index_register);
self.retrieve_variable_from_array(
array_pointer,
index_register,
Expand All @@ -582,6 +583,7 @@ impl<'block> BrilligBlock<'block> {
result_ids[0],
dfg,
);
self.validate_array_index(source_variable, index_register);

self.convert_ssa_array_set(
source_variable,
Expand Down Expand Up @@ -690,6 +692,37 @@ impl<'block> BrilligBlock<'block> {
.post_call_prep_returns_load_registers(&returned_registers, &saved_registers);
}

fn validate_array_index(
&mut self,
array_variable: BrilligVariable,
index_register: RegisterIndex,
) {
let (size_as_register, should_deallocate_size) = match array_variable {
BrilligVariable::BrilligArray(BrilligArray { size, .. }) => {
(self.brillig_context.make_constant(size.into()), true)
}
BrilligVariable::BrilligVector(BrilligVector { size, .. }) => (size, false),
_ => unreachable!("ICE: validate array index on non-array"),
};

let condition = self.brillig_context.allocate_register();

self.brillig_context.memory_op(
index_register,
size_as_register,
condition,
BinaryIntOp::LessThan,
);

self.brillig_context
.constrain_instruction(condition, Some("Array index out of bounds".to_owned()));

if should_deallocate_size {
self.brillig_context.deallocate_register(size_as_register);
}
self.brillig_context.deallocate_register(condition);
}

pub(crate) fn retrieve_variable_from_array(
&mut self,
array_pointer: RegisterIndex,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
name = "out_of_bounds_alignment"
type = "bin"
authors = [""]
[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fn out_of_bounds(arr_1: [Field; 50]) -> Field {
arr_1[50 + 1]
}

unconstrained fn out_of_bounds_unconstrained_wrapper(arr_1: [Field; 50], arr_2: [Field; 50]) -> Field {
out_of_bounds(arr_1)
}

#[test(should_fail)]
fn test_acir() {
assert_eq(out_of_bounds([0; 50]), 0);
}

#[test(should_fail)]
fn test_brillig() {
assert_eq(out_of_bounds_unconstrained_wrapper([0; 50], [0; 50]), 0);
}

0 comments on commit c29f85f

Please sign in to comment.