Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: avoid integer overflows #2713

Merged
merged 73 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
c3c7ba5
error on integer overflow
guipublic Sep 15, 2023
a783b24
range constraint block box is not supported in brillig
guipublic Sep 15, 2023
0ae7b49
Merge branch 'master' into gd/integer_overflow
guipublic Sep 18, 2023
cddd13b
Merge branch 'master' into gd/integer_overflow
guipublic Sep 20, 2023
0511c9d
Merge branch 'master' into gd/integer_overflow
guipublic Sep 25, 2023
187f77a
Merge branch 'master' into gd/integer_overflow
guipublic Sep 29, 2023
8e611d3
note: fix signed arithmetic
vezenovm Oct 9, 2023
558a6b0
Merge branch 'master' into gd/integer_overflow
guipublic Oct 11, 2023
2ed45ac
check overflow for signed integers
guipublic Oct 12, 2023
c4c4f3c
code review
guipublic Oct 12, 2023
f9cd90d
Merge branch 'master' into gd/integer_overflow
guipublic Oct 12, 2023
e0f71d1
Merge branch 'master' into gd/integer_overflow
guipublic Oct 16, 2023
d08ca79
Fix issue with sha2_byte
guipublic Oct 16, 2023
458fde9
remove debug code
guipublic Oct 16, 2023
34292f5
Merge branch 'master' into gd/integer_overflow
guipublic Oct 16, 2023
fa2d1e4
Merge branch 'master' into gd/integer_overflow
guipublic Oct 18, 2023
f7333c4
Merge branch 'master' into gd/integer_overflow
guipublic Oct 24, 2023
f214e7d
chore: Update ACIR artifacts (#3263)
kevaundray Oct 24, 2023
3d50049
fix: recompile artefacts from a different noir version (#3248)
guipublic Oct 24, 2023
8528283
feat(debugger): Print limited source code context (#3217)
mverzilli Oct 24, 2023
7c9b414
chore(docs): Document `nargo fmt` (#3262)
Savio-Sou Oct 24, 2023
7c4444f
chore(docs): Rearrange NoirJS section (#3260)
Savio-Sou Oct 24, 2023
e1d5077
feat: noir-wasm takes dependency graph (#3213)
alexghr Oct 24, 2023
9e9dc5d
feat!: Switch to new pedersen implementation (#3151)
kevaundray Oct 24, 2023
84f408e
feat: replace boolean range constraints with arithmetic opcodes (#3234)
TomAFrench Oct 24, 2023
66a8453
feat: implement euclidean division and signed division in terms of `A…
TomAFrench Oct 24, 2023
3f4f2ee
chore: fix empty constructor formatting (#3265)
Oct 24, 2023
9ce4237
feat: add crate for pub modifier (#3271)
guipublic Oct 24, 2023
bc02046
chore: method specialization (#3268)
signorecello Oct 24, 2023
a2b078f
fix: Fix lexer error formatting (#3274)
jfecher Oct 24, 2023
5c3fb2d
chore: standardize workflow triggers (#3277)
TomAFrench Oct 24, 2023
7b08b21
chore: add constrain formatter (#3272)
Oct 24, 2023
6ae06ed
fix: Add size checks to integer literals (#3236)
jfecher Oct 24, 2023
12fc657
feat: implement `bound_constraint_with_offset` in terms of `AcirVar`s…
TomAFrench Oct 24, 2023
33c083e
chore: adding fix to master branch (#3279)
signorecello Oct 24, 2023
a456bdb
feat!: expose pedersen hash in acir and bb solver (#3269)
sirasistant Oct 25, 2023
0d07ad9
chore: turn off `aztec` flag by default (#3280)
TomAFrench Oct 25, 2023
7b7f861
chore: Release Noir(0.18.0) (#3242)
kevaundray Oct 25, 2023
2917c01
chore: pinning NoirJS guide versions to 0.17.0 and adding note on noi…
signorecello Oct 25, 2023
cd6c003
feat: Extract Brillig VM to allow step debugging (#3259)
ggiraldez Oct 25, 2023
e5eda81
feat: perform compile-time euclidean division on constants (#3231)
TomAFrench Oct 25, 2023
3f0bd3a
chore: update artefacts (#3287)
kevaundray Oct 25, 2023
7a534e9
chore: remove `default` empty features array (#3285)
alexghr Oct 25, 2023
c08fd00
chore(docs): Cut v0.17.0 docs (#3290)
Savio-Sou Oct 25, 2023
3758f19
chore: update relative paths for docs imports (#3291)
critesjosh Oct 25, 2023
2d3d6f6
chore(docs): Recommend `yarn` instead of for cutting doc versions (#3…
Savio-Sou Oct 25, 2023
593f3c1
feat: Allow a trait to be implemented multiple times for the same str…
jfecher Oct 25, 2023
a5245b2
chore: format submodule/contract (#3286)
Oct 25, 2023
e8a9274
chore(github): Update PR template per new documentation workflow (#3241)
Savio-Sou Oct 25, 2023
23bb4ce
feat: handle warnings in evaluator (#3205)
guipublic Oct 26, 2023
b9e5b5f
feat: `compute_note_hash_and_nullifier` check (#3216)
benesjan Oct 26, 2023
7950bc9
feat: Make generic impls callable (#3297)
jfecher Oct 26, 2023
ab982ef
chore: create publish-docs.yml (#3298)
signorecello Oct 26, 2023
219f862
chore(docs): Supplement descriptions for defaulting loop indices to b…
Savio-Sou Oct 26, 2023
881add7
chore: bump bb version to 0.12.0 (#3304)
TomAFrench Oct 26, 2023
07f4f93
chore: upload acir artifacts as a github artifact (#3288)
TomAFrench Oct 26, 2023
1c01fbd
feat: add exports of JS black box solvers to noirJS (#3295)
TomAFrench Oct 26, 2023
ffacd52
chore: add bn254 attribute when needed in the stdlib (#3208)
guipublic Oct 26, 2023
fc03aa4
chore: recrawl docs on merge (#3306)
signorecello Oct 26, 2023
bab2fca
Merge branch 'master' into gd/integer_overflow
guipublic Oct 30, 2023
04663f4
fix the merge
guipublic Oct 30, 2023
339ae8a
feat!: expose pedersen hash in acir and bb solver (#3269)
guipublic Oct 30, 2023
3af8912
Merge branch 'master' into gd/integer_overflow
guipublic Oct 30, 2023
2ec6998
Merge branch 'master' into gd/integer_overflow
guipublic Oct 30, 2023
76ffae7
Merge branch 'master' into gd/integer_overflow
guipublic Oct 30, 2023
5358c10
Merge branch 'master' into gd/integer_overflow
guipublic Oct 30, 2023
e74c98d
fix merge issues
guipublic Oct 30, 2023
6d259c1
fix test case
guipublic Oct 31, 2023
024a949
Merge branch 'master' into gd/integer_overflow
guipublic Oct 31, 2023
7623e97
Merge branch 'master' into gd/integer_overflow
kevaundray Oct 31, 2023
acdd2db
Code review
guipublic Oct 31, 2023
84131b8
Merge branch 'master' into gd/integer_overflow
guipublic Oct 31, 2023
f13b01c
Merge branch 'master' into gd/integer_overflow
guipublic Oct 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use acvm::brillig_vm::brillig::HeapVector;
use acvm::FieldElement;
use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
use iter_extended::vecmap;
use num_bigint::BigUint;

use super::brillig_black_box::convert_black_box_call;
use super::brillig_block_variables::BlockVariables;
Expand Down Expand Up @@ -554,6 +555,25 @@ impl<'block> BrilligBlock<'block> {
value_variable,
);
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let left = self.convert_ssa_register_value(*value, dfg);
let max = BigUint::from(2_u128).pow(*max_bit_size);
let right = self.brillig_context.allocate_register();
self.brillig_context.const_instruction(
right,
FieldElement::from_be_bytes_reduce(&max.to_bytes_be()).into(),
);

let brillig_binary_op = BrilligBinaryOp::Integer {
op: BinaryIntOp::LessThan,
bit_size: max_bit_size + 1,
};
let condition = self.brillig_context.allocate_register();
self.brillig_context.binary_instruction(left, right, condition, brillig_binary_op);
self.brillig_context.constrain_instruction(condition, assert_message.clone());
self.brillig_context.deallocate_register(condition);
self.brillig_context.deallocate_register(right);
}
_ => todo!("ICE: Instruction not supported {instruction:?}"),
};

Expand Down
22 changes: 18 additions & 4 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,15 +656,23 @@ impl AcirContext {
let remainder_var = r_value.into_var()?;

// Constrain `q < 2^{max_q_bits}`.
self.range_constrain_var(quotient_var, &NumericType::Unsigned { bit_size: max_q_bits })?;
self.range_constrain_var(
quotient_var,
&NumericType::Unsigned { bit_size: max_q_bits },
None,
)?;

// Constrain `r < 2^{max_rhs_bits}`.
//
// If `rhs` is a power of 2, then is just a looser version of the following bound constraint.
// In the case where `rhs` isn't a power of 2 then this range constraint is required
// as the bound constraint creates a new witness.
// This opcode will be optimized out if it is redundant so we always add it for safety.
self.range_constrain_var(remainder_var, &NumericType::Unsigned { bit_size: max_rhs_bits })?;
self.range_constrain_var(
remainder_var,
&NumericType::Unsigned { bit_size: max_rhs_bits },
None,
)?;

// Constrain `r < rhs`.
self.bound_constraint_with_offset(remainder_var, rhs, predicate, max_rhs_bits)?;
Expand Down Expand Up @@ -768,12 +776,12 @@ impl AcirContext {
let r_var = self.add_constant(r.into());
let aor = self.add_var(lhs_offset, r_var)?;
// lhs_offset<=rhs_offset <=> lhs_offset + r < rhs_offset + r = 2^bit_size <=> witness < 2^bit_size
self.range_constrain_var(aor, &NumericType::Unsigned { bit_size })?;
self.range_constrain_var(aor, &NumericType::Unsigned { bit_size }, None)?;
return Ok(());
}
// General case: lhs_offset<=rhs <=> rhs-lhs_offset>=0 <=> rhs-lhs_offset is a 'bits' bit integer
let sub_expression = self.sub_var(rhs, lhs_offset)?; //rhs-lhs_offset
self.range_constrain_var(sub_expression, &NumericType::Unsigned { bit_size: bits })?;
self.range_constrain_var(sub_expression, &NumericType::Unsigned { bit_size: bits }, None)?;

Ok(())
}
Expand Down Expand Up @@ -874,6 +882,7 @@ impl AcirContext {
&mut self,
variable: AcirVar,
numeric_type: &NumericType,
message: Option<String>,
) -> Result<AcirVar, RuntimeError> {
match numeric_type {
NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => {
Expand All @@ -886,6 +895,11 @@ impl AcirContext {
}
let witness = self.var_to_witness(variable)?;
self.acir_ir.range_constraint(witness, *bit_size)?;
if let Some(message) = message {
self.acir_ir
.assert_messages
.insert(self.acir_ir.last_acir_opcode_location(), message);
}
}
NumericType::NativeField => {
// Range constraining a Field is a no-op
Expand Down
10 changes: 9 additions & 1 deletion compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ impl Context {
) -> Result<AcirVar, RuntimeError> {
let acir_var = self.acir_context.add_variable();
if matches!(numeric_type, NumericType::Signed { .. } | NumericType::Unsigned { .. }) {
self.acir_context.range_constrain_var(acir_var, numeric_type)?;
self.acir_context.range_constrain_var(acir_var, numeric_type, None)?;
}
Ok(acir_var)
}
Expand Down Expand Up @@ -529,6 +529,14 @@ impl Context {
Instruction::Load { .. } => {
unreachable!("Expected all load instructions to be removed before acir_gen")
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let acir_var = self.convert_numeric_value(*value, dfg)?;
self.acir_context.range_constrain_var(
acir_var,
&NumericType::Unsigned { bit_size: *max_bit_size },
assert_message.clone(),
)?;
}
}
self.acir_context.set_call_stack(CallStack::new());
Ok(())
Expand Down
79 changes: 73 additions & 6 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ pub(crate) enum Instruction {
/// Constrains two values to be equal to one another.
Constrain(ValueId, ValueId, Option<String>),

/// Range constrain `value` to `max_bit_size`
RangeCheck { value: ValueId, max_bit_size: u32, assert_message: Option<String> },

/// Performs a function call with a list of its arguments.
Call { func: ValueId, arguments: Vec<ValueId> },

Expand Down Expand Up @@ -203,7 +206,8 @@ impl Instruction {
Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array),
Instruction::Constrain(..)
| Instruction::Store { .. }
| Instruction::EnableSideEffects { .. } => InstructionResultType::None,
| Instruction::EnableSideEffects { .. }
| Instruction::RangeCheck { .. } => InstructionResultType::None,
Instruction::Load { .. } | Instruction::ArrayGet { .. } | Instruction::Call { .. } => {
InstructionResultType::Unknown
}
Expand All @@ -230,9 +234,12 @@ impl Instruction {
Truncate { .. } => false,

// These either have side-effects or interact with memory
Constrain(..) | EnableSideEffects { .. } | Allocate | Load { .. } | Store { .. } => {
false
}
Constrain(..)
| EnableSideEffects { .. }
| Allocate
| Load { .. }
| Store { .. }
| RangeCheck { .. } => false,

Call { func, .. } => match dfg[*func] {
Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
Expand Down Expand Up @@ -263,7 +270,7 @@ impl Instruction {
| ArrayGet { .. }
| ArraySet { .. } => false,

Constrain(..) | Store { .. } | EnableSideEffects { .. } => true,
Constrain(..) | Store { .. } | EnableSideEffects { .. } | RangeCheck { .. } => true,

// Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
Call { func, .. } => match dfg[*func] {
Expand Down Expand Up @@ -320,6 +327,13 @@ impl Instruction {
Instruction::ArraySet { array, index, value } => {
Instruction::ArraySet { array: f(*array), index: f(*index), value: f(*value) }
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
Instruction::RangeCheck {
value: f(*value),
max_bit_size: *max_bit_size,
assert_message: assert_message.clone(),
}
}
}
}

Expand Down Expand Up @@ -364,6 +378,9 @@ impl Instruction {
Instruction::EnableSideEffects { condition } => {
f(*condition);
}
Instruction::RangeCheck { value, .. } => {
f(*value);
}
}
}

Expand Down Expand Up @@ -461,6 +478,14 @@ impl Instruction {
Instruction::Allocate { .. } => None,
Instruction::Load { .. } => None,
Instruction::Store { .. } => None,
Instruction::RangeCheck { value, max_bit_size, .. } => {
if let Some(numeric_constant) = dfg.get_numeric_constant(*value) {
if numeric_constant.num_bits() < *max_bit_size {
return Remove;
}
}
None
}
}
}
}
Expand All @@ -484,7 +509,11 @@ fn simplify_cast(value: ValueId, dst_typ: &Type, dfg: &mut DataFlowGraph) -> Sim
SimplifiedTo(dfg.make_constant(constant, dst_typ.clone()))
}
(
Type::Numeric(NumericType::NativeField | NumericType::Unsigned { .. }),
Type::Numeric(
NumericType::NativeField
| NumericType::Unsigned { .. }
| NumericType::Signed { .. },
),
Type::Numeric(NumericType::Unsigned { bit_size }),
) => {
// Field/Unsigned -> unsigned: truncate
Expand Down Expand Up @@ -822,6 +851,29 @@ fn eval_constant_binary_op(
let result = function(lhs, rhs);
truncate(result, *bit_size).into()
}
Type::Numeric(NumericType::Signed { bit_size }) => {
let function = operator.get_i128_function();

let lhs = truncate(lhs.try_into_u128()?, *bit_size);
let rhs = truncate(rhs.try_into_u128()?, *bit_size);
let l_pos = lhs < 2u128.pow(bit_size - 1);
let r_pos = rhs < 2u128.pow(bit_size - 1);
let lhs = if l_pos { lhs as i128 } else { -((2u128.pow(*bit_size) - lhs) as i128) };
let rhs = if r_pos { rhs as i128 } else { -((2u128.pow(*bit_size) - rhs) as i128) };
// The divisor is being truncated into the type of the operand, which can potentially
// lead to the rhs being zero.
// If the rhs of a division is zero, attempting to evaluate the division will cause a compiler panic.
// Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
// and the operation should be handled by ACIR generation.
if matches!(operator, BinaryOp::Div | BinaryOp::Mod) && rhs == 0 {
return None;
}

let result = function(lhs, rhs);
let result =
if result >= 0 { result as u128 } else { (2i128.pow(*bit_size) + result) as u128 };
truncate(result, *bit_size).into()
}
_ => return None,
};

Expand Down Expand Up @@ -868,6 +920,21 @@ impl BinaryOp {
BinaryOp::Lt => |x, y| (x < y) as u128,
}
}

fn get_i128_function(self) -> fn(i128, i128) -> i128 {
match self {
BinaryOp::Add => i128::wrapping_add,
BinaryOp::Sub => i128::wrapping_sub,
BinaryOp::Mul => i128::wrapping_mul,
BinaryOp::Div => i128::wrapping_div,
BinaryOp::Mod => i128::wrapping_rem,
BinaryOp::And => |x, y| x & y,
BinaryOp::Or => |x, y| x | y,
BinaryOp::Xor => |x, y| x ^ y,
BinaryOp::Eq => |x, y| (x == y) as i128,
BinaryOp::Lt => |x, y| (x < y) as i128,
}
}
}

/// Binary Operations allowed in the IR.
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,5 +172,8 @@ pub(crate) fn display_instruction(
show(*value)
)
}
Instruction::RangeCheck { value, max_bit_size, .. } => {
write!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
}
}
}
Loading
Loading