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.

Sign up for GitHub

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: Handle BrilligCall opcodes in the debugger #4897

Merged
merged 7 commits into from
Apr 24, 2024
Merged

feat: Handle BrilligCall opcodes in the debugger #4897

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
22f26ca
feat: Handle BrilligCall opcodes when step debugging from REPL
ggiraldez Apr 23, 2024
ca37785
feat: Handle BrilligCall rendering opcodes, stacktrace and setting br…
ggiraldez Apr 23, 2024
72dceac
feat: Remove duplicate functionality
ggiraldez Apr 23, 2024
17d4679
feat: Handle external Brillig functions rendering opcodes for DAP
ggiraldez Apr 23, 2024
ad0baa6
Apply suggestions from code review
ggiraldez Apr 24, 2024
00cebdf
Remove code related to Opcode::Brillig which is no longer emitted
ggiraldez Apr 24, 2024
f465b62
Remove Opcode::Brillig in debugger context tests
ggiraldez Apr 24, 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
50 changes: 46 additions & 4 deletions acvm-repo/acvm/src/pwg/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
RequiresForeignCall(ForeignCallWaitInfo),

/// The ACVM has encountered a request for an ACIR [call][acir::circuit::Opcode]
/// to execute a separate ACVM instance. The result of the ACIR call must be passd back to the ACVM.

Check warning on line 54 in acvm-repo/acvm/src/pwg/mod.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (passd)
///
/// Once this is done, the ACVM can be restarted to solve the remaining opcodes.
RequiresAcirCall(AcirCallWaitInfo),
Expand Down Expand Up @@ -430,7 +430,7 @@
let Opcode::BrilligCall { id, inputs, outputs, predicate } =
&self.opcodes[self.instruction_pointer]
else {
unreachable!("Not executing a Brillig opcode");
unreachable!("Not executing a BrilligCall opcode");
};

let witness = &mut self.witness_map;
Expand Down Expand Up @@ -468,9 +468,17 @@
}
}

pub fn step_into_brillig(&mut self) -> StepResult<'a, B> {
match &self.opcodes[self.instruction_pointer] {
Opcode::Brillig(_) => self.step_into_brillig_opcode(),
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { .. } => self.step_into_brillig_call_opcode(),
_ => StepResult::Status(self.solve_opcode()),
}
}

pub fn step_into_brillig_opcode(&mut self) -> StepResult<'a, B> {
let Opcode::Brillig(brillig) = &self.opcodes[self.instruction_pointer] else {
return StepResult::Status(self.solve_opcode());
unreachable!("Not executing a Brillig opcode");
};

let witness = &mut self.witness_map;
Expand Down Expand Up @@ -498,9 +506,43 @@
}
}

fn step_into_brillig_call_opcode(&mut self) -> StepResult<'a, B> {
let Opcode::BrilligCall { id, inputs, outputs, predicate } =
&self.opcodes[self.instruction_pointer]
else {
unreachable!("Not executing a BrilligCall opcode");
};

let witness = &mut self.witness_map;
let should_skip = match is_predicate_false(witness, predicate) {
Ok(result) => result,
Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))),
};
if should_skip {
let resolution = BrilligSolver::<B>::zero_out_brillig_outputs(witness, outputs);
return StepResult::Status(self.handle_opcode_resolution(resolution));
}

let solver = BrilligSolver::new_call(
witness,
&self.block_solvers,
inputs,
&self.unconstrained_functions[*id as usize].bytecode,
self.backend,
self.instruction_pointer,
);
match solver {
Ok(solver) => StepResult::IntoBrillig(solver),
Err(..) => StepResult::Status(self.handle_opcode_resolution(solver.map(|_| ()))),
}
}

