From 0aca49559a5dbc4dd192ef70dd2fceac31a7a31a Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 12:44:44 +0100 Subject: [PATCH 1/7] Double the fuzz time --- tooling/ast_fuzzer/fuzz/src/targets/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/ast_fuzzer/fuzz/src/targets/mod.rs b/tooling/ast_fuzzer/fuzz/src/targets/mod.rs index 524b61f2f1b..20c3b1791d2 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/mod.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/mod.rs @@ -46,7 +46,7 @@ mod tests { f(u).unwrap(); Ok(()) }) - .budget(Duration::from_secs(10)) + .budget(Duration::from_secs(20)) .size_min(1 << 12) .size_max(1 << 20); From fd917e3feff0fcc70ab45bb476fe627e771c6e23 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 12:45:07 +0100 Subject: [PATCH 2/7] Enable overflows and negatives in ACIR vs Brillig --- tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 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 2bc28a6174c..5c93ff55faf 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs @@ -10,12 +10,9 @@ use noir_ast_fuzzer::rewrite::change_all_functions_into_unconstrained; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let config = Config { - // We created enough bug tickets due to overflows - avoid_overflow: true, - // also with negative values - avoid_negative_int_literals: true, - // and it gets old to have to edit u128 to fit into u32 for the frontend to parse - avoid_large_int_literals: true, + // 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() }; From 92007561f432e3ec799111cff0fdb190bf70751c Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 14:14:39 +0100 Subject: [PATCH 3/7] Enable overflows and negatives in min vs full --- .../ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs | 1 + tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs | 12 +++--------- 2 files changed, 4 insertions(+), 9 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 5c93ff55faf..c441ed1662f 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/acir_vs_brillig.rs @@ -13,6 +13,7 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { // 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()?, + avoid_large_int_literals: true, ..Default::default() }; 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 709c4f26d64..a6dc0b1fb9c 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs @@ -15,11 +15,8 @@ use noirc_evaluator::ssa::minimal_passes; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let passes = minimal_passes(); let config = Config { - // Try to avoid using overflowing operations; see below for the reason. - avoid_overflow: true, - // Avoid stuff that would be difficult to copy as Noir; we want to verify these failures with nargo. - avoid_large_int_literals: true, - avoid_negative_int_literals: true, + // Overflows are easy to trigger. + avoid_overflow: u.arbitrary()?, ..Default::default() }; @@ -49,10 +46,7 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let result = inputs.exec()?; - // Unfortunately the minimal pipeline can fail on assertions of instructions that get eliminated from the final pipeline, - // so if the minimal version fails and the final succeeds, it is most likely because of some overflow in a variable that - // was ultimately unused. Therefore we only compare results if both succeeded, or if only the final failed. - if matches!(result, CompareResult::BothFailed(_, _) | CompareResult::LeftFailed(_, _)) { + if matches!(result, CompareResult::BothFailed(_, _)) { Ok(()) } else { compare_results_compiled(&inputs, &result) From 0fb26fe1190e71bd3843165a38b35870c248a8b6 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 14:34:18 +0100 Subject: [PATCH 4/7] Enable overflows in pass-vs-prev and orig-vs-morph --- tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs | 2 +- tooling/ast_fuzzer/fuzz/src/targets/pass_vs_prev.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 7ff4f71015c..c7078d4a2f1 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs @@ -16,7 +16,7 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let rules = rules::all(); let max_rewrites = 10; let config = Config { - avoid_negative_int_literals: true, + avoid_overflow: u.arbitrary()?, avoid_large_int_literals: true, ..Default::default() }; diff --git a/tooling/ast_fuzzer/fuzz/src/targets/pass_vs_prev.rs b/tooling/ast_fuzzer/fuzz/src/targets/pass_vs_prev.rs index 8cfcf8f82df..7e424b1b1a2 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/pass_vs_prev.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/pass_vs_prev.rs @@ -13,7 +13,7 @@ use noirc_evaluator::ssa::ssa_gen::Ssa; use noirc_evaluator::ssa::{SsaPass, primary_passes, ssa_gen}; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { - let config = Config::default(); + let config = Config { avoid_overflow: u.arbitrary()?, ..Config::default() }; let inputs = CompareInterpreted::arb(u, config, |u, program| { let options = CompareOptions::arbitrary(u)?; From 8cf47bc207c285056bcd04150a2b07bc064239bd Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 16:20:58 +0100 Subject: [PATCH 5/7] Enable overflow in comptime-vs-brillig --- .../fuzz/src/targets/comptime_vs_brillig.rs | 19 +++++++------------ .../fuzz/src/targets/min_vs_full.rs | 1 + 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig.rs b/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig.rs index e21d56f17ec..121dae55c24 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig.rs @@ -13,24 +13,19 @@ use noir_ast_fuzzer::rewrite::change_all_functions_into_unconstrained; pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let config = Config { - // We created enough bug tickets due to overflows - // TODO(#8817): Comptime code fails to compile if there is an overflow, which causes a panic. - avoid_overflow: true, - // also with negative values - avoid_negative_int_literals: true, - // also divisions - avoid_err_by_zero: true, - // and it gets old to have to edit u128 to fit into u32 for the frontend to parse + // It's easy to overflow. + avoid_overflow: u.arbitrary()?, + // Avoid using large integers in for loops that the frontend would reject. avoid_large_int_literals: true, + // Also avoid negative integers, because the frontend rejects them for loops. + avoid_negative_int_literals: true, // Avoid break/continue avoid_loop_control: true, - // TODO(#8817): Comptime code fails to compile if there is an assertion failure, which causes a panic. - avoid_constrain: true, // Has to only use expressions valid in comptime comptime_friendly: true, - // Force brillig + // Force brillig, to generate loops that the interpreter can do but ACIR cannot. force_brillig: true, - // Use lower limits because of the interpreter. + // Use lower limits because of the interpreter, to avoid stack overflow max_loop_size: 5, max_recursive_calls: 5, ..Default::default() 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 a6dc0b1fb9c..56ce7e8e716 100644 --- a/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs +++ b/tooling/ast_fuzzer/fuzz/src/targets/min_vs_full.rs @@ -17,6 +17,7 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> { let config = Config { // Overflows are easy to trigger. avoid_overflow: u.arbitrary()?, + avoid_large_int_literals: true, ..Default::default() }; From d4a33e17b9c9621c46925ae757fb8c298f508826 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 9 Jun 2025 16:53:02 +0100 Subject: [PATCH 6/7] Turn custom diagnostics from comptime into a runtime assertion error --- tooling/ast_fuzzer/src/compare/compiled.rs | 7 +++ tooling/ast_fuzzer/src/compare/comptime.rs | 56 +++++++++++++++++----- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/tooling/ast_fuzzer/src/compare/compiled.rs b/tooling/ast_fuzzer/src/compare/compiled.rs index 4a35283985f..9b629123e88 100644 --- a/tooling/ast_fuzzer/src/compare/compiled.rs +++ b/tooling/ast_fuzzer/src/compare/compiled.rs @@ -80,6 +80,13 @@ impl Comparable for NargoError { }; match (ee1, ee2) { + ( + AssertionFailed(ResolvedAssertionPayload::String(c), _, _), + AssertionFailed(_, _, _), + ) if c.contains("CustomDiagnostic") => { + // Looks like the workaround we have for comptime failures originating from overflows and similar assertion failures. + true + } (AssertionFailed(p1, _, _), AssertionFailed(p2, _, _)) => p1 == p2, (SolvingError(s1, _), SolvingError(s2, _)) => format!("{s1}") == format!("{s2}"), (SolvingError(s, _), AssertionFailed(p, _, _)) diff --git a/tooling/ast_fuzzer/src/compare/comptime.rs b/tooling/ast_fuzzer/src/compare/comptime.rs index 060e617ad6c..f54ad7e682a 100644 --- a/tooling/ast_fuzzer/src/compare/comptime.rs +++ b/tooling/ast_fuzzer/src/compare/comptime.rs @@ -7,6 +7,8 @@ use std::{cell::RefCell, collections::BTreeMap}; use arbitrary::Unstructured; use bn254_blackbox_solver::Bn254BlackBoxSolver; use color_eyre::eyre::{self, WrapErr}; +use nargo::NargoError; +use nargo::errors::ExecutionError; use nargo::{foreign_calls::DefaultForeignCallBuilder, parse_all}; use noirc_abi::Abi; use noirc_driver::{ @@ -75,20 +77,10 @@ pub struct CompareComptime { impl CompareComptime { /// Execute the Noir code and the SSA, then compare the results. pub fn exec(&self) -> eyre::Result { - log::debug!("comptime src:\n{}", self.source); - let (program1, output1) = match prepare_and_compile_snippet( - self.source.clone(), - self.force_brillig, - Vec::new(), - ) { - Ok(((program, output), _)) => (program, output), - Err(e) => panic!("failed to compile program:\n{}\n{e:?}", self.source), - }; - let comptime_print = String::from_utf8(output1).expect("should be valid utf8 string"); - let blackbox_solver = Bn254BlackBoxSolver(false); let initial_witness = self.abi.encode(&BTreeMap::new(), None).wrap_err("abi::encode")?; + // Execute a compiled Program. let do_exec = |program| { let mut print = Vec::new(); @@ -108,9 +100,49 @@ impl CompareComptime { (res, print) }; - let (res1, print1) = do_exec(&program1.program); + // Execute the 2nd (Brillig) program. let (res2, print2) = do_exec(&self.ssa.artifact.program); + // Try to compile the 1st (comptime) version from string. + log::debug!("comptime src:\n{}", self.source); + let (program1, output1) = match prepare_and_compile_snippet( + self.source.clone(), + self.force_brillig, + Vec::new(), + ) { + Ok(((program, output), _)) => (program, output), + Err(e) => { + // If the comptime code failed to compile, it could be because it executed the code + // and encountered an overflow, which would be a runtime error in Brillig. + let is_assertion = e.iter().any(|e| { + e.secondaries.iter().any(|s| s.message == "Assertion failed") + || e.message.contains("overflow") + || e.message.contains("divide by zero") + }); + if is_assertion { + let msg = format!("{e:?}"); + let err = ExecutionError::AssertionFailed( + acvm::pwg::ResolvedAssertionPayload::String(msg), + vec![], + None, + ); + let res1 = Err(NargoError::ExecutionError(err)); + return CompareCompiledResult::new( + &self.abi, + (res1, "".to_string()), + (res2, print2), + ); + } else { + panic!("failed to compile program:\n{e:?}\n{}", self.source); + } + } + }; + // Capture any println that happened during the compilation, which in these tests should be the whole program. + let comptime_print = String::from_utf8(output1).expect("should be valid utf8 string"); + + // Execute the 1st (comptime) program. + let (res1, print1) = do_exec(&program1.program); + CompareCompiledResult::new(&self.abi, (res1, comptime_print + &print1), (res2, print2)) } From 96e0c8b0b0ed7e3beceba5fc806c43157809750f Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 10 Jun 2025 08:52:43 +0100 Subject: [PATCH 7/7] Print 'as' for numeric literals at comptime --- .../noirc_frontend/src/monomorphization/printer.rs | 10 +++++++++- tooling/ast_fuzzer/src/program/mod.rs | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index 8e4d877f864..19045e530a8 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -28,6 +28,7 @@ pub struct AstPrinter { pub show_clone_and_drop: bool, pub show_print_as_std: bool, pub show_type_in_let: bool, + pub show_type_of_int_literal: bool, } impl Default for AstPrinter { @@ -39,6 +40,7 @@ impl Default for AstPrinter { show_clone_and_drop: true, show_print_as_std: false, show_type_in_let: false, + show_type_of_int_literal: false, } } } @@ -245,7 +247,13 @@ impl AstPrinter { self.print_comma_separated(&array.contents, f)?; write!(f, "]") } - super::ast::Literal::Integer(x, _, _) => x.fmt(f), + super::ast::Literal::Integer(x, typ, _) => { + if self.show_type_of_int_literal { + write!(f, "{x} as {typ}") + } else { + x.fmt(f) + } + } super::ast::Literal::Bool(x) => x.fmt(f), super::ast::Literal::Str(s) => { if s.contains("\"") { diff --git a/tooling/ast_fuzzer/src/program/mod.rs b/tooling/ast_fuzzer/src/program/mod.rs index 83853f0e870..92194bca0c0 100644 --- a/tooling/ast_fuzzer/src/program/mod.rs +++ b/tooling/ast_fuzzer/src/program/mod.rs @@ -531,6 +531,10 @@ impl std::fmt::Display for DisplayAstAsNoir<'_> { printer.show_clone_and_drop = false; printer.show_print_as_std = true; printer.show_type_in_let = true; + // Most of the time it doesn't affect testing, except the comptime tests where + // we parse back the code. For that we use `DisplayAstAsNoirComptime`. + // It is quite noisy, so keep it off by default. + printer.show_type_of_int_literal = false; printer.print_program(self.0, f) } } @@ -551,6 +555,10 @@ impl std::fmt::Display for DisplayAstAsNoirComptime<'_> { // Declare the type in `let` so that when we parse snippets we can match the types which // the AST had, otherwise a literal which was a `u32` in the AST might be inferred as `Field`. printer.show_type_in_let = true; + // Also annotate literals with their type, so we don't have subtle differences in expressions, + // for example `for i in (5 / 10) as u32 .. 2` is `0..2` or `1..2` depending on whether 5 and 10 + // were some number in the AST or `Field` when parsed by the test. + printer.show_type_of_int_literal = true; for function in &self.0.functions { if function.id == Program::main_id() { let mut function = function.clone();