Rewrite GdbStub::run to use the state machine implementation (and more!) (#88)

This is a set of API changes I've been sitting on for many months now, and I think it's high-time that I pushed these changes up.

The docs are still a mess, but I've opted to commit things as is to get hands-on feedback from folks to make sure things work, and then I'll tackle the documentation later.

At a high level, this PR introduces the following API tweaks:

#### Removed the `read` and `peek` methods from `Connection`, moving them into a separate `ConnectionExt` trait.

The state machine API was explicitly designed to _circumvent_ the need for a blocking `read` API. Nonetheless, it is useful to have a reference implementation of a blocking single-byte read for various types (e.g: TcpStream) built-in to `gdbstub`, so I've split these implementations off into a separate `ConnectionExt` trait - which as the name implies, requires the type to also implement `Connection`. This new trait is used as part of the `GdbStub::run_blocking` API described below...

#### `Target::resume` is now _non blocking_, and does not support immediately returning a `StopReason`

The previous iteration of the API kept the ability to return a `StopReason` from `Target::resume` in order to maintain compatibility with existing blocking `Target::resume` implementations (such as the one in the `armv4t` example).

With this new API, targets are now _forced_ to resume in a non-blocking manner, with the responsibility of "driving" the resumed target lifted up into the state machine event loop.

This change also means that `Target::resume` no longer takes a `GdbInterrupt` callback, which has simplified a _ton_ of code within `gdbstub`!

#### The bespoke blocking `GdbStub::run` implementation has been swapped out with a new `GdbStub::run_blocking` method - backed by the state machine API.

Instead of maintaining two parallel "front-ends" to `GdbStub`, I've consolidated the two to both use the `GdbStubStateMachine` API under the hood. This new API is _looks_ more complex to the old `GdbStub::run` API, as it requires the user to provide an implementation of `gdbstub_run_blocking::BlockingEventLoop` for their particular `Target` + `Connection`, but in reality, all this new API has done is "lift" most of the code that used to live in `Target::resume` up into `gdbstub_run_blocking::BlockingEventLoop`.

Author: daniel5151

README.md

@@ -28,7 +28,7 @@ Why use `gdbstub`?
 -   **`#![no_std]` Ready & Size Optimized**
     -   `gdbstub` is a **`no_std` first** library, whereby all protocol features are required to be `no_std` compatible.
     -   `gdbstub` does not require _any_ dynamic memory allocation, and can be configured to use fixed-size, pre-allocated buffers. This enables `gdbstub` to be used on even the most resource constrained, no-[`alloc`](https://doc.rust-lang.org/alloc/) platforms.
-    -   `gdbstub` is entirely **panic free** (when compiled in release mode, without the `paranoid_unsafe` cargo feature).
+    -   `gdbstub` is entirely **panic free** in most minimal configurations (when compiled in release mode, without the `paranoid_unsafe` cargo feature).
         -   Validated by inspecting the asm output of the in-tree `example_no_std`.
     -   `gdbstub` is transport-layer agnostic, and uses a basic [`Connection`](https://docs.rs/gdbstub/latest/gdbstub/trait.Connection.html) interface to communicate with the GDB server. As long as target has some method of performing in-order, serial, byte-wise I/O (e.g: putchar/getchar over UART), it's possible to run `gdbstub` on it!
     -   "You don't pay for what you don't use": All code related to parsing/handling protocol extensions is guaranteed to be dead-code-eliminated from an optimized binary if left unimplemented! See the [Zero-overhead Protocol Extensions](#zero-overhead-protocol-extensions) section below for more details.

example_no_std/src/conn.rs

@@ -50,6 +50,24 @@ impl TcpConnection {
             Ok(buf[0])
         }
     }
+
+    #[allow(dead_code)]
+    pub fn peek(&mut self) -> Result<Option<u8>, &'static str> {
+        let mut buf = [0];
+        let ret = unsafe {
+            libc::recv(
+                self.fd,
+                buf.as_mut_ptr() as *mut _,
+                buf.len(),
+                libc::MSG_PEEK,
+            )
+        };
+        if ret == -1 || ret != 1 {
+            Err("socket peek failed")
+        } else {
+            Ok(Some(buf[0]))
+        }
+    }
 }
 
 impl Drop for TcpConnection {
@@ -74,23 +92,6 @@ impl Connection for TcpConnection {
         }
     }
 
-    fn peek(&mut self) -> Result<Option<u8>, &'static str> {
-        let mut buf = [0];
-        let ret = unsafe {
-            libc::recv(
-                self.fd,
-                buf.as_mut_ptr() as *mut _,
-                buf.len(),
-                libc::MSG_PEEK,
-            )
-        };
-        if ret == -1 || ret != 1 {
-            Err("socket peek failed")
-        } else {
-            Ok(Some(buf[0]))
-        }
-    }
-
     fn flush(&mut self) -> Result<(), &'static str> {
         // huh, apparently flushing isn't a "thing" for Tcp streams.
         // see https://doc.rust-lang.org/src/std/net/tcp.rs.html#592-609

