Skip to content

Commit

Permalink
feat: Add support for brillig call stacks in runtime errors (#2549)
Browse files Browse the repository at this point in the history
  • Loading branch information
sirasistant authored Sep 5, 2023
1 parent 669e0da commit a077391
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 65 deletions.
28 changes: 14 additions & 14 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ edition = "2021"
rust-version = "1.66"

[workspace.dependencies]
acvm = "0.24.1"
acvm = "0.25.0"
arena = { path = "crates/arena" }
fm = { path = "crates/fm" }
iter-extended = { path = "crates/iter-extended" }
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub enum NargoError {
#[derive(Debug, Error)]
pub enum ExecutionError {
#[error("Failed assertion: '{}'", .0)]
AssertionFailed(String, OpcodeLocation),
AssertionFailed(String, Vec<OpcodeLocation>),

#[error(transparent)]
SolvingError(#[from] OpcodeResolutionError),
Expand Down
27 changes: 19 additions & 8 deletions crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,28 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver>(
unreachable!("Execution should not stop while in `InProgress` state.")
}
ACVMStatus::Failure(error) => {
return Err(NargoError::ExecutionError(match error {
let call_stack = match &error {
OpcodeResolutionError::UnsatisfiedConstrain {
opcode_location: ErrorLocation::Resolved(opcode_location),
} => match get_assert_message(&opcode_location) {
Some(assert_message) => {
ExecutionError::AssertionFailed(assert_message, opcode_location)
} => Some(vec![*opcode_location]),
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => {
Some(call_stack.clone())
}
_ => None,
};

return Err(NargoError::ExecutionError(match call_stack {
Some(call_stack) => {
if let Some(assert_message) = get_assert_message(
call_stack.last().expect("Call stacks should not be empty"),
) {
ExecutionError::AssertionFailed(assert_message, call_stack)
} else {
ExecutionError::SolvingError(error)
}
None => ExecutionError::SolvingError(error),
},
_ => ExecutionError::SolvingError(error),
}))
}
None => ExecutionError::SolvingError(error),
}));
}
ACVMStatus::RequiresForeignCall(foreign_call) => {
let foreign_call_result = ForeignCall::execute(&foreign_call, show_output)?;
Expand Down
95 changes: 54 additions & 41 deletions crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,20 @@ fn execute_package(
/// If the location has been resolved we return the contained [OpcodeLocation].
fn extract_opcode_error_from_nargo_error(
nargo_err: &NargoError,
) -> Option<(OpcodeLocation, &ExecutionError)> {
) -> Option<(Vec<OpcodeLocation>, &ExecutionError)> {
let execution_error = match nargo_err {
NargoError::ExecutionError(err) => err,
_ => return None,
};

match execution_error {
ExecutionError::AssertionFailed(_, location) => Some((*location, execution_error)),
ExecutionError::SolvingError(OpcodeResolutionError::BrilligFunctionFailed {
call_stack,
..
})
| ExecutionError::AssertionFailed(_, call_stack) => {
Some((call_stack.clone(), execution_error))
}
ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
opcode_location: error_location,
..
Expand All @@ -113,54 +119,61 @@ fn extract_opcode_error_from_nargo_error(
ErrorLocation::Unresolved => {
unreachable!("Cannot resolve index for unsatisfied constraint")
}
ErrorLocation::Resolved(opcode_location) => Some((*opcode_location, execution_error)),
ErrorLocation::Resolved(opcode_location) => {
Some((vec![*opcode_location], execution_error))
}
},
_ => None,
}
}

/// Resolve an [OpcodeLocation] using debug information generated during compilation
/// to determine an opcode's call stack. Then report the error using the resolved
/// call stack and any other relevant error information returned from the ACVM.
fn report_error_with_opcode_location(
opcode_err_info: Option<(OpcodeLocation, &ExecutionError)>,
/// Resolve the vector of [OpcodeLocation] that caused an execution error using the debug information
/// generated during compilation to determine the complete call stack for an error. Then report the error using
/// the resolved call stack and any other relevant error information returned from the ACVM.
fn report_error_with_opcode_locations(
opcode_err_info: Option<(Vec<OpcodeLocation>, &ExecutionError)>,
debug: &DebugInfo,
context: &Context,
) {
if let Some((opcode_location, opcode_err)) = opcode_err_info {
if let Some(locations) = debug.opcode_location(&opcode_location) {
// The location of the error itself will be the location at the top
// of the call stack (the last item in the Vec).
if let Some(location) = locations.last() {
let message = match opcode_err {
ExecutionError::AssertionFailed(message, _) => {
format!("Assertion failed: '{message}'")
}
ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
index,
array_size,
..
}) => {
format!(
if let Some((opcode_locations, opcode_err)) = opcode_err_info {
let source_locations: Vec<_> = opcode_locations
.iter()
.flat_map(|opcode_location| {
let locations = debug.opcode_location(opcode_location);
locations.unwrap_or_default()
})
.collect();
// The location of the error itself will be the location at the top
// of the call stack (the last item in the Vec).
if let Some(location) = source_locations.last() {
let message = match opcode_err {
ExecutionError::AssertionFailed(message, _) => {
format!("Assertion failed: '{message}'")
}
ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
index,
array_size,
..
}) => {
format!(
"Index out of bounds, array has size {array_size:?}, but index was {index:?}"
)
}
ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
..
}) => "Failed constraint".into(),
_ => {
// All other errors that do not have corresponding opcode locations
// should not be reported in this method.
// If an error with an opcode location is not handled in this match statement
// the basic message attached to the original error from the ACVM should be reported.
return;
}
};
CustomDiagnostic::simple_error(message, String::new(), location.span)
.in_file(location.file)
.with_call_stack(locations)
.report(&context.file_manager, false);
}
}
ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
..
}) => "Failed constraint".into(),
_ => {
// All other errors that do not have corresponding opcode locations
// should not be reported in this method.
// If an error with an opcode location is not handled in this match statement
// the basic message attached to the original error from the ACVM should be reported.
return;
}
};
CustomDiagnostic::simple_error(message, String::new(), location.span)
.in_file(location.file)
.with_call_stack(source_locations)
.report(&context.file_manager, false);
}
}
}
Expand All @@ -184,7 +197,7 @@ pub(crate) fn execute_program(
Err(err) => {
if let Some((debug, context)) = debug_data {
let opcode_err_info = extract_opcode_error_from_nargo_error(&err);
report_error_with_opcode_location(opcode_err_info, &debug, &context);
report_error_with_opcode_locations(opcode_err_info, &debug, &context);
}

Err(crate::errors::CliError::NargoError(err))
Expand Down

0 comments on commit a077391

Please sign in to comment.