create Arch + Register traits

Ah ha! No more leaking GDB internals to users*!

*Assuming the user is using an architecture listed already implemented
under the `arch` module, they don't have to dig through the GDB source
for some obscure XML files.

The unfortunate side-effect of this change is the really digusting type
syntax which `Target` has to use. `<Self::Arch as Arch>::Usize` is
really gnarly :sad:

If rust-lang/rust#38078 ever gets fixed,
the type could be re-written as `Self::Arch::Usize`, which is a _lot_
cleaner.

Author: daniel5151

examples/armv4t/gdb.rs

@@ -1,16 +1,12 @@
 use armv4t_emu::{reg, Memory};
-use gdbstub::{HwBreakOp, Target, TargetState, WatchKind};
+use gdbstub::{arch, HwBreakOp, Target, TargetState, WatchKind};
 
 use crate::emu::{Emu, Event};
 
 impl Target for Emu {
-    type Usize = u32;
+    type Arch = arch::arm::Armv4t;
     type Error = &'static str;
 
-    fn target_description_xml() -> Option<&'static str> {
-        Some(r#"<target version="1.0"><architecture>armv4t</architecture></target>"#)
-    }
-
     fn step(&mut self) -> Result<TargetState<u32>, Self::Error> {
         let event = match self.step() {
             Some(event) => event,
@@ -32,54 +28,33 @@ impl Target for Emu {
     }
 
     // order specified in binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
-    fn read_registers(&mut self, mut push_reg: impl FnMut(&[u8])) -> Result<(), &'static str> {
+    fn read_registers(
+        &mut self,
+        regs: &mut arch::arm::reg::ArmCoreRegs,
+    ) -> Result<(), &'static str> {
         let mode = self.cpu.mode();
-        for i in 0..13 {
-            push_reg(&self.cpu.reg_get(mode, i).to_le_bytes());
-        }
-        push_reg(&self.cpu.reg_get(mode, reg::SP).to_le_bytes()); // 13
-        push_reg(&self.cpu.reg_get(mode, reg::LR).to_le_bytes()); // 14
-        push_reg(&self.cpu.reg_get(mode, reg::PC).to_le_bytes()); // 15
 
-        // Floating point registers, unused
-        for _ in 0..25 {
-            push_reg(&[0, 0, 0, 0]);
+        for i in 0..13 {
+            regs.r[i] = self.cpu.reg_get(mode, i as u8);
         }
-
-        push_reg(&self.cpu.reg_get(mode, reg::CPSR).to_le_bytes());
+        regs.sp = self.cpu.reg_get(mode, reg::SP);
+        regs.lr = self.cpu.reg_get(mode, reg::LR);
+        regs.pc = self.cpu.reg_get(mode, reg::PC);
+        regs.cpsr = self.cpu.reg_get(mode, reg::CPSR);
 
         Ok(())
     }
 
-    fn write_registers(
-        &mut self,
-        mut pop_reg: impl FnMut() -> Option<u8>,
-    ) -> Result<(), &'static str> {
-        const ERR: &str = "malformed write register packet";
-
-        let mut next = {
-            move || -> Option<u32> {
-                Some(
-                    (pop_reg()? as u32)
-                        | (pop_reg()? as u32) << 8
-                        | (pop_reg()? as u32) << 16
-                        | (pop_reg()? as u32) << 24,
-                )
-            }
-        };
+    fn write_registers(&mut self, regs: &arch::arm::reg::ArmCoreRegs) -> Result<(), &'static str> {
         let mode = self.cpu.mode();
+
         for i in 0..13 {
-            self.cpu.reg_set(mode, i, next().ok_or(ERR)?);
-        }
-        self.cpu.reg_set(mode, reg::SP, next().ok_or(ERR)?);
-        self.cpu.reg_set(mode, reg::LR, next().ok_or(ERR)?);
-        self.cpu.reg_set(mode, reg::PC, next().ok_or(ERR)?);
-        // Floating point registers, unused
-        for _ in 0..25 {
-            next().ok_or(ERR)?;
+            self.cpu.reg_set(mode, i, regs.r[i as usize]);
         }
-
-        self.cpu.reg_set(mode, reg::CPSR, next().ok_or(ERR)?);
+        self.cpu.reg_set(mode, reg::SP, regs.sp);
+        self.cpu.reg_set(mode, reg::LR, regs.lr);
+        self.cpu.reg_set(mode, reg::PC, regs.pc);
+        self.cpu.reg_set(mode, reg::CPSR, regs.cpsr);
 
         Ok(())
     }

src/arch/arm/mod.rs

@@ -0,0 +1,22 @@
+//! Implementations for various ARM architectures.
+
+use crate::Arch;
+
+pub mod reg;
+
+/// Implements `Arch` for ARMv4T
+#[derive(Eq, PartialEq)]
+pub struct Armv4t;
+
+impl Arch for Armv4t {
+    type Usize = u32;
+    type Registers = reg::ArmCoreRegs;
+
+    fn read_pc(regs: &reg::ArmCoreRegs) -> u32 {
+        regs.pc
+    }
+
+    fn target_description_xml() -> Option<&'static str> {
+        Some(r#"<target version="1.0"><architecture>armv4t</architecture></target>"#)
+    }
+}

src/arch/arm/reg/arm_core.rs

@@ -0,0 +1,70 @@
+use crate::Registers;
+
+/// 32-bit ARM core registers.
+///
+/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
+#[derive(Default)]
+pub struct ArmCoreRegs {
+    /// General purpose registers (R0-R12)
+    pub r: [u32; 13],
+    /// Stack Pointer (R13)
+    pub sp: u32,
+    /// Link Register (R14)
+    pub lr: u32,
+    /// Program Counter (R15)
+    pub pc: u32,
+    /// Current Program Status Register (cpsr)
+    pub cpsr: u32,
+}
+
+impl Registers for ArmCoreRegs {
+    fn gdb_serialize(&self, mut write_byte: impl FnMut(Option<u8>)) {
+        macro_rules! write_bytes {
+            ($bytes:expr) => {
+                for b in $bytes {
+                    write_byte(Some(*b))
+                }
+            };
+        }
+
+        for reg in self.r.iter() {
+            write_bytes!(&reg.to_le_bytes());
+        }
+        write_bytes!(&self.sp.to_le_bytes());
+        write_bytes!(&self.lr.to_le_bytes());
+        write_bytes!(&self.pc.to_le_bytes());
+
+        // Floating point registers (unused)
+        for _ in 0..25 {
+            (0..4).for_each(|_| write_byte(None))
+        }
+
+        write_bytes!(&self.cpsr.to_le_bytes());
+    }
+
+    fn gdb_deserialize(&mut self, mut bytes: impl Iterator<Item = u8>) -> Result<(), ()> {
+        let mut next_u32 = move || -> Option<u32> {
+            let val = (bytes.next()? as u32)
+                | (bytes.next()? as u32) << 8
+                | (bytes.next()? as u32) << 16
+                | (bytes.next()? as u32) << 24;
+            Some(val)
+        };
+
+        for reg in self.r.iter_mut() {
+            *reg = next_u32().ok_or(())?
+        }
+        self.sp = next_u32().ok_or(())?;
+        self.lr = next_u32().ok_or(())?;
+        self.pc = next_u32().ok_or(())?;
+
+        // Floating point registers (unused)
+        for _ in 0..25 {
+            next_u32().ok_or(())?;
+        }
+
+        self.cpsr = next_u32().ok_or(())?;
+
+        Ok(())
+    }
+}

src/arch/arm/reg/mod.rs

@@ -0,0 +1,5 @@
+//! `GdbRegister` structs for various ARM architectures.
+
+mod arm_core;
+
+pub use arm_core::ArmCoreRegs;

src/arch/mod.rs

@@ -0,0 +1,8 @@
+//! Built-in implementations of the [`Arch`](arch/trait.Arch.html) and
+//! [`Registers`](arch/trait.Registers.html) traits for various
+//! architectures.
+//!
+//! **Note:** Please consider upstreaming any missing `Arch` + `Registers`
+//! implementations you happen to implement yourself!
+
+pub mod arm;

src/arch_traits.rs

@@ -0,0 +1,59 @@
+use core::fmt::{self, Debug};
+
+use num_traits::{Num, PrimInt, Unsigned};
+
+/// A struct which corresponds to a particular architecture's registers.
+pub trait Registers: Default {
+    /// Serialize `self` into a GDB register bytestream.
+    ///
+    /// The registers must be serialized in the order specified by the
+    /// architecture's `<target>.xml`. Missing registers are serialized by
+    /// passing `None` to write_byte (which gets translated to an "xx" string
+    /// within the GdbStub).
+    ///
+    /// e.g: for ARM:
+    /// github.com/bminor/binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
+    fn gdb_serialize(&self, write_byte: impl FnMut(Option<u8>));
+
+    /// Deserialize a GDB register bytestream into `self`.
+    ///
+    /// The bytes will be provided in the order specified by the architecture's
+    /// `<target>.xml`.
+    ///
+    /// e.g: for ARM:
+    /// github.com/bminor/binutils-gdb/blob/master/gdb/features/arm/arm-core.xml
+    fn gdb_deserialize(&mut self, bytes: impl Iterator<Item = u8>) -> Result<(), ()>;
+}
+
+/// Encodes architecture-specific information, such as pointer size, register
+/// layout, etc...
+pub trait Arch: Eq + PartialEq {
+    /// The architecture's pointer size (e.g: `u32` on a 32-bit system).
+    type Usize: Num + PrimInt + Unsigned + Debug + fmt::LowerHex;
+
+    /// The architecture's register file
+    type Registers: Registers;
+
+    /// Read the target's current PC.
+    fn read_pc(registers: &Self::Registers) -> Self::Usize;
+
+    /// (optional) Return the platform's `features.xml` file.
+    ///
+    /// Implementing this method enables `gdb` to automatically detect the
+    /// target's architecture, saving the hassle of having to run `set
+    /// architecture <arch>` when starting a debugging session.
+    ///
+    /// These descriptions can be quite succinct. For example, the target
+    /// description for an `armv4t` platform can be as simple as:
+    ///
+    /// ```
+    /// r#"<target version="1.0"><architecture>armv4t</architecture></target>"#
+    /// # ;
+    /// ```
+    ///
+    /// See the [GDB docs](https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html)
+    /// for details on the target description XML format.
+    fn target_description_xml() -> Option<&'static str> {
+        None
+    }
+}

src/lib.rs

@@ -242,14 +242,19 @@
 //! open a PR to add it to this list!
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// #![deny(missing_docs)]
 
+pub mod arch;
+
+mod arch_traits;
 mod connection;
 mod error;
 mod protocol;
 mod stub;
 mod target;
 mod util;
 
+pub use arch_traits::{Arch, Registers};
 pub use connection::Connection;
 pub use error::Error;
 pub use stub::GdbStub;

src/stub.rs

@@ -5,7 +5,7 @@ use num_traits::ops::saturating::Saturating;
 use crate::util::slicevec::SliceVec;
 use crate::{
     protocol::{Command, Packet, ResponseWriter},
-    Connection, Error, HwBreakOp, Target, TargetState, WatchKind,
+    Arch, Connection, Error, HwBreakOp, Registers, Target, TargetState, WatchKind,
 };
 
 enum ExecState {
@@ -24,7 +24,7 @@ pub struct GdbStub<'a, T: Target, C: Connection> {
     conn: C,
     exec_state: ExecState,
     // TODO?: use BTreeSet if std is enabled for infinite swbreaks?
-    swbreak: ArrayVec<[T::Usize; MAX_SWBREAK]>,
+    swbreak: ArrayVec<[<T::Arch as Arch>::Usize; MAX_SWBREAK]>,
     packet_buffer: Option<&'a mut [u8]>,
 }
 
@@ -67,14 +67,14 @@ impl<'a, T: Target, C: Connection> GdbStub<'a, T, C> {
                 // res.write_str("ConditionalBreakpoints+;")?;
 
                 // probe support for target description xml
-                if T::target_description_xml().is_some() {
+                if T::Arch::target_description_xml().is_some() {
                     res.write_str("qXfer:features:read+;")?;
                 }
             }
             Command::vContQuestionMark(_) => res.write_str("vCont;c;s;t")?,
             Command::qXferFeaturesRead(cmd) => {
                 assert_eq!(cmd.annex, "target.xml");
-                match T::target_description_xml() {
+                match T::Arch::target_description_xml() {
                     Some(xml) => {
                         let xml = xml.trim();
                         if cmd.offset >= xml.len() {
@@ -102,28 +102,33 @@ impl<'a, T: Target, C: Connection> GdbStub<'a, T, C> {
             Command::QuestionMark(_) => res.write_str("S05")?,
             Command::qAttached(_) => res.write_str("1")?,
             Command::g(_) => {
-                let mut err = Ok(());
+                let mut regs: <T::Arch as Arch>::Registers = Default::default();
                 target
-                    .read_registers(|reg| {
-                        if let Err(e) = res.write_hex_buf(reg) {
-                            err = Err(e)
-                        }
-                    })
+                    .read_registers(&mut regs)
                     .map_err(Error::TargetError)?;
+                let mut err = Ok(());
+                regs.gdb_serialize(|val| {
+                    let res = match val {
+                        Some(b) => res.write_hex(b),
+                        None => res.write_str("xx"),
+                    };
+                    if let Err(e) = res {
+                        err = Err(e);
+                    }
+                });
                 err?;
             }
-            Command::G(mut cmd) => {
-                // TODO: use the length of the slice returned by `target.read_registers` to
-                // validate that the server sent the correct amount of data
-                target
-                    .write_registers(|| cmd.vals.next())
-                    .map_err(Error::TargetError)?;
+            Command::G(cmd) => {
+                let mut regs: <T::Arch as Arch>::Registers = Default::default();
+                regs.gdb_deserialize(cmd.vals)
+                    .map_err(|_| Error::PacketParse)?; // FIXME: more granular error?
+                target.write_registers(&regs).map_err(Error::TargetError)?;
                 res.write_str("OK")?;
             }
             Command::m(cmd) => {
                 let mut err = Ok(());
                 // XXX: get rid of these unwraps ahhh
-                let start: T::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
+                let start: <T::Arch as Arch>::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
                 // XXX: on overflow, this _should_ wrap around to low addresses (maybe?)
                 let end = start.saturating_add(num_traits::NumCast::from(cmd.len).unwrap());
 
@@ -156,7 +161,7 @@ impl<'a, T: Target, C: Connection> GdbStub<'a, T, C> {
             }
             Command::Z(cmd) => {
                 // XXX: get rid of this unwrap ahhh
-                let addr: T::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
+                let addr: <T::Arch as Arch>::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
 
                 use HwBreakOp::*;
                 let supported = match cmd.type_ {
@@ -177,7 +182,7 @@ impl<'a, T: Target, C: Connection> GdbStub<'a, T, C> {
             }
             Command::z(cmd) => {
                 // XXX: get rid of this unwrap ahhh
-                let addr: T::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
+                let addr: <T::Arch as Arch>::Usize = num_traits::NumCast::from(cmd.addr).unwrap();
 
                 use HwBreakOp::*;
                 let supported = match cmd.type_ {
@@ -293,7 +298,7 @@ impl<'a, T: Target, C: Connection> GdbStub<'a, T, C> {
     ///
     /// Returns once the GDB client cleanly closes the debugging session (via
     /// the GDB `quit` command).
-    pub fn run(&mut self, target: &mut T) -> Result<TargetState<T::Usize>, Error<T, C>> {
+    pub fn run(&mut self, target: &mut T) -> Result<TargetState<T::Arch>, Error<T, C>> {
         let packet_buffer = self.packet_buffer.take().unwrap();
 
         loop {