diff --git a/acvm-repo/acir/src/circuit/mod.rs b/acvm-repo/acir/src/circuit/mod.rs index cb846bdaffa..9c858617acf 100644 --- a/acvm-repo/acir/src/circuit/mod.rs +++ b/acvm-repo/acir/src/circuit/mod.rs @@ -86,6 +86,16 @@ impl Circuit { } } +#[derive(Debug, Copy, Clone)] +/// The opcode location for a call to a separate ACIR circuit +/// This includes the function index of the caller within a [program][Program] +/// and the index in the callers ACIR to the specific call opcode. +/// This is only resolved and set during circuit execution. +pub struct ResolvedOpcodeLocation { + pub acir_function_index: usize, + pub opcode_location: OpcodeLocation, +} + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] /// Opcodes are locatable so that callers can /// map opcodes to debug information related to their context. diff --git a/compiler/noirc_driver/src/contract.rs b/compiler/noirc_driver/src/contract.rs index a33a9b809d3..d6c3dc6205d 100644 --- a/compiler/noirc_driver/src/contract.rs +++ b/compiler/noirc_driver/src/contract.rs @@ -53,7 +53,7 @@ pub struct ContractFunction { )] pub bytecode: Program, - pub debug: DebugInfo, + pub debug: Vec, /// Names of the functions in the program. These are used for more informative debugging and benchmarking. pub names: Vec, diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 6fe44780484..8a554879e9f 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -430,7 +430,8 @@ fn compile_contract_inner( } if errors.is_empty() { - let debug_infos: Vec<_> = functions.iter().map(|function| function.debug.clone()).collect(); + let debug_infos: Vec<_> = + functions.iter().flat_map(|function| function.debug.clone()).collect(); let file_map = filter_relevant_files(&debug_infos, &context.file_manager); let out_structs = contract @@ -547,13 +548,8 @@ pub fn compile_no_check( Ok(CompiledProgram { hash, - // TODO(https://github.com/noir-lang/noir/issues/4428) program, - // TODO(https://github.com/noir-lang/noir/issues/4428) - // Debug info is only relevant for errors at execution time which is not yet supported - // The CompileProgram `debug` field is used in multiple places and is better - // left to be updated once execution of multiple ACIR functions is enabled - debug: debug[0].clone(), + debug, abi, file_map, noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(), diff --git a/compiler/noirc_driver/src/program.rs b/compiler/noirc_driver/src/program.rs index 9ffd2d70dda..ed7ddb29f59 100644 --- a/compiler/noirc_driver/src/program.rs +++ b/compiler/noirc_driver/src/program.rs @@ -24,7 +24,7 @@ pub struct CompiledProgram { )] pub program: Program, pub abi: noirc_abi::Abi, - pub debug: DebugInfo, + pub debug: Vec, pub file_map: BTreeMap, pub warnings: Vec, /// Names of the functions in the program. These are used for more informative debugging and benchmarking. diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 09117bdc3b7..54e2521e413 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -46,6 +46,49 @@ pub type DebugVariables = BTreeMap; pub type DebugFunctions = BTreeMap; pub type DebugTypes = BTreeMap; +#[derive(Default, Debug, Clone, Deserialize, Serialize)] +pub struct ProgramDebugInfo { + pub debug_infos: Vec, +} + +impl ProgramDebugInfo { + pub fn serialize_compressed_base64_json( + debug_info: &ProgramDebugInfo, + s: S, + ) -> Result + where + S: Serializer, + { + let json_str = serde_json::to_string(debug_info).map_err(S::Error::custom)?; + + let mut encoder = DeflateEncoder::new(Vec::new(), Compression::default()); + encoder.write_all(json_str.as_bytes()).map_err(S::Error::custom)?; + let compressed_data = encoder.finish().map_err(S::Error::custom)?; + + let encoded_b64 = base64::prelude::BASE64_STANDARD.encode(compressed_data); + s.serialize_str(&encoded_b64) + } + + pub fn deserialize_compressed_base64_json<'de, D>( + deserializer: D, + ) -> Result + where + D: Deserializer<'de>, + { + let encoded_b64: String = Deserialize::deserialize(deserializer)?; + + let compressed_data = + base64::prelude::BASE64_STANDARD.decode(encoded_b64).map_err(D::Error::custom)?; + + let mut decoder = DeflateDecoder::new(&compressed_data[..]); + let mut decompressed_data = Vec::new(); + decoder.read_to_end(&mut decompressed_data).map_err(D::Error::custom)?; + + let json_str = String::from_utf8(decompressed_data).map_err(D::Error::custom)?; + serde_json::from_str(&json_str).map_err(D::Error::custom) + } +} + #[serde_as] #[derive(Default, Debug, Clone, Deserialize, Serialize)] pub struct DebugInfo { @@ -130,40 +173,4 @@ impl DebugInfo { counted_opcodes } - - pub fn serialize_compressed_base64_json( - debug_info: &DebugInfo, - s: S, - ) -> Result - where - S: Serializer, - { - let json_str = serde_json::to_string(debug_info).map_err(S::Error::custom)?; - - let mut encoder = DeflateEncoder::new(Vec::new(), Compression::default()); - encoder.write_all(json_str.as_bytes()).map_err(S::Error::custom)?; - let compressed_data = encoder.finish().map_err(S::Error::custom)?; - - let encoded_b64 = base64::prelude::BASE64_STANDARD.encode(compressed_data); - s.serialize_str(&encoded_b64) - } - - pub fn deserialize_compressed_base64_json<'de, D>( - deserializer: D, - ) -> Result - where - D: Deserializer<'de>, - { - let encoded_b64: String = Deserialize::deserialize(deserializer)?; - - let compressed_data = - base64::prelude::BASE64_STANDARD.decode(encoded_b64).map_err(D::Error::custom)?; - - let mut decoder = DeflateDecoder::new(&compressed_data[..]); - let mut decompressed_data = Vec::new(); - decoder.read_to_end(&mut decompressed_data).map_err(D::Error::custom)?; - - let json_str = String::from_utf8(decompressed_data).map_err(D::Error::custom)?; - serde_json::from_str(&json_str).map_err(D::Error::custom) - } } diff --git a/compiler/wasm/src/types/noir_artifact.ts b/compiler/wasm/src/types/noir_artifact.ts index f241b539dc7..6ecc3ccd56f 100644 --- a/compiler/wasm/src/types/noir_artifact.ts +++ b/compiler/wasm/src/types/noir_artifact.ts @@ -151,6 +151,16 @@ export interface DebugInfo { locations: Record; } +/** + * The debug information for a given program. + */ +export interface ProgramDebugInfo { + /** + * An array that maps to each function of a program. + */ + debug_infos: Array; +} + /** * Maps a file ID to its metadata for debugging purposes. */ diff --git a/compiler/wasm/test/compiler/shared/compile.test.ts b/compiler/wasm/test/compiler/shared/compile.test.ts index 52cef14968b..f9e37530cbc 100644 --- a/compiler/wasm/test/compiler/shared/compile.test.ts +++ b/compiler/wasm/test/compiler/shared/compile.test.ts @@ -5,6 +5,7 @@ import { ContractCompilationArtifacts, DebugFileMap, DebugInfo, + ProgramDebugInfo, NoirFunctionEntry, ProgramArtifact, ProgramCompilationArtifacts, @@ -15,7 +16,7 @@ export function shouldCompileProgramIdentically( expect: typeof Expect, timeout = 5000, ) { - it('both nargo and noir_wasm should compile identically', async () => { + it('both nargo and noir_wasm should compile program identically', async () => { // Compile! const { nargoArtifact, noirWasmArtifact } = await compileFn(); @@ -51,7 +52,7 @@ export function shouldCompileContractIdentically( expect: typeof Expect, timeout = 5000, ) { - it('both nargo and noir_wasm should compile identically', async () => { + it('both nargo and noir_wasm should compile contract identically', async () => { // Compile! const { nargoArtifact, noirWasmArtifact } = await compileFn(); @@ -90,7 +91,7 @@ function extractDebugInfos(fns: NoirFunctionEntry[]) { return fns.map((fn) => { const debugSymbols = inflateDebugSymbols(fn.debug_symbols); delete (fn as Partial).debug_symbols; - clearFileIdentifiers(debugSymbols); + clearFileIdentifiersProgram(debugSymbols); return debugSymbols; }); } @@ -113,6 +114,12 @@ function deleteContractDebugMetadata(contract: ContractArtifact) { return [extractDebugInfos(contract.functions), fileMap]; } +function clearFileIdentifiersProgram(debugSymbols: ProgramDebugInfo) { + debugSymbols.debug_infos.map((debug_info) => { + clearFileIdentifiers(debug_info); + }); +} + /** Clears file identifiers from a set of debug symbols. */ function clearFileIdentifiers(debugSymbols: DebugInfo) { for (const loc of Object.values(debugSymbols.locations)) { diff --git a/test_programs/execution_failure/fold_dyn_index_fail/Nargo.toml b/test_programs/execution_failure/fold_dyn_index_fail/Nargo.toml new file mode 100644 index 00000000000..e49a82cf0fb --- /dev/null +++ b/test_programs/execution_failure/fold_dyn_index_fail/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fold_dyn_index_fail" +type = "bin" +authors = [""] +compiler_version = ">=0.26.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/fold_dyn_index_fail/Prover.toml b/test_programs/execution_failure/fold_dyn_index_fail/Prover.toml new file mode 100644 index 00000000000..caf3448c56f --- /dev/null +++ b/test_programs/execution_failure/fold_dyn_index_fail/Prover.toml @@ -0,0 +1,2 @@ +x = [104, 101, 108, 108, 111] +z = "4" diff --git a/test_programs/execution_failure/fold_dyn_index_fail/src/main.nr b/test_programs/execution_failure/fold_dyn_index_fail/src/main.nr new file mode 100644 index 00000000000..b12dea630b0 --- /dev/null +++ b/test_programs/execution_failure/fold_dyn_index_fail/src/main.nr @@ -0,0 +1,10 @@ +fn main(mut x: [u32; 5], z: Field) { + x[z] = 4; + dynamic_index_check(x, z + 10); +} + +#[fold] +fn dynamic_index_check(x: [u32; 5], idx: Field) { + // Dynamic index is greater than length of the array + assert(x[idx] != 0); +} diff --git a/test_programs/execution_failure/fold_nested_brillig_assert_fail/Nargo.toml b/test_programs/execution_failure/fold_nested_brillig_assert_fail/Nargo.toml new file mode 100644 index 00000000000..bb7d5e20dcc --- /dev/null +++ b/test_programs/execution_failure/fold_nested_brillig_assert_fail/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fold_nested_brillig_assert_fail" +type = "bin" +authors = [""] +compiler_version = ">=0.26.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/fold_nested_brillig_assert_fail/Prover.toml b/test_programs/execution_failure/fold_nested_brillig_assert_fail/Prover.toml new file mode 100644 index 00000000000..11497a473bc --- /dev/null +++ b/test_programs/execution_failure/fold_nested_brillig_assert_fail/Prover.toml @@ -0,0 +1 @@ +x = "0" diff --git a/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr b/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr new file mode 100644 index 00000000000..0a5038c179b --- /dev/null +++ b/test_programs/execution_failure/fold_nested_brillig_assert_fail/src/main.nr @@ -0,0 +1,26 @@ +// Tests a very simple program. +// +// The features being tested is using assert on brillig that is triggered through nested ACIR calls. +// We want to make sure we get a call stack from the original call in main to the failed assert. +fn main(x: Field) { + assert(1 == fold_conditional_wrapper(x as bool)); +} + +#[fold] +fn fold_conditional_wrapper(x: bool) -> Field { + fold_conditional(x) +} + +#[fold] +fn fold_conditional(x: bool) -> Field { + conditional_wrapper(x) +} + +unconstrained fn conditional_wrapper(x: bool) -> Field { + conditional(x) +} + +unconstrained fn conditional(x: bool) -> Field { + assert(x); + 1 +} diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index b211832518d..c8ab544fa70 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -331,7 +331,8 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.handle_foreign_call(foreign_call) } Err(err) => DebugCommandResult::Error(NargoError::ExecutionError( - ExecutionError::SolvingError(err), + // TODO: debugger does not not handle multiple acir calls + ExecutionError::SolvingError(err, None), )), } } @@ -374,7 +375,8 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } ACVMStatus::Failure(error) => DebugCommandResult::Error(NargoError::ExecutionError( - ExecutionError::SolvingError(error), + // TODO: debugger does not not handle multiple acir calls + ExecutionError::SolvingError(error, None), )), ACVMStatus::RequiresForeignCall(_) => { unreachable!("Unexpected pending foreign call resolution"); diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index ea3204ebbbc..8dc3c541540 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -603,7 +603,7 @@ pub fn run_session( initial_witness: WitnessMap, ) -> Result<(), ServerError> { let debug_artifact = DebugArtifact { - debug_symbols: vec![program.debug], + debug_symbols: program.debug, file_map: program.file_map, warnings: program.warnings, }; diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index 89719947689..7d06bc87c85 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -84,9 +84,11 @@ fn on_profile_run_request_inner( let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); - let span_opcodes = compiled_program.debug.count_span_opcodes(); - let debug_artifact: DebugArtifact = compiled_program.clone().into(); - opcodes_counts.extend(span_opcodes); + for function_debug in compiled_program.debug.iter() { + let span_opcodes = function_debug.count_span_opcodes(); + opcodes_counts.extend(span_opcodes); + } + let debug_artifact: DebugArtifact = compiled_program.into(); file_map.extend(debug_artifact.file_map); } @@ -94,14 +96,17 @@ fn on_profile_run_request_inner( let compiled_contract = nargo::ops::transform_contract(compiled_contract, expression_width); - let function_debug_info: Vec<_> = - compiled_contract.functions.iter().map(|func| &func.debug).cloned().collect(); - let debug_artifact: DebugArtifact = compiled_contract.into(); - file_map.extend(debug_artifact.file_map); + let function_debug_info = compiled_contract + .functions + .iter() + .flat_map(|func| &func.debug) + .collect::>(); for contract_function_debug in function_debug_info { let span_opcodes = contract_function_debug.count_span_opcodes(); opcodes_counts.extend(span_opcodes); } + let debug_artifact: DebugArtifact = compiled_contract.into(); + file_map.extend(debug_artifact.file_map); } let result = NargoProfileRunResult { file_map, opcodes_counts }; diff --git a/tooling/nargo/src/artifacts/contract.rs b/tooling/nargo/src/artifacts/contract.rs index 868fb4404fd..83bb4b94f82 100644 --- a/tooling/nargo/src/artifacts/contract.rs +++ b/tooling/nargo/src/artifacts/contract.rs @@ -4,7 +4,7 @@ use noirc_driver::{CompiledContract, CompiledContractOutputs, ContractFunction}; use serde::{Deserialize, Serialize}; use noirc_driver::DebugFile; -use noirc_errors::debug_info::DebugInfo; +use noirc_errors::debug_info::ProgramDebugInfo; use std::collections::{BTreeMap, HashMap}; use fm::FileId; @@ -68,10 +68,10 @@ pub struct ContractFunctionArtifact { pub bytecode: Program, #[serde( - serialize_with = "DebugInfo::serialize_compressed_base64_json", - deserialize_with = "DebugInfo::deserialize_compressed_base64_json" + serialize_with = "ProgramDebugInfo::serialize_compressed_base64_json", + deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" )] - pub debug_symbols: DebugInfo, + pub debug_symbols: ProgramDebugInfo, } impl From for ContractFunctionArtifact { @@ -82,7 +82,7 @@ impl From for ContractFunctionArtifact { custom_attributes: func.custom_attributes, abi: func.abi, bytecode: func.bytecode, - debug_symbols: func.debug, + debug_symbols: ProgramDebugInfo { debug_infos: func.debug }, } } } diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index fbdf59805c9..496896468cc 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -121,7 +121,7 @@ impl DebugArtifact { impl From for DebugArtifact { fn from(compiled_program: CompiledProgram) -> Self { DebugArtifact { - debug_symbols: vec![compiled_program.debug], + debug_symbols: compiled_program.debug, file_map: compiled_program.file_map, warnings: compiled_program.warnings, } @@ -133,7 +133,7 @@ impl From for DebugArtifact { let all_functions_debug: Vec = compiled_artifact .functions .into_iter() - .map(|contract_function| contract_function.debug) + .flat_map(|contract_function| contract_function.debug) .collect(); DebugArtifact { diff --git a/tooling/nargo/src/artifacts/program.rs b/tooling/nargo/src/artifacts/program.rs index 9e660cbd359..67ac9f53ec8 100644 --- a/tooling/nargo/src/artifacts/program.rs +++ b/tooling/nargo/src/artifacts/program.rs @@ -5,7 +5,7 @@ use fm::FileId; use noirc_abi::Abi; use noirc_driver::CompiledProgram; use noirc_driver::DebugFile; -use noirc_errors::debug_info::DebugInfo; +use noirc_errors::debug_info::ProgramDebugInfo; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -27,10 +27,10 @@ pub struct ProgramArtifact { pub bytecode: Program, #[serde( - serialize_with = "DebugInfo::serialize_compressed_base64_json", - deserialize_with = "DebugInfo::deserialize_compressed_base64_json" + serialize_with = "ProgramDebugInfo::serialize_compressed_base64_json", + deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" )] - pub debug_symbols: DebugInfo, + pub debug_symbols: ProgramDebugInfo, /// Map of file Id to the source code so locations in debug info can be mapped to source code they point to. pub file_map: BTreeMap, @@ -45,7 +45,7 @@ impl From for ProgramArtifact { abi: compiled_program.abi, noir_version: compiled_program.noir_version, bytecode: compiled_program.program, - debug_symbols: compiled_program.debug, + debug_symbols: ProgramDebugInfo { debug_infos: compiled_program.debug }, file_map: compiled_program.file_map, names: compiled_program.names, } @@ -59,7 +59,7 @@ impl From for CompiledProgram { abi: program.abi, noir_version: program.noir_version, program: program.bytecode, - debug: program.debug_symbols, + debug: program.debug_symbols.debug_infos, file_map: program.file_map, warnings: vec![], names: program.names, diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index ff238d79a46..fa44bcc459b 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -1,5 +1,5 @@ use acvm::{ - acir::circuit::OpcodeLocation, + acir::circuit::{OpcodeLocation, ResolvedOpcodeLocation}, pwg::{ErrorLocation, OpcodeResolutionError}, }; use noirc_errors::{ @@ -61,7 +61,7 @@ impl NargoError { match execution_error { ExecutionError::AssertionFailed(message, _) => Some(message), - ExecutionError::SolvingError(error) => match error { + ExecutionError::SolvingError(error, _) => match error { OpcodeResolutionError::IndexOutOfBounds { .. } | OpcodeResolutionError::OpcodeNotSolvable(_) | OpcodeResolutionError::UnsatisfiedConstrain { .. } @@ -77,81 +77,105 @@ impl NargoError { #[derive(Debug, Error)] pub enum ExecutionError { #[error("Failed assertion: '{}'", .0)] - AssertionFailed(String, Vec), + AssertionFailed(String, Vec), - #[error(transparent)] - SolvingError(#[from] OpcodeResolutionError), + #[error("Failed to solve program: '{}'", .0)] + SolvingError(OpcodeResolutionError, Option>), } /// Extracts the opcode locations from a nargo error. fn extract_locations_from_error( error: &ExecutionError, - debug: &DebugInfo, + debug: &[DebugInfo], ) -> Option> { let mut opcode_locations = match error { - ExecutionError::SolvingError(OpcodeResolutionError::BrilligFunctionFailed { - call_stack, - .. - }) - | ExecutionError::AssertionFailed(_, call_stack) => Some(call_stack.clone()), - ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds { - opcode_location: error_location, - .. - }) - | ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain { - opcode_location: error_location, - }) => match error_location { + ExecutionError::SolvingError( + OpcodeResolutionError::BrilligFunctionFailed { .. }, + acir_call_stack, + ) => acir_call_stack.clone(), + ExecutionError::AssertionFailed(_, call_stack) => Some(call_stack.clone()), + ExecutionError::SolvingError( + OpcodeResolutionError::IndexOutOfBounds { opcode_location: error_location, .. }, + acir_call_stack, + ) + | ExecutionError::SolvingError( + OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: error_location }, + acir_call_stack, + ) => match error_location { ErrorLocation::Unresolved => { unreachable!("Cannot resolve index for unsatisfied constraint") } - ErrorLocation::Resolved(opcode_location) => Some(vec![*opcode_location]), + ErrorLocation::Resolved(_) => acir_call_stack.clone(), }, _ => None, }?; - if let Some(OpcodeLocation::Brillig { acir_index, .. }) = opcode_locations.first() { - opcode_locations.insert(0, OpcodeLocation::Acir(*acir_index)); + // Insert the top-level Acir location where the Brillig function failed + for (i, resolved_location) in opcode_locations.iter().enumerate() { + if let ResolvedOpcodeLocation { + acir_function_index, + opcode_location: OpcodeLocation::Brillig { acir_index, .. }, + } = resolved_location + { + let acir_location = ResolvedOpcodeLocation { + acir_function_index: *acir_function_index, + opcode_location: OpcodeLocation::Acir(*acir_index), + }; + + opcode_locations.insert(i, acir_location); + // Go until the first brillig opcode as that means we have the start of a Brillig call stack. + // We have to loop through the opcode locations in case we had ACIR calls + // before the brillig function failure. + break; + } } Some( opcode_locations .iter() - .flat_map(|opcode_location| debug.opcode_location(opcode_location).unwrap_or_default()) + .flat_map(|resolved_location| { + debug[resolved_location.acir_function_index] + .opcode_location(&resolved_location.opcode_location) + .unwrap_or_default() + }) .collect(), ) } -/// Tries to generate a runtime diagnostic from a nargo error. It will successfully do so if it's a runtime error with a call stack. -pub fn try_to_diagnose_runtime_error( - nargo_err: &NargoError, - debug: &DebugInfo, -) -> Option { - let execution_error = match nargo_err { - NargoError::ExecutionError(execution_error) => execution_error, - _ => return None, - }; - - let source_locations = extract_locations_from_error(execution_error, debug)?; - - // The location of the error itself will be the location at the top - // of the call stack (the last item in the Vec). - let location = source_locations.last()?; - - let message = match nargo_err { +fn extract_message_from_error(nargo_err: &NargoError) -> String { + match nargo_err { NargoError::ExecutionError(ExecutionError::AssertionFailed(message, _)) => { format!("Assertion failed: '{message}'") } NargoError::ExecutionError(ExecutionError::SolvingError( OpcodeResolutionError::IndexOutOfBounds { index, array_size, .. }, + _, )) => { format!("Index out of bounds, array has size {array_size:?}, but index was {index:?}") } NargoError::ExecutionError(ExecutionError::SolvingError( OpcodeResolutionError::UnsatisfiedConstrain { .. }, + _, )) => "Failed constraint".into(), _ => nargo_err.to_string(), - }; + } +} +/// Tries to generate a runtime diagnostic from a nargo error. It will successfully do so if it's a runtime error with a call stack. +pub fn try_to_diagnose_runtime_error( + nargo_err: &NargoError, + debug: &[DebugInfo], +) -> Option { + let source_locations = match nargo_err { + NargoError::ExecutionError(execution_error) => { + extract_locations_from_error(execution_error, debug)? + } + _ => return None, + }; + // The location of the error itself will be the location at the top + // of the call stack (the last item in the Vec). + let location = source_locations.last()?; + let message = extract_message_from_error(nargo_err); Some( CustomDiagnostic::simple_error(message, String::new(), location.span) .in_file(location.file) diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index 6d328d65119..76f6da7c1bc 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -1,4 +1,4 @@ -use acvm::acir::circuit::Program; +use acvm::acir::circuit::{OpcodeLocation, Program, ResolvedOpcodeLocation}; use acvm::acir::native_types::WitnessStack; use acvm::brillig_vm::brillig::ForeignCallResult; use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ACVM}; @@ -18,6 +18,15 @@ struct ProgramExecutor<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> { blackbox_solver: &'a B, foreign_call_executor: &'a mut F, + + // The Noir compiler codegens per function and call stacks are not shared across ACIR function calls. + // We must rebuild a call stack when executing a program of many circuits. + call_stack: Vec, + + // Tracks the index of the current function we are executing. + // This is used to fetch the function we want to execute + // and to resolve call stack locations across many function calls. + current_function_index: usize, } impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, B, F> { @@ -31,6 +40,8 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, witness_stack: WitnessStack::default(), blackbox_solver, foreign_call_executor, + call_stack: Vec::default(), + current_function_index: 0, } } @@ -39,11 +50,8 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, } #[tracing::instrument(level = "trace", skip_all)] - fn execute_circuit( - &mut self, - circuit: &Circuit, - initial_witness: WitnessMap, - ) -> Result { + fn execute_circuit(&mut self, initial_witness: WitnessMap) -> Result { + let circuit = &self.functions[self.current_function_index]; let mut acvm = ACVM::new(self.blackbox_solver, &circuit.opcodes, initial_witness); // This message should be resolved by a nargo foreign call only when we have an unsatisfied assertion. @@ -60,9 +68,26 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, let call_stack = match &error { OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Resolved(opcode_location), - } => Some(vec![*opcode_location]), + } + | OpcodeResolutionError::IndexOutOfBounds { + opcode_location: ErrorLocation::Resolved(opcode_location), + .. + } => { + let resolved_location = ResolvedOpcodeLocation { + acir_function_index: self.current_function_index, + opcode_location: *opcode_location, + }; + self.call_stack.push(resolved_location); + Some(self.call_stack.clone()) + } OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { - Some(call_stack.clone()) + let brillig_call_stack = + call_stack.iter().map(|location| ResolvedOpcodeLocation { + acir_function_index: self.current_function_index, + opcode_location: *location, + }); + self.call_stack.extend(brillig_call_stack); + Some(self.call_stack.clone()) } _ => None, }; @@ -79,17 +104,20 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, call_stack, ) } else if let Some(assert_message) = circuit.get_assert_message( - *call_stack.last().expect("Call stacks should not be empty"), + call_stack + .last() + .expect("Call stacks should not be empty") + .opcode_location, ) { ExecutionError::AssertionFailed( assert_message.to_owned(), call_stack, ) } else { - ExecutionError::SolvingError(error) + ExecutionError::SolvingError(error, Some(call_stack)) } } - None => ExecutionError::SolvingError(error), + None => ExecutionError::SolvingError(error, None), })); } ACVMStatus::RequiresForeignCall(foreign_call) => { @@ -109,10 +137,24 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, } } ACVMStatus::RequiresAcirCall(call_info) => { + // Store the parent function index whose context we are currently executing + let acir_function_caller = self.current_function_index; + // Add call opcode to the call stack with a reference to the parent function index + self.call_stack.push(ResolvedOpcodeLocation { + acir_function_index: acir_function_caller, + opcode_location: OpcodeLocation::Acir(acvm.instruction_pointer()), + }); + + // Set current function to the circuit we are about to execute + self.current_function_index = call_info.id as usize; + // Execute the ACIR call let acir_to_call = &self.functions[call_info.id as usize]; let initial_witness = call_info.initial_witness; - let call_solved_witness = - self.execute_circuit(acir_to_call, initial_witness)?; + let call_solved_witness = self.execute_circuit(initial_witness)?; + + // Set tracking index back to the parent function after ACIR call execution + self.current_function_index = acir_function_caller; + let mut call_resolved_outputs = Vec::new(); for return_witness_index in acir_to_call.return_values.indices() { if let Some(return_value) = @@ -122,6 +164,7 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, } else { return Err(ExecutionError::SolvingError( OpcodeNotSolvable::MissingAssignment(return_witness_index).into(), + None, // Missing assignment errors do not supply user-facing diagnostics so we do not need to attach a call stack ) .into()); } @@ -143,11 +186,9 @@ pub fn execute_program( blackbox_solver: &B, foreign_call_executor: &mut F, ) -> Result { - let main = &program.functions[0]; - let mut executor = ProgramExecutor::new(&program.functions, blackbox_solver, foreign_call_executor); - let main_witness = executor.execute_circuit(main, initial_witness)?; + let main_witness = executor.execute_circuit(initial_witness)?; executor.witness_stack.push(0, main_witness); Ok(executor.finalize()) diff --git a/tooling/nargo/src/ops/optimize.rs b/tooling/nargo/src/ops/optimize.rs index cfaaf27ea98..a62f4696328 100644 --- a/tooling/nargo/src/ops/optimize.rs +++ b/tooling/nargo/src/ops/optimize.rs @@ -1,25 +1,36 @@ +use acvm::acir::circuit::Program; use iter_extended::vecmap; use noirc_driver::{CompiledContract, CompiledProgram}; - -/// TODO(https://github.com/noir-lang/noir/issues/4428): Need to update how these passes are run to account for -/// multiple ACIR functions +use noirc_errors::debug_info::DebugInfo; pub fn optimize_program(mut compiled_program: CompiledProgram) -> CompiledProgram { - let (optimized_circuit, location_map) = - acvm::compiler::optimize(std::mem::take(&mut compiled_program.program.functions[0])); - compiled_program.program.functions[0] = optimized_circuit; - compiled_program.debug.update_acir(location_map); + compiled_program.program = + optimize_program_internal(compiled_program.program, &mut compiled_program.debug); compiled_program } pub fn optimize_contract(contract: CompiledContract) -> CompiledContract { let functions = vecmap(contract.functions, |mut func| { - let (optimized_bytecode, location_map) = - acvm::compiler::optimize(std::mem::take(&mut func.bytecode.functions[0])); - func.bytecode.functions[0] = optimized_bytecode; - func.debug.update_acir(location_map); + func.bytecode = optimize_program_internal(func.bytecode, &mut func.debug); func }); CompiledContract { functions, ..contract } } + +fn optimize_program_internal(mut program: Program, debug: &mut [DebugInfo]) -> Program { + let functions = std::mem::take(&mut program.functions); + + let optimized_functions = functions + .into_iter() + .enumerate() + .map(|(i, function)| { + let (optimized_circuit, location_map) = acvm::compiler::optimize(function); + debug[i].update_acir(location_map); + optimized_circuit + }) + .collect::>(); + + program.functions = optimized_functions; + program +} diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 45b1a88f99c..b216fff827d 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -84,7 +84,7 @@ fn test_status_program_compile_fail(err: CompileError, test_function: &TestFunct /// passed/failed to determine the test status. fn test_status_program_compile_pass( test_function: &TestFunction, - debug: DebugInfo, + debug: Vec, circuit_execution: Result, ) -> TestStatus { let circuit_execution_err = match circuit_execution { diff --git a/tooling/nargo/src/ops/transform.rs b/tooling/nargo/src/ops/transform.rs index 274286a46e4..b4811bd5780 100644 --- a/tooling/nargo/src/ops/transform.rs +++ b/tooling/nargo/src/ops/transform.rs @@ -1,21 +1,17 @@ -use acvm::acir::circuit::ExpressionWidth; +use acvm::acir::circuit::{ExpressionWidth, Program}; use iter_extended::vecmap; use noirc_driver::{CompiledContract, CompiledProgram}; - -/// TODO(https://github.com/noir-lang/noir/issues/4428): Need to update how these passes are run to account for -/// multiple ACIR functions +use noirc_errors::debug_info::DebugInfo; pub fn transform_program( mut compiled_program: CompiledProgram, expression_width: ExpressionWidth, ) -> CompiledProgram { - let (optimized_circuit, location_map) = acvm::compiler::compile( - std::mem::take(&mut compiled_program.program.functions[0]), + compiled_program.program = transform_program_internal( + compiled_program.program, + &mut compiled_program.debug, expression_width, ); - - compiled_program.program.functions[0] = optimized_circuit; - compiled_program.debug.update_acir(location_map); compiled_program } @@ -24,14 +20,33 @@ pub fn transform_contract( expression_width: ExpressionWidth, ) -> CompiledContract { let functions = vecmap(contract.functions, |mut func| { - let (optimized_bytecode, location_map) = acvm::compiler::compile( - std::mem::take(&mut func.bytecode.functions[0]), - expression_width, - ); - func.bytecode.functions[0] = optimized_bytecode; - func.debug.update_acir(location_map); + func.bytecode = + transform_program_internal(func.bytecode, &mut func.debug, expression_width); + func }); CompiledContract { functions, ..contract } } + +fn transform_program_internal( + mut program: Program, + debug: &mut [DebugInfo], + expression_width: ExpressionWidth, +) -> Program { + let functions = std::mem::take(&mut program.functions); + + let optimized_functions = functions + .into_iter() + .enumerate() + .map(|(i, function)| { + let (optimized_circuit, location_map) = + acvm::compiler::compile(function, expression_width); + debug[i].update_acir(location_map); + optimized_circuit + }) + .collect::>(); + + program.functions = optimized_functions; + program +} diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 4f3e2886b2e..6a3f0094a08 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -232,7 +232,7 @@ pub(crate) fn debug_program( let initial_witness = compiled_program.abi.encode(inputs_map, None)?; let debug_artifact = DebugArtifact { - debug_symbols: vec![compiled_program.debug.clone()], + debug_symbols: compiled_program.debug.clone(), file_map: compiled_program.file_map.clone(), warnings: compiled_program.warnings.clone(), }; diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 697f6d7c1ea..a353065491f 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -149,7 +149,7 @@ pub(crate) fn execute_program( Ok(solved_witness_stack) => Ok(solved_witness_stack), Err(err) => { let debug_artifact = DebugArtifact { - debug_symbols: vec![compiled_program.debug.clone()], + debug_symbols: compiled_program.debug.clone(), file_map: compiled_program.file_map.clone(), warnings: compiled_program.warnings.clone(), }; diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 72784013e17..77a84de07a4 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -97,17 +97,21 @@ pub(crate) fn run( if args.profile_info { for compiled_program in &compiled_programs { - let span_opcodes = compiled_program.debug.count_span_opcodes(); let debug_artifact = DebugArtifact::from(compiled_program.clone()); - print_span_opcodes(span_opcodes, &debug_artifact); + for function_debug in compiled_program.debug.iter() { + let span_opcodes = function_debug.count_span_opcodes(); + print_span_opcodes(span_opcodes, &debug_artifact); + } } for compiled_contract in &compiled_contracts { let debug_artifact = DebugArtifact::from(compiled_contract.clone()); let functions = &compiled_contract.functions; for contract_function in functions { - let span_opcodes = contract_function.debug.count_span_opcodes(); - print_span_opcodes(span_opcodes, &debug_artifact); + for function_debug in contract_function.debug.iter() { + let span_opcodes = function_debug.count_span_opcodes(); + print_span_opcodes(span_opcodes, &debug_artifact); + } } } }