diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 3fa7d8ed5e3..bb6c9ad895b 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -78,6 +78,11 @@ pub enum RuntimeError { /// code when a break or continue is generated. #[error("Break or continue")] BreakOrContinue { call_stack: CallStack }, + + #[error( + "Only constant indices are supported when indexing an array containing reference values" + )] + DynamicIndexingWithReference { call_stack: CallStack }, } #[derive(Debug, Clone, Serialize, Deserialize, Hash)] @@ -187,10 +192,11 @@ impl RuntimeError { | RuntimeError::BigIntModulus { call_stack, .. } | RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack } | RuntimeError::UnconstrainedOracleReturnToConstrained { call_stack } + | RuntimeError::ReturnedReferenceFromDynamicIf { call_stack } + | RuntimeError::ReturnedFunctionFromDynamicIf { call_stack } + | RuntimeError::BreakOrContinue { call_stack } + | RuntimeError::DynamicIndexingWithReference { call_stack } | RuntimeError::UnknownReference { call_stack } => call_stack, - RuntimeError::ReturnedReferenceFromDynamicIf { call_stack } => call_stack, - RuntimeError::ReturnedFunctionFromDynamicIf { call_stack } => call_stack, - RuntimeError::BreakOrContinue { call_stack } => call_stack, } } } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 96b907073a1..b3a44da2d45 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -212,6 +212,10 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec { // A function can be potentially unreachable post-DIE if all calls to that function were removed. SsaPass::new(Ssa::remove_unreachable_functions, "Removing Unreachable Functions"), SsaPass::new(Ssa::checked_to_unchecked, "Checked to unchecked"), + SsaPass::new_try( + Ssa::verify_no_dynamic_indices_to_references, + "Verifying no dynamic array indices to reference value elements", + ), ] } diff --git a/compiler/noirc_evaluator/src/ssa/validation/dynamic_array_indices.rs b/compiler/noirc_evaluator/src/ssa/validation/dynamic_array_indices.rs new file mode 100644 index 00000000000..c15ee1b5b2c --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/validation/dynamic_array_indices.rs @@ -0,0 +1,107 @@ +use crate::{ + errors::{RtResult, RuntimeError}, + ssa::{ + ir::{function::Function, instruction::Instruction}, + ssa_gen::Ssa, + }, +}; + +impl Ssa { + /// Verifies there are no `array_get` or `array_set` instructions remaining + /// with dynamic indices where the element type may contain a reference type. + /// This effectively bans dynamic-indexing of arrays with reference elements + /// since we cannot guarantee we optimize references out of the program in that case. + /// + /// This pass expects to be run late in the Ssa pipeline such that all array indices + /// used are either constant or derived from an input to the program. E.g. we expect + /// optimizations from inlining, flattening, and mem2reg to already be complete. + pub(crate) fn verify_no_dynamic_indices_to_references(self) -> RtResult { + for function in self.functions.values() { + function.verify_no_dynamic_indices_to_references()?; + } + Ok(self) + } +} + +impl Function { + pub(crate) fn verify_no_dynamic_indices_to_references(&self) -> RtResult<()> { + if self.runtime().is_brillig() { + return Ok(()); + } + + for block in self.reachable_blocks() { + for instruction in self.dfg[block].instructions() { + match &self.dfg[*instruction] { + Instruction::ArrayGet { array, index, .. } + | Instruction::ArraySet { array, index, .. } => { + let array_type = self.dfg.type_of_value(*array); + let contains_reference = array_type.contains_reference(); + + if contains_reference && self.dfg.get_numeric_constant(*index).is_none() { + let call_stack = self.dfg.get_instruction_call_stack(*instruction); + return Err(RuntimeError::DynamicIndexingWithReference { call_stack }); + } + } + _ => (), + } + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::{errors::RuntimeError, ssa::ssa_gen::Ssa}; + + #[test] + fn dynamic_array_of_2_mut_bools() { + // https://github.com/noir-lang/noir/issues/8750 + // fn main(c: u32) -> pub bool { + // let b: [&mut bool; 2] = [&mut false, &mut true]; + // *b[c % 2] + //} + let src = r#" + acir(inline) predicate_pure fn main f0 { + b0(v0: u32): + v1 = allocate -> &mut u1 + v2 = allocate -> &mut u1 + v3 = make_array [v1, v2] : [&mut u1; 2] + v4 = truncate v0 to 1 bits, max_bit_size: 32 + v6 = lt v4, u32 2 + constrain v6 == u1 1, "Index out of bounds" + v8 = array_get v3, index v4 -> &mut u1 + v9 = load v8 -> u1 + return v9 + }"#; + + let ssa = Ssa::from_str(src).unwrap(); + let result = ssa.verify_no_dynamic_indices_to_references(); + assert!(matches!(result, Err(RuntimeError::DynamicIndexingWithReference { .. }))); + } + + #[test] + fn no_error_in_brillig() { + // unconstrained fn main(c: u32) -> pub bool { + // let b: [&mut bool; 2] = [&mut false, &mut true]; + // *b[c % 2] + //} + let src = r#" + brillig(inline) predicate_pure fn main f0 { + b0(v0: u32): + v1 = allocate -> &mut u1 + v2 = allocate -> &mut u1 + v3 = make_array [v1, v2] : [&mut u1; 2] + v4 = truncate v0 to 1 bits, max_bit_size: 32 + v6 = lt v4, u32 2 + constrain v6 == u1 1, "Index out of bounds" + v8 = array_get v3, index v4 -> &mut u1 + v9 = load v8 -> u1 + return v9 + }"#; + + let ssa = Ssa::from_str(src).unwrap(); + let result = ssa.verify_no_dynamic_indices_to_references(); + assert!(result.is_ok()); + } +} diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index eac124bd03a..99d657d6f82 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -15,6 +15,8 @@ use acvm::{AcirField, FieldElement}; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; +pub(crate) mod dynamic_array_indices; + use crate::ssa::ir::instruction::TerminatorInstruction; use super::ir::{ diff --git a/docs/docs/noir/concepts/data_types/arrays.md b/docs/docs/noir/concepts/data_types/arrays.md index bd906edbd1e..8a6c8d6a1d8 100644 --- a/docs/docs/noir/concepts/data_types/arrays.md +++ b/docs/docs/noir/concepts/data_types/arrays.md @@ -77,6 +77,42 @@ However, multidimensional slices are not supported. For example, the following c let slice : [[Field]] = &[]; ``` +## Dynamic Indexing + +Using constant indices of arrays will often be more efficient at runtime in constrained code. +Indexing an array with non-constant indices (indices derived from the inputs to the program) is also +called "dynamic indexing" and incurs a slight runtime cost: + +```rust +fn main(x: u32) { + let array = [1, 2, 3, 4]; + + // This is a constant index, after inlining the compiler sees that this + // will always be `array[2]` + let _a = array[double(1)]; + + // This is a non-constant index, there is no way to know which u32 value + // will be used as an index here + let _b = array[double(x)]; +} + +fn double(y: u32) -> u32 { + y * 2 +} +``` + +There is another restriction with dynamic indices: they cannot be used on arrays with +elements which contain a reference type: + +```rust +fn main(x: u32) { + let array = [&mut 1, &mut 2, &mut 3, &mut 4]; + + // error! Only constant indices may be used here since `array` contains references internally! + let _c = array[x]; +} +``` + ## Types You can create arrays of primitive types or structs. There is not yet support for nested arrays