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: Add instrumentation for tracking variables in debugging #4122

Merged
merged 21 commits into from
Feb 5, 2024
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
67ccaae
feat: Implement debug instrumentation to track variable values
ggiraldez Jan 19, 2024
5170d2f
feat: Add debugging instrumentation to files in the package debugged
ggiraldez Jan 22, 2024
93d4c80
feat: Hide debug module stack frames in the debugger
ggiraldez Jan 22, 2024
de3a78e
chore: Ignore debugger failing tests and use Brillig mode
ggiraldez Jan 22, 2024
ca45375
chore: Refactor instrumentation patching code during monomorphization
ggiraldez Jan 22, 2024
6744c1d
feat: Only link the debug crate when instrumenting code for the debugger
ggiraldez Jan 23, 2024
260add6
chore: Ignore two more tests for debugging which fail with assertions
ggiraldez Jan 23, 2024
eee6724
Merge remote-tracking branch 'upstream/master' into debug-vars-instru…
ggiraldez Jan 24, 2024
0c0848a
fix: Address code review comments and remove unneeded code
ggiraldez Jan 29, 2024
8ebf01e
fix: First pass addressing code review comments
ggiraldez Jan 31, 2024
e0a4c88
chore: Extract debug related code in monomorphization
ggiraldez Jan 31, 2024
937793d
feat: Use var names collected during injection in monomorphization phase
ggiraldez Feb 1, 2024
bd68ee8
feat: Refactor debug instrumentation code in the compiler
ggiraldez Feb 2, 2024
10ae07a
chore: Missing renames from last refactor
ggiraldez Feb 2, 2024
6e26f60
chore: Remove unneeded code and undo unnecessary changes
ggiraldez Feb 2, 2024
8d4142b
chore: Small documentation update
ggiraldez Feb 2, 2024
c4745ba
chore: And one more missing rename
ggiraldez Feb 2, 2024
236239b
Merge remote-tracking branch 'upstream/master' into debug-vars-instru…
ggiraldez Feb 2, 2024
c01a993
fix: Compiler errors after merge
ggiraldez Feb 2, 2024
9a54467
chore: cargo fmt
ggiraldez Feb 2, 2024
82318b3
feat: Hide instrumentation compile option by default
ggiraldez Feb 5, 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
Prev Previous commit
Next Next commit
feat: Use var names collected during injection in monomorphization phase
Also, move some debug related internal state from Monomorphizer into DebugTypes.
ggiraldez committed Feb 1, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 937793d0b538579bdbcad03f648e22f729857e02
3 changes: 1 addition & 2 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
@@ -418,8 +418,7 @@ pub fn compile_no_check(
force_compile: bool,
) -> Result<CompiledProgram, RuntimeError> {
let program = if options.instrument_debug {
let field_names = context.debug_state.field_names.clone();
monomorphize_debug(main_function, &mut context.def_interner, field_names)
monomorphize_debug(main_function, &mut context.def_interner, &context.debug_state)
} else {
monomorphize(main_function, &mut context.def_interner)
};
119 changes: 53 additions & 66 deletions compiler/noirc_frontend/src/monomorphization/debug.rs
Original file line number Diff line number Diff line change
@@ -12,9 +12,13 @@ const DEBUG_VALUE_ARG_SLOT: usize = 1;
const DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT: usize = 2;

impl<'interner> Monomorphizer<'interner> {
/// Try to patch instrumentation code inserted for debugging. This will
/// record tracked variables and their types, and assign them an ID to use
/// at runtime.
/// 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 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,
@@ -41,22 +45,17 @@ impl<'interner> Monomorphizer<'interner> {
/// 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 Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT) else {
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = var_id_arg else {
unreachable!("Missing FE var ID in __debug_var_assign call");
};
let Some(HirExpression::Ident(HirIdent { id, .. })) = hir_arguments.get(DEBUG_VALUE_ARG_SLOT) else {
unreachable!("Missing value identifier in __debug_var_assign call");
};

// update variable assignments
let var_def = self.interner.definition(*id);
// 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 fe_var_id = fe_var_id.to_u128() as u32;
let var_id = if var_def.name != "__debug_expr" {
self.debug_types.insert_var(fe_var_id, &var_def.name, var_type)
} else {
self.debug_types.get_var_id(fe_var_id).unwrap()
};
// then update the ID used for tracking at runtime
let var_id = self.debug_types.insert_var(fe_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);
}
@@ -66,15 +65,18 @@ impl<'interner> Monomorphizer<'interner> {
/// 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 Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT) else {
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = var_id_arg else {
unreachable!("Missing FE var ID in __debug_var_drop call");
};
// update variable drops (ie. when the var goes out of scope)
// update variable ID for tracked drops (ie. when the var goes out of scope)
let fe_var_id = fe_var_id.to_u128() as u32;
if let Some(var_id) = self.debug_types.get_var_id(fe_var_id) {
let interned_var_id = self.intern_var_id(var_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
}
let var_id = self
.debug_types
.get_var_id(fe_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
@@ -90,43 +92,39 @@ impl<'interner> Monomorphizer<'interner> {
arity: usize,
) {
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT) else {
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(fe_var_id, _))) = var_id_arg else {
unreachable!("Missing FE var ID in __debug_member_assign call");
};
let Some(HirExpression::Ident(HirIdent { id, .. })) = hir_arguments.get(DEBUG_VALUE_ARG_SLOT) else {
unreachable!("Missing value identifier in __debug_member_assign call");
};
// update variable member assignments
let var_def_name = self.interner.definition(*id).name.clone();
let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]);
let fe_var_id = fe_var_id.to_u128() as u32;

let mut cursor_type = self
let var_type = self
.debug_types
.get_type(fe_var_id)
.unwrap_or_else(|| panic!("type not found for fe_var_id={fe_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 mut index = fe_i.to_i128();
let index = fe_i.to_i128().unsigned_abs();
if *i_neg {
index = -index;
}
if index < 0 {
let index = index.unsigned_abs();
let field_name = self
.debug_field_names
.get(&(index as u32))
.unwrap_or_else(|| panic!("field name not available for {i:?}"));
let field_i = (get_field(&cursor_type, field_name)
.unwrap_or_else(|| panic!("failed to find field_name: {field_name}"))
as i128)
.unsigned_abs();
cursor_type = element_type_at_index(&cursor_type, field_i as usize);
// 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 code path is also used
// for tuple member access.
let field_index = self
.debug_types
.resolve_field_index(index as u32, 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_i.into(), false),
HirLiteral::Integer(field_index.into(), false),
));
self.interner.push_expr_type(&index_id, crate::Type::FieldElement);
self.interner.push_expr_location(
@@ -136,41 +134,30 @@ impl<'interner> Monomorphizer<'interner> {
);
arguments[DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i] = self.expr(index_id);
} else {
cursor_type = element_type_at_index(&cursor_type, 0);
// array/string element using constant index
cursor_type = element_type_at_index(cursor_type, index as usize);
}
} else {
cursor_type = element_type_at_index(&cursor_type, 0);
// array element using non-constant index
cursor_type = element_type_at_index(cursor_type, 0);
}
}

let var_id = if &var_def_name != "__debug_expr" {
self.debug_types.insert_var(fe_var_id, &var_def_name, var_type)
} else {
self.debug_types.get_var_id(fe_var_id).unwrap()
};
let var_id = self
.debug_types
.get_var_id(fe_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 get_field(ptype: &PrintableType, field_name: &str) -> Option<usize> {
match ptype {
PrintableType::Struct { fields, .. } => {
fields.iter().position(|(name, _)| name == field_name)
}
PrintableType::Tuple { .. } | PrintableType::Array { .. } => {
field_name.parse::<usize>().ok()
}
_ => None,
}
}

fn element_type_at_index(ptype: &PrintableType, i: usize) -> PrintableType {
fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType {
match ptype {
PrintableType::Array { length: _length, typ } => (**typ).clone(),
PrintableType::Tuple { types } => types[i].clone(),
PrintableType::Struct { name: _name, fields } => fields[i].1.clone(),
PrintableType::String { length: _length } => PrintableType::UnsignedInteger { width: 8 },
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"]
}
37 changes: 35 additions & 2 deletions compiler/noirc_frontend/src/monomorphization/debug_types.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::hir_def::types::Type;
use crate::{debug::DebugState, hir_def::types::Type};
pub use noirc_errors::debug_info::{Types, VariableTypes, Variables};
use noirc_printable_type::PrintableType;
use std::collections::HashMap;
@@ -11,6 +11,8 @@
/// will have a valid type.
#[derive(Debug, Clone, Default)]
pub struct DebugTypes {
debug_variables: HashMap<u32, String>,
debug_field_names: HashMap<u32, String>,
fe_to_vars: HashMap<u32, u32>, // fe_var_id => var_id
variables: HashMap<u32, (String, u32)>, // var_id => (var_name, type_id)
types: HashMap<PrintableType, u32>,
@@ -20,8 +22,27 @@
}

impl DebugTypes {
pub fn insert_var(&mut self, fe_var_id: u32, var_name: &str, var_type: Type) -> u32 {
pub fn build_from_debug_state(debug_state: &DebugState) -> Self {
DebugTypes {
debug_variables: debug_state.variables.clone(),
debug_field_names: debug_state.field_names.clone(),
..DebugTypes::default()
}
}

pub fn resolve_field_index(&self, field_id: u32, cursor_type: &PrintableType) -> Option<usize> {
self.debug_field_names
.get(&field_id)
.and_then(|field_name| get_field(cursor_type, field_name))
}

pub fn insert_var(&mut self, fe_var_id: u32, var_type: Type) -> u32 {
let var_name = self
.debug_variables
.get(&fe_var_id)
.unwrap_or_else(|| unreachable!("cannot find name for debug variable {fe_var_id}"));

let ptype: PrintableType = var_type.follow_bindings().into();

Check warning on line 45 in compiler/noirc_frontend/src/monomorphization/debug_types.rs

GitHub Actions / Code

Unknown word (ptype)
let type_id = self.types.get(&ptype).cloned().unwrap_or_else(|| {
let type_id = self.next_type_id;
self.next_type_id += 1;
@@ -56,6 +77,18 @@
}
}

fn get_field(ptype: &PrintableType, field_name: &str) -> Option<usize> {
match ptype {
PrintableType::Struct { fields, .. } => {
fields.iter().position(|(name, _)| name == field_name)
}
PrintableType::Tuple { .. } | PrintableType::Array { .. } => {
field_name.parse::<usize>().ok()
}
_ => None,
}
}

impl From<DebugTypes> for VariableTypes {
fn from(val: DebugTypes) -> Self {
(
14 changes: 7 additions & 7 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ use std::{
};

use crate::{
debug::DebugState,
hir_def::{
expr::*,
function::{FuncMeta, FunctionSignature, Parameters},
@@ -82,7 +83,6 @@ struct Monomorphizer<'interner> {
return_location: Option<Location>,

debug_types: DebugTypes,
debug_field_names: HashMap<u32, String>,
}

type HirType = crate::Type;
@@ -100,15 +100,16 @@ type HirType = crate::Type;
/// 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: &mut NodeInterner) -> Program {
monomorphize_debug(main, interner, HashMap::default())
monomorphize_debug(main, interner, &DebugState::default())
}

pub fn monomorphize_debug(
main: node_interner::FuncId,
interner: &mut NodeInterner,
field_names: HashMap<u32, String>,
debug_state: &DebugState,
) -> Program {
let mut monomorphizer = Monomorphizer::new(interner, field_names);
let debug_types = DebugTypes::build_from_debug_state(debug_state);
let mut monomorphizer = Monomorphizer::new(interner, debug_types);
let function_sig = monomorphizer.compile_main(main);

while !monomorphizer.queue.is_empty() {
@@ -137,7 +138,7 @@ pub fn monomorphize_debug(
}

impl<'interner> Monomorphizer<'interner> {
fn new(interner: &'interner mut NodeInterner, debug_field_names: HashMap<u32, String>) -> Self {
fn new(interner: &'interner mut NodeInterner, debug_types: DebugTypes) -> Self {
Monomorphizer {
globals: HashMap::new(),
locals: HashMap::new(),
@@ -149,8 +150,7 @@ impl<'interner> Monomorphizer<'interner> {
lambda_envs_stack: Vec::new(),
is_range_loop: false,
return_location: None,
debug_types: DebugTypes::default(),
debug_field_names,
debug_types,
}
}

Loading