diff --git a/compiler/noirc_errors/src/debug_info.rs b/compiler/noirc_errors/src/debug_info.rs index 67ec851d46d..09117bdc3b7 100644 --- a/compiler/noirc_errors/src/debug_info.rs +++ b/compiler/noirc_errors/src/debug_info.rs @@ -24,6 +24,9 @@ use serde::{ #[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 DebugFnId(pub u32); + #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)] pub struct DebugTypeId(pub u32); @@ -33,7 +36,14 @@ pub struct DebugVariable { pub debug_type_id: DebugTypeId, } +#[derive(Debug, Clone, Hash, Deserialize, Serialize)] +pub struct DebugFunction { + pub name: String, + pub arg_names: Vec, +} + pub type DebugVariables = BTreeMap; +pub type DebugFunctions = BTreeMap; pub type DebugTypes = BTreeMap; #[serde_as] @@ -45,6 +55,7 @@ pub struct DebugInfo { #[serde_as(as = "BTreeMap")] pub locations: BTreeMap>, pub variables: DebugVariables, + pub functions: DebugFunctions, pub types: DebugTypes, } @@ -60,9 +71,10 @@ impl DebugInfo { pub fn new( locations: BTreeMap>, variables: DebugVariables, + functions: DebugFunctions, types: DebugTypes, ) -> Self { - Self { locations, variables, types } + Self { locations, variables, functions, types } } /// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified. diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 0bb81efe977..56cb76adbe4 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -88,6 +88,7 @@ pub fn create_circuit( ) -> Result<(Circuit, DebugInfo, Vec, Vec, Vec), RuntimeError> { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); + let debug_functions = program.debug_functions.clone(); let func_sig = program.main_function_signature.clone(); let recursive = program.recursive; let mut generated_acir = optimize_into_acir( @@ -130,7 +131,7 @@ pub fn create_circuit( .map(|(index, locations)| (index, locations.into_iter().collect())) .collect(); - let mut debug_info = DebugInfo::new(locations, debug_variables, debug_types); + let mut debug_info = DebugInfo::new(locations, debug_variables, debug_functions, debug_types); // Perform any ACIR-level optimizations let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index a88567fcaf9..05916502d73 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -4,9 +4,11 @@ use crate::{ ast::{Path, PathKind}, parser::{Item, ItemKind}, }; +use noirc_errors::debug_info::{DebugFnId, DebugFunction}; use noirc_errors::{Span, Spanned}; use std::collections::HashMap; use std::collections::VecDeque; +use std::mem::take; const MAX_MEMBER_ASSIGN_DEPTH: usize = 8; @@ -26,8 +28,12 @@ pub struct DebugInstrumenter { // all field names referenced when assigning to a member of a variable pub field_names: HashMap, + // all collected function metadata (name + argument names) + pub functions: HashMap, + next_var_id: u32, next_field_name_id: u32, + next_fn_id: u32, // last seen variable names and their IDs grouped by scope scope: Vec>, @@ -38,9 +44,11 @@ impl Default for DebugInstrumenter { Self { variables: HashMap::default(), field_names: HashMap::default(), + functions: HashMap::default(), scope: vec![], next_var_id: 0, next_field_name_id: 1, + next_fn_id: 0, } } } @@ -76,10 +84,22 @@ impl DebugInstrumenter { field_name_id } + fn insert_function(&mut self, fn_name: String, arguments: Vec) -> DebugFnId { + let fn_id = DebugFnId(self.next_fn_id); + self.next_fn_id += 1; + self.functions.insert(fn_id, DebugFunction { name: fn_name, arg_names: arguments }); + fn_id + } + fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) { + let func_name = func.name.0.contents.clone(); + let func_args = + func.parameters.iter().map(|param| pattern_to_string(¶m.pattern)).collect(); + let fn_id = self.insert_function(func_name, func_args); + let enter_stmt = build_debug_call_stmt("enter", fn_id, func.span); self.scope.push(HashMap::default()); - let set_fn_params = func + let set_fn_params: Vec<_> = func .parameters .iter() .flat_map(|param| { @@ -93,10 +113,21 @@ impl DebugInstrumenter { }) .collect(); - self.walk_scope(&mut func.body.0, func.span); + let func_body = &mut func.body.0; + let mut statements = take(func_body); + + self.walk_scope(&mut statements, func.span); - // prepend fn params: - func.body.0 = [set_fn_params, func.body.0.clone()].concat(); + // walk_scope ensures that the last statement is the return value of the function + let last_stmt = statements.pop().expect("at least one statement after walk_scope"); + let exit_stmt = build_debug_call_stmt("exit", fn_id, last_stmt.span); + + // rebuild function body + func_body.push(enter_stmt); + func_body.extend(set_fn_params); + func_body.extend(statements); + func_body.push(exit_stmt); + func_body.push(last_stmt); } // Modify a vector of statements in-place, adding instrumentation for sets and drops. @@ -427,6 +458,8 @@ impl DebugInstrumenter { use dep::__debug::{{ __debug_var_assign, __debug_var_drop, + __debug_fn_enter, + __debug_fn_exit, __debug_dereference_assign, {member_assigns}, }};"# @@ -451,14 +484,32 @@ pub fn build_debug_crate_file() -> String { } #[oracle(__debug_var_drop)] - unconstrained fn __debug_var_drop_oracle(_var_id: u32) {} - unconstrained fn __debug_var_drop_inner(var_id: u32) { + 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) { + pub fn __debug_var_drop(var_id: u32) { __debug_var_drop_inner(var_id); } + #[oracle(__debug_fn_enter)] + unconstrained fn __debug_fn_enter_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_enter_inner(fn_id: u32) { + __debug_fn_enter_oracle(fn_id); + } + pub fn __debug_fn_enter(fn_id: u32) { + __debug_fn_enter_inner(fn_id); + } + + #[oracle(__debug_fn_exit)] + unconstrained fn __debug_fn_exit_oracle(_fn_id: u32) {} + unconstrained fn __debug_fn_exit_inner(fn_id: u32) { + __debug_fn_exit_oracle(fn_id); + } + pub fn __debug_fn_exit(fn_id: u32) { + __debug_fn_exit_inner(fn_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) { @@ -561,6 +612,21 @@ fn build_assign_member_stmt( ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span } } +fn build_debug_call_stmt(fname: &str, fn_id: DebugFnId, 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(&format!["__debug_fn_{fname}"], span)], + kind: PathKind::Plain, + span, + }), + span, + }), + arguments: vec![uint_expr(fn_id.0 as u128, span)], + })); + 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)]); @@ -585,6 +651,30 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> { vars } +fn pattern_to_string(pattern: &ast::Pattern) -> String { + match pattern { + ast::Pattern::Identifier(id) => id.0.contents.clone(), + ast::Pattern::Mutable(mpat, _, _) => format!("mut {}", pattern_to_string(mpat.as_ref())), + ast::Pattern::Tuple(elements, _) => format!( + "({})", + elements.iter().map(pattern_to_string).collect::>().join(", ") + ), + ast::Pattern::Struct(name, fields, _) => { + format!( + "{} {{ {} }}", + name, + fields + .iter() + .map(|(field_ident, field_pattern)| { + format!("{}: {}", &field_ident.0.contents, pattern_to_string(field_pattern)) + }) + .collect::>() + .join(", "), + ) + } + } +} + fn ident(s: &str, span: Span) -> ast::Ident { ast::Ident(Spanned::from(span, s.to_string())) } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index b70aa43701c..13fa41733cf 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1768,9 +1768,11 @@ impl From<&Type> for PrintableType { Type::TypeVariable(_, _) => unreachable!(), Type::NamedGeneric(..) => unreachable!(), Type::Forall(..) => unreachable!(), - Type::Function(_, _, env) => { - PrintableType::Function { env: Box::new(env.as_ref().into()) } - } + Type::Function(arguments, return_type, env) => PrintableType::Function { + arguments: arguments.iter().map(|arg| arg.into()).collect(), + return_type: Box::new(return_type.as_ref().into()), + env: Box::new(env.as_ref().into()), + }, Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index e4e619d5d92..7fcf8e87792 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -1,7 +1,7 @@ use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{ - debug_info::{DebugTypes, DebugVariables}, + debug_info::{DebugFunctions, DebugTypes, DebugVariables}, Location, }; @@ -253,6 +253,7 @@ pub struct Program { /// Indicates to a backend whether a SNARK-friendly prover should be used. pub recursive: bool, pub debug_variables: DebugVariables, + pub debug_functions: DebugFunctions, pub debug_types: DebugTypes, } @@ -266,6 +267,7 @@ impl Program { return_visibility: Visibility, recursive: bool, debug_variables: DebugVariables, + debug_functions: DebugFunctions, debug_types: DebugTypes, ) -> Program { Program { @@ -276,6 +278,7 @@ impl Program { return_visibility, recursive, debug_variables, + debug_functions, debug_types, } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index a8ff4399f99..cf4e0ab792e 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -76,7 +76,7 @@ impl<'interner> Monomorphizer<'interner> { 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 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)?; Ok(()) diff --git a/compiler/noirc_frontend/src/monomorphization/debug_types.rs b/compiler/noirc_frontend/src/monomorphization/debug_types.rs index fea073d394f..16b82d1e7b9 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug_types.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug_types.rs @@ -3,7 +3,8 @@ use crate::{ hir_def::types::Type, }; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugFunctions, DebugTypeId, DebugTypes, DebugVarId, DebugVariable, + DebugVariables, }; use noirc_printable_type::PrintableType; use std::collections::HashMap; @@ -30,7 +31,10 @@ pub struct DebugTypeTracker { // All instances of tracked variables variables: HashMap, - // Types of tracked variables + // Function metadata collected during instrumentation injection + functions: HashMap, + + // Types of tracked variables and functions types: HashMap, types_reverse: HashMap, @@ -43,34 +47,29 @@ impl DebugTypeTracker { DebugTypeTracker { source_variables: instrumenter.variables.clone(), source_field_names: instrumenter.field_names.clone(), + functions: instrumenter.functions.clone(), ..DebugTypeTracker::default() } } - pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugTypes) { + pub fn extract_vars_and_types(&self) -> (DebugVariables, DebugFunctions, 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, - }, - ) + let var_name = + self.source_variables.get(&source_var_id).cloned().unwrap_or_else(|| { + unreachable!("failed to retrieve variable name for {source_var_id:?}"); + }); + (var_id, DebugVariable { name: var_name, debug_type_id: type_id }) }) .collect(); + + let debug_functions = self.functions.clone().into_iter().collect(); let debug_types = self.types.clone().into_iter().collect(); - (debug_variables, debug_types) + (debug_variables, debug_functions, debug_types) } pub fn resolve_field_index( @@ -83,19 +82,24 @@ impl DebugTypeTracker { .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(|| { + fn insert_type(&mut self, the_type: &Type) -> DebugTypeId { + let ptype: PrintableType = the_type.follow_bindings().into(); + 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 - }); + }) + } + + 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 type_id = self.insert_type(var_type); + // 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(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ce880401d77..cfd9a61d13f 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -165,7 +165,8 @@ pub fn monomorphize_debug( let FuncMeta { return_distinctness, return_visibility, kind, .. } = monomorphizer.interner.function_meta(&main); - let (debug_variables, debug_types) = monomorphizer.debug_type_tracker.extract_vars_and_types(); + let (debug_variables, debug_functions, debug_types) = + monomorphizer.debug_type_tracker.extract_vars_and_types(); let program = Program::new( functions, function_sig, @@ -174,6 +175,7 @@ pub fn monomorphize_debug( *return_visibility, *kind == FunctionKind::Recursive, debug_variables, + debug_functions, debug_types, ); Ok(program) diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 24f4f275a14..60f233cd86d 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -33,6 +33,8 @@ pub enum PrintableType { length: u64, }, Function { + arguments: Vec, + return_type: Box, env: Box, }, MutableReference { @@ -176,8 +178,8 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { output.push_str("false"); } } - (PrintableValue::Field(_), PrintableType::Function { .. }) => { - output.push_str("<>"); + (PrintableValue::Field(_), PrintableType::Function { arguments, return_type, .. }) => { + output.push_str(&format!("< {:?}>>", arguments, return_type,)); } (_, PrintableType::MutableReference { .. }) => { output.push_str("<>"); @@ -350,7 +352,7 @@ pub fn decode_value( PrintableValue::Struct(struct_map) } - PrintableType::Function { env } => { + PrintableType::Function { env, .. } => { let field_element = field_iterator.next().unwrap(); let func_ref = PrintableValue::Field(field_element); // we want to consume the fields from the environment, but for now they are not actually printed diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt index c472e828739..231d4d897a9 100644 --- a/tooling/debugger/ignored-tests.txt +++ b/tooling/debugger/ignored-tests.txt @@ -1,21 +1,13 @@ array_dynamic_blackbox_input -array_sort -assign_ex +bigint bit_shifts_comptime -brillig_cow -brillig_nested_arrays brillig_references brillig_to_bytes_integration debug_logs double_verify_nested_proof 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 -bigint diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 515edf0bb06..a3ee89263a4 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -8,11 +8,14 @@ use acvm::pwg::{ }; use acvm::{BlackBoxFunctionSolver, FieldElement}; -use nargo::artifacts::debug::DebugArtifact; +use codespan_reporting::files::{Files, SimpleFile}; +use fm::FileId; +use nargo::artifacts::debug::{DebugArtifact, StackFrame}; use nargo::errors::{ExecutionError, Location}; use nargo::NargoError; -use noirc_printable_type::{PrintableType, PrintableValue}; +use noirc_driver::DebugFile; +use std::collections::BTreeMap; use std::collections::{hash_set::Iter, HashSet}; #[derive(Debug)] @@ -29,6 +32,7 @@ pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { foreign_call_executor: Box, debug_artifact: &'a DebugArtifact, breakpoints: HashSet, + source_to_opcodes: BTreeMap>, } impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { @@ -39,12 +43,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness: WitnessMap, foreign_call_executor: Box, ) -> Self { + let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); Self { acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), brillig_solver: None, foreign_call_executor, debug_artifact, breakpoints: HashSet::new(), + source_to_opcodes, } } @@ -100,10 +106,50 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.debug_artifact .file_map .get(&location.file) - .map(|file| file.path.starts_with("__debug/")) + .map(is_debug_file_in_debug_crate) .unwrap_or(false) } + /// Find an opcode location matching a source code location + // We apply some heuristics here, and there are four possibilities for the + // return value of this function: + // 1. the source location is not found -> None + // 2. an exact unique location is found (very rare) -> Some(opcode_location) + // 3. an exact but not unique location is found, ie. a source location may + // be mapped to multiple opcodes, and those may be disjoint, for example for + // functions called multiple times throughout the program + // -> return the first opcode in program order that matches the source location + // 4. exact location is not found, so an opcode for a nearby source location + // is returned (this again could actually be more than one opcodes) + // -> return the opcode for the next source line that is mapped + pub(super) fn find_opcode_for_source_location( + &self, + file_id: &FileId, + line: i64, + ) -> Option { + let line = line as usize; + let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { + return None; + }; + let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { + Ok(index) => { + // move backwards to find the first opcode which matches the line + let mut index = index; + while index > 0 && line_to_opcodes[index - 1].0 == line { + index -= 1; + } + line_to_opcodes[index].1 + } + Err(index) => { + if index >= line_to_opcodes.len() { + return None; + } + line_to_opcodes[index].1 + } + }; + Some(found_index) + } + /// 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 @@ -128,6 +174,9 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { &self, opcode_location: &OpcodeLocation, ) -> Vec { + // TODO: this assumes we're debugging a program (ie. the DebugArtifact + // will contain a single DebugInfo), but this assumption doesn't hold + // for contracts self.debug_artifact.debug_symbols[0] .opcode_location(opcode_location) .map(|source_locations| { @@ -467,10 +516,14 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } - pub(super) fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + pub(super) fn get_variables(&self) -> Vec { return self.foreign_call_executor.get_variables(); } + pub(super) fn current_stack_frame(&self) -> Option { + return self.foreign_call_executor.current_stack_frame(); + } + fn breakpoint_reached(&self) -> bool { if let Some(location) = self.get_current_opcode_location() { self.breakpoints.contains(&location) @@ -526,6 +579,52 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { } } +fn is_debug_file_in_debug_crate(debug_file: &DebugFile) -> bool { + debug_file.path.starts_with("__debug/") +} + +/// Builds a map from FileId to an ordered vector of tuples with line +/// numbers and opcode locations corresponding to those line numbers +fn build_source_to_opcode_debug_mappings( + debug_artifact: &DebugArtifact, +) -> BTreeMap> { + if debug_artifact.debug_symbols.is_empty() { + return BTreeMap::new(); + } + let locations = &debug_artifact.debug_symbols[0].locations; + let simple_files: BTreeMap<_, _> = debug_artifact + .file_map + .iter() + .filter(|(_, debug_file)| !is_debug_file_in_debug_crate(debug_file)) + .map(|(file_id, debug_file)| { + ( + file_id, + SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), + ) + }) + .collect(); + + let mut result: BTreeMap> = BTreeMap::new(); + locations.iter().for_each(|(opcode_location, source_locations)| { + source_locations.iter().for_each(|source_location| { + let span = source_location.span; + let file_id = source_location.file; + let Some(file) = simple_files.get(&file_id) else { + return; + }; + let Ok(line_index) = file.line_index((), span.start() as usize) else { + return; + }; + let line_number = line_index + 1; + + result.entry(file_id).or_default().push((line_number, *opcode_location)); + }); + }); + result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| (x.0, x.1))); + + result +} + #[cfg(test)] mod tests { use super::*; diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 7e67a26b257..7c722ed0a61 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -5,7 +5,6 @@ use std::str::FromStr; use acvm::acir::circuit::{Circuit, OpcodeLocation}; use acvm::acir::native_types::WitnessMap; use acvm::BlackBoxFunctionSolver; -use codespan_reporting::files::{Files, SimpleFile}; use crate::context::DebugCommandResult; use crate::context::DebugContext; @@ -30,15 +29,16 @@ use nargo::artifacts::debug::DebugArtifact; use fm::FileId; use noirc_driver::CompiledProgram; +type BreakpointId = i64; + pub struct DapSession<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> { server: Server, context: DebugContext<'a, B>, debug_artifact: &'a DebugArtifact, running: bool, - source_to_opcodes: BTreeMap>, - next_breakpoint_id: i64, - instruction_breakpoints: Vec<(OpcodeLocation, i64)>, - source_breakpoints: BTreeMap>, + next_breakpoint_id: BreakpointId, + instruction_breakpoints: Vec<(OpcodeLocation, BreakpointId)>, + source_breakpoints: BTreeMap>, } enum ScopeReferences { @@ -57,8 +57,6 @@ impl From for ScopeReferences { } } -// BTreeMap - impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { pub fn new( server: Server, @@ -67,7 +65,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, ) -> Self { - let source_to_opcodes = Self::build_source_to_opcode_debug_mappings(debug_artifact); let context = DebugContext::new( solver, circuit, @@ -79,7 +76,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { server, context, debug_artifact, - source_to_opcodes, running: false, next_breakpoint_id: 1, instruction_breakpoints: vec![], @@ -87,46 +83,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } } - /// Builds a map from FileId to an ordered vector of tuples with line - /// numbers and opcode locations corresponding to those line numbers - fn build_source_to_opcode_debug_mappings( - debug_artifact: &'a DebugArtifact, - ) -> BTreeMap> { - if debug_artifact.debug_symbols.is_empty() { - return BTreeMap::new(); - } - let locations = &debug_artifact.debug_symbols[0].locations; - let simple_files: BTreeMap<_, _> = debug_artifact - .file_map - .iter() - .map(|(file_id, debug_file)| { - ( - file_id, - SimpleFile::new(debug_file.path.to_str().unwrap(), debug_file.source.as_str()), - ) - }) - .collect(); - - let mut result: BTreeMap> = BTreeMap::new(); - locations.iter().for_each(|(opcode_location, source_locations)| { - if source_locations.is_empty() { - return; - } - let source_location = source_locations[0]; - let span = source_location.span; - let file_id = source_location.file; - let Ok(line_index) = &simple_files[&file_id].line_index((), span.start() as usize) - else { - return; - }; - let line_number = line_index + 1; - - result.entry(file_id).or_default().push((line_number, *opcode_location)); - }); - result.iter_mut().for_each(|(_, file_locations)| file_locations.sort_by_key(|x| x.0)); - result - } - fn send_stopped_event(&mut self, reason: StoppedEventReason) -> Result<(), ServerError> { let description = format!("{:?}", &reason); self.server.send_event(Event::Stopped(StoppedEventBody { @@ -230,6 +186,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_stack_trace(&self) -> Vec { + let stack_frames = self.context.get_variables(); + self.context .get_source_call_stack() .iter() @@ -239,9 +197,15 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { self.debug_artifact.location_line_number(*source_location).unwrap(); let column_number = self.debug_artifact.location_column_number(*source_location).unwrap(); + + let name = match stack_frames.get(index) { + Some(frame) => format!("{} {}", frame.function_name, index), + None => format!("frame #{index}"), + }; + StackFrame { id: index as i64, - name: format!("frame #{index}"), + name, source: Some(Source { path: self.debug_artifact.file_map[&source_location.file] .path @@ -422,7 +386,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { Ok(()) } - fn get_next_breakpoint_id(&mut self) -> i64 { + fn get_next_breakpoint_id(&mut self) -> BreakpointId { let id = self.next_breakpoint_id; self.next_breakpoint_id += 1; id @@ -493,36 +457,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { found.map(|iter| *iter.0) } - // TODO: there are four possibilities for the return value of this function: - // 1. the source location is not found -> None - // 2. an exact unique location is found -> Some(opcode_location) - // 3. an exact but not unique location is found (ie. a source location may - // be mapped to multiple opcodes, and those may be disjoint, for example for - // functions called multiple times throughout the program) - // 4. exact location is not found, so an opcode for a nearby source location - // is returned (this again could actually be more than one opcodes) - // Case 3 is not supported yet, and 4 is not correctly handled. - fn find_opcode_for_source_location( - &self, - file_id: &FileId, - line: i64, - ) -> Option { - let line = line as usize; - let Some(line_to_opcodes) = self.source_to_opcodes.get(file_id) else { - return None; - }; - let found_index = match line_to_opcodes.binary_search_by(|x| x.0.cmp(&line)) { - Ok(index) => line_to_opcodes[index].1, - Err(index) => { - if index >= line_to_opcodes.len() { - return None; - } - line_to_opcodes[index].1 - } - }; - Some(found_index) - } - fn map_source_breakpoints(&mut self, args: &SetBreakpointsArguments) -> Vec { let Some(ref source) = &args.source.path else { return vec![]; @@ -539,7 +473,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { .iter() .map(|breakpoint| { let line = breakpoint.line; - let Some(location) = self.find_opcode_for_source_location(&file_id, line) else { + let Some(location) = self.context.find_opcode_for_source_location(&file_id, line) + else { return Breakpoint { verified: false, message: Some(String::from( @@ -608,16 +543,20 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } fn build_local_variables(&self) -> Vec { - let mut variables: Vec<_> = self - .context - .get_variables() + let Some(current_stack_frame) = self.context.current_stack_frame() else { + return vec![]; + }; + + let mut variables = current_stack_frame + .variables .iter() .map(|(name, value, _var_type)| Variable { name: String::from(*name), value: format!("{:?}", *value), ..Variable::default() }) - .collect(); + .collect::>(); + variables.sort_by(|a, b| a.name.partial_cmp(&b.name).unwrap()); variables } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 68c4d3947b0..25f126ff490 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -3,17 +3,19 @@ use acvm::{ pwg::ForeignCallWaitInfo, }; use nargo::{ - artifacts::debug::{DebugArtifact, DebugVars}, + artifacts::debug::{DebugArtifact, DebugVars, StackFrame}, ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, }; -use noirc_errors::debug_info::DebugVarId; -use noirc_printable_type::{ForeignCallError, PrintableType, PrintableValue}; +use noirc_errors::debug_info::{DebugFnId, DebugVarId}; +use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { VarAssign, VarDrop, MemberAssign(u32), DerefAssign, + FnEnter, + FnExit, } impl DebugForeignCall { @@ -28,13 +30,16 @@ impl DebugForeignCall { "__debug_var_assign" => Some(DebugForeignCall::VarAssign), "__debug_var_drop" => Some(DebugForeignCall::VarDrop), "__debug_deref_assign" => Some(DebugForeignCall::DerefAssign), + "__debug_fn_enter" => Some(DebugForeignCall::FnEnter), + "__debug_fn_exit" => Some(DebugForeignCall::FnExit), _ => None, } } } pub trait DebugForeignCallExecutor: ForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)>; + fn get_variables(&self) -> Vec; + fn current_stack_frame(&self) -> Option; } pub struct DefaultDebugForeignCallExecutor { @@ -57,23 +62,33 @@ impl DefaultDebugForeignCallExecutor { } 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); - }); + // TODO: handle loading from the correct DebugInfo when we support + // debugging contracts + let Some(info) = artifact.debug_symbols.get(0) else { + return; + }; + self.debug_vars.insert_debug_info(info); } } impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor { - fn get_variables(&self) -> Vec<(&str, &PrintableValue, &PrintableType)> { + fn get_variables(&self) -> Vec { self.debug_vars.get_variables() } + + fn current_stack_frame(&self) -> Option { + self.debug_vars.current_stack_frame() + } } fn debug_var_id(value: &Value) -> DebugVarId { DebugVarId(value.to_u128() as u32) } +fn debug_fn_id(value: &Value) -> DebugFnId { + DebugFnId(value.to_u128() as u32) +} + impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { fn execute( &mut self, @@ -136,6 +151,19 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { } Ok(ForeignCallResult::default().into()) } + Some(DebugForeignCall::FnEnter) => { + let fcp_fn_id = &foreign_call.inputs[0]; + let ForeignCallParam::Single(fn_id_value) = fcp_fn_id else { + panic!("unexpected foreign call parameter in fn enter: {fcp_fn_id:?}") + }; + let fn_id = debug_fn_id(fn_id_value); + self.debug_vars.push_fn(fn_id); + Ok(ForeignCallResult::default().into()) + } + Some(DebugForeignCall::FnExit) => { + self.debug_vars.pop_fn(); + Ok(ForeignCallResult::default().into()) + } None => self.executor.execute(foreign_call), } } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 8441dbde9be..41dbf604f99 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -337,11 +337,13 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } 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); + for frame in self.context.get_variables() { + println!("{}({})", frame.function_name, frame.function_params.join(", ")); + for (var_name, value, var_type) in frame.variables.iter() { + let printable_value = + PrintableValueDisplay::Plain((*value).clone(), (*var_type).clone()); + println!(" {var_name}:{var_type:?} = {}", printable_value); + } } } @@ -530,7 +532,7 @@ pub fn run( .add( "vars", command! { - "show variable values available at this point in execution", + "show variables for each function scope available at this point in execution", () => || { ref_context.borrow_mut().show_vars(); Ok(CommandStatus::Done) diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index b5ffdb12d01..e298eb8aadd 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -30,7 +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, locations: &[Location]) { +pub(super) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { let locations = locations.iter(); for loc in locations { @@ -269,8 +269,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + 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 4cb678192b8..143ee7987f8 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -12,7 +12,7 @@ mod tests { let nargo_bin = cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); - let mut dbg_session = spawn_bash(Some(10000)).expect("Could not start bash session"); + let mut dbg_session = spawn_bash(Some(15000)).expect("Could not start bash session"); // Set backend to `/dev/null` to force an error if nargo tries to speak to a backend. dbg_session diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index a249ecb03ad..fbdf59805c9 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -8,7 +8,7 @@ use std::{ ops::Range, }; -pub use super::debug_vars::DebugVars; +pub use super::debug_vars::{DebugVars, StackFrame}; use fm::{FileId, FileManager, PathString}; /// A Debug Artifact stores, for a given program, the debug info for every function @@ -231,8 +231,12 @@ mod tests { let mut opcode_locations = BTreeMap::>::new(); opcode_locations.insert(OpcodeLocation::Acir(42), vec![loc]); - let debug_symbols = - vec![DebugInfo::new(opcode_locations, BTreeMap::default(), BTreeMap::default())]; + let debug_symbols = vec![DebugInfo::new( + opcode_locations, + BTreeMap::default(), + 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 index 20f2637f7d6..66568bec833 100644 --- a/tooling/nargo/src/artifacts/debug_vars.rs +++ b/tooling/nargo/src/artifacts/debug_vars.rs @@ -1,54 +1,85 @@ use acvm::brillig_vm::brillig::Value; use noirc_errors::debug_info::{ - DebugTypeId, DebugTypes, DebugVarId, DebugVariable, DebugVariables, + DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, }; use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; #[derive(Debug, Default, Clone)] pub struct DebugVars { variables: HashMap, + functions: HashMap, types: HashMap, - active: HashSet, - values: HashMap, + frames: Vec<(DebugFnId, HashMap)>, +} + +pub struct StackFrame<'a> { + pub function_name: &'a str, + pub function_params: Vec<&'a str>, + pub variables: Vec<(&'a str, &'a PrintableValue, &'a PrintableType)>, } 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_debug_info(&mut self, info: &DebugInfo) { + self.variables.extend(info.variables.clone()); + self.types.extend(info.types.clone()); + self.functions.extend(info.functions.clone()); + } + + pub fn get_variables(&self) -> Vec { + self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() } - pub fn insert_variables(&mut self, vars: &DebugVariables) { - self.variables.extend(vars.clone()); + pub fn current_stack_frame(&self) -> Option { + self.frames.last().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)) } - pub fn insert_types(&mut self, types: &DebugTypes) { - self.types.extend(types.clone()); + fn lookup_var(&self, var_id: DebugVarId) -> Option<(&str, &PrintableType)> { + self.variables.get(&var_id).and_then(|debug_var| { + let Some(ptype) = self.types.get(&debug_var.debug_type_id) else { + return None; + }; + Some((debug_var.name.as_str(), ptype)) + }) + } + + fn build_stack_frame<'a>( + &'a self, + fn_id: &DebugFnId, + frame: &'a HashMap, + ) -> StackFrame { + let debug_fn = &self.functions.get(fn_id).expect("failed to find function metadata"); + + let params: Vec<&str> = + debug_fn.arg_names.iter().map(|arg_name| arg_name.as_str()).collect(); + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame + .iter() + .filter_map(|(var_id, var_value)| { + self.lookup_var(*var_id).map(|(name, typ)| (name, var_value, typ)) + }) + .collect(); + + StackFrame { + function_name: debug_fn.name.as_str(), + function_params: params, + variables: vars, + } } 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)); + + self.frames + .last_mut() + .expect("unexpected empty stack frames") + .1 + .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 + let current_frame = &mut self.frames.last_mut().expect("unexpected empty stack frames").1; + let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); let cursor_type_id = &self @@ -102,7 +133,6 @@ impl DebugVars { }; } *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]) { @@ -114,6 +144,14 @@ impl DebugVars { } pub fn drop_var(&mut self, var_id: DebugVarId) { - self.active.remove(&var_id); + self.frames.last_mut().expect("unexpected empty stack frames").1.remove(&var_id); + } + + pub fn push_fn(&mut self, fn_id: DebugFnId) { + self.frames.push((fn_id, HashMap::default())); + } + + pub fn pop_fn(&mut self) { + self.frames.pop(); } }