From 5a8f56d53bc2b361f0b14840fbb97795a1ca5422 Mon Sep 17 00:00:00 2001 From: synthia Date: Tue, 3 Oct 2023 14:58:32 -0700 Subject: [PATCH] refactor debug logic around ReplContext methods --- tooling/nargo/src/ops/debug.rs | 214 ++++++++++++++----------------- tooling/nargo_cli/src/cli/mod.rs | 1 - 2 files changed, 97 insertions(+), 118 deletions(-) diff --git a/tooling/nargo/src/ops/debug.rs b/tooling/nargo/src/ops/debug.rs index 8d5c533b73d..7630254925e 100644 --- a/tooling/nargo/src/ops/debug.rs +++ b/tooling/nargo/src/ops/debug.rs @@ -9,7 +9,8 @@ use crate::NargoError; use super::foreign_calls::ForeignCallExecutor; -use std::io::{self, Write}; +use std::rc::Rc; +use std::sync::Mutex; use reedline_repl_rs::clap::{ ArgMatches as ReplArgMatches, @@ -22,20 +23,92 @@ enum SolveResult { Ok, } -enum Command { - Step, - Continue, - Stop, -} - struct ReplContext<'backend, B: BlackBoxFunctionSolver> { - acvm: ACVM<'backend, B>, + acvm: Option>, debug_artifact: DebugArtifact, foreign_call_executor: ForeignCallExecutor, circuit: Circuit, show_output: bool, } +impl<'backend, B> ReplContext<'backend, B> where B: BlackBoxFunctionSolver { + fn step_opcode(&mut self) -> Result { + // Assert messages are not a map due to https://github.com/noir-lang/acvm/issues/522 + let assert_messages = &self.circuit.assert_messages; + let get_assert_message = |opcode_location| { + assert_messages + .iter() + .find(|(loc, _)| loc == opcode_location) + .map(|(_, message)| message.clone()) + }; + + let acvm = self.acvm.as_mut().unwrap(); + let solver_status = acvm.solve_opcode(); + + match solver_status { + ACVMStatus::Solved => Ok(SolveResult::Done), + ACVMStatus::InProgress => Ok(SolveResult::Ok), + ACVMStatus::Failure(error) => { + let call_stack = match &error { + OpcodeResolutionError::UnsatisfiedConstrain { + opcode_location: ErrorLocation::Resolved(opcode_location), + } => Some(vec![*opcode_location]), + OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { + Some(call_stack.clone()) + } + _ => None, + }; + + Err(NargoError::ExecutionError(match call_stack { + Some(call_stack) => { + if let Some(assert_message) = get_assert_message( + call_stack.last().expect("Call stacks should not be empty"), + ) { + ExecutionError::AssertionFailed(assert_message, call_stack) + } else { + ExecutionError::SolvingError(error) + } + } + None => ExecutionError::SolvingError(error), + })) + } + ACVMStatus::RequiresForeignCall(foreign_call) => { + let foreign_call_result = self.foreign_call_executor.execute(&foreign_call, self.show_output)?; + acvm.resolve_pending_foreign_call(foreign_call_result); + Ok(SolveResult::Ok) + } + } + } + + fn show_current_vm_status(&self) { + let acvm = self.acvm.as_ref().unwrap(); + let ip = acvm.instruction_pointer(); + println!("Stopped at opcode {}: {}", ip, acvm.opcodes()[ip]); + self.show_source_code_location(&OpcodeLocation::Acir(ip)); + } + + fn show_source_code_location(&self, location: &OpcodeLocation) { + let locations = self.debug_artifact.debug_symbols[0].opcode_location(&location); + match locations { + Some(locations) => { + for loc in locations { + let file = &self.debug_artifact.file_map[&loc.file]; + let source = &file.source.as_str(); + let start = loc.span.start() as usize; + let end = loc.span.end() as usize; + println!("At {}:{start}-{end}", file.path.as_path().display()); + println!("\n{}\n", &source[start..end]); + } + }, + None => {} + } + } + + fn finalize(&mut self) -> WitnessMap { + self.acvm.take().unwrap().finalize() + } +} + impl From for NargoError { fn from(_e: reedline_repl_rs::Error) -> Self { NargoError::CompilationError @@ -62,17 +135,16 @@ pub fn debug_circuit( ) -> Result { let opcodes = circuit.opcodes.clone(); let acvm = ACVM::new(blackbox_solver, opcodes, initial_witness); - let mut foreign_call_executor = ForeignCallExecutor::default(); + let foreign_call_executor = ForeignCallExecutor::default(); - let repl_context = ReplContext { - acvm, + let repl_context = Rc::new(Mutex::new(ReplContext { + acvm: Some(acvm), debug_artifact, foreign_call_executor, circuit, show_output, - }; - - let mut repl = Repl::new(repl_context) + })); + let mut repl = Repl::new(repl_context.clone()) .with_name("debug") .with_version(env!["CARGO_PKG_VERSION"]) .with_command( @@ -91,117 +163,25 @@ pub fn debug_circuit( quit_command, ); repl.run().unwrap(); - - let solved_witness = repl_context.acvm.finalize(); + let solved_witness = repl_context.lock().unwrap().finalize(); Ok(solved_witness) } -fn step_opcode( - acvm: &mut ACVM, - assert_messages: &Vec<(OpcodeLocation, String)>, - show_output: bool, - foreign_call_executor: &mut ForeignCallExecutor, -) -> Result { - // Assert messages are not a map due to https://github.com/noir-lang/acvm/issues/522 - let get_assert_message = |opcode_location| { - assert_messages - .iter() - .find(|(loc, _)| loc == opcode_location) - .map(|(_, message)| message.clone()) - }; - - let solver_status = acvm.solve_opcode(); - - match solver_status { - ACVMStatus::Solved => Ok(SolveResult::Done), - ACVMStatus::InProgress => Ok(SolveResult::Ok), - ACVMStatus::Failure(error) => { - let call_stack = match &error { - OpcodeResolutionError::UnsatisfiedConstrain { - opcode_location: ErrorLocation::Resolved(opcode_location), - } => Some(vec![*opcode_location]), - OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { - Some(call_stack.clone()) - } - _ => None, - }; - - Err(NargoError::ExecutionError(match call_stack { - Some(call_stack) => { - if let Some(assert_message) = get_assert_message( - call_stack.last().expect("Call stacks should not be empty"), - ) { - ExecutionError::AssertionFailed(assert_message, call_stack) - } else { - ExecutionError::SolvingError(error) - } - } - None => ExecutionError::SolvingError(error), - })) - } - ACVMStatus::RequiresForeignCall(foreign_call) => { - let foreign_call_result = foreign_call_executor.execute(&foreign_call, show_output)?; - acvm.resolve_pending_foreign_call(foreign_call_result); - Ok(SolveResult::Ok) - } - } -} - -fn show_source_code_location(location: &OpcodeLocation, debug_artifact: &DebugArtifact) { - let locations = debug_artifact.debug_symbols[0].opcode_location(&location); - match locations { - Some(locations) => { - for loc in locations { - let file = &debug_artifact.file_map[&loc.file]; - let source = &file.source.as_str(); - let start = loc.span.start() as usize; - let end = loc.span.end() as usize; - println!("At {}:{start}-{end}", file.path.as_path().display()); - println!("\n{}\n", &source[start..end]); - } - }, - None => {} - } -} - -fn show_current_vm_status (acvm: &ACVM, debug_artifact: &DebugArtifact) { - let ip = acvm.instruction_pointer(); - println!("Stopped at opcode {}: {}", ip, acvm.opcodes()[ip]); - show_source_code_location(&OpcodeLocation::Acir(ip), &debug_artifact); -} - -fn read_command() -> Result { - loop { - let mut line = String::new(); - print!(">>> "); - io::stdout().flush().unwrap(); - io::stdin().read_line(&mut line)?; - if line.is_empty() { - return Ok(Command::Stop); - } - match line.trim() { - "s" => return Ok(Command::Step), - "c" => return Ok(Command::Continue), - "q" => return Ok(Command::Stop), - "" => continue, - _ => println!("ERROR: unknown command") - } - } -} - -fn step_command(_args: ReplArgMatches, context: &mut ReplContext) -> Result, NargoError> { - show_current_vm_status(&mut context.acvm, &mut context.debug_artifact); - match step_opcode(&mut context.acvm, &mut context.circuit.assert_messages, context.show_output, &mut context.foreign_call_executor)? { +fn step_command(_args: ReplArgMatches, context: &mut Rc>>) -> Result, NargoError> { + let mut c = context.lock().unwrap(); + c.show_current_vm_status(); + match c.step_opcode()? { SolveResult::Done => Ok(Some("Done".to_string())), SolveResult::Ok => Ok(Some("Ok".to_string())), } } -fn continue_command(_args: ReplArgMatches, context: &mut ReplContext) -> Result, NargoError> { - show_current_vm_status(&mut context.acvm, &mut context.debug_artifact); +fn continue_command(_args: ReplArgMatches, context: &mut Rc>>) -> Result, NargoError> { + let mut c = context.lock().unwrap(); + c.show_current_vm_status(); println!("(Continuing execution...)"); loop { - match step_opcode(&mut context.acvm, &context.circuit.assert_messages, context.show_output, &mut context.foreign_call_executor)? { + match c.step_opcode()? { SolveResult::Done => break, SolveResult::Ok => {}, } @@ -209,7 +189,7 @@ fn continue_command(_args: ReplArgMatches, context: & Ok(Some("Ok".to_string())) } -fn quit_command(_args: ReplArgMatches, context: &mut ReplContext) -> Result, NargoError> { - show_current_vm_status(&mut context.acvm, &mut context.debug_artifact); +fn quit_command(_args: ReplArgMatches, context: &mut Rc>>) -> Result, NargoError> { + context.lock().unwrap().show_current_vm_status(); Err(NargoError::ExecutionError(ExecutionError::Halted)) } diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 03214dec9c3..f3a2fe90b67 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -22,7 +22,6 @@ mod new_cmd; mod prove_cmd; mod test_cmd; mod verify_cmd; -mod debug_cmd; const GIT_HASH: &str = env!("GIT_COMMIT"); const IS_DIRTY: &str = env!("GIT_DIRTY");