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(nargo): Handle call stacks for multiple Acir calls #4711

Merged
merged 27 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6138253
initial updates to track debug info for each to program's circuit not…
vezenovm Apr 3, 2024
c48911a
working full call stacks with Acir calls
vezenovm Apr 3, 2024
9949ea8
nargo fmt
vezenovm Apr 3, 2024
477eda8
merge conflcits w/ master
vezenovm Apr 3, 2024
4b02b6e
remove old fold_poseidon test
vezenovm Apr 3, 2024
7c2ddaf
cleanup old comments
vezenovm Apr 3, 2024
7120ea9
have multiple debug infos for artifacts
vezenovm Apr 3, 2024
ba685f4
remove old DebugInfo compression
vezenovm Apr 3, 2024
0b4ef92
added ProgramDebugInfo to noir wasm compiler, tests now passing
vezenovm Apr 4, 2024
95b307e
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 4, 2024
b360e4a
some comments
vezenovm Apr 4, 2024
24c0058
Merge remote-tracking branch 'origin/mv/update-program-debug-info' in…
vezenovm Apr 4, 2024
eb2f27b
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 8, 2024
bdc0e74
merge conflicts w/ master
vezenovm Apr 8, 2024
264d6a3
remove serde_as
vezenovm Apr 9, 2024
8ff73f6
Update tooling/nargo/src/errors.rs
vezenovm Apr 9, 2024
c40ed05
Merge remote-tracking branch 'origin/mv/update-program-debug-info' in…
vezenovm Apr 9, 2024
9b7f4c8
Update tooling/nargo/src/errors.rs
vezenovm Apr 9, 2024
c15272d
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 9, 2024
1c93eeb
switch to using ResolvedOpcodeLocation rather a recursive error
vezenovm Apr 11, 2024
7236bad
cargo fmt
vezenovm Apr 11, 2024
5c52ac4
cleanup
vezenovm Apr 11, 2024
fe69f3d
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 11, 2024
3c19225
merge conflicts w/ master
vezenovm Apr 11, 2024
4346575
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 11, 2024
84e2c96
use current_function_index to fetch circuit
vezenovm Apr 11, 2024
af14e52
Merge branch 'master' into mv/update-program-debug-info
vezenovm Apr 12, 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
10 changes: 10 additions & 0 deletions acvm-repo/acir/src/circuit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ impl Circuit {
}
}

#[derive(Debug, Copy, Clone)]
/// The opcode location for a call to a separate ACIR circuit
/// This includes the function index of the caller within a [program][Program]
/// and the index in the callers ACIR to the specific call opcode.
/// This is only resolved and set during circuit execution.
pub struct ResolvedOpcodeLocation {
pub acir_function_index: usize,
pub opcode_location: OpcodeLocation,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)]
/// Opcodes are locatable so that callers can
/// map opcodes to debug information related to their context.
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub struct ContractFunction {
)]
pub bytecode: Program,

pub debug: DebugInfo,
pub debug: Vec<DebugInfo>,

/// Names of the functions in the program. These are used for more informative debugging and benchmarking.
pub names: Vec<String>,
Expand Down
10 changes: 3 additions & 7 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ fn compile_contract_inner(
}

