Skip to content

Commit

Permalink
feat: Add instrumentation for tracking variables in debugging (#4122)
Browse files Browse the repository at this point in the history
# 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 <sy@nthia.dev>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
3 people committed Feb 5, 2024
1 parent bdf64ed commit c58d691
Show file tree
Hide file tree
Showing 41 changed files with 1,591 additions and 87 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions acvm-repo/acir_field/src/generic_ark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ impl<F: PrimeField> FieldElement<F> {
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()))
}
Expand Down Expand Up @@ -240,6 +244,12 @@ impl<F: PrimeField> FieldElement<F> {
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<u64> {
(self.num_bits() <= 64).then(|| self.to_u128() as u64)
}
Expand Down
40 changes: 35 additions & 5 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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<ExpressionWidth, std::io::Error> {
Expand Down Expand Up @@ -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
}
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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<CompiledContract, ErrorsAndWarnings> {
Expand Down Expand Up @@ -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<CompiledProgram>,
force_compile: bool,
) -> Result<CompiledProgram, RuntimeError> {
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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_errors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 24 additions & 2 deletions compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<DebugVarId, DebugVariable>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[serde_as]
#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
Expand All @@ -28,6 +44,8 @@ pub struct DebugInfo {
/// that they should be serialized to/from strings.
#[serde_as(as = "BTreeMap<DisplayFromStr, _>")]
pub locations: BTreeMap<OpcodeLocation, Vec<Location>>,
pub variables: DebugVariables,
pub types: DebugTypes,
}

/// Holds OpCodes Counts for Acir and Brillig Opcodes
Expand All @@ -39,8 +57,12 @@ pub struct OpCodesCount {
}

impl DebugInfo {
pub fn new(locations: BTreeMap<OpcodeLocation, Vec<Location>>) -> Self {
DebugInfo { locations }
pub fn new(
locations: BTreeMap<OpcodeLocation, Vec<Location>>,
variables: DebugVariables,
types: DebugTypes,
) -> Self {
Self { locations, variables, types }
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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")
}
}
}
24 changes: 18 additions & 6 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GeneratedAcir, RuntimeError> {
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
Expand Down Expand Up @@ -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<Witness>, Vec<Witness>, Vec<SsaReport>), 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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -169,8 +177,12 @@ struct SsaBuilder {
}

impl SsaBuilder {
fn new(program: Program, print_ssa_passes: bool) -> Result<SsaBuilder, RuntimeError> {
let ssa = ssa_gen::generate_ssa(program)?;
fn new(
program: Program,
print_ssa_passes: bool,
force_brillig_runtime: bool,
) -> Result<SsaBuilder, RuntimeError> {
let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?;
Ok(SsaBuilder { print_ssa_passes, ssa }.print("Initial SSA:"))
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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."
),
Expand Down
11 changes: 9 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Ssa, RuntimeError> {
pub(crate) fn generate_ssa(
program: Program,
force_brillig_runtime: bool,
) -> Result<Ssa, RuntimeError> {
// see which parameter has call_data/return_data attribute
let is_databus = DataBusBuilder::is_databus(&program.main_function_signature);

Expand All @@ -56,7 +59,11 @@ pub(crate) fn generate_ssa(program: Program) -> Result<Ssa, RuntimeError> {
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,
);

Expand Down

0 comments on commit c58d691

Please sign in to comment.