Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
70ebf607e566a95ff7eb2c7a0eee7c36465ba5b4
30c50f52a6d58163e39006b73f4eb5003afc239b
26 changes: 24 additions & 2 deletions noir/noir-repo/compiler/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct CustomDiagnostic {
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum DiagnosticKind {
Error,
Bug,
Warning,
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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(),
};

Expand Down
29 changes: 27 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -56,6 +61,7 @@ pub enum RuntimeError {
#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum SsaReport {
Warning(InternalWarning),
Bug(InternalBug),
}

impl From<SsaReport> for FileDiagnostic {
Expand All @@ -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)
}
}
}
}
Expand All @@ -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")]
Expand Down
20 changes: 16 additions & 4 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
56 changes: 35 additions & 21 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down Expand Up @@ -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());
}
}
Loading