example_no_std/src/gdb.rs

@@ -1,8 +1,6 @@
 use gdbstub::common::Tid;
 use gdbstub::target;
-use gdbstub::target::ext::base::multithread::{
-    GdbInterrupt, MultiThreadOps, ResumeAction, ThreadStopReason,
-};
+use gdbstub::target::ext::base::multithread::{MultiThreadOps, ResumeAction};
 use gdbstub::target::{Target, TargetResult};
 
 use crate::print_str::print_str;
@@ -42,13 +40,9 @@ impl Target for DummyTarget {
 
 impl MultiThreadOps for DummyTarget {
     #[inline(never)]
-    fn resume(
-        &mut self,
-        _default_resume_action: ResumeAction,
-        _check_gdb_interrupt: GdbInterrupt<'_>,
-    ) -> Result<Option<ThreadStopReason<u32>>, Self::Error> {
+    fn resume(&mut self) -> Result<(), Self::Error> {
         print_str("> resume");
-        Ok(Some(ThreadStopReason::DoneStep))
+        Ok(())
     }
 
     #[inline(never)]

example_no_std/src/main.rs

@@ -4,6 +4,7 @@
 extern crate libc;
 
 use gdbstub::state_machine::GdbStubStateMachine;
+use gdbstub::target::ext::base::multithread::ThreadStopReason;
 use gdbstub::{DisconnectReason, GdbStubBuilder, GdbStubError};
 
 mod conn;
@@ -18,8 +19,6 @@ fn panic(_info: &core::panic::PanicInfo) -> ! {
 }
 
 fn rust_main() -> Result<(), i32> {
-    // pretty_env_logger::init();
-
     let mut target = gdb::DummyTarget::new();
 
     let conn = match conn::TcpConnection::new_localhost(9001) {
@@ -41,34 +40,39 @@ fn rust_main() -> Result<(), i32> {
 
     let mut gdb = gdb.run_state_machine().map_err(|_| 1)?;
 
-    loop {
+    let res = loop {
         gdb = match gdb {
             GdbStubStateMachine::Pump(mut gdb) => {
                 let byte = gdb.borrow_conn().read().map_err(|_| 1)?;
                 match gdb.pump(&mut target, byte) {
-                    Ok((_, Some(disconnect_reason))) => {
-                        match disconnect_reason {
-                            DisconnectReason::Disconnect => print_str("GDB Disconnected"),
-                            DisconnectReason::TargetExited(_) => print_str("Target exited"),
-                            DisconnectReason::TargetTerminated(_) => print_str("Target halted"),
-                            DisconnectReason::Kill => print_str("GDB sent a kill command"),
-                        }
-                        break;
-                    }
+                    Ok((_, Some(disconnect_reason))) => break Ok(disconnect_reason),
+                    Ok((gdb, None)) => gdb,
+                    Err(e) => break Err(e),
+                }
+            }
+
+            GdbStubStateMachine::DeferredStopReason(gdb) => {
+                match gdb.deferred_stop_reason(&mut target, ThreadStopReason::DoneStep) {
+                    Ok((_, Some(disconnect_reason))) => break Ok(disconnect_reason),
                     Ok((gdb, None)) => gdb,
-                    Err(GdbStubError::TargetError(_e)) => {
-                        print_str("Target raised a fatal error");
-                        break;
-                    }
-                    Err(_e) => {
-                        print_str("gdbstub internal error");
-                        break;
-                    }
+                    Err(e) => break Err(e),
                 }
             }
+        }
+    };
 
-            // example_no_std stubs out resume, so this will never happen
-            GdbStubStateMachine::DeferredStopReason(_) => return Err(-1),
+    match res {
+        Ok(disconnect_reason) => match disconnect_reason {
+            DisconnectReason::Disconnect => print_str("GDB Disconnected"),
+            DisconnectReason::TargetExited(_) => print_str("Target exited"),
+            DisconnectReason::TargetTerminated(_) => print_str("Target halted"),
+            DisconnectReason::Kill => print_str("GDB sent a kill command"),
+        },
+        Err(GdbStubError::TargetError(_e)) => {
+            print_str("Target raised a fatal error");
+        }
+        Err(_e) => {
+            print_str("gdbstub internal error");
         }
     }
 

examples/armv4t/emu.rs

@@ -7,19 +7,28 @@ const HLE_RETURN_ADDR: u32 = 0x12345678;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum Event {
+    DoneStep,
     Halted,
     Break,
     WatchWrite(u32),
     WatchRead(u32),
 }
 
+pub enum ExecMode {
+    Step,
+    Continue,
+    RangeStep(u32, u32),
+}
+
 /// incredibly barebones armv4t-based emulator
 pub struct Emu {
     start_addr: u32,
 
     // example custom register. only read/written to from the GDB client
     pub(crate) custom_reg: u32,
 
+    pub(crate) exec_mode: ExecMode,
+
     pub(crate) cpu: Cpu,
     pub(crate) mem: ExampleMem,
 
@@ -68,6 +77,8 @@ impl Emu {
 
             custom_reg: 0x12345678,
 
+            exec_mode: ExecMode::Continue,
+
             cpu,
             mem,
 
@@ -84,6 +95,7 @@ impl Emu {
         self.cpu.reg_set(Mode::User, reg::CPSR, 0x10);
     }
 
+    /// single-step the interpreter
     pub fn step(&mut self) -> Option<Event> {
         let mut hit_watchpoint = None;
 
@@ -114,4 +126,57 @@ impl Emu {
 
         None
     }
+
+    /// run the emulator in accordance with the currently set `ExecutionMode`.
+    ///
+    /// since the emulator runs in the same thread as the GDB loop, the emulator
+    /// will use the provided callback to poll the connection for incoming data
+    /// every 1024 steps.
+    pub fn run(&mut self, mut poll_incoming_data: impl FnMut() -> bool) -> RunEvent {
+        match self.exec_mode {
+            ExecMode::Step => RunEvent::Event(self.step().unwrap_or(Event::DoneStep)),
+            ExecMode::Continue => {
+                let mut cycles = 0;
+                loop {
+                    if cycles % 1024 == 0 {
+                        // poll for incoming data
+                        if poll_incoming_data() {
+                            break RunEvent::IncomingData;
+                        }
+                    }
+                    cycles += 1;
+
+                    if let Some(event) = self.step() {
+                        break RunEvent::Event(event);
+                    };
+                }
+            }
+            // just continue, but with an extra PC check
+            ExecMode::RangeStep(start, end) => {
+                let mut cycles = 0;
+                loop {
+                    if cycles % 1024 == 0 {
+                        // poll for incoming data
+                        if poll_incoming_data() {
+                            break RunEvent::IncomingData;
+                        }
+                    }
+                    cycles += 1;
+
+                    if let Some(event) = self.step() {
+                        break RunEvent::Event(event);
+                    };
+
+                    if !(start..end).contains(&self.cpu.reg_get(self.cpu.mode(), reg::PC)) {
+                        break RunEvent::Event(Event::DoneStep);
+                    }
+                }
+            }
+        }
+    }
+}
+
+pub enum RunEvent {
+    IncomingData,
+    Event(Event),
 }