pub fn finish_brillig_with_solver(&mut self, solver: BrilligSolver<'a, B>) -> ACVMStatus {
if !matches!(&self.opcodes[self.instruction_pointer], Opcode::Brillig(..)) {
unreachable!("Not executing a Brillig opcode");
if !matches!(
self.opcodes[self.instruction_pointer],
Opcode::Brillig(..) | Opcode::BrilligCall { .. }
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
) {
unreachable!("Not executing a Brillig/BrilligCall opcode");
}
self.brillig_solver = Some(solver);
self.solve_opcode()
Expand Down
113 changes: 63 additions & 50 deletions tooling/debugger/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
debug_artifact: &'a DebugArtifact,
breakpoints: HashSet<OpcodeLocation>,
source_to_opcodes: BTreeMap<FileId, Vec<(usize, OpcodeLocation)>>,
unconstrained_functions: &'a [BrilligBytecode],
}

impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> {
Expand All @@ -59,6 +60,7 @@
debug_artifact,
breakpoints: HashSet::new(),
source_to_opcodes,
unconstrained_functions,
}
}

Expand Down Expand Up @@ -216,6 +218,9 @@
.iter()
.map(|opcode| match opcode {
Opcode::Brillig(brillig_block) => brillig_block.bytecode.len(),
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
self.unconstrained_functions[*id as usize].bytecode.len()
}
_ => 1,
})
.collect()
Expand Down Expand Up @@ -296,19 +301,30 @@
None => String::from("invalid"),
Some(OpcodeLocation::Acir(acir_index)) => {
let opcode = &opcodes[*acir_index];
if let Opcode::Brillig(ref brillig) = opcode {
let first_opcode = &brillig.bytecode[0];
format!("BRILLIG {first_opcode:?}")
} else {
format!("{opcode:?}")
match opcode {
Opcode::Brillig(brillig) => {
let first_opcode = &brillig.bytecode[0];
format!("BRILLIG {first_opcode:?}")
}
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
let first_opcode = &self.unconstrained_functions[*id as usize].bytecode[0];
format!("BRILLIG CALL {first_opcode:?}")
}
_ => format!("{opcode:?}"),
}
}
Some(OpcodeLocation::Brillig { acir_index, brillig_index }) => {
if let Opcode::Brillig(ref brillig) = opcodes[*acir_index] {
let opcode = &brillig.bytecode[*brillig_index];
format!(" | {opcode:?}")
} else {
String::from(" | invalid")
match &opcodes[*acir_index] {
Opcode::Brillig(brillig) => {
let opcode = &brillig.bytecode[*brillig_index];
format!(" | {opcode:?}")
}
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
let bytecode = &self.unconstrained_functions[*id as usize].bytecode;
let opcode = &bytecode[*brillig_index];
format!(" | {opcode:?}")
}
_ => String::from(" | invalid"),
}
}
}
Expand Down Expand Up @@ -400,7 +416,7 @@
return self.step_brillig_opcode();
}

