From 8111bea6cfee63ebf360204a49bd2c110229f877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Fri, 13 Oct 2023 23:21:09 -0400 Subject: [PATCH] feat: add debugging commands to manage breakpoints Also: - add a constructor for `DebugContext` - don't quit the debugger when finishing execution of the current circuit --- acvm-repo/acvm/src/pwg/mod.rs | 4 + tooling/debugger/src/lib.rs | 136 +++++++++++++++++++------ tooling/nargo_cli/src/cli/debug_cmd.rs | 1 - 3 files changed, 110 insertions(+), 31 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 08b2463c9ce..b860b098099 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -207,6 +207,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { status } + pub fn get_status(&self) -> &ACVMStatus { + &self.status + } + /// Sets the VM status to [ACVMStatus::Failure] using the provided `error`. /// Returns the new status. fn fail(&mut self, error: OpcodeResolutionError) -> ACVMStatus { diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index cb98c5eedf6..9b85f47de27 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -14,6 +14,7 @@ use fm::FileId; use noirc_driver::DebugFile; use noirc_errors::Location; use std::cell::{Cell, RefCell}; +use std::collections::HashSet; enum SolveResult { Done, @@ -81,9 +82,26 @@ struct DebugContext<'backend, B: BlackBoxFunctionSolver> { foreign_call_executor: ForeignCallExecutor, circuit: &'backend Circuit, show_output: bool, + breakpoints: HashSet, } impl<'backend, B: BlackBoxFunctionSolver> DebugContext<'backend, B> { + fn new( + blackbox_solver: &'backend B, + circuit: &'backend Circuit, + debug_artifact: DebugArtifact, + initial_witness: WitnessMap, + ) -> Self { + Self { + acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), + foreign_call_executor: ForeignCallExecutor::default(), + circuit, + debug_artifact, + show_output: true, + breakpoints: HashSet::new(), + } + } + fn handle_acvm_status(&mut self, status: ACVMStatus) -> Result { match status { ACVMStatus::Solved => Ok(SolveResult::Done), @@ -130,10 +148,10 @@ impl<'backend, B: BlackBoxFunctionSolver> DebugContext<'backend, B> { Some(location) => { match location { OpcodeLocation::Acir(ip) => { - println!("Stopped at opcode {}: {}", ip, opcodes[ip]) + println!("At opcode {}: {}", ip, opcodes[ip]) } OpcodeLocation::Brillig { acir_index: ip, brillig_index } => println!( - "Stopped at opcode {} in Brillig block {}: {}", + "At opcode {} in Brillig block {}: {}", brillig_index, ip, opcodes[ip] ), } @@ -184,7 +202,13 @@ impl<'backend, B: BlackBoxFunctionSolver> DebugContext<'backend, B> { fn display_opcodes(&self) { let current = self.acvm.instruction_pointer(); for (ip, opcode) in self.acvm.opcodes().iter().enumerate() { - let marker = if ip == current { "->" } else { "" }; + let marker = if ip == current { + "->" + } else if self.breakpoints.contains(&OpcodeLocation::Acir(ip)) { + "*" + } else { + "" + }; println!("{:>3} {:2} {:?}", ip, marker, opcode); } } @@ -205,19 +229,41 @@ impl<'backend, B: BlackBoxFunctionSolver> DebugContext<'backend, B> { SolveResult::Done => break, SolveResult::Ok => {} } + if let Some(location) = self.acvm.location() { + if self.breakpoints.contains(&location) { + println!("Stopped at breakpoint in opcode {}", location); + return Ok(SolveResult::Ok); + } + } } Ok(SolveResult::Done) } - fn finalize(self) -> WitnessMap { - self.acvm.finalize() + fn add_breakpoint(&mut self, ip: usize) { + if self.breakpoints.insert(OpcodeLocation::Acir(ip)) { + println!("Added breakpoint at opcode {ip}"); + } else { + println!("Breakpoint at opcode {ip} already set"); + } } -} -fn map_command_status(result: SolveResult) -> CommandStatus { - match result { - SolveResult::Ok => CommandStatus::Done, - SolveResult::Done => CommandStatus::Quit, + fn delete_breakpoint(&mut self, ip: usize) { + if self.breakpoints.remove(&OpcodeLocation::Acir(ip)) { + println!("Breakpoint at opcode {ip} deleted"); + } else { + println!("Breakpoint at opcode {ip} not set"); + } + } + + fn finished(&self) -> bool { + !matches!( + self.acvm.get_status(), + ACVMStatus::InProgress | ACVMStatus::RequiresForeignCall { .. } + ) + } + + fn finalize(self) -> WitnessMap { + self.acvm.finalize() } } @@ -226,15 +272,9 @@ pub fn debug_circuit( circuit: &Circuit, debug_artifact: DebugArtifact, initial_witness: WitnessMap, - show_output: bool, ) -> Result, NargoError> { - let context = RefCell::new(DebugContext { - acvm: ACVM::new(blackbox_solver, &circuit.opcodes, initial_witness), - foreign_call_executor: ForeignCallExecutor::default(), - circuit, - debug_artifact, - show_output, - }); + let context = + RefCell::new(DebugContext::new(blackbox_solver, circuit, debug_artifact, initial_witness)); let ref_context = &context; let solved = Cell::new(false); @@ -243,7 +283,7 @@ pub fn debug_circuit( let handle_result = |result| { solved.set(matches!(result, SolveResult::Done)); - Ok(map_command_status(result)) + Ok(CommandStatus::Done) }; let mut repl = Repl::builder() @@ -252,9 +292,14 @@ pub fn debug_circuit( command! { "step to the next ACIR opcode", () => || { - let result = ref_context.borrow_mut().step_acir_opcode().into_critical()?; - ref_context.borrow().show_current_vm_status(); - handle_result(result) + if ref_context.borrow().finished() { + println!("Execution finished"); + Ok(CommandStatus::Done) + } else { + let result = ref_context.borrow_mut().step_acir_opcode().into_critical()?; + ref_context.borrow().show_current_vm_status(); + handle_result(result) + } } }, ) @@ -263,9 +308,14 @@ pub fn debug_circuit( command! { "step into to the next opcode", () => || { - let result = ref_context.borrow_mut().step_into_opcode().into_critical()?; - ref_context.borrow().show_current_vm_status(); - handle_result(result) + if ref_context.borrow().finished() { + println!("Execution finished"); + Ok(CommandStatus::Done) + } else { + let result = ref_context.borrow_mut().step_into_opcode().into_critical()?; + ref_context.borrow().show_current_vm_status(); + handle_result(result) + } } }, ) @@ -274,9 +324,15 @@ pub fn debug_circuit( command! { "continue execution until the end of the program", () => || { - println!("(Continuing execution...)"); - let result = ref_context.borrow_mut().cont().into_critical()?; - handle_result(result) + if ref_context.borrow().finished() { + println!("Execution finished"); + Ok(CommandStatus::Done) + } else { + println!("(Continuing execution...)"); + let result = ref_context.borrow_mut().cont().into_critical()?; + ref_context.borrow().show_current_vm_status(); + handle_result(result) + } } }, ) @@ -311,15 +367,35 @@ pub fn debug_circuit( }, ) .add( - "current", + "location", command! { - "display status and next opcode to evaluate", + "display next opcode to evaluate", () => || { ref_context.borrow().show_current_vm_status(); Ok(CommandStatus::Done) } }, ) + .add( + "break", + command! { + "add a breakpoint at an opcode location", + (LOCATION:usize) => |ip| { + ref_context.borrow_mut().add_breakpoint(ip); + Ok(CommandStatus::Done) + } + }, + ) + .add( + "delete", + command! { + "delete breakpoint at an opcode location", + (LOCATION:usize) => |ip| { + ref_context.borrow_mut().delete_breakpoint(ip); + Ok(CommandStatus::Done) + } + }, + ) .build() .expect("Failed to initialize debugger repl"); diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 7d8c0dc9a15..c7aa7f3d4ca 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -123,7 +123,6 @@ pub(crate) fn debug_program( &compiled_program.circuit, debug_artifact, initial_witness, - true, ) .map_err(CliError::from) }