diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs index 6dafa4d2e1e..5c34af41157 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs @@ -127,6 +127,7 @@ use std::sync::Arc; use acvm::{AcirField, FieldElement}; +use im::HashSet; use noirc_errors::call_stack::CallStackId; use crate::ssa::{ @@ -154,7 +155,7 @@ impl Ssa { } } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Copy, Clone)] enum Reachability { /// By default instructions are reachable. Reachable, @@ -175,12 +176,16 @@ impl Function { // after an always failing one was found. let mut current_block_reachability = Reachability::Reachable; + // In some cases a side effect variable can become effective again. + let mut unreachable_predicates = HashSet::new(); + self.simple_optimization(|context| { let block_id = context.block_id; if current_block_id != Some(block_id) { current_block_id = Some(block_id); current_block_reachability = Reachability::Reachable; + unreachable_predicates.clear(); } if current_block_reachability == Reachability::Unreachable { @@ -190,29 +195,35 @@ impl Function { let instruction = context.instruction(); if let Instruction::EnableSideEffectsIf { condition } = instruction { - current_block_reachability = match context.dfg.get_numeric_constant(*condition) { - Some(predicate) if predicate.is_zero() => { - // We can replace side effecting instructions with defaults until the next predicate. - Reachability::UnreachableUnderPredicate - } - _ => Reachability::Reachable, - }; + current_block_reachability = + if let Some(predicate) = context.dfg.get_numeric_constant(*condition) { + // If side effects are turned off, we can replace side effecting instructions with defaults until the next predicate + if predicate.is_zero() { + Reachability::UnreachableUnderPredicate + } else { + Reachability::Reachable + } + } else { + // During loops a previous predicate variable can be restored. + if unreachable_predicates.contains(condition) { + Reachability::UnreachableUnderPredicate + } else { + Reachability::Reachable + } + }; return; }; if current_block_reachability == Reachability::UnreachableUnderPredicate { - // Instructions that don't interact with the predicate should be left alone, - // because the `remove_enable_side_effects` pass might have moved the boundaries around them. - if !instruction.requires_acir_gen_predicate(context.dfg) { - return; + if should_replace_instruction_with_defaults(context) { + remove_and_replace_with_defaults(context, func_id, block_id); } - remove_and_replace_with_defaults(context, func_id, block_id); return; } // Check if the current predicate is known to be enabled. let is_predicate_constant_one = - || match context.dfg.get_numeric_constant(context.enable_side_effects) { + match context.dfg.get_numeric_constant(context.enable_side_effects) { Some(predicate) => predicate.is_one(), None => false, // The predicate is a variable }; @@ -252,34 +263,16 @@ impl Function { binary.requires_acir_gen_predicate(context.dfg); let fails_under_predicate = - requires_acir_gen_predicate && !is_predicate_constant_one(); + requires_acir_gen_predicate && !is_predicate_constant_one; - // Insert the instruction right away so we can add a constrain immediately after it - context.insert_current_instruction(); - - // Insert a constraint which makes it easy to see that this instruction will fail. - let guard = if fails_under_predicate { - context.enable_side_effects - } else { - context.dfg.make_constant(1_u128.into(), NumericType::bool()) - }; - let zero = context.dfg.make_constant(0_u128.into(), NumericType::bool()); - let message = Some(ConstrainError::StaticString(message)); - let instruction = Instruction::Constrain(zero, guard, message); - let call_stack = - context.dfg.get_instruction_call_stack_id(context.instruction_id); - - context.dfg.insert_instruction_and_results( - instruction, - block_id, - None, - call_stack, - ); + insert_constraint(context, block_id, message); // Subsequent instructions can either be removed, of replaced by defaults until the next predicate. current_block_reachability = if fails_under_predicate { + remove_and_replace_with_defaults(context, func_id, block_id); Reachability::UnreachableUnderPredicate } else { + context.remove_current_instruction(); Reachability::Unreachable } } @@ -302,7 +295,7 @@ impl Function { return; } - let always_fail = is_predicate_constant_one(); + let always_fail = is_predicate_constant_one; // We could leave the array operation to trigger an OOB on the invalid access, however if the array contains and returns // references, then the SSA passes still won't be able to deal with them as nothing ever stores to references which are // never created. Instead, we can replace the result of the instruction with defaults, which will allocate and store @@ -341,7 +334,7 @@ impl Function { // If the compiler knows the slice is empty, there is no point trying to pop from it, we know it will fail. // Barretenberg doesn't handle memory operations with predicates, so we can't rely on those to disable the operation // based on the current side effect variable. Instead we need to replace it with a conditional constraint. - let always_fail = is_predicate_constant_one(); + let always_fail = is_predicate_constant_one; // We might think that if the predicate is constant 1, we can leave the pop as it will always fail. // However by turning the block Unreachable, ACIR-gen would create empty bytecode and not fail the circuit. @@ -369,6 +362,11 @@ impl Function { context.dfg[block_id] .set_terminator(TerminatorInstruction::Unreachable { call_stack }); } + if current_block_reachability == Reachability::UnreachableUnderPredicate + && !is_predicate_constant_one + { + unreachable_predicates.insert(context.enable_side_effects); + } }); } } @@ -505,6 +503,55 @@ fn insert_constraint( context.dfg.insert_instruction_and_results(instruction, block_id, None, call_stack); } +/// Check if an instruction should be replaced with default values if we are in the +/// `UnreachableUnderPredicate` mode. +/// +/// These are generally the ones that require an ACIR predicate, except for `ArrayGet`, +/// which might appear safe after having its index replaced by a default zero value, +/// but by doing so we may have made the item and result types misaligned. +fn should_replace_instruction_with_defaults(context: &SimpleOptimizationContext) -> bool { + let instruction = context.instruction(); + + // ArrayGet needs special handling: if we replaced the index with a default value, it could be invalid. + if let Instruction::ArrayGet { array, index, offset: _ } = instruction { + // If we replaced the index with a default, it's going to be zero. + let index_zero = context.dfg.get_numeric_constant(*index).is_some_and(|c| c.is_zero()); + + // If it's zero, make sure that the type in the results + if index_zero { + let typ = match context.dfg.type_of_value(*array) { + Type::Array(typ, _) | Type::Slice(typ) => typ, + other => unreachable!("Array or Slice type expected; got {other:?}"), + }; + let results = context.dfg.instruction_results(context.instruction_id).to_vec(); + let result_type = context.dfg.type_of_value(results[0]); + // If the type doesn't agree then we should not use this any more, + // as the type in the array will replace the type we wanted to get, + // and cause problems further on. + if typ[0] != result_type { + return true; + } + // If the array contains a reference, then we should replace the results + // with defaults because unloaded references also cause issues. + if context.dfg.runtime().is_acir() && result_type.contains_reference() { + return true; + } + // Note that it may be incorrect to replace a *safe* ArrayGet with defaults, + // because `remove_enable_side_effects` may have moved the side effect + // boundaries around them, and then `fold_constants_with_brillig` could + // have replaced some with `enable_side_effect u1 0`. If we then replace + // a *safe* ArrayGet with a default, that might be a result that would + // really be enabled, had it not been skipped over by its original side + // effect variable. Instructions which use its result would then get + // incorrect zero, instead of whatever was in the array. + } + }; + + // Instructions that don't interact with the predicate should be left alone, + // because the `remove_enable_side_effects` pass might have moved the boundaries around them. + instruction.requires_acir_gen_predicate(context.dfg) +} + #[cfg(test)] mod test { use crate::{ @@ -578,7 +625,6 @@ mod test { assert_ssa_snapshot!(ssa, @r#" acir(inline) predicate_pure fn main f0 { b0(): - v2 = sub u32 0, u32 1 constrain u1 0 == u1 1, "attempt to subtract with overflow" unreachable } @@ -601,7 +647,6 @@ mod test { assert_ssa_snapshot!(ssa, @r#" acir(inline) predicate_pure fn main f0 { b0(): - v2 = div u32 1, u32 0 constrain u1 0 == u1 1, "attempt to divide by zero" unreachable } @@ -643,7 +688,6 @@ mod test { acir(inline) predicate_pure fn main f0 { b0(v0: u1): enable_side_effects v0 - v3 = sub u32 0, u32 1 constrain u1 0 == v0, "attempt to subtract with overflow" return u32 0 } @@ -671,7 +715,6 @@ mod test { acir(inline) predicate_pure fn main f0 { b0(v0: u1): enable_side_effects v0 - v3 = sub u32 0, u32 1 constrain u1 0 == v0, "attempt to subtract with overflow" return u32 0 } @@ -726,14 +769,50 @@ mod test { b0(v0: u1): v1 = make_array [] : [&mut u1; 0] enable_side_effects v0 - v4 = mod u32 1, u32 0 constrain u1 0 == v0, "attempt to calculate the remainder with a divisor of zero" constrain u1 0 == v0, "Index out of bounds" - v6 = allocate -> &mut u1 - store u1 0 at v6 - v7 = load v6 -> u1 + v3 = allocate -> &mut u1 + store u1 0 at v3 + v4 = load v3 -> u1 enable_side_effects u1 1 - return v7 + return v4 + } + "#); + } + + #[test] + fn replaces_array_get_following_conditional_constraint_with_default_if_index_was_defaulted() { + let src = r#" + acir(inline) predicate_pure fn main f0 { + b0(v0: u1): + enable_side_effects v0 + v1 = make_array [u8 1, u8 2] : [u8; 2] + v2 = make_array [v1, u1 0] : [([u8; 2], u1); 1] + v3 = mul u32 4294967295, u32 2 // overflow + v4 = add v3, u32 1 // after overflow, replaced by default + enable_side_effects u1 1 // end of side effects mode + enable_side_effects v0 // restore side effects to what we know will fail + v5 = array_get v1, index v4 -> u1 // if v4 is replaced by default, the item at 0 is not a u1 + v6 = unchecked_mul v0, v5 // v5 is no longer a u1, but [u8; 2] + enable_side_effects u1 1 + return + } + "#; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_unreachable_instructions(); + + // The `array_get` is should be replaced by `u1 0` + assert_ssa_snapshot!(ssa, @r#" + acir(inline) predicate_pure fn main f0 { + b0(v0: u1): + enable_side_effects v0 + v3 = make_array [u8 1, u8 2] : [u8; 2] + v5 = make_array [v3, u1 0] : [([u8; 2], u1); 1] + constrain u1 0 == v0, "attempt to multiply with overflow" + enable_side_effects u1 1 + enable_side_effects v0 + enable_side_effects u1 1 + return } "#); } @@ -758,11 +837,9 @@ mod test { acir(inline) predicate_pure fn main f0 { b0(v0: u1): enable_side_effects v0 - v3 = sub u32 0, u32 1 constrain u1 0 == v0, "attempt to subtract with overflow" enable_side_effects u1 1 - v6 = add v3, u32 1 - return v6 + return u32 1 } "#); } @@ -1257,7 +1334,6 @@ mod test { acir(inline) predicate_pure fn main f0 { b0(v0: u1): enable_side_effects v0 - v3 = div u64 1, u64 0 constrain u1 0 == v0, "attempt to divide by zero" enable_side_effects u1 1 return diff --git a/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs b/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs index e66a4101972..50c8ed33d1b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs @@ -130,16 +130,24 @@ impl SimpleOptimizationContext<'_, '_> { self.values_to_replace.insert(from, to); } + /// Check if the instruction has changed relative to its original contents, + /// e.g. because any of its values have been replaced. + fn has_instruction_changed(&self) -> bool { + // If the instruction changed, then there is a chance that we can (or have to) + // simplify it before we insert it back into the block. + let instruction_hash = rustc_hash::FxBuildHasher.hash_one(self.instruction()); + self.orig_instruction_hash != instruction_hash + } + /// Instructs this context to insert the current instruction right away, as opposed /// to doing this at the end of `mutate`'s block (unless `remove_current_instruction is called`). /// - /// If the instruction or its values has relative to their original content, + /// If the instruction or its values has changed relative to their original content, /// we attempt to simplify the instruction before re-inserting it into the block. pub(crate) fn insert_current_instruction(&mut self) { // If the instruction changed, then there is a chance that we can (or have to) // simplify it before we insert it back into the block. - let instruction_hash = rustc_hash::FxBuildHasher.hash_one(self.instruction()); - let simplify = self.orig_instruction_hash != instruction_hash; + let simplify = self.has_instruction_changed(); if simplify { // Based on FunctionInserter::push_instruction_value. @@ -148,6 +156,7 @@ impl SimpleOptimizationContext<'_, '_> { let ctrl_typevars = instruction .requires_ctrl_typevars() .then(|| vecmap(&results, |result| self.dfg.type_of_value(*result))); + let new_results = self.dfg.insert_instruction_and_results_if_simplified( instruction, self.block_id, diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index d493c623a1a..de7d7de1304 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -518,12 +518,16 @@ impl<'a> FunctionContext<'a> { } /// Create a const offset of an address for an array load or store - pub(super) fn make_offset(&mut self, mut address: ValueId, offset: u128) -> ValueId { + pub(super) fn make_offset( + &mut self, + mut address: ValueId, + offset: u128, + unchecked: bool, + ) -> ValueId { if offset != 0 { let typ = self.builder.type_of_value(address).unwrap_numeric(); let offset = self.builder.numeric_constant(offset, typ); - address = - self.builder.insert_binary(address, BinaryOp::Add { unchecked: true }, offset); + address = self.builder.insert_binary(address, BinaryOp::Add { unchecked }, offset); } address } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 0dde800c226..262a0b6eb77 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -466,13 +466,13 @@ impl FunctionContext<'_> { self.builder.numeric_constant(type_size as u128, NumericType::length_type()); let array_type = &self.builder.type_of_value(array); + let runtime = self.builder.current_function.runtime(); // Checks for index Out-of-bounds match array_type { Type::Array(_, len) => { // Out of bounds array accesses are guaranteed to fail in ACIR so this check is performed implicitly. // We then only need to inject it for brillig functions. - let runtime = self.builder.current_function.runtime(); if runtime.is_brillig() { let len = self.builder.numeric_constant(u128::from(*len), NumericType::length_type()); @@ -491,16 +491,20 @@ impl FunctionContext<'_> { _ => unreachable!("must have array or slice but got {array_type}"), } - // This shouldn't overflow because the initial index is within array bounds + // This can overflow if the original index is already not in the bounds of the array + // and we skipped inserting constraints. To stay on the conservative side, start with + // a checked operation, and simplify to unchecked if it can be evaluated. + // In Brillig we will be protected from overflows by constraints, so we can go unchecked. + let unchecked = runtime.is_brillig(); let base_index = self.builder.set_location(location).insert_binary( index, - BinaryOp::Mul { unchecked: true }, + BinaryOp::Mul { unchecked }, type_size, ); let mut field_index = 0u128; Ok(Self::map_type(element_type, |typ| { - let index = self.make_offset(base_index, field_index); + let index = self.make_offset(base_index, field_index, unchecked); field_index += 1; // Reference counting in brillig relies on us incrementing reference @@ -631,7 +635,7 @@ impl FunctionContext<'_> { let result = self.codegen_expression(&for_expr.block); self.codegen_unless_break_or_continue(result, |this, _| { - let new_loop_index = this.make_offset(loop_index, 1); + let new_loop_index = this.make_offset(loop_index, 1, true); this.builder.terminate_with_jmp(loop_entry, vec![new_loop_index]); })?; @@ -1198,7 +1202,7 @@ impl FunctionContext<'_> { // Must remember to increment i before jumping if let Some(loop_index) = loop_.loop_index { - let new_loop_index = self.make_offset(loop_index, 1); + let new_loop_index = self.make_offset(loop_index, 1, true); self.builder.terminate_with_jmp(loop_.loop_entry, vec![new_loop_index]); } else { self.builder.terminate_with_jmp(loop_.loop_entry, vec![]); diff --git a/test_programs/compile_success_with_bug/regression_9888/Nargo.toml b/test_programs/compile_success_with_bug/regression_9888/Nargo.toml new file mode 100644 index 00000000000..1c375361173 --- /dev/null +++ b/test_programs/compile_success_with_bug/regression_9888/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "regression_9888" +type = "bin" +authors = [""] +compiler_unstable_features = ["enums"] + + +[dependencies] diff --git a/test_programs/compile_success_with_bug/regression_9888/src/main.nr b/test_programs/compile_success_with_bug/regression_9888/src/main.nr new file mode 100644 index 00000000000..efd26b204ab --- /dev/null +++ b/test_programs/compile_success_with_bug/regression_9888/src/main.nr @@ -0,0 +1,72 @@ +global G_A: str<1> = "B"; +global G_B: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)) = ( + (218246652875437164009655208784095375107, -313608744114162797896018893666882091480, true), + ["ERD"], "IAT", -120526842023866287823029064655616936845, + (-158893399940351649111039879038827064887, 114242794008481451001418400118000869414, false), +); +fn main() -> return_data (Field, Field, bool) { + let _n = unsafe { + func_3( + if ( + if G_B.0.2 { + let b = G_B; + match b { + (item_c, _item_d, _item_e, _item_f, item_g) => if unsafe { + func_3(G_B, [G_A, G_A], G_B.0.2) + } { + for _idx_h in 177_u8..180_u8 { + for _idx_i in 198_u8..207_u8 { + for _idx_j in 133_u8..135_u8 { + let mut k: [str<1>; 2] = [ + G_A, + if (item_c.2 > unsafe { func_4([]) }[1532016929_u32].3) { + G_A + } else { + if unsafe { func_3(G_B, [G_A, G_A], item_g.2) } { + "N" + } else { + G_A + } + }, + ]; + k = k; + } + } + } + (item_c.2 == G_B.0.2) + } else { + true + }, + } + } else { + G_B.4.2 + } + <= (G_B.4.2 < G_B.0.2) + ) { + G_B + } else { + G_B + }, + [G_A, G_A], + G_B.4.2, + ) + }; + G_B.4 +} + +unconstrained fn func_3( + _a: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)), + _b: [str<1>; 2], + _c: bool, +) -> bool { + true +} + +unconstrained fn func_4(_a: [[str<3>; 4]; 0]) -> [(str<3>, str<3>, bool, bool); 4] { + [ + ("ABC", "CDE", false, true), + ("FGH", "IJK", false, true), + ("LMN", "OPQ", false, true), + ("RST", "UVW", false, true), + ] +} diff --git a/test_programs/execution_failure/regression_9856/Nargo.toml b/test_programs/execution_failure/regression_9856/Nargo.toml new file mode 100644 index 00000000000..8d031643a40 --- /dev/null +++ b/test_programs/execution_failure/regression_9856/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9856" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/regression_9856/src/main.nr b/test_programs/execution_failure/regression_9856/src/main.nr new file mode 100644 index 00000000000..e4e173e5187 --- /dev/null +++ b/test_programs/execution_failure/regression_9856/src/main.nr @@ -0,0 +1,4 @@ +fn main() -> pub bool { + let mut e: [(Field, bool); 1] = [((-1), true)]; + e[2147483648_u32].1 +} diff --git a/test_programs/execution_success/regression_9888/Nargo.toml b/test_programs/execution_success/regression_9888/Nargo.toml new file mode 100644 index 00000000000..61af0095301 --- /dev/null +++ b/test_programs/execution_success/regression_9888/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_9888" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/regression_9888/Prover.toml b/test_programs/execution_success/regression_9888/Prover.toml new file mode 100644 index 00000000000..ed2c12c0a53 --- /dev/null +++ b/test_programs/execution_success/regression_9888/Prover.toml @@ -0,0 +1,2 @@ +b = 1 +return = 1 diff --git a/test_programs/execution_success/regression_9888/src/main.nr b/test_programs/execution_success/regression_9888/src/main.nr new file mode 100644 index 00000000000..bd06f2fecef --- /dev/null +++ b/test_programs/execution_success/regression_9888/src/main.nr @@ -0,0 +1,14 @@ +global G_A: [Field] = &[ + (-89475623400669066614290885116958338078), + (48908553793922955213460396216351077763), + (-334812293488173760196216156611283261679), +]; +fn main(b: pub Field) -> pub Field { + if b == -284015218230919225711528337869756109769 { + -b + } else if b == -64182487568962597178733740932221199270 { + G_A[1887189234_u32] + } else { + b + } +} diff --git a/test_programs/noir_test_success/regression_9174/src/main.nr b/test_programs/noir_test_success/regression_9174/src/main.nr index 0e86953b5ee..9eb6e47abf4 100644 --- a/test_programs/noir_test_success/regression_9174/src/main.nr +++ b/test_programs/noir_test_success/regression_9174/src/main.nr @@ -35,7 +35,7 @@ unconstrained fn func_1_proxy(_a: u32) -> Field { 0 } -#[test(should_fail_with = "Cannot satisfy constraint")] +#[test(should_fail_with = "attempt to calculate the remainder with a divisor of zero")] fn fuzz_main(a: bool, b: bool) -> (Field, Field) { main( a, diff --git a/tooling/ast_fuzzer/src/compare/compiled.rs b/tooling/ast_fuzzer/src/compare/compiled.rs index 64c6ce4831a..88639cad13a 100644 --- a/tooling/ast_fuzzer/src/compare/compiled.rs +++ b/tooling/ast_fuzzer/src/compare/compiled.rs @@ -160,7 +160,18 @@ impl Comparable for NargoErrorWithTypes { || msg.contains("division by zero") }) || both(&msg1, &msg2, |msg| { - msg.contains("attempted to shift by") || msg.contains("bit-shift with overflow") + msg.contains("attempted to shift by") + || msg.contains("shift with overflow") + || msg.contains("shift right with overflow") + || msg.contains("shift left with overflow") + }) + || both(&msg1, &msg2, |msg| { + // In Brillig we have constraints protecting overflows, + // while in ACIR we have checked multiplication unless we know its safe. + msg.contains("multiply with overflow") || msg.contains("index out of bounds") + }) + || both(&msg1, &msg2, |msg| { + msg.contains("add with overflow") || msg.contains("index out of bounds") }) } else { false diff --git a/tooling/ast_fuzzer/src/compare/interpreted.rs b/tooling/ast_fuzzer/src/compare/interpreted.rs index ea52cde1044..114f1088fe4 100644 --- a/tooling/ast_fuzzer/src/compare/interpreted.rs +++ b/tooling/ast_fuzzer/src/compare/interpreted.rs @@ -202,7 +202,9 @@ impl Comparable for ssa::interpreter::errors::InterpreterError { msg2.as_ref().is_some_and(|msg| msg == msg1) } (DivisionByZero { .. }, ConstrainEqFailed { msg, .. }) => { - msg.as_ref().is_some_and(|msg| msg == "attempt to divide by zero") + msg.as_ref().is_some_and(|msg| { + msg == "attempt to divide by zero" || msg.contains("divisor of zero") + }) } (PoppedFromEmptySlice { .. }, ConstrainEqFailed { msg, .. }) => { // The removal of unreachable instructions can replace popping from an empty slice with an always-fail constraint. diff --git a/tooling/ast_fuzzer/src/lib.rs b/tooling/ast_fuzzer/src/lib.rs index 4d2e2886f9b..fa23f242b21 100644 --- a/tooling/ast_fuzzer/src/lib.rs +++ b/tooling/ast_fuzzer/src/lib.rs @@ -109,7 +109,7 @@ impl Default for Config { ("constrain", 4), ]); let stmt_freqs_brillig = Freqs::new(&[ - ("break", 30), + ("break", 35), ("continue", 25), ("assign", 30), ("if", 10), diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__expanded.snap new file mode 100644 index 00000000000..ee6cace504a --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__expanded.snap @@ -0,0 +1,112 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +global G_A: str<1> = "B"; + +global G_B: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)) = ( + (218246652875437164009655208784095375107, -313608744114162797896018893666882091480, true), + ["ERD"], "IAT", -120526842023866287823029064655616936845, + (-158893399940351649111039879038827064887, 114242794008481451001418400118000869414, false), +); + +fn main() -> return_data (Field, Field, bool) { + // Safety: comment added by `nargo expand` + let _n: bool = unsafe { + func_3( + if if G_B.0.2 { + let b: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)) = + G_B; + { + let internal___variable: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)) = + b; + match internal___variable { + ( + internal_match_variable_0, internal_match_variable_1, + internal_match_variable_2, internal_match_variable_3, + internal_match_variable_4, + ) => { + let item_g: (Field, Field, bool) = internal_match_variable_4; + { + let _item_f: Field = internal_match_variable_3; + { + let _item_e: str<3> = internal_match_variable_2; + { + let _item_d: [str<3>; 1] = internal_match_variable_1; + { + let item_c: (Field, Field, bool) = + internal_match_variable_0; + // Safety: comment added by `nargo expand` + if unsafe { func_3(G_B, [G_A, G_A], G_B.0.2) } { + for _idx_h in 177_u8..180_u8 { + for _idx_i in 198_u8..207_u8 { + for _idx_j in 133_u8..135_u8 { + // Safety: comment added by `nargo expand` + let mut k: [str<1>; 2] = [ + G_A, + if item_c.2 + > unsafe { func_4([]) } + [1532016929_u32] + .3 { + G_A + } else { + // Safety: comment added by `nargo expand` + if unsafe { + func_3( + G_B, + [G_A, G_A], + item_g.2, + ) + } { + "N" + } else { + G_A + } + }, + ]; + k = k; + } + } + } + item_c.2 == G_B.0.2 + } else { + true + } + } + } + } + } + }, + } + } + } else { + G_B.4.2 + } + <= (G_B.4.2 < G_B.0.2) { + G_B + } else { + G_B + }, + [G_A, G_A], + G_B.4.2, + ) + }; + G_B.4 +} + +unconstrained fn func_3( + _a: ((Field, Field, bool), [str<3>; 1], str<3>, Field, (Field, Field, bool)), + _b: [str<1>; 2], + _c: bool, +) -> bool { + true +} + +unconstrained fn func_4(_a: [[str<3>; 4]; 0]) -> [(str<3>, str<3>, bool, bool); 4] { + [ + ("ABC", "CDE", false, true), + ("FGH", "IJK", false, true), + ("LMN", "OPQ", false, true), + ("RST", "UVW", false, true), + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__stderr.snap b/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__stderr.snap new file mode 100644 index 00000000000..a2926e6fa0d --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_with_bug/regression_9888/execute__tests__stderr.snap @@ -0,0 +1,61 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stderr +--- +warning: Unnecessary `unsafe` block + ┌─ src/main.nr:14:75 + │ +14 │ (item_c, _item_d, _item_e, _item_f, item_g) => if unsafe { + │ ------ Because it's nested inside another `unsafe` block + │ + +warning: Unnecessary `unsafe` block + ┌─ src/main.nr:22:60 + │ +22 │ if (item_c.2 > unsafe { func_4([]) }[1532016929_u32].3) { + │ ------ Because it's nested inside another `unsafe` block + │ + +warning: Unnecessary `unsafe` block + ┌─ src/main.nr:25:52 + │ +25 │ if unsafe { func_3(G_B, [G_A, G_A], item_g.2) } { + │ ------ Because it's nested inside another `unsafe` block + │ + +warning: Unsafe block must have a safety comment above it + ┌─ src/main.nr:8:14 + │ +8 │ let _n = unsafe { + │ ------ The comment must start with the "Safety: " word + │ + +warning: Unsafe block must have a safety comment above it + ┌─ src/main.nr:14:75 + │ +14 │ (item_c, _item_d, _item_e, _item_f, item_g) => if unsafe { + │ ------ The comment must start with the "Safety: " word + │ + +warning: Unsafe block must have a safety comment above it + ┌─ src/main.nr:22:60 + │ +22 │ if (item_c.2 > unsafe { func_4([]) }[1532016929_u32].3) { + │ ------ The comment must start with the "Safety: " word + │ + +warning: Unsafe block must have a safety comment above it + ┌─ src/main.nr:25:52 + │ +25 │ if unsafe { func_3(G_B, [G_A, G_A], item_g.2) } { + │ ------ The comment must start with the "Safety: " word + │ + +bug: Assertion is always false: attempt to multiply with overflow + ┌─ src/main.nr:22:60 + │ +22 │ if (item_c.2 > unsafe { func_4([]) }[1532016929_u32].3) { + │ ------------------------------------- As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution + │ + = Call stack: + 1. src/main.nr:22:60 diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__expanded.snap new file mode 100644 index 00000000000..3a4ce836fdf --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__expanded.snap @@ -0,0 +1,19 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +global G_A: [Field] = &[ + -89475623400669066614290885116958338078, + 48908553793922955213460396216351077763, + -334812293488173760196216156611283261679, +]; + +fn main(b: pub Field) -> pub Field { + if b == -284015218230919225711528337869756109769_Field { + -b + } else if b == -64182487568962597178733740932221199270_Field { + G_A[1887189234_u32] + } else { + b + } +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__stdout.snap new file mode 100644 index 00000000000..3a7b4a272af --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/regression_9888/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- +[regression_9888] Circuit output: 0x01