feat(fuzz): Allow index OOB with a small probability

tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs

@@ -1,20 +1,15 @@
 //! Compare the execution of random ASTs between the normal execution
 //! vs when everything is forced to be Brillig.
+use crate::targets::default_config;
 use crate::{compare_results_compiled, create_ssa_or_die, default_ssa_options};
 use arbitrary::Arbitrary;
 use arbitrary::Unstructured;
 use color_eyre::eyre;
-use noir_ast_fuzzer::Config;
 use noir_ast_fuzzer::compare::{CompareOptions, ComparePipelines};
 use noir_ast_fuzzer::rewrite::change_all_functions_into_unconstrained;
 
 pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
-    let config = Config {
-        // Overflows can be triggered easily, so in half the cases we avoid them,
-        // to make sure they don't mask other errors.
-        avoid_overflow: u.arbitrary()?,
-        ..Default::default()
-    };
+    let config = default_config(u)?;
 
     let inputs = ComparePipelines::arb(
         u,

tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_direct.rs

@@ -6,6 +6,7 @@
 //! This variant accesses the interpreter directly instead of going
 //! through nargo, which speeds up execution but also currently
 //! has some issues (inability to use prints among others).
+use crate::targets::default_config;
 use crate::{compare_results_comptime, create_ssa_or_die, default_ssa_options};
 use arbitrary::Unstructured;
 use color_eyre::eyre;
@@ -24,15 +25,13 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
         comptime_friendly: true,
         // Force brillig, to generate loops that the interpreter can do but ACIR cannot.
         force_brillig: true,
-        // Allow overflows half the time.
-        avoid_overflow: u.arbitrary()?,
         // Slices need some parts of the stdlib that we can't just append to the source
         // the way it is currently done to support prints, because they are low level extensions.
         avoid_slices: true,
         // Use lower limits because of the interpreter, to avoid stack overflow
         max_loop_size: 5,
         max_recursive_calls: 5,
-        ..Default::default()
+        ..default_config(u)?
     };
 
     let inputs = CompareComptime::arb(u, config, |program| {

tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_nargo.rs

@@ -6,6 +6,7 @@
 //! This variant lets nargo parse the resulting source code which is slow
 //! but at the moment is more feature complete than using the interpreter
 //! directly.
+use crate::targets::default_config;
 use crate::{compare_results_comptime, create_ssa_or_die, default_ssa_options};
 use arbitrary::Unstructured;
 use color_eyre::eyre;
@@ -16,8 +17,6 @@ use noir_ast_fuzzer::rewrite::change_all_functions_into_unconstrained;
 
 pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
     let config = Config {
-        // It's easy to overflow.
-        avoid_overflow: u.arbitrary()?,
         // Avoid break/continue
         avoid_loop_control: true,
         // Match is not yet implemented in comptime.
@@ -29,7 +28,7 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
         // Use lower limits because of the interpreter, to avoid stack overflow
         max_loop_size: 5,
         max_recursive_calls: 5,
-        ..Default::default()
+        ..default_config(u)?
     };
 
     let inputs = CompareComptime::arb(u, config, |program| {

tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs

@@ -1,24 +1,19 @@
 //! Compare the execution of random ASTs between the initial SSA
 //! (or as close as we can stay to the initial state)
 //! and the fully optimized version.
+use crate::targets::default_config;
 use crate::{
     compare_results_compiled, create_ssa_or_die, create_ssa_with_passes_or_die, default_ssa_options,
 };
 use arbitrary::{Arbitrary, Unstructured};
 use color_eyre::eyre;
 use noir_ast_fuzzer::compare::{CompareOptions, ComparePipelines};
-use noir_ast_fuzzer::{
-    Config, compare::CompareResult, rewrite::change_all_functions_into_unconstrained,
-};
+use noir_ast_fuzzer::{compare::CompareResult, rewrite::change_all_functions_into_unconstrained};
 use noirc_evaluator::ssa::minimal_passes;
 
 pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
     let passes = minimal_passes();
-    let config = Config {
-        // Overflows are easy to trigger.
-        avoid_overflow: u.arbitrary()?,
-        ..Default::default()
-    };
+    let config = default_config(u)?;
 
     let inputs = ComparePipelines::arb(
         u,

tooling/ast_fuzzer/fuzz/src/targets/mod.rs

@@ -1,10 +1,27 @@
+use arbitrary::Unstructured;
+use noir_ast_fuzzer::Config;
+
 pub mod acir_vs_brillig;
 pub mod comptime_vs_brillig_direct;
 pub mod comptime_vs_brillig_nargo;
 pub mod min_vs_full;
 pub mod orig_vs_morph;
 pub mod pass_vs_prev;
 
+/// Create a default configuration instance, with some common flags randomized.
+fn default_config(u: &mut Unstructured) -> arbitrary::Result<Config> {
+    // Some errors such as overflows and OOB are easy to trigger, so in half
+    // the cases we avoid all of them, to make sure they don't mask other errors.
+    let avoid_frequent_errors = u.arbitrary()?;
+    let config = Config {
+        avoid_overflow: avoid_frequent_errors,
+        avoid_index_out_of_bounds: avoid_frequent_errors,
+        ..Default::default()
+    };
+    Ok(config)
+}
+
+/// Common functions used in the test modules of targets.
 #[cfg(test)]
 mod tests {
 

tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs

@@ -3,12 +3,13 @@
 
 use std::cell::{Cell, RefCell};
 
+use crate::targets::default_config;
 use crate::{compare_results_compiled, create_ssa_or_die, default_ssa_options};
 use arbitrary::{Arbitrary, Unstructured};
 use color_eyre::eyre;
 use noir_ast_fuzzer::compare::{CompareMorph, CompareOptions};
 use noir_ast_fuzzer::scope::ScopeStack;
-use noir_ast_fuzzer::{Config, visitor::visit_expr_be_mut};
+use noir_ast_fuzzer::visitor::visit_expr_be_mut;
 use noir_ast_fuzzer::{rewrite, visitor};
 use noirc_frontend::ast::UnaryOp;
 use noirc_frontend::monomorphization::ast::{
@@ -18,7 +19,7 @@ use noirc_frontend::monomorphization::ast::{
 pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
     let rules = rules::all();
     let max_rewrites = 10;
-    let config = Config { avoid_overflow: u.arbitrary()?, ..Default::default() };
+    let config = default_config(u)?;
     let inputs = CompareMorph::arb(
         u,
         config,

tooling/ast_fuzzer/src/compare/comptime.rs

@@ -268,11 +268,13 @@ impl CompareComptime {
 
     /// Check if a comptime error is due to some kind of arithmetic or constraint failure.
     fn is_assertion_diagnostic(e: &CustomDiagnostic) -> bool {
+        let msg = e.message.to_lowercase();
         e.secondaries.iter().any(|s| s.message == "Assertion failed")
-            || e.message.to_lowercase().contains("overflow")
-            || e.message.to_lowercase().contains("cannot fit into") // covers signed overflows
-            || e.message.to_lowercase().contains("divide by zero")
-            || e.message.to_lowercase().contains("division by zero")
+            || msg.contains("overflow")
+            || msg.contains("cannot fit into") // covers signed overflows
+            || msg.contains("divide by zero")
+            || msg.contains("division by zero")
+            || msg.contains("out of bounds")
     }
 
     /// Fabricate a result from a comptime `CustomDiagnostic` on the 1st side,

tooling/ast_fuzzer/src/compare/interpreted.rs

@@ -227,6 +227,9 @@ impl Comparable for ssa::interpreter::errors::InterpreterError {
                 // The removal of unreachable instructions can replace popping from an empty slice with an always-fail constraint.
                 msg.as_ref().is_some_and(|msg| msg == "Index out of bounds")
             }
+            (IndexOutOfBounds { .. }, ConstrainEqFailed { msg, .. }) => {
+                msg.as_ref().is_some_and(|msg| msg.contains("Index out of bounds"))
+            }
             (e1, e2) => {
                 // The format strings contain SSA instructions,
                 // where the only difference might be the value ID.

tooling/ast_fuzzer/src/lib.rs

@@ -58,6 +58,13 @@ pub struct Config {
     /// Try to avoid overflowing operations. Useful when testing the minimal pipeline,
     /// to avoid trivial failures due to multiplying or adding constants.
     pub avoid_overflow: bool,
+    /// Try to avoid "Index out of bounds" by using modulo to limit indexing to the
+    /// range that an array or slice is expected to contain.
+    ///
+    /// This is easy to trigger (a random `u32` will most certainly be out of the range
+    /// of the arrays we generate), so by default it is "on". When it's "off", a random
+    /// decision is taken for each index operation whether to apply the modulo or not.
+    pub avoid_index_out_of_bounds: bool,
     /// Try to avoid operations that can result in error when zero is on the RHS.
     pub avoid_err_by_zero: bool,
     /// Avoid using negative integer literals where the frontend expects unsigned types.
@@ -134,6 +141,7 @@ impl Default for Config {
             stmt_freqs_brillig,
             force_brillig: false,
             avoid_overflow: false,
+            avoid_index_out_of_bounds: true,
             avoid_err_by_zero: false,
             avoid_large_int_literals: false,
             avoid_negative_int_literals: false,

tooling/ast_fuzzer/src/program/func.rs

@@ -911,7 +911,7 @@ impl<'a> FunctionContext<'a> {
             unreachable!("only expected to be called with Slice");
         };
         // The rules around dynamic indexing is the same as for arrays.
-        let (idx_expr, idx_dyn) = if max_depth == 0 || bool::arbitrary(u)? {
+        let (mut idx_expr, idx_dyn) = if max_depth == 0 || bool::arbitrary(u)? {
             // Avoid any stack overflow where we look for an index in the slice itself.
             (self.gen_literal(u, &types::U32)?, false)
         } else {
@@ -951,8 +951,10 @@ impl<'a> FunctionContext<'a> {
         // Get the runtime length.
         let len_expr = self.call_array_len(Expression::Ident(ident_1), src_type.clone());
 
-        // Take the modulo.
-        let idx_expr = expr::modulo(idx_expr, len_expr);
+        // Take the modulo, but leave a small chance for index OOB.
+        if self.avoid_index_out_of_bounds(u)? {
+            idx_expr = expr::modulo(idx_expr, len_expr);
+        }
 
         // Access the item by index
         let item_expr = access_item(self, ident_2, idx_expr);
@@ -999,9 +1001,15 @@ impl<'a> FunctionContext<'a> {
     ) -> arbitrary::Result<TrackedExpression> {
         assert!(len > 0, "cannot index empty array");
         if max_depth > 0 && u.ratio(1, 3)? {
-            let (idx, idx_dyn) =
+            let (mut idx, idx_dyn) =
                 self.gen_expr(u, &types::U32, max_depth.saturating_sub(1), Flags::NESTED)?;
-            Ok((expr::index_modulo(idx, len), idx_dyn))
+
+            // Limit the index to be in the valid range for the array length, with a small chance of index OOB.
+            if self.avoid_index_out_of_bounds(u)? {
+                idx = expr::index_modulo(idx, len);
+            }
+
+            Ok((idx, idx_dyn))
         } else {
             let idx = u.choose_index(len as usize)?;
             Ok((expr::u32_literal(idx as u32), false))
@@ -2278,6 +2286,18 @@ impl<'a> FunctionContext<'a> {
             vec![slice, idx, item],
         )
     }
+
+    /// Random decision whether to allow "Index out of bounds" errors to happen
+    /// on a specific array or slice access operation.
+    ///
+    /// If [Config::avoid_index_out_of_bounds] is turned on, then this is always `true`.
+    fn avoid_index_out_of_bounds(&self, u: &mut Unstructured) -> arbitrary::Result<bool> {
+        if self.config().avoid_index_out_of_bounds {
+            return Ok(true);
+        }
+        // Avoid OOB with 90% chance.
+        u.ratio(9, 10)
+    }
 }
 
 #[cfg(test)]