Skip to content

Commit

Permalink
feat: optionally output a debug artifact on compile (#2260)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
sirasistant and TomAFrench authored Aug 14, 2023
1 parent 053662a commit edded24
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 50 deletions.
4 changes: 3 additions & 1 deletion crates/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ impl From<&PathBuf> for PathString {
pub struct FileMap(SimpleFiles<PathString, String>);

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)]
#[derive(
Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize, PartialOrd, Ord,
)]
pub struct FileId(usize);

impl FileId {
Expand Down
2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
assert!(old_value.is_none(), "ice: the same path was inserted into the file manager twice");
}

pub fn fetch_file(&mut self, file_id: FileId) -> File {
pub fn fetch_file(&self, file_id: FileId) -> File {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
Expand Down
50 changes: 50 additions & 0 deletions crates/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use serde::{Deserialize, Serialize};
use std::{
collections::{BTreeMap, BTreeSet},
path::PathBuf,
};

use fm::FileId;
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::hir::Context;

/// For a given file, we store the source code and the path to the file
/// so consumers of the debug artifact can reconstruct the original source code structure.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugFile {
pub source: String,
pub path: PathBuf,
}

/// A Debug Artifact stores, for a given program, the debug info for every function
/// along with a map of file Id to the source code so locations in debug info can be mapped to source code they point to.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugArtifact {
pub debug_symbols: Vec<DebugInfo>,
pub file_map: BTreeMap<FileId, DebugFile>,
}

impl DebugArtifact {
pub fn new(debug_symbols: Vec<DebugInfo>, compilation_context: &Context) -> Self {
let mut file_map = BTreeMap::new();

let files_with_debug_symbols: BTreeSet<FileId> = debug_symbols
.iter()
.flat_map(|function_symbols| function_symbols.locations.values().map(|loc| loc.file))
.collect();

for file_id in files_with_debug_symbols {
let file_source = compilation_context.file_manager.fetch_file(file_id).source();

file_map.insert(
file_id,
DebugFile {
source: file_source.to_string(),
path: compilation_context.file_manager.path(file_id).to_path_buf(),
},
);
}

Self { debug_symbols, file_map }
}
}
1 change: 1 addition & 0 deletions crates/nargo/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use base64::Engine;
use serde::{Deserializer, Serializer};

pub mod contract;
pub mod debug;
pub mod program;

// TODO: move these down into ACVM.
Expand Down
23 changes: 13 additions & 10 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
include_keys: bool,
common_reference_string: &[u8],
func: ContractFunction,
) -> Result<PreprocessedContractFunction, B::Error> {
) -> Result<(PreprocessedContractFunction, DebugInfo), B::Error> {
// TODO: currently `func`'s bytecode is already optimized for the backend.
// In future we'll need to apply those optimizations here.
let optimized_bytecode = func.bytecode;
Expand All @@ -54,14 +54,17 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
(None, None)
};

Ok(PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,
Ok((
PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,

bytecode: optimized_bytecode,
proving_key,
verification_key,
})
bytecode: optimized_bytecode,
proving_key,
verification_key,
},
func.debug,
))
}
97 changes: 68 additions & 29 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ use acvm::acir::circuit::OpcodeLabel;
use acvm::{acir::circuit::Circuit, Backend};
use iter_extended::try_vecmap;
use iter_extended::vecmap;
use nargo::artifacts::debug::DebugArtifact;
use nargo::package::Package;
use nargo::prepare_package;
use nargo::{artifacts::contract::PreprocessedContract, NargoError};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use noirc_driver::{
compile_contracts, compile_main, CompileOptions, CompiledContract, CompiledProgram,
ContractFunction, ErrorsAndWarnings, Warnings,
ErrorsAndWarnings, Warnings,
};
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::graph::CrateName;
use noirc_frontend::hir::Context;

Expand All @@ -19,6 +21,7 @@ use nargo::ops::{preprocess_contract_function, preprocess_program};

use crate::errors::{CliError, CompileError};

