Merge branch 'master' into ab/unary-operator

compiler/noirc_errors/src/reporter.rs

@@ -17,6 +17,7 @@ pub struct CustomDiagnostic {
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum DiagnosticKind {
     Error,
+    Bug,
     Warning,
 }
 
@@ -62,6 +63,19 @@ impl CustomDiagnostic {
         }
     }
 
+    pub fn simple_bug(
+        primary_message: String,
+        secondary_message: String,
+        secondary_span: Span,
+    ) -> CustomDiagnostic {
+        CustomDiagnostic {
+            message: primary_message,
+            secondaries: vec![CustomLabel::new(secondary_message, secondary_span)],
+            notes: Vec::new(),
+            kind: DiagnosticKind::Bug,
+        }
+    }
+
     pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
         FileDiagnostic::new(file_id, self)
     }
@@ -81,6 +95,10 @@ impl CustomDiagnostic {
     pub fn is_warning(&self) -> bool {
         matches!(self.kind, DiagnosticKind::Warning)
     }
+
+    pub fn is_bug(&self) -> bool {
+        matches!(self.kind, DiagnosticKind::Bug)
+    }
 }
 
 impl std::fmt::Display for CustomDiagnostic {
@@ -120,10 +138,13 @@ pub fn report_all<'files>(
     silence_warnings: bool,
 ) -> ReportedErrors {
     // Report warnings before any errors
-    let (warnings, mut errors): (Vec<_>, _) =
-        diagnostics.iter().partition(|item| item.diagnostic.is_warning());
+    let (warnings_and_bugs, mut errors): (Vec<_>, _) =
+        diagnostics.iter().partition(|item| !item.diagnostic.is_error());
 
+    let (warnings, mut bugs): (Vec<_>, _) =
+        warnings_and_bugs.iter().partition(|item| item.diagnostic.is_warning());
     let mut diagnostics = if silence_warnings { Vec::new() } else { warnings };
+    diagnostics.append(&mut bugs);
     diagnostics.append(&mut errors);
 
     let error_count =
@@ -170,6 +191,7 @@ fn convert_diagnostic(
 ) -> Diagnostic<fm::FileId> {
     let diagnostic = match (cd.kind, deny_warnings) {
         (DiagnosticKind::Warning, false) => Diagnostic::warning(),
+        (DiagnosticKind::Bug, ..) => Diagnostic::bug(),
         _ => Diagnostic::error(),
     };
 

compiler/noirc_evaluator/src/errors.rs

@@ -23,8 +23,13 @@ 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("The value `{value:?}` cannot fit into `{typ}` which has range `{range}`")]
+    IntegerOutOfBounds {
+        value: FieldElement,
+        typ: NumericType,
+        range: String,
+        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")]
@@ -56,6 +61,7 @@ pub enum RuntimeError {
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub enum SsaReport {
     Warning(InternalWarning),
+    Bug(InternalBug),
 }
 
 impl From<SsaReport> for FileDiagnostic {
@@ -78,6 +84,19 @@ impl From<SsaReport> for FileDiagnostic {
                     Diagnostic::simple_warning(message, secondary_message, location.span);
                 diagnostic.in_file(file_id).with_call_stack(call_stack)
             }
+            SsaReport::Bug(bug) => {
+                let message = bug.to_string();
+                let (secondary_message, call_stack) = match bug {
+                    InternalBug::IndependentSubgraph { call_stack } => {
+                        ("There is no path from the output of this brillig call to either return values or inputs of the circuit, which creates an independent subgraph. This is quite likely a soundness vulnerability".to_string(),call_stack)
+                    }
+                };
+                let call_stack = vecmap(call_stack, |location| location);
+                let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
+                let location = call_stack.last().expect("Expected RuntimeError to have a location");
+                let diagnostic = Diagnostic::simple_bug(message, secondary_message, location.span);
+                diagnostic.in_file(file_id).with_call_stack(call_stack)
+            }
         }
     }
 }
@@ -90,6 +109,12 @@ pub enum InternalWarning {
     VerifyProof { call_stack: CallStack },
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize)]
+pub enum InternalBug {
+    #[error("Input to brillig function is in a separate subgraph to output")]
+    IndependentSubgraph { call_stack: CallStack },
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, Error)]
 pub enum InternalError {
     #[error("ICE: Both expressions should have degree<=1")]

compiler/noirc_evaluator/src/ssa.rs

@@ -41,6 +41,8 @@ pub mod ir;
 mod opt;
 pub mod ssa_gen;
 
+pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec<SsaReport>);
+
 /// Optimize the given program by converting it into SSA
 /// form and performing optimizations there. When finished,
 /// convert the final SSA into an ACIR program and return it.