if errors.is_empty() {
let debug_infos: Vec<_> = functions.iter().map(|function| function.debug.clone()).collect();
let debug_infos: Vec<_> =
functions.iter().flat_map(|function| function.debug.clone()).collect();
let file_map = filter_relevant_files(&debug_infos, &context.file_manager);

let out_structs = contract
Expand Down Expand Up @@ -547,13 +548,8 @@ pub fn compile_no_check(

Ok(CompiledProgram {
hash,
// TODO(https://github.com/noir-lang/noir/issues/4428)
program,
// TODO(https://github.com/noir-lang/noir/issues/4428)
// Debug info is only relevant for errors at execution time which is not yet supported
// The CompileProgram `debug` field is used in multiple places and is better
// left to be updated once execution of multiple ACIR functions is enabled
debug: debug[0].clone(),
debug,
abi,
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct CompiledProgram {
)]
pub program: Program,
pub abi: noirc_abi::Abi,
pub debug: DebugInfo,
pub debug: Vec<DebugInfo>,
pub file_map: BTreeMap<FileId, DebugFile>,
pub warnings: Vec<SsaReport>,
/// Names of the functions in the program. These are used for more informative debugging and benchmarking.
Expand Down
79 changes: 43 additions & 36 deletions compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,49 @@ pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct ProgramDebugInfo {
pub debug_infos: Vec<DebugInfo>,
}

impl ProgramDebugInfo {
pub fn serialize_compressed_base64_json<S>(
debug_info: &ProgramDebugInfo,
s: S,
) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let json_str = serde_json::to_string(debug_info).map_err(S::Error::custom)?;

let mut encoder = DeflateEncoder::new(Vec::new(), Compression::default());
encoder.write_all(json_str.as_bytes()).map_err(S::Error::custom)?;
let compressed_data = encoder.finish().map_err(S::Error::custom)?;

let encoded_b64 = base64::prelude::BASE64_STANDARD.encode(compressed_data);
s.serialize_str(&encoded_b64)
}

pub fn deserialize_compressed_base64_json<'de, D>(
deserializer: D,
) -> Result<ProgramDebugInfo, D::Error>
where
D: Deserializer<'de>,
{
let encoded_b64: String = Deserialize::deserialize(deserializer)?;

let compressed_data =
base64::prelude::BASE64_STANDARD.decode(encoded_b64).map_err(D::Error::custom)?;

let mut decoder = DeflateDecoder::new(&compressed_data[..]);
let mut decompressed_data = Vec::new();
decoder.read_to_end(&mut decompressed_data).map_err(D::Error::custom)?;

let json_str = String::from_utf8(decompressed_data).map_err(D::Error::custom)?;
serde_json::from_str(&json_str).map_err(D::Error::custom)
}
}

#[serde_as]
#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
Expand Down Expand Up @@ -130,40 +173,4 @@ impl DebugInfo {

counted_opcodes
}

pub fn serialize_compressed_base64_json<S>(
debug_info: &DebugInfo,
s: S,
) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let json_str = serde_json::to_string(debug_info).map_err(S::Error::custom)?;

let mut encoder = DeflateEncoder::new(Vec::new(), Compression::default());
encoder.write_all(json_str.as_bytes()).map_err(S::Error::custom)?;
let compressed_data = encoder.finish().map_err(S::Error::custom)?;

let encoded_b64 = base64::prelude::BASE64_STANDARD.encode(compressed_data);
s.serialize_str(&encoded_b64)
}

pub fn deserialize_compressed_base64_json<'de, D>(
deserializer: D,
) -> Result<DebugInfo, D::Error>
where
D: Deserializer<'de>,
{
let encoded_b64: String = Deserialize::deserialize(deserializer)?;

let compressed_data =
base64::prelude::BASE64_STANDARD.decode(encoded_b64).map_err(D::Error::custom)?;

let mut decoder = DeflateDecoder::new(&compressed_data[..]);
let mut decompressed_data = Vec::new();
decoder.read_to_end(&mut decompressed_data).map_err(D::Error::custom)?;

let json_str = String::from_utf8(decompressed_data).map_err(D::Error::custom)?;
serde_json::from_str(&json_str).map_err(D::Error::custom)
}
}
10 changes: 10 additions & 0 deletions compiler/wasm/src/types/noir_artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,16 @@ export interface DebugInfo {
locations: Record<OpcodeLocation, SourceCodeLocation[]>;
}

/**
* The debug information for a given program.
*/
export interface ProgramDebugInfo {
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
/**
* An array that maps to each function of a program.
*/
debug_infos: Array<DebugInfo>;
}

