From d5d87fe72d5efe33d6e862838211952a76f620d1 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 29 Jan 2025 17:36:05 -0500 Subject: [PATCH 1/2] add raw-source-printing flag to debugger to disable highlighting, add test of expected trace when stepping through test that was failing in #7195 --- .../regression_7195/Nargo.toml | 6 + .../regression_7195/Prover.toml | 2 + .../regression_7195/src/main.nr | 13 +++ tooling/debugger/src/lib.rs | 3 +- tooling/debugger/src/repl.rs | 15 ++- tooling/debugger/src/source_code_printer.rs | 32 +++++- tooling/debugger/tests/debug.rs | 108 ++++++++++++++++++ tooling/nargo_cli/src/cli/debug_cmd.rs | 20 +++- 8 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 test_programs/execution_success/regression_7195/Nargo.toml create mode 100644 test_programs/execution_success/regression_7195/Prover.toml create mode 100644 test_programs/execution_success/regression_7195/src/main.nr diff --git a/test_programs/execution_success/regression_7195/Nargo.toml b/test_programs/execution_success/regression_7195/Nargo.toml new file mode 100644 index 00000000000..d9ef0fa7388 --- /dev/null +++ b/test_programs/execution_success/regression_7195/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_7195" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/regression_7195/Prover.toml b/test_programs/execution_success/regression_7195/Prover.toml new file mode 100644 index 00000000000..8c12ebba6cf --- /dev/null +++ b/test_programs/execution_success/regression_7195/Prover.toml @@ -0,0 +1,2 @@ +x = "1" +y = "2" diff --git a/test_programs/execution_success/regression_7195/src/main.nr b/test_programs/execution_success/regression_7195/src/main.nr new file mode 100644 index 00000000000..a7ddca6a0d6 --- /dev/null +++ b/test_programs/execution_success/regression_7195/src/main.nr @@ -0,0 +1,13 @@ +fn bar(y: Field) { + assert(y != 0); +} + +fn foo(x: Field) { + bar(x) +} + +fn main(x: Field, y: pub Field) { + foo(x); + foo(y); + assert(x != y); +} diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 37ac088ca35..f0dc859beb3 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -19,8 +19,9 @@ pub fn run_repl_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + raw_source_printing: bool, ) -> Result>, NargoError> { - repl::run(solver, program, initial_witness) + repl::run(solver, program, initial_witness, raw_source_printing) } pub fn run_dap_loop>( diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index eda3cbfd895..cc44af9f6cc 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -32,6 +32,9 @@ pub struct ReplDebugger<'a, B: BlackBoxFunctionSolver> { // Brillig functions referenced from the ACIR circuits above unconstrained_functions: &'a [BrilligBytecode], + + // whether to print the source without highlighting / pretty-printing + raw_source_printing: bool, } impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { @@ -41,6 +44,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], + raw_source_printing: bool, ) -> Self { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact( PrintOutput::Stdout, @@ -68,6 +72,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { initial_witness, last_result, unconstrained_functions, + raw_source_printing, } } @@ -97,7 +102,11 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_debug_location(&location); - print_source_code_location(self.debug_artifact, &locations); + print_source_code_location( + self.debug_artifact, + &locations, + self.raw_source_printing, + ); } } } @@ -125,7 +134,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { } } let locations = self.context.get_source_location_for_debug_location(debug_location); - print_source_code_location(self.debug_artifact, &locations); + print_source_code_location(self.debug_artifact, &locations, self.raw_source_printing); } pub fn show_current_call_stack(&self) { @@ -427,6 +436,7 @@ pub fn run>( blackbox_solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + raw_source_printing: bool, ) -> Result>, NargoError> { let circuits = &program.program.functions; let debug_artifact = @@ -438,6 +448,7 @@ pub fn run>( debug_artifact, initial_witness, unconstrained_functions, + raw_source_printing, )); let ref_context = &context; diff --git a/tooling/debugger/src/source_code_printer.rs b/tooling/debugger/src/source_code_printer.rs index b3682c9016f..b567a217911 100644 --- a/tooling/debugger/src/source_code_printer.rs +++ b/tooling/debugger/src/source_code_printer.rs @@ -30,7 +30,11 @@ struct LocationPrintContext { // Given a DebugArtifact and an OpcodeLocation, prints all the source code // locations the OpcodeLocation maps to, with some surrounding context and // visual aids to highlight the location itself. -pub(super) fn print_source_code_location(debug_artifact: &DebugArtifact, locations: &[Location]) { +pub(super) fn print_source_code_location( + debug_artifact: &DebugArtifact, + locations: &[Location], + raw_source_printing: bool, +) { let locations = locations.iter(); for loc in locations { @@ -41,9 +45,11 @@ pub(super) fn print_source_code_location(debug_artifact: &DebugArtifact, locatio for line in lines { match line { PrintedLine::Skip => {} - PrintedLine::Ellipsis { line_number } => print_ellipsis(line_number), + PrintedLine::Ellipsis { line_number } => { + print_ellipsis(line_number, raw_source_printing) + } PrintedLine::Content { line_number, cursor, content, highlight } => { - print_content(line_number, cursor, content, highlight) + print_content(line_number, cursor, content, highlight, raw_source_printing) } } } @@ -57,11 +63,27 @@ fn print_location_path(debug_artifact: &DebugArtifact, loc: Location) { println!("At {}:{line_number}:{column_number}", debug_artifact.name(loc.file).unwrap()); } -fn print_ellipsis(line_number: usize) { +fn print_ellipsis(line_number: usize, raw_source_printing: bool) { + if raw_source_printing { + println!("..."); + return; + } + println!("{:>3} {:2} {}", line_number.dimmed(), "", "...".dimmed()); } -fn print_content(line_number: usize, cursor: &str, content: &str, highlight: Option>) { +fn print_content( + line_number: usize, + cursor: &str, + content: &str, + highlight: Option>, + raw_source_printing: bool, +) { + if raw_source_printing { + println!("{}", content); + return; + } + match highlight { Some(highlight) => { println!( diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index eb43cf9cc6d..303fb4b94f0 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -49,4 +49,112 @@ mod tests { // Exit the bash session. dbg_session.send_line("exit").expect("Failed to quit bash session"); } + + #[test] + fn debugger_expected_call_stack() { + let nargo_bin = + cargo_bin("nargo").into_os_string().into_string().expect("Cannot parse nargo path"); + + let timeout_seconds = 30; + let mut dbg_session = + spawn_bash(Some(timeout_seconds * 1000)).expect("Could not start bash session"); + + let test_program_path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../test_programs/execution_success/regression_7195"); + let test_program_dir = test_program_path.display(); + + // Start debugger and test that it loads for the given program. + dbg_session + .execute( + &format!( + "{nargo_bin} debug --raw-source-printing true --program-dir {test_program_dir} --force-brillig --expression-width 3" + ), + ".*\\Starting debugger.*", + ) + .expect("Could not start debugger"); + + for step_number in 0..8 { + // While running the debugger, issue a "next" cmd, + // which should run to the program to the next source line given + // we haven't set any breakpoints. + // ">" is the debugger's prompt, so finding one + // after running "next" indicates that the + // debugger has not panicked for this step. + dbg_session + .send_line("next") + .expect("Debugger panicked while attempting to step through program."); + dbg_session + .exp_string(">") + .expect("Failed while waiting for debugger to step through program."); + + if step_number == 7 { + let mut lines = vec![]; + let mut read_line = dbg_session.read_line(); + while read_line.is_ok() { + if let Ok(line) = read_line { + // only keep the results after the most recent "next" command + if line.contains("> next") { + lines = vec![]; + } else { + lines.push(line); + } + } + read_line = dbg_session.read_line(); + } + + let lines_expected_to_contain: Vec<&str> = vec![ + "At opcode ", + "At ", + "...", + " bar(x)", + "}", + "", + "fn main(x: Field, y: pub Field) {", + " foo(x);", + " foo(y);", + " assert(x != y);", + "}", + "At ", + "fn bar(y: Field) {", + " assert(y != 0);", + "}", + "", + "fn foo(x: Field) {", + " bar(x)", + "}", + "", + "fn main(x: Field, y: pub Field) {", + " foo(x);", + "...", + "At ", + "fn bar(y: Field) {", + " assert(y != 0);", + "}", + "", + "fn foo(x: Field) {", + " bar(x)", + "...", + ]; + + for (line, line_expected_to_contain) in + lines.into_iter().zip(lines_expected_to_contain) + { + let ascii_line: String = line.chars().filter(char::is_ascii).collect(); + let line_expected_to_contain = line_expected_to_contain.trim_start(); + assert!( + ascii_line.contains(&line_expected_to_contain), + "{:?}\ndid not contain\n{:?}", + ascii_line, + line_expected_to_contain, + ); + } + } + } + + // Run the "quit" command + dbg_session.send_line("quit").expect("Failed to quit debugger"); + + // Exit the bash session. + dbg_session.send_line("exit").expect("Failed to quit bash session"); + } } diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 2ed30639d32..090bbea8f10 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -50,6 +50,10 @@ pub(crate) struct DebugCommand { /// Disable vars debug instrumentation (enabled by default) #[clap(long)] skip_instrumentation: Option, + + /// Raw string printing of source for testing + #[clap(long, hide = true, default_value = "false")] + raw_source_printing: Option, } pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliError> { @@ -92,6 +96,7 @@ pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliErro &args.witness_name, target_dir, args.compile_options.pedantic_solving, + args.raw_source_printing.unwrap_or(false), ) } @@ -180,14 +185,20 @@ fn run_async( witness_name: &Option, target_dir: &PathBuf, pedantic_solving: bool, + raw_source_printing: bool, ) -> Result<(), CliError> { use tokio::runtime::Builder; let runtime = Builder::new_current_thread().enable_all().build().unwrap(); runtime.block_on(async { println!("[{}] Starting debugger", package.name); - let (return_value, witness_stack) = - debug_program_and_decode(program, package, prover_name, pedantic_solving)?; + let (return_value, witness_stack) = debug_program_and_decode( + program, + package, + prover_name, + pedantic_solving, + raw_source_printing, + )?; if let Some(solved_witness_stack) = witness_stack { println!("[{}] Circuit witness successfully solved", package.name); @@ -215,12 +226,13 @@ fn debug_program_and_decode( package: &Package, prover_name: &str, pedantic_solving: bool, + raw_source_printing: bool, ) -> Result<(Option, Option>), CliError> { // Parse the initial witness values from Prover.toml let (inputs_map, _) = read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; let program_abi = program.abi.clone(); - let witness_stack = debug_program(program, &inputs_map, pedantic_solving)?; + let witness_stack = debug_program(program, &inputs_map, pedantic_solving, raw_source_printing)?; match witness_stack { Some(witness_stack) => { @@ -239,6 +251,7 @@ pub(crate) fn debug_program( compiled_program: CompiledProgram, inputs_map: &InputMap, pedantic_solving: bool, + raw_source_printing: bool, ) -> Result>, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; @@ -246,6 +259,7 @@ pub(crate) fn debug_program( &Bn254BlackBoxSolver(pedantic_solving), compiled_program, initial_witness, + raw_source_printing, ) .map_err(CliError::from) } From 69313d97e4deaa8b9e97156aae756a4b4782c269 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 29 Jan 2025 17:53:06 -0500 Subject: [PATCH 2/2] cargo clippy --- tooling/debugger/tests/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/debugger/tests/debug.rs b/tooling/debugger/tests/debug.rs index 303fb4b94f0..d4c883380b3 100644 --- a/tooling/debugger/tests/debug.rs +++ b/tooling/debugger/tests/debug.rs @@ -142,7 +142,7 @@ mod tests { let ascii_line: String = line.chars().filter(char::is_ascii).collect(); let line_expected_to_contain = line_expected_to_contain.trim_start(); assert!( - ascii_line.contains(&line_expected_to_contain), + ascii_line.contains(line_expected_to_contain), "{:?}\ndid not contain\n{:?}", ascii_line, line_expected_to_contain,