Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Track stack frames and their variables in the debugger #4188

Merged
merged 26 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
87355b8
feat: Include function stack info in debug vars command
nthiad Jan 22, 2024
9a057cc
fix: use new get_variables() signature in dap debugger
nthiad Jan 24, 2024
22be9a3
Use frame function name in stacktrace
mverzilli Jan 25, 2024
434fc94
cargo fmt
mverzilli Jan 25, 2024
95c4a68
fix: pass span through to enter/exit
nthiad Jan 25, 2024
fdb8e79
chore: cargo fmt
ggiraldez Jan 25, 2024
3e15b97
chore: Remove unnecessary parameter to print_source_code_location
ggiraldez Jan 26, 2024
d90b17a
feat: Improve opcode lookup heuristic when setting source breakpoints
ggiraldez Jan 26, 2024
68acf85
support frames in debug mappings
mverzilli Jan 26, 2024
4484bee
Fix stackframes
mverzilli Jan 26, 2024
fc4e196
chore: Re-enable some debugger tests and increase timeout for eddsa
ggiraldez Feb 9, 2024
c772979
chore: Some refactoring for code organization
ggiraldez Feb 10, 2024
a459122
chore: Remove unneeded function
ggiraldez Feb 12, 2024
3b39a39
chore: Load only first DebugInfo when building DebugVars
ggiraldez Feb 12, 2024
3fae7dc
feat: Put debug info for functions separate from variables
ggiraldez Feb 12, 2024
e741141
fix: Don't put names in a function's PrintableType
ggiraldez Feb 12, 2024
3d392d2
feat: Insert enter/exit instrumentation calls in walk_fn
ggiraldez Feb 12, 2024
8a4937e
chore: Rename parameter in debug instrumentation oracle functions
ggiraldez Feb 12, 2024
47a5a55
fix: Handle edge case when mapping source lines into opcodes
ggiraldez Feb 12, 2024
d58388e
Merge remote-tracking branch 'upstream/master' into stack-frames
ggiraldez Feb 12, 2024
100ad57
fix: Compilation errors
ggiraldez Feb 12, 2024
8f8e134
Merge remote-tracking branch 'upstream/master' into stack-frames
ggiraldez Feb 13, 2024
e834fac
Merge remote-tracking branch 'upstream/master' into stack-frames
ggiraldez Feb 20, 2024
ccd3ec7
chore: cargo clippy
ggiraldez Feb 20, 2024
4dc7b76
Merge remote-tracking branch 'upstream/master' into stack-frames
ggiraldez Feb 27, 2024
c7631ea
Merge remote-tracking branch 'upstream/master' into stack-frames
ggiraldez Mar 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading