From e8c14d8c5d66b10bd033603b88eb3fb5c45fd415 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 20:11:43 +0200 Subject: [PATCH 1/8] Add `--breakpoints` flag (but it doesn't do anything yet) --- desktop/src/args.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/desktop/src/args.rs b/desktop/src/args.rs index b42840e..9384b07 100644 --- a/desktop/src/args.rs +++ b/desktop/src/args.rs @@ -1,6 +1,9 @@ +use std::path::PathBuf; + use minifb::Scale; use structopt::StructOpt; -use std::path::PathBuf; + +use mahboi::primitives::Word; #[derive(Debug, StructOpt)] @@ -25,6 +28,15 @@ pub(crate) struct Args { help = "Path to the ROM that should be loaded into the emulator.", )] pub(crate) path_to_rom: PathBuf, + + #[structopt( + long = "--breakpoints", + parse(try_from_str = "parse_breakpoint"), + requires = "debug", + help = "Breakpoint that is added to the debugger at the very beginning. Breakpoints are \ + specified in hexadecimal.", + )] + pub(crate) breakpoints: Vec, } fn parse_scale(src: &str) -> Result { @@ -39,3 +51,9 @@ fn parse_scale(src: &str) -> Result { _ => Err("only '1', '2', '4', '8', '16', '32' or 'fit' are allowed"), } } + +fn parse_breakpoint(src: &str) -> Result { + u16::from_str_radix(src, 16) + .map(Word::new) + .map_err(|e| format!("failed to parse breakpoint: {}", e)) +} From 96d5af8f77287bbd3946af5f00a2325750f9e5b5 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 20:19:46 +0200 Subject: [PATCH 2/8] Add and use breakpoints added via CLI --- desktop/src/debug/tui/mod.rs | 8 ++++++-- desktop/src/main.rs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/desktop/src/debug/tui/mod.rs b/desktop/src/debug/tui/mod.rs index 460efe3..841348f 100644 --- a/desktop/src/debug/tui/mod.rs +++ b/desktop/src/debug/tui/mod.rs @@ -181,6 +181,10 @@ impl TuiDebugger { Ok(out) } + pub(crate) fn breakpoints(&self) -> &Breakpoints { + &self.breakpoints + } + /// Updates the debugger view and handles events. Should be called /// regularly. /// @@ -554,7 +558,7 @@ impl TuiDebugger { /// easily usable from everywhere. Just `clone()` this to get another owned /// reference. #[derive(Clone)] -struct Breakpoints(Rc>>); +pub(crate) struct Breakpoints(Rc>>); impl Breakpoints { fn new() -> Self { @@ -563,7 +567,7 @@ impl Breakpoints { /// Add a breakpoint to the collection. If it's already inside, nothing /// happens. - fn add(&self, addr: Word) { + pub(crate) fn add(&self, addr: Word) { self.0.borrow_mut().insert(addr); } diff --git a/desktop/src/main.rs b/desktop/src/main.rs index 435a257..83952af 100644 --- a/desktop/src/main.rs +++ b/desktop/src/main.rs @@ -45,7 +45,12 @@ fn run() -> Result<(), Error> { // Create the TUI debugger if we're in debug mode. let mut tui_debugger = if args.debug { - Some(TuiDebugger::new()?) + let debugger = TuiDebugger::new()?; + for &bp in &args.breakpoints { + debugger.breakpoints().add(bp); + } + + Some(debugger) } else { None }; From 173d2929d96d7188f794aeb76e211e12beec632b Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 20:22:04 +0200 Subject: [PATCH 3/8] Add `--instant-start` CLI flag --- desktop/src/args.rs | 8 ++++++++ desktop/src/main.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/desktop/src/args.rs b/desktop/src/args.rs index 9384b07..1db6465 100644 --- a/desktop/src/args.rs +++ b/desktop/src/args.rs @@ -37,6 +37,14 @@ pub(crate) struct Args { specified in hexadecimal.", )] pub(crate) breakpoints: Vec, + + #[structopt( + long = "--instant-start", + requires = "debug", + help = "When starting in debugging mode, don't pause at the beginning, but start running \ + right ahead (particularly useful in combination with `--breakpoints`)", + )] + pub(crate) instant_start: bool, } fn parse_scale(src: &str) -> Result { diff --git a/desktop/src/main.rs b/desktop/src/main.rs index 83952af..d77b7a6 100644 --- a/desktop/src/main.rs +++ b/desktop/src/main.rs @@ -67,7 +67,7 @@ fn run() -> Result<(), Error> { let mut window = NativeWindow::open(&args).context("failed to open window")?; info!("Opened window"); - let mut is_paused = args.debug; + let mut is_paused = args.debug && !args.instant_start; while !window.should_stop() { // Update window buffer and read input. window.update()?; From c4878127a5026fd884f844f2e547e63dbac5c924 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 20:45:02 +0200 Subject: [PATCH 4/8] Make `opcode` macros public --- core/src/instr.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/instr.rs b/core/src/instr.rs index 20e3485..19adcdf 100644 --- a/core/src/instr.rs +++ b/core/src/instr.rs @@ -644,6 +644,7 @@ pub const PREFIXED_INSTRUCTIONS: InstrDb = InstrDb([ Instr::new(0xff, "SET 7, A", 2, 8, None), ]); +#[macro_export] macro_rules! opcode { ("NOP") => { 0x00 }; ("LD BC, d16") => { 0x01 }; @@ -907,6 +908,7 @@ macro_rules! opcode { ("RST 38H") => { 0xff }; } +#[macro_export] macro_rules! prefixed_opcode { ("RLC B") => { 0x00 }; ("RLC C") => { 0x01 }; From 4ad2b2cc59301aeaaa9352330ed1fe009e34c5bf Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 20:55:09 +0200 Subject: [PATCH 5/8] Add debugger feature: run until the end of the current function is reached --- desktop/src/debug/tui/mod.rs | 42 +++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/desktop/src/debug/tui/mod.rs b/desktop/src/debug/tui/mod.rs index 841348f..f5409ec 100644 --- a/desktop/src/debug/tui/mod.rs +++ b/desktop/src/debug/tui/mod.rs @@ -24,6 +24,7 @@ use lazy_static::lazy_static; use log::{Log, Record, Level, Metadata}; use mahboi::{ + opcode, log::*, machine::{Cpu, Machine}, primitives::Word, @@ -128,6 +129,10 @@ pub(crate) struct TuiDebugger { /// A set of addresses at which we will pause execution breakpoints: Breakpoints, + + /// Flag that is set when the user requested to run until the next RET + /// instruction. + pause_on_ret: bool, } impl TuiDebugger { @@ -173,6 +178,7 @@ impl TuiDebugger { event_sink, step_over: None, breakpoints: Breakpoints::new(), + pause_on_ret: false, }; // Build the TUI view @@ -247,6 +253,14 @@ impl TuiDebugger { return Ok(Action::Continue); } } + 'f' => { + if self.pause_mode { + self.step_over = Some(machine.cpu.pc); + self.pause_on_ret = true; + self.resume(); + return Ok(Action::Continue); + } + } _ => panic!("internal error: unexpected event"), } } @@ -308,6 +322,25 @@ impl TuiDebugger { return true; } + // If we are supposed to pause on a RET instruction... + if self.pause_on_ret { + // ... check if the next instruction is an RET-like instruction + let opcode = machine.load_byte(machine.cpu.pc); + match opcode.get() { + opcode!("RET") + | opcode!("RETI") + | opcode!("RET NZ") + | opcode!("RET NC") + | opcode!("RET Z") + | opcode!("RET C") => { + // Reset the flag + self.pause_on_ret = false; + return true; + } + _ => {} + } + } + false } @@ -319,7 +352,7 @@ impl TuiDebugger { // Other global events are just forwarded to be handled in the next // `update()` call. - for &c in &['p', 'r', 's'] { + for &c in &['p', 'r', 's', 'f'] { let tx = self.event_sink.clone(); self.siv.add_global_callback(c, move |_| tx.send(c).unwrap()); } @@ -448,17 +481,20 @@ impl TuiDebugger { }) }; - // Buttons for the 'r' and 's' actions + // Buttons for the 'r', 's' and 'f' actions let tx = self.event_sink.clone(); let run_button = Button::new("Continue [r]", move |_| tx.send('r').unwrap()); let tx = self.event_sink.clone(); let step_button = Button::new("Single step [s]", move |_| tx.send('s').unwrap()); + let tx = self.event_sink.clone(); + let fun_end_button = Button::new("Run to RET-like [f]", move |_| tx.send('f').unwrap()); // Wrap all buttons let debug_buttons = LinearLayout::vertical() .child(button_breakpoints) .child(run_button) - .child(step_button); + .child(step_button) + .child(fun_end_button); let debug_buttons = Dialog::around(debug_buttons).title("Actions"); // Build the complete right side From 264047501a168b7e44569a56b4fdfcc143bb68a4 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 21:00:03 +0200 Subject: [PATCH 6/8] Pass args to debugger instead of configure it in main() --- desktop/src/debug/tui/mod.rs | 12 +++++++----- desktop/src/main.rs | 7 +------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/desktop/src/debug/tui/mod.rs b/desktop/src/debug/tui/mod.rs index f5409ec..7858c29 100644 --- a/desktop/src/debug/tui/mod.rs +++ b/desktop/src/debug/tui/mod.rs @@ -29,6 +29,7 @@ use mahboi::{ machine::{Cpu, Machine}, primitives::Word, }; +use crate::args::Args; use super::{Action}; use self::{ asm_view::AsmView, @@ -136,7 +137,7 @@ pub(crate) struct TuiDebugger { } impl TuiDebugger { - pub(crate) fn new() -> Result { + pub(crate) fn new(args: &Args) -> Result { // Create a handle to the terminal (with the correct backend). let mut siv = Cursive::ncurses(); @@ -181,16 +182,17 @@ impl TuiDebugger { pause_on_ret: false, }; + // Add all breakpoints specified by CLI + for &bp in &args.breakpoints { + out.breakpoints.add(bp); + } + // Build the TUI view out.setup_tui(); Ok(out) } - pub(crate) fn breakpoints(&self) -> &Breakpoints { - &self.breakpoints - } - /// Updates the debugger view and handles events. Should be called /// regularly. /// diff --git a/desktop/src/main.rs b/desktop/src/main.rs index d77b7a6..d54c8a0 100644 --- a/desktop/src/main.rs +++ b/desktop/src/main.rs @@ -45,12 +45,7 @@ fn run() -> Result<(), Error> { // Create the TUI debugger if we're in debug mode. let mut tui_debugger = if args.debug { - let debugger = TuiDebugger::new()?; - for &bp in &args.breakpoints { - debugger.breakpoints().add(bp); - } - - Some(debugger) + Some(TuiDebugger::new(&args)?) } else { None }; From 1395c97ea5794b66c73b3b425e1071ce781bdc5a Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 25 Aug 2018 21:24:38 +0200 Subject: [PATCH 7/8] Add box showing the stack's content --- desktop/src/debug/tui/mod.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/desktop/src/debug/tui/mod.rs b/desktop/src/debug/tui/mod.rs index 7858c29..70a8b29 100644 --- a/desktop/src/debug/tui/mod.rs +++ b/desktop/src/debug/tui/mod.rs @@ -216,6 +216,7 @@ impl TuiDebugger { if is_paused { self.siv.find_id::("asm_view").unwrap().update(machine); self.update_cpu_data(&machine.cpu); + self.update_stack_data(machine); } // Append all log messages that were pushed to the global buffer into @@ -405,6 +406,31 @@ impl TuiDebugger { ) } + fn update_stack_data(&mut self, machine: &Machine) { + let mut body = StyledString::new(); + + let start = machine.cpu.sp.get(); + let end = start.saturating_add(10); + + for addr in start..end { + let addr = Word::new(addr); + body.append_styled(addr.to_string(), Color::Light(BaseColor::Blue)); + body.append_styled(" │ ", Color::Light(BaseColor::Blue)); + body.append_styled( + machine.load_byte(addr).to_string(), + Color::Dark(BaseColor::Yellow), + ); + + if addr == start { + body.append_plain(" ← SP"); + } + + body.append_plain("\n"); + } + + self.siv.find_id::("stack_view").unwrap().set_content(body); + } + fn update_cpu_data(&mut self, cpu: &Cpu) { let reg_style = Color::Light(BaseColor::Magenta); @@ -474,6 +500,8 @@ impl TuiDebugger { let cpu_body = TextView::new("no data yet").center().with_id("cpu_data"); let cpu_view = Dialog::around(cpu_body).title("CPU registers"); + let stack_body = TextView::new("no data yet").with_id("stack_view"); + let stack_view = Dialog::around(stack_body).title("Stack"); // Setup Buttons let button_breakpoints = { @@ -503,6 +531,8 @@ impl TuiDebugger { let right_panel = LinearLayout::vertical() .child(cpu_view) .child(DummyView) + .child(stack_view) + .child(DummyView) .child(debug_buttons) .fixed_width(30); From b256fe8611a10bdf8f61801409be4f85404757d7 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sun, 26 Aug 2018 00:52:11 +0200 Subject: [PATCH 8/8] Add more explanation for the `--breakpoints` flag --- desktop/src/args.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/desktop/src/args.rs b/desktop/src/args.rs index 1db6465..ab6a680 100644 --- a/desktop/src/args.rs +++ b/desktop/src/args.rs @@ -34,7 +34,9 @@ pub(crate) struct Args { parse(try_from_str = "parse_breakpoint"), requires = "debug", help = "Breakpoint that is added to the debugger at the very beginning. Breakpoints are \ - specified in hexadecimal.", + specified in hexadecimal. To add multiple breakpoints, you can either list them after \ + one `--breakpoints` flag or specify `--breakpoints` multiple times. Example: \ + `--breakpoints 23 FF --breakpoints 10B`.", )] pub(crate) breakpoints: Vec, @@ -63,5 +65,9 @@ fn parse_scale(src: &str) -> Result { fn parse_breakpoint(src: &str) -> Result { u16::from_str_radix(src, 16) .map(Word::new) - .map_err(|e| format!("failed to parse breakpoint: {}", e)) + .map_err(|e| format!( + "failed to parse breakpoint: {} (values like '1f' are valid -- no \ + leading `0x`!)", + e, + )) }