From 2e328454054a7c90b8b762b7c9ff0823eb0997c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Tue, 6 Feb 2024 18:11:42 -0500 Subject: [PATCH] fix: Prevent debugger crashing on circuits with no opcodes (#4283) # Description ## Problem\* Resolves #4231 The REPL debugger is not handling the case when the compiled circuit has no opcodes and since it assumes there will be some opcodes, it panics on continue. ## Summary\* With this PR, the REPL debugger will start in a "finished" stated in those cases and the DAP will not start the main loop. ## Additional Context This edge case is very unlikely to happen, especially after applying #4185 which changes the default mode for the debugger to Brillig (seems to generate at least a trampoline-like fragment even for empty programs) and after the introduction of debugging instrumentation. Nevertheless it will still be possible (eg. empty program and passing `--acir-mode` option). ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- tooling/debugger/src/dap.rs | 4 ++-- tooling/debugger/src/repl.rs | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index 2de7dac77d8..3d135abe1cd 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -125,9 +125,9 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession<'a, R, W, B> { } pub fn run_loop(&mut self) -> Result<(), ServerError> { - self.running = true; + self.running = self.context.get_current_opcode_location().is_some(); - if matches!(self.context.get_current_source_location(), None) { + if self.running && matches!(self.context.get_current_source_location(), None) { // TODO: remove this? This is to ensure that the tool has a proper // source location to show when first starting the debugger, but // maybe the default behavior should be to start executing until the diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 1ba5391b0cf..17be7c1a792 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -38,14 +38,13 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { initial_witness.clone(), foreign_call_executor, ); - Self { - context, - blackbox_solver, - circuit, - debug_artifact, - initial_witness, - last_result: DebugCommandResult::Ok, - } + let last_result = if context.get_current_opcode_location().is_none() { + // handle circuit with no opcodes + DebugCommandResult::Done + } else { + DebugCommandResult::Ok + }; + Self { context, blackbox_solver, circuit, debug_artifact, initial_witness, last_result } } pub fn show_current_vm_status(&self) {