/**
* Maps a file ID to its metadata for debugging purposes.
*/
Expand Down
13 changes: 10 additions & 3 deletions compiler/wasm/test/compiler/shared/compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ContractCompilationArtifacts,
DebugFileMap,
DebugInfo,
ProgramDebugInfo,
NoirFunctionEntry,
ProgramArtifact,
ProgramCompilationArtifacts,
Expand All @@ -15,7 +16,7 @@ export function shouldCompileProgramIdentically(
expect: typeof Expect,
timeout = 5000,
) {
it('both nargo and noir_wasm should compile identically', async () => {
it('both nargo and noir_wasm should compile program identically', async () => {
// Compile!
const { nargoArtifact, noirWasmArtifact } = await compileFn();

Expand Down Expand Up @@ -51,7 +52,7 @@ export function shouldCompileContractIdentically(
expect: typeof Expect,
timeout = 5000,
) {
it('both nargo and noir_wasm should compile identically', async () => {
it('both nargo and noir_wasm should compile contract identically', async () => {
// Compile!
const { nargoArtifact, noirWasmArtifact } = await compileFn();

Expand Down Expand Up @@ -90,7 +91,7 @@ function extractDebugInfos(fns: NoirFunctionEntry[]) {
return fns.map((fn) => {
const debugSymbols = inflateDebugSymbols(fn.debug_symbols);
delete (fn as Partial<NoirFunctionEntry>).debug_symbols;
clearFileIdentifiers(debugSymbols);
clearFileIdentifiersProgram(debugSymbols);
return debugSymbols;
});
}
Expand All @@ -113,6 +114,12 @@ function deleteContractDebugMetadata(contract: ContractArtifact) {
return [extractDebugInfos(contract.functions), fileMap];
}

function clearFileIdentifiersProgram(debugSymbols: ProgramDebugInfo) {
debugSymbols.debug_infos.map((debug_info) => {
clearFileIdentifiers(debug_info);
});
}

/** Clears file identifiers from a set of debug symbols. */
function clearFileIdentifiers(debugSymbols: DebugInfo) {
for (const loc of Object.values(debugSymbols.locations)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "fold_dyn_index_fail"
type = "bin"
authors = [""]
compiler_version = ">=0.26.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = [104, 101, 108, 108, 111]
z = "4"
10 changes: 10 additions & 0 deletions test_programs/execution_failure/fold_dyn_index_fail/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main(mut x: [u32; 5], z: Field) {
x[z] = 4;
dynamic_index_check(x, z + 10);
}

#[fold]
fn dynamic_index_check(x: [u32; 5], idx: Field) {
// Dynamic index is greater than length of the array
assert(x[idx] != 0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "fold_nested_brillig_assert_fail"
type = "bin"
authors = [""]
compiler_version = ">=0.26.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Tests a very simple program.
//
// The features being tested is using assert on brillig that is triggered through nested ACIR calls.
// We want to make sure we get a call stack from the original call in main to the failed assert.
fn main(x: Field) {
assert(1 == fold_conditional_wrapper(x as bool));
}

#[fold]
fn fold_conditional_wrapper(x: bool) -> Field {
fold_conditional(x)
}

#[fold]
fn fold_conditional(x: bool) -> Field {
conditional_wrapper(x)
}

unconstrained fn conditional_wrapper(x: bool) -> Field {
conditional(x)
}

unconstrained fn conditional(x: bool) -> Field {
assert(x);
1
}
6 changes: 4 additions & 2 deletions tooling/debugger/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
self.handle_foreign_call(foreign_call)
}
Err(err) => DebugCommandResult::Error(NargoError::ExecutionError(
ExecutionError::SolvingError(err),
// TODO: debugger does not not handle multiple acir calls
ExecutionError::SolvingError(err, None),
)),
}
}
Expand Down Expand Up @@ -374,7 +375,8 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
}
}
ACVMStatus::Failure(error) => DebugCommandResult::Error(NargoError::ExecutionError(
ExecutionError::SolvingError(error),
// TODO: debugger does not not handle multiple acir calls
ExecutionError::SolvingError(error, None),
)),
ACVMStatus::RequiresForeignCall(_) => {
unreachable!("Unexpected pending foreign call resolution");
Expand Down
2 changes: 1 addition & 1 deletion tooling/debugger/src/dap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ pub fn run_session<R: Read, W: Write, B: BlackBoxFunctionSolver>(
initial_witness: WitnessMap,
) -> Result<(), ServerError> {
let debug_artifact = DebugArtifact {
debug_symbols: vec![program.debug],
debug_symbols: program.debug,
file_map: program.file_map,
warnings: program.warnings,
};
Expand Down
19 changes: 12 additions & 7 deletions tooling/lsp/src/requests/profile_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,29 @@ fn on_profile_run_request_inner(
let compiled_program =
nargo::ops::transform_program(compiled_program, expression_width);

let span_opcodes = compiled_program.debug.count_span_opcodes();
let debug_artifact: DebugArtifact = compiled_program.clone().into();
opcodes_counts.extend(span_opcodes);
for function_debug in compiled_program.debug.iter() {
let span_opcodes = function_debug.count_span_opcodes();
opcodes_counts.extend(span_opcodes);
}
let debug_artifact: DebugArtifact = compiled_program.into();
file_map.extend(debug_artifact.file_map);
}

for compiled_contract in compiled_contracts {
let compiled_contract =
nargo::ops::transform_contract(compiled_contract, expression_width);

let function_debug_info: Vec<_> =
compiled_contract.functions.iter().map(|func| &func.debug).cloned().collect();
let debug_artifact: DebugArtifact = compiled_contract.into();
file_map.extend(debug_artifact.file_map);
let function_debug_info = compiled_contract
.functions
.iter()
.flat_map(|func| &func.debug)
.collect::<Vec<_>>();
for contract_function_debug in function_debug_info {
let span_opcodes = contract_function_debug.count_span_opcodes();
opcodes_counts.extend(span_opcodes);
}
let debug_artifact: DebugArtifact = compiled_contract.into();
file_map.extend(debug_artifact.file_map);
}

let result = NargoProfileRunResult { file_map, opcodes_counts };
Expand Down
10 changes: 5 additions & 5 deletions tooling/nargo/src/artifacts/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use noirc_driver::{CompiledContract, CompiledContractOutputs, ContractFunction};
use serde::{Deserialize, Serialize};

use noirc_driver::DebugFile;
use noirc_errors::debug_info::DebugInfo;
use noirc_errors::debug_info::ProgramDebugInfo;
use std::collections::{BTreeMap, HashMap};

use fm::FileId;
Expand Down Expand Up @@ -68,10 +68,10 @@ pub struct ContractFunctionArtifact {
pub bytecode: Program,

#[serde(
serialize_with = "DebugInfo::serialize_compressed_base64_json",
deserialize_with = "DebugInfo::deserialize_compressed_base64_json"
serialize_with = "ProgramDebugInfo::serialize_compressed_base64_json",
deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json"
)]
pub debug_symbols: DebugInfo,
pub debug_symbols: ProgramDebugInfo,
}

impl From<ContractFunction> for ContractFunctionArtifact {
Expand All @@ -82,7 +82,7 @@ impl From<ContractFunction> for ContractFunctionArtifact {
custom_attributes: func.custom_attributes,
abi: func.abi,
bytecode: func.bytecode,
debug_symbols: func.debug,
debug_symbols: ProgramDebugInfo { debug_infos: func.debug },
}
}
}
4 changes: 2 additions & 2 deletions tooling/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl DebugArtifact {
impl From<CompiledProgram> for DebugArtifact {
fn from(compiled_program: CompiledProgram) -> Self {
DebugArtifact {
debug_symbols: vec![compiled_program.debug],
debug_symbols: compiled_program.debug,
file_map: compiled_program.file_map,
warnings: compiled_program.warnings,
}
Expand All @@ -133,7 +133,7 @@ impl From<CompiledContract> for DebugArtifact {
let all_functions_debug: Vec<DebugInfo> = compiled_artifact
.functions
.into_iter()
.map(|contract_function| contract_function.debug)
.flat_map(|contract_function| contract_function.debug)
.collect();

DebugArtifact {
Expand Down
Loading
Loading