From 162dac01b88712ec0f545abcb363406a5d92eb70 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 10 Sep 2025 10:00:37 +0100 Subject: [PATCH 1/3] Allow index OOB --- tooling/ast_fuzzer/src/compare/comptime.rs | 10 ++++--- tooling/ast_fuzzer/src/compare/interpreted.rs | 3 ++ tooling/ast_fuzzer/src/program/func.rs | 28 +++++++++++++++---- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/tooling/ast_fuzzer/src/compare/comptime.rs b/tooling/ast_fuzzer/src/compare/comptime.rs index 7838c39bfb0..f2cd9281621 100644 --- a/tooling/ast_fuzzer/src/compare/comptime.rs +++ b/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, diff --git a/tooling/ast_fuzzer/src/compare/interpreted.rs b/tooling/ast_fuzzer/src/compare/interpreted.rs index 1ffdaaed544..c04d49cbc50 100644 --- a/tooling/ast_fuzzer/src/compare/interpreted.rs +++ b/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. diff --git a/tooling/ast_fuzzer/src/program/func.rs b/tooling/ast_fuzzer/src/program/func.rs index a42d8324cfb..52d5901dc21 100644 --- a/tooling/ast_fuzzer/src/program/func.rs +++ b/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 { 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,16 @@ 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. + fn avoid_index_out_of_bounds(&self, u: &mut Unstructured) -> arbitrary::Result { + // We often use the `avoid_overflow` configuration in tests to turn off "easy to trigger errors". + if self.config().avoid_overflow { + return Ok(false); + } + u.ratio(1, 10) + } } #[cfg(test)] From df61425db2950aa7a2b9bf6f03e3a8a951ec7ce8 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 11 Sep 2025 11:52:03 +0100 Subject: [PATCH 2/3] Add avoid_index_out_of_bounds --- .../fuzz/src/targets/acir_vs_brillig.rs | 9 ++------- .../src/targets/comptime_vs_brillig_direct.rs | 5 ++--- .../src/targets/comptime_vs_brillig_nargo.rs | 5 ++--- .../ast_fuzzer/fuzz/src/targets/min_vs_full.rs | 11 +++-------- tooling/ast_fuzzer/fuzz/src/targets/mod.rs | 17 +++++++++++++++++ .../fuzz/src/targets/orig_vs_morph.rs | 5 +++-- tooling/ast_fuzzer/src/lib.rs | 8 ++++++++ tooling/ast_fuzzer/src/program/func.rs | 10 ++++++---- 8 files changed, 43 insertions(+), 27 deletions(-) diff --git a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs b/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs index be9d5ca8fdf..a7c45b59d79 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs +++ b/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, diff --git a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_direct.rs b/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_direct.rs index 17d321055c6..887783ceef3 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_direct.rs +++ b/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| { diff --git a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_nargo.rs b/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_nargo.rs index 60fc57f316b..6238d4b7456 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig_nargo.rs +++ b/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| { diff --git a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs b/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs index ea320c5a74b..b93ddc7c37c 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs +++ b/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, diff --git a/tooling/ast_fuzzer/fuzz/src/targets/mod.rs b/tooling/ast_fuzzer/fuzz/src/targets/mod.rs index a5b73c1b279..b6f8b520984 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/mod.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/mod.rs @@ -1,3 +1,6 @@ +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; @@ -5,6 +8,20 @@ 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 { + // 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 { diff --git a/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs b/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs index 59be42e5e6f..2493dc57822 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs +++ b/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, diff --git a/tooling/ast_fuzzer/src/lib.rs b/tooling/ast_fuzzer/src/lib.rs index 59c3f8c20e0..4d2e2886f9b 100644 --- a/tooling/ast_fuzzer/src/lib.rs +++ b/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, diff --git a/tooling/ast_fuzzer/src/program/func.rs b/tooling/ast_fuzzer/src/program/func.rs index 52d5901dc21..085895cd955 100644 --- a/tooling/ast_fuzzer/src/program/func.rs +++ b/tooling/ast_fuzzer/src/program/func.rs @@ -2289,12 +2289,14 @@ impl<'a> FunctionContext<'a> { /// Random decision whether to allow "Index out of bounds" errors to happen /// on a specific array or slice access operation. + /// + /// If `avoid_oob` is turned on in the config, then this is always `true`. fn avoid_index_out_of_bounds(&self, u: &mut Unstructured) -> arbitrary::Result { - // We often use the `avoid_overflow` configuration in tests to turn off "easy to trigger errors". - if self.config().avoid_overflow { - return Ok(false); + if self.config().avoid_index_out_of_bounds { + return Ok(true); } - u.ratio(1, 10) + // Avoid OOB with 90% chance. + u.ratio(9, 10) } } From e6ed0e16b145804e774172aa4dfdae6bf0300af2 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 11 Sep 2025 11:55:34 +0100 Subject: [PATCH 3/3] Fix docstring --- tooling/ast_fuzzer/src/program/func.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/ast_fuzzer/src/program/func.rs b/tooling/ast_fuzzer/src/program/func.rs index 085895cd955..7b6015878aa 100644 --- a/tooling/ast_fuzzer/src/program/func.rs +++ b/tooling/ast_fuzzer/src/program/func.rs @@ -2290,7 +2290,7 @@ impl<'a> FunctionContext<'a> { /// Random decision whether to allow "Index out of bounds" errors to happen /// on a specific array or slice access operation. /// - /// If `avoid_oob` is turned on in the config, then this is always `true`. + /// 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 { if self.config().avoid_index_out_of_bounds { return Ok(true);