From 031a098f1a5b7c5c1ee53c91f1fe3b756ec56995 Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 13:45:08 +0000 Subject: [PATCH 1/7] fix --- .../src/brillig/brillig_ir/instructions.rs | 1 + tooling/ssa_fuzzer/src/lib.rs | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs index 95023b9284f..2e142210620 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs @@ -156,6 +156,7 @@ impl BrilligContext< right.bit_size ); let bit_size = left.bit_size; + assert!(bit_size != 254, "Attempt to modulo fields"); let scratch_var_i = SingleAddrVariable::new(self.allocate_register(), bit_size); let scratch_var_j = SingleAddrVariable::new(self.allocate_register(), bit_size); diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index 1ab4c7a05c8..be2a2e900b7 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -10,10 +10,12 @@ pub mod typed_value; mod tests { use crate::builder::{FuzzerBuilder, InstructionWithTwoArgs}; use crate::config; + use crate::runner::execute_single; 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_evaluator::ssa::ir::instruction::BinaryOp; use rand::RngCore; struct TestHelper { @@ -225,4 +227,39 @@ mod tests { run_instruction_double_arg(FuzzerBuilder::insert_shr_instruction, values.clone()); compare_results(values[0] >> values[1], noir_res); } + + #[test] + fn regression_fields_mod_op_brillig() { + let mut brillig_builder = FuzzerBuilder::new_brillig(); + let field_var_brillig_id_1 = + brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + let field_var_brillig_id_2 = + brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; + + let mod_brillig = brillig_builder.builder.insert_binary( + field_var_brillig_id_1, + BinaryOp::Mod, + field_var_brillig_id_2, + ); + + brillig_builder.builder.terminate_with_return(vec![mod_brillig]); + let brillig_program = brillig_builder.compile(); + let brillig_program = match brillig_program { + Ok(program) => program, + Err(_e) => { + // compilation failed, its nice + return; + } + }; + + let mut initial_witness = WitnessMap::new(); + initial_witness.insert(Witness(0), FieldElement::from(13371337_u32)); + initial_witness.insert(Witness(1), FieldElement::from(12341234_u32)); + + let result_witness = Witness(2); + let execution_result = + execute_single(&brillig_program.program, initial_witness, result_witness); + println!("{:?}", execution_result); + assert!(execution_result.is_err()); + } } From 505bfc94da3626e277c18c67eaa8646d8177131c Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 14:47:16 +0000 Subject: [PATCH 2/7] change hardoced 254 to Field::bitsize; remove confusion --- .../src/brillig/brillig_ir/instructions.rs | 2 +- tooling/ssa_fuzzer/src/lib.rs | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs index 2e142210620..edbcd854d45 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs @@ -156,7 +156,7 @@ impl BrilligContext< right.bit_size ); let bit_size = left.bit_size; - assert!(bit_size != 254, "Attempt to modulo fields"); + assert!(bit_size != BitSize::Field.to_u32::(), "Attempt to modulo fields"); let scratch_var_i = SingleAddrVariable::new(self.allocate_register(), bit_size); let scratch_var_j = SingleAddrVariable::new(self.allocate_register(), bit_size); diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index be2a2e900b7..3d51ab2b117 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -10,7 +10,6 @@ pub mod typed_value; mod tests { use crate::builder::{FuzzerBuilder, InstructionWithTwoArgs}; use crate::config; - use crate::runner::execute_single; use crate::runner::{CompareResults, run_and_compare}; use crate::typed_value::{TypedValue, ValueType}; use acvm::FieldElement; @@ -244,22 +243,12 @@ mod tests { brillig_builder.builder.terminate_with_return(vec![mod_brillig]); let brillig_program = brillig_builder.compile(); - let brillig_program = match brillig_program { - Ok(program) => program, + match brillig_program { + Ok(_program) => panic!("Should fail on compilation"), Err(_e) => { // compilation failed, its nice return; } }; - - let mut initial_witness = WitnessMap::new(); - initial_witness.insert(Witness(0), FieldElement::from(13371337_u32)); - initial_witness.insert(Witness(1), FieldElement::from(12341234_u32)); - - let result_witness = Witness(2); - let execution_result = - execute_single(&brillig_program.program, initial_witness, result_witness); - println!("{:?}", execution_result); - assert!(execution_result.is_err()); } } From 29f0437fbbe16b25de26461fc4688e6e6b0a4ddb Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 14:54:04 +0000 Subject: [PATCH 3/7] clippy --- tooling/ssa_fuzzer/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index 3d51ab2b117..17f92357214 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -247,7 +247,6 @@ mod tests { Ok(_program) => panic!("Should fail on compilation"), Err(_e) => { // compilation failed, its nice - return; } }; } From cb5115bbce25aa0af76321e0ce52965ea64ce7f7 Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 15:12:32 +0000 Subject: [PATCH 4/7] replace test with parset --- .../noirc_evaluator/src/ssa/parser/tests.rs | 20 +++++++++++++++ tooling/ssa_fuzzer/src/lib.rs | 25 ------------------- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 1cdbcbc62a3..e87bfe3f4a6 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -633,3 +633,23 @@ fn test_parses_keyword_in_function_name() { "; assert_ssa_roundtrip(src); } + +#[test] +fn regression_modulo_fields_brillig() { + use crate::brillig::BrilligOptions; + use std::panic::AssertUnwindSafe; + + let src = " + brillig(inline) predicate_pure fn main f0 { + b0(v0: Field, v1: Field): + v2 = mod v0, v1 + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + match std::panic::catch_unwind(AssertUnwindSafe(|| ssa.to_brillig(&BrilligOptions::default()))) + { + Ok(_) => panic!("Should fail on compilation"), + Err(_) => (), + } +} diff --git a/tooling/ssa_fuzzer/src/lib.rs b/tooling/ssa_fuzzer/src/lib.rs index 17f92357214..1ab4c7a05c8 100644 --- a/tooling/ssa_fuzzer/src/lib.rs +++ b/tooling/ssa_fuzzer/src/lib.rs @@ -14,7 +14,6 @@ mod tests { use crate::typed_value::{TypedValue, ValueType}; use acvm::FieldElement; use acvm::acir::native_types::{Witness, WitnessMap}; - use noirc_evaluator::ssa::ir::instruction::BinaryOp; use rand::RngCore; struct TestHelper { @@ -226,28 +225,4 @@ mod tests { run_instruction_double_arg(FuzzerBuilder::insert_shr_instruction, values.clone()); compare_results(values[0] >> values[1], noir_res); } - - #[test] - fn regression_fields_mod_op_brillig() { - let mut brillig_builder = FuzzerBuilder::new_brillig(); - let field_var_brillig_id_1 = - brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; - let field_var_brillig_id_2 = - brillig_builder.insert_variable(ValueType::Field.to_ssa_type()).value_id; - - let mod_brillig = brillig_builder.builder.insert_binary( - field_var_brillig_id_1, - BinaryOp::Mod, - field_var_brillig_id_2, - ); - - brillig_builder.builder.terminate_with_return(vec![mod_brillig]); - let brillig_program = brillig_builder.compile(); - match brillig_program { - Ok(_program) => panic!("Should fail on compilation"), - Err(_e) => { - // compilation failed, its nice - } - }; - } } From fe701dbe5ae6587a07430a9ac2451548f7f41ac8 Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 15:21:58 +0000 Subject: [PATCH 5/7] clippy again --- compiler/noirc_evaluator/src/ssa/parser/tests.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index e87bfe3f4a6..382e47e5318 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -647,9 +647,8 @@ fn regression_modulo_fields_brillig() { } "; let ssa = Ssa::from_str(src).unwrap(); - match std::panic::catch_unwind(AssertUnwindSafe(|| ssa.to_brillig(&BrilligOptions::default()))) - { - Ok(_) => panic!("Should fail on compilation"), - Err(_) => (), - } + assert!( + std::panic::catch_unwind(AssertUnwindSafe(|| ssa.to_brillig(&BrilligOptions::default()))) + .is_err() + ); } From 43dcc26b2fb8fdd05e1b8cdfb37d57cca9f84af2 Mon Sep 17 00:00:00 2001 From: defkit <84741533+jewelofchaos9@users.noreply.github.com> Date: Wed, 16 Apr 2025 15:28:25 +0000 Subject: [PATCH 6/7] Update compiler/noirc_evaluator/src/ssa/parser/tests.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- compiler/noirc_evaluator/src/ssa/parser/tests.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 382e47e5318..279b26a5897 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -635,6 +635,7 @@ fn test_parses_keyword_in_function_name() { } #[test] +#[should_panic = "Attempt to modulo fields"] fn regression_modulo_fields_brillig() { use crate::brillig::BrilligOptions; use std::panic::AssertUnwindSafe; @@ -647,8 +648,5 @@ fn regression_modulo_fields_brillig() { } "; let ssa = Ssa::from_str(src).unwrap(); - assert!( - std::panic::catch_unwind(AssertUnwindSafe(|| ssa.to_brillig(&BrilligOptions::default()))) - .is_err() - ); + ssa.to_brillig(&BrilligOptions::default()); } From 46210035ceb09f4ef4c84f81aeecc98d153a1e6d Mon Sep 17 00:00:00 2001 From: jewelofchaos9 Date: Wed, 16 Apr 2025 15:28:58 +0000 Subject: [PATCH 7/7] remove unused import --- compiler/noirc_evaluator/src/ssa/parser/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 279b26a5897..9f401aab10a 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -638,7 +638,6 @@ fn test_parses_keyword_in_function_name() { #[should_panic = "Attempt to modulo fields"] fn regression_modulo_fields_brillig() { use crate::brillig::BrilligOptions; - use std::panic::AssertUnwindSafe; let src = " brillig(inline) predicate_pure fn main f0 {