@@ -49,10 +51,10 @@ pub mod ssa_gen;
 pub(crate) fn optimize_into_acir(
     program: Program,
     options: &SsaEvaluatorOptions,
-) -> Result<Artifacts, RuntimeError> {
+) -> Result<ArtifactsAndWarnings, RuntimeError> {
     let ssa_gen_span = span!(Level::TRACE, "ssa_generation");
     let ssa_gen_span_guard = ssa_gen_span.enter();
-    let ssa = SsaBuilder::new(
+    let mut ssa = SsaBuilder::new(
         program,
         options.enable_ssa_logging,
         options.force_brillig_output,
@@ -89,13 +91,15 @@ pub(crate) fn optimize_into_acir(
     .run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:")
     .finish();
 
+    let ssa_level_warnings = ssa.check_for_underconstrained_values();
     let brillig = time("SSA to Brillig", options.print_codegen_timings, || {
         ssa.to_brillig(options.enable_brillig_logging)
     });
 
     drop(ssa_gen_span_guard);
 
-    time("SSA to ACIR", options.print_codegen_timings, || ssa.into_acir(&brillig))
+    let artifacts = time("SSA to ACIR", options.print_codegen_timings, || ssa.into_acir(&brillig))?;
+    Ok(ArtifactsAndWarnings(artifacts, ssa_level_warnings))
 }
 
 // Helper to time SSA passes
@@ -149,6 +153,10 @@ impl SsaProgramArtifact {
         }
         self.names.push(circuit_artifact.name);
     }
+
+    fn add_warnings(&mut self, mut warnings: Vec<SsaReport>) {
+        self.warnings.append(&mut warnings);
+    }
 }
 
 pub struct SsaEvaluatorOptions {
@@ -179,14 +187,18 @@ pub fn create_program(
     let func_sigs = program.function_signatures.clone();
 
     let recursive = program.recursive;
-    let (generated_acirs, generated_brillig, error_types) = optimize_into_acir(program, options)?;
+    let ArtifactsAndWarnings((generated_acirs, generated_brillig, error_types), ssa_level_warnings) =
+        optimize_into_acir(program, options)?;
     assert_eq!(
         generated_acirs.len(),
         func_sigs.len(),
         "The generated ACIRs should match the supplied function signatures"
     );
 
     let mut program_artifact = SsaProgramArtifact::new(generated_brillig, error_types);
+
+    // Add warnings collected at the Ssa stage
+    program_artifact.add_warnings(ssa_level_warnings);
     // For setting up the ABI we need separately specify main's input and return witnesses
     let mut is_main = true;
     for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) {

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

@@ -431,6 +431,8 @@ impl<'a> Context<'a> {
         }
 
         warnings.extend(return_warnings);
+
+        // Add the warnings from the alter Ssa passes
         Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))
     }
 

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

@@ -29,23 +29,37 @@ 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, negative: bool) -> bool {
+    /// Returns None if the given Field value is within the numeric limits
+    /// for the current NumericType. Otherwise returns a string describing
+    /// the limits, as a range.
+    pub(crate) fn value_is_outside_limits(
+        self,
+        field: FieldElement,
+        negative: bool,
+    ) -> Option<String> {
         match self {
             NumericType::Unsigned { bit_size } => {
+                let max = 2u128.pow(bit_size) - 1;
                 if negative {
-                    return false;
+                    return Some(format!("0..={}", max));
+                }
+                if field <= max.into() {
+                    None
+                } else {
+                    Some(format!("0..={}", max))
                 }
-                let max = 2u128.pow(bit_size) - 1;
-                field <= max.into()
             }
             NumericType::Signed { bit_size } => {
-                let max =
-                    if negative { 2u128.pow(bit_size - 1) } else { 2u128.pow(bit_size - 1) - 1 };
-                field <= max.into()
+                let min = 2u128.pow(bit_size - 1);
+                let max = 2u128.pow(bit_size - 1) - 1;
+                let target_max = if negative { min } else { max };
+                if field <= target_max.into() {
+                    None
+                } else {
+                    Some(format!("-{}..={}", min, max))
+                }
             }
-            NumericType::NativeField => true,
+            NumericType::NativeField => None,
         }
     }
 }
@@ -224,21 +238,21 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_u8_is_within_limits() {
+    fn test_u8_value_is_outside_limits() {
         let u8 = NumericType::Unsigned { bit_size: 8 };
-        assert!(!u8.value_is_within_limits(FieldElement::from(1_i128), true));
-        assert!(u8.value_is_within_limits(FieldElement::from(0_i128), false));
-        assert!(u8.value_is_within_limits(FieldElement::from(255_i128), false));
-        assert!(!u8.value_is_within_limits(FieldElement::from(256_i128), false));
+        assert!(u8.value_is_outside_limits(FieldElement::from(1_i128), true).is_some());
+        assert!(u8.value_is_outside_limits(FieldElement::from(0_i128), false).is_none());
+        assert!(u8.value_is_outside_limits(FieldElement::from(255_i128), false).is_none());
+        assert!(u8.value_is_outside_limits(FieldElement::from(256_i128), false).is_some());
     }
 
     #[test]
-    fn test_i8_is_within_limits() {
+    fn test_i8_value_is_outside_limits() {
         let i8 = NumericType::Signed { bit_size: 8 };
-        assert!(!i8.value_is_within_limits(FieldElement::from(129_i128), true));
-        assert!(i8.value_is_within_limits(FieldElement::from(128_i128), true));
-        assert!(i8.value_is_within_limits(FieldElement::from(0_i128), false));
-        assert!(i8.value_is_within_limits(FieldElement::from(127_i128), false));
-        assert!(!i8.value_is_within_limits(FieldElement::from(128_i128), false));
+        assert!(i8.value_is_outside_limits(FieldElement::from(129_i128), true).is_some());
+        assert!(i8.value_is_outside_limits(FieldElement::from(128_i128), true).is_none());
+        assert!(i8.value_is_outside_limits(FieldElement::from(0_i128), false).is_none());
+        assert!(i8.value_is_outside_limits(FieldElement::from(127_i128), false).is_none());
+        assert!(i8.value_is_outside_limits(FieldElement::from(128_i128), false).is_some());
     }
 }