almost complete GDB agent + conditional breakpoint support

wowie what a doozy. getting this working nicely required a lot of
through and refactoring, though as it stands I'm pretty happy with how
the API is shaping up.

One notable missing piece as of this commit is that the gdbstub doesn't
actually evaluate any conditions/commands after a breakpoint it hit.
See, I realized that the Sw/HwBreak stop reasons don't actually include
the breakpoint address (I mean, why would they), but I need the
breakpoint address to ask the target for a list of BytecodeIds to
execute.

Oof.

I think I'm going to have to do something cheecky and modify the
`Registers` trait to include a `get_pc` method, which is kinda stinky,
but I don't see any better way... That way, I can simply call
`get_registers` after a breakpoint is hit and extract the PC.

This seems cleaner than the alternative, which would be to package the
PC value alongside the Sw/HwBreak stop reason. That would be a very
annoying breaking change, as opposed to the `Registers` trait tweak,
which is technically also a breaking change, but given that most folks
are using the built-in register defenitions, it shouldn't break too many
downtream users.

examples/armv4t/emu.rs

@@ -1,4 +1,7 @@
+use std::collections::HashMap;
+
 use armv4t_emu::{reg, Cpu, ExampleMem, Memory, Mode};
+use gdbstub::target::ext::agent::BytecodeId;
 
 use crate::mem_sniffer::{AccessKind, MemSniffer};
 use crate::DynResult;
@@ -13,6 +16,25 @@ pub enum Event {
     WatchRead(u32),
 }
 
+#[derive(Default, Debug, Clone)]
+pub(crate) struct GdbAgent {
+    pub bytecode_id_counter: usize,
+    pub agent_bytecode: HashMap<BytecodeId, Vec<u8>>,
+    pub breakpoint_conditions: HashMap<u32, Vec<BytecodeId>>,
+    pub breakpoint_commands: HashMap<u32, Vec<BytecodeId>>,
+}
+
+impl GdbAgent {
+    pub fn new() -> GdbAgent {
+        GdbAgent {
+            bytecode_id_counter: 0,
+            agent_bytecode: HashMap::new(),
+            breakpoint_conditions: HashMap::new(),
+            breakpoint_commands: HashMap::new(),
+        }
+    }
+}
+
 /// incredibly barebones armv4t-based emulator
 pub struct Emu {
     start_addr: u32,
@@ -22,6 +44,8 @@ pub struct Emu {
 
     pub(crate) watchpoints: Vec<u32>,
     pub(crate) breakpoints: Vec<u32>,
+
+    pub(crate) agent: Option<Box<GdbAgent>>,
 }
 
 impl Emu {
@@ -63,8 +87,11 @@ impl Emu {
             start_addr: elf_header.entry as u32,
             cpu,
             mem,
+
             watchpoints: Vec::new(),
             breakpoints: Vec::new(),
+
+            agent: Some(Box::new(GdbAgent::new())),
         })
     }
 

examples/armv4t/gdb/agent.rs

@@ -0,0 +1,34 @@
+use gdbstub::target;
+use gdbstub::target::ext::agent::BytecodeId;
+use gdbstub::target::TargetResult;
+
+use crate::emu::Emu;
+
+impl target::ext::agent::Agent for Emu {
+    fn enabled(&mut self, _enabled: bool) -> Result<(), Self::Error> {
+        Ok(())
+    }
+
+    fn register_bytecode(&mut self, bytecode: &[u8]) -> TargetResult<BytecodeId, Self> {
+        let agent = self.agent.as_mut().unwrap();
+
+        agent.bytecode_id_counter += 1;
+        let id = BytecodeId::new(agent.bytecode_id_counter).unwrap();
+        agent.agent_bytecode.insert(id, bytecode.to_vec());
+        log::warn!("Registered {:?}", id);
+        Ok(id)
+    }
+
+    fn unregister_bytecode(&mut self, id: BytecodeId) -> TargetResult<(), Self> {
+        let agent = self.agent.as_mut().unwrap();
+
+        agent.agent_bytecode.remove(&id);
+        log::warn!("Unregistered {:?}", id);
+        Ok(())
+    }
+
+    fn evaluate(&mut self, id: BytecodeId) -> TargetResult<u32, Self> {
+        log::error!("Evaluating {:?} - STUBBED, RETURNING 0", id);
+        Ok(0)
+    }
+}

examples/armv4t/gdb/breakpoints.rs

@@ -0,0 +1,146 @@
+use gdbstub::arch;
+use gdbstub::target;
+use gdbstub::target::ext::agent::BytecodeId;
+use gdbstub::target::ext::breakpoints::{BreakpointAgentOps, BreakpointBytecodeKind, WatchKind};
+use gdbstub::target::TargetResult;
+
+use crate::emu::Emu;
+
+impl target::ext::breakpoints::Breakpoints for Emu {
+    fn sw_breakpoint(&mut self) -> Option<target::ext::breakpoints::SwBreakpointOps<Self>> {
+        Some(self)
+    }
+
+    fn hw_watchpoint(&mut self) -> Option<target::ext::breakpoints::HwWatchpointOps<Self>> {
+        Some(self)
+    }
+
+    fn breakpoint_agent(&mut self) -> Option<BreakpointAgentOps<Self>> {
+        Some(self)
+    }
+}
+
+impl target::ext::breakpoints::SwBreakpoint for Emu {
+    fn add_sw_breakpoint(
+        &mut self,
+        addr: u32,
+        _kind: arch::arm::ArmBreakpointKind,
+    ) -> TargetResult<bool, Self> {
+        self.breakpoints.push(addr);
+        Ok(true)
+    }
+
+    fn remove_sw_breakpoint(
+        &mut self,
+        addr: u32,
+        _kind: arch::arm::ArmBreakpointKind,
+    ) -> TargetResult<bool, Self> {
+        match self.breakpoints.iter().position(|x| *x == addr) {
+            None => return Ok(false),
+            Some(pos) => self.breakpoints.remove(pos),
+        };
+
+        Ok(true)
+    }
+}
+
+impl target::ext::breakpoints::HwWatchpoint for Emu {
+    fn add_hw_watchpoint(&mut self, addr: u32, kind: WatchKind) -> TargetResult<bool, Self> {
+        match kind {
+            WatchKind::Write => self.watchpoints.push(addr),
+            WatchKind::Read => self.watchpoints.push(addr),
+            WatchKind::ReadWrite => self.watchpoints.push(addr),
+        };
+
+        Ok(true)
+    }
+
+    fn remove_hw_watchpoint(&mut self, addr: u32, kind: WatchKind) -> TargetResult<bool, Self> {
+        let pos = match self.watchpoints.iter().position(|x| *x == addr) {
+            None => return Ok(false),
+            Some(pos) => pos,
+        };
+
+        match kind {
+            WatchKind::Write => self.watchpoints.remove(pos),
+            WatchKind::Read => self.watchpoints.remove(pos),
+            WatchKind::ReadWrite => self.watchpoints.remove(pos),
+        };
+
+        Ok(true)
+    }
+}
+
+impl target::ext::breakpoints::BreakpointAgent for Emu {
+    fn add_breakpoint_bytecode(
+        &mut self,
+        kind: BreakpointBytecodeKind,
+        addr: u32,
+        id: BytecodeId,
+        _persist: bool,
+    ) -> TargetResult<(), Self> {
+        log::warn!("Registered {:?} {:#010x?}:{:?}", kind, addr, id);
+
+        let agent = self.agent.as_mut().unwrap();
+        match kind {
+            BreakpointBytecodeKind::Command => &mut agent.breakpoint_commands,
+            BreakpointBytecodeKind::Condition => &mut agent.breakpoint_conditions,
+        }
+        .entry(addr)
+        .or_default()
+        .push(id);
+
+        Ok(())
+    }
+
+    fn clear_breakpoint_bytecode(
+        &mut self,
+        kind: BreakpointBytecodeKind,
+        addr: u32,
+    ) -> TargetResult<(), Self> {
+        log::warn!("Unregistered all {:?} from {:#010x?}", kind, addr);
+
+        let agent = self.agent.as_mut().unwrap();
+        if let Some(s) = match kind {
+            BreakpointBytecodeKind::Command => &mut agent.breakpoint_commands,
+            BreakpointBytecodeKind::Condition => &mut agent.breakpoint_conditions,
+        }
+        .get_mut(&addr)
+        {
+            s.clear()
+        }
+
+        Ok(())
+    }
+
+    fn get_breakpoint_bytecode(
+        &mut self,
+        kind: BreakpointBytecodeKind,
+        addr: u32,
+        callback: &mut dyn FnMut(BreakpointAgentOps<Self>, BytecodeId) -> TargetResult<(), Self>,
+    ) -> TargetResult<(), Self> {
+        // see the method's documentation for more info why this rigamarole is required.
+
+        // temporarily take ownership of the agent data.
+        let mut agent = self.agent.take().unwrap();
+
+        let ids = match kind {
+            BreakpointBytecodeKind::Command => &mut agent.breakpoint_commands,
+            BreakpointBytecodeKind::Condition => &mut agent.breakpoint_conditions,
+        }
+        .entry(addr)
+        .or_default();
+
+        let mut res = Ok(());
+        for id in ids.iter() {
+            res = callback(self, *id);
+            if res.is_err() {
+                break;
+            }
+        }
+
+        self.agent = Some(agent);
+
+        res
+    }
+}

examples/armv4t/gdb/mod.rs

@@ -12,6 +12,8 @@ use crate::emu::{Emu, Event};
 
 // Additional GDB extensions
 
+mod agent;
+mod breakpoints;
 mod extended_mode;
 mod monitor_cmd;
 mod section_offsets;
@@ -51,6 +53,10 @@ impl Target for Emu {
     fn section_offsets(&mut self) -> Option<target::ext::section_offsets::SectionOffsetsOps<Self>> {
         Some(self)
     }
+
+    fn agent(&mut self) -> Option<target::ext::agent::AgentOps<Self>> {
+        Some(self)
+    }
 }
 
 impl SingleThreadOps for Emu {
@@ -82,7 +88,7 @@ impl SingleThreadOps for Emu {
 
         Ok(match event {
             Event::Halted => StopReason::Halted,
-            Event::Break => StopReason::HwBreak,
+            Event::Break => StopReason::SwBreak,
             Event::WatchWrite(addr) => StopReason::Watch {
                 kind: WatchKind::Write,
                 addr,
@@ -167,64 +173,3 @@ impl SingleThreadOps for Emu {
         Ok(())
     }
 }
-
-impl target::ext::breakpoints::Breakpoints for Emu {
-    fn sw_breakpoint(&mut self) -> Option<target::ext::breakpoints::SwBreakpointOps<Self>> {
-        Some(self)
-    }
-
-    fn hw_watchpoint(&mut self) -> Option<target::ext::breakpoints::HwWatchpointOps<Self>> {
-        Some(self)
-    }
-}
-
-impl target::ext::breakpoints::SwBreakpoint for Emu {
-    fn add_sw_breakpoint(
-        &mut self,
-        addr: u32,
-        _kind: arch::arm::ArmBreakpointKind,
-    ) -> TargetResult<bool, Self> {
-        self.breakpoints.push(addr);
-        Ok(true)
-    }
-
-    fn remove_sw_breakpoint(
-        &mut self,
-        addr: u32,
-        _kind: arch::arm::ArmBreakpointKind,
-    ) -> TargetResult<bool, Self> {
-        match self.breakpoints.iter().position(|x| *x == addr) {
-            None => return Ok(false),
-            Some(pos) => self.breakpoints.remove(pos),
-        };
-
-        Ok(true)
-    }
-}
-
-impl target::ext::breakpoints::HwWatchpoint for Emu {
-    fn add_hw_watchpoint(&mut self, addr: u32, kind: WatchKind) -> TargetResult<bool, Self> {
-        match kind {
-            WatchKind::Write => self.watchpoints.push(addr),
-            WatchKind::Read => self.watchpoints.push(addr),
-            WatchKind::ReadWrite => self.watchpoints.push(addr),
-        };
-
-        Ok(true)
-    }
-
-    fn remove_hw_watchpoint(&mut self, addr: u32, kind: WatchKind) -> TargetResult<bool, Self> {
-        let pos = match self.watchpoints.iter().position(|x| *x == addr) {
-            None => return Ok(false),
-            Some(pos) => pos,
-        };
-
-        match kind {
-            WatchKind::Write => self.watchpoints.remove(pos),
-            WatchKind::Read => self.watchpoints.remove(pos),
-            WatchKind::ReadWrite => self.watchpoints.remove(pos),
-        };
-
-        Ok(true)
-    }
-}

src/gdbstub_impl/error.rs

@@ -21,7 +21,8 @@ pub enum GdbStubError<T, C> {
     PacketBufferOverlow,
     /// Could not parse the packet into a valid command.
     PacketParse(PacketParseError),
-    /// GDB client sent an unexpected packet.
+    /// GDB client sent an unexpected packet. This should never happen!
+    /// Please file an issue at https://github.com/daniel5151/gdbstub/issues
     PacketUnexpected,
     /// GDB client sent a packet with too much data for the given target.
     TargetMismatch,
@@ -62,7 +63,7 @@ where
             MissingPacketBuffer => write!(f, "GdbStub was not provided with a packet buffer in `no_std` mode (missing call to `with_packet_buffer`)"),
             PacketBufferOverlow => write!(f, "Packet too big for provided buffer!"),
             PacketParse(e) => write!(f, "Could not parse the packet into a valid command: {:?}", e),
-            PacketUnexpected => write!(f, "Client sent an unexpected packet."),
+            PacketUnexpected => write!(f, "Client sent an unexpected packet. This should never happen! Please file an issue at https://github.com/daniel5151/gdbstub/issues"),
             TargetMismatch => write!(f, "GDB client sent a packet with too much data for the given target."),
             TargetError(e) => write!(f, "Target threw a fatal error: {:?}", e),
             NoActiveThreads => write!(f, "Target didn't report any active threads."),

src/gdbstub_impl/ext/agent.rs

@@ -0,0 +1,25 @@
+use super::prelude::*;
+use crate::protocol::commands::ext::Agent;
+
+impl<T: Target, C: Connection> GdbStubImpl<T, C> {
+    pub(crate) fn handle_agent(
+        &mut self,
+        _res: &mut ResponseWriter<C>,
+        target: &mut T,
+        command: Agent,
+    ) -> Result<HandlerStatus, Error<T::Error, C::Error>> {
+        let ops = match target.agent() {
+            Some(ops) => ops,
+            None => return Ok(HandlerStatus::Handled),
+        };
+
+        let handler_status = match command {
+            Agent::QAgent(cmd) => {
+                ops.enabled(cmd.value).map_err(Error::TargetError)?;
+                HandlerStatus::NeedsOK
+            }
+        };
+
+        Ok(handler_status)
+    }
+}