match self.acvm.step_into_brillig_opcode() {
match self.acvm.step_into_brillig() {
StepResult::IntoBrillig(solver) => {
self.brillig_solver = Some(solver);
self.step_brillig_opcode()
Expand All @@ -409,20 +425,6 @@
}
}

fn currently_executing_brillig(&self) -> bool {
if self.brillig_solver.is_some() {
return true;
}

match self.get_current_opcode_location() {
Some(OpcodeLocation::Brillig { .. }) => true,
Some(OpcodeLocation::Acir(acir_index)) => {
matches!(self.get_opcodes()[acir_index], Opcode::Brillig(_))
}
_ => false,
}
}

fn get_current_acir_index(&self) -> Option<usize> {
self.get_current_opcode_location().map(|opcode_location| match opcode_location {
OpcodeLocation::Acir(acir_index) => acir_index,
Expand All @@ -446,8 +448,25 @@
}
}

pub(super) fn is_executing_brillig(&self) -> bool {
if self.brillig_solver.is_some() {
return true;
}

match self.get_current_opcode_location() {
Some(OpcodeLocation::Brillig { .. }) => true,
Some(OpcodeLocation::Acir(acir_index)) => {
matches!(
self.get_opcodes()[acir_index],
Opcode::Brillig(_) | Opcode::BrilligCall { .. }
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
)
}
_ => false,
}
}

pub(super) fn step_acir_opcode(&mut self) -> DebugCommandResult {
if self.currently_executing_brillig() {
if self.is_executing_brillig() {
self.step_out_of_brillig_opcode()
} else {
let status = self.acvm.solve_opcode();
Expand Down Expand Up @@ -511,12 +530,6 @@
}
}

pub(super) fn is_executing_brillig(&self) -> bool {
let opcodes = self.get_opcodes();
let acir_index = self.acvm.instruction_pointer();
acir_index < opcodes.len() && matches!(opcodes[acir_index], Opcode::Brillig(..))
}

pub(super) fn get_brillig_memory(&self) -> Option<&[MemoryValue]> {
self.brillig_solver.as_ref().map(|solver| solver.get_memory())
}
Expand Down Expand Up @@ -552,15 +565,18 @@
match *location {
OpcodeLocation::Acir(acir_index) => acir_index < opcodes.len(),
OpcodeLocation::Brillig { acir_index, brillig_index } => {
acir_index < opcodes.len()
&& matches!(opcodes[acir_index], Opcode::Brillig(..))
&& {
if let Opcode::Brillig(ref brillig) = opcodes[acir_index] {
brillig_index < brillig.bytecode.len()
} else {
false
if acir_index < opcodes.len() {
match &opcodes[acir_index] {
Opcode::Brillig(brillig) => brillig_index < brillig.bytecode.len(),
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
let bytecode = &self.unconstrained_functions[*id as usize].bytecode;
brillig_index < bytecode.len()
}
_ => false,
}
} else {
false
}
}
}
}
Expand Down Expand Up @@ -707,14 +723,14 @@

let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact));
let brillig_funcs = &vec![];

Check warning on line 726 in tooling/debugger/src/context.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (funcs)
let mut context = DebugContext::new(
&StubbedBlackBoxSolver,
circuit,
debug_artifact,
initial_witness,
foreign_call_executor,
brillig_funcs,

Check warning on line 733 in tooling/debugger/src/context.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (funcs)
);

assert_eq!(context.get_current_opcode_location(), Some(OpcodeLocation::Acir(0)));
Expand Down Expand Up @@ -816,14 +832,14 @@

let foreign_call_executor =
Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact));
let brillig_funcs = &vec![];

Check warning on line 835 in tooling/debugger/src/context.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (funcs)
let mut context = DebugContext::new(
&StubbedBlackBoxSolver,
circuit,
debug_artifact,
initial_witness,
foreign_call_executor,
brillig_funcs,

Check warning on line 842 in tooling/debugger/src/context.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (funcs)
);