use super::fs::program::save_debug_artifact_to_file;
use super::fs::{
common_reference_string::{
read_cached_common_reference_string, update_common_reference_string,
Expand All @@ -38,6 +41,10 @@ pub(crate) struct CompileCommand {
#[arg(long)]
include_keys: bool,

/// Output debug files
#[arg(long, hide = true)]
output_debug: bool,

/// The name of the package to compile
#[clap(long)]
package: Option<CrateName>,
Expand Down Expand Up @@ -70,49 +77,72 @@ pub(crate) fn run<B: Backend>(
// As can be seen here, It seems like a leaky abstraction where ContractFunctions (essentially CompiledPrograms)
// are compiled via nargo-core and then the PreprocessedContract is constructed here.
// This is due to EACH function needing it's own CRS, PKey, and VKey from the backend.
let preprocessed_contracts: Result<Vec<PreprocessedContract>, CliError<B>> =
try_vecmap(optimized_contracts, |contract| {
let preprocessed_contract_functions = try_vecmap(contract.functions, |func| {
common_reference_string = update_common_reference_string(
backend,
&common_reference_string,
&func.bytecode,
)
.map_err(CliError::CommonReferenceStringError)?;

preprocess_contract_function(
backend,
args.include_keys,
&common_reference_string,
func,
)
.map_err(CliError::ProofSystemCompilerError)
})?;

Ok(PreprocessedContract {
let preprocessed_contracts: Result<
Vec<(PreprocessedContract, Vec<DebugInfo>)>,
CliError<B>,
> = try_vecmap(optimized_contracts, |contract| {
let preprocess_result = try_vecmap(contract.functions, |func| {
common_reference_string = update_common_reference_string(
backend,
&common_reference_string,
&func.bytecode,
)
.map_err(CliError::CommonReferenceStringError)?;

preprocess_contract_function(
backend,
args.include_keys,
&common_reference_string,
func,
)
.map_err(CliError::ProofSystemCompilerError)
})?;

let (preprocessed_contract_functions, debug_infos): (Vec<_>, Vec<_>) =
preprocess_result.into_iter().unzip();

Ok((
PreprocessedContract {
name: contract.name,
backend: String::from(BACKEND_IDENTIFIER),
functions: preprocessed_contract_functions,
})
});
for contract in preprocessed_contracts? {
},
debug_infos,
))
});
for (contract, debug_infos) in preprocessed_contracts? {
save_contract_to_file(
&contract,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);

if args.output_debug {
let debug_artifact = DebugArtifact::new(debug_infos, &context);
save_debug_artifact_to_file(
&debug_artifact,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);
}
}
} else {
let (_, program) = compile_package(backend, package, &args.compile_options)?;
let (context, program) = compile_package(backend, package, &args.compile_options)?;

common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;

let (preprocessed_program, _) =
let (preprocessed_program, debug_info) =
preprocess_program(backend, args.include_keys, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
save_program_to_file(&preprocessed_program, &package.name, &circuit_dir);

if args.output_debug {
let debug_artifact = DebugArtifact::new(vec![debug_info], &context);
let circuit_name: String = (&package.name).into();
save_debug_artifact_to_file(&debug_artifact, &circuit_name, &circuit_dir);
}
}
}

Expand Down Expand Up @@ -167,9 +197,18 @@ pub(super) fn optimize_contract<B: Backend>(
backend: &B,
contract: CompiledContract,
) -> Result<CompiledContract, CliError<B>> {
let functions = try_vecmap(contract.functions, |func| {
let optimized_bytecode = optimize_circuit(backend, func.bytecode)?.0;
Ok::<_, CliError<B>>(ContractFunction { bytecode: optimized_bytecode, ..func })
let functions = try_vecmap(contract.functions, |mut func| {
let (optimized_bytecode, opcode_labels) = optimize_circuit(backend, func.bytecode)?;
let opcode_ids = vecmap(opcode_labels, |label| match label {
OpcodeLabel::Unresolved => {
unreachable!("Compiled circuit opcodes must resolve to some index")
}
OpcodeLabel::Resolved(index) => index as usize,
});

func.bytecode = optimized_bytecode;
func.debug.update_acir(opcode_ids);
Ok::<_, CliError<B>>(func)
})?;

Ok(CompiledContract { functions, ..contract })
Expand Down
19 changes: 16 additions & 3 deletions crates/nargo_cli/src/cli/fs/program.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::path::{Path, PathBuf};

use nargo::artifacts::{contract::PreprocessedContract, program::PreprocessedProgram};
use nargo::artifacts::{
contract::PreprocessedContract, debug::DebugArtifact, program::PreprocessedProgram,
};
use noirc_frontend::graph::CrateName;

use crate::errors::FilesystemError;
Expand All @@ -15,20 +17,31 @@ pub(crate) fn save_program_to_file<P: AsRef<Path>>(
let circuit_name: String = crate_name.into();
save_build_artifact_to_file(compiled_program, &circuit_name, circuit_dir)
}

pub(crate) fn save_contract_to_file<P: AsRef<Path>>(
compiled_contract: &PreprocessedContract,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
save_build_artifact_to_file(compiled_contract, circuit_name, circuit_dir)
}

pub(crate) fn save_debug_artifact_to_file<P: AsRef<Path>>(
debug_artifact: &DebugArtifact,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
let artifact_name = format!("debug_{}", circuit_name);
save_build_artifact_to_file(debug_artifact, &artifact_name, circuit_dir)
}

fn save_build_artifact_to_file<P: AsRef<Path>, T: ?Sized + serde::Serialize>(
build_artifact: &T,
circuit_name: &str,
artifact_name: &str,
circuit_dir: P,
) -> PathBuf {
create_named_dir(circuit_dir.as_ref(), "target");
let circuit_path = circuit_dir.as_ref().join(circuit_name).with_extension("json");
let circuit_path = circuit_dir.as_ref().join(artifact_name).with_extension("json");

write_to_file(&serde_json::to_vec(build_artifact).unwrap(), &circuit_path);

Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::program::{deserialize_circuit, serialize_circuit};
use acvm::acir::circuit::Circuit;
use noirc_abi::Abi;
use noirc_errors::debug_info::DebugInfo;
use serde::{Deserialize, Serialize};

/// Describes the types of smart contract functions that are allowed.
Expand Down Expand Up @@ -47,6 +48,8 @@ pub struct ContractFunction {

#[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")]
pub bytecode: Circuit,

pub debug: DebugInfo,
}

impl ContractFunctionType {
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ fn compile_contract(
is_internal: func_meta.is_internal.unwrap_or(false),
abi: function.abi,
bytecode: function.circuit,
debug: function.debug,
});
}

Expand Down
8 changes: 4 additions & 4 deletions crates/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use std::collections::HashMap;
use std::collections::BTreeMap;

use crate::Location;
use serde::{Deserialize, Serialize};

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
/// Map opcode index of an ACIR circuit into the source code location
pub locations: HashMap<usize, Location>,
pub locations: BTreeMap<usize, Location>,
}

impl DebugInfo {
pub fn new(locations: HashMap<usize, Location>) -> Self {
pub fn new(locations: BTreeMap<usize, Location>) -> Self {
DebugInfo { locations }
}

Expand All @@ -24,7 +24,7 @@ impl DebugInfo {
/// This is the case during fallback or width 'optimization'
/// opcode_indices is this list of mixed indices
pub fn update_acir(&mut self, opcode_indices: Vec<usize>) {
let mut new_locations = HashMap::new();
let mut new_locations = BTreeMap::new();
for (i, idx) in opcode_indices.iter().enumerate() {
if self.locations.contains_key(idx) {
new_locations.insert(i, self.locations[idx]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! `GeneratedAcir` is constructed as part of the `acir_gen` pass to accumulate all of the ACIR
//! program as it is being converted from SSA form.
use std::collections::HashMap;
use std::collections::BTreeMap;

use crate::{
brillig::brillig_gen::brillig_directive,
Expand Down Expand Up @@ -46,7 +46,7 @@ pub(crate) struct GeneratedAcir {
pub(crate) input_witnesses: Vec<Witness>,

/// Correspondance between an opcode index (in opcodes) and the source code location which generated it
pub(crate) locations: HashMap<usize, Location>,
pub(crate) locations: BTreeMap<usize, Location>,

/// Source code location of the current instruction being processed
/// None if we do not know the location
Expand Down

0 comments on commit edded24

Please sign in to comment.