Add initial debuging capabilities to the VM. v1 PROPOSED

jpl: 1
 Add initial debuging capabilities to the VM.

 2 files changed, 51 insertions(+), 4 deletions(-)

[PATCH] Add initial debuging capabilities to the VM. Export this patch

Still need to add debugging in instruction
execution, and perhaps add a better VM
constructor for debugging, that can still
allow for toggling the debug mode in the
middle of execution.
---

Debugging inside the CPU would involve passing around the debug flag
and the stderr lock into the CPU module, which seems simple enough,
but might get unhandy, especially since StderrLock has an associated
lifetime, which might propagate some complexity.

tmpod <tom@tmpod.dev>

2 years ago

Well, that ain't great, but I'm not seeing a way around it. At least not an
immediate one.

Next

I'm not sure if there's a better way of handling it as of now,
but if you've got any debug design you'd rather implement, I'm all 
ears.

tmpod <tom@tmpod.dev>

2 years ago

I'm not exactly pursuing raw performance with this VM, but I somewhat worry
about the implications of the extra function call and branching on every
small operation. Perhaps we could elide debugging in release builds?

Next

 vm/src/main.rs | 11 ++++++++++-
 vm/src/vm.rs   | 44 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/vm/src/main.rs b/vm/src/main.rs
index e4dd0ba..0984ac0 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -18,6 +18,10 @@ struct Args {
     /// Image name
     #[clap()]
     image: String,
+
+    /// Debug flag
+    #[clap(short, long)]
+    debug: bool,
 }
 
 fn main() -> std::io::Result<()> {
@@ -26,7 +30,12 @@ fn main() -> std::io::Result<()> {
     let image_file = args.image;
     let image = Image::from_file(image_file);
 
-    let mut vm = VM::new();
+    let mut vm = if args.debug {
+        VM::with_debug()
+    } else {
+        VM::new()
+    };

tmpod <tom@tmpod.dev>

2 years ago

Mmmm, I think I'd rather do `VM::new(args.debug)` than this. 
Just have the func signature be `pub new(debug: bool)` and *maybe* have a
`pub with_debug()` which just aliases to `new(true)`? Not sure.

Next

+
     vm.load_image(&image);
     vm.run()?;
 
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
index e5d7c93..710f54f 100644
--- a/vm/src/vm.rs
+++ b/vm/src/vm.rs
@@ -1,51 +1,89 @@
-use crossterm::terminal;
+use std::io::Write;
+
+use crossterm::{terminal, Result};
 use toasty_lc3_primitives::{registers::Register, types::Word};
 
 use crate::{cpu::CPU, image::Image, memory::Memory};
 // use crate::keyboard::Keyboard;
 
-pub struct VM {
+pub struct VM<'a> {
     pub cpu: CPU,
     pub memory: Memory,
     // TODO: improve interface
     pub screen: std::io::Stdout,
     // pub keyboard: Keyboard,
+    debug: bool,
+    stderr_lock: std::io::StderrLock<'a>,
 }
 
-impl VM {
+impl<'a> VM<'a> {
     pub fn new() -> Self {
         Self {
             cpu: CPU::new(),
             memory: Memory::new(),
             screen: std::io::stdout(),
+            debug: false,
+            stderr_lock: std::io::stderr().lock(),
+        }
+    }
+
+    pub fn with_debug() -> Self {
+        Self {
+            cpu: CPU::new(),
+            memory: Memory::new(),
+            screen: std::io::stdout(),
+            debug: true,
+            stderr_lock: std::io::stderr().lock(),
         }
     }
 
     pub fn load_image(&mut self, image: &Image) {
+        // TODO: Handle result
+        self.debug(b"Loading image\n");
         self.memory.load_image(&image);
         self.cpu.registers[Register::PC] = image.origin as Word;
     }
 
     pub fn boot_up(&mut self) -> crossterm::Result<()> {
+        self.debug(b"TERMINAL: Enabling raw mode\n")?;
         terminal::enable_raw_mode()?;
+
         crossterm::execute!(
             self.screen,
             terminal::EnterAlternateScreen,
             terminal::Clear(terminal::ClearType::All)
         )?;
+
         Ok(())
     }
 
     pub fn shut_down(&mut self) -> crossterm::Result<()> {
+        self.debug(b"TERMINAL: Disabling raw mode\n")?;
         terminal::disable_raw_mode()?;
+
         crossterm::execute!(self.screen, terminal::LeaveAlternateScreen)?;
+
         Ok(())
     }
 
     pub fn run(&mut self) -> crossterm::Result<()> {
+        self.debug(b"VM: Booting up\n")?;
         self.boot_up()?;
+
+        self.debug(b"CPU: Starting execution\n")?;
         self.cpu.run(&mut self.memory, &mut self.screen)?;
+
+        self.debug(b"VM: Shutting down\n")?;
         self.shut_down()?;
+
         Ok(())
     }
+
+    fn debug(&mut self, buf: &[u8]) -> Result<()> {

tmpod <tom@tmpod.dev>

2 years ago

You probably didn't want that, but that `Result` over there actually comes
from `crossterm`. I assume your LSP imported it for you or something.
You should be using `std::io::Result`, and revert the import back to `use
crossterm::terminal`.

+        if self.debug {
+            self.stderr_lock.write_all(buf)
+        } else {
+            Ok(())
+        }
+    }
 }
-- 
2.37.1