Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/protocol/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()>;
}
Expand Down Expand Up @@ -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(())
Expand Down Expand Up @@ -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>,
}
Expand Down
52 changes: 52 additions & 0 deletions src/protocol/commands/_x_lowcase.rs
Original file line number Diff line number Diff line change
@@ -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 })
}
}
2 changes: 2 additions & 0 deletions src/stub/core_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/stub/core_impl/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => (
Expand Down
62 changes: 62 additions & 0 deletions src/stub/core_impl/x_lowcase_packet.rs
Original file line number Diff line number Diff line change
@@ -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) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: we should be able to share the implementation of this method with the m handler in handle_base, and avoid copy-pasting.

I'll look into this myself after this diff lands, since I want to make sure however we do it, it doesn't result in poor codegen on minimal stubs.

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)
}
}
18 changes: 18 additions & 0 deletions src/target/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,23 @@ pub trait Target {
true
}

/// Enable/disable using the `x` packet to read to target
/// memory (as opposed to the basic `m` packet).
///
/// By default, this method returns `false`.
///
/// GDB and LLDB have different responses for the `x` packet, and until
/// `gdbstub` supports a disabmiguation mechanism to correctly handle
/// both GDB and LLDB, this is by default set to `false`.
///
/// _Author's note:_ Unless you're _really_ trying to squeeze `gdbstub` onto
/// a particularly resource-constrained platform, you may as well leave this
/// optimization enabled.
#[inline(always)]
fn use_x_lowcase_packet(&self) -> bool {
false
}

/// Enable/disable using the more efficient `X` packet to write to target
/// memory (as opposed to the basic `M` packet).
///
Expand Down Expand Up @@ -802,6 +819,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);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this here, though

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is def right, and in-fact, you've just made me realize that we haven't been correctly delegating some of the new methods that were added here! I'll need to send out a follow-up bug fix to address that, lol.

__delegate!(fn use_x_upcase_packet(&self) -> bool);
__delegate!(fn use_resume_stub(&self) -> bool);
__delegate!(fn use_rle(&self) -> bool);
Expand Down
Loading