// set breakpoint
Expand All @@ -849,16 +865,7 @@
#[test]
fn test_offset_opcode_location() {
let opcodes = vec![
Opcode::Brillig(Brillig {
inputs: vec![],
outputs: vec![],
bytecode: vec![
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
],
predicate: None,
}),
Opcode::BrilligCall { id: 0, inputs: vec![], outputs: vec![], predicate: None },
Opcode::MemoryInit { block_id: BlockId(0), init: vec![] },
Opcode::Brillig(Brillig {
inputs: vec![],
Expand All @@ -875,7 +882,13 @@
let circuit = Circuit { opcodes, ..Circuit::default() };
let debug_artifact =
DebugArtifact { debug_symbols: vec![], file_map: BTreeMap::new(), warnings: vec![] };
let brillig_funcs = &vec![];
let brillig_funcs = &vec![BrilligBytecode {

Check warning on line 885 in tooling/debugger/src/context.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (funcs)
bytecode: vec![
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 },
],
}];
let context = DebugContext::new(
&StubbedBlackBoxSolver,
&circuit,
Expand Down
56 changes: 39 additions & 17 deletions tooling/debugger/src/repl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::context::{DebugCommandResult, DebugContext};
use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation};
use acvm::acir::native_types::{Witness, WitnessMap};
use acvm::brillig_vm::brillig::Opcode as BrilligOpcode;
use acvm::{BlackBoxFunctionSolver, FieldElement};

use crate::foreign_calls::DefaultDebugForeignCallExecutor;
Expand Down Expand Up @@ -79,12 +80,16 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> {
println!("At opcode {}: {}", ip, opcode_summary);
}
OpcodeLocation::Brillig { acir_index, brillig_index } => {
let Opcode::Brillig(ref brillig) = opcodes[acir_index] else {
unreachable!("Brillig location does not contain a Brillig block");
let brillig_bytecode = match opcodes[acir_index] {
Opcode::Brillig(ref brillig) => &brillig.bytecode,
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
&self.unconstrained_functions[id as usize].bytecode
}
_ => unreachable!("Brillig location does not contain a Brillig block"),
};
println!(
"At opcode {}.{}: {:?}",
acir_index, brillig_index, brillig.bytecode[brillig_index]
acir_index, brillig_index, brillig_bytecode[brillig_index]
);
}
}
Expand All @@ -104,12 +109,16 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> {
)
}
OpcodeLocation::Brillig { acir_index, brillig_index } => {
let Opcode::Brillig(ref brillig) = opcodes[*acir_index] else {
unreachable!("Brillig location does not contain a Brillig block");
let brillig_bytecode = match opcodes[*acir_index] {
Opcode::Brillig(ref brillig) => &brillig.bytecode,
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, .. } => {
&self.unconstrained_functions[id as usize].bytecode
}
_ => unreachable!("Brillig location does not contain a Brillig block"),
};
println!(
"Frame #{index}, opcode {}.{}: {:?}",
acir_index, brillig_index, brillig.bytecode[*brillig_index]
acir_index, brillig_index, brillig_bytecode[*brillig_index]
);
}
}
Expand Down Expand Up @@ -162,22 +171,35 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> {
""
}
};
let print_brillig_bytecode = |acir_index, bytecode: &[BrilligOpcode]| {
for (brillig_index, brillig_opcode) in bytecode.iter().enumerate() {
println!(
"{:>3}.{:<2} |{:2} {:?}",
acir_index,
brillig_index,
brillig_marker(acir_index, brillig_index),
brillig_opcode
);
}
};
for (acir_index, opcode) in opcodes.iter().enumerate() {
let marker = outer_marker(acir_index);
if let Opcode::Brillig(brillig) = opcode {
println!("{:>3} {:2} BRILLIG inputs={:?}", acir_index, marker, brillig.inputs);
println!(" | outputs={:?}", brillig.outputs);
for (brillig_index, brillig_opcode) in brillig.bytecode.iter().enumerate() {
match &opcode {
Opcode::Brillig(brillig) => {
println!("{:>3} {:2} BRILLIG inputs={:?}", acir_index, marker, brillig.inputs);
println!(" | outputs={:?}", brillig.outputs);
print_brillig_bytecode(acir_index, &brillig.bytecode);
}
ggiraldez marked this conversation as resolved.
Show resolved Hide resolved
Opcode::BrilligCall { id, inputs, outputs, .. } => {
println!(
"{:>3}.{:<2} |{:2} {:?}",
acir_index,
brillig_index,
brillig_marker(acir_index, brillig_index),
brillig_opcode
"{:>3} {:2} BRILLIG CALL id={} inputs={:?}",
acir_index, marker, id, inputs
);
println!(" | outputs={:?}", outputs);
let bytecode = &self.unconstrained_functions[*id as usize].bytecode;
print_brillig_bytecode(acir_index, bytecode);
}
} else {
println!("{:>3} {:2} {:?}", acir_index, marker, opcode);
_ => println!("{:>3} {:2} {:?}", acir_index, marker, opcode),
}
}
}
Expand Down
Loading