From 4b47d64ac49889199805004bfce9529d5407398e Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 14 Apr 2025 10:36:38 +0100 Subject: [PATCH 1/6] Add smoke test for fuzzer --- Cargo.lock | 10 ++++ Cargo.toml | 3 +- cspell.json | 1 + tooling/ast_fuzzer/Cargo.toml | 1 + tooling/ast_fuzzer/tests/smoke.rs | 81 +++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 tooling/ast_fuzzer/tests/smoke.rs diff --git a/Cargo.lock b/Cargo.lock index fe16eb9cc98..74c95956cc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,6 +275,15 @@ version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dde20b3d026af13f561bdd0f15edf01fc734f0dafcedbaf42bba506a9517f223" +[[package]] +name = "arbtest" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a3be567977128c0f71ad1462d9624ccda712193d124e944252f0c5789a06d46" +dependencies = [ + "arbitrary", +] + [[package]] name = "ark-bls12-381" version = "0.5.0" @@ -3295,6 +3304,7 @@ dependencies = [ "acir", "acvm", "arbitrary", + "arbtest", "bn254_blackbox_solver", "build-data", "color-eyre", diff --git a/Cargo.toml b/Cargo.toml index 1077dcfd12b..857a7e8d443 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -153,6 +153,7 @@ rmp = "0.8.14" rmp-serde = "1.3.0" arbitrary = "1.4.1" +arbtest = "0.3.2" cfg-if = "1.0.0" dirs = "4" serde = { version = "1.0.136", features = ["derive"] } @@ -225,4 +226,4 @@ panic = "abort" opt-level = "z" [profile.bench] -lto = true \ No newline at end of file +lto = true diff --git a/cspell.json b/cspell.json index 484858ac3a9..cd5a5706749 100644 --- a/cspell.json +++ b/cspell.json @@ -11,6 +11,7 @@ "aeiou", "appender", "Arbitrum", + "arbtest", "arithmetization", "arity", "arkworks", diff --git a/tooling/ast_fuzzer/Cargo.toml b/tooling/ast_fuzzer/Cargo.toml index 91bb35bed7d..016a2db7302 100644 --- a/tooling/ast_fuzzer/Cargo.toml +++ b/tooling/ast_fuzzer/Cargo.toml @@ -31,4 +31,5 @@ noirc_frontend.workspace = true noir_fuzzer.workspace = true [dev-dependencies] +arbtest.workspace = true rand.workspace = true diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs new file mode 100644 index 00000000000..b0e1732f02d --- /dev/null +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -0,0 +1,81 @@ +//! Smoke test for the AST fuzzer, which generates a bunch of +//! random programs and executes them, without asserting anything +//! about the outcome. The only criteria it needs to pass is not +//! to crash the compiler, which could indicate invalid input. +//! +//! ```shell +//! cargo test -p noir_ast_fuzzer --test smoke +//! ``` +use std::time::Duration; + +use acir::circuit::ExpressionWidth; +use arbtest::arbtest; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use nargo::{NargoError, foreign_calls::DefaultForeignCallBuilder}; +use noir_ast_fuzzer::{Config, arb_inputs, arb_program, program_abi}; +use noirc_evaluator::{brillig::BrilligOptions, ssa}; + +#[test] +fn arb_program_can_be_executed() { + arbtest(|u| { + let program = arb_program(u, Config::default())?; + let abi = program_abi(&program); + + let options = ssa::SsaEvaluatorOptions { + ssa_logging: ssa::SsaLogging::None, + brillig_options: BrilligOptions::default(), + print_codegen_timings: false, + expression_width: ExpressionWidth::default(), + emit_ssa: None, + skip_underconstrained_check: true, + skip_brillig_constraints_check: true, + enable_brillig_constraints_check_lookback: false, + inliner_aggressiveness: 0, + max_bytecode_increase_percent: None, + }; + + // Print the AST if something goes wrong, then panic. + let print_ast_and_panic = |msg: &str| -> ! { + eprintln!("{program}"); + panic!("{msg}") + }; + + // If we have a seed to debug and we know it's going to crash, print the AST. + // eprintln!("{program}"); + + let ssa = ssa::create_program(program.clone(), &options) + .unwrap_or_else(|e| print_ast_and_panic(&format!("Failed to compile program: {e}"))); + + let inputs = arb_inputs(u, &ssa.program, &abi)?; + + let blackbox_solver = Bn254BlackBoxSolver(false); + let initial_witness = abi.encode(&inputs, None).unwrap(); + + let mut foreign_call_executor = + DefaultForeignCallBuilder::default().with_mocks(false).build(); + + let res = nargo::ops::execute_program( + &ssa.program, + initial_witness, + &blackbox_solver, + &mut foreign_call_executor, + ); + + match res { + Err(NargoError::CompilationError) => { + print_ast_and_panic(&format!("Failed to compile program into ACIR.")) + } + Err(NargoError::ForeignCallError(e)) => { + print_ast_and_panic(&format!("Failed to call foreign function: {e}")) + } + Err(NargoError::ExecutionError(_)) | Ok(_) => { + // If some assertion failed, it's okay, we can't tell if it should or shouldn't. + Ok(()) + } + } + }) + //.seed(0xbb4e420300005f5d) // Uncomment and paste the seed printed by arbtest to debug a failure. + .budget(Duration::from_secs(10)) + .size_min(1 << 12) + .size_max(1 << 20); +} From 5ce1333dba2c82d73e34f0b581a06e79241d41ab Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 14 Apr 2025 10:58:12 +0100 Subject: [PATCH 2/6] Fix clippy --- tooling/ast_fuzzer/tests/smoke.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs index b0e1732f02d..0364c99de16 100644 --- a/tooling/ast_fuzzer/tests/smoke.rs +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -12,7 +12,7 @@ use acir::circuit::ExpressionWidth; use arbtest::arbtest; use bn254_blackbox_solver::Bn254BlackBoxSolver; use nargo::{NargoError, foreign_calls::DefaultForeignCallBuilder}; -use noir_ast_fuzzer::{Config, arb_inputs, arb_program, program_abi}; +use noir_ast_fuzzer::{Config, DisplayAstAsNoir, arb_inputs, arb_program, program_abi}; use noirc_evaluator::{brillig::BrilligOptions, ssa}; #[test] @@ -36,7 +36,7 @@ fn arb_program_can_be_executed() { // Print the AST if something goes wrong, then panic. let print_ast_and_panic = |msg: &str| -> ! { - eprintln!("{program}"); + eprintln!("{}", DisplayAstAsNoir(&program)); panic!("{msg}") }; @@ -63,7 +63,7 @@ fn arb_program_can_be_executed() { match res { Err(NargoError::CompilationError) => { - print_ast_and_panic(&format!("Failed to compile program into ACIR.")) + print_ast_and_panic("Failed to compile program into ACIR.") } Err(NargoError::ForeignCallError(e)) => { print_ast_and_panic(&format!("Failed to call foreign function: {e}")) @@ -74,7 +74,7 @@ fn arb_program_can_be_executed() { } } }) - //.seed(0xbb4e420300005f5d) // Uncomment and paste the seed printed by arbtest to debug a failure. + // .seed(0x1796975f00100000) // Uncomment and paste the seed printed by arbtest to debug a failure. .budget(Duration::from_secs(10)) .size_min(1 << 12) .size_max(1 << 20); From f43633cb3169040b0b6fd80463915b9d2e87cfac Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 14 Apr 2025 11:23:11 +0100 Subject: [PATCH 3/6] Add NOIR_ARBTEST_SEED --- tooling/ast_fuzzer/tests/smoke.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs index 0364c99de16..606df4dc807 100644 --- a/tooling/ast_fuzzer/tests/smoke.rs +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -15,9 +15,18 @@ use nargo::{NargoError, foreign_calls::DefaultForeignCallBuilder}; use noir_ast_fuzzer::{Config, DisplayAstAsNoir, arb_inputs, arb_program, program_abi}; use noirc_evaluator::{brillig::BrilligOptions, ssa}; +fn seed_from_env() -> Option { + let Ok(seed) = std::env::var("NOIR_ARBTEST_SEED") else { return None }; + let seed = u64::from_str_radix(seed.trim_start_matches("0x"), 16) + .unwrap_or_else(|e| panic!("failed to parse seed '{seed}': {e}")); + Some(seed) +} + #[test] fn arb_program_can_be_executed() { - arbtest(|u| { + let maybe_seed = seed_from_env(); + + let mut prop = arbtest(|u| { let program = arb_program(u, Config::default())?; let abi = program_abi(&program); @@ -41,7 +50,9 @@ fn arb_program_can_be_executed() { }; // If we have a seed to debug and we know it's going to crash, print the AST. - // eprintln!("{program}"); + if maybe_seed.is_some() { + eprintln!("{program}"); + } let ssa = ssa::create_program(program.clone(), &options) .unwrap_or_else(|e| print_ast_and_panic(&format!("Failed to compile program: {e}"))); @@ -74,8 +85,13 @@ fn arb_program_can_be_executed() { } } }) - // .seed(0x1796975f00100000) // Uncomment and paste the seed printed by arbtest to debug a failure. .budget(Duration::from_secs(10)) .size_min(1 << 12) .size_max(1 << 20); + + if let Some(seed) = maybe_seed { + prop = prop.seed(seed); + } + + prop.run(); } From b9cadfe9a31fc848d94fa6b597a65b5e357d4d7f Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 14 Apr 2025 23:26:48 +0100 Subject: [PATCH 4/6] Do not call ACIR from Brillig --- tooling/ast_fuzzer/src/program/func.rs | 28 +++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tooling/ast_fuzzer/src/program/func.rs b/tooling/ast_fuzzer/src/program/func.rs index 3593b160269..8b9b5573965 100644 --- a/tooling/ast_fuzzer/src/program/func.rs +++ b/tooling/ast_fuzzer/src/program/func.rs @@ -56,6 +56,14 @@ impl FunctionDeclaration { (param_types, return_type) } + + fn is_acir(&self) -> bool { + !self.unconstrained + } + + fn is_brillig(&self) -> bool { + self.unconstrained + } } /// HIR representation of a function parameter. @@ -149,19 +157,33 @@ impl<'a> FunctionContext<'a> { let call_targets = ctx .function_declarations .iter() - .filter_map(|(callee_id, decl)| { + .filter_map(|(callee_id, callee_decl)| { // We can't call `main`. if *callee_id == Program::main_id() { return None; } + // From an ACIR function we can call any Brillig function, // but we avoid creating infinite recursive ACIR calls by // only calling functions with higher IDs than ours, // otherwise the inliner could get stuck. - if !decl.unconstrained && *callee_id <= id { + if decl.is_acir() && callee_decl.is_acir() && *callee_id <= id { return None; } - Some((*callee_id, types::types_produced(&decl.return_type))) + + // From a Brillig function we restrict ourselves to only call + // other Brillig functions. That's because the `Monomorphizer` + // would make an unconstrained copy of any ACIR function called + // from Brillig, and this is expected by the inliner for example, + // but if we did similarly in the generator after we know who + // calls who, we would incur two drawbacks: + // 1) it would make programs bigger for little benefit + // 2) it would skew calibration frequencies as ACIR freqs would overlay Brillig ones + if decl.is_brillig() && !callee_decl.is_brillig() { + return None; + } + + Some((*callee_id, types::types_produced(&callee_decl.return_type))) }) .collect(); From 867baf2e8cbafd4ac33b2db5ac129353c0887ec0 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 24 Apr 2025 14:29:50 +0100 Subject: [PATCH 5/6] Display AST as Noir --- tooling/ast_fuzzer/tests/smoke.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs index 606df4dc807..5ffd7bf8aa1 100644 --- a/tooling/ast_fuzzer/tests/smoke.rs +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -51,7 +51,7 @@ fn arb_program_can_be_executed() { // If we have a seed to debug and we know it's going to crash, print the AST. if maybe_seed.is_some() { - eprintln!("{program}"); + eprintln!("{}", DisplayAstAsNoir(&program)); } let ssa = ssa::create_program(program.clone(), &options) From ac6580d91f098d53c9964ec6f977f26a57def76a Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 24 Apr 2025 14:30:56 +0100 Subject: [PATCH 6/6] Comment about not showing input --- tooling/ast_fuzzer/tests/smoke.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/ast_fuzzer/tests/smoke.rs b/tooling/ast_fuzzer/tests/smoke.rs index 5ffd7bf8aa1..e60f3f319fd 100644 --- a/tooling/ast_fuzzer/tests/smoke.rs +++ b/tooling/ast_fuzzer/tests/smoke.rs @@ -51,6 +51,8 @@ fn arb_program_can_be_executed() { // If we have a seed to debug and we know it's going to crash, print the AST. if maybe_seed.is_some() { + // It could be useful to also show the input, but in the smoke test we're mostly interested in compiler crashes, + // not the execution. For that we have the actual fuzz targets. eprintln!("{}", DisplayAstAsNoir(&program)); }