diff --git a/crates/nargo_cli/tests/test_data/array_dynamic/Prover.toml b/crates/nargo_cli/tests/test_data/array_dynamic/Prover.toml index ff6f02ccdac..750b3129ec9 100644 --- a/crates/nargo_cli/tests/test_data/array_dynamic/Prover.toml +++ b/crates/nargo_cli/tests/test_data/array_dynamic/Prover.toml @@ -2,5 +2,8 @@ x = [104, 101, 108, 108, 111] z = "59" t = "10" index = [0,1,2,3,4] +index2 = [0,1,2,3,4] +offset = 1 +sublen = 2 diff --git a/crates/nargo_cli/tests/test_data/array_dynamic/src/main.nr b/crates/nargo_cli/tests/test_data/array_dynamic/src/main.nr index a4073fd55cb..69e5fdc93db 100644 --- a/crates/nargo_cli/tests/test_data/array_dynamic/src/main.nr +++ b/crates/nargo_cli/tests/test_data/array_dynamic/src/main.nr @@ -1,16 +1,20 @@ -fn main(x: [u32; 5], mut z: u32, t: u32, index: [Field;5]) { +fn main(x: [u32; 5], mut z: u32, t: u32, index: [Field;5], index2: [Field;5], offset: Field, sublen: Field) { let idx = (z - 5*t - 5) as Field; //dynamic array test dyn_array(x, idx, idx - 3); - // regression for issue 1283 + //regression for issue 1283 let mut s = 0; let x3 = [246,159,32,176,8]; for i in 0..5 { s += x3[index[i]]; } assert(s!=0); + + if 3 < (sublen as u32) { + assert(index[offset + 3] == index2[3]); + } } fn dyn_array(mut x: [u32; 5], y: Field, z: Field) { diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs index ac3395d9411..704de442eb2 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs @@ -263,11 +263,20 @@ impl AcirMem { } // Load array values into InternalVars - // If create_witness is true, we create witnesses for values that do not have witness - pub(super) fn load_array(&mut self, array: &MemArray) -> Vec { + pub(super) fn load_array( + &mut self, + array: &MemArray, + evaluator: &mut Evaluator, + ) -> Vec { vecmap(0..array.len, |offset| { - self.load_array_element_constant_index(array, offset) - .expect("infallible: array out of bounds error") + operations::load::evaluate_with( + array, + &InternalVar::from(FieldElement::from(offset as i128)), + self, + None, + evaluator, + ) + .expect("infallible: array out of bounds error") }) } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs index 0f8091e2f6f..acc34e1ebac 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/cmp.rs @@ -126,8 +126,8 @@ fn array_eq( // Fetch the elements in both `MemArrays`s, these are `InternalVar`s // We then convert these to `Expressions` let internal_var_to_expr = |internal_var: InternalVar| internal_var.expression().clone(); - let a_values = vecmap(memory_map.load_array(a), internal_var_to_expr); - let b_values = vecmap(memory_map.load_array(b), internal_var_to_expr); + let a_values = vecmap(memory_map.load_array(a, evaluator), internal_var_to_expr); + let b_values = vecmap(memory_map.load_array(b, evaluator), internal_var_to_expr); constraints::arrays_eq_predicate(&a_values, &b_values, evaluator) } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs index 24453a771f0..f2580c43f59 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/load.rs @@ -6,14 +6,14 @@ use crate::{ ssa::{ acir_gen::{acir_mem::AcirMem, internal_var_cache::InternalVarCache, InternalVar}, context::SsaContext, - mem::{self, ArrayId}, + mem::{self, ArrayId, MemArray}, node::NodeId, }, Evaluator, }; // Returns a variable corresponding to the element at the provided index in the array -// Returns None if index is constant and out-of-bound. +// Returns an error if index is constant and out-of-bound. pub(crate) fn evaluate( array_id: ArrayId, index: NodeId, @@ -25,11 +25,21 @@ pub(crate) fn evaluate( ) -> Result { let mem_array = &ctx.mem[array_id]; let index = var_cache.get_or_compute_internal_var_unwrap(index, evaluator, ctx); + evaluate_with(mem_array, &index, acir_mem, location, evaluator) +} +// Same as evaluate(), but using MemArray and InternalVar instead of ArrayId and NodeId +pub(crate) fn evaluate_with( + array: &MemArray, + index: &InternalVar, + acir_mem: &mut AcirMem, + location: Option, + evaluator: &mut Evaluator, +) -> Result { if let Some(idx) = index.to_const() { let idx = mem::Memory::as_u32(idx); // Check to see if the index has gone out of bounds - let array_length = mem_array.len; + let array_length = array.len; if idx >= array_length { return Err(RuntimeError { location, @@ -40,13 +50,13 @@ pub(crate) fn evaluate( }); } - let array_element = acir_mem.load_array_element_constant_index(mem_array, idx); + let array_element = acir_mem.load_array_element_constant_index(array, idx); if let Some(element) = array_element { return Ok(element); } } let w = evaluator.add_witness_to_cs(); - acir_mem.add_to_trace(&array_id, index.to_expression(), w.into(), Expression::zero()); + acir_mem.add_to_trace(&array.id, index.to_expression(), w.into(), Expression::zero()); Ok(InternalVar::from_witness(w)) } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs index 6aaa3b2fbbd..aa7586fb944 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs @@ -32,7 +32,7 @@ pub(crate) fn evaluate( let objects = match Memory::deref(ctx, *node_id) { Some(a) => { let array = &ctx.mem[a]; - memory_map.load_array(array) + memory_map.load_array(array, evaluator) } None => { vec![var_cache.get_or_compute_internal_var_unwrap(*node_id, evaluator, ctx)] diff --git a/crates/noirc_evaluator/src/ssa/conditional.rs b/crates/noirc_evaluator/src/ssa/conditional.rs index c7a9adc7a02..b762a3d3f55 100644 --- a/crates/noirc_evaluator/src/ssa/conditional.rs +++ b/crates/noirc_evaluator/src/ssa/conditional.rs @@ -577,6 +577,36 @@ impl DecisionTree { DecisionTree::short_circuit(ctx, stack, ass_value, &error, *location)?; return Ok(false); } + } else { + // we use a 0 index when the predicate is false, to avoid out-of-bound issues + let ass_value = Self::unwrap_predicate(ctx, &Some(ass_value)); + let op = Operation::Cast(ass_value); + let pred = ctx.add_instruction(Instruction::new( + op, + ctx.object_type(*index), + Some(stack.block), + )); + stack.push(pred); + let op = Operation::Binary(node::Binary { + lhs: *index, + rhs: pred, + operator: BinaryOp::Mul, + predicate: None, + }); + let idx = ctx.add_instruction(Instruction::new( + op, + ObjectType::native_field(), + Some(stack.block), + )); + optimizations::simplify_id(ctx, idx).unwrap(); + stack.push(idx); + + let ins2 = ctx.instruction_mut(ins_id); + ins2.operation = Operation::Load { + array_id: *array_id, + index: idx, + location: *location, + }; } stack.push(ins_id); } @@ -651,10 +681,37 @@ impl DecisionTree { if !stack.is_new_array(ctx, array_id) && ctx.under_assumption(ass_value) { let pred = Self::and_conditions(Some(ass_value), *predicate, stack, ctx); + // we use a 0 index when the predicate is false and index is not constant, to avoid out-of-bound issues + let idx = if ctx.get_as_constant(*index).is_some() { + *index + } else { + let op = Operation::Cast(Self::unwrap_predicate(ctx, &pred)); + + let cast = ctx.add_instruction(Instruction::new( + op, + ctx.object_type(*index), + Some(stack.block), + )); + stack.push(cast); + let op = Operation::Binary(node::Binary { + lhs: *index, + rhs: cast, + operator: BinaryOp::Mul, + predicate: None, + }); + let idx = ctx.add_instruction(Instruction::new( + op, + ObjectType::native_field(), + Some(stack.block), + )); + optimizations::simplify_id(ctx, idx).unwrap(); + stack.push(idx); + idx + }; let ins2 = ctx.instruction_mut(ins_id); ins2.operation = Operation::Store { array_id: *array_id, - index: *index, + index: idx, value: *value, predicate: pred, location: *location,