From c58d69141b54a918cd1675400c00bfd48720f896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 5 Feb 2024 10:37:51 -0500 Subject: [PATCH] feat: Add instrumentation for tracking variables in debugging (#4122) # Description ## Problem\* Part of #3015 ## Summary\* This PR injects instrumentation snippets to the compiled code when running under a debugger to track the values assigned to variables in the program. It also provides the debugging context with necessary support code (in the form of foreign functions) and new functionality to inspect the tracked values. Instrumentation occurs in two phases: 1. During parsing, when assignments are detected, they are replaced by a small snippet that captures the value assigned and a temporary identifier for the variable being assigned, and a foreign function (oracle) is called with this information. 2. At monomorphization time, ie. after the exact types of all variables has been determined, replacing the temporary identifier by a final one which will depend on the variable itself and the type at each instance of the function being compiled (for generic functions). Also, since structs are replaced for tuples during compilation, at this stage field and other member accesses is resolved for the final types (ie. indices in the tuples). ## Additional Context Besides the runtime support in the debugger, the compiler also injects a synthetic crate `__debug` which holds the definition of the oracle functions used in the injected code for tracking the variables and their assigned values. These changes were extracted from the `dap-with-vars` branch of `manastech/noir` repository where most of the development of this feature occurred. We will submit additional PRs for the remaining features in that branch, notably improved REPL and DAP support to make use of this instrumentation code. ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [X] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: synthia Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- Cargo.lock | 2 + acvm-repo/acir_field/src/generic_ark.rs | 10 + compiler/noirc_driver/src/lib.rs | 40 +- compiler/noirc_errors/Cargo.toml | 1 + compiler/noirc_errors/src/debug_info.rs | 26 +- .../src/brillig/brillig_gen/brillig_block.rs | 14 +- .../brillig_gen/brillig_block_variables.rs | 8 +- compiler/noirc_evaluator/src/ssa.rs | 24 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 8 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 11 +- compiler/noirc_frontend/src/debug/mod.rs | 607 ++++++++++++++++++ compiler/noirc_frontend/src/hir/mod.rs | 5 + compiler/noirc_frontend/src/lib.rs | 1 + .../src/monomorphization/ast.rs | 11 +- .../src/monomorphization/debug.rs | 190 ++++++ .../src/monomorphization/debug_types.rs | 137 ++++ .../src/monomorphization/mod.rs | 48 +- compiler/noirc_frontend/src/tests.rs | 4 +- compiler/noirc_printable_type/src/lib.rs | 5 +- tooling/debugger/Cargo.toml | 3 +- tooling/debugger/build.rs | 14 +- tooling/debugger/ignored-tests.txt | 18 + tooling/debugger/src/context.rs | 62 +- tooling/debugger/src/dap.rs | 4 +- tooling/debugger/src/foreign_calls.rs | 138 ++++ tooling/debugger/src/lib.rs | 1 + tooling/debugger/src/repl.rs | 35 +- tooling/debugger/src/source_code_printer.rs | 12 +- tooling/debugger/tests/debug.rs | 2 +- tooling/lsp/src/requests/test_run.rs | 2 +- tooling/nargo/src/artifacts/debug.rs | 7 +- tooling/nargo/src/artifacts/debug_vars.rs | 117 ++++ tooling/nargo/src/artifacts/mod.rs | 1 + tooling/nargo/src/ops/compile.rs | 26 +- tooling/nargo/src/ops/foreign_calls.rs | 2 +- tooling/nargo/src/ops/mod.rs | 8 +- tooling/nargo/src/ops/test.rs | 2 +- tooling/nargo_cli/src/cli/dap_cmd.rs | 21 +- tooling/nargo_cli/src/cli/debug_cmd.rs | 47 +- tooling/nargo_cli/src/cli/export_cmd.rs | 2 +- tooling/nargo_cli/src/cli/test_cmd.rs | 2 +- 41 files changed, 1591 insertions(+), 87 deletions(-) create mode 100644 compiler/noirc_frontend/src/debug/mod.rs create mode 100644 compiler/noirc_frontend/src/monomorphization/debug.rs create mode 100644 compiler/noirc_frontend/src/monomorphization/debug_types.rs create mode 100644 tooling/debugger/ignored-tests.txt create mode 100644 tooling/debugger/src/foreign_calls.rs create mode 100644 tooling/nargo/src/artifacts/debug_vars.rs diff --git a/Cargo.lock b/Cargo.lock index 55cbd0dd2f..05f0ab0176 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2813,6 +2813,7 @@ dependencies = [ "nargo", "noirc_driver", "noirc_errors", + "noirc_frontend", "noirc_printable_type", "owo-colors", "rexpect", @@ -2949,6 +2950,7 @@ dependencies = [ "codespan-reporting", "flate2", "fm", + "noirc_printable_type", "serde", "serde_json", "serde_with", diff --git a/acvm-repo/acir_field/src/generic_ark.rs b/acvm-repo/acir_field/src/generic_ark.rs index 542e291982..dc54d271be 100644 --- a/acvm-repo/acir_field/src/generic_ark.rs +++ b/acvm-repo/acir_field/src/generic_ark.rs @@ -175,6 +175,10 @@ impl FieldElement { self == &Self::one() } + pub fn is_negative(&self) -> bool { + self.neg().num_bits() < self.num_bits() + } + pub fn pow(&self, exponent: &Self) -> Self { FieldElement(self.0.pow(exponent.0.into_bigint())) } @@ -240,6 +244,12 @@ impl FieldElement { self.fits_in_u128().then(|| self.to_u128()) } + pub fn to_i128(self) -> i128 { + let is_negative = self.is_negative(); + let bytes = if is_negative { self.neg() } else { self }.to_be_bytes(); + i128::from_be_bytes(bytes[16..32].try_into().unwrap()) * if is_negative { -1 } else { 1 } + } + pub fn try_to_u64(&self) -> Option { (self.num_bits() <= 64).then(|| self.to_u128() as u64) } diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 25d4229048..d782330a67 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -11,11 +11,12 @@ use noirc_abi::{AbiParameter, AbiType, ContractEvent}; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; use noirc_evaluator::create_circuit; use noirc_evaluator::errors::RuntimeError; +use noirc_frontend::debug::build_debug_crate_file; use noirc_frontend::graph::{CrateId, CrateName}; use noirc_frontend::hir::def_map::{Contract, CrateDefMap}; use noirc_frontend::hir::Context; use noirc_frontend::macros_api::MacroProcessor; -use noirc_frontend::monomorphization::monomorphize; +use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug}; use noirc_frontend::node_interner::FuncId; use std::path::Path; use tracing::info; @@ -33,6 +34,7 @@ pub use debug::DebugFile; pub use program::CompiledProgram; const STD_CRATE_NAME: &str = "std"; +const DEBUG_CRATE_NAME: &str = "__debug"; pub const GIT_COMMIT: &str = env!("GIT_COMMIT"); pub const GIT_DIRTY: &str = env!("GIT_DIRTY"); @@ -83,6 +85,14 @@ pub struct CompileOptions { /// Outputs the monomorphized IR to stdout for debugging #[arg(long, hide = true)] pub show_monomorphized: bool, + + /// Insert debug symbols to inspect variables + #[arg(long, hide = true)] + pub instrument_debug: bool, + + /// Force Brillig output (for step debugging) + #[arg(long, hide = true)] + pub force_brillig: bool, } fn parse_expression_width(input: &str) -> Result { @@ -113,6 +123,7 @@ pub fn file_manager_with_stdlib(root: &Path) -> FileManager { let mut file_manager = FileManager::new(root); add_stdlib_source_to_file_manager(&mut file_manager); + add_debug_source_to_file_manager(&mut file_manager); file_manager } @@ -129,6 +140,15 @@ fn add_stdlib_source_to_file_manager(file_manager: &mut FileManager) { } } +/// Adds the source code of the debug crate needed to support instrumentation to +/// track variables values +fn add_debug_source_to_file_manager(file_manager: &mut FileManager) { + // Adds the synthetic debug module for instrumentation into the file manager + let path_to_debug_lib_file = Path::new(DEBUG_CRATE_NAME).join("lib.nr"); + file_manager + .add_file_with_source_canonical_path(&path_to_debug_lib_file, build_debug_crate_file()); +} + /// Adds the file from the file system at `Path` to the crate graph as a root file /// /// Note: This methods adds the stdlib as a dependency to the crate. @@ -150,6 +170,12 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { root_crate_id } +pub fn link_to_debug_crate(context: &mut Context, root_crate_id: CrateId) { + let path_to_debug_lib_file = Path::new(DEBUG_CRATE_NAME).join("lib.nr"); + let debug_crate_id = prepare_dependency(context, &path_to_debug_lib_file); + add_dep(context, root_crate_id, debug_crate_id, DEBUG_CRATE_NAME.parse().unwrap()); +} + // Adds the file from the file system at `Path` to the crate graph pub fn prepare_dependency(context: &mut Context, file_name: &Path) -> CrateId { let root_file_id = context @@ -327,7 +353,7 @@ fn has_errors(errors: &[FileDiagnostic], deny_warnings: bool) -> bool { /// Compile all of the functions associated with a Noir contract. fn compile_contract_inner( - context: &Context, + context: &mut Context, contract: Contract, options: &CompileOptions, ) -> Result { @@ -403,13 +429,17 @@ fn compile_contract_inner( /// This function assumes [`check_crate`] is called beforehand. #[tracing::instrument(level = "trace", skip_all, fields(function_name = context.function_name(&main_function)))] pub fn compile_no_check( - context: &Context, + context: &mut Context, options: &CompileOptions, main_function: FuncId, cached_program: Option, force_compile: bool, ) -> Result { - let program = monomorphize(main_function, &context.def_interner); + let program = if options.instrument_debug { + monomorphize_debug(main_function, &mut context.def_interner, &context.debug_instrumenter) + } else { + monomorphize(main_function, &mut context.def_interner) + }; let hash = fxhash::hash64(&program); let hashes_match = cached_program.as_ref().map_or(false, |program| program.hash == hash); @@ -428,7 +458,7 @@ pub fn compile_no_check( } let visibility = program.return_visibility; let (circuit, debug, input_witnesses, return_witnesses, warnings) = - create_circuit(program, options.show_ssa, options.show_brillig)?; + create_circuit(program, options.show_ssa, options.show_brillig, options.force_brillig)?; let abi = abi_gen::gen_abi(context, &main_function, input_witnesses, return_witnesses, visibility); diff --git a/compiler/noirc_errors/Cargo.toml b/compiler/noirc_errors/Cargo.toml index 935137ba2f..da18399971 100644 --- a/compiler/noirc_errors/Cargo.toml +++ b/compiler/noirc_errors/Cargo.toml @@ -13,6 +13,7 @@ codespan-reporting.workspace = true codespan.workspace = true fm.workspace = true chumsky.workspace = true +noirc_printable_type.workspace = true serde.workspace = true serde_with = "3.2.0" tracing.workspace = true diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index ffca8fbf2e..25722aac57 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -16,10 +16,26 @@ use std::io::Write; use std::mem; use crate::Location; +use noirc_printable_type::PrintableType; use serde::{ de::Error as DeserializationError, ser::Error as SerializationError, Deserialize, Serialize, }; +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] +pub struct DebugVarId(pub u32); + +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] +pub struct DebugTypeId(pub u32); + +#[derive(Debug, Clone, Hash, Deserialize, Serialize)] +pub struct DebugVariable { + pub name: String, + pub debug_type_id: DebugTypeId, +} + +pub type DebugVariables = BTreeMap; +pub type DebugTypes = BTreeMap; + #[serde_as] #[derive(Default, Debug, Clone, Deserialize, Serialize)] pub struct DebugInfo { @@ -28,6 +44,8 @@ pub struct DebugInfo { /// that they should be serialized to/from strings. #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, + pub variables: DebugVariables, + pub types: DebugTypes, } /// Holds OpCodes Counts for Acir and Brillig Opcodes @@ -39,8 +57,12 @@ pub struct OpCodesCount { } impl DebugInfo { - pub fn new(locations: BTreeMap>) -> Self { - DebugInfo { locations } + pub fn new( + locations: BTreeMap>, + variables: DebugVariables, + types: DebugTypes, + ) -> Self { + Self { locations, variables, types } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 632add74e6..e236d43556 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1250,7 +1250,19 @@ impl<'block> BrilligBlock<'block> { new_variable } } - Value::Function(_) | Value::Intrinsic(_) | Value::ForeignFunction(_) => { + Value::Function(_) => { + // For the debugger instrumentation we want to allow passing + // around values representing function pointers, even though + // there is no interaction with the function possible given that + // value. + let new_variable = + self.variables.allocate_constant(self.brillig_context, value_id, dfg); + let register_index = new_variable.extract_register(); + + self.brillig_context.const_instruction(register_index, value_id.to_usize().into()); + new_variable + } + Value::Intrinsic(_) | Value::ForeignFunction(_) => { todo!("ICE: Cannot convert value {value:?}") } } diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs index f2e698c0aa..64e8bb1acf 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block_variables.rs @@ -173,7 +173,10 @@ pub(crate) fn allocate_value( let typ = dfg.type_of_value(value_id); match typ { - Type::Numeric(_) | Type::Reference(_) => { + Type::Numeric(_) | Type::Reference(_) | Type::Function => { + // NB. function references are converted to a constant when + // translating from SSA to Brillig (to allow for debugger + // instrumentation to work properly) let register = brillig_context.allocate_register(); BrilligVariable::Simple(register) } @@ -199,8 +202,5 @@ pub(crate) fn allocate_value( rc: rc_register, }) } - Type::Function => { - unreachable!("ICE: Function values should have been removed from the SSA") - } } } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 0e3076923e..694351006c 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -40,12 +40,13 @@ pub(crate) fn optimize_into_acir( program: Program, print_ssa_passes: bool, print_brillig_trace: bool, + force_brillig_output: bool, ) -> Result { let abi_distinctness = program.return_distinctness; let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); let ssa_gen_span_guard = ssa_gen_span.enter(); - let ssa = SsaBuilder::new(program, print_ssa_passes)? + let ssa = SsaBuilder::new(program, print_ssa_passes, force_brillig_output)? .run_pass(Ssa::defunctionalize, "After Defunctionalization:") .run_pass(Ssa::inline_functions, "After Inlining:") // Run mem2reg with the CFG separated into blocks @@ -83,10 +84,17 @@ pub fn create_circuit( program: Program, enable_ssa_logging: bool, enable_brillig_logging: bool, + force_brillig_output: bool, ) -> Result<(Circuit, DebugInfo, Vec, Vec, Vec), RuntimeError> { + let debug_variables = program.debug_variables.clone(); + let debug_types = program.debug_types.clone(); let func_sig = program.main_function_signature.clone(); - let mut generated_acir = - optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?; + let mut generated_acir = optimize_into_acir( + program, + enable_ssa_logging, + enable_brillig_logging, + force_brillig_output, + )?; let opcodes = generated_acir.take_opcodes(); let current_witness_index = generated_acir.current_witness_index().0; let GeneratedAcir { @@ -119,7 +127,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations); + let mut debug_info = DebugInfo::new(locations, debug_variables, debug_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); @@ -169,8 +177,12 @@ struct SsaBuilder { } impl SsaBuilder { - fn new(program: Program, print_ssa_passes: bool) -> Result { - let ssa = ssa_gen::generate_ssa(program)?; + fn new( + program: Program, + print_ssa_passes: bool, + force_brillig_runtime: bool, + ) -> Result { + let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?; Ok(SsaBuilder { print_ssa_passes, ssa }.print("Initial SSA:")) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 211506a6d1..03b65d513a 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1368,7 +1368,13 @@ impl Context { AcirValue::Array(elements.collect()) } Value::Intrinsic(..) => todo!(), - Value::Function(..) => unreachable!("ICE: All functions should have been inlined"), + Value::Function(function_id) => { + // This conversion is for debugging support only, to allow the + // debugging instrumentation code to work. Taking the reference + // of a function in ACIR is useless. + let id = self.acir_context.add_constant(function_id.to_usize()); + AcirValue::Var(id, AcirType::field()) + } Value::ForeignFunction(_) => unimplemented!( "Oracle calls directly in constrained functions are not yet available." ), diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 0efc73fb85..9a0dbe585a 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -38,7 +38,10 @@ use super::{ /// Generates SSA for the given monomorphized program. /// /// This function will generate the SSA but does not perform any optimizations on it. -pub(crate) fn generate_ssa(program: Program) -> Result { +pub(crate) fn generate_ssa( + program: Program, + force_brillig_runtime: bool, +) -> Result { // see which parameter has call_data/return_data attribute let is_databus = DataBusBuilder::is_databus(&program.main_function_signature); @@ -56,7 +59,11 @@ pub(crate) fn generate_ssa(program: Program) -> Result { let mut function_context = FunctionContext::new( main.name.clone(), &main.parameters, - if main.unconstrained { RuntimeType::Brillig } else { RuntimeType::Acir }, + if force_brillig_runtime || main.unconstrained { + RuntimeType::Brillig + } else { + RuntimeType::Acir + }, &context, ); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs new file mode 100644 index 0000000000..9f182d2baa --- /dev/null +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -0,0 +1,607 @@ +use crate::parser::{parse_program, ParsedModule}; +use crate::{ + ast, + ast::{Path, PathKind}, + parser::{Item, ItemKind}, +}; +use noirc_errors::{Span, Spanned}; +use std::collections::HashMap; +use std::collections::VecDeque; + +const MAX_MEMBER_ASSIGN_DEPTH: usize = 8; + +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +pub struct SourceVarId(pub u32); + +#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +pub struct SourceFieldId(pub u32); + +/// This structure is used to collect information about variables to track +/// for debugging during the instrumentation injection phase. +#[derive(Debug, Clone)] +pub struct DebugInstrumenter { + // all collected variable names while instrumenting the source for variable tracking + pub variables: HashMap, + + // all field names referenced when assigning to a member of a variable + pub field_names: HashMap, + + next_var_id: u32, + next_field_name_id: u32, + + // last seen variable names and their IDs grouped by scope + scope: Vec>, +} + +impl Default for DebugInstrumenter { + fn default() -> Self { + Self { + variables: HashMap::default(), + field_names: HashMap::default(), + scope: vec![], + next_var_id: 0, + next_field_name_id: 1, + } + } +} + +impl DebugInstrumenter { + pub fn instrument_module(&mut self, module: &mut ParsedModule) { + module.items.iter_mut().for_each(|item| { + if let Item { kind: ItemKind::Function(f), .. } = item { + self.walk_fn(&mut f.def); + } + }); + // this part absolutely must happen after ast traversal above + // so that oracle functions don't get wrapped, resulting in infinite recursion: + self.insert_state_set_oracle(module, 8); + } + + fn insert_var(&mut self, var_name: &str) -> SourceVarId { + let var_id = SourceVarId(self.next_var_id); + self.next_var_id += 1; + self.variables.insert(var_id, var_name.to_string()); + self.scope.last_mut().unwrap().insert(var_name.to_string(), var_id); + var_id + } + + fn lookup_var(&self, var_name: &str) -> Option { + self.scope.iter().rev().find_map(|vars| vars.get(var_name).copied()) + } + + fn insert_field_name(&mut self, field_name: &str) -> SourceFieldId { + let field_name_id = SourceFieldId(self.next_field_name_id); + self.next_field_name_id += 1; + self.field_names.insert(field_name_id, field_name.to_string()); + field_name_id + } + + fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { + self.scope.push(HashMap::default()); + + let set_fn_params = func + .parameters + .iter() + .flat_map(|param| { + pattern_vars(¶m.pattern) + .iter() + .map(|(id, _is_mut)| { + let var_id = self.insert_var(&id.0.contents); + build_assign_var_stmt(var_id, id_expr(id)) + }) + .collect::>() + }) + .collect(); + + self.walk_scope(&mut func.body.0, func.span); + + // prepend fn params: + func.body.0 = vec![set_fn_params, func.body.0.clone()].concat(); + } + + // Modify a vector of statements in-place, adding instrumentation for sets and drops. + // This function will consume a scope level. + fn walk_scope(&mut self, statements: &mut Vec, span: Span) { + statements.iter_mut().for_each(|stmt| self.walk_statement(stmt)); + + // extract and save the return value from the scope if there is one + let ret_stmt = statements.pop(); + let has_ret_expr = match ret_stmt { + None => false, + Some(ast::Statement { kind: ast::StatementKind::Expression(ret_expr), .. }) => { + let save_ret_expr = ast::Statement { + kind: ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Identifier(ident("__debug_expr", ret_expr.span)), + r#type: ast::UnresolvedType::unspecified(), + expression: ret_expr.clone(), + }), + span: ret_expr.span, + }; + statements.push(save_ret_expr); + true + } + Some(ret_stmt) => { + // not an expression, so leave it untouched + statements.push(ret_stmt); + false + } + }; + + let span = Span::empty(span.end()); + + // drop scope variables + let scope_vars = self.scope.pop().unwrap_or(HashMap::default()); + let drop_vars_stmts = scope_vars.values().map(|var_id| build_drop_var_stmt(*var_id, span)); + statements.extend(drop_vars_stmts); + + // return the saved value in __debug_expr, or unit otherwise + let last_stmt = if has_ret_expr { + ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_expr", span)], + kind: PathKind::Plain, + span, + }), + span, + }), + span, + } + } else { + ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Unit), + span, + }), + span, + } + }; + statements.push(last_stmt); + } + + fn walk_let_statement(&mut self, let_stmt: &ast::LetStatement, span: &Span) -> ast::Statement { + // rewrites let statements written like this: + // let (((a,b,c),D { d }),e,f) = x; + // + // into statements like this: + // + // let (a,b,c,d,e,f,g) = { + // let (((a,b,c),D { d }),e,f) = x; + // wrap(1, a); + // wrap(2, b); + // ... + // wrap(6, f); + // (a,b,c,d,e,f,g) + // }; + + // a.b.c[3].x[i*4+1].z + + let vars = pattern_vars(&let_stmt.pattern); + let vars_pattern: Vec = vars + .iter() + .map(|(id, is_mut)| { + if *is_mut { + ast::Pattern::Mutable(Box::new(ast::Pattern::Identifier(id.clone())), id.span()) + } else { + ast::Pattern::Identifier(id.clone()) + } + }) + .collect(); + let vars_exprs: Vec = vars.iter().map(|(id, _)| id_expr(id)).collect(); + + let mut block_stmts = + vec![ast::Statement { kind: ast::StatementKind::Let(let_stmt.clone()), span: *span }]; + block_stmts.extend(vars.iter().map(|(id, _)| { + let var_id = self.insert_var(&id.0.contents); + build_assign_var_stmt(var_id, id_expr(id)) + })); + block_stmts.push(ast::Statement { + kind: ast::StatementKind::Expression(ast::Expression { + kind: ast::ExpressionKind::Tuple(vars_exprs), + span: let_stmt.pattern.span(), + }), + span: let_stmt.pattern.span(), + }); + + ast::Statement { + kind: ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Tuple(vars_pattern, let_stmt.pattern.span()), + r#type: ast::UnresolvedType::unspecified(), + expression: ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(block_stmts)), + span: let_stmt.expression.span, + }, + }), + span: *span, + } + } + + fn walk_assign_statement( + &mut self, + assign_stmt: &ast::AssignStatement, + span: &Span, + ) -> ast::Statement { + // X = Y becomes: + // X = { + // let __debug_expr = Y; + // + // __debug_var_assign(17, __debug_expr); + // // or: + // __debug_member_assign_{arity}(17, __debug_expr, _v0, _v1..., _v{arity}); + // + // __debug_expr + // }; + + let let_kind = ast::StatementKind::Let(ast::LetStatement { + pattern: ast::Pattern::Identifier(ident("__debug_expr", assign_stmt.expression.span)), + r#type: ast::UnresolvedType::unspecified(), + expression: assign_stmt.expression.clone(), + }); + let expression_span = assign_stmt.expression.span; + let new_assign_stmt = match &assign_stmt.lvalue { + ast::LValue::Ident(id) => { + let var_id = self + .lookup_var(&id.0.contents) + .unwrap_or_else(|| panic!("var lookup failed for var_name={}", &id.0.contents)); + build_assign_var_stmt(var_id, id_expr(&ident("__debug_expr", id.span()))) + } + ast::LValue::Dereference(_lv) => { + // TODO: this is a dummy statement for now, but we should + // somehow track the derefence and update the pointed to + // variable + ast::Statement { + kind: ast::StatementKind::Expression(uint_expr(0, *span)), + span: *span, + } + } + _ => { + let mut indexes = vec![]; + let mut cursor = &assign_stmt.lvalue; + let var_id; + loop { + match cursor { + ast::LValue::Ident(id) => { + var_id = self.lookup_var(&id.0.contents).unwrap_or_else(|| { + panic!("var lookup failed for var_name={}", &id.0.contents) + }); + break; + } + ast::LValue::MemberAccess { object, field_name } => { + cursor = object; + let field_name_id = self.insert_field_name(&field_name.0.contents); + indexes.push(sint_expr(-(field_name_id.0 as i128), expression_span)); + } + ast::LValue::Index { index, array } => { + cursor = array; + indexes.push(index.clone()); + } + ast::LValue::Dereference(_ref) => { + unimplemented![] + } + } + } + build_assign_member_stmt( + var_id, + &indexes, + &id_expr(&ident("__debug_expr", expression_span)), + ) + } + }; + let ret_kind = + ast::StatementKind::Expression(id_expr(&ident("__debug_expr", expression_span))); + + ast::Statement { + kind: ast::StatementKind::Assign(ast::AssignStatement { + lvalue: assign_stmt.lvalue.clone(), + expression: ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ + ast::Statement { kind: let_kind, span: expression_span }, + new_assign_stmt, + ast::Statement { kind: ret_kind, span: expression_span }, + ])), + span: expression_span, + }, + }), + span: *span, + } + } + + fn walk_expr(&mut self, expr: &mut ast::Expression) { + match &mut expr.kind { + ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => { + self.scope.push(HashMap::default()); + self.walk_scope(statements, expr.span); + } + ast::ExpressionKind::Prefix(prefix_expr) => { + self.walk_expr(&mut prefix_expr.rhs); + } + ast::ExpressionKind::Index(index_expr) => { + self.walk_expr(&mut index_expr.collection); + self.walk_expr(&mut index_expr.index); + } + ast::ExpressionKind::Call(call_expr) => { + // TODO: push a stack frame or something here? + self.walk_expr(&mut call_expr.func); + call_expr.arguments.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + } + ast::ExpressionKind::MethodCall(mc_expr) => { + // TODO: also push a stack frame here + self.walk_expr(&mut mc_expr.object); + mc_expr.arguments.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + } + ast::ExpressionKind::Constructor(c_expr) => { + c_expr.fields.iter_mut().for_each(|(_id, ref mut expr)| { + self.walk_expr(expr); + }); + } + ast::ExpressionKind::MemberAccess(ma_expr) => { + self.walk_expr(&mut ma_expr.lhs); + } + ast::ExpressionKind::Cast(cast_expr) => { + self.walk_expr(&mut cast_expr.lhs); + } + ast::ExpressionKind::Infix(infix_expr) => { + self.walk_expr(&mut infix_expr.lhs); + self.walk_expr(&mut infix_expr.rhs); + } + ast::ExpressionKind::If(if_expr) => { + self.walk_expr(&mut if_expr.condition); + self.walk_expr(&mut if_expr.consequence); + if let Some(ref mut alt) = if_expr.alternative { + self.walk_expr(alt); + } + } + ast::ExpressionKind::Tuple(exprs) => { + exprs.iter_mut().for_each(|ref mut expr| { + self.walk_expr(expr); + }); + } + ast::ExpressionKind::Lambda(lambda) => { + self.walk_expr(&mut lambda.body); + } + ast::ExpressionKind::Parenthesized(expr) => { + self.walk_expr(expr); + } + _ => {} + } + } + + fn walk_for(&mut self, for_stmt: &mut ast::ForLoopStatement) { + let var_name = &for_stmt.identifier.0.contents; + let var_id = self.insert_var(var_name); + + let set_stmt = build_assign_var_stmt(var_id, id_expr(&for_stmt.identifier)); + let drop_stmt = build_drop_var_stmt(var_id, Span::empty(for_stmt.span.end())); + + self.walk_expr(&mut for_stmt.block); + for_stmt.block = ast::Expression { + kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![ + set_stmt, + ast::Statement { + kind: ast::StatementKind::Semi(for_stmt.block.clone()), + span: for_stmt.block.span, + }, + drop_stmt, + ])), + span: for_stmt.span, + }; + } + + fn walk_statement(&mut self, stmt: &mut ast::Statement) { + match &mut stmt.kind { + ast::StatementKind::Let(let_stmt) => { + *stmt = self.walk_let_statement(let_stmt, &stmt.span); + } + ast::StatementKind::Assign(assign_stmt) => { + *stmt = self.walk_assign_statement(assign_stmt, &stmt.span); + } + ast::StatementKind::Expression(expr) => { + self.walk_expr(expr); + } + ast::StatementKind::Semi(expr) => { + self.walk_expr(expr); + } + ast::StatementKind::For(ref mut for_stmt) => { + self.walk_for(for_stmt); + } + _ => {} // Constrain, Error + } + } + + fn insert_state_set_oracle(&self, module: &mut ParsedModule, n: u32) { + let member_assigns = (1..=n) + .map(|i| format!["__debug_member_assign_{i}"]) + .collect::>() + .join(",\n"); + let (program, errors) = parse_program(&format!( + r#" + use dep::__debug::{{ + __debug_var_assign, + __debug_var_drop, + __debug_dereference_assign, + {member_assigns}, + }};"# + )); + if !errors.is_empty() { + panic!("errors parsing internal oracle definitions: {errors:?}") + } + module.items.extend(program.items); + } +} + +pub fn build_debug_crate_file() -> String { + [ + r#" + #[oracle(__debug_var_assign)] + unconstrained fn __debug_var_assign_oracle(_var_id: u32, _value: T) {} + unconstrained fn __debug_var_assign_inner(var_id: u32, value: T) { + __debug_var_assign_oracle(var_id, value); + } + pub fn __debug_var_assign(var_id: u32, value: T) { + __debug_var_assign_inner(var_id, value); + } + + #[oracle(__debug_var_drop)] + unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} + unconstrained fn __debug_var_drop_inner(var_id: u32) { + __debug_var_drop_oracle(var_id); + } + pub fn __debug_var_drop(var_id: u32) { + __debug_var_drop_inner(var_id); + } + + #[oracle(__debug_dereference_assign)] + unconstrained fn __debug_dereference_assign_oracle(_var_id: u32, _value: T) {} + unconstrained fn __debug_dereference_assign_inner(var_id: u32, value: T) { + __debug_dereference_assign_oracle(var_id, value); + } + pub fn __debug_dereference_assign(var_id: u32, value: T) { + __debug_dereference_assign_inner(var_id, value); + } + "# + .to_string(), + (1..=MAX_MEMBER_ASSIGN_DEPTH) + .map(|n| { + let var_sig = + (0..n).map(|i| format!["_v{i}: Field"]).collect::>().join(", "); + let vars = (0..n).map(|i| format!["_v{i}"]).collect::>().join(", "); + format!( + r#" + #[oracle(__debug_member_assign_{n})] + unconstrained fn __debug_oracle_member_assign_{n}( + _var_id: u32, _value: T, {var_sig} + ) {{}} + unconstrained fn __debug_inner_member_assign_{n}( + var_id: u32, value: T, {var_sig} + ) {{ + __debug_oracle_member_assign_{n}(var_id, value, {vars}); + }} + pub fn __debug_member_assign_{n}(var_id: u32, value: T, {var_sig}) {{ + __debug_inner_member_assign_{n}(var_id, value, {vars}); + }} + + "# + ) + }) + .collect::>() + .join("\n"), + ] + .join("\n") +} + +fn build_assign_var_stmt(var_id: SourceVarId, expr: ast::Expression) -> ast::Statement { + let span = expr.span; + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_var_assign", span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: vec![uint_expr(var_id.0 as u128, span), expr], + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } +} + +fn build_drop_var_stmt(var_id: SourceVarId, span: Span) -> ast::Statement { + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident("__debug_var_drop", span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: vec![uint_expr(var_id.0 as u128, span)], + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } +} + +fn build_assign_member_stmt( + var_id: SourceVarId, + indexes: &[ast::Expression], + expr: &ast::Expression, +) -> ast::Statement { + let arity = indexes.len(); + if arity > MAX_MEMBER_ASSIGN_DEPTH { + unreachable!("Assignment to member exceeds maximum depth for debugging"); + } + let span = expr.span; + let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { + func: Box::new(ast::Expression { + kind: ast::ExpressionKind::Variable(ast::Path { + segments: vec![ident(&format!["__debug_member_assign_{arity}"], span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: [ + vec![uint_expr(var_id.0 as u128, span)], + vec![expr.clone()], + indexes.iter().rev().cloned().collect(), + ] + .concat(), + })); + ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } +} + +fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { + let mut vars = vec![]; + let mut stack = VecDeque::from([(pattern, false)]); + while stack.front().is_some() { + let (pattern, is_mut) = stack.pop_front().unwrap(); + match pattern { + ast::Pattern::Identifier(id) => { + vars.push((id.clone(), is_mut)); + } + ast::Pattern::Mutable(pattern, _) => { + stack.push_back((pattern, true)); + } + ast::Pattern::Tuple(patterns, _) => { + stack.extend(patterns.iter().map(|pattern| (pattern, false))); + } + ast::Pattern::Struct(_, pids, _) => { + stack.extend(pids.iter().map(|(_, pattern)| (pattern, is_mut))); + vars.extend(pids.iter().map(|(id, _)| (id.clone(), false))); + } + } + } + vars +} + +fn ident(s: &str, span: Span) -> ast::Ident { + ast::Ident(Spanned::from(span, s.to_string())) +} + +fn id_expr(id: &ast::Ident) -> ast::Expression { + ast::Expression { + kind: ast::ExpressionKind::Variable(Path { + segments: vec![id.clone()], + kind: PathKind::Plain, + span: id.span(), + }), + span: id.span(), + } +} + +fn uint_expr(x: u128, span: Span) -> ast::Expression { + ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Integer(x.into(), false)), + span, + } +} + +fn sint_expr(x: i128, span: Span) -> ast::Expression { + ast::Expression { + kind: ast::ExpressionKind::Literal(ast::Literal::Integer(x.abs().into(), x < 0)), + span, + } +} diff --git a/compiler/noirc_frontend/src/hir/mod.rs b/compiler/noirc_frontend/src/hir/mod.rs index 2124b5281f..4d3800f1a5 100644 --- a/compiler/noirc_frontend/src/hir/mod.rs +++ b/compiler/noirc_frontend/src/hir/mod.rs @@ -4,6 +4,7 @@ pub mod resolution; pub mod scope; pub mod type_check; +use crate::debug::DebugInstrumenter; use crate::graph::{CrateGraph, CrateId}; use crate::hir_def::function::FuncMeta; use crate::node_interner::{FuncId, NodeInterner, StructId}; @@ -31,6 +32,8 @@ pub struct Context<'file_manager, 'parsed_files> { // is read-only however, once it has been passed to the Context. pub file_manager: Cow<'file_manager, FileManager>, + pub debug_instrumenter: DebugInstrumenter, + /// A map of each file that already has been visited from a prior `mod foo;` declaration. /// This is used to issue an error if a second `mod foo;` is declared to the same file. pub visited_files: BTreeMap, @@ -56,6 +59,7 @@ impl Context<'_, '_> { visited_files: BTreeMap::new(), crate_graph: CrateGraph::default(), file_manager: Cow::Owned(file_manager), + debug_instrumenter: DebugInstrumenter::default(), parsed_files: Cow::Owned(parsed_files), } } @@ -70,6 +74,7 @@ impl Context<'_, '_> { visited_files: BTreeMap::new(), crate_graph: CrateGraph::default(), file_manager: Cow::Borrowed(file_manager), + debug_instrumenter: DebugInstrumenter::default(), parsed_files: Cow::Borrowed(parsed_files), } } diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index b14cb632f8..eb00a61adf 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -11,6 +11,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] pub mod ast; +pub mod debug; pub mod graph; pub mod lexer; pub mod monomorphization; diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 5172eabde4..ccfce12aff 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -1,6 +1,9 @@ use acvm::FieldElement; use iter_extended::vecmap; -use noirc_errors::Location; +use noirc_errors::{ + debug_info::{DebugTypes, DebugVariables}, + Location, +}; use crate::{ hir_def::function::FunctionSignature, BinaryOpKind, Distinctness, Signedness, Visibility, @@ -246,6 +249,8 @@ pub struct Program { pub return_distinctness: Distinctness, pub return_location: Option, pub return_visibility: Visibility, + pub debug_variables: DebugVariables, + pub debug_types: DebugTypes, } impl Program { @@ -255,6 +260,8 @@ impl Program { return_distinctness: Distinctness, return_location: Option, return_visibility: Visibility, + debug_variables: DebugVariables, + debug_types: DebugTypes, ) -> Program { Program { functions, @@ -262,6 +269,8 @@ impl Program { return_distinctness, return_location, return_visibility, + debug_variables, + debug_types, } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs new file mode 100644 index 0000000000..d36816e3d3 --- /dev/null +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -0,0 +1,190 @@ +use iter_extended::vecmap; +use noirc_errors::debug_info::DebugVarId; +use noirc_errors::Location; +use noirc_printable_type::PrintableType; + +use crate::debug::{SourceFieldId, SourceVarId}; +use crate::hir_def::expr::*; +use crate::node_interner::ExprId; + +use super::ast::{Expression, Ident}; +use super::Monomorphizer; + +const DEBUG_MEMBER_ASSIGN_PREFIX: &str = "__debug_member_assign_"; +const DEBUG_VAR_ID_ARG_SLOT: usize = 0; +const DEBUG_VALUE_ARG_SLOT: usize = 1; +const DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT: usize = 2; + +impl From for SourceVarId { + fn from(var_id: u128) -> Self { + Self(var_id as u32) + } +} + +impl From for SourceFieldId { + fn from(field_id: u128) -> Self { + Self(field_id as u32) + } +} + +impl<'interner> Monomorphizer<'interner> { + /// Patch instrumentation calls inserted for debugging. This will record + /// tracked variables and their types, and assign them an ID to use at + /// runtime. This ID is different from the source ID assigned at + /// instrumentation time because at that point we haven't fully resolved the + /// types for generic functions. So a single generic function may be + /// instantiated multiple times with its tracked variables being of + /// different types for each instance at runtime. + pub(super) fn patch_debug_instrumentation_call( + &mut self, + call: &HirCallExpression, + arguments: &mut [Expression], + ) { + let original_func = Box::new(self.expr(call.func)); + if let Expression::Ident(Ident { name, .. }) = original_func.as_ref() { + if name == "__debug_var_assign" { + self.patch_debug_var_assign(call, arguments); + } else if name == "__debug_var_drop" { + self.patch_debug_var_drop(call, arguments); + } else if let Some(arity) = name.strip_prefix(DEBUG_MEMBER_ASSIGN_PREFIX) { + let arity = arity.parse::().expect("failed to parse member assign arity"); + self.patch_debug_member_assign(call, arguments, arity); + } + } + } + + /// Update instrumentation code inserted on variable assignment. We need to + /// register the variable instance, its type and replace the source_var_id + /// with the ID of the registration. Multiple registrations of the same + /// variable are possible if using generic functions, hence the temporary ID + /// created when injecting the instrumentation code can map to multiple IDs + /// at runtime. + fn patch_debug_var_assign(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) { + let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT); + let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else { + unreachable!("Missing source_var_id in __debug_var_assign call"); + }; + + // instantiate tracked variable for the value type and associate it with + // the ID used by the injected instrumentation code + let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]); + let source_var_id = source_var_id.to_u128().into(); + // then update the ID used for tracking at runtime + let var_id = self.debug_type_tracker.insert_var(source_var_id, var_type); + let interned_var_id = self.intern_var_id(var_id, &call.location); + arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); + } + + /// Update instrumentation code for a variable being dropped out of scope. + /// Given the source_var_id we search for the last assigned debug var_id and + /// replace it instead. + fn patch_debug_var_drop(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) { + let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT); + let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else { + unreachable!("Missing source_var_id in __debug_var_drop call"); + }; + // update variable ID for tracked drops (ie. when the var goes out of scope) + let source_var_id = source_var_id.to_u128().into(); + let var_id = self + .debug_type_tracker + .get_var_id(source_var_id) + .unwrap_or_else(|| unreachable!("failed to find debug variable")); + let interned_var_id = self.intern_var_id(var_id, &call.location); + arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); + } + + /// Update instrumentation code inserted when assigning to a member of an + /// existing variable. Same as above for replacing the source_var_id, but also + /// we need to resolve the path and the type of the member being assigned. + /// For this last part, we need to resolve the mapping from field names in + /// structs to positions in the runtime tuple, since all structs are + /// replaced by tuples during compilation. + fn patch_debug_member_assign( + &mut self, + call: &HirCallExpression, + arguments: &mut [Expression], + arity: usize, + ) { + let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT); + let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else { + unreachable!("Missing source_var_id in __debug_member_assign call"); + }; + // update variable member assignments + let source_var_id = source_var_id.to_u128().into(); + + let var_type = self + .debug_type_tracker + .get_type(source_var_id) + .unwrap_or_else(|| panic!("type not found for {source_var_id:?}")) + .clone(); + let mut cursor_type = &var_type; + for i in 0..arity { + if let Some(HirExpression::Literal(HirLiteral::Integer(fe_i, i_neg))) = + hir_arguments.get(DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i) + { + let index = fe_i.to_i128().unsigned_abs(); + if *i_neg { + // We use negative indices at instrumentation time to indicate + // and reference member accesses by name which cannot be + // resolved until we have a type. This strategy is also used + // for tuple member access because syntactically they are + // the same as named field accesses. + let field_index = self + .debug_type_tracker + .resolve_field_index(index.into(), cursor_type) + .unwrap_or_else(|| { + unreachable!("failed to resolve {i}-th member indirection on type {cursor_type:?}") + }); + + cursor_type = element_type_at_index(cursor_type, field_index); + let index_id = self.interner.push_expr(HirExpression::Literal( + HirLiteral::Integer(field_index.into(), false), + )); + self.interner.push_expr_type(&index_id, crate::Type::FieldElement); + self.interner.push_expr_location( + index_id, + call.location.span, + call.location.file, + ); + arguments[DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i] = self.expr(index_id); + } else { + // array/string element using constant index + cursor_type = element_type_at_index(cursor_type, index as usize); + } + } else { + // array element using non-constant index + cursor_type = element_type_at_index(cursor_type, 0); + } + } + + let var_id = self + .debug_type_tracker + .get_var_id(source_var_id) + .unwrap_or_else(|| unreachable!("failed to find debug variable")); + let interned_var_id = self.intern_var_id(var_id, &call.location); + arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id); + } + + fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId { + let var_id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false); + let expr_id = self.interner.push_expr(HirExpression::Literal(var_id_literal)); + self.interner.push_expr_type(&expr_id, crate::Type::FieldElement); + self.interner.push_expr_location(expr_id, location.span, location.file); + expr_id + } +} + +fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType { + match ptype { + PrintableType::Array { length: _length, typ } => typ.as_ref(), + PrintableType::Tuple { types } => &types[i], + PrintableType::Struct { name: _name, fields } => &fields[i].1, + PrintableType::String { length: _length } => &PrintableType::UnsignedInteger { width: 8 }, + _ => { + panic!["expected type with sub-fields, found terminal type"] + } + } +} diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs new file mode 100644 index 0000000000..fea073d394 --- /dev/null +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -0,0 +1,137 @@ +use crate::{ + debug::{DebugInstrumenter, SourceFieldId, SourceVarId}, + hir_def::types::Type, +}; +use noirc_errors::debug_info::{ + DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, +}; +use noirc_printable_type::PrintableType; +use std::collections::HashMap; + +/// We keep a collection of the debug variables and their types in this +/// structure. The source_var_id refers to the ID given by the debug +/// instrumenter. This variable does not have a type yet and hence it +/// can be instantiated for multiple types if it's in the context of a generic +/// variable. The var_id refers to the ID of the instantiated variable which +/// will have a valid type. +#[derive(Debug, Clone, Default)] +pub struct DebugTypeTracker { + // Variable names collected during instrumentation injection + source_variables: HashMap, + + // Field names used for member access collected during instrumentation injection + source_field_names: HashMap, + + // Current instances of tracked variables from the ID given during + // instrumentation. The tracked var_id will change for each source_var_id + // when compiling generic functions. + source_to_debug_vars: HashMap, + + // All instances of tracked variables + variables: HashMap, + + // Types of tracked variables + types: HashMap, + types_reverse: HashMap, + + next_var_id: u32, + next_type_id: u32, +} + +impl DebugTypeTracker { + pub fn build_from_debug_instrumenter(instrumenter: &DebugInstrumenter) -> Self { + DebugTypeTracker { + source_variables: instrumenter.variables.clone(), + source_field_names: instrumenter.field_names.clone(), + ..DebugTypeTracker::default() + } + } + + pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugTypes) { + let debug_variables = self + .variables + .clone() + .into_iter() + .map(|(var_id, (source_var_id, type_id))| { + ( + var_id, + DebugVariable { + name: self.source_variables.get(&source_var_id).cloned().unwrap_or_else( + || { + unreachable!( + "failed to retrieve variable name for {source_var_id:?}" + ); + }, + ), + debug_type_id: type_id, + }, + ) + }) + .collect(); + let debug_types = self.types.clone().into_iter().collect(); + + (debug_variables, debug_types) + } + + pub fn resolve_field_index( + &self, + field_id: SourceFieldId, + cursor_type: &PrintableType, + ) -> Option { + self.source_field_names + .get(&field_id) + .and_then(|field_name| get_field(cursor_type, field_name)) + } + + pub fn insert_var(&mut self, source_var_id: SourceVarId, var_type: Type) -> DebugVarId { + if !self.source_variables.contains_key(&source_var_id) { + unreachable!("cannot find source debug variable {source_var_id:?}"); + } + + let ptype: PrintableType = var_type.follow_bindings().into(); + let type_id = self.types_reverse.get(&ptype).copied().unwrap_or_else(|| { + let type_id = DebugTypeId(self.next_type_id); + self.next_type_id += 1; + self.types_reverse.insert(ptype.clone(), type_id); + self.types.insert(type_id, ptype); + type_id + }); + // check if we need to instantiate the var with a new type + let existing_var_id = self.source_to_debug_vars.get(&source_var_id).and_then(|var_id| { + let (_, existing_type_id) = self.variables.get(var_id).unwrap(); + (*existing_type_id == type_id).then_some(var_id) + }); + if let Some(var_id) = existing_var_id { + *var_id + } else { + let var_id = DebugVarId(self.next_var_id); + self.next_var_id += 1; + self.variables.insert(var_id, (source_var_id, type_id)); + self.source_to_debug_vars.insert(source_var_id, var_id); + var_id + } + } + + pub fn get_var_id(&self, source_var_id: SourceVarId) -> Option { + self.source_to_debug_vars.get(&source_var_id).copied() + } + + pub fn get_type(&self, source_var_id: SourceVarId) -> Option<&PrintableType> { + self.source_to_debug_vars + .get(&source_var_id) + .and_then(|var_id| self.variables.get(var_id)) + .and_then(|(_, type_id)| self.types.get(type_id)) + } +} + +fn get_field(ptype: &PrintableType, field_name: &str) -> Option { + match ptype { + PrintableType::Struct { fields, .. } => { + fields.iter().position(|(name, _)| name == field_name) + } + PrintableType::Tuple { .. } | PrintableType::Array { .. } => { + field_name.parse::().ok() + } + _ => None, + } +} diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 7e06b94ca5..a4780e81f7 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -18,9 +18,10 @@ use std::{ }; use crate::{ + debug::DebugInstrumenter, hir_def::{ expr::*, - function::{FunctionSignature, Parameters}, + function::{FuncMeta, FunctionSignature, Parameters}, stmt::{HirAssignStatement, HirLValue, HirLetStatement, HirPattern, HirStatement}, types, }, @@ -31,8 +32,11 @@ use crate::{ }; use self::ast::{Definition, FuncId, Function, LocalId, Program}; +use self::debug_types::DebugTypeTracker; pub mod ast; +mod debug; +pub mod debug_types; pub mod printer; struct LambdaContext { @@ -67,7 +71,7 @@ struct Monomorphizer<'interner> { finished_functions: BTreeMap, /// Used to reference existing definitions in the HIR - interner: &'interner NodeInterner, + interner: &'interner mut NodeInterner, lambda_envs_stack: Vec, @@ -77,6 +81,8 @@ struct Monomorphizer<'interner> { is_range_loop: bool, return_location: Option, + + debug_type_tracker: DebugTypeTracker, } type HirType = crate::Type; @@ -93,8 +99,17 @@ type HirType = crate::Type; /// this function. Typically, this is the function named "main" in the source project, /// but it can also be, for example, an arbitrary test function for running `nargo test`. #[tracing::instrument(level = "trace", skip(main, interner))] -pub fn monomorphize(main: node_interner::FuncId, interner: &NodeInterner) -> Program { - let mut monomorphizer = Monomorphizer::new(interner); +pub fn monomorphize(main: node_interner::FuncId, interner: &mut NodeInterner) -> Program { + monomorphize_debug(main, interner, &DebugInstrumenter::default()) +} + +pub fn monomorphize_debug( + main: node_interner::FuncId, + interner: &mut NodeInterner, + debug_instrumenter: &DebugInstrumenter, +) -> Program { + let debug_type_tracker = DebugTypeTracker::build_from_debug_instrumenter(debug_instrumenter); + let mut monomorphizer = Monomorphizer::new(interner, debug_type_tracker); let function_sig = monomorphizer.compile_main(main); while !monomorphizer.queue.is_empty() { @@ -109,19 +124,23 @@ pub fn monomorphize(main: node_interner::FuncId, interner: &NodeInterner) -> Pro } let functions = vecmap(monomorphizer.finished_functions, |(_, f)| f); - let meta = interner.function_meta(&main); + let FuncMeta { return_distinctness, return_visibility, .. } = + monomorphizer.interner.function_meta(&main); + let (debug_variables, debug_types) = monomorphizer.debug_type_tracker.extract_vars_and_types(); Program::new( functions, function_sig, - meta.return_distinctness, + *return_distinctness, monomorphizer.return_location, - meta.return_visibility, + *return_visibility, + debug_variables, + debug_types, ) } impl<'interner> Monomorphizer<'interner> { - fn new(interner: &'interner NodeInterner) -> Self { + fn new(interner: &'interner mut NodeInterner, debug_type_tracker: DebugTypeTracker) -> Self { Monomorphizer { globals: HashMap::new(), locals: HashMap::new(), @@ -133,6 +152,7 @@ impl<'interner> Monomorphizer<'interner> { lambda_envs_stack: Vec::new(), is_range_loop: false, return_location: None, + debug_type_tracker, } } @@ -230,7 +250,7 @@ impl<'interner> Monomorphizer<'interner> { the_trait.self_type_typevar.force_bind(self_type); } - let meta = self.interner.function_meta(&f); + let meta = self.interner.function_meta(&f).clone(); let modifiers = self.interner.function_modifiers(&f); let name = self.interner.function_name(&f).to_owned(); @@ -240,14 +260,13 @@ impl<'interner> Monomorphizer<'interner> { Type::TraitAsType(..) => &body_return_type, _ => meta.return_type(), }); - - let parameters = self.parameters(&meta.parameters); - - let body = self.expr(body_expr_id); let unconstrained = modifiers.is_unconstrained || matches!(modifiers.contract_function_type, Some(ContractFunctionType::Open)); + let parameters = self.parameters(&meta.parameters); + let body = self.expr(body_expr_id); let function = ast::Function { id, name, parameters, body, return_type, unconstrained }; + self.push_function(id, function); } @@ -919,6 +938,9 @@ impl<'interner> Monomorphizer<'interner> { let original_func = Box::new(self.expr(call.func)); let mut arguments = vecmap(&call.arguments, |id| self.expr(*id)); let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id)); + + self.patch_debug_instrumentation_call(&call, &mut arguments); + let return_type = self.interner.id_type(id); let return_type = self.convert_type(&return_type); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index a4246a9fe7..82f66fe892 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1128,9 +1128,9 @@ mod test { } fn check_rewrite(src: &str, expected: &str) { - let (_program, context, _errors) = get_program(src); + let (_program, mut context, _errors) = get_program(src); let main_func_id = context.def_interner.find_function("main").unwrap(); - let program = monomorphize(main_func_id, &context.def_interner); + let program = monomorphize(main_func_id, &mut context.def_interner); assert!(format!("{}", program) == expected); } diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index eb206f0af3..24f4f275a1 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -6,7 +6,7 @@ use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; use thiserror::Error; -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] pub enum PrintableType { Field, @@ -50,6 +50,7 @@ pub enum PrintableValue { String(String), Vec(Vec), Struct(BTreeMap), + Other, } /// In order to display a `PrintableValue` we need a `PrintableType` to accurately @@ -296,7 +297,7 @@ fn format_field_string(field: FieldElement) -> String { } /// Assumes that `field_iterator` contains enough [FieldElement] in order to decode the [PrintableType] -fn decode_value( +pub fn decode_value( field_iterator: &mut impl Iterator, typ: &PrintableType, ) -> PrintableValue { diff --git a/tooling/debugger/Cargo.toml b/tooling/debugger/Cargo.toml index 4d240c61f9..785eacf946 100644 --- a/tooling/debugger/Cargo.toml +++ b/tooling/debugger/Cargo.toml @@ -11,11 +11,12 @@ build-data.workspace = true [dependencies] acvm.workspace = true +fm.workspace = true nargo.workspace = true +noirc_frontend.workspace = true noirc_printable_type.workspace = true noirc_errors.workspace = true noirc_driver.workspace = true -fm.workspace = true thiserror.workspace = true codespan-reporting.workspace = true dap.workspace = true diff --git a/tooling/debugger/build.rs b/tooling/debugger/build.rs index cedeebcae8..26a8bc64b0 100644 --- a/tooling/debugger/build.rs +++ b/tooling/debugger/build.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; @@ -6,9 +7,6 @@ use std::{env, fs}; const GIT_COMMIT: &&str = &"GIT_COMMIT"; fn main() { - // Rebuild if the tests have changed - println!("cargo:rerun-if-changed=tests"); - // Only use build_data if the environment variable isn't set // The environment variable is always set when working via Nix if std::env::var(GIT_COMMIT).is_err() { @@ -29,6 +27,11 @@ fn main() { }; let test_dir = root_dir.join("test_programs"); + // Rebuild if the tests have changed + println!("cargo:rerun-if-changed=tests"); + println!("cargo:rerun-if-changed=ignored-tests.txt"); + println!("cargo:rerun-if-changed={}", test_dir.as_os_str().to_str().unwrap()); + generate_debugger_tests(&mut test_file, &test_dir); } @@ -38,10 +41,13 @@ fn generate_debugger_tests(test_file: &mut File, test_data_dir: &Path) { let test_case_dirs = fs::read_dir(test_data_dir).unwrap().flatten().filter(|c| c.path().is_dir()); + let ignored_tests_contents = fs::read_to_string("ignored-tests.txt").unwrap(); + let ignored_tests = ignored_tests_contents.lines().collect::>(); for test_dir in test_case_dirs { let test_name = test_dir.file_name().into_string().expect("Directory can't be converted to string"); + let ignored = ignored_tests.contains(test_name.as_str()); if test_name.contains('-') { panic!( "Invalid test directory: {test_name}. Cannot include `-`, please convert to `_`" @@ -53,11 +59,13 @@ fn generate_debugger_tests(test_file: &mut File, test_data_dir: &Path) { test_file, r#" #[test] +{ignored} fn debug_{test_name}() {{ debugger_execution_success("{test_dir}"); }} "#, test_dir = test_dir.display(), + ignored = if ignored { "#[ignore]" } else { "" }, ) .expect("Could not write templated test file."); } diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt new file mode 100644 index 0000000000..1aa886866f --- /dev/null +++ b/tooling/debugger/ignored-tests.txt @@ -0,0 +1,18 @@ +array_sort +assign_ex +bit_shifts_comptime +brillig_cow +brillig_nested_arrays +brillig_references +brillig_to_bytes_integration +debug_logs +double_verify_proof +modulus +nested_array_dynamic +nested_array_in_slice +nested_arrays_from_brillig +references +scalar_mul +signed_comparison +simple_2d_array +to_bytes_integration diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index ab807c65a4..3cababe760 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -1,3 +1,4 @@ +use crate::foreign_calls::DebugForeignCallExecutor; use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation}; use acvm::acir::native_types::{Witness, WitnessMap}; use acvm::brillig_vm::brillig::ForeignCallResult; @@ -9,8 +10,8 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use nargo::artifacts::debug::DebugArtifact; use nargo::errors::{ExecutionError, Location}; -use nargo::ops::ForeignCallExecutor; use nargo::NargoError; +use noirc_printable_type::{PrintableType, PrintableValue}; use std::collections::{hash_set::Iter, HashSet}; @@ -25,7 +26,7 @@ pub(super) enum DebugCommandResult { pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { acvm: ACVM<'a, B>, brillig_solver: Option>, - foreign_call_executor: Box, + foreign_call_executor: Box, debug_artifact: &'a DebugArtifact, breakpoints: HashSet, } @@ -36,7 +37,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { circuit: &'a Circuit, debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, - foreign_call_executor: Box, + foreign_call_executor: Box, ) -> Self { Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), @@ -77,15 +78,49 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } + pub(super) fn is_source_location_in_debug_module(&self, location: &Location) -> bool { + self.debug_artifact + .file_map + .get(&location.file) + .map(|file| file.path.starts_with("__debug/")) + .unwrap_or(false) + } + /// Returns the callstack in source code locations for the currently /// executing opcode. This can be `None` if the execution finished (and /// `get_current_opcode_location()` returns `None`) or if the opcode is not /// mapped to a specific source location in the debug artifact (which can - /// happen for certain opcodes inserted synthetically by the compiler) + /// happen for certain opcodes inserted synthetically by the compiler). + /// This function also filters source locations that are determined to be in + /// the internal debug module. pub(super) fn get_current_source_location(&self) -> Option> { self.get_current_opcode_location() .as_ref() - .and_then(|location| self.debug_artifact.debug_symbols[0].opcode_location(location)) + .map(|opcode_location| self.get_source_location_for_opcode_location(opcode_location)) + .filter(|v: &Vec| !v.is_empty()) + } + + /// Returns the (possible) stack of source locations corresponding to the + /// given opcode location. Due to compiler inlining it's possible for this + /// function to return multiple source locations. An empty vector means that + /// the given opcode location cannot be mapped back to a source location + /// (eg. it may be pure debug instrumentation code or other synthetically + /// produced opcode by the compiler) + pub(super) fn get_source_location_for_opcode_location( + &self, + opcode_location: &OpcodeLocation, + ) -> Vec { + self.debug_artifact.debug_symbols[0] + .opcode_location(opcode_location) + .map(|source_locations| { + source_locations + .into_iter() + .filter(|source_location| { + !self.is_source_location_in_debug_module(source_location) + }) + .collect() + }) + .unwrap_or(vec![]) } fn get_opcodes_sizes(&self) -> Vec { @@ -376,6 +411,10 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } + pub(super) fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + return self.foreign_call_executor.get_variables(); + } + fn breakpoint_reached(&self) -> bool { if let Some(location) = self.get_current_opcode_location() { self.breakpoints.contains(&location) @@ -436,6 +475,7 @@ mod tests { use super::*; use crate::context::{DebugCommandResult, DebugContext}; + use crate::foreign_calls::DefaultDebugForeignCallExecutor; use acvm::{ acir::{ circuit::{ @@ -449,7 +489,7 @@ mod tests { BinaryFieldOp, Opcode as BrilligOpcode, RegisterIndex, RegisterOrMemory, }, }; - use nargo::{artifacts::debug::DebugArtifact, ops::DefaultForeignCallExecutor}; + use nargo::artifacts::debug::DebugArtifact; use std::collections::BTreeMap; #[test] @@ -489,12 +529,14 @@ mod tests { let initial_witness = BTreeMap::from([(Witness(1), fe_1)]).into(); + let foreign_call_executor = + Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, &debug_artifact)); let mut context = DebugContext::new( &StubbedBlackBoxSolver, circuit, debug_artifact, initial_witness, - Box::new(DefaultForeignCallExecutor::new(true, None)), + foreign_call_executor, ); assert_eq!(context.get_current_opcode_location(), Some(OpcodeLocation::Acir(0))); @@ -581,12 +623,14 @@ mod tests { let initial_witness = BTreeMap::from([(Witness(1), fe_1), (Witness(2), fe_1)]).into(); + let foreign_call_executor = + Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, &debug_artifact)); let mut context = DebugContext::new( &StubbedBlackBoxSolver, circuit, debug_artifact, initial_witness, - Box::new(DefaultForeignCallExecutor::new(true, None)), + foreign_call_executor, ); // set breakpoint @@ -635,7 +679,7 @@ mod tests { &circuit, &debug_artifact, WitnessMap::new(), - Box::new(DefaultForeignCallExecutor::new(true, None)), + Box::new(DefaultDebugForeignCallExecutor::new(true)), ); assert_eq!(context.offset_opcode_location(&None, 0), (None, 0)); diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 803f9f108d..2de7dac77d 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -9,6 +9,7 @@ use codespan_reporting::files::{Files, SimpleFile}; use crate::context::DebugCommandResult; use crate::context::DebugContext; +use crate::foreign_calls::DefaultDebugForeignCallExecutor; use dap::errors::ServerError; use dap::events::StoppedEventBody; @@ -25,7 +26,6 @@ use dap::types::{ StoppedEventReason, Thread, }; use nargo::artifacts::debug::DebugArtifact; -use nargo::ops::DefaultForeignCallExecutor; use fm::FileId; use noirc_driver::CompiledProgram; @@ -57,7 +57,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { circuit, debug_artifact, initial_witness, - Box::new(DefaultForeignCallExecutor::new(true, None)), + Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)), ); Self { server, diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs new file mode 100644 index 0000000000..01676adfef --- /dev/null +++ b/tooling/debugger/src/foreign_calls.rs @@ -0,0 +1,138 @@ +use acvm::{ + acir::brillig::{ForeignCallParam, ForeignCallResult, Value}, + pwg::ForeignCallWaitInfo, +}; +use nargo::{ + artifacts::debug::{DebugArtifact, DebugVars}, + ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, +}; +use noirc_errors::debug_info::DebugVarId; +use noirc_printable_type::{ForeignCallError, PrintableType, PrintableValue}; + +pub(crate) enum DebugForeignCall { + VarAssign, + VarDrop, + MemberAssign(u32), + DerefAssign, +} + +impl DebugForeignCall { + pub(crate) fn lookup(op_name: &str) -> Option { + let member_pre = "__debug_member_assign_"; + if let Some(op_suffix) = op_name.strip_prefix(member_pre) { + let arity = + op_suffix.parse::().expect("failed to parse debug_member_assign arity"); + return Some(DebugForeignCall::MemberAssign(arity)); + } + match op_name { + "__debug_var_assign" => Some(DebugForeignCall::VarAssign), + "__debug_var_drop" => Some(DebugForeignCall::VarDrop), + "__debug_deref_assign" => Some(DebugForeignCall::DerefAssign), + _ => None, + } + } +} + +pub trait DebugForeignCallExecutor: ForeignCallExecutor { + fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)>; +} + +pub struct DefaultDebugForeignCallExecutor { + executor: DefaultForeignCallExecutor, + pub debug_vars: DebugVars, +} + +impl DefaultDebugForeignCallExecutor { + pub fn new(show_output: bool) -> Self { + Self { + executor: DefaultForeignCallExecutor::new(show_output, None), + debug_vars: DebugVars::default(), + } + } + + pub fn from_artifact(show_output: bool, artifact: &DebugArtifact) -> Self { + let mut ex = Self::new(show_output); + ex.load_artifact(artifact); + ex + } + + pub fn load_artifact(&mut self, artifact: &DebugArtifact) { + artifact.debug_symbols.iter().for_each(|info| { + self.debug_vars.insert_variables(&info.variables); + self.debug_vars.insert_types(&info.types); + }); + } +} + +impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { + fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + self.debug_vars.get_variables() + } +} + +fn debug_var_id(value: &Value) -> DebugVarId { + DebugVarId(value.to_u128() as u32) +} + +impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result { + let foreign_call_name = foreign_call.function.as_str(); + match DebugForeignCall::lookup(foreign_call_name) { + Some(DebugForeignCall::VarAssign) => { + let fcp_var_id = &foreign_call.inputs[0]; + if let ForeignCallParam::Single(var_id_value) = fcp_var_id { + let var_id = debug_var_id(var_id_value); + let values: Vec = + foreign_call.inputs[1..].iter().flat_map(|x| x.values()).collect(); + self.debug_vars.assign_var(var_id, &values); + } + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::VarDrop) => { + let fcp_var_id = &foreign_call.inputs[0]; + if let ForeignCallParam::Single(var_id_value) = fcp_var_id { + let var_id = debug_var_id(var_id_value); + self.debug_vars.drop_var(var_id); + } + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::MemberAssign(arity)) => { + if let Some(ForeignCallParam::Single(var_id_value)) = foreign_call.inputs.get(0) { + let arity = arity as usize; + let var_id = debug_var_id(var_id_value); + let n = foreign_call.inputs.len(); + let indexes: Vec = foreign_call.inputs[(n - arity)..n] + .iter() + .map(|fcp_v| { + if let ForeignCallParam::Single(v) = fcp_v { + v.to_u128() as u32 + } else { + panic!("expected ForeignCallParam::Single(v)"); + } + }) + .collect(); + let values: Vec = (0..n - 1 - arity) + .flat_map(|i| { + foreign_call.inputs.get(1 + i).map(|fci| fci.values()).unwrap_or(vec![]) + }) + .collect(); + self.debug_vars.assign_field(var_id, indexes, &values); + } + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::DerefAssign) => { + let fcp_var_id = &foreign_call.inputs[0]; + let fcp_value = &foreign_call.inputs[1]; + if let ForeignCallParam::Single(var_id_value) = fcp_var_id { + let var_id = debug_var_id(var_id_value); + self.debug_vars.assign_deref(var_id, &fcp_value.values()); + } + Ok(ForeignCallResult::default().into()) + } + None => self.executor.execute(foreign_call), + } + } +} diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 21834e44f9..35014f9a8c 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -1,5 +1,6 @@ mod context; mod dap; +mod foreign_calls; mod repl; mod source_code_printer; diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 40ee6efdb8..1ba5391b0c 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -4,9 +4,11 @@ use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation}; use acvm::acir::native_types::{Witness, WitnessMap}; use acvm::{BlackBoxFunctionSolver, FieldElement}; -use nargo::{artifacts::debug::DebugArtifact, ops::DefaultForeignCallExecutor, NargoError}; +use crate::foreign_calls::DefaultDebugForeignCallExecutor; +use nargo::{artifacts::debug::DebugArtifact, NargoError}; use easy_repl::{command, CommandStatus, Repl}; +use noirc_printable_type::PrintableValueDisplay; use std::cell::RefCell; use crate::source_code_printer::print_source_code_location; @@ -27,12 +29,14 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, ) -> Self { + let foreign_call_executor = + Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); let context = DebugContext::new( blackbox_solver, circuit, debug_artifact, initial_witness.clone(), - Box::new(DefaultForeignCallExecutor::new(true, None)), + foreign_call_executor, ); Self { context, @@ -73,8 +77,8 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { ); } } - - print_source_code_location(self.debug_artifact, &location); + let locations = self.context.get_source_location_for_opcode_location(&location); + print_source_code_location(self.debug_artifact, &locations); } } } @@ -211,12 +215,14 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { fn restart_session(&mut self) { let breakpoints: Vec = self.context.iterate_breakpoints().copied().collect(); + let foreign_call_executor = + Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, self.debug_artifact)); self.context = DebugContext::new( self.blackbox_solver, self.circuit, self.debug_artifact, self.initial_witness.clone(), - Box::new(DefaultForeignCallExecutor::new(true, None)), + foreign_call_executor, ); for opcode_location in breakpoints { self.context.add_breakpoint(opcode_location); @@ -313,6 +319,15 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.context.write_brillig_memory(index, field_value); } + pub fn show_vars(&self) { + let vars = self.context.get_variables(); + for (var_name, value, var_type) in vars.iter() { + let printable_value = + PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); + println!("{var_name}:{var_type:?} = {}", printable_value); + } + } + fn is_solved(&self) -> bool { self.context.is_solved() } @@ -485,6 +500,16 @@ pub fn run( } }, ) + .add( + "vars", + command! { + "show variable values available at this point in execution", + () => || { + ref_context.borrow_mut().show_vars(); + Ok(CommandStatus::Done) + } + }, + ) .build() .expect("Failed to initialize debugger repl"); diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index 1707f9066b..b5ffdb12d0 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -1,4 +1,3 @@ -use acvm::acir::circuit::OpcodeLocation; use codespan_reporting::files::Files; use nargo::artifacts::debug::DebugArtifact; use noirc_errors::Location; @@ -31,13 +30,7 @@ struct LocationPrintContext { // Given a DebugArtifact and an OpcodeLocation, prints all the source code // locations the OpcodeLocation maps to, with some surrounding context and // visual aids to highlight the location itself. -pub(crate) fn print_source_code_location( - debug_artifact: &DebugArtifact, - location: &OpcodeLocation, -) { - let locations = debug_artifact.debug_symbols[0].opcode_location(location); - let Some(locations) = locations else { return; }; - +pub(crate) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { let locations = locations.iter(); for loc in locations { @@ -276,7 +269,8 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = vec![DebugInfo::new(opcode_locations)]; + let debug_symbols = + vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_rendered: Vec<_> = render_location(&debug_artifact, &loc).collect(); diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index e8b17b8a7a..82872ce273 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -27,7 +27,7 @@ mod tests { // Start debugger and test that it loads for the given program. dbg_session .execute( - &format!("{} debug --program-dir {}", nargo_bin, test_program_dir), + &format!("{} debug --program-dir {} --force-brillig", nargo_bin, test_program_dir), ".*\\Starting debugger.*", ) .expect("Could not start debugger"); diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 135090d7ed..0b88d81426 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -83,7 +83,7 @@ fn on_test_run_request_inner( let test_result = run_test( &state.solver, - &context, + &mut context, test_function, false, None, diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 2e2d98f279..a249ecb03a 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -8,6 +8,7 @@ use std::{ ops::Range, }; +pub use super::debug_vars::DebugVars; use fm::{FileId, FileManager, PathString}; /// A Debug Artifact stores, for a given program, the debug info for every function @@ -86,7 +87,8 @@ impl DebugArtifact { let line_index = self.line_index(location.file, location_start)?; let line_span = self.line_range(location.file, line_index)?; - let line_length = line_span.end - (line_span.start + 1); + let line_length = + if line_span.end > line_span.start { line_span.end - (line_span.start + 1) } else { 0 }; let start_in_line = location_start - line_span.start; // The location might continue beyond the line, @@ -229,7 +231,8 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = vec![DebugInfo::new(opcode_locations)]; + let debug_symbols = + vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; let debug_artifact = DebugArtifact::new(debug_symbols, &fm); let location_in_line = debug_artifact.location_in_line(loc).expect("Expected a range"); diff --git a/tooling/nargo/src/artifacts/debug_vars.rs b/tooling/nargo/src/artifacts/debug_vars.rs new file mode 100644 index 0000000000..b5559ca53c --- /dev/null +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -0,0 +1,117 @@ +use acvm::brillig_vm::brillig::Value; +use noirc_errors::debug_info::{ + DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, +}; +use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; +use std::collections::{HashMap, HashSet}; + +#[derive(Debug, Default, Clone)] +pub struct DebugVars { + variables: HashMap, + types: HashMap, + active: HashSet, + values: HashMap, +} + +impl DebugVars { + pub fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + self.active + .iter() + .filter_map(|var_id| { + self.variables + .get(var_id) + .and_then(|debug_var| { + let Some(value) = self.values.get(var_id) else { return None; }; + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { return None; }; + Some((debug_var.name.as_str(), value, ptype)) + }) + }) + .collect() + } + + pub fn insert_variables(&mut self, vars: &DebugVariables) { + self.variables.extend(vars.clone().into_iter()); + } + + pub fn insert_types(&mut self, types: &DebugTypes) { + self.types.extend(types.clone().into_iter()); + } + + pub fn assign_var(&mut self, var_id: DebugVarId, values: &[Value]) { + self.active.insert(var_id); + let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; + let ptype = self.types.get(type_id).unwrap(); + self.values.insert(var_id, decode_value(&mut values.iter().map(|v| v.to_field()), ptype)); + } + + pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[Value]) { + let mut cursor: &mut PrintableValue = self + .values + .get_mut(&var_id) + .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); + let cursor_type_id = &self + .variables + .get(&var_id) + .unwrap_or_else(|| panic!("variable {var_id:?} not found")) + .debug_type_id; + let mut cursor_type = self + .types + .get(cursor_type_id) + .unwrap_or_else(|| panic!("type unavailable for type id {cursor_type_id:?}")); + for index in indexes.iter() { + (cursor, cursor_type) = match (cursor, cursor_type) { + (PrintableValue::Vec(array), PrintableType::Array { length, typ }) => { + if let Some(len) = length { + if *index as u64 >= *len { + panic!("unexpected field index past array length") + } + if *len != array.len() as u64 { + panic!("type/array length mismatch") + } + } + (array.get_mut(*index as usize).unwrap(), &*Box::leak(typ.clone())) + } + ( + PrintableValue::Struct(field_map), + PrintableType::Struct { name: _name, fields }, + ) => { + if *index as usize >= fields.len() { + panic!("unexpected field index past struct field length") + } + let (key, typ) = fields.get(*index as usize).unwrap(); + (field_map.get_mut(key).unwrap(), typ) + } + (PrintableValue::Vec(array), PrintableType::Tuple { types }) => { + if *index >= types.len() as u32 { + panic!( + "unexpected field index ({index}) past tuple length ({})", + types.len() + ); + } + if types.len() != array.len() { + panic!("type/array length mismatch") + } + let typ = types.get(*index as usize).unwrap(); + (array.get_mut(*index as usize).unwrap(), typ) + } + _ => { + panic!("unexpected assign field of {cursor_type:?} type"); + } + }; + } + *cursor = decode_value(&mut values.iter().map(|v| v.to_field()), cursor_type); + self.active.insert(var_id); + } + + pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[Value]) { + unimplemented![] + } + + pub fn get_type(&self, var_id: DebugVarId) -> Option<&PrintableType> { + self.variables.get(&var_id).and_then(|debug_var| self.types.get(&debug_var.debug_type_id)) + } + + pub fn drop_var(&mut self, var_id: DebugVarId) { + self.active.remove(&var_id); + } +} diff --git a/tooling/nargo/src/artifacts/mod.rs b/tooling/nargo/src/artifacts/mod.rs index 180a900fd8..c7b3736f90 100644 --- a/tooling/nargo/src/artifacts/mod.rs +++ b/tooling/nargo/src/artifacts/mod.rs @@ -5,4 +5,5 @@ //! to generate them using these artifacts as a starting point. pub mod contract; pub mod debug; +mod debug_vars; pub mod program; diff --git a/tooling/nargo/src/ops/compile.rs b/tooling/nargo/src/ops/compile.rs index dccd2cedbf..bd1850649c 100644 --- a/tooling/nargo/src/ops/compile.rs +++ b/tooling/nargo/src/ops/compile.rs @@ -1,5 +1,8 @@ use fm::FileManager; -use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, CompiledProgram}; +use noirc_driver::{ + link_to_debug_crate, CompilationResult, CompileOptions, CompiledContract, CompiledProgram, +}; +use noirc_frontend::debug::DebugInstrumenter; use noirc_frontend::hir::ParsedFiles; use crate::errors::CompileError; @@ -68,8 +71,29 @@ pub fn compile_program( package: &Package, compile_options: &CompileOptions, cached_program: Option, +) -> CompilationResult { + compile_program_with_debug_instrumenter( + file_manager, + parsed_files, + package, + compile_options, + cached_program, + DebugInstrumenter::default(), + ) +} + +pub fn compile_program_with_debug_instrumenter( + file_manager: &FileManager, + parsed_files: &ParsedFiles, + package: &Package, + compile_options: &CompileOptions, + cached_program: Option, + debug_instrumenter: DebugInstrumenter, ) -> CompilationResult { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); + link_to_debug_crate(&mut context, crate_id); + context.debug_instrumenter = debug_instrumenter; + noirc_driver::compile_main(&mut context, crate_id, compile_options, cached_program) } diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index f600b3047f..0f31be3c7f 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -69,7 +69,7 @@ impl From> for NargoForeignCallResult { /// This enumeration represents the Brillig foreign calls that are natively supported by nargo. /// After resolution of a foreign call, nargo will restart execution of the ACVM -pub(crate) enum ForeignCall { +pub enum ForeignCall { Print, AssertMessage, CreateMock, diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index 4f92faa73a..23dd0db15b 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -1,6 +1,10 @@ -pub use self::compile::{compile_contract, compile_program, compile_workspace}; +pub use self::compile::{ + compile_contract, compile_program, compile_program_with_debug_instrumenter, compile_workspace, +}; pub use self::execute::execute_circuit; -pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor}; +pub use self::foreign_calls::{ + DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor, NargoForeignCallResult, +}; pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index f38dcad0c2..0929739a6a 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -16,7 +16,7 @@ pub enum TestStatus { pub fn run_test( blackbox_solver: &B, - context: &Context, + context: &mut Context, test_function: TestFunction, show_output: bool, foreign_call_resolver_url: Option<&str>, diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 67322b1873..d5eb3325ba 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -3,7 +3,7 @@ use acvm::ExpressionWidth; use backend_interface::Backend; use clap::Args; use nargo::constants::PROVER_INPUT_FILE; -use nargo::ops::compile_program; +use nargo::ops::compile_program_with_debug_instrumenter; use nargo::workspace::Workspace; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; @@ -24,6 +24,7 @@ use dap::types::Capabilities; use serde_json::Value; use super::compile_cmd::report_errors; +use super::debug_cmd::instrument_package_files; use super::fs::inputs::read_inputs_from_file; use crate::errors::CliError; @@ -84,11 +85,21 @@ fn load_and_compile_project( let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let mut parsed_files = parse_all(&workspace_file_manager); - let compile_options = CompileOptions::default(); - let compilation_result = - compile_program(&workspace_file_manager, &parsed_files, package, &compile_options, None); + let compile_options = + CompileOptions { instrument_debug: true, force_brillig: true, ..CompileOptions::default() }; + + let debug_state = instrument_package_files(&mut parsed_files, &workspace_file_manager, package); + + let compilation_result = compile_program_with_debug_instrumenter( + &workspace_file_manager, + &parsed_files, + package, + &compile_options, + None, + debug_state, + ); let compiled_program = report_errors( compilation_result, diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index a0bac3bdac..b3ee913753 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -4,9 +4,10 @@ use acvm::acir::native_types::WitnessMap; use bn254_blackbox_solver::Bn254BlackBoxSolver; use clap::Args; +use fm::FileManager; use nargo::artifacts::debug::DebugArtifact; use nargo::constants::PROVER_INPUT_FILE; -use nargo::ops::compile_program; +use nargo::ops::compile_program_with_debug_instrumenter; use nargo::package::Package; use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; @@ -15,7 +16,9 @@ use noirc_abi::InputMap; use noirc_driver::{ file_manager_with_stdlib, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING, }; +use noirc_frontend::debug::DebugInstrumenter; use noirc_frontend::graph::CrateName; +use noirc_frontend::hir::ParsedFiles; use super::compile_cmd::report_errors; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; @@ -46,6 +49,10 @@ pub(crate) fn run( args: DebugCommand, config: NargoConfig, ) -> Result<(), CliError> { + // Override clap default for compiler option flag + let mut args = args.clone(); + args.compile_options.instrument_debug = true; + let toml_path = get_package_manifest(&config.program_dir)?; let selection = args.package.map_or(PackageSelection::DefaultOrAll, PackageSelection::Selected); let workspace = resolve_workspace_from_toml( @@ -61,7 +68,7 @@ pub(crate) fn run( let mut workspace_file_manager = file_manager_with_stdlib(std::path::Path::new("")); insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); - let parsed_files = parse_all(&workspace_file_manager); + let mut parsed_files = parse_all(&workspace_file_manager); let Some(package) = workspace.into_iter().find(|p| p.is_binary()) else { println!( @@ -70,12 +77,16 @@ pub(crate) fn run( return Ok(()); }; - let compilation_result = compile_program( + let debug_instrumenter = + instrument_package_files(&mut parsed_files, &workspace_file_manager, package); + + let compilation_result = compile_program_with_debug_instrumenter( &workspace_file_manager, &parsed_files, package, &args.compile_options, None, + debug_instrumenter, ); let compiled_program = report_errors( @@ -90,6 +101,36 @@ pub(crate) fn run( run_async(package, compiled_program, &args.prover_name, &args.witness_name, target_dir) } +/// Add debugging instrumentation to all parsed files belonging to the package +/// being compiled +pub(crate) fn instrument_package_files( + parsed_files: &mut ParsedFiles, + file_manager: &FileManager, + package: &Package, +) -> DebugInstrumenter { + // Start off at the entry path and read all files in the parent directory. + let entry_path_parent = package + .entry_path + .parent() + .unwrap_or_else(|| panic!("The entry path is expected to be a single file within a directory and so should have a parent {:?}", package.entry_path)) + .clone(); + + let mut debug_instrumenter = DebugInstrumenter::default(); + + for (file_id, parsed_file) in parsed_files.iter_mut() { + let file_path = + file_manager.path(*file_id).expect("Parsed file ID not found in file manager"); + for ancestor in file_path.ancestors() { + if ancestor == entry_path_parent { + // file is in package + debug_instrumenter.instrument_module(&mut parsed_file.0); + } + } + } + + debug_instrumenter +} + fn run_async( package: &Package, program: CompiledProgram, diff --git a/tooling/nargo_cli/src/cli/export_cmd.rs b/tooling/nargo_cli/src/cli/export_cmd.rs index feaa55857e..96b24796a2 100644 --- a/tooling/nargo_cli/src/cli/export_cmd.rs +++ b/tooling/nargo_cli/src/cli/export_cmd.rs @@ -102,7 +102,7 @@ fn compile_exported_functions( exported_functions, |(function_name, function_id)| -> Result<(String, CompiledProgram), CompileError> { // TODO: We should to refactor how to deal with compilation errors to avoid this. - let program = compile_no_check(&context, compile_options, function_id, None, false) + let program = compile_no_check(&mut context, compile_options, function_id, None, false) .map_err(|error| vec![FileDiagnostic::from(error)]); let program = report_errors( diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 9fee27b917..503fd5afdd 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -160,7 +160,7 @@ fn run_tests( let test_status = run_test( blackbox_solver, - &context, + &mut context, test_function, show_output, foreign_call_resolver_url,