Add support for x packet (#186)

- Add `gdbstub` support for `x` packet that responds with `b`
  prefixed packed to indicate a binary format
- This is by default disabled, implement the `use_x_lowcase_packet`
  method of `Target` to enable it
- When this is enabled, `gdbstub` responds with `binary-upload+` to
  `qSupported` packet, to indicate support of this command.

Signed-off-by: Doru Blânzeanu <dblnz@pm.me>

Author: dblnz

src/protocol/commands.rs

@@ -94,6 +94,7 @@ macro_rules! commands {
                     fn support_reverse_step(&mut self) -> Option<()>;
                     fn support_reverse_cont(&mut self) -> Option<()>;
                     fn support_no_ack_mode(&mut self) -> Option<()>;
+                    fn support_x_lowcase_packet(&mut self) -> Option<()>;
                     fn support_x_upcase_packet(&mut self) -> Option<()>;
                     fn support_thread_extra_info(&mut self) -> Option<()>;
                 }
@@ -155,6 +156,14 @@ macro_rules! commands {
                         }
                     }
 
+                    fn support_x_lowcase_packet(&mut self) -> Option<()> {
+                        if self.use_x_lowcase_packet() {
+                            Some(())
+                        } else {
+                            None
+                        }
+                    }
+
                     fn support_x_upcase_packet(&mut self) -> Option<()> {
                         if self.use_x_upcase_packet() {
                             Some(())
@@ -255,6 +264,10 @@ commands! {
         "vCont" => _vCont::vCont<'a>,
     }
 
+    x_lowcase_packet use 'a {
+        "x" => _x_lowcase::x<'a>,
+    }
+
     x_upcase_packet use 'a {
         "X" => _x_upcase::X<'a>,
     }

src/protocol/commands/_x_lowcase.rs

@@ -0,0 +1,52 @@
+use super::prelude::*;
+
+#[derive(Debug)]
+pub struct x<'a> {
+    pub addr: &'a [u8],
+    pub len: usize,
+
+    /// Reuse PacketBuf underlying buffer to read the binary data into it
+    pub buf: &'a mut [u8],
+}
+
+impl<'a> ParseCommand<'a> for x<'a> {
+    #[inline(always)]
+    fn from_packet(buf: PacketBuf<'a>) -> Option<Self> {
+        // the total packet buffer currently looks like:
+        //
+        // +------+--------------------+-------------------+-------+-----------------+
+        // | "$x" | addr (hex-encoded) | len (hex-encoded) | "#XX" | empty space ... |
+        // +------+--------------------+-------------------+-------+-----------------+
+        //
+        // Unfortunately, while `len` can be hex-decoded right here and now into a
+        // `usize`, `addr` corresponds to a Target::Arch::Usize, which requires holding
+        // on to a valid &[u8] reference into the buffer.
+        //
+        // While it's not _perfectly_ efficient, simply leaving the decoded addr in
+        // place and wasting a couple bytes is probably the easiest way to tackle this
+        // problem:
+        //
+        // +------+------------------+------------------------------------------------+
+        // | "$x" | addr (raw bytes) | usable buffer ...                              |
+        // +------+------------------+------------------------------------------------+
+
+        let (buf, body_range) = buf.into_raw_buf();
+        let body = buf.get_mut(body_range.start..body_range.end)?;
+
+        let mut body = body.split_mut(|b| *b == b',');
+
+        let addr = decode_hex_buf(body.next()?).ok()?;
+        let addr_len = addr.len();
+        let len = decode_hex(body.next()?).ok()?;
+
+        // ensures that `split_at_mut` doesn't panic
+        if buf.len() < body_range.start + addr_len {
+            return None;
+        }
+
+        let (addr, buf) = buf.split_at_mut(body_range.start + addr_len);
+        let addr = addr.get(b"$x".len()..)?;
+
+        Some(x { addr, len, buf })
+    }
+}

src/stub/core_impl.rs

@@ -46,6 +46,7 @@ mod target_xml;
 mod thread_extra_info;
 mod tracepoints;
 mod wasm;
+mod x_lowcase_packet;
 mod x_upcase_packet;
 
 pub(crate) use resume::FinishExecStatus;
@@ -204,6 +205,7 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
             Command::TargetXml(cmd) => self.handle_target_xml(res, target, cmd),
             Command::Resume(cmd) => self.handle_stop_resume(res, target, cmd),
             Command::NoAckMode(cmd) => self.handle_no_ack_mode(res, target, cmd),
+            Command::XLowcasePacket(cmd) => self.handle_x_lowcase_packet(res, target, cmd),
             Command::XUpcasePacket(cmd) => self.handle_x_upcase_packet(res, target, cmd),
             Command::SingleRegisterAccess(cmd) => {
                 self.handle_single_register_access(res, target, cmd)

src/stub/core_impl/base.rs

@@ -131,6 +131,10 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
                     res.write_str(";vforkdone-events+")?;
                 }
 
+                if target.use_x_lowcase_packet() {
+                    res.write_str(";binary-upload+")?;
+                }
+
                 if let Some(resume_ops) = target.base_ops().resume_ops() {
                     let (reverse_cont, reverse_step) = match resume_ops {
                         ResumeOps::MultiThread(ops) => (

src/stub/core_impl/x_lowcase_packet.rs

@@ -0,0 +1,62 @@
+use super::prelude::*;
+use crate::arch::Arch;
+use crate::protocol::commands::ext::XLowcasePacket;
+use crate::target::ext::base::BaseOps;
+
+impl<T: Target, C: Connection> GdbStubImpl<T, C> {
+    pub(crate) fn handle_x_lowcase_packet(
+        &mut self,
+        res: &mut ResponseWriter<'_, C>,
+        target: &mut T,
+        command: XLowcasePacket<'_>,
+    ) -> Result<HandlerStatus, Error<T::Error, C::Error>> {
+        if !target.use_x_lowcase_packet() {
+            return Ok(HandlerStatus::Handled);
+        }
+
+        crate::__dead_code_marker!("x_lowcase_packet", "impl");
+
+        let handler_status = match command {
+            XLowcasePacket::x(cmd) => {
+                let buf = cmd.buf;
+                let addr = <T::Arch as Arch>::Usize::from_be_bytes(cmd.addr)
+                    .ok_or(Error::TargetMismatch)?;
+
+                let mut i = 0;
+                let mut n = cmd.len;
+                while n != 0 {
+                    let chunk_size = n.min(buf.len());
+
+                    use num_traits::NumCast;
+
+                    let addr = addr + NumCast::from(i).ok_or(Error::TargetMismatch)?;
+                    let data = &mut buf[..chunk_size];
+                    let data_len = match target.base_ops() {
+                        BaseOps::SingleThread(ops) => ops.read_addrs(addr, data),
+                        BaseOps::MultiThread(ops) => {
+                            ops.read_addrs(addr, data, self.current_mem_tid)
+                        }
+                    }
+                    .handle_error()?;
+
+                    // TODO: add more specific error variant?
+                    let data = data.get(..data_len).ok_or(Error::PacketBufferOverflow)?;
+
+                    // Start data with 'b' to indicate binary data
+                    if i == 0 {
+                        res.write_str("b")?;
+                    }
+
+                    n -= chunk_size;
+                    i += chunk_size;
+
+                    res.write_binary(data)?;
+                }
+
+                HandlerStatus::Handled
+            }
+        };
+
+        Ok(handler_status)
+    }
+}

src/target/mod.rs

@@ -530,6 +530,31 @@ pub trait Target {
         true
     }
 
+    /// Enable/disable using the more efficient `x` packet to read to target
+    /// memory (as opposed to the basic `m` packet).
+    ///
+    /// **By default, this method returns `false`.**
+    ///
+    /// This packet is disabled by default in order to maximize out-of-the-box
+    /// compatibility between `gdbstub` and all released GDB and LLDB versions.
+    /// For more context, see the discussion at
+    /// [daniel5151/gdbstub#163](https://github.com/daniel5151/gdbstub/issues/163#issuecomment-4049691552).
+    ///
+    /// `gdbstub` implements the `x` packet according to the GDB RSP spec, and
+    /// as such, enabling this packet will break compatibility with older LLDB
+    /// version.
+    ///
+    /// _Author's note:_ If you are _certain_ that your target will only even be
+    /// debugged used alongside a sufficiently recent GDB / LLDB version (i.e:
+    /// those released sometime after ~early 2026), you can set this to `true`
+    /// for improved performance. That said, unless you're planning to fetch
+    /// a _lot_ of memory data from your `Target`, you may as well leave
+    /// this optimization disabled, and guarantee client compatibility.
+    #[inline(always)]
+    fn use_x_lowcase_packet(&self) -> bool {
+        false
+    }
+
     /// Whether `gdbstub` should provide a "stub" `resume` implementation on
     /// targets without support for resumption.
     ///
@@ -802,6 +827,7 @@ macro_rules! impl_dyn_target {
             __delegate!(fn guard_rail_implicit_sw_breakpoints(&self) -> bool);
 
             __delegate!(fn use_no_ack_mode(&self) -> bool);
+            __delegate!(fn use_x_lowcase_packet(&self) -> bool);
             __delegate!(fn use_x_upcase_packet(&self) -> bool);
             __delegate!(fn use_resume_stub(&self) -> bool);
             __delegate!(fn use_rle(&self) -> bool);