Skip to content

Commit

Permalink
feat: Track stack frames and their variables in the debugger (#4188)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Part of #3015

We want to track call frames and observe variables local to the
currently executing frame.

## Summary\*

This PR adds a bit more debugging instrumentation to track when function
call frames are entered and exited, which allows to determine which
variables are "live" at the current point in execution. This new
instrumentation also allows labeling the frames in the call stack from
the names of the functions being executed.

Also, it includes improvements on how source breakpoints are mapped into
opcode breakpoints. Both features combined bring substantial
improvements to the debugger usability.

## Additional Context

## 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 <sy@nthia.dev>
Co-authored-by: Martin Verzilli <martin.verzilli@gmail.com>
  • Loading branch information
3 people authored Mar 7, 2024
1 parent 2a555a0 commit ae1a9d9
Show file tree
Hide file tree
Showing 18 changed files with 415 additions and 193 deletions.
14 changes: 13 additions & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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<String>,
}

pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[serde_as]
Expand All @@ -45,6 +55,7 @@ pub struct DebugInfo {
#[serde_as(as = "BTreeMap<DisplayFromStr, _>")]
pub locations: BTreeMap<OpcodeLocation, Vec<Location>>,
pub variables: DebugVariables,
pub functions: DebugFunctions,
pub types: DebugTypes,
}

Expand All @@ -60,9 +71,10 @@ impl DebugInfo {
pub fn new(
locations: BTreeMap<OpcodeLocation, Vec<Location>>,
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.
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub fn create_circuit(
) -> Result<(Circuit, DebugInfo, Vec<Witness>, Vec<Witness>, Vec<SsaReport>), 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(
Expand Down Expand Up @@ -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);
Expand Down
104 changes: 97 additions & 7 deletions compiler/noirc_frontend/src/debug/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -26,8 +28,12 @@ pub struct DebugInstrumenter {
// all field names referenced when assigning to a member of a variable
pub field_names: HashMap<SourceFieldId, String>,

// all collected function metadata (name + argument names)
pub functions: HashMap<DebugFnId, DebugFunction>,

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<HashMap<String, SourceVarId>>,
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -76,10 +84,22 @@ impl DebugInstrumenter {
field_name_id
}

fn insert_function(&mut self, fn_name: String, arguments: Vec<String>) -> 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(&param.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| {
Expand All @@ -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.
Expand Down Expand Up @@ -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},
}};"#
Expand All @@ -451,14 +484,32 @@ pub fn build_debug_crate_file() -> String {
}
#[oracle(__debug_var_drop)]
unconstrained fn __debug_var_drop_oracle<T>(_var_id: u32) {}
unconstrained fn __debug_var_drop_inner<T>(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<T>(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<T>(_var_id: u32, _value: T) {}
unconstrained fn __debug_dereference_assign_inner<T>(var_id: u32, value: T) {
Expand Down Expand Up @@ -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)]);
Expand All @@ -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::<Vec<String>>().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::<Vec<_>>()
.join(", "),
)
}
}
}

fn ident(s: &str, span: Span) -> ast::Ident {
ast::Ident(Spanned::from(span, s.to_string()))
}
Expand Down
8 changes: 5 additions & 3 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()) }
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use acvm::FieldElement;
use iter_extended::vecmap;
use noirc_errors::{
debug_info::{DebugTypes, DebugVariables},
debug_info::{DebugFunctions, DebugTypes, DebugVariables},
Location,
};

Expand Down Expand Up @@ -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,
}

Expand All @@ -266,6 +267,7 @@ impl Program {
return_visibility: Visibility,
recursive: bool,
debug_variables: DebugVariables,
debug_functions: DebugFunctions,
debug_types: DebugTypes,
) -> Program {
Program {
Expand All @@ -276,6 +278,7 @@ impl Program {
return_visibility,
recursive,
debug_variables,
debug_functions,
debug_types,
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/monomorphization/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
Loading

0 comments on commit ae1a9d9

Please sign in to comment.