From 25631ec3f9477e2983557e43df2846a68a649666 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Mon, 13 May 2024 17:51:20 +0000 Subject: [PATCH] fix(avm-transpiler): patch debug infos with modified PCs --- avm-transpiler/Cargo.lock | 1 + avm-transpiler/Cargo.toml | 3 +- avm-transpiler/src/main.rs | 6 +- avm-transpiler/src/transpile.rs | 54 +++++++++++++---- avm-transpiler/src/transpile_contract.rs | 76 +++++++++++++++--------- 5 files changed, 97 insertions(+), 43 deletions(-) diff --git a/avm-transpiler/Cargo.lock b/avm-transpiler/Cargo.lock index e0b11bd742a..14beeb02d67 100644 --- a/avm-transpiler/Cargo.lock +++ b/avm-transpiler/Cargo.lock @@ -313,6 +313,7 @@ dependencies = [ "env_logger", "log", "noirc_driver", + "noirc_errors", "regex", "serde", "serde_json", diff --git a/avm-transpiler/Cargo.toml b/avm-transpiler/Cargo.toml index d71c4f893d6..0e868d476bb 100644 --- a/avm-transpiler/Cargo.toml +++ b/avm-transpiler/Cargo.toml @@ -11,6 +11,7 @@ license = "MIT OR Apache-2.0" # local acvm = { path = "../noir/noir-repo/acvm-repo/acvm" } noirc_driver = { path = "../noir/noir-repo/compiler/noirc_driver" } +noirc_errors = { path = "../noir/noir-repo/compiler/noirc_errors" } # external base64 = "0.21" @@ -18,4 +19,4 @@ regex = "1.10" env_logger = "0.11" log = "0.4" serde_json = "1.0" -serde = { version = "1.0.136", features = ["derive"]} \ No newline at end of file +serde = { version = "1.0.136", features = ["derive"]} diff --git a/avm-transpiler/src/main.rs b/avm-transpiler/src/main.rs index 0ae4ebea1ee..d08011ca411 100644 --- a/avm-transpiler/src/main.rs +++ b/avm-transpiler/src/main.rs @@ -9,7 +9,7 @@ mod transpile; mod transpile_contract; mod utils; -use transpile_contract::{CompiledAcirContract, TranspiledContract}; +use transpile_contract::{CompiledAcirContractArtifact, TranspiledContractArtifact}; fn main() { env_logger::init(); @@ -38,11 +38,11 @@ fn main() { .expect("Unable to backup file"); // Parse json into contract object - let contract: CompiledAcirContract = + let contract: CompiledAcirContractArtifact = serde_json::from_str(&contract_json).expect("Unable to parse json"); // Transpile contract to AVM bytecode - let transpiled_contract = TranspiledContract::from(contract); + let transpiled_contract = TranspiledContractArtifact::from(contract); let transpiled_json = serde_json::to_string(&transpiled_contract).expect("Unable to serialize json"); fs::write(out_transpiled_artifact_path, transpiled_json).expect("Unable to write file"); diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 11007c9497e..c76802e14bf 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -1,9 +1,14 @@ +use std::collections::BTreeMap; + use acvm::acir::brillig::Opcode as BrilligOpcode; +use acvm::acir::circuit::OpcodeLocation; use acvm::brillig_vm::brillig::{ BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, HeapVector, MemoryAddress, ValueOrArray, }; use acvm::FieldElement; +use noirc_errors::debug_info::DebugInfo; +use noirc_errors::Location; use crate::instructions::{ AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT, @@ -13,15 +18,14 @@ use crate::opcodes::AvmOpcode; use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program}; /// Transpile a Brillig program to AVM bytecode -pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> Vec { +pub fn brillig_to_avm( + brillig_bytecode: &[BrilligOpcode], + brillig_pcs_to_avm_pcs: &Vec, +) -> Vec { dbg_print_brillig_program(brillig_bytecode); let mut avm_instrs: Vec = Vec::new(); - // Map Brillig pcs to AVM pcs - // (some Brillig instructions map to >1 AVM instruction) - let brillig_pcs_to_avm_pcs = map_brillig_pcs_to_avm_pcs(avm_instrs.len(), brillig_bytecode); - // Transpile a Brillig instruction to one or more AVM instructions for brillig_instr in brillig_bytecode { match brillig_instr { @@ -1116,6 +1120,39 @@ fn handle_storage_read( }) } +/// Patch a Noir function's debug info with updated PCs since transpilation injects extra +/// instructions in some cases. +pub fn patch_debug_info_pcs( + debug_infos: &Vec, + brillig_pcs_to_avm_pcs: &Vec, +) -> Vec { + let mut patched_debug_infos = debug_infos.clone(); + + for patched_debug_info in patched_debug_infos.iter_mut() { + // create a new map with all of its keys (OpcodeLocations) patched + let mut patched_locations: BTreeMap> = BTreeMap::new(); + for (original_opcode_location, source_locations) in patched_debug_info.locations.iter() { + match original_opcode_location { + OpcodeLocation::Brillig { + acir_index, + brillig_index, + } => { + let avm_opcode_location = OpcodeLocation::Brillig { + acir_index: *acir_index, + // patch the PC + brillig_index: brillig_pcs_to_avm_pcs[*brillig_index], + }; + patched_locations.insert(avm_opcode_location, source_locations.clone()); + } + OpcodeLocation::Acir(_) => (), + } + } + // patch debug_info entry + patched_debug_info.locations = patched_locations + } + patched_debug_infos +} + /// Compute an array that maps each Brillig pc to an AVM pc. /// This must be done before transpiling to properly transpile jump destinations. /// This is necessary for two reasons: @@ -1126,13 +1163,10 @@ fn handle_storage_read( /// brillig: the Brillig program /// returns: an array where each index is a Brillig pc, /// and each value is the corresponding AVM pc. -fn map_brillig_pcs_to_avm_pcs( - initial_offset: usize, - brillig_bytecode: &[BrilligOpcode], -) -> Vec { +pub fn map_brillig_pcs_to_avm_pcs(brillig_bytecode: &[BrilligOpcode]) -> Vec { let mut pc_map = vec![0; brillig_bytecode.len()]; - pc_map[0] = initial_offset; + pc_map[0] = 0; // first PC is always 0 as there are no instructions inserted by AVM at start for i in 0..brillig_bytecode.len() - 1 { let num_avm_instrs_for_this_brillig_instr = match &brillig_bytecode[i] { BrilligOpcode::Const { bit_size: 254, .. } => 2, diff --git a/avm-transpiler/src/transpile_contract.rs b/avm-transpiler/src/transpile_contract.rs index e28b20aa4a3..1b2987403b0 100644 --- a/avm-transpiler/src/transpile_contract.rs +++ b/avm-transpiler/src/transpile_contract.rs @@ -3,52 +3,56 @@ use log::info; use serde::{Deserialize, Serialize}; use acvm::acir::circuit::Program; +use noirc_errors::debug_info::DebugInfo; +use noirc_errors::debug_info::ProgramDebugInfo; -use crate::transpile::brillig_to_avm; +use crate::transpile::{brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_debug_info_pcs}; use crate::utils::extract_brillig_from_acir_program; /// Representation of a contract with some transpiled functions #[derive(Debug, Serialize, Deserialize)] -pub struct TranspiledContract { +pub struct TranspiledContractArtifact { pub transpiled: bool, pub noir_version: String, pub name: String, // Functions can be ACIR or AVM - pub functions: Vec, + pub functions: Vec, pub outputs: serde_json::Value, pub file_map: serde_json::Value, - //pub warnings: serde_json::Value, } /// A regular contract with ACIR+Brillig functions /// but with fields irrelevant to transpilation /// represented as catch-all serde Values #[derive(Debug, Serialize, Deserialize)] -pub struct CompiledAcirContract { +pub struct CompiledAcirContractArtifact { pub noir_version: String, pub name: String, - pub functions: Vec, + pub functions: Vec, pub outputs: serde_json::Value, pub file_map: serde_json::Value, - //pub warnings: serde_json::Value, } /// Representation of a contract function /// with AVM bytecode as a base64 string #[derive(Debug, Serialize, Deserialize)] -pub struct AvmContractFunction { +pub struct AvmContractFunctionArtifact { pub name: String, pub is_unconstrained: bool, pub custom_attributes: Vec, pub abi: serde_json::Value, pub bytecode: String, // base64 - pub debug_symbols: serde_json::Value, + #[serde( + serialize_with = "ProgramDebugInfo::serialize_compressed_base64_json", + deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" + )] + pub debug_symbols: ProgramDebugInfo, } /// Representation of an ACIR contract function but with /// catch-all serde Values for fields irrelevant to transpilation #[derive(Debug, Serialize, Deserialize)] -pub struct AcirContractFunction { +pub struct AcirContractFunctionArtifact { pub name: String, pub is_unconstrained: bool, pub custom_attributes: Vec, @@ -58,23 +62,27 @@ pub struct AcirContractFunction { deserialize_with = "Program::deserialize_program_base64" )] pub bytecode: Program, - pub debug_symbols: serde_json::Value, + #[serde( + serialize_with = "ProgramDebugInfo::serialize_compressed_base64_json", + deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" + )] + pub debug_symbols: ProgramDebugInfo, } /// An enum that allows the TranspiledContract struct to contain /// functions with either ACIR or AVM bytecode #[derive(Debug, Serialize, Deserialize)] #[serde(untagged)] // omit Acir/Avm tag for these objects in json -pub enum AvmOrAcirContractFunction { - Acir(AcirContractFunction), - Avm(AvmContractFunction), +pub enum AvmOrAcirContractFunctionArtifact { + Acir(AcirContractFunctionArtifact), + Avm(AvmContractFunctionArtifact), } /// Transpilation is performed when a TranspiledContract /// is constructed from a CompiledAcirContract -impl From for TranspiledContract { - fn from(contract: CompiledAcirContract) -> Self { - let mut functions = Vec::new(); +impl From for TranspiledContractArtifact { + fn from(contract: CompiledAcirContractArtifact) -> Self { + let mut functions: Vec = Vec::new(); for function in contract.functions { // TODO(4269): once functions are tagged for transpilation to AVM, check tag @@ -90,31 +98,41 @@ impl From for TranspiledContract { let acir_program = function.bytecode; let brillig_bytecode = extract_brillig_from_acir_program(&acir_program); + // Map Brillig pcs to AVM pcs (index is Brillig PC, value is AVM PC) + let brillig_pcs_to_avm_pcs = map_brillig_pcs_to_avm_pcs(brillig_bytecode); + // Transpile to AVM - let avm_bytecode = brillig_to_avm(brillig_bytecode); + let avm_bytecode = brillig_to_avm(brillig_bytecode, &brillig_pcs_to_avm_pcs); + + // Patch the debug infos with updated PCs + let debug_infos = patch_debug_info_pcs( + &function.debug_symbols.debug_infos, + &brillig_pcs_to_avm_pcs, + ); // Push modified function entry to ABI - functions.push(AvmOrAcirContractFunction::Avm(AvmContractFunction { - name: function.name, - is_unconstrained: function.is_unconstrained, - custom_attributes: function.custom_attributes, - abi: function.abi, - bytecode: base64::prelude::BASE64_STANDARD.encode(avm_bytecode), - debug_symbols: function.debug_symbols, - })); + functions.push(AvmOrAcirContractFunctionArtifact::Avm( + AvmContractFunctionArtifact { + name: function.name, + is_unconstrained: function.is_unconstrained, + custom_attributes: function.custom_attributes, + abi: function.abi, + bytecode: base64::prelude::BASE64_STANDARD.encode(avm_bytecode), + debug_symbols: ProgramDebugInfo { debug_infos }, + }, + )); } else { // This function is not flagged for transpilation. Push original entry. - functions.push(AvmOrAcirContractFunction::Acir(function)); + functions.push(AvmOrAcirContractFunctionArtifact::Acir(function)); } } - TranspiledContract { + TranspiledContractArtifact { transpiled: true, noir_version: contract.noir_version, name: contract.name, functions, // some acir, some transpiled avm functions outputs: contract.outputs, file_map: contract.file_map, - //warnings: contract.warnings, } } }