diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 8e55debec1d..95a7e694e5f 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -715,7 +715,12 @@ impl<'a> FunctionContext<'a> { } } ast::LValue::Index { array, index, location, .. } => { - self.index_lvalue(array, index, location)?.2 + let (_array_or_slice, index, lvalue_ref, length) = + self.index_lvalue(array, index, location)?; + + self.codegen_slice_access_check(index, length); + + lvalue_ref } ast::LValue::MemberAccess { object, field_index } => { let (old_object, object_lvalue) = self.extract_current_value_recursive(object)?; @@ -747,37 +752,29 @@ impl<'a> FunctionContext<'a> { } /// Compile the given `array[index]` expression as a reference. - /// This will return a triple of (array, index, lvalue_ref, Option) where the lvalue_ref records the + /// This will return a tuple of four elements, (array, index, lvalue_ref, length), where the lvalue_ref records the /// structure of the lvalue expression for use by `assign_new_value`. - /// The optional length is for indexing slices rather than arrays since slices - /// are represented as a tuple in the form: (length, slice contents). fn index_lvalue( &mut self, array: &ast::LValue, index: &ast::Expression, location: &Location, - ) -> Result<(ValueId, ValueId, LValue, Option), RuntimeError> { + ) -> Result<(ValueId, ValueId, LValue, ValueId), RuntimeError> { let (old_array, array_lvalue) = self.extract_current_value_recursive(array)?; let index = self.codegen_non_tuple_expression(index)?; let array_lvalue = Box::new(array_lvalue); let array_values = old_array.clone().into_value_list(self); let location = *location; - // A slice is represented as a tuple (length, slice contents). - // We need to fetch the second value. - Ok(if array_values.len() > 1 { - let slice_lvalue = LValue::SliceIndex { - old_slice: old_array, - index, - slice_lvalue: array_lvalue, - location, - }; - (array_values[1], index, slice_lvalue, Some(array_values[0])) + let (array_or_slice, length) = self.extract_indexable_type_length(&array_values); + + let lvalue_ref = if array_values.len() > 1 { + LValue::SliceIndex { old_slice: old_array, index, slice_lvalue: array_lvalue, location } } else { - let array_lvalue = - LValue::Index { old_array: array_values[0], index, array_lvalue, location }; - (array_values[0], index, array_lvalue, None) - }) + LValue::Index { old_array: array_or_slice, index, array_lvalue, location } + }; + + Ok((array_or_slice, index, lvalue_ref, length)) } fn extract_current_value_recursive( diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 468a8573307..b9e0d594445 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -368,20 +368,14 @@ impl<'a> FunctionContext<'a> { fn codegen_index(&mut self, index: &ast::Index) -> Result { let array_or_slice = self.codegen_expression(&index.collection)?.into_value_list(self); let index_value = self.codegen_non_tuple_expression(&index.index)?; - // Slices are represented as a tuple in the form: (length, slice contents). - // Thus, slices require two value ids for their representation. - let (array, slice_length) = if array_or_slice.len() > 1 { - (array_or_slice[1], Some(array_or_slice[0])) - } else { - (array_or_slice[0], None) - }; + let (array_or_slice, length) = self.extract_indexable_type_length(&array_or_slice); self.codegen_array_index( - array, + array_or_slice, index_value, &index.element_type, index.location, - slice_length, + length, ) } @@ -397,7 +391,7 @@ impl<'a> FunctionContext<'a> { index: super::ir::value::ValueId, element_type: &ast::Type, location: Location, - length: Option, + length: super::ir::value::ValueId, ) -> Result { // base_index = index * type_size let index = self.make_array_index(index); @@ -412,16 +406,7 @@ impl<'a> FunctionContext<'a> { let offset = self.make_offset(base_index, field_index); field_index += 1; - let array_type = &self.builder.type_of_value(array); - match array_type { - Type::Slice(_) => { - self.codegen_slice_access_check(index, length); - } - Type::Array(..) => { - // Nothing needs to done to prepare an array access on an array - } - _ => unreachable!("must have array or slice but got {array_type}"), - } + self.codegen_slice_access_check(index, length); // Reference counting in brillig relies on us incrementing reference // counts when nested arrays/slices are constructed or indexed. This @@ -438,12 +423,11 @@ impl<'a> FunctionContext<'a> { fn codegen_slice_access_check( &mut self, index: super::ir::value::ValueId, - length: Option, + length: super::ir::value::ValueId, ) { let index = self.make_array_index(index); // We convert the length as an array index type for comparison - let array_len = self - .make_array_index(length.expect("ICE: a length must be supplied for indexing slices")); + let array_len = self.make_array_index(length); let is_offset_out_of_bounds = self.builder.insert_binary(index, BinaryOp::Lt, array_len); let true_const = self.builder.numeric_constant(true, Type::bool()); @@ -638,10 +622,10 @@ impl<'a> FunctionContext<'a> { // can be converted to a slice push back let len_plus_one = self.builder.insert_binary(arguments[0], BinaryOp::Add, one); - self.codegen_slice_access_check(arguments[2], Some(len_plus_one)); + self.codegen_slice_access_check(arguments[2], len_plus_one); } Intrinsic::SliceRemove => { - self.codegen_slice_access_check(arguments[2], Some(arguments[0])); + self.codegen_slice_access_check(arguments[2], arguments[0]); } _ => { // Do nothing as the other intrinsics do not require checks @@ -749,4 +733,23 @@ impl<'a> FunctionContext<'a> { self.builder.terminate_with_jmp(loop_.loop_entry, vec![new_loop_index]); Self::unit_value() } + + // Given an indexable type (array or slice) that can either be a single value (an array) or two values (a slice), + // returns two values where the first one is the array or the slice, and the second one is the + // length. + fn extract_indexable_type_length(&mut self, array_or_slice: &[ValueId]) -> (ValueId, ValueId) { + // Slices are represented as a tuple in the form: (length, slice contents). + // Thus, slices require two value ids for their representation. + if array_or_slice.len() > 1 { + (array_or_slice[1], array_or_slice[0]) + } else { + let array_type = &self.builder.type_of_value(array_or_slice[0]); + let Type::Array(_, length) = array_type else { + panic!("Expected array type when array is just a single value"); + }; + + let length = self.builder.numeric_constant(*length, Type::unsigned(32)); + (array_or_slice[0], length) + } + } } diff --git a/test_programs/execution_failure/array_get_known_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/array_get_known_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..98b59477906 --- /dev/null +++ b/test_programs/execution_failure/array_get_known_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_get_known_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/array_get_known_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/array_get_known_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/execution_failure/array_get_known_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/array_get_known_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..bdc645ec483 --- /dev/null +++ b/test_programs/execution_failure/array_get_known_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main() { + let array = [1, 2, 3]; + let _ = array[10]; // Index out of bounds +} diff --git a/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..986e9e8e2e6 --- /dev/null +++ b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_get_unknown_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..1ec81884d61 --- /dev/null +++ b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/Prover.toml @@ -0,0 +1 @@ +x = "10" \ No newline at end of file diff --git a/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..15c2d1f1f23 --- /dev/null +++ b/test_programs/execution_failure/array_get_unknown_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main(x: Field) { + let array = [1, 2, 3]; + let _ = array[x]; // Index out of bounds +} diff --git a/test_programs/execution_failure/array_set_known_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/array_set_known_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..f6beafc4e90 --- /dev/null +++ b/test_programs/execution_failure/array_set_known_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_set_known_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/array_set_known_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/array_set_known_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/execution_failure/array_set_known_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/array_set_known_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..9c447aee08f --- /dev/null +++ b/test_programs/execution_failure/array_set_known_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main() { + let mut array = [1, 2, 3]; + array[10] = 1; // Index out of bounds +} diff --git a/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..8d3b47b2b86 --- /dev/null +++ b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_set_unknown_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..1ec81884d61 --- /dev/null +++ b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/Prover.toml @@ -0,0 +1 @@ +x = "10" \ No newline at end of file diff --git a/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..dbde898f7a9 --- /dev/null +++ b/test_programs/execution_failure/array_set_unknown_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main(x: Field) { + let mut array = [1, 2, 3]; + array[x] = 1; // Index out of bounds +} diff --git a/test_programs/execution_failure/slice_get_known_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/slice_get_known_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..7520ed9a9c4 --- /dev/null +++ b/test_programs/execution_failure/slice_get_known_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "slice_get_known_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/slice_get_known_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/slice_get_known_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/execution_failure/slice_get_known_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/slice_get_known_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..59e57664bbe --- /dev/null +++ b/test_programs/execution_failure/slice_get_known_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main() { + let slice = &[1, 2, 3]; + let _ = slice[10]; // Index out of bounds +} diff --git a/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..ddda52adeaf --- /dev/null +++ b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "slice_get_unknown_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..1ec81884d61 --- /dev/null +++ b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/Prover.toml @@ -0,0 +1 @@ +x = "10" \ No newline at end of file diff --git a/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..5a62e0e9843 --- /dev/null +++ b/test_programs/execution_failure/slice_get_unknown_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main(x: Field) { + let slice = &[1, 2, 3]; + let _ = slice[x]; // Index out of bounds +} diff --git a/test_programs/execution_failure/slice_set_known_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/slice_set_known_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..ea535921e99 --- /dev/null +++ b/test_programs/execution_failure/slice_set_known_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "slice_set_known_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/slice_set_known_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/slice_set_known_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/execution_failure/slice_set_known_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/slice_set_known_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..31d06cc200d --- /dev/null +++ b/test_programs/execution_failure/slice_set_known_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main() { + let mut slice = &[1, 2, 3]; + slice[10] = 1; // Index out of bounds +} diff --git a/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Nargo.toml b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Nargo.toml new file mode 100644 index 00000000000..597f7be81ba --- /dev/null +++ b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "slice_set_unknown_index_out_of_bounds" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Prover.toml b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Prover.toml new file mode 100644 index 00000000000..1ec81884d61 --- /dev/null +++ b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/Prover.toml @@ -0,0 +1 @@ +x = "10" \ No newline at end of file diff --git a/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/src/main.nr b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/src/main.nr new file mode 100644 index 00000000000..33d30a4b91a --- /dev/null +++ b/test_programs/execution_failure/slice_set_unknown_index_out_of_bounds/src/main.nr @@ -0,0 +1,4 @@ +fn main(x: Field) { + let mut slice = &[1, 2, 3]; + slice[x] = 1; // Index out of bounds +}