feat!: avoid integer overflows

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -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;
@@ -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:?}"),
         };
 

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -597,11 +597,17 @@ 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 } => {
                 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

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -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)
     }
@@ -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(())

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -150,6 +150,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> },
 
@@ -199,7 +202,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
             }
@@ -226,9 +230,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(),
@@ -250,7 +257,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] {
@@ -307,6 +314,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(),
+                }
+            }
         }
     }
 
@@ -351,6 +365,9 @@ impl Instruction {
             Instruction::EnableSideEffects { condition } => {
                 f(*condition);
             }
+            Instruction::RangeCheck { value, .. } => {
+                f(*value);
+            }
         }
     }
 
@@ -448,6 +465,7 @@ impl Instruction {
             Instruction::Allocate { .. } => None,
             Instruction::Load { .. } => None,
             Instruction::Store { .. } => None,
+            Instruction::RangeCheck { .. } => None,
         }
     }
 }

compiler/noirc_evaluator/src/ssa/ir/printer.rs

@@ -172,5 +172,8 @@ pub(crate) fn display_instruction(
                 show(*value)
             )
         }
+        Instruction::RangeCheck { value, max_bit_size, .. } => {
+            write!(f, "range {} to {} bits", show(*value), *max_bit_size,)
+        }
     }
 }

compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs

@@ -12,7 +12,7 @@ use crate::ssa::function_builder::FunctionBuilder;
 use crate::ssa::ir::dfg::DataFlowGraph;
 use crate::ssa::ir::function::FunctionId as IrFunctionId;
 use crate::ssa::ir::function::{Function, RuntimeType};
-use crate::ssa::ir::instruction::{BinaryOp, Endian, Intrinsic};
+use crate::ssa::ir::instruction::{BinaryOp, Endian, Instruction, Intrinsic};
 use crate::ssa::ir::map::AtomicCounter;
 use crate::ssa::ir::types::{NumericType, Type};
 use crate::ssa::ir::value::ValueId;
@@ -313,21 +313,35 @@ impl<'a> FunctionContext<'a> {
             }
         };
 
-        if let Some(max_bit_size) = operator_result_max_bit_size_to_truncate(
+        // Check for integer overflow
+        if matches!(
             operator,
-            lhs,
-            rhs,
-            &self.builder.current_function.dfg,
+            BinaryOpKind::Add
+                | BinaryOpKind::Subtract
+                | BinaryOpKind::Multiply
+                | BinaryOpKind::ShiftLeft
         ) {
             let result_type = self.builder.current_function.dfg.type_of_value(result);
-            let bit_size = match result_type {
+            match result_type {
                 Type::Numeric(NumericType::Signed { bit_size })
-                | Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size,
-                _ => {
-                    unreachable!("ICE: Truncation attempted on non-integer");
+                | Type::Numeric(NumericType::Unsigned { bit_size }) => {
+                    let op_name = match operator {
+                        BinaryOpKind::Add => "add",
+                        BinaryOpKind::Subtract => "subtract",
+                        BinaryOpKind::Multiply => "multiply",
+                        BinaryOpKind::ShiftLeft => "left shift",
+                        _ => unreachable!("operator {} should not overflow", operator),
+                    };
+                    let message = format!("attempt to {} with overflow", op_name);
+                    let range_constraint = Instruction::RangeCheck {
+                        value: result,
+                        max_bit_size: bit_size,
+                        assert_message: Some(message),
+                    };
+                    self.builder.set_location(location).insert_instruction(range_constraint, None);
                 }
-            };
-            result = self.builder.insert_truncate(result, bit_size, max_bit_size);
+                _ => (),
+            }
         }
 
         if operator_requires_not(operator) {

tooling/nargo_cli/tests/compile_success_empty/unary_operators/src/main.nr

@@ -2,6 +2,8 @@ fn main() {
     let x = -1;
     assert(x == 1 - 2);
 
+    // TODO: subtraction for signed integers
+    // This will error with 'attempt to subtract with overflow'
     let y: i32 = -1;
     assert(x == 1 - 2);
 }

tooling/nargo_cli/tests/execution_success/brillig_fns_as_values/Prover.toml

@@ -1 +1 @@
-x = "0"
+x = "5"

tooling/nargo_cli/tests/execution_success/signed_division/src/main.nr

@@ -9,9 +9,15 @@ fn main(mut x: i32, mut y: i32, mut z: i32) {
     assert(x / y == z);
 
     // -7/3 = -2
-    let minus_x = std::wrapping_sub(0,x);
+    let zero: i32 = 0;
+    let minus_x: i32 = std::wrapping_sub(zero,x);
     let minus_z = std::wrapping_sub(0,z);
     let minus_y = std::wrapping_sub(0,y);
+    // TODO: Fix addition and subtraction for signed integers
+    // We should not have to wrap to do 7 + (-7)
+    // let x_plus_minus_x = std::wrapping_add(x, minus_x);
+    // This will print `0` as we expect for normal signed addition
+    // std::println(x_plus_minus_x);
     assert(x+minus_x == 0);
     assert(z+minus_z == 0);
     assert(minus_x / y == minus_z);