From c949584a9c2d58e638279a089310a257dc0e2c9e Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 12 Jun 2025 16:03:34 +0000 Subject: [PATCH 1/2] add validation to the SSA fuzzer --- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 2 +- tooling/ssa_executor/src/compiler.rs | 21 +++--- tooling/ssa_fuzzer/src/compiler.rs | 5 +- tooling/ssa_fuzzer/src/lib.rs | 75 ++++++++++++++++++- 4 files changed, 89 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 97430e649c5..6f820174c96 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -137,7 +137,7 @@ pub fn generate_ssa(program: Program) -> Result { Ok(ssa) } -pub(crate) fn validate_ssa(ssa: &Ssa) { +pub fn validate_ssa(ssa: &Ssa) { for function in ssa.functions.values() { validate_function(function); } diff --git a/tooling/ssa_executor/src/compiler.rs b/tooling/ssa_executor/src/compiler.rs index 00560e01bd7..28d95e512d9 100644 --- a/tooling/ssa_executor/src/compiler.rs +++ b/tooling/ssa_executor/src/compiler.rs @@ -15,8 +15,10 @@ use noirc_evaluator::{ errors::{InternalError, RuntimeError}, ssa::{ ArtifactsAndWarnings, SsaBuilder, SsaCircuitArtifact, SsaEvaluatorOptions, SsaLogging, - SsaProgramArtifact, ir::instruction::ErrorType, optimize_ssa_builder_into_acir, - primary_passes, secondary_passes, ssa_gen::Ssa, + SsaProgramArtifact, + ir::instruction::ErrorType, + optimize_ssa_builder_into_acir, primary_passes, secondary_passes, + ssa_gen::{Ssa, validate_ssa}, }, }; use std::collections::{BTreeMap, BTreeSet, HashMap}; @@ -27,13 +29,6 @@ pub fn optimize_ssa_into_acir( ssa: Ssa, options: SsaEvaluatorOptions, ) -> Result { - let builder = SsaBuilder { - ssa, - ssa_logging: options.ssa_logging.clone(), - print_codegen_timings: options.print_codegen_timings, - passed: HashMap::new(), - skip_passes: vec![], - }; let previous_hook = std::panic::take_hook(); let panic_message = std::sync::Arc::new(std::sync::Mutex::new(String::new())); let hook_message = panic_message.clone(); @@ -55,6 +50,14 @@ pub fn optimize_ssa_into_acir( } })); let result = std::panic::catch_unwind(AssertUnwindSafe(|| { + validate_ssa(&ssa); + let builder = SsaBuilder { + ssa, + ssa_logging: options.ssa_logging.clone(), + print_codegen_timings: options.print_codegen_timings, + passed: HashMap::new(), + skip_passes: vec![], + }; optimize_ssa_builder_into_acir( builder, &options, diff --git a/tooling/ssa_fuzzer/src/compiler.rs b/tooling/ssa_fuzzer/src/compiler.rs index f3516219ac6..2c0b7e45e28 100644 --- a/tooling/ssa_fuzzer/src/compiler.rs +++ b/tooling/ssa_fuzzer/src/compiler.rs @@ -11,7 +11,8 @@ use std::collections::BTreeMap; /// Optimizes the given FunctionBuilder into ACIR /// its taken from noirc_evaluator::ssa::optimize_all, but modified to accept FunctionBuilder /// and to catch panics... It cannot be caught with just catch_unwind. -fn optimize_into_acir( +/// This function will also run Ssa validation to make sure that the hand written Ssa has been well formed. +fn optimize_into_acir_and_validate( builder: FunctionBuilder, options: SsaEvaluatorOptions, ) -> Result { @@ -24,7 +25,7 @@ pub fn compile_from_builder( builder: FunctionBuilder, options: &CompileOptions, ) -> Result { - let artifacts = optimize_into_acir(builder, evaluator_options(options))?; + let artifacts = optimize_into_acir_and_validate(builder, evaluator_options(options))?; compile_from_artifacts( artifacts, Abi { parameters: vec![], return_type: None, error_types: BTreeMap::new() }, diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index 91de3d473cc..46639a6bef9 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -10,14 +10,17 @@ pub mod typed_value; #[cfg(test)] mod tests { - use crate::builder::{FuzzerBuilder, InstructionWithTwoArgs}; + use crate::builder::{FuzzerBuilder, FuzzerBuilderError, InstructionWithTwoArgs}; use crate::runner::{CompareResults, run_and_compare}; use crate::typed_value::{TypedValue, ValueType}; use acvm::FieldElement; use acvm::acir::native_types::{Witness, WitnessMap}; - use noirc_driver::CompileOptions; + use noirc_driver::{CompileOptions, CompiledProgram}; use rand::RngCore; + use noirc_evaluator::ssa::ir::instruction::BinaryOp; + use noirc_evaluator::ssa::ir::types::NumericType; + const NUMBER_OF_VARIABLES_INITIAL: u32 = 7; struct TestHelper { @@ -224,4 +227,72 @@ mod tests { run_instruction_double_arg(FuzzerBuilder::insert_shr_instruction, values.clone()); compare_results(values[0] >> values[1], noir_res); } + + #[test] + fn regression_multiplication_without_range_check() { + let mut acir_builder = FuzzerBuilder::new_acir(); + let mut brillig_builder = FuzzerBuilder::new_brillig(); + + let field_acir_var = acir_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + let field_brillig_var = + brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + + let truncated_acir = acir_builder.builder.insert_truncate(field_acir_var, 16, 254); + let truncated_brillig = brillig_builder.builder.insert_truncate(field_brillig_var, 16, 254); + + let field_casted_i16_acir = + acir_builder.builder.insert_cast(truncated_acir, NumericType::Signed { bit_size: 16 }); + let field_casted_i16_brillig = brillig_builder + .builder + .insert_cast(truncated_brillig, NumericType::Signed { bit_size: 16 }); + + let casted_pow_2_acir = acir_builder.builder.insert_binary( + field_casted_i16_acir, + BinaryOp::Mul { unchecked: false }, + field_casted_i16_acir, + ); + let casted_pow_2_brillig = brillig_builder.builder.insert_binary( + field_casted_i16_brillig, + BinaryOp::Mul { unchecked: false }, + field_casted_i16_brillig, + ); + + let last_var = acir_builder.builder.insert_binary( + casted_pow_2_acir, + BinaryOp::Div, + field_casted_i16_acir, + ); + let last_var_brillig = brillig_builder.builder.insert_binary( + casted_pow_2_brillig, + BinaryOp::Div, + field_casted_i16_brillig, + ); + + acir_builder.builder.terminate_with_return(vec![last_var]); + brillig_builder.builder.terminate_with_return(vec![last_var_brillig]); + + let acir_result = acir_builder.compile(CompileOptions::default()); + check_expected_validation_error( + acir_result, + "Signed binary operation does not follow overflow pattern", + ); + + let brillig_result = brillig_builder.compile(CompileOptions::default()); + check_expected_validation_error( + brillig_result, + "Signed binary operation does not follow overflow pattern", + ); + } + + fn check_expected_validation_error( + compilation_result: Result, + expected_message: &str, + ) { + match compilation_result { + Ok(_) => panic!("Expected an SSA validation failure"), + Err(FuzzerBuilderError::RuntimeError(error)) => { + assert!(error.contains(expected_message)) + } + } + } } From f736e938d2ee0025438a0b983573547b4f6bbd81 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 12 Jun 2025 16:11:33 +0000 Subject: [PATCH 2/2] additional regression tests --- tooling/ssa_fuzzer/src/lib.rs | 104 ++++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 10 deletions(-) diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index 46639a6bef9..7965f2ead68 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -228,6 +228,18 @@ mod tests { compare_results(values[0] >> values[1], noir_res); } + fn check_expected_validation_error( + compilation_result: Result, + expected_message: &str, + ) { + match compilation_result { + Ok(_) => panic!("Expected an SSA validation failure"), + Err(FuzzerBuilderError::RuntimeError(error)) => { + assert!(error.contains(expected_message)); + } + } + } + #[test] fn regression_multiplication_without_range_check() { let mut acir_builder = FuzzerBuilder::new_acir(); @@ -284,15 +296,87 @@ mod tests { ); } - fn check_expected_validation_error( - compilation_result: Result, - expected_message: &str, - ) { - match compilation_result { - Ok(_) => panic!("Expected an SSA validation failure"), - Err(FuzzerBuilderError::RuntimeError(error)) => { - assert!(error.contains(expected_message)) - } - } + #[test] + fn regression_cast_without_truncate() { + let mut acir_builder = FuzzerBuilder::new_acir(); + let mut brillig_builder = FuzzerBuilder::new_brillig(); + + let field_var_acir_id_1 = + acir_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + let u64_var_acir_id_2 = acir_builder.insert_variable(ValueType::U64.to_ssa_type()).value_id; + let field_var_brillig_id_1 = + brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + let u64_var_brillig_id_2 = + brillig_builder.insert_variable(ValueType::U64.to_ssa_type()).value_id; + + let casted_acir = acir_builder + .builder + .insert_cast(field_var_acir_id_1, NumericType::Unsigned { bit_size: 64 }); + let casted_brillig = brillig_builder + .builder + .insert_cast(field_var_brillig_id_1, NumericType::Unsigned { bit_size: 64 }); + + let mul_acir = acir_builder.builder.insert_binary( + casted_acir, + BinaryOp::Mul { unchecked: false }, + u64_var_acir_id_2, + ); + let mul_brillig = brillig_builder.builder.insert_binary( + casted_brillig, + BinaryOp::Mul { unchecked: false }, + u64_var_brillig_id_2, + ); + + acir_builder.builder.terminate_with_return(vec![mul_acir]); + brillig_builder.builder.terminate_with_return(vec![mul_brillig]); + + let acir_result = acir_builder.compile(CompileOptions::default()); + check_expected_validation_error( + acir_result, + "Invalid cast from Field, not preceded by valid truncation or known safe value", + ); + let brillig_result = brillig_builder.compile(CompileOptions::default()); + check_expected_validation_error( + brillig_result, + "Invalid cast from Field, not preceded by valid truncation or known safe value", + ); + } + + #[test] + fn regression_signed_sub() { + let mut acir_builder = FuzzerBuilder::new_acir(); + let mut brillig_builder = FuzzerBuilder::new_brillig(); + + let i16_acir_var_1 = acir_builder.insert_variable(ValueType::I16.to_ssa_type()).value_id; + let i16_acir_var_2 = acir_builder.insert_variable(ValueType::I16.to_ssa_type()).value_id; + let i16_brillig_var_1 = + brillig_builder.insert_variable(ValueType::I16.to_ssa_type()).value_id; + let i16_brillig_var_2 = + brillig_builder.insert_variable(ValueType::I16.to_ssa_type()).value_id; + + let sub_acir = acir_builder.builder.insert_binary( + i16_acir_var_1, + BinaryOp::Sub { unchecked: false }, + i16_acir_var_2, + ); + let sub_brillig = brillig_builder.builder.insert_binary( + i16_brillig_var_1, + BinaryOp::Sub { unchecked: false }, + i16_brillig_var_2, + ); + + acir_builder.builder.terminate_with_return(vec![sub_acir]); + brillig_builder.builder.terminate_with_return(vec![sub_brillig]); + + let acir_result = acir_builder.compile(CompileOptions::default()); + check_expected_validation_error( + acir_result, + "Signed binary operation does not follow overflow pattern", + ); + let brillig_result = brillig_builder.compile(CompileOptions::default()); + check_expected_validation_error( + brillig_result, + "Signed binary operation does not follow overflow pattern", + ); } }