fix: Add size checks to integer literals (#3236)

compiler/noirc_evaluator/src/errors.rs

@@ -7,12 +7,12 @@
 //! An Error of the former is a user Error
 //!
 //! An Error of the latter is an error in the implementation of the compiler
-use acvm::acir::native_types::Expression;
+use acvm::{acir::native_types::Expression, FieldElement};
 use iter_extended::vecmap;
 use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic};
 use thiserror::Error;
 
-use crate::ssa::ir::dfg::CallStack;
+use crate::ssa::ir::{dfg::CallStack, types::NumericType};
 
 #[derive(Debug, PartialEq, Eq, Clone, Error)]
 pub enum RuntimeError {
@@ -29,6 +29,8 @@ pub enum RuntimeError {
     IndexOutOfBounds { index: usize, array_size: usize, call_stack: CallStack },
     #[error("Range constraint of {num_bits} bits is too large for the Field size")]
     InvalidRangeConstraint { num_bits: u32, call_stack: CallStack },
+    #[error("{value} does not fit within the type bounds for {typ}")]
+    IntegerOutOfBounds { value: FieldElement, typ: NumericType, call_stack: CallStack },
     #[error("Expected array index to fit into a u64")]
     TypeConversion { from: String, into: String, call_stack: CallStack },
     #[error("{name:?} is not initialized")]
@@ -91,6 +93,7 @@ impl RuntimeError {
             | RuntimeError::UnInitialized { call_stack, .. }
             | RuntimeError::UnknownLoopBound { call_stack }
             | RuntimeError::AssertConstantFailed { call_stack }
+            | RuntimeError::IntegerOutOfBounds { call_stack, .. }
             | RuntimeError::UnsupportedIntegerSize { call_stack, .. } => call_stack,
         }
     }

compiler/noirc_evaluator/src/ssa.rs

@@ -39,7 +39,7 @@ pub(crate) fn optimize_into_acir(
     print_brillig_trace: bool,
 ) -> Result<GeneratedAcir, RuntimeError> {
     let abi_distinctness = program.return_distinctness;
-    let ssa = SsaBuilder::new(program, print_ssa_passes)
+    let ssa = SsaBuilder::new(program, print_ssa_passes)?
         .run_pass(Ssa::defunctionalize, "After Defunctionalization:")
         .run_pass(Ssa::inline_functions, "After Inlining:")
         // Run mem2reg with the CFG separated into blocks
@@ -129,8 +129,9 @@ struct SsaBuilder {
 }
 
 impl SsaBuilder {
-    fn new(program: Program, print_ssa_passes: bool) -> SsaBuilder {
-        SsaBuilder { print_ssa_passes, ssa: ssa_gen::generate_ssa(program) }.print("Initial SSA:")
+    fn new(program: Program, print_ssa_passes: bool) -> Result<SsaBuilder, RuntimeError> {
+        let ssa = ssa_gen::generate_ssa(program)?;
+        Ok(SsaBuilder { print_ssa_passes, ssa }.print("Initial SSA:"))
     }
 
     fn finish(self) -> Ssa {

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

@@ -183,6 +183,10 @@ impl FunctionBuilder {
         self
     }
 
+    pub(crate) fn get_call_stack(&self) -> CallStack {
+        self.call_stack.clone()
+    }
+
     /// Insert a Load instruction at the end of the current block, loading from the given offset
     /// of the given address which should point to a previous Allocate instruction. Note that
     /// this is limited to loading a single value. Loading multiple values (such as a tuple)

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

@@ -1,5 +1,6 @@
 use std::rc::Rc;
 
+use acvm::FieldElement;
 use iter_extended::vecmap;
 
 /// A numeric type in the Intermediate representation
@@ -11,7 +12,7 @@ use iter_extended::vecmap;
 /// Fields do not have a notion of ordering, so this distinction
 /// is reasonable.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd)]
-pub(crate) enum NumericType {
+pub enum NumericType {
     Signed { bit_size: u32 },
     Unsigned { bit_size: u32 },
     NativeField,
@@ -94,6 +95,26 @@ impl Type {
     }
 }
 
+impl NumericType {
+    /// Returns true if the given Field value is within the numeric limits
+    /// for the current NumericType.
+    pub(crate) fn value_is_within_limits(self, field: FieldElement) -> bool {
+        match self {
+            NumericType::Signed { bit_size } => {
+                let min = -(2i128.pow(bit_size - 1));
+                let max = 2u128.pow(bit_size - 1) - 1;
+                // Signed integers are odd since they will overflow the field value
+                field <= max.into() || field >= min.into()
+            }
+            NumericType::Unsigned { bit_size } => {
+                let max = 2u128.pow(bit_size) - 1;
+                field <= max.into()
+            }
+            NumericType::NativeField => true,
+        }
+    }
+}
+
 /// Composite Types are essentially flattened struct or tuple types.
 /// Array types may have these as elements where each flattened field is
 /// included in the array sequentially.

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

@@ -8,6 +8,7 @@ use noirc_frontend::monomorphization::ast::{self, LocalId, Parameters};
 use noirc_frontend::monomorphization::ast::{FuncId, Program};
 use noirc_frontend::{BinaryOpKind, Signedness};
 
+use crate::errors::RuntimeError;
 use crate::ssa::function_builder::FunctionBuilder;
 use crate::ssa::ir::dfg::DataFlowGraph;
 use crate::ssa::ir::function::FunctionId as IrFunctionId;
@@ -240,6 +241,30 @@ impl<'a> FunctionContext<'a> {
         Values::empty()
     }
 
+    /// Insert a numeric constant into the current function
+    ///
+    /// Unlike FunctionBuilder::numeric_constant, this version checks the given constant
+    /// is within the range of the given type. This is needed for user provided values where
+    /// otherwise values like 2^128 can be assigned to a u8 without error or wrapping.
+    pub(super) fn checked_numeric_constant(
+        &mut self,
+        value: impl Into<FieldElement>,
+        typ: Type,
+    ) -> Result<ValueId, RuntimeError> {
+        let value = value.into();
+
+        if let Type::Numeric(typ) = typ {
+            if !typ.value_is_within_limits(value) {
+                let call_stack = self.builder.get_call_stack();
+                return Err(RuntimeError::IntegerOutOfBounds { value, typ, call_stack });
+            }
+        } else {
+            panic!("Expected type for numeric constant to be a numeric type, found {typ}");
+        }
+
+        Ok(self.builder.numeric_constant(value, typ))
+    }
+
     /// Insert ssa instructions which computes lhs << rhs by doing lhs*2^rhs
     fn insert_shift_left(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId {
         let base = self.builder.field_constant(FieldElement::from(2_u128));
@@ -544,8 +569,11 @@ impl<'a> FunctionContext<'a> {
     /// This is operationally equivalent to extract_current_value_recursive, but splitting these
     /// into two separate functions avoids cloning the outermost `Values` returned by the recursive
     /// version, as it is only needed for recursion.
-    pub(super) fn extract_current_value(&mut self, lvalue: &ast::LValue) -> LValue {
-        match lvalue {
+    pub(super) fn extract_current_value(
+        &mut self,
+        lvalue: &ast::LValue,
+    ) -> Result<LValue, RuntimeError> {
+        Ok(match lvalue {
             ast::LValue::Ident(ident) => {
                 let (reference, should_auto_deref) = self.ident_lvalue(ident);
                 if should_auto_deref {
@@ -555,18 +583,18 @@ impl<'a> FunctionContext<'a> {
                 }
             }
             ast::LValue::Index { array, index, location, .. } => {
-                self.index_lvalue(array, index, location).2
+                self.index_lvalue(array, index, location)?.2
             }
             ast::LValue::MemberAccess { object, field_index } => {
-                let (old_object, object_lvalue) = self.extract_current_value_recursive(object);
+                let (old_object, object_lvalue) = self.extract_current_value_recursive(object)?;
                 let object_lvalue = Box::new(object_lvalue);
                 LValue::MemberAccess { old_object, object_lvalue, index: *field_index }
             }
             ast::LValue::Dereference { reference, .. } => {
-                let (reference, _) = self.extract_current_value_recursive(reference);
+                let (reference, _) = self.extract_current_value_recursive(reference)?;
                 LValue::Dereference { reference }
             }
-        }
+        })
     }
 
     fn dereference_lvalue(&mut self, values: &Values, element_type: &ast::Type) -> Values {
@@ -596,16 +624,16 @@ impl<'a> FunctionContext<'a> {
         array: &ast::LValue,
         index: &ast::Expression,
         location: &Location,
-    ) -> (ValueId, ValueId, LValue, Option<ValueId>) {
-        let (old_array, array_lvalue) = self.extract_current_value_recursive(array);
-        let index = self.codegen_non_tuple_expression(index);
+    ) -> Result<(ValueId, ValueId, LValue, Option<ValueId>), RuntimeError> {
+        let (old_array, array_lvalue) = self.extract_current_value_recursive(array)?;
+        let index = self.codegen_non_tuple_expression(index)?;
         let array_lvalue = Box::new(array_lvalue);
         let array_values = old_array.clone().into_value_list(self);
 
         let location = *location;
         // A slice is represented as a tuple (length, slice contents).
         // We need to fetch the second value.
-        if array_values.len() > 1 {
+        Ok(if array_values.len() > 1 {
             let slice_lvalue = LValue::SliceIndex {
                 old_slice: old_array,
                 index,
@@ -617,37 +645,45 @@ impl<'a> FunctionContext<'a> {
             let array_lvalue =
                 LValue::Index { old_array: array_values[0], index, array_lvalue, location };
             (array_values[0], index, array_lvalue, None)
-        }
+        })
     }
 
-    fn extract_current_value_recursive(&mut self, lvalue: &ast::LValue) -> (Values, LValue) {
+    fn extract_current_value_recursive(
+        &mut self,
+        lvalue: &ast::LValue,
+    ) -> Result<(Values, LValue), RuntimeError> {
         match lvalue {
             ast::LValue::Ident(ident) => {
                 let (variable, should_auto_deref) = self.ident_lvalue(ident);
                 if should_auto_deref {
                     let dereferenced = self.dereference_lvalue(&variable, &ident.typ);
-                    (dereferenced, LValue::Dereference { reference: variable })
+                    Ok((dereferenced, LValue::Dereference { reference: variable }))
                 } else {
-                    (variable.clone(), LValue::Ident)
+                    Ok((variable.clone(), LValue::Ident))
                 }
             }
             ast::LValue::Index { array, index, element_type, location } => {
                 let (old_array, index, index_lvalue, max_length) =
-                    self.index_lvalue(array, index, location);
-                let element =
-                    self.codegen_array_index(old_array, index, element_type, *location, max_length);
-                (element, index_lvalue)
+                    self.index_lvalue(array, index, location)?;
+                let element = self.codegen_array_index(
+                    old_array,
+                    index,
+                    element_type,
+                    *location,
+                    max_length,
+                )?;
+                Ok((element, index_lvalue))
             }
             ast::LValue::MemberAccess { object, field_index: index } => {
-                let (old_object, object_lvalue) = self.extract_current_value_recursive(object);
+                let (old_object, object_lvalue) = self.extract_current_value_recursive(object)?;
                 let object_lvalue = Box::new(object_lvalue);
                 let element = Self::get_field_ref(&old_object, *index).clone();
-                (element, LValue::MemberAccess { old_object, object_lvalue, index: *index })
+                Ok((element, LValue::MemberAccess { old_object, object_lvalue, index: *index }))
             }
             ast::LValue::Dereference { reference, element_type } => {
-                let (reference, _) = self.extract_current_value_recursive(reference);
+                let (reference, _) = self.extract_current_value_recursive(reference)?;
                 let dereferenced = self.dereference_lvalue(&reference, element_type);
-                (dereferenced, LValue::Dereference { reference })
+                Ok((dereferenced, LValue::Dereference { reference }))
             }
         }
     }