Skip to content
Merged
8 changes: 4 additions & 4 deletions compiler/noirc_evaluator/src/ssa/interpreter/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ pub enum InterpreterError {
/// These errors are all the result from malformed input SSA
#[error("{0}")]
Internal(InternalError),
#[error("constrain {lhs_id} == {rhs_id} failed:\n {lhs} != {rhs}")]
ConstrainEqFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId },
#[error("constrain {lhs_id} != {rhs_id} failed:\n {lhs} == {rhs}")]
ConstrainNeFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId },
#[error("constrain {lhs_id} == {rhs_id}{msg} failed:\n {lhs} != {rhs}")]
ConstrainEqFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId, msg: String },
#[error("constrain {lhs_id} != {rhs_id}{msg} failed:\n {lhs} == {rhs}")]
ConstrainNeFailed { lhs: String, lhs_id: ValueId, rhs: String, rhs_id: ValueId, msg: String },
#[error("static_assert `{condition}` failed: {message}")]
StaticAssertFailed { condition: ValueId, message: String },
#[error(
Expand Down
30 changes: 27 additions & 3 deletions compiler/noirc_evaluator/src/ssa/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
ir::{
dfg::DataFlowGraph,
function::{Function, FunctionId, RuntimeType},
instruction::{ArrayOffset, Binary, BinaryOp, Instruction, TerminatorInstruction},
instruction::{
ArrayOffset, Binary, BinaryOp, ConstrainError, Instruction, TerminatorInstruction,
},
types::Type,
value::ValueId,
},
Expand Down Expand Up @@ -413,7 +415,18 @@
let rhs = rhs.to_string();
let lhs_id = *lhs_id;
let rhs_id = *rhs_id;
return Err(InterpreterError::ConstrainEqFailed { lhs, lhs_id, rhs, rhs_id });
let msg = if let Some(ConstrainError::StaticString(msg)) = constrain_error {
format!(", \"{msg}\"")
} else {
"".to_string()
};
return Err(InterpreterError::ConstrainEqFailed {
lhs,
lhs_id,
rhs,
rhs_id,
msg,
});
}
Ok(())
}
Expand All @@ -425,7 +438,18 @@
let rhs = rhs.to_string();
let lhs_id = *lhs_id;
let rhs_id = *rhs_id;
return Err(InterpreterError::ConstrainNeFailed { lhs, lhs_id, rhs, rhs_id });
let msg = if let Some(ConstrainError::StaticString(msg)) = constrain_error {
format!(", \"{msg}\"")
} else {
"".to_string()
};
return Err(InterpreterError::ConstrainNeFailed {
lhs,
lhs_id,
rhs,
rhs_id,
msg,
});
}
Ok(())
}
Expand Down Expand Up @@ -877,7 +901,7 @@
}
}

macro_rules! apply_int_binop {

Check warning on line 904 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
($lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{
use value::NumericValue::*;
match ($lhs, $rhs) {
Expand Down Expand Up @@ -908,7 +932,7 @@
}};
}

macro_rules! apply_int_binop_opt {

Check warning on line 935 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
($dfg:expr, $lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{
use value::NumericValue::*;

Expand Down Expand Up @@ -1008,7 +1032,7 @@
}));
}

// Disable this instruction if it is side-effectful and side effects are disabled.

Check warning on line 1035 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
if !self.side_effects_enabled() && binary.requires_acir_gen_predicate(self.dfg()) {
let zero = NumericValue::zero(lhs.get_type());
return Ok(Value::Numeric(zero));
Expand All @@ -1025,13 +1049,13 @@
let dfg = self.dfg();
let result = match binary.operator {
BinaryOp::Add { unchecked: false } => {
apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedAdd::checked_add)

Check warning on line 1052 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Add { unchecked: true } => {
apply_int_binop!(lhs, rhs, binary, num_traits::WrappingAdd::wrapping_add)

Check warning on line 1055 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Sub { unchecked: false } => {
apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedSub::checked_sub)

Check warning on line 1058 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Sub { unchecked: true } => {
apply_int_binop!(lhs, rhs, binary, num_traits::WrappingSub::wrapping_sub)
Expand Down
12 changes: 9 additions & 3 deletions compiler/noirc_frontend/src/monomorphization/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,15 @@ impl AstPrinter {
)?;
self.print_expr(&let_expr.expression, f)
}
Expression::Constrain(expr, ..) => {
write!(f, "constrain ")?;
self.print_expr(expr, f)
Expression::Constrain(expr, _, payload) => {
write!(f, "assert(")?;
self.print_expr(expr, f)?;
if let Some(payload) = payload {
write!(f, ", ")?;
self.print_expr(&payload.as_ref().0, f)?;
}
write!(f, ")")?;
Ok(())
}
Expression::Assign(assign) => {
self.print_lvalue(&assign.lvalue, f)?;
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/ownership/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ fn last_use_in_if_branches() {
unconstrained fn main$f0(d$l0: [Field; 2]) -> () {
if (len$f1(d$l0.clone()) == 2) {
if (len$f1(d$l0.clone()) == 2) {
constrain eq$f2(d$l0, [5, 6]);
assert(eq$f2(d$l0, [5, 6]));
}
} else {
constrain eq$f2(d$l0, [5, 6]);
assert(eq$f2(d$l0, [5, 6]));
}
}
unconstrained fn len$f1(arr$l1: [Field; 2]) -> u32 {
Expand Down
3 changes: 3 additions & 0 deletions tooling/ast_fuzzer/fuzz/src/targets/comptime_vs_brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ 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,
Expand All @@ -23,6 +24,8 @@ pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
avoid_large_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
Expand Down
62 changes: 32 additions & 30 deletions tooling/ast_fuzzer/fuzz/src/targets/orig_vs_morph.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
//! Perform random equivalence mutations on the AST and check that the
//! execution result does not change, a.k.a. metamorphic testing.

use std::collections::{HashSet, VecDeque};

use crate::{compare_results_compiled, create_ssa_or_die, default_ssa_options};
use arbitrary::{Arbitrary, Unstructured};
use color_eyre::eyre;
use noir_ast_fuzzer::compare::{CompareMorph, CompareOptions};
use noir_ast_fuzzer::visitor;
use noir_ast_fuzzer::{Config, visitor::visit_expr_mut};
use noir_ast_fuzzer::{expr, visitor};
use noirc_frontend::ast::UnaryOp;
use noirc_frontend::monomorphization::ast::{Expression, FuncId, Function, Program, Unary};
use noirc_frontend::monomorphization::ast::{
Call, Definition, Expression, Function, Ident, Program, Unary,
};

pub fn fuzz(u: &mut Unstructured) -> eyre::Result<()> {
let rules = rules::all();
Expand Down Expand Up @@ -42,9 +42,8 @@ fn rewrite_program(
rules: &[rules::Rule],
max_rewrites: usize,
) {
let reachable = reachable_functions(program);
for func in program.functions.iter_mut() {
if func.name.ends_with("_proxy") || !reachable.contains(&func.id) {
if func.name.ends_with("_proxy") {
continue;
}
rewrite_function(u, func, rules, max_rewrites);
Expand Down Expand Up @@ -92,9 +91,11 @@ impl MorphContext<'_> {
}
match expr {
Expression::For(for_) => {
// Separate context for just the ranges.
let range_ctx = rules::Context { is_in_range: true, ..*ctx };
self.rewrite_expr(&range_ctx, u, &mut for_.start_range);
self.rewrite_expr(&range_ctx, u, &mut for_.end_range);
// Original context for the body.
self.rewrite_expr(ctx, u, &mut for_.block);
// No need to visit children, we just visited them.
false
Expand All @@ -106,6 +107,14 @@ impl MorphContext<'_> {
self.rewrite_expr(&ctx, u, &mut unary.rhs);
false
}
Expression::Call(call) if is_special_call(call) => {
let ctx = rules::Context { is_in_special_call: true, ..*ctx };
for arg in call.arguments.iter_mut() {
self.rewrite_expr(&ctx, u, arg);
}
false
}
// The rest can just have the rules applied on them, using the same context.
_ => {
for rule in self.rules {
match self.try_apply_rule(ctx, u, expr, rule) {
Expand Down Expand Up @@ -174,29 +183,15 @@ fn estimate_applicable_rules(
count
}

/// Collect the functions reachable from `main`.
///
/// We don't want to waste our time morphing functions that won't be called.
///
/// It would be nice if they were removed during AST generation, but if we
/// remove an item from `Programs::functions`, the calls made to them would
/// need to be updated according to their new position in the vector.
fn reachable_functions(program: &Program) -> HashSet<FuncId> {
Comment thread
aakoshh marked this conversation as resolved.
let mut reachable = HashSet::new();
let mut queue = VecDeque::new();

queue.push_back(Program::main_id());

while let Some(func_id) = queue.pop_front() {
if !reachable.insert(func_id) {
continue;
}
let func = &program.functions[func_id.0 as usize];
let callees = expr::callees(&func.body);
queue.extend(callees);
}

reachable
/// Check if we are calling an oracle or builtin function.
fn is_special_call(call: &Call) -> bool {
matches!(
call.func.as_ref(),
Expression::Ident(Ident {
definition: Definition::Oracle(_) | Definition::Builtin(_) | Definition::LowLevel(_),
..
})
)
}

/// Metamorphic transformation rules.
Expand All @@ -212,10 +207,12 @@ mod rules {
pub struct Context {
/// Is the function we're rewriting unconstrained?
pub unconstrained: bool,
/// Are we rewriting an expression which is a range of a `for` loop?
/// Are we rewriting an expression which is a `start` or `end` of a `for` loop?
pub is_in_range: bool,
/// Are we in an expression that we're just taking a mutable reference to?
pub is_in_ref_mut: bool,
/// Are we processing the arguments of an non-user function call, such as an oracle or built-in?
pub is_in_special_call: bool,
}

/// Check if the rule can be applied on an expression.
Expand Down Expand Up @@ -301,6 +298,11 @@ mod rules {
if ctx.is_in_ref_mut {
return false;
}
// We don't want to mess with the arguments of a `println`, because the printer assumes they are bool literals.
// Similarly a `constrain` call is expected to have a single boolean expression.
if ctx.is_in_special_call {
return false;
}
// We can apply boolean rule on anything that returns a bool,
// unless the expression can have a side effect, which we don't want to duplicate.
if let Some(typ) = expr.return_type() {
Expand Down
15 changes: 15 additions & 0 deletions tooling/ast_fuzzer/src/compare/interpreted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
}

pub fn exec(&self) -> eyre::Result<CompareInterpretedResult> {
// Debug prints up fron tin case the interpreter panics. Turn them on with `RUST_LOG=debug cargo test ...`

Check warning on line 77 in tooling/ast_fuzzer/src/compare/interpreted.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (fron)
log::debug!("Program: \n{}\n", crate::DisplayAstAsNoir(&self.program));
log::debug!(
"ABI inputs: \n{}\n",
Expand Down Expand Up @@ -123,7 +123,7 @@
false
}
(Overflow { instruction: i1 }, Overflow { instruction: i2 }) => {
// Overflows can occur or uncomparable instructions, but in a parentheses it contains the values that caused it.

Check warning on line 126 in tooling/ast_fuzzer/src/compare/interpreted.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (uncomparable)
fn details(s: &str) -> Option<&str> {
let start = s.find("(")?;
let end = s.find(")")?;
Expand All @@ -134,6 +134,21 @@
}
details_or_sanitize(i1) == details_or_sanitize(i2)
}
(
ConstrainEqFailed { lhs: _lhs1, rhs: _rhs1, msg: msg1, .. },
ConstrainEqFailed { lhs: _lhs2, rhs: _rhs2, msg: msg2, .. },
)
| (
ConstrainNeFailed { lhs: _lhs1, rhs: _rhs1, msg: msg1, .. },
ConstrainNeFailed { lhs: _lhs2, rhs: _rhs2, msg: msg2, .. },
) => {
// The sides might be flipped: `u1 0 == u1 1` vs `u1 1 == u1 0`.
// Unfortunately we often see the type change as well, which makes it more difficult to compare,
// for example `Field 313339671284855045676773137498590239475 != Field 0` vs `u128 313339671284855045676773137498590239475 != u128 0`,
// or `i64 -1615928006 != i64 -5568658583620095790` vs `u64 18446744072093623610 != u64 12878085490089455826`
// (lhs1 == lhs2 && rhs1 == rhs2 || lhs1 == rhs2 && rhs1 == lhs2) && msg1 == msg2
msg1 == msg2
}
(e1, e2) => {
// The format strings contain SSA instructions,
// where the only difference might be the value ID.
Expand Down
2 changes: 1 addition & 1 deletion tooling/ast_fuzzer/src/compare/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ where
// Both programs failed the same way.
Ok(None)
} else {
bail!("both programs failed: {e1} vs {e2}\n{e1:?}\n{e2:?}")
bail!("both programs failed:\n{e1}\n!=\n{e2}\n\n{e1:?}\n{e2:?}")
}
}
CompareResult::LeftFailed(e, _) => {
Expand Down
5 changes: 5 additions & 0 deletions tooling/ast_fuzzer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub struct Config {
pub avoid_loop_control: bool,
/// Avoid using function pointers in parameters.
pub avoid_lambdas: bool,
/// Avoid using constrain statements.
pub avoid_constrain: bool,
/// Only use comptime friendly expressions.
pub comptime_friendly: bool,
}
Expand All @@ -85,6 +87,7 @@ impl Default for Config {
("for", 22),
("let", 25),
("call", 5),
("constrain", 5),
]);
let stmt_freqs_brillig = Freqs::new(&[
("drop", 0),
Expand All @@ -98,6 +101,7 @@ impl Default for Config {
("let", 20),
("call", 5),
("print", 15),
("constrain", 10),
]);
Self {
max_globals: 3,
Expand All @@ -122,6 +126,7 @@ impl Default for Config {
avoid_negative_int_literals: false,
avoid_loop_control: false,
avoid_lambdas: false,
avoid_constrain: false,
comptime_friendly: false,
}
}
Expand Down
30 changes: 29 additions & 1 deletion tooling/ast_fuzzer/src/program/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
types,
};

/// Use random strings to identify constraints.
const CONSTRAIN_MSG_TYPE: Type = Type::String(3);

/// Something akin to a forward declaration of a function, capturing the details required to:
/// 1. call the function from the other function bodies
/// 2. generate the final HIR function signature
Expand Down Expand Up @@ -693,7 +696,6 @@
Freq::new(u, &self.ctx.config.stmt_freqs_acir)?
};
// TODO(#7926): Match
// TODO(#7932): Constrain

// Start with `drop`, it doesn't need to be frequent even if others are disabled.
if freq.enabled("drop") {
Expand All @@ -702,6 +704,13 @@
}
}

// We don't want constraints to get too frequent, as it could dominate all outcome.
if freq.enabled_when("constrain", !self.ctx.config.avoid_constrain) {
if let Some(e) = self.gen_constrain(u)? {
return Ok(e);
}
}

// Require a positive budget, so that we have some for the block itself and its contents.
if freq.enabled_when("if", self.budget > 1) {
return self.gen_if(u, &Type::Unit, self.max_depth(), Flags::TOP);
Expand Down Expand Up @@ -906,6 +915,25 @@
Ok(Some(call))
}

/// Generate a `constrain` statement, if there is some local variable we can do it on.
///
/// Arbitrary constraints are very likely to fail, so we don't want too many of them,
/// otherwise they might mask disagreements in return values.
fn gen_constrain(&mut self, u: &mut Unstructured) -> arbitrary::Result<Option<Expression>> {
// Generate a condition that evaluates to bool.
let Some(cond) = self.gen_binary(u, &Type::Bool, self.max_depth())? else {
return Ok(None);
};
// Generate a unique message for the assertion, so it's easy to find which one failed.
let msg = expr::gen_literal(u, &CONSTRAIN_MSG_TYPE)?;
let cons = Expression::Constrain(
Box::new(cond),
Location::dummy(),
Some(Box::new((msg, types::to_hir_type(&CONSTRAIN_MSG_TYPE)))),
);
Ok(Some(cons))
}

/// Generate an if-then-else statement or expression.
fn gen_if(
&mut self,
Expand Down Expand Up @@ -1332,8 +1360,8 @@
ctx.config.max_loop_size = 10;
ctx.config.vary_loop_size = false;
ctx.gen_main_decl(&mut u);
let mut fctx = FunctionContext::new(&mut ctx, FuncId(0));

Check warning on line 1363 in tooling/ast_fuzzer/src/program/func.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (fctx)
fctx.budget = 2;

Check warning on line 1364 in tooling/ast_fuzzer/src/program/func.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (fctx)
let loop_code = format!("{}", fctx.gen_loop(&mut u).unwrap()).replace(" ", "");

println!("{loop_code}");
Expand Down
11 changes: 10 additions & 1 deletion tooling/ast_fuzzer/tests/calibration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! ```shell
//! cargo test -p noir_ast_fuzzer --test calibration -- --nocapture
//! ```
use std::collections::BTreeMap;
use std::{collections::BTreeMap, ops::RangeInclusive};

use arbtest::arbtest;
use noir_ast_fuzzer::{Config, arb_program, visitor::visit_expr};
Expand Down Expand Up @@ -67,6 +67,13 @@ fn arb_program_freqs_in_expected_range() {
keys.iter().map(|key| counts[&unconstrained][group][key] * 100 / total).sum::<usize>()
};

let assert_both = |group: &str, key: &str, range: RangeInclusive<usize>| {
let a = freq_100(false, group, &[key]);
let b = freq_100(true, group, &[key]);
assert!(range.contains(&a), "ACIR {group}/{key} should be in {range:?}: {a}");
assert!(range.contains(&b), "Brillig {group}/{key} should be in {range:?}: {b}");
};

// Assert relative frequencies
let loops_a = freq_100(false, "stmt", &["for"]);
let loops_b = freq_100(true, "stmt", &["for", "loop", "while"]);
Expand All @@ -76,6 +83,8 @@ fn arb_program_freqs_in_expected_range() {
assert!(loop_range.contains(&loops_a), "ACIR loops should be ~10: {loops_a}");
assert!(loop_range.contains(&loops_b), "Brillig loops should be ~10: {loops_b}");
assert!(break_b >= loops_b, "Brillig should break out of loops: {break_b} >= {loops_b}");

assert_both("stmt", "constrain", 1..=3);
}

/// Classify the expression into "expr" or "stmt" for frequency settings.
Expand Down
Loading