From cb6b95ac6d374e347018465f59258d5842391c61 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 9 Dec 2024 18:51:34 -0500 Subject: [PATCH 01/29] tracepoint extension support --- README.md | 3 + src/protocol/commands.rs | 20 ++ src/protocol/commands/_QTBuffer.rs | 53 ++++ src/protocol/commands/_QTDP.rs | 75 +++++ src/protocol/commands/_QTFrame.rs | 42 +++ src/protocol/commands/_QTStart.rs | 14 + src/protocol/commands/_QTStop.rs | 14 + src/protocol/commands/_QTinit.rs | 11 + src/protocol/commands/_qTP.rs | 27 ++ src/protocol/commands/_qTStatus.rs | 11 + src/protocol/commands/_qTfP.rs | 14 + src/protocol/commands/_qTfV.rs | 14 + src/protocol/commands/_qTsP.rs | 14 + src/protocol/commands/_qTsV.rs | 14 + src/stub/core_impl.rs | 2 + src/stub/core_impl/base.rs | 5 + src/stub/core_impl/tracepoints.rs | 297 +++++++++++++++++++ src/target/ext/mod.rs | 1 + src/target/ext/tracepoints.rs | 448 +++++++++++++++++++++++++++++ src/target/mod.rs | 7 + 20 files changed, 1086 insertions(+) create mode 100644 src/protocol/commands/_QTBuffer.rs create mode 100644 src/protocol/commands/_QTDP.rs create mode 100644 src/protocol/commands/_QTFrame.rs create mode 100644 src/protocol/commands/_QTStart.rs create mode 100644 src/protocol/commands/_QTStop.rs create mode 100644 src/protocol/commands/_QTinit.rs create mode 100644 src/protocol/commands/_qTP.rs create mode 100644 src/protocol/commands/_qTStatus.rs create mode 100644 src/protocol/commands/_qTfP.rs create mode 100644 src/protocol/commands/_qTfV.rs create mode 100644 src/protocol/commands/_qTsP.rs create mode 100644 src/protocol/commands/_qTsV.rs create mode 100644 src/stub/core_impl/tracepoints.rs create mode 100644 src/target/ext/tracepoints.rs diff --git a/README.md b/README.md index 9a8133b..2e0b0c5 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,9 @@ Of course, most use-cases will want to support additional debugging features as - Read auxiliary vector (`info auxv`) - Extra thread info (`info threads`) - Extra library information (`info sharedlibraries`) +- Tracepoints + - Configure tracepoints and actions to perform when hit + - Select and interrogate collected trace frames _Note:_ GDB features are implemented on an as-needed basis by `gdbstub`'s contributors. If there's a missing GDB feature that you'd like `gdbstub` to implement, please file an issue and/or open a PR! diff --git a/src/protocol/commands.rs b/src/protocol/commands.rs index 633aeca..ea3aeec 100644 --- a/src/protocol/commands.rs +++ b/src/protocol/commands.rs @@ -51,6 +51,7 @@ macro_rules! commands { pub mod ext { $( #[allow(non_camel_case_types, clippy::enum_variant_names)] + #[derive(Debug)] pub enum [<$ext:camel>] $(<$lt>)? { $($command(super::$mod::$command<$($lifetime)?>),)* } @@ -58,6 +59,7 @@ macro_rules! commands { use super::breakpoint::{BasicBreakpoint, BytecodeBreakpoint}; #[allow(non_camel_case_types)] + #[derive(Debug)] pub enum Breakpoints<'a> { z(BasicBreakpoint<'a>), Z(BasicBreakpoint<'a>), @@ -69,6 +71,7 @@ macro_rules! commands { } /// GDB commands + #[derive(Debug)] pub enum Command<'a> { $( [<$ext:camel>](ext::[<$ext:camel>]$(<$lt>)?), @@ -336,4 +339,21 @@ commands! { libraries_svr4 use 'a { "qXfer:libraries-svr4:read" => _qXfer_libraries_svr4_read::qXferLibrariesSvr4Read<'a>, } + + tracepoints use 'a { + "QTDP" => _QTDP::QTDP<'a>, + "QTinit" => _QTinit::QTinit, + "QTBuffer" => _QTBuffer::QTBuffer<'a>, + // TODO: QTNotes? + "QTStart" => _QTStart::QTStart, + "QTStop" => _QTStop::QTStop, + "QTFrame" => _QTFrame::QTFrame<'a>, + + "qTStatus" => _qTStatus::qTStatus, + "qTP" => _qTP::qTP<'a>, + "qTfP" => _qTfP::qTfP, + "qTsP" => _qTsP::qTsP, + "qTfV" => _qTfV::qTfV, + "qTsV" => _qTsV::qTsV, + } } diff --git a/src/protocol/commands/_QTBuffer.rs b/src/protocol/commands/_QTBuffer.rs new file mode 100644 index 0000000..ee8f76f --- /dev/null +++ b/src/protocol/commands/_QTBuffer.rs @@ -0,0 +1,53 @@ +use super::prelude::*; +use crate::target::ext::tracepoints::{BufferShape, TraceBuffer}; + +#[derive(Debug)] +pub enum QTBuffer<'a> +{ + Request { offset: u64, length: usize, data: &'a mut [u8] }, + Configure { buffer: TraceBuffer }, +} + +impl<'a> ParseCommand<'a> for QTBuffer<'a> { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + let (buf, body_range) = buf.into_raw_buf(); + let body = &buf[body_range]; + match body { + [b':', body @ ..] => { + let mut s = body.split(|b| *b == b':'); + let opt = s.next()?; + Some(match opt.as_ref() { + b"circular" => { + let shape = s.next()?; + QTBuffer::Configure { buffer: TraceBuffer::Shape(match shape { + [b'1'] => Some(BufferShape::Circular), + [b'0'] => Some(BufferShape::Linear), + _ => None, + }?)} + }, + b"size" => { + let size = s.next()?; + QTBuffer::Configure { buffer: TraceBuffer::Size(match size { + [b'-', b'1'] => None, + i => Some(decode_hex(i).ok()?) + })} + }, + req => { + let mut req_opts = req.split(|b| *b == b','); + let (offset, length) = (req_opts.next()?, req_opts.next()?); + let offset = decode_hex(offset).ok()?; + let length = decode_hex(length).ok()?; + // Our response has to be a hex encoded buffer that fits within + // our packet size, which means we actually have half as much space + // as our slice would indicate. + let (front, _back) = buf.split_at_mut(buf.len() / 2); + QTBuffer::Request { offset, length, data: front } + }, + }) + + }, + _ => None, + } + } +} diff --git a/src/protocol/commands/_QTDP.rs b/src/protocol/commands/_QTDP.rs new file mode 100644 index 0000000..f681151 --- /dev/null +++ b/src/protocol/commands/_QTDP.rs @@ -0,0 +1,75 @@ +use super::prelude::*; +use crate::target::ext::tracepoints::Tracepoint; + +#[derive(Debug)] +pub enum QTDP<'a> { + Create(CreateTDP<'a>), + Define(DefineTDP<'a>), +} + +#[derive(Debug)] +pub struct CreateTDP<'a> { + pub number: Tracepoint, + pub addr: &'a [u8], + pub enable: bool, + pub step: u64, + pub pass: u64, + pub fast: Option<&'a [u8]>, + pub condition: Option<&'a [u8]>, + pub more: bool, +} + +#[derive(Debug)] +pub struct DefineTDP<'a> { + pub number: Tracepoint, + pub addr: &'a [u8], + pub while_stepping: bool, + pub actions: &'a mut [u8], +} + +impl<'a> ParseCommand<'a> for QTDP<'a> { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + let body = buf.into_body(); + match body { + [b':', b'-', actions @ ..] => { + let mut params = actions.splitn_mut(4, |b| matches!(*b, b':')); + let number = Tracepoint(decode_hex(params.next()?).ok()?); + let addr = decode_hex_buf(params.next()?).ok()?; + let actions = params.next()?; + Some(QTDP::Define(DefineTDP { + number, + addr, + while_stepping: false, + actions + })) + }, + [b':', tracepoint @ ..] => { + let more = tracepoint.ends_with(&[b'-']); + let mut params = tracepoint.splitn_mut(6, |b| matches!(*b, b':' | b'-')); + let n = Tracepoint(decode_hex(params.next()?).ok()?); + let addr = decode_hex_buf(params.next()?).ok()?; + let ena = params.next()?; + let step = decode_hex(params.next()?).ok()?; + let pass_and_end = params.next()?; + let pass = decode_hex(pass_and_end).ok()?; + // TODO: parse `F` fast tracepoint options + // TODO: parse `X` tracepoint conditions + let _options = params.next(); + return Some(QTDP::Create(CreateTDP { + number: n, + addr, + enable: match ena { [b'E'] => Some(true), [b'D'] => Some(false), _ => None }?, + step, + pass, + more, + // TODO: populate fast tracepoint options + // TODO: populate tracepoint conditions + fast: None, + condition: None, + })) + }, + _ => None, + } + } +} diff --git a/src/protocol/commands/_QTFrame.rs b/src/protocol/commands/_QTFrame.rs new file mode 100644 index 0000000..79002fb --- /dev/null +++ b/src/protocol/commands/_QTFrame.rs @@ -0,0 +1,42 @@ +use super::prelude::*; +use crate::target::ext::tracepoints::{Tracepoint, FrameRequest}; + +#[derive(Debug)] +pub struct QTFrame<'a>(pub FrameRequest<&'a mut [u8]>); + +impl<'a> ParseCommand<'a> for QTFrame<'a> { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + let body = buf.into_body(); + match body { + [b':', body @ ..] => { + let mut s = body.split_mut(|b| *b == b':'); + let selector = s.next()?; + Some(match selector.as_ref() { + b"pc" => { + let addr = decode_hex_buf(s.next()?).ok()?; + QTFrame(FrameRequest::AtPC(addr)) + }, + b"tdp" => { + let tp = Tracepoint(decode_hex(s.next()?).ok()?); + QTFrame(FrameRequest::Hit(tp)) + }, + b"range" => { + let start = decode_hex_buf(s.next()?).ok()?; + let end = decode_hex_buf(s.next()?).ok()?; + QTFrame(FrameRequest::Between(start, end)) + }, + b"outside" => { + let start = decode_hex_buf(s.next()?).ok()?; + let end = decode_hex_buf(s.next()?).ok()?; + QTFrame(FrameRequest::Outside(start, end)) + }, + n => { + QTFrame(FrameRequest::Select(decode_hex(n).ok()?)) + }, + }) + }, + _ => None, + } + } +} diff --git a/src/protocol/commands/_QTStart.rs b/src/protocol/commands/_QTStart.rs new file mode 100644 index 0000000..209d7f5 --- /dev/null +++ b/src/protocol/commands/_QTStart.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct QTStart; + +impl<'a> ParseCommand<'a> for QTStart { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(QTStart) + } +} diff --git a/src/protocol/commands/_QTStop.rs b/src/protocol/commands/_QTStop.rs new file mode 100644 index 0000000..4a31e67 --- /dev/null +++ b/src/protocol/commands/_QTStop.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct QTStop; + +impl<'a> ParseCommand<'a> for QTStop { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(QTStop) + } +} diff --git a/src/protocol/commands/_QTinit.rs b/src/protocol/commands/_QTinit.rs new file mode 100644 index 0000000..13d33fc --- /dev/null +++ b/src/protocol/commands/_QTinit.rs @@ -0,0 +1,11 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct QTinit { } + +impl<'a> ParseCommand<'a> for QTinit { + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { None } + else { Some(Self { }) } + } +} diff --git a/src/protocol/commands/_qTP.rs b/src/protocol/commands/_qTP.rs new file mode 100644 index 0000000..14520ba --- /dev/null +++ b/src/protocol/commands/_qTP.rs @@ -0,0 +1,27 @@ +use super::prelude::*; +use crate::target::ext::tracepoints::Tracepoint; + +#[derive(Debug)] +pub struct qTP<'a> { + pub tracepoint: Tracepoint, + pub addr: &'a [u8], +} + +impl<'a> ParseCommand<'a> for qTP<'a> { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + let body = buf.into_body(); + match body { + [b':', body @ ..] => { + let mut s = body.split_mut(|b| *b == b':'); + let tracepoint = Tracepoint(decode_hex(s.next()?).ok()?); + let addr = decode_hex_buf(s.next()?).ok()?; + Some(qTP { + tracepoint, + addr + }) + }, + _ => None, + } + } +} diff --git a/src/protocol/commands/_qTStatus.rs b/src/protocol/commands/_qTStatus.rs new file mode 100644 index 0000000..8463a57 --- /dev/null +++ b/src/protocol/commands/_qTStatus.rs @@ -0,0 +1,11 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTStatus { } + +impl<'a> ParseCommand<'a> for qTStatus { + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { None } + else { Some(Self { }) } + } +} diff --git a/src/protocol/commands/_qTfP.rs b/src/protocol/commands/_qTfP.rs new file mode 100644 index 0000000..6b8c960 --- /dev/null +++ b/src/protocol/commands/_qTfP.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTfP; + +impl<'a> ParseCommand<'a> for qTfP { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(qTfP) + } +} diff --git a/src/protocol/commands/_qTfV.rs b/src/protocol/commands/_qTfV.rs new file mode 100644 index 0000000..a669753 --- /dev/null +++ b/src/protocol/commands/_qTfV.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTfV; + +impl<'a> ParseCommand<'a> for qTfV { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(qTfV) + } +} diff --git a/src/protocol/commands/_qTsP.rs b/src/protocol/commands/_qTsP.rs new file mode 100644 index 0000000..bc8dfa7 --- /dev/null +++ b/src/protocol/commands/_qTsP.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTsP; + +impl<'a> ParseCommand<'a> for qTsP { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(qTsP) + } +} diff --git a/src/protocol/commands/_qTsV.rs b/src/protocol/commands/_qTsV.rs new file mode 100644 index 0000000..ef28e05 --- /dev/null +++ b/src/protocol/commands/_qTsV.rs @@ -0,0 +1,14 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTsV; + +impl<'a> ParseCommand<'a> for qTsV { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + if !buf.into_body().is_empty() { + return None; + } + Some(qTsV) + } +} diff --git a/src/stub/core_impl.rs b/src/stub/core_impl.rs index 2ea3bc8..d5ca4d5 100644 --- a/src/stub/core_impl.rs +++ b/src/stub/core_impl.rs @@ -42,6 +42,7 @@ mod section_offsets; mod single_register_access; mod target_xml; mod thread_extra_info; +mod tracepoints; mod x_upcase_packet; pub(crate) use resume::FinishExecStatus; @@ -218,6 +219,7 @@ impl GdbStubImpl { Command::ThreadExtraInfo(cmd) => self.handle_thread_extra_info(res, target, cmd), Command::LldbRegisterInfo(cmd) => self.handle_lldb_register_info(res, target, cmd), Command::LibrariesSvr4(cmd) => self.handle_libraries_svr4(res, target, cmd), + Command::Tracepoints(cmd) => self.handle_tracepoints(res, target, cmd), // in the worst case, the command could not be parsed... Command::Unknown(cmd) => { // HACK: if the user accidentally sends a resume command to a diff --git a/src/stub/core_impl/base.rs b/src/stub/core_impl/base.rs index c637f45..969dfd4 100644 --- a/src/stub/core_impl/base.rs +++ b/src/stub/core_impl/base.rs @@ -172,6 +172,11 @@ impl GdbStubImpl { } } + if let Some(_ops) = target.support_tracepoints() { + res.write_str(";InstallInTrace+")?; + res.write_str(";QTBuffer:size+")?; + } + if target.support_catch_syscalls().is_some() { res.write_str(";QCatchSyscalls+")?; } diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs new file mode 100644 index 0000000..e6bb06e --- /dev/null +++ b/src/stub/core_impl/tracepoints.rs @@ -0,0 +1,297 @@ +use super::prelude::*; +use crate::arch::Arch; +use crate::internal::BeBytes; +use crate::protocol::commands::_QTBuffer::QTBuffer; +use crate::protocol::commands::ext::Tracepoints; +use crate::protocol::commands::prelude::decode_hex; +use crate::protocol::commands::prelude::decode_hex_buf; +use crate::protocol::commands::_QTDP::CreateTDP; +use crate::protocol::commands::_QTDP::DefineTDP; +use crate::protocol::commands::_QTDP::QTDP; +use crate::target::ext::tracepoints::DefineTracepoint; +use crate::target::ext::tracepoints::FrameDescription; +use crate::target::ext::tracepoints::FrameRequest; +use crate::target::ext::tracepoints::NewTracepoint; +use crate::target::ext::tracepoints::TracepointAction; +use crate::target::ext::tracepoints::TracepointActionList; +use crate::target::ext::tracepoints::TracepointItem; + +impl NewTracepoint { + /// Parse from a raw CreateTDP packet. + pub fn from_tdp(ctdp: CreateTDP<'_>) -> Option { + let arch_int = |b: &[u8]| -> Result { U::from_be_bytes(b).ok_or(()) }; + Some(Self { + number: ctdp.number, + addr: arch_int(ctdp.addr).ok()?, + enabled: ctdp.enable, + pass_count: ctdp.pass, + step_count: ctdp.step, + more: ctdp.more, + }) + } +} + +impl<'a, U: BeBytes> DefineTracepoint<'a, U> { + /// Parse from a raw DefineTDP packet. + pub fn from_tdp(dtdp: DefineTDP<'a>) -> Option { + let arch_int = |b: &[u8]| -> Result { U::from_be_bytes(b).ok_or(()) }; + + Some(Self { + number: dtdp.number, + addr: arch_int(dtdp.addr).ok()?, + actions: TracepointActionList::Raw { data: dtdp.actions }, + }) + } + + /// Parse the actions that should be added to the definition of this + /// tracepoint, calling `f` on each action. + /// + /// Returns None if parsing of actions failed, or hit unsupported actions. + /// Return `Some(more)` on success, where `more` is a bool indicating if + /// there will be additional "tracepoint define" packets for this + /// tracepoint. + pub fn actions(self, mut f: impl FnMut(&TracepointAction<'_, U>)) -> Option { + match self.actions { + TracepointActionList::Raw { data } => Self::parse_raw_actions(data, f), + TracepointActionList::Parsed { mut actions, more } => { + for action in actions.iter_mut() { + (f)(action); + } + Some(more) + } + } + } + + fn parse_raw_actions( + actions: &mut [u8], + mut f: impl FnMut(&TracepointAction<'_, U>), + ) -> Option { + let mut more = false; + let mut unparsed: Option<&mut [u8]> = Some(actions); + loop { + match unparsed { + Some([b'S', ..]) => { + // TODO: how can gdbstub even implement this? it changes how + // future packets should be interpreted, but as a trait we + // can't keep a flag around for that (unless we specifically + // have a `mark_while_stepping` callback for the target to + // keep track future tracepoint_defines should be treated different). + // If we go that route we also would need to return two vectors + // here, "normal" actions and "while stepping" actions...but + // "normals" actions may still be "while stepping" actions, + // just continued from the previous packet, which we forgot + // about! + return None; + } + Some([b'R', mask @ ..]) => { + let mask_end = mask + .iter() + .cloned() + .enumerate() + .find(|(_i, b)| matches!(b, b'S' | b'R' | b'M' | b'X' | b'-')); + // We may or may not have another action after our mask + let mask = if let Some(mask_end) = mask_end { + let (mask_bytes, next) = mask.split_at_mut(mask_end.0); + unparsed = Some(next); + decode_hex_buf(mask_bytes).ok()? + } else { + unparsed = None; + decode_hex_buf(mask).ok()? + }; + (f)(&TracepointAction::Registers { mask }); + } + Some([b'M', _mem_args @ ..]) => { + // Unimplemented: even simple actions like `collect *(int*)0x0` + // are actually assembled as `X` bytecode actions + return None; + } + Some([b'X', eval_args @ ..]) => { + let mut len_end = eval_args.splitn_mut(2, |b| *b == b','); + let (len_bytes, rem) = (len_end.next()?, len_end.next()?); + let len: usize = decode_hex(len_bytes).ok()?; + let (expr_bytes, next_bytes) = rem.split_at_mut(len * 2); + let expr = decode_hex_buf(expr_bytes).ok()?; + (f)(&TracepointAction::Expression { expr }); + unparsed = Some(next_bytes); + } + Some([b'-']) => { + more = true; + break; + } + Some([]) | None => { + break; + } + _ => return None, + } + } + + Some(more) + } +} + +impl GdbStubImpl { + pub(crate) fn handle_tracepoints( + &mut self, + res: &mut ResponseWriter<'_, C>, + target: &mut T, + command: Tracepoints<'_>, + ) -> Result> { + let ops = match target.support_tracepoints() { + Some(ops) => ops, + None => return Ok(HandlerStatus::Handled), + }; + + crate::__dead_code_marker!("tracepoints", "impl"); + + match command { + Tracepoints::QTinit(_) => { + ops.tracepoints_init().handle_error()?; + // GDB documentation doesn't say it, but this requires "OK" in order + // to signify we support tracepoints. + return Ok(HandlerStatus::NeedsOk); + } + Tracepoints::qTStatus(_) => { + let status = ops.trace_experiment_status().handle_error()?; + res.write_str("T")?; + res.write_dec(if status.running { 1 } else { 0 })?; + for explanation in status.explanations.iter() { + res.write_str(";")?; + explanation.write(res)?; + } + } + Tracepoints::qTP(qtp) => { + let addr = ::Usize::from_be_bytes(qtp.addr) + .ok_or(Error::TargetMismatch)?; + let (hits, usage) = ops.tracepoint_status(qtp.tracepoint, addr).handle_error()?; + res.write_str("V")?; + res.write_num(hits)?; + res.write_str(":")?; + res.write_num(usage)?; + } + Tracepoints::QTDP(q) => { + match q { + QTDP::Create(ctdp) => { + let new_tracepoint = + NewTracepoint::<::Usize>::from_tdp(ctdp) + .ok_or(Error::TargetMismatch)?; + ops.tracepoint_create(new_tracepoint).handle_error()? + } + QTDP::Define(dtdp) => { + let define_tracepoint = + DefineTracepoint::<::Usize>::from_tdp(dtdp) + .ok_or(Error::TargetMismatch)?; + ops.tracepoint_define(define_tracepoint).handle_error()? + } + }; + // TODO: support qRelocInsn? + return Ok(HandlerStatus::NeedsOk); + } + Tracepoints::QTBuffer(buf) => { + match buf { + QTBuffer::Request { + offset, + length, + data, + } => { + let read = ops + .trace_buffer_request(offset, length, data) + .handle_error()?; + if let Some(read) = read { + let read = read.min(data.len()); + res.write_hex_buf(&data[..read])?; + } else { + res.write_str("l")?; + } + } + QTBuffer::Configure { buffer } => { + ops.trace_buffer_configure(buffer).handle_error()?; + } + } + // Documentation doesn't mention this, but it needs OK + return Ok(HandlerStatus::NeedsOk); + } + Tracepoints::QTStart(_) => { + ops.trace_experiment_start().handle_error()?; + // Documentation doesn't mention this, but it needs OK + // TODO: qRelocInsn reply support? + return Ok(HandlerStatus::NeedsOk); + } + Tracepoints::QTStop(_) => { + ops.trace_experiment_stop().handle_error()?; + // Documentation doesn't mention this, but it needs OK + return Ok(HandlerStatus::NeedsOk); + } + Tracepoints::QTFrame(req) => { + let parsed_qtframe: Option::Usize>> = req.0.into(); + let parsed_req = parsed_qtframe.ok_or(Error::TargetMismatch)?; + let mut err: Result<_, Error> = Ok(()); + let mut any_results = false; + ops.select_frame(parsed_req, &mut |desc| { + // TODO: bubble up the C::Error from this closure? list_thread_active does this + // instead + let e = (|| -> Result<_, _> { + match desc { + FrameDescription::FrameNumber(n) => { + res.write_str("F ")?; + if let Some(n) = n { + res.write_num(n)?; + } else { + res.write_str("-1")?; + } + } + FrameDescription::Hit(tdp) => { + res.write_str("T ")?; + res.write_num(tdp.0)?; + } + } + any_results = true; + Ok(()) + })(); + if let Err(e) = e { + err = Err(e) + } + }) + .handle_error()?; + if !any_results { + res.write_str("F -1")?; + } + } + // The GDB protocol for this is very weird: it sends this first packet + // to initialize a state machine on our stub, and then sends the subsequent + // packets N times in order to drive the state machine to the end in + // order to ask for all our tracepoints and their actions. gdbstub + // is agnostic about the end-user state and shouldn't allocate, however, + // which means we can't really setup the state machine ourselves or + // turn it into a nicer API. + Tracepoints::qTfP(_) => { + let tp = ops.tracepoint_enumerate_start().handle_error()?; + if let Some(tp) = tp { + match tp { + TracepointItem::New(ctp) => ctp.write(res)?, + TracepointItem::Define(dtp) => dtp.write(res)?, + } + } + } + Tracepoints::qTsP(_) => { + let tp = ops.tracepoint_enumerate_step().handle_error()?; + if let Some(tp) = tp { + match tp { + TracepointItem::New(ctp) => ctp.write(res)?, + TracepointItem::Define(dtp) => dtp.write(res)?, + } + } + } + + // Likewise, the same type of driven state machine is used for trace + // state variables + Tracepoints::qTfV(_) => { + // TODO: State variables + } + Tracepoints::qTsV(_) => { + // TODO: State variables + } + }; + + Ok(HandlerStatus::Handled) + } +} diff --git a/src/target/ext/mod.rs b/src/target/ext/mod.rs index cb18db9..ffdbdab 100644 --- a/src/target/ext/mod.rs +++ b/src/target/ext/mod.rs @@ -272,3 +272,4 @@ pub mod monitor_cmd; pub mod section_offsets; pub mod target_description_xml_override; pub mod thread_extra_info; +pub mod tracepoints; diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs new file mode 100644 index 0000000..881416e --- /dev/null +++ b/src/target/ext/tracepoints.rs @@ -0,0 +1,448 @@ +//! Provide tracepoints for the target. +use crate::conn::Connection; +use crate::protocol::ResponseWriter; +use crate::protocol::ResponseWriterError; +use crate::target::Arch; +use crate::target::Target; +use crate::target::TargetResult; +use managed::ManagedSlice; +use num_traits::PrimInt; + +/// A tracepoint, identified by a unique number. +#[derive(Debug, Clone, Copy)] +pub struct Tracepoint(pub usize); + +/// A state variable, identified by a unique number. +#[derive(Debug, Clone, Copy)] +pub struct StateVariable(usize); + +/// Describes a new tracepoint. It may be configured by later +/// [DefineTracepoint] structs. GDB may ask for the state of current +/// tracepoints, which are described with this same structure. +#[derive(Debug)] +pub struct NewTracepoint { + /// The tracepoint number + pub number: Tracepoint, + /// If the tracepoint is enabled or not + pub enabled: bool, + /// The address the tracepoint is set at. + pub addr: U, + /// The tracepoint's step count + pub step_count: u64, + /// The tracepoint's pass count. + pub pass_count: u64, + /// If there will be tracepoint "define" packets that follow this. + pub more: bool, +} + +impl NewTracepoint { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + res.write_str("QTDP:")?; + res.write_num(self.number.0)?; + res.write_str(":")?; + let mut buf = [0; 8]; + self.addr + .to_be_bytes(&mut buf) + .ok_or_else(|| unreachable!())?; + res.write_hex_buf(&buf)?; + res.write_str(":")?; + res.write_str(if self.enabled { "E" } else { "D" })?; + res.write_str(":")?; + res.write_num(self.step_count)?; + res.write_str(":")?; + res.write_num(self.pass_count)?; + + Ok(()) + } +} + +/// Describes how to collect information for a trace frame when the tracepoint +/// it is attached to is hit. A tracepoint may have more than one action +/// attached. +#[derive(Debug)] +#[allow(missing_docs)] +pub enum TracepointAction<'a, U> { + /// Collect the registers whose bits are set in `mask` (big endian). + /// Note that `mask` may be larger than the word length. + Registers { mask: &'a [u8] }, + /// Collect `len` bytes of memory starting at the address in register number + /// `basereg`, plus `offset`. If `basereg` is None, then treat it as a fixed + /// address. + Memory { + basereg: Option, + offset: U, + length: u64, + }, + /// Evaluate `expr`, which is a GDB agent bytecode expression, and collect + /// memory as it directs. + Expression { expr: &'a [u8] }, +} + +impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAction<'a, U> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + match self { + TracepointAction::Registers { mask } => { + res.write_str("R ")?; + res.write_hex_buf(mask)?; + } + TracepointAction::Memory { + basereg, + offset, + length, + } => { + res.write_str("M ")?; + match basereg { + Some(r) => res.write_num(*r), + None => res.write_str("-1"), + }?; + res.write_str(",")?; + let mut buf = [0; 8]; + offset.to_be_bytes(&mut buf).ok_or_else(|| unreachable!())?; + res.write_hex_buf(&buf)?; + res.write_str(",")?; + res.write_num(*length)?; + } + TracepointAction::Expression { expr } => { + res.write_str("X ")?; + res.write_num(expr.len())?; + res.write_str(",")?; + res.write_hex_buf(expr)?; + } + } + Ok(()) + } +} + +/// A list of TracepointActions, either raw and unparsed from a GDB packet, or +/// a slice of parsed structures like which may be returned from enumerating +/// tracepoints. +#[derive(Debug)] +#[allow(missing_docs)] +pub enum TracepointActionList<'a, U> { + /// Raw and unparsed actions, such as from GDB. + Raw { data: &'a mut [u8] }, + /// A slice of parsed actions, such as what may be returned by a target when + /// enumerating tracepoints. `more` must be set if there will be another + /// "tracepoint definition" with more actions for this tracepoint. + Parsed { + actions: ManagedSlice<'a, TracepointAction<'a, U>>, + more: bool, + }, +} + +/// Definition data for a tracepoint. A tracepoint may have more than one define +/// structure for all of its data. GDB may ask for the state of current +/// tracepoints, which are described with this same structure. +#[derive(Debug)] +pub struct DefineTracepoint<'a, U> { + /// The tracepoint that is having actions appended to its definition. + pub number: Tracepoint, + /// The PC address of the tracepoint that is being defined. + pub addr: U, + /// A list of actions that should be appended to the tracepoint. + pub actions: TracepointActionList<'a, U>, +} + +impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepoint<'a, U> { + pub(crate) fn write( + self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + res.write_str("QTDP:-")?; + res.write_num(self.number.0)?; + res.write_str(":")?; + let mut buf = [0; 8]; + self.addr + .to_be_bytes(&mut buf) + .ok_or_else(|| unreachable!())?; + res.write_hex_buf(&buf)?; + res.write_str(":")?; + let mut err = None; + let more = self.actions(|action| { + if let Err(e) = action.write(res) { + err = Some(e) + } + }); + if let Some(e) = err { + return Err(e); + } + if let Some(true) = more { + res.write_str("-")?; + } + + Ok(()) + } +} + +/// An item from a stream of tracepoint descriptions. Enumerating tracepoints +/// should emit a sequence of Create and Define items for all the tracepoints +/// that are loaded. +#[derive(Debug)] +pub enum TracepointItem<'a, U> { + /// Introduce a new tracepoint and describe its properties. This must be + /// emitted before any [TracepointItem::Define] items that use the same tracepoint + /// number, and must have the `more` flag set if it will be followed by + /// [TracepointItem::Define] items for this tracepoint. + New(NewTracepoint), + /// Define additional data for a tracepoint. This must be emitted after a + /// [TracepointItem::New] item that introduces the tracepoint number, and must have + /// the `more` flag set if it will be followed by more [TracepointItem::Define] items + /// for this tracepoint. + Define(DefineTracepoint<'a, U>), +} + +/// Description of the currently running trace experiment. +pub struct ExperimentStatus<'a> { + /// If a trace is presently running + pub running: bool, + /// A list of optional explanations for the trace status. + pub explanations: ManagedSlice<'a, ExperimentExplanation<'a>>, +} + +/// An explanation of some detail of the currently running trace experiment. +#[derive(Debug)] +pub enum ExperimentExplanation<'a> { + /// No trace has been ran yet. + NotRun, + /// The trace was stopped by the user. May contain an optional user-supplied + /// stop reason. + Stop(Option<&'a [u8]>), + /// The trace stopped because the buffer is full. + Full, + /// The trace stopped because GDB disconnect from the target. + Disconnected, + /// The trace stopped because the specified tracepoint exceeded its pass + /// count. + PassCount(Tracepoint), + /// The trace stopped because the specified tracepoint had an error. + Error(&'a [u8], Tracepoint), + /// The trace stopped for some other reason. + Unknown, + + // Statistical information + /// The number of trace frames in the buffer. + Frames(usize), + /// The total number of trace frames created during the run. This may be + /// larger than the trace frame count, if the buffer is circular. + Created(usize), + /// The total size of the trace buffer, in bytes. + Size(usize), + /// The number of bytes still unused in the buffer. + Free(usize), + /// The value of the circular trace buffer flag. True means the trace buffer + /// is circular and old trace frames will be discarded if necessary to + /// make room, false means that the trace buffer is linear and may fill + /// up. + Circular(bool), + /// The value of the disconnected tracing flag. True means that tracing will + /// continue after GDB disconnects, false means that the trace run will + /// stop. + Disconn(bool), + + /// Report a raw string as a trace status explanation. + Other(managed::Managed<'a, str>), +} + +impl<'a> ExperimentExplanation<'a> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + use ExperimentExplanation::*; + match self { + NotRun => res.write_str("tnotrun:0")?, + Stop(ref t) => match t { + Some(text) => { + res.write_str("tstop:")?; + res.write_hex_buf(text)?; + res.write_str(":0")?; + } + None => res.write_str("tstop:0")?, + }, + Full => res.write_str("tfull:0")?, + Disconnected => res.write_str("tdisconnected:0")?, + PassCount(tpnum) => { + res.write_str("tpasscount:")?; + res.write_num(tpnum.0)?; + } + Error(text, tpnum) => { + res.write_str("terror:")?; + res.write_hex_buf(text)?; + res.write_str(":")?; + res.write_num(tpnum.0)?; + } + Unknown => res.write_str("tunknown:0")?, + + Frames(u) => { + res.write_str("tframes:")?; + res.write_num(*u)?; + } + Created(u) => { + res.write_str("tcreated:")?; + res.write_num(*u)?; + } + Size(u) => { + res.write_str("tsize:")?; + res.write_num(*u)?; + } + Free(u) => { + res.write_str("tfree:")?; + res.write_num(*u)?; + } + Circular(u) => { + res.write_str("circular:")?; + res.write_num(if *u { 1 } else { 0 })?; + } + Disconn(dis) => match dis { + true => res.write_str("disconn:1")?, + false => res.write_str("disconn:0")?, + }, + Other(body) => res.write_str(body.as_ref())?, + }; + + Ok(()) + } +} + +/// Shape of the trace buffer +#[derive(Debug)] +pub enum BufferShape { + /// A circular trace buffer + Circular, + /// A linear trace buffer + Linear, +} + +/// Configuration for the trace buffer. +#[derive(Debug)] +pub enum TraceBuffer { + /// Set the buffer's shape. + Shape(BufferShape), + /// Set the buffer's size in bytes. If None, the target should use whatever + /// size it prefers. + Size(Option), +} + +/// Request to select a new frame from the trace buffer. +#[derive(Debug)] +pub enum FrameRequest { + /// Select the specified tracepoint frame in the buffer. + Select(u64), + /// Select a tracepoint frame that has a specified PC after the currently + /// selected frame. + AtPC(U), + /// Select a tracepoint frame that hit a specified tracepoint after the + /// currently selected frame. + Hit(Tracepoint), + /// Select a tramepoint frame that has a PC between a start (inclusive) and + /// end (inclusive). + Between(U, U), + /// Select a tracepoint frame that has a PC outside the range of addresses + /// (exclusive). + Outside(U, U), +} + +impl<'a, U: crate::internal::BeBytes> From> for Option> { + fn from(s: FrameRequest<&'a mut [u8]>) -> Self { + Some(match s { + FrameRequest::Select(u) => FrameRequest::Select(u), + FrameRequest::AtPC(u) => FrameRequest::AtPC(U::from_be_bytes(u)?), + FrameRequest::Hit(tp) => FrameRequest::Hit(tp), + FrameRequest::Between(s, e) => { + FrameRequest::Between(U::from_be_bytes(s)?, U::from_be_bytes(e)?) + } + FrameRequest::Outside(s, e) => { + FrameRequest::Outside(U::from_be_bytes(s)?, U::from_be_bytes(e)?) + } + }) + } +} + +/// Describes a detail of a frame from the trace buffer +#[derive(Debug)] +pub enum FrameDescription { + /// The frame is at the specified index in the trace buffer + FrameNumber(Option), + /// The frame is a hit of the specified tracepoint + Hit(Tracepoint), +} + +/// Target Extension - Provide tracepoints. +pub trait Tracepoints: Target { + /// Clear any saved tracepoints and empty the trace frame buffer + fn tracepoints_init(&mut self) -> TargetResult<(), Self>; + + /// Create a new tracepoint according to the description `tdp` + fn tracepoint_create( + &mut self, + tdp: NewTracepoint<::Usize>, + ) -> TargetResult<(), Self>; + /// Configure an existing tracepoint, appending new actions + fn tracepoint_define( + &mut self, + dtdp: DefineTracepoint<'_, ::Usize>, + ) -> TargetResult<(), Self>; + /// Request the status of tracepoint `tp` at address `addr`. + /// + /// Returns `(number of tracepoint hits, number of bytes used for frames)`. + fn tracepoint_status( + &self, + tp: Tracepoint, + addr: ::Usize, + ) -> TargetResult<(u64, u64), Self>; + + /// Begin enumerating tracepoints. The target implementation should + /// initialize a state machine that is stepped by + /// [Tracepoints::tracepoint_enumerate_step], and returns TracepointItems that + /// correspond with the currently configured tracepoints. + fn tracepoint_enumerate_start( + &mut self, + ) -> TargetResult::Usize>>, Self>; + /// Step the tracepoint enumeration state machine. The target implementation + /// should return TracepointItems that correspond with the currently + /// configured tracepoints. + fn tracepoint_enumerate_step( + &mut self, + ) -> TargetResult::Usize>>, Self>; + + /// Reconfigure the trace buffer to include or modify an attribute. + fn trace_buffer_configure(&mut self, tb: TraceBuffer) -> TargetResult<(), Self>; + + /// Return up to `len` bytes from the trace buffer, starting at `offset`. + /// The trace buffer is treated as a contiguous collection of traceframes, + /// as per [GDB's trace file format](https://sourceware.org/gdb/current/onlinedocs/gdb.html/Trace-File-Format.html). + /// The return value should be the number of bytes written. + fn trace_buffer_request( + &mut self, + offset: u64, + len: usize, + buf: &mut [u8], + ) -> TargetResult, Self>; + + /// Return the status of the current trace experiment. + fn trace_experiment_status(&self) -> TargetResult, Self>; + /// Start a new trace experiment. + fn trace_experiment_start(&mut self) -> TargetResult<(), Self>; + /// Stop the currently running trace experiment. + fn trace_experiment_stop(&mut self) -> TargetResult<(), Self>; + + /// Select a new frame in the trace buffer. The target should attempt to + /// fulfill the request according to the [FrameRequest]. If it's + /// successful it should call `report` with a series of calls describing + /// the found frame, and then record it as the currently selected frame. + /// Future register and memory requests should be fulfilled from the + /// currently selected frame. + fn select_frame( + &mut self, + frame: FrameRequest<::Usize>, + report: &mut dyn FnMut(FrameDescription), + ) -> TargetResult<(), Self>; +} + +define_ext!(TracepointsOps, Tracepoints); diff --git a/src/target/mod.rs b/src/target/mod.rs index da9541c..f7a5b51 100644 --- a/src/target/mod.rs +++ b/src/target/mod.rs @@ -626,6 +626,12 @@ pub trait Target { None } + /// Support for setting / removing tracepoints. + #[inline(always)] + fn support_tracepoints(&mut self) -> Option> { + None + } + /// Support for overriding the target description XML specified by /// `Target::Arch`. #[inline(always)] @@ -738,6 +744,7 @@ macro_rules! impl_dyn_target { __delegate_support!(host_io); __delegate_support!(exec_file); __delegate_support!(auxv); + __delegate_support!(tracepoints); } }; } From a470c6c98c0ce10977ac40346dca7b708e135039 Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 10 Dec 2024 16:58:54 -0500 Subject: [PATCH 02/29] wip get_owned --- examples/armv4t/emu.rs | 15 ++++ examples/armv4t/gdb/mod.rs | 8 ++ examples/armv4t/gdb/tracepoints.rs | 119 +++++++++++++++++++++++++++++ src/stub/core_impl/tracepoints.rs | 9 ++- src/target/ext/tracepoints.rs | 69 +++++++++++++++-- 5 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 examples/armv4t/gdb/tracepoints.rs diff --git a/examples/armv4t/emu.rs b/examples/armv4t/emu.rs index e94131b..b3c8229 100644 --- a/examples/armv4t/emu.rs +++ b/examples/armv4t/emu.rs @@ -1,5 +1,6 @@ use crate::mem_sniffer::AccessKind; use crate::mem_sniffer::MemSniffer; +use crate::gdb::tracepoints::TraceFrame; use crate::DynResult; use armv4t_emu::reg; use armv4t_emu::Cpu; @@ -7,6 +8,8 @@ use armv4t_emu::ExampleMem; use armv4t_emu::Memory; use armv4t_emu::Mode; use gdbstub::common::Pid; +use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem}; +use std::collections::HashMap; const HLE_RETURN_ADDR: u32 = 0x12345678; @@ -41,6 +44,12 @@ pub struct Emu { pub(crate) breakpoints: Vec, pub(crate) files: Vec>, + pub(crate) tracepoints: HashMap>>, + pub(crate) traceframes: Vec, + pub(crate) tracepoint_enumerate_machine: (Vec>, usize), + pub(crate) tracing: bool, + pub(crate) selected_frame: Option, + pub(crate) reported_pid: Pid, } @@ -93,6 +102,12 @@ impl Emu { breakpoints: Vec::new(), files: Vec::new(), + tracepoints: HashMap::new(), + traceframes: Vec::new(), + tracepoint_enumerate_machine: (Vec::new(), 0), + tracing: false, + selected_frame: None, + reported_pid: Pid::new(1).unwrap(), }) } diff --git a/examples/armv4t/gdb/mod.rs b/examples/armv4t/gdb/mod.rs index d4190dc..09a5ba7 100644 --- a/examples/armv4t/gdb/mod.rs +++ b/examples/armv4t/gdb/mod.rs @@ -26,6 +26,7 @@ mod memory_map; mod monitor_cmd; mod section_offsets; mod target_description_xml_override; +pub(crate) mod tracepoints; /// Turn a `ArmCoreRegId` into an internal register number of `armv4t_emu`. fn cpu_reg_id(id: ArmCoreRegId) -> Option { @@ -161,6 +162,13 @@ impl Target for Emu { ) -> Option> { Some(self) } + + #[inline(always)] + fn support_tracepoints( + &mut self, + ) -> Option> { + Some(self) + } } impl SingleThreadBase for Emu { diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs new file mode 100644 index 0000000..f7d7709 --- /dev/null +++ b/examples/armv4t/gdb/tracepoints.rs @@ -0,0 +1,119 @@ +use crate::emu::Emu; +use gdbstub::target; +use gdbstub::target::TargetResult; +use gdbstub::target::TargetError; +use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem, NewTracepoint, DefineTracepoint, ExperimentStatus, FrameRequest, FrameDescription, TraceBuffer}; +use managed::ManagedSlice; + +use armv4t_emu::Cpu; +pub struct TraceFrame { + number: Tracepoint, + addr: u32, + snapshot: Cpu, +} + +impl target::ext::tracepoints::Tracepoints for Emu { + fn tracepoints_init(&mut self) -> TargetResult<(), Self> { + self.tracepoints.clear(); + self.traceframes.clear(); + Ok(()) + } + + fn tracepoint_create(&mut self, tp: NewTracepoint) -> TargetResult<(), Self> { + self.tracepoints.insert(tp.addr, vec![TracepointItem::New(tp)]); + Ok(()) + } + + fn tracepoint_define(&mut self, tp: DefineTracepoint<'_, u32>) -> TargetResult<(), Self> { + self.tracepoints.get_mut(&tp.addr).map(move |existing| { + existing.push(TracepointItem::Define(tp.get_owned())); + () + }).ok_or_else(move || TargetError::Fatal("define on non-existing tracepoint")) + } + + fn tracepoint_status(&self, tp: Tracepoint, addr: u32) -> TargetResult<(u64, u64), Self> { + // We don't collect "real" trace buffer frames, so just report hit count + // and say the number of bytes is always 0 + Ok((self.traceframes.iter().filter(|frame| frame.number.0 == tp.0).count() as u64, 0)) + } + + fn tracepoint_enumerate_start(&mut self) -> TargetResult>, Self> { + let tracepoints: Vec<_> = self.tracepoints.iter().flat_map(|(key, value)| { + value.iter().map(|item| item.get_owned()) + }).collect(); + self.tracepoint_enumerate_machine = (tracepoints, 0); + + self.tracepoint_enumerate_step() + } + + fn tracepoint_enumerate_step( + &mut self, + ) -> TargetResult>, Self> { + let (tracepoints, index) = &mut self.tracepoint_enumerate_machine; + if let Some(item) = tracepoints.iter().nth(*index) { + *index += 1; + let item2: TracepointItem<'static, u32> = item.get_owned(); + dbg!(&item2); + Ok(Some(item2)) + } else { + Ok(None) + } + } + + fn trace_buffer_configure(&mut self, tb: TraceBuffer) -> TargetResult<(), Self> { + // we don't collect a "real" trace buffer, so just ignore configuration attempts. + Ok(()) + } + + fn trace_buffer_request( + &mut self, + offset: u64, + len: usize, + buf: &mut [u8], + ) -> TargetResult, Self> { + // We don't have a "real" trace buffer, so fail all raw read requests. + Ok(None) + } + + fn trace_experiment_status(&self) -> TargetResult, Self> { + // For a bare-bones example, we don't provide in-depth status explanations. + Ok(ExperimentStatus { + running: self.tracing, + explanations: ManagedSlice::Owned(vec![]), + }) + } + + fn select_frame( + &mut self, + frame: FrameRequest, + report: &mut dyn FnMut(FrameDescription), + ) -> TargetResult<(), Self> { + // For a bare-bones example, we only support `tfind ` style frame + // selection and not the more complicated ones. + match frame { + FrameRequest::Select(n) => { + self.selected_frame = self.traceframes.iter().nth(n as usize).map(|frame| { + (report)(FrameDescription::FrameNumber(Some(n))); + (report)(FrameDescription::Hit(frame.number)); + Some(n as usize) + }).unwrap_or_else(|| { + (report)(FrameDescription::FrameNumber(None)); + None + }); + Ok(()) + }, + _ => Err(TargetError::NonFatal), + } + } + + fn trace_experiment_start(&mut self) -> TargetResult<(), Self> { + self.tracing = true; + Ok(()) + } + + fn trace_experiment_stop(&mut self) -> TargetResult<(), Self> { + self.tracing = false; + Ok(()) + } +} + diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index e6bb06e..d342e37 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -15,6 +15,7 @@ use crate::target::ext::tracepoints::NewTracepoint; use crate::target::ext::tracepoints::TracepointAction; use crate::target::ext::tracepoints::TracepointActionList; use crate::target::ext::tracepoints::TracepointItem; +use managed::ManagedSlice; impl NewTracepoint { /// Parse from a raw CreateTDP packet. @@ -39,7 +40,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { Some(Self { number: dtdp.number, addr: arch_int(dtdp.addr).ok()?, - actions: TracepointActionList::Raw { data: dtdp.actions }, + actions: TracepointActionList::Raw { data: ManagedSlice::Borrowed(dtdp.actions) }, }) } @@ -52,7 +53,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// tracepoint. pub fn actions(self, mut f: impl FnMut(&TracepointAction<'_, U>)) -> Option { match self.actions { - TracepointActionList::Raw { data } => Self::parse_raw_actions(data, f), + TracepointActionList::Raw { mut data } => Self::parse_raw_actions(&mut data, f), TracepointActionList::Parsed { mut actions, more } => { for action in actions.iter_mut() { (f)(action); @@ -98,7 +99,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { unparsed = None; decode_hex_buf(mask).ok()? }; - (f)(&TracepointAction::Registers { mask }); + (f)(&TracepointAction::Registers { mask: ManagedSlice::Borrowed(mask) }); } Some([b'M', _mem_args @ ..]) => { // Unimplemented: even simple actions like `collect *(int*)0x0` @@ -111,7 +112,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { let len: usize = decode_hex(len_bytes).ok()?; let (expr_bytes, next_bytes) = rem.split_at_mut(len * 2); let expr = decode_hex_buf(expr_bytes).ok()?; - (f)(&TracepointAction::Expression { expr }); + (f)(&TracepointAction::Expression { expr: ManagedSlice::Borrowed(expr) }); unparsed = Some(next_bytes); } Some([b'-']) => { diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 881416e..3382e6b 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -7,6 +7,7 @@ use crate::target::Target; use crate::target::TargetResult; use managed::ManagedSlice; use num_traits::PrimInt; +use std::ops::Deref; /// A tracepoint, identified by a unique number. #[derive(Debug, Clone, Copy)] @@ -19,7 +20,7 @@ pub struct StateVariable(usize); /// Describes a new tracepoint. It may be configured by later /// [DefineTracepoint] structs. GDB may ask for the state of current /// tracepoints, which are described with this same structure. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct NewTracepoint { /// The tracepoint number pub number: Tracepoint, @@ -35,6 +36,14 @@ pub struct NewTracepoint { pub more: bool, } +#[cfg(feature = "alloc")] +impl NewTracepoint { + pub fn get_owned(&self) -> NewTracepoint { + self.clone() + } +} + + impl NewTracepoint { pub(crate) fn write( &self, @@ -67,7 +76,7 @@ impl NewTracepoint pub enum TracepointAction<'a, U> { /// Collect the registers whose bits are set in `mask` (big endian). /// Note that `mask` may be larger than the word length. - Registers { mask: &'a [u8] }, + Registers { mask: ManagedSlice<'a, u8> }, /// Collect `len` bytes of memory starting at the address in register number /// `basereg`, plus `offset`. If `basereg` is None, then treat it as a fixed /// address. @@ -78,7 +87,18 @@ pub enum TracepointAction<'a, U> { }, /// Evaluate `expr`, which is a GDB agent bytecode expression, and collect /// memory as it directs. - Expression { expr: &'a [u8] }, + Expression { expr: ManagedSlice<'a, u8> }, +} + +#[cfg(feature = "alloc")] +impl<'a, U: Copy> TracepointAction<'a, U> { + pub fn new_owned(&self) -> TracepointAction<'static, U> { + match self { + TracepointAction::Registers { mask } => TracepointAction::Registers { mask: ManagedSlice::Owned(mask.deref().into()) }, + TracepointAction::Memory { basereg, offset, length } => TracepointAction::Memory { basereg: *basereg, offset: *offset, length: *length }, + TracepointAction::Expression { expr } => TracepointAction::Expression { expr: ManagedSlice::Owned(expr.deref().into()) }, + } + } } impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAction<'a, U> { @@ -124,18 +144,31 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct /// tracepoints. #[derive(Debug)] #[allow(missing_docs)] -pub enum TracepointActionList<'a, U> { +pub enum TracepointActionList<'a, 'b, U> { /// Raw and unparsed actions, such as from GDB. - Raw { data: &'a mut [u8] }, + Raw { data: ManagedSlice<'a, u8> }, /// A slice of parsed actions, such as what may be returned by a target when /// enumerating tracepoints. `more` must be set if there will be another /// "tracepoint definition" with more actions for this tracepoint. Parsed { - actions: ManagedSlice<'a, TracepointAction<'a, U>>, + actions: ManagedSlice<'a, TracepointAction<'b, U>>, more: bool, }, } +#[cfg(feature = "alloc")] +impl<'a, 'b, U: Copy> TracepointActionList<'a, 'b, U> { + pub fn get_owned(&self) -> TracepointActionList<'static, 'static, U> { + match self { + TracepointActionList::Raw { data } => TracepointActionList::Raw { data: ManagedSlice::Owned(data.deref().into()) }, + TracepointActionList::Parsed { actions, more } => TracepointActionList::Parsed { + actions: ManagedSlice::Owned(actions.iter().map(|action| action.new_owned()).collect()), + more: *more + }, + } + } +} + /// Definition data for a tracepoint. A tracepoint may have more than one define /// structure for all of its data. GDB may ask for the state of current /// tracepoints, which are described with this same structure. @@ -146,7 +179,18 @@ pub struct DefineTracepoint<'a, U> { /// The PC address of the tracepoint that is being defined. pub addr: U, /// A list of actions that should be appended to the tracepoint. - pub actions: TracepointActionList<'a, U>, + pub actions: TracepointActionList<'a, 'a, U>, +} + +#[cfg(feature = "alloc")] +impl<'a, U: Copy> DefineTracepoint<'a, U> { + pub fn get_owned(&self) -> DefineTracepoint<'static, U> { + DefineTracepoint { + number: self.number, + addr: self.addr, + actions: self.actions.get_owned() + } + } } impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepoint<'a, U> { @@ -197,6 +241,17 @@ pub enum TracepointItem<'a, U> { Define(DefineTracepoint<'a, U>), } +#[cfg(feature = "alloc")] +impl<'a, U: Copy> TracepointItem<'a, U> { + pub fn get_owned(&self) -> TracepointItem<'static, U> { + match self { + TracepointItem::New(n) => TracepointItem::New(n.get_owned()), + TracepointItem::Define(d) => TracepointItem::Define(d.get_owned()), + } + } +} + + /// Description of the currently running trace experiment. pub struct ExperimentStatus<'a> { /// If a trace is presently running From f5b5d3cb86e4cf09efc91adabdfca2b9104e381d Mon Sep 17 00:00:00 2001 From: cczetier Date: Wed, 11 Dec 2024 13:20:27 -0500 Subject: [PATCH 03/29] add armv4t tracepoint support --- examples/armv4t/emu.rs | 24 +++++- examples/armv4t/gdb/mod.rs | 33 ++++++-- examples/armv4t/gdb/tracepoints.rs | 118 +++++++++++++++++------------ src/target/ext/tracepoints.rs | 30 +++++--- 4 files changed, 138 insertions(+), 67 deletions(-) diff --git a/examples/armv4t/emu.rs b/examples/armv4t/emu.rs index b3c8229..ea4b343 100644 --- a/examples/armv4t/emu.rs +++ b/examples/armv4t/emu.rs @@ -44,7 +44,7 @@ pub struct Emu { pub(crate) breakpoints: Vec, pub(crate) files: Vec>, - pub(crate) tracepoints: HashMap>>, + pub(crate) tracepoints: HashMap>>, pub(crate) traceframes: Vec, pub(crate) tracepoint_enumerate_machine: (Vec>, usize), pub(crate) tracing: bool, @@ -121,6 +121,28 @@ impl Emu { /// single-step the interpreter pub fn step(&mut self) -> Option { + if self.tracing { + let pc = self.cpu.reg_get(self.cpu.mode(), reg::PC); + let frames: Vec<_> = self.tracepoints.iter().filter(|(_tracepoint, definition)| { + if let Some(TracepointItem::New(new)) = definition.get(0) { + new.enabled && new.addr == pc + } else { + false + } + }).map(|(tracepoint, _definition)| { + // our `tracepoint_define` restricts our loaded tracepoints to only contain + // register collect actions. instead of only collecting the registers requested + // in the register mask and recording a minimal trace frame, we just collect + // all of them by cloning the cpu itself. + TraceFrame { + number: *tracepoint, + addr: pc, + snapshot: self.cpu.clone(), + } + }).collect(); + self.traceframes.extend(frames); + } + let mut hit_watchpoint = None; let mut sniffer = MemSniffer::new(&mut self.mem, &self.watchpoints, |access| { diff --git a/examples/armv4t/gdb/mod.rs b/examples/armv4t/gdb/mod.rs index 09a5ba7..eb59fe4 100644 --- a/examples/armv4t/gdb/mod.rs +++ b/examples/armv4t/gdb/mod.rs @@ -176,15 +176,21 @@ impl SingleThreadBase for Emu { &mut self, regs: &mut custom_arch::ArmCoreRegsCustom, ) -> TargetResult<(), Self> { - let mode = self.cpu.mode(); + // if we selected a frame from a tracepoint, return registers from that snapshot + let cpu = self.selected_frame.and_then(|selected| { + self.traceframes.get(selected) + }).map(|frame| { + frame.snapshot + }).unwrap_or_else(|| self.cpu); + let mode = cpu.mode(); for i in 0..13 { - regs.core.r[i] = self.cpu.reg_get(mode, i as u8); + regs.core.r[i] = cpu.reg_get(mode, i as u8); } - regs.core.sp = self.cpu.reg_get(mode, reg::SP); - regs.core.lr = self.cpu.reg_get(mode, reg::LR); - regs.core.pc = self.cpu.reg_get(mode, reg::PC); - regs.core.cpsr = self.cpu.reg_get(mode, reg::CPSR); + regs.core.sp = cpu.reg_get(mode, reg::SP); + regs.core.lr = cpu.reg_get(mode, reg::LR); + regs.core.pc = cpu.reg_get(mode, reg::PC); + regs.core.cpsr = cpu.reg_get(mode, reg::CPSR); regs.custom = self.custom_reg; @@ -192,6 +198,10 @@ impl SingleThreadBase for Emu { } fn write_registers(&mut self, regs: &custom_arch::ArmCoreRegsCustom) -> TargetResult<(), Self> { + if self.selected_frame.is_some() { + // we can't modify registers in a tracepoint frame + return Err(TargetError::NonFatal); + } let mode = self.cpu.mode(); for i in 0..13 { @@ -216,6 +226,12 @@ impl SingleThreadBase for Emu { } fn read_addrs(&mut self, start_addr: u32, data: &mut [u8]) -> TargetResult { + if self.selected_frame.is_some() { + // we only support register collection actions for our tracepoint frames. + // if we have a selected frame, then we don't have any memory we can + // return from the frame snapshot. + return Ok(0) + } // this is a simple emulator, with RAM covering the entire 32 bit address space for (addr, val) in (start_addr..).zip(data.iter_mut()) { *val = self.mem.r8(addr) @@ -224,6 +240,11 @@ impl SingleThreadBase for Emu { } fn write_addrs(&mut self, start_addr: u32, data: &[u8]) -> TargetResult<(), Self> { + if self.selected_frame.is_some() { + // we can't modify memory in a tracepoint frame + return Err(TargetError::NonFatal); + } + // this is a simple emulator, with RAM covering the entire 32 bit address space for (addr, val) in (start_addr..).zip(data.iter().copied()) { self.mem.w8(addr, val) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index f7d7709..67e8cae 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -2,14 +2,15 @@ use crate::emu::Emu; use gdbstub::target; use gdbstub::target::TargetResult; use gdbstub::target::TargetError; -use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem, NewTracepoint, DefineTracepoint, ExperimentStatus, FrameRequest, FrameDescription, TraceBuffer}; +use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem, NewTracepoint, DefineTracepoint, ExperimentStatus, FrameRequest, FrameDescription, TraceBuffer, ExperimentExplanation, TracepointAction}; use managed::ManagedSlice; use armv4t_emu::Cpu; +#[derive(Debug)] pub struct TraceFrame { - number: Tracepoint, - addr: u32, - snapshot: Cpu, + pub number: Tracepoint, + pub addr: u32, + pub snapshot: Cpu, } impl target::ext::tracepoints::Tracepoints for Emu { @@ -20,25 +21,39 @@ impl target::ext::tracepoints::Tracepoints for Emu { } fn tracepoint_create(&mut self, tp: NewTracepoint) -> TargetResult<(), Self> { - self.tracepoints.insert(tp.addr, vec![TracepointItem::New(tp)]); + self.tracepoints.insert(tp.number, vec![TracepointItem::New(tp)]); Ok(()) } fn tracepoint_define(&mut self, tp: DefineTracepoint<'_, u32>) -> TargetResult<(), Self> { - self.tracepoints.get_mut(&tp.addr).map(move |existing| { - existing.push(TracepointItem::Define(tp.get_owned())); + let tp_copy = tp.get_owned(); + let mut valid = true; + tp.actions(|action| { + if let TracepointAction::Registers { mask: _ } = action { + // we only handle register collection actions for the simple case + } else { + valid = false; + } + }); + if !valid { + return Err(TargetError::NonFatal) + } + self.tracepoints.get_mut(&tp_copy.number).map(move |existing| { + existing.push(TracepointItem::Define(tp_copy)); () }).ok_or_else(move || TargetError::Fatal("define on non-existing tracepoint")) } - fn tracepoint_status(&self, tp: Tracepoint, addr: u32) -> TargetResult<(u64, u64), Self> { + fn tracepoint_status(&self, tp: Tracepoint, _addr: u32) -> TargetResult<(u64, u64), Self> { // We don't collect "real" trace buffer frames, so just report hit count - // and say the number of bytes is always 0 + // and say the number of bytes is always 0. + // Because we don't implement "while-stepping" actions, we don't need to + // also check that `addr` matches. Ok((self.traceframes.iter().filter(|frame| frame.number.0 == tp.0).count() as u64, 0)) } fn tracepoint_enumerate_start(&mut self) -> TargetResult>, Self> { - let tracepoints: Vec<_> = self.tracepoints.iter().flat_map(|(key, value)| { + let tracepoints: Vec<_> = self.tracepoints.iter().flat_map(|(_key, value)| { value.iter().map(|item| item.get_owned()) }).collect(); self.tracepoint_enumerate_machine = (tracepoints, 0); @@ -46,30 +61,28 @@ impl target::ext::tracepoints::Tracepoints for Emu { self.tracepoint_enumerate_step() } - fn tracepoint_enumerate_step( - &mut self, - ) -> TargetResult>, Self> { + fn tracepoint_enumerate_step<'a>( + &'a mut self, + ) -> TargetResult>, Self> { let (tracepoints, index) = &mut self.tracepoint_enumerate_machine; if let Some(item) = tracepoints.iter().nth(*index) { *index += 1; - let item2: TracepointItem<'static, u32> = item.get_owned(); - dbg!(&item2); - Ok(Some(item2)) + Ok(Some(item.get_owned())) } else { Ok(None) } } - fn trace_buffer_configure(&mut self, tb: TraceBuffer) -> TargetResult<(), Self> { + fn trace_buffer_configure(&mut self, _tb: TraceBuffer) -> TargetResult<(), Self> { // we don't collect a "real" trace buffer, so just ignore configuration attempts. Ok(()) } fn trace_buffer_request( &mut self, - offset: u64, - len: usize, - buf: &mut [u8], + _offset: u64, + _len: usize, + _buf: &mut [u8], ) -> TargetResult, Self> { // We don't have a "real" trace buffer, so fail all raw read requests. Ok(None) @@ -79,41 +92,50 @@ impl target::ext::tracepoints::Tracepoints for Emu { // For a bare-bones example, we don't provide in-depth status explanations. Ok(ExperimentStatus { running: self.tracing, - explanations: ManagedSlice::Owned(vec![]), + explanations: ManagedSlice::Owned(vec![ExperimentExplanation::Frames(self.traceframes.len())]), }) } - fn select_frame( + fn select_frame( &mut self, frame: FrameRequest, report: &mut dyn FnMut(FrameDescription), - ) -> TargetResult<(), Self> { - // For a bare-bones example, we only support `tfind ` style frame - // selection and not the more complicated ones. - match frame { - FrameRequest::Select(n) => { - self.selected_frame = self.traceframes.iter().nth(n as usize).map(|frame| { - (report)(FrameDescription::FrameNumber(Some(n))); - (report)(FrameDescription::Hit(frame.number)); - Some(n as usize) - }).unwrap_or_else(|| { - (report)(FrameDescription::FrameNumber(None)); - None - }); - Ok(()) - }, - _ => Err(TargetError::NonFatal), - } - } + ) -> TargetResult<(), Self> { + // For a bare-bones example, we only support `tfind ` and `tfind tracepoint ` + // style frame selection and not the more complicated ones. + let found = match frame { + FrameRequest::Select(n) => { + self.traceframes.iter().nth(n as usize).map(|frame| (n, frame)) + }, + FrameRequest::Hit(tp) => { + let start = self.selected_frame.map(|selected| selected + 1).unwrap_or(0); + self.traceframes.get(start..).and_then(|frames| { + frames.iter().enumerate().filter(|(_n, frame)| { + frame.number == tp + }).map(|(n, frame)| ((start + n) as u64, frame) ).next() + }) + }, + _ => return Err(TargetError::NonFatal), + }; + if let Some((n, frame)) = found { + (report)(FrameDescription::FrameNumber(Some(n))); + (report)(FrameDescription::Hit(frame.number)); + self.selected_frame = Some(n as usize); + } else { + (report)(FrameDescription::FrameNumber(None)); + self.selected_frame = None; + } + Ok(()) + } - fn trace_experiment_start(&mut self) -> TargetResult<(), Self> { - self.tracing = true; - Ok(()) - } + fn trace_experiment_start(&mut self) -> TargetResult<(), Self> { + self.tracing = true; + Ok(()) + } - fn trace_experiment_stop(&mut self) -> TargetResult<(), Self> { - self.tracing = false; - Ok(()) - } + fn trace_experiment_stop(&mut self) -> TargetResult<(), Self> { + self.tracing = false; + Ok(()) + } } diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 3382e6b..2a36a8f 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -7,14 +7,13 @@ use crate::target::Target; use crate::target::TargetResult; use managed::ManagedSlice; use num_traits::PrimInt; -use std::ops::Deref; /// A tracepoint, identified by a unique number. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Tracepoint(pub usize); /// A state variable, identified by a unique number. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct StateVariable(usize); /// Describes a new tracepoint. It may be configured by later @@ -38,6 +37,7 @@ pub struct NewTracepoint { #[cfg(feature = "alloc")] impl NewTracepoint { + /// Allocate an owned copy of this structure. pub fn get_owned(&self) -> NewTracepoint { self.clone() } @@ -92,7 +92,9 @@ pub enum TracepointAction<'a, U> { #[cfg(feature = "alloc")] impl<'a, U: Copy> TracepointAction<'a, U> { - pub fn new_owned(&self) -> TracepointAction<'static, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointAction<'b, U> { + use core::ops::Deref; match self { TracepointAction::Registers { mask } => TracepointAction::Registers { mask: ManagedSlice::Owned(mask.deref().into()) }, TracepointAction::Memory { basereg, offset, length } => TracepointAction::Memory { basereg: *basereg, offset: *offset, length: *length }, @@ -144,25 +146,27 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct /// tracepoints. #[derive(Debug)] #[allow(missing_docs)] -pub enum TracepointActionList<'a, 'b, U> { +pub enum TracepointActionList<'a, U> { /// Raw and unparsed actions, such as from GDB. Raw { data: ManagedSlice<'a, u8> }, /// A slice of parsed actions, such as what may be returned by a target when /// enumerating tracepoints. `more` must be set if there will be another /// "tracepoint definition" with more actions for this tracepoint. Parsed { - actions: ManagedSlice<'a, TracepointAction<'b, U>>, + actions: ManagedSlice<'a, TracepointAction<'a, U>>, more: bool, }, } #[cfg(feature = "alloc")] -impl<'a, 'b, U: Copy> TracepointActionList<'a, 'b, U> { - pub fn get_owned(&self) -> TracepointActionList<'static, 'static, U> { +impl<'a, U: Copy> TracepointActionList<'a, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointActionList<'b, U> { + use core::ops::Deref; match self { TracepointActionList::Raw { data } => TracepointActionList::Raw { data: ManagedSlice::Owned(data.deref().into()) }, TracepointActionList::Parsed { actions, more } => TracepointActionList::Parsed { - actions: ManagedSlice::Owned(actions.iter().map(|action| action.new_owned()).collect()), + actions: ManagedSlice::Owned(actions.iter().map(|action| action.get_owned()).collect()), more: *more }, } @@ -179,12 +183,13 @@ pub struct DefineTracepoint<'a, U> { /// The PC address of the tracepoint that is being defined. pub addr: U, /// A list of actions that should be appended to the tracepoint. - pub actions: TracepointActionList<'a, 'a, U>, + pub actions: TracepointActionList<'a, U>, } #[cfg(feature = "alloc")] impl<'a, U: Copy> DefineTracepoint<'a, U> { - pub fn get_owned(&self) -> DefineTracepoint<'static, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> DefineTracepoint<'b, U> { DefineTracepoint { number: self.number, addr: self.addr, @@ -243,7 +248,8 @@ pub enum TracepointItem<'a, U> { #[cfg(feature = "alloc")] impl<'a, U: Copy> TracepointItem<'a, U> { - pub fn get_owned(&self) -> TracepointItem<'static, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointItem<'b, U> { match self { TracepointItem::New(n) => TracepointItem::New(n.get_owned()), TracepointItem::Define(d) => TracepointItem::Define(d.get_owned()), From 9b5f8b79a490e7b1e662e2cf60d9ed483d5ff806 Mon Sep 17 00:00:00 2001 From: cczetier Date: Wed, 11 Dec 2024 18:22:31 -0500 Subject: [PATCH 04/29] run cargo fmt --- examples/armv4t/emu.rs | 44 ++++++++------- examples/armv4t/gdb/mod.rs | 12 ++-- examples/armv4t/gdb/tracepoints.rs | 89 +++++++++++++++++++++--------- src/stub/core_impl/tracepoints.rs | 12 +++- src/target/ext/tracepoints.rs | 48 ++++++++++------ 5 files changed, 133 insertions(+), 72 deletions(-) diff --git a/examples/armv4t/emu.rs b/examples/armv4t/emu.rs index ea4b343..1baf33e 100644 --- a/examples/armv4t/emu.rs +++ b/examples/armv4t/emu.rs @@ -1,6 +1,6 @@ +use crate::gdb::tracepoints::TraceFrame; use crate::mem_sniffer::AccessKind; use crate::mem_sniffer::MemSniffer; -use crate::gdb::tracepoints::TraceFrame; use crate::DynResult; use armv4t_emu::reg; use armv4t_emu::Cpu; @@ -8,7 +8,8 @@ use armv4t_emu::ExampleMem; use armv4t_emu::Memory; use armv4t_emu::Mode; use gdbstub::common::Pid; -use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem}; +use gdbstub::target::ext::tracepoints::Tracepoint; +use gdbstub::target::ext::tracepoints::TracepointItem; use std::collections::HashMap; const HLE_RETURN_ADDR: u32 = 0x12345678; @@ -123,23 +124,28 @@ impl Emu { pub fn step(&mut self) -> Option { if self.tracing { let pc = self.cpu.reg_get(self.cpu.mode(), reg::PC); - let frames: Vec<_> = self.tracepoints.iter().filter(|(_tracepoint, definition)| { - if let Some(TracepointItem::New(new)) = definition.get(0) { - new.enabled && new.addr == pc - } else { - false - } - }).map(|(tracepoint, _definition)| { - // our `tracepoint_define` restricts our loaded tracepoints to only contain - // register collect actions. instead of only collecting the registers requested - // in the register mask and recording a minimal trace frame, we just collect - // all of them by cloning the cpu itself. - TraceFrame { - number: *tracepoint, - addr: pc, - snapshot: self.cpu.clone(), - } - }).collect(); + let frames: Vec<_> = self + .tracepoints + .iter() + .filter(|(_tracepoint, definition)| { + if let Some(TracepointItem::New(new)) = definition.get(0) { + new.enabled && new.addr == pc + } else { + false + } + }) + .map(|(tracepoint, _definition)| { + // our `tracepoint_define` restricts our loaded tracepoints to only contain + // register collect actions. instead of only collecting the registers requested + // in the register mask and recording a minimal trace frame, we just collect + // all of them by cloning the cpu itself. + TraceFrame { + number: *tracepoint, + addr: pc, + snapshot: self.cpu.clone(), + } + }) + .collect(); self.traceframes.extend(frames); } diff --git a/examples/armv4t/gdb/mod.rs b/examples/armv4t/gdb/mod.rs index eb59fe4..ab8a8c0 100644 --- a/examples/armv4t/gdb/mod.rs +++ b/examples/armv4t/gdb/mod.rs @@ -177,11 +177,11 @@ impl SingleThreadBase for Emu { regs: &mut custom_arch::ArmCoreRegsCustom, ) -> TargetResult<(), Self> { // if we selected a frame from a tracepoint, return registers from that snapshot - let cpu = self.selected_frame.and_then(|selected| { - self.traceframes.get(selected) - }).map(|frame| { - frame.snapshot - }).unwrap_or_else(|| self.cpu); + let cpu = self + .selected_frame + .and_then(|selected| self.traceframes.get(selected)) + .map(|frame| frame.snapshot) + .unwrap_or_else(|| self.cpu); let mode = cpu.mode(); for i in 0..13 { @@ -230,7 +230,7 @@ impl SingleThreadBase for Emu { // we only support register collection actions for our tracepoint frames. // if we have a selected frame, then we don't have any memory we can // return from the frame snapshot. - return Ok(0) + return Ok(0); } // this is a simple emulator, with RAM covering the entire 32 bit address space for (addr, val) in (start_addr..).zip(data.iter_mut()) { diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 67e8cae..2bb88fe 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -1,8 +1,17 @@ use crate::emu::Emu; use gdbstub::target; -use gdbstub::target::TargetResult; +use gdbstub::target::ext::tracepoints::DefineTracepoint; +use gdbstub::target::ext::tracepoints::ExperimentExplanation; +use gdbstub::target::ext::tracepoints::ExperimentStatus; +use gdbstub::target::ext::tracepoints::FrameDescription; +use gdbstub::target::ext::tracepoints::FrameRequest; +use gdbstub::target::ext::tracepoints::NewTracepoint; +use gdbstub::target::ext::tracepoints::TraceBuffer; +use gdbstub::target::ext::tracepoints::Tracepoint; +use gdbstub::target::ext::tracepoints::TracepointAction; +use gdbstub::target::ext::tracepoints::TracepointItem; use gdbstub::target::TargetError; -use gdbstub::target::ext::tracepoints::{Tracepoint, TracepointItem, NewTracepoint, DefineTracepoint, ExperimentStatus, FrameRequest, FrameDescription, TraceBuffer, ExperimentExplanation, TracepointAction}; +use gdbstub::target::TargetResult; use managed::ManagedSlice; use armv4t_emu::Cpu; @@ -21,7 +30,8 @@ impl target::ext::tracepoints::Tracepoints for Emu { } fn tracepoint_create(&mut self, tp: NewTracepoint) -> TargetResult<(), Self> { - self.tracepoints.insert(tp.number, vec![TracepointItem::New(tp)]); + self.tracepoints + .insert(tp.number, vec![TracepointItem::New(tp)]); Ok(()) } @@ -30,18 +40,22 @@ impl target::ext::tracepoints::Tracepoints for Emu { let mut valid = true; tp.actions(|action| { if let TracepointAction::Registers { mask: _ } = action { - // we only handle register collection actions for the simple case + // we only handle register collection actions for the simple + // case } else { valid = false; } }); if !valid { - return Err(TargetError::NonFatal) + return Err(TargetError::NonFatal); } - self.tracepoints.get_mut(&tp_copy.number).map(move |existing| { - existing.push(TracepointItem::Define(tp_copy)); - () - }).ok_or_else(move || TargetError::Fatal("define on non-existing tracepoint")) + self.tracepoints + .get_mut(&tp_copy.number) + .map(move |existing| { + existing.push(TracepointItem::Define(tp_copy)); + () + }) + .ok_or_else(move || TargetError::Fatal("define on non-existing tracepoint")) } fn tracepoint_status(&self, tp: Tracepoint, _addr: u32) -> TargetResult<(u64, u64), Self> { @@ -49,13 +63,23 @@ impl target::ext::tracepoints::Tracepoints for Emu { // and say the number of bytes is always 0. // Because we don't implement "while-stepping" actions, we don't need to // also check that `addr` matches. - Ok((self.traceframes.iter().filter(|frame| frame.number.0 == tp.0).count() as u64, 0)) + Ok(( + self.traceframes + .iter() + .filter(|frame| frame.number.0 == tp.0) + .count() as u64, + 0, + )) } - fn tracepoint_enumerate_start(&mut self) -> TargetResult>, Self> { - let tracepoints: Vec<_> = self.tracepoints.iter().flat_map(|(_key, value)| { - value.iter().map(|item| item.get_owned()) - }).collect(); + fn tracepoint_enumerate_start( + &mut self, + ) -> TargetResult>, Self> { + let tracepoints: Vec<_> = self + .tracepoints + .iter() + .flat_map(|(_key, value)| value.iter().map(|item| item.get_owned())) + .collect(); self.tracepoint_enumerate_machine = (tracepoints, 0); self.tracepoint_enumerate_step() @@ -74,7 +98,8 @@ impl target::ext::tracepoints::Tracepoints for Emu { } fn trace_buffer_configure(&mut self, _tb: TraceBuffer) -> TargetResult<(), Self> { - // we don't collect a "real" trace buffer, so just ignore configuration attempts. + // we don't collect a "real" trace buffer, so just ignore configuration + // attempts. Ok(()) } @@ -92,7 +117,9 @@ impl target::ext::tracepoints::Tracepoints for Emu { // For a bare-bones example, we don't provide in-depth status explanations. Ok(ExperimentStatus { running: self.tracing, - explanations: ManagedSlice::Owned(vec![ExperimentExplanation::Frames(self.traceframes.len())]), + explanations: ManagedSlice::Owned(vec![ExperimentExplanation::Frames( + self.traceframes.len(), + )]), }) } @@ -101,20 +128,29 @@ impl target::ext::tracepoints::Tracepoints for Emu { frame: FrameRequest, report: &mut dyn FnMut(FrameDescription), ) -> TargetResult<(), Self> { - // For a bare-bones example, we only support `tfind ` and `tfind tracepoint ` - // style frame selection and not the more complicated ones. + // For a bare-bones example, we only support `tfind ` and `tfind + // tracepoint ` style frame selection and not the more + // complicated ones. let found = match frame { - FrameRequest::Select(n) => { - self.traceframes.iter().nth(n as usize).map(|frame| (n, frame)) - }, + FrameRequest::Select(n) => self + .traceframes + .iter() + .nth(n as usize) + .map(|frame| (n, frame)), FrameRequest::Hit(tp) => { - let start = self.selected_frame.map(|selected| selected + 1).unwrap_or(0); + let start = self + .selected_frame + .map(|selected| selected + 1) + .unwrap_or(0); self.traceframes.get(start..).and_then(|frames| { - frames.iter().enumerate().filter(|(_n, frame)| { - frame.number == tp - }).map(|(n, frame)| ((start + n) as u64, frame) ).next() + frames + .iter() + .enumerate() + .filter(|(_n, frame)| frame.number == tp) + .map(|(n, frame)| ((start + n) as u64, frame)) + .next() }) - }, + } _ => return Err(TargetError::NonFatal), }; if let Some((n, frame)) = found { @@ -138,4 +174,3 @@ impl target::ext::tracepoints::Tracepoints for Emu { Ok(()) } } - diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index d342e37..ae42965 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -40,7 +40,9 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { Some(Self { number: dtdp.number, addr: arch_int(dtdp.addr).ok()?, - actions: TracepointActionList::Raw { data: ManagedSlice::Borrowed(dtdp.actions) }, + actions: TracepointActionList::Raw { + data: ManagedSlice::Borrowed(dtdp.actions), + }, }) } @@ -99,7 +101,9 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { unparsed = None; decode_hex_buf(mask).ok()? }; - (f)(&TracepointAction::Registers { mask: ManagedSlice::Borrowed(mask) }); + (f)(&TracepointAction::Registers { + mask: ManagedSlice::Borrowed(mask), + }); } Some([b'M', _mem_args @ ..]) => { // Unimplemented: even simple actions like `collect *(int*)0x0` @@ -112,7 +116,9 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { let len: usize = decode_hex(len_bytes).ok()?; let (expr_bytes, next_bytes) = rem.split_at_mut(len * 2); let expr = decode_hex_buf(expr_bytes).ok()?; - (f)(&TracepointAction::Expression { expr: ManagedSlice::Borrowed(expr) }); + (f)(&TracepointAction::Expression { + expr: ManagedSlice::Borrowed(expr), + }); unparsed = Some(next_bytes); } Some([b'-']) => { diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 2a36a8f..49e7bda 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -43,7 +43,6 @@ impl NewTracepoint { } } - impl NewTracepoint { pub(crate) fn write( &self, @@ -96,9 +95,21 @@ impl<'a, U: Copy> TracepointAction<'a, U> { pub fn get_owned<'b>(&self) -> TracepointAction<'b, U> { use core::ops::Deref; match self { - TracepointAction::Registers { mask } => TracepointAction::Registers { mask: ManagedSlice::Owned(mask.deref().into()) }, - TracepointAction::Memory { basereg, offset, length } => TracepointAction::Memory { basereg: *basereg, offset: *offset, length: *length }, - TracepointAction::Expression { expr } => TracepointAction::Expression { expr: ManagedSlice::Owned(expr.deref().into()) }, + TracepointAction::Registers { mask } => TracepointAction::Registers { + mask: ManagedSlice::Owned(mask.deref().into()), + }, + TracepointAction::Memory { + basereg, + offset, + length, + } => TracepointAction::Memory { + basereg: *basereg, + offset: *offset, + length: *length, + }, + TracepointAction::Expression { expr } => TracepointAction::Expression { + expr: ManagedSlice::Owned(expr.deref().into()), + }, } } } @@ -164,10 +175,14 @@ impl<'a, U: Copy> TracepointActionList<'a, U> { pub fn get_owned<'b>(&self) -> TracepointActionList<'b, U> { use core::ops::Deref; match self { - TracepointActionList::Raw { data } => TracepointActionList::Raw { data: ManagedSlice::Owned(data.deref().into()) }, + TracepointActionList::Raw { data } => TracepointActionList::Raw { + data: ManagedSlice::Owned(data.deref().into()), + }, TracepointActionList::Parsed { actions, more } => TracepointActionList::Parsed { - actions: ManagedSlice::Owned(actions.iter().map(|action| action.get_owned()).collect()), - more: *more + actions: ManagedSlice::Owned( + actions.iter().map(|action| action.get_owned()).collect(), + ), + more: *more, }, } } @@ -193,7 +208,7 @@ impl<'a, U: Copy> DefineTracepoint<'a, U> { DefineTracepoint { number: self.number, addr: self.addr, - actions: self.actions.get_owned() + actions: self.actions.get_owned(), } } } @@ -235,14 +250,14 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepo #[derive(Debug)] pub enum TracepointItem<'a, U> { /// Introduce a new tracepoint and describe its properties. This must be - /// emitted before any [TracepointItem::Define] items that use the same tracepoint - /// number, and must have the `more` flag set if it will be followed by - /// [TracepointItem::Define] items for this tracepoint. + /// emitted before any [TracepointItem::Define] items that use the same + /// tracepoint number, and must have the `more` flag set if it will be + /// followed by [TracepointItem::Define] items for this tracepoint. New(NewTracepoint), /// Define additional data for a tracepoint. This must be emitted after a - /// [TracepointItem::New] item that introduces the tracepoint number, and must have - /// the `more` flag set if it will be followed by more [TracepointItem::Define] items - /// for this tracepoint. + /// [TracepointItem::New] item that introduces the tracepoint number, and + /// must have the `more` flag set if it will be followed by more + /// [TracepointItem::Define] items for this tracepoint. Define(DefineTracepoint<'a, U>), } @@ -257,7 +272,6 @@ impl<'a, U: Copy> TracepointItem<'a, U> { } } - /// Description of the currently running trace experiment. pub struct ExperimentStatus<'a> { /// If a trace is presently running @@ -460,8 +474,8 @@ pub trait Tracepoints: Target { /// Begin enumerating tracepoints. The target implementation should /// initialize a state machine that is stepped by - /// [Tracepoints::tracepoint_enumerate_step], and returns TracepointItems that - /// correspond with the currently configured tracepoints. + /// [Tracepoints::tracepoint_enumerate_step], and returns TracepointItems + /// that correspond with the currently configured tracepoints. fn tracepoint_enumerate_start( &mut self, ) -> TargetResult::Usize>>, Self>; From 882736b5097491d3f238042ed0c582bd20ebc303 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 30 Dec 2024 15:14:05 -0500 Subject: [PATCH 05/29] remove incorrect spaces in packets --- src/stub/core_impl/tracepoints.rs | 6 +++--- src/target/ext/tracepoints.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index ae42965..7357bc9 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -239,7 +239,7 @@ impl GdbStubImpl { let e = (|| -> Result<_, _> { match desc { FrameDescription::FrameNumber(n) => { - res.write_str("F ")?; + res.write_str("F")?; if let Some(n) = n { res.write_num(n)?; } else { @@ -247,7 +247,7 @@ impl GdbStubImpl { } } FrameDescription::Hit(tdp) => { - res.write_str("T ")?; + res.write_str("T")?; res.write_num(tdp.0)?; } } @@ -260,7 +260,7 @@ impl GdbStubImpl { }) .handle_error()?; if !any_results { - res.write_str("F -1")?; + res.write_str("F-1")?; } } // The GDB protocol for this is very weird: it sends this first packet diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 49e7bda..aad792d 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -121,7 +121,7 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct ) -> Result<(), ResponseWriterError> { match self { TracepointAction::Registers { mask } => { - res.write_str("R ")?; + res.write_str("R")?; res.write_hex_buf(mask)?; } TracepointAction::Memory { @@ -129,7 +129,7 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct offset, length, } => { - res.write_str("M ")?; + res.write_str("M")?; match basereg { Some(r) => res.write_num(*r), None => res.write_str("-1"), @@ -142,7 +142,7 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct res.write_num(*length)?; } TracepointAction::Expression { expr } => { - res.write_str("X ")?; + res.write_str("X")?; res.write_num(expr.len())?; res.write_str(",")?; res.write_hex_buf(expr)?; From ad92d991c3dc4efc719dec2a23274597e87ebf60 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 30 Dec 2024 15:46:06 -0500 Subject: [PATCH 06/29] better type safety for experiment status --- examples/armv4t/gdb/tracepoints.rs | 7 +++- src/stub/core_impl/tracepoints.rs | 5 ++- src/target/ext/tracepoints.rs | 63 +++++++++++++++++++++++------- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 2bb88fe..df6483f 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -2,6 +2,7 @@ use crate::emu::Emu; use gdbstub::target; use gdbstub::target::ext::tracepoints::DefineTracepoint; use gdbstub::target::ext::tracepoints::ExperimentExplanation; +use gdbstub::target::ext::tracepoints::ExperimentState; use gdbstub::target::ext::tracepoints::ExperimentStatus; use gdbstub::target::ext::tracepoints::FrameDescription; use gdbstub::target::ext::tracepoints::FrameRequest; @@ -116,7 +117,11 @@ impl target::ext::tracepoints::Tracepoints for Emu { fn trace_experiment_status(&self) -> TargetResult, Self> { // For a bare-bones example, we don't provide in-depth status explanations. Ok(ExperimentStatus { - running: self.tracing, + state: if self.tracing { + ExperimentState::Running + } else { + ExperimentState::NotRunning + }, explanations: ManagedSlice::Owned(vec![ExperimentExplanation::Frames( self.traceframes.len(), )]), diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 7357bc9..2bfc2db 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -159,8 +159,9 @@ impl GdbStubImpl { } Tracepoints::qTStatus(_) => { let status = ops.trace_experiment_status().handle_error()?; - res.write_str("T")?; - res.write_dec(if status.running { 1 } else { 0 })?; + // Write stop status + status.state.write(res)?; + // And then all the optional statistical information for explanation in status.explanations.iter() { res.write_str(";")?; explanation.write(res)?; diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index aad792d..06c7f11 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -274,15 +274,20 @@ impl<'a, U: Copy> TracepointItem<'a, U> { /// Description of the currently running trace experiment. pub struct ExperimentStatus<'a> { - /// If a trace is presently running - pub running: bool, + /// The running state of the experiment. + pub state: ExperimentState<'a>, /// A list of optional explanations for the trace status. pub explanations: ManagedSlice<'a, ExperimentExplanation<'a>>, } -/// An explanation of some detail of the currently running trace experiment. +/// The state of the trace experiment. #[derive(Debug)] -pub enum ExperimentExplanation<'a> { +pub enum ExperimentState<'a> { + /// The experiment is currently running + Running, + /// The experiment is not currently running, with no more information given + /// as to why. + NotRunning, /// No trace has been ran yet. NotRun, /// The trace was stopped by the user. May contain an optional user-supplied @@ -299,8 +304,11 @@ pub enum ExperimentExplanation<'a> { Error(&'a [u8], Tracepoint), /// The trace stopped for some other reason. Unknown, +} - // Statistical information +/// An explanation of some detail of the currently running trace experiment. +#[derive(Debug)] +pub enum ExperimentExplanation<'a> { /// The number of trace frames in the buffer. Frames(usize), /// The total number of trace frames created during the run. This may be @@ -324,36 +332,61 @@ pub enum ExperimentExplanation<'a> { Other(managed::Managed<'a, str>), } -impl<'a> ExperimentExplanation<'a> { +impl<'a> ExperimentState<'a> { pub(crate) fn write( &self, res: &mut ResponseWriter<'_, C>, ) -> Result<(), ResponseWriterError> { - use ExperimentExplanation::*; + use ExperimentState::*; + if let Running = self { + return res.write_str("T1"); + } + // We're stopped for some reason, and may have an explanation for why + res.write_str("T0")?; match self { - NotRun => res.write_str("tnotrun:0")?, + Running => { + /* unreachable */ + () + } + NotRunning => { + /* no information */ + () + } + NotRun => res.write_str(";tnotrun:0")?, Stop(ref t) => match t { Some(text) => { - res.write_str("tstop:")?; + res.write_str(";tstop:")?; res.write_hex_buf(text)?; res.write_str(":0")?; } - None => res.write_str("tstop:0")?, + None => res.write_str(";tstop:0")?, }, - Full => res.write_str("tfull:0")?, - Disconnected => res.write_str("tdisconnected:0")?, + Full => res.write_str(";tfull:0")?, + Disconnected => res.write_str(";tdisconnected:0")?, PassCount(tpnum) => { - res.write_str("tpasscount:")?; + res.write_str(";tpasscount:")?; res.write_num(tpnum.0)?; } Error(text, tpnum) => { - res.write_str("terror:")?; + res.write_str(";terror:")?; res.write_hex_buf(text)?; res.write_str(":")?; res.write_num(tpnum.0)?; } - Unknown => res.write_str("tunknown:0")?, + Unknown => res.write_str(";tunknown:0")?, + } + Ok(()) + } +} + +impl<'a> ExperimentExplanation<'a> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + use ExperimentExplanation::*; + match self { Frames(u) => { res.write_str("tframes:")?; res.write_num(*u)?; From 4cccd75feec9ef24d5a44db920188d80173e731a Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 30 Dec 2024 15:56:20 -0500 Subject: [PATCH 07/29] use write_num helper instead of manual byte conversion --- src/target/ext/tracepoints.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 06c7f11..d9d5423 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -51,11 +51,7 @@ impl NewTracepoint res.write_str("QTDP:")?; res.write_num(self.number.0)?; res.write_str(":")?; - let mut buf = [0; 8]; - self.addr - .to_be_bytes(&mut buf) - .ok_or_else(|| unreachable!())?; - res.write_hex_buf(&buf)?; + res.write_num(self.addr)?; res.write_str(":")?; res.write_str(if self.enabled { "E" } else { "D" })?; res.write_str(":")?; @@ -135,9 +131,7 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAct None => res.write_str("-1"), }?; res.write_str(",")?; - let mut buf = [0; 8]; - offset.to_be_bytes(&mut buf).ok_or_else(|| unreachable!())?; - res.write_hex_buf(&buf)?; + res.write_num(*offset)?; res.write_str(",")?; res.write_num(*length)?; } @@ -221,11 +215,7 @@ impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepo res.write_str("QTDP:-")?; res.write_num(self.number.0)?; res.write_str(":")?; - let mut buf = [0; 8]; - self.addr - .to_be_bytes(&mut buf) - .ok_or_else(|| unreachable!())?; - res.write_hex_buf(&buf)?; + res.write_num(self.addr)?; res.write_str(":")?; let mut err = None; let more = self.actions(|action| { From 0e6943e9794234ae18dae8e6078f70c272987195 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 30 Dec 2024 19:43:55 -0500 Subject: [PATCH 08/29] remove extra cloned --- src/stub/core_impl/tracepoints.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 2bfc2db..6d1748a 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -89,7 +89,6 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { Some([b'R', mask @ ..]) => { let mask_end = mask .iter() - .cloned() .enumerate() .find(|(_i, b)| matches!(b, b'S' | b'R' | b'M' | b'X' | b'-')); // We may or may not have another action after our mask From dbeb9cc3272f0325b2371910699fc0a66b8b61af Mon Sep 17 00:00:00 2001 From: cczetier Date: Wed, 1 Jan 2025 10:42:25 -0500 Subject: [PATCH 09/29] split experiment status in two --- examples/armv4t/gdb/tracepoints.rs | 24 +++++++++++++----------- src/stub/core_impl/tracepoints.rs | 24 ++++++++++++++++++------ src/target/ext/tracepoints.rs | 22 ++++++++++------------ 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index df6483f..84a5408 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -2,7 +2,6 @@ use crate::emu::Emu; use gdbstub::target; use gdbstub::target::ext::tracepoints::DefineTracepoint; use gdbstub::target::ext::tracepoints::ExperimentExplanation; -use gdbstub::target::ext::tracepoints::ExperimentState; use gdbstub::target::ext::tracepoints::ExperimentStatus; use gdbstub::target::ext::tracepoints::FrameDescription; use gdbstub::target::ext::tracepoints::FrameRequest; @@ -13,7 +12,6 @@ use gdbstub::target::ext::tracepoints::TracepointAction; use gdbstub::target::ext::tracepoints::TracepointItem; use gdbstub::target::TargetError; use gdbstub::target::TargetResult; -use managed::ManagedSlice; use armv4t_emu::Cpu; #[derive(Debug)] @@ -116,18 +114,22 @@ impl target::ext::tracepoints::Tracepoints for Emu { fn trace_experiment_status(&self) -> TargetResult, Self> { // For a bare-bones example, we don't provide in-depth status explanations. - Ok(ExperimentStatus { - state: if self.tracing { - ExperimentState::Running - } else { - ExperimentState::NotRunning - }, - explanations: ManagedSlice::Owned(vec![ExperimentExplanation::Frames( - self.traceframes.len(), - )]), + Ok(if self.tracing { + ExperimentStatus::Running + } else { + ExperimentStatus::NotRunning }) } + fn trace_experiment_info( + &self, + report: &mut dyn FnMut(ExperimentExplanation<'_>), + ) -> TargetResult<(), Self> { + (report)(ExperimentExplanation::Frames(self.traceframes.len())); + + Ok(()) + } + fn select_frame( &mut self, frame: FrameRequest, diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 6d1748a..4c668a4 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -9,6 +9,7 @@ use crate::protocol::commands::_QTDP::CreateTDP; use crate::protocol::commands::_QTDP::DefineTDP; use crate::protocol::commands::_QTDP::QTDP; use crate::target::ext::tracepoints::DefineTracepoint; +use crate::target::ext::tracepoints::ExperimentExplanation; use crate::target::ext::tracepoints::FrameDescription; use crate::target::ext::tracepoints::FrameRequest; use crate::target::ext::tracepoints::NewTracepoint; @@ -158,13 +159,24 @@ impl GdbStubImpl { } Tracepoints::qTStatus(_) => { let status = ops.trace_experiment_status().handle_error()?; - // Write stop status - status.state.write(res)?; - // And then all the optional statistical information - for explanation in status.explanations.iter() { - res.write_str(";")?; - explanation.write(res)?; + status.write(res)?; + let mut err = None; + ops.trace_experiment_info(&mut |explanation: ExperimentExplanation<'_>| { + if let Err(e) = explanation.write(res) { + err = Some(e) + } + }) + .handle_error()?; + if let Some(e) = err { + return Err(e.into()); } + //// Write stop status + //status.state.write(res)?; + //// And then all the optional statistical information + //status.explanations(|explanation| { + // res.write_str(";")?; + // explanation.write(res)?; + //})?; } Tracepoints::qTP(qtp) => { let addr = ::Usize::from_be_bytes(qtp.addr) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index d9d5423..272cc96 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -262,17 +262,9 @@ impl<'a, U: Copy> TracepointItem<'a, U> { } } -/// Description of the currently running trace experiment. -pub struct ExperimentStatus<'a> { - /// The running state of the experiment. - pub state: ExperimentState<'a>, - /// A list of optional explanations for the trace status. - pub explanations: ManagedSlice<'a, ExperimentExplanation<'a>>, -} - -/// The state of the trace experiment. +/// The running state of a trace experiment. #[derive(Debug)] -pub enum ExperimentState<'a> { +pub enum ExperimentStatus<'a> { /// The experiment is currently running Running, /// The experiment is not currently running, with no more information given @@ -322,12 +314,12 @@ pub enum ExperimentExplanation<'a> { Other(managed::Managed<'a, str>), } -impl<'a> ExperimentState<'a> { +impl<'a> ExperimentStatus<'a> { pub(crate) fn write( &self, res: &mut ResponseWriter<'_, C>, ) -> Result<(), ResponseWriterError> { - use ExperimentState::*; + use ExperimentStatus::*; if let Running = self { return res.write_str("T1"); } @@ -525,6 +517,12 @@ pub trait Tracepoints: Target { /// Return the status of the current trace experiment. fn trace_experiment_status(&self) -> TargetResult, Self>; + /// List any statistical information for the current trace experiment, by + /// calling `report` with each [ExperimentExplanation] item. + fn trace_experiment_info( + &self, + report: &mut dyn FnMut(ExperimentExplanation<'_>), + ) -> TargetResult<(), Self>; /// Start a new trace experiment. fn trace_experiment_start(&mut self) -> TargetResult<(), Self>; /// Stop the currently running trace experiment. From 943b7f4fec318e29deb6f7519323148df7adc836 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 13:16:45 -0500 Subject: [PATCH 10/29] remove qtnotes --- src/protocol/commands.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/protocol/commands.rs b/src/protocol/commands.rs index ea3aeec..4a0f7ac 100644 --- a/src/protocol/commands.rs +++ b/src/protocol/commands.rs @@ -344,7 +344,6 @@ commands! { "QTDP" => _QTDP::QTDP<'a>, "QTinit" => _QTinit::QTinit, "QTBuffer" => _QTBuffer::QTBuffer<'a>, - // TODO: QTNotes? "QTStart" => _QTStart::QTStart, "QTStop" => _QTStop::QTStop, "QTFrame" => _QTFrame::QTFrame<'a>, @@ -353,6 +352,10 @@ commands! { "qTP" => _qTP::qTP<'a>, "qTfP" => _qTfP::qTfP, "qTsP" => _qTsP::qTsP, + + // These are currently stubbed out to no-ops for tracepoints v1: they're + // needed to suppress "not implemented" errors. + // QTDV is unimplemented. "qTfV" => _qTfV::qTfV, "qTsV" => _qTsV::qTsV, } From 4e7dcca5c258b64c37f49027c7161a0e7cf6bb31 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 13:59:46 -0500 Subject: [PATCH 11/29] split qTBuffer/QTBuffer, fix trace explanations --- src/protocol/commands.rs | 3 ++- src/protocol/commands/_QTBuffer.rs | 40 +++++++++-------------------- src/protocol/commands/_qTBuffer.rs | 30 ++++++++++++++++++++++ src/stub/core_impl/tracepoints.rs | 41 ++++++++++++++---------------- 4 files changed, 63 insertions(+), 51 deletions(-) create mode 100644 src/protocol/commands/_qTBuffer.rs diff --git a/src/protocol/commands.rs b/src/protocol/commands.rs index 4a0f7ac..4431f84 100644 --- a/src/protocol/commands.rs +++ b/src/protocol/commands.rs @@ -343,11 +343,12 @@ commands! { tracepoints use 'a { "QTDP" => _QTDP::QTDP<'a>, "QTinit" => _QTinit::QTinit, - "QTBuffer" => _QTBuffer::QTBuffer<'a>, + "QTBuffer" => _QTBuffer::QTBuffer, "QTStart" => _QTStart::QTStart, "QTStop" => _QTStop::QTStop, "QTFrame" => _QTFrame::QTFrame<'a>, + "qTBuffer" => _qTBuffer::qTBuffer<'a>, "qTStatus" => _qTStatus::qTStatus, "qTP" => _qTP::qTP<'a>, "qTfP" => _qTfP::qTfP, diff --git a/src/protocol/commands/_QTBuffer.rs b/src/protocol/commands/_QTBuffer.rs index ee8f76f..50d0226 100644 --- a/src/protocol/commands/_QTBuffer.rs +++ b/src/protocol/commands/_QTBuffer.rs @@ -2,49 +2,33 @@ use super::prelude::*; use crate::target::ext::tracepoints::{BufferShape, TraceBuffer}; #[derive(Debug)] -pub enum QTBuffer<'a> -{ - Request { offset: u64, length: usize, data: &'a mut [u8] }, - Configure { buffer: TraceBuffer }, -} +pub struct QTBuffer(pub TraceBuffer); -impl<'a> ParseCommand<'a> for QTBuffer<'a> { +impl ParseCommand<'_> for QTBuffer { #[inline(always)] - fn from_packet(buf: PacketBuf<'a>) -> Option { - let (buf, body_range) = buf.into_raw_buf(); - let body = &buf[body_range]; - match body { + fn from_packet(buf: PacketBuf<'_>) -> Option { + match buf.into_body() { [b':', body @ ..] => { - let mut s = body.split(|b| *b == b':'); + let mut s = body.splitn_mut(2, |b| *b == b':'); let opt = s.next()?; - Some(match opt.as_ref() { + match opt.as_ref() { b"circular" => { let shape = s.next()?; - QTBuffer::Configure { buffer: TraceBuffer::Shape(match shape { + Some(QTBuffer(TraceBuffer::Shape(match shape { [b'1'] => Some(BufferShape::Circular), [b'0'] => Some(BufferShape::Linear), _ => None, - }?)} + }?))) }, b"size" => { let size = s.next()?; - QTBuffer::Configure { buffer: TraceBuffer::Size(match size { + Some(QTBuffer(TraceBuffer::Size(match size { [b'-', b'1'] => None, i => Some(decode_hex(i).ok()?) - })} - }, - req => { - let mut req_opts = req.split(|b| *b == b','); - let (offset, length) = (req_opts.next()?, req_opts.next()?); - let offset = decode_hex(offset).ok()?; - let length = decode_hex(length).ok()?; - // Our response has to be a hex encoded buffer that fits within - // our packet size, which means we actually have half as much space - // as our slice would indicate. - let (front, _back) = buf.split_at_mut(buf.len() / 2); - QTBuffer::Request { offset, length, data: front } + }))) }, - }) + _ => None, + } }, _ => None, diff --git a/src/protocol/commands/_qTBuffer.rs b/src/protocol/commands/_qTBuffer.rs new file mode 100644 index 0000000..531a4cb --- /dev/null +++ b/src/protocol/commands/_qTBuffer.rs @@ -0,0 +1,30 @@ +use super::prelude::*; + +#[derive(Debug)] +pub struct qTBuffer<'a> { + pub offset: u64, + pub length: usize, + pub data: &'a mut [u8] +} + +impl<'a> ParseCommand<'a> for qTBuffer<'a> { + #[inline(always)] + fn from_packet(buf: PacketBuf<'a>) -> Option { + let (buf, body_range) = buf.into_raw_buf(); + let body = &buf[body_range]; + match body { + [b':', body @ ..] => { + let mut req_opts = body.split(|b| *b == b','); + let (offset, length) = (req_opts.next()?, req_opts.next()?); + let offset = decode_hex(offset).ok()?; + let length = decode_hex(length).ok()?; + // Our response has to be a hex encoded buffer that fits within + // our packet size, which means we actually have half as much space + // as our slice would indicate. + let (front, _back) = buf.split_at_mut(buf.len() / 2); + Some(qTBuffer { offset, length, data: front }) + }, + _ => None + } + } +} diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 4c668a4..3ee8e01 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -1,7 +1,7 @@ use super::prelude::*; use crate::arch::Arch; use crate::internal::BeBytes; -use crate::protocol::commands::_QTBuffer::QTBuffer; +use crate::protocol::commands::_qTBuffer::qTBuffer; use crate::protocol::commands::ext::Tracepoints; use crate::protocol::commands::prelude::decode_hex; use crate::protocol::commands::prelude::decode_hex_buf; @@ -162,7 +162,7 @@ impl GdbStubImpl { status.write(res)?; let mut err = None; ops.trace_experiment_info(&mut |explanation: ExperimentExplanation<'_>| { - if let Err(e) = explanation.write(res) { + if let Err(e) = res.write_str(";").and_then(|()| explanation.write(res)) { err = Some(e) } }) @@ -205,27 +205,24 @@ impl GdbStubImpl { // TODO: support qRelocInsn? return Ok(HandlerStatus::NeedsOk); } - Tracepoints::QTBuffer(buf) => { - match buf { - QTBuffer::Request { - offset, - length, - data, - } => { - let read = ops - .trace_buffer_request(offset, length, data) - .handle_error()?; - if let Some(read) = read { - let read = read.min(data.len()); - res.write_hex_buf(&data[..read])?; - } else { - res.write_str("l")?; - } - } - QTBuffer::Configure { buffer } => { - ops.trace_buffer_configure(buffer).handle_error()?; - } + Tracepoints::qTBuffer(buf) => { + let qTBuffer { + offset, + length, + data, + } = buf; + let read = ops + .trace_buffer_request(offset, length, data) + .handle_error()?; + if let Some(read) = read { + let read = read.min(data.len()); + res.write_hex_buf(&data[..read])?; + } else { + res.write_str("l")?; } + } + Tracepoints::QTBuffer(conf) => { + ops.trace_buffer_configure(conf.0).handle_error()?; // Documentation doesn't mention this, but it needs OK return Ok(HandlerStatus::NeedsOk); } From c5e2ed619e81da7fc6fe6793adf554753a6ae7e2 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 14:21:22 -0500 Subject: [PATCH 12/29] address nits --- examples/armv4t/gdb/tracepoints.rs | 4 ++-- src/protocol/commands/_QTBuffer.rs | 8 ++++---- src/protocol/commands/_QTDP.rs | 2 +- src/target/ext/tracepoints.rs | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 84a5408..7133f0a 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -6,7 +6,7 @@ use gdbstub::target::ext::tracepoints::ExperimentStatus; use gdbstub::target::ext::tracepoints::FrameDescription; use gdbstub::target::ext::tracepoints::FrameRequest; use gdbstub::target::ext::tracepoints::NewTracepoint; -use gdbstub::target::ext::tracepoints::TraceBuffer; +use gdbstub::target::ext::tracepoints::TraceBufferConfig; use gdbstub::target::ext::tracepoints::Tracepoint; use gdbstub::target::ext::tracepoints::TracepointAction; use gdbstub::target::ext::tracepoints::TracepointItem; @@ -96,7 +96,7 @@ impl target::ext::tracepoints::Tracepoints for Emu { } } - fn trace_buffer_configure(&mut self, _tb: TraceBuffer) -> TargetResult<(), Self> { + fn trace_buffer_configure(&mut self, _config: TraceBufferConfig) -> TargetResult<(), Self> { // we don't collect a "real" trace buffer, so just ignore configuration // attempts. Ok(()) diff --git a/src/protocol/commands/_QTBuffer.rs b/src/protocol/commands/_QTBuffer.rs index 50d0226..f190e38 100644 --- a/src/protocol/commands/_QTBuffer.rs +++ b/src/protocol/commands/_QTBuffer.rs @@ -1,8 +1,8 @@ use super::prelude::*; -use crate::target::ext::tracepoints::{BufferShape, TraceBuffer}; +use crate::target::ext::tracepoints::{BufferShape, TraceBufferConfig}; #[derive(Debug)] -pub struct QTBuffer(pub TraceBuffer); +pub struct QTBuffer(pub TraceBufferConfig); impl ParseCommand<'_> for QTBuffer { #[inline(always)] @@ -14,7 +14,7 @@ impl ParseCommand<'_> for QTBuffer { match opt.as_ref() { b"circular" => { let shape = s.next()?; - Some(QTBuffer(TraceBuffer::Shape(match shape { + Some(QTBuffer(TraceBufferConfig::Shape(match shape { [b'1'] => Some(BufferShape::Circular), [b'0'] => Some(BufferShape::Linear), _ => None, @@ -22,7 +22,7 @@ impl ParseCommand<'_> for QTBuffer { }, b"size" => { let size = s.next()?; - Some(QTBuffer(TraceBuffer::Size(match size { + Some(QTBuffer(TraceBufferConfig::Size(match size { [b'-', b'1'] => None, i => Some(decode_hex(i).ok()?) }))) diff --git a/src/protocol/commands/_QTDP.rs b/src/protocol/commands/_QTDP.rs index f681151..12433b9 100644 --- a/src/protocol/commands/_QTDP.rs +++ b/src/protocol/commands/_QTDP.rs @@ -33,7 +33,7 @@ impl<'a> ParseCommand<'a> for QTDP<'a> { let body = buf.into_body(); match body { [b':', b'-', actions @ ..] => { - let mut params = actions.splitn_mut(4, |b| matches!(*b, b':')); + let mut params = actions.splitn_mut(4, |b| *b == b':'); let number = Tracepoint(decode_hex(params.next()?).ok()?); let addr = decode_hex_buf(params.next()?).ok()?; let actions = params.next()?; diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 272cc96..e59b4d2 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -411,7 +411,7 @@ pub enum BufferShape { /// Configuration for the trace buffer. #[derive(Debug)] -pub enum TraceBuffer { +pub enum TraceBufferConfig { /// Set the buffer's shape. Shape(BufferShape), /// Set the buffer's size in bytes. If None, the target should use whatever @@ -502,7 +502,7 @@ pub trait Tracepoints: Target { ) -> TargetResult::Usize>>, Self>; /// Reconfigure the trace buffer to include or modify an attribute. - fn trace_buffer_configure(&mut self, tb: TraceBuffer) -> TargetResult<(), Self>; + fn trace_buffer_configure(&mut self, config: TraceBufferConfig) -> TargetResult<(), Self>; /// Return up to `len` bytes from the trace buffer, starting at `offset`. /// The trace buffer is treated as a contiguous collection of traceframes, From 8b358de8a51daee583e73143dfdd2d85d570f350 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 15:16:51 -0500 Subject: [PATCH 13/29] strip suffix for QTDP --- src/protocol/commands/_QTDP.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/protocol/commands/_QTDP.rs b/src/protocol/commands/_QTDP.rs index 12433b9..751f70e 100644 --- a/src/protocol/commands/_QTDP.rs +++ b/src/protocol/commands/_QTDP.rs @@ -45,8 +45,13 @@ impl<'a> ParseCommand<'a> for QTDP<'a> { })) }, [b':', tracepoint @ ..] => { - let more = tracepoint.ends_with(&[b'-']); - let mut params = tracepoint.splitn_mut(6, |b| matches!(*b, b':' | b'-')); + // Strip off the trailing '-' that indicates if there will be + // more packets after this + let (tracepoint, more) = match tracepoint { + [rest @ .., b'-'] => (rest, true), + x => (x, false) + }; + let mut params = tracepoint.splitn_mut(6, |b| *b == b':'); let n = Tracepoint(decode_hex(params.next()?).ok()?); let addr = decode_hex_buf(params.next()?).ok()?; let ena = params.next()?; From 804487ee16b21cae4fe9f9bd6b36c8a6279acfa2 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 15:20:36 -0500 Subject: [PATCH 14/29] strip suffix on QTDP parse raw actions --- src/stub/core_impl/tracepoints.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 3ee8e01..edd642a 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -70,7 +70,10 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { actions: &mut [u8], mut f: impl FnMut(&TracepointAction<'_, U>), ) -> Option { - let mut more = false; + let (actions, more) = match actions { + [rest @ .., b'-'] => (rest, true), + x => (x, false), + }; let mut unparsed: Option<&mut [u8]> = Some(actions); loop { match unparsed { @@ -91,7 +94,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { let mask_end = mask .iter() .enumerate() - .find(|(_i, b)| matches!(b, b'S' | b'R' | b'M' | b'X' | b'-')); + .find(|(_i, b)| matches!(b, b'S' | b'R' | b'M' | b'X')); // We may or may not have another action after our mask let mask = if let Some(mask_end) = mask_end { let (mask_bytes, next) = mask.split_at_mut(mask_end.0); @@ -121,10 +124,6 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { }); unparsed = Some(next_bytes); } - Some([b'-']) => { - more = true; - break; - } Some([]) | None => { break; } From 3871bc5a16859b58d392487a8096c329ac2f3fd4 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 15:52:32 -0500 Subject: [PATCH 15/29] refactor tracepoint enumeration to use a callback --- examples/armv4t/gdb/tracepoints.rs | 14 +++++----- src/stub/core_impl/tracepoints.rs | 41 +++++++++++++++++------------- src/target/ext/tracepoints.rs | 15 ++++++----- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 7133f0a..2a1624b 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -73,7 +73,8 @@ impl target::ext::tracepoints::Tracepoints for Emu { fn tracepoint_enumerate_start( &mut self, - ) -> TargetResult>, Self> { + f: &mut dyn FnMut(TracepointItem<'_, u32>), + ) -> TargetResult<(), Self> { let tracepoints: Vec<_> = self .tracepoints .iter() @@ -81,19 +82,20 @@ impl target::ext::tracepoints::Tracepoints for Emu { .collect(); self.tracepoint_enumerate_machine = (tracepoints, 0); - self.tracepoint_enumerate_step() + self.tracepoint_enumerate_step(f) } fn tracepoint_enumerate_step<'a>( &'a mut self, - ) -> TargetResult>, Self> { + f: &mut dyn FnMut(TracepointItem<'_, u32>), + ) -> TargetResult<(), Self> { let (tracepoints, index) = &mut self.tracepoint_enumerate_machine; if let Some(item) = tracepoints.iter().nth(*index) { *index += 1; - Ok(Some(item.get_owned())) - } else { - Ok(None) + f(item.get_owned()) } + + Ok(()) } fn trace_buffer_configure(&mut self, _config: TraceBufferConfig) -> TargetResult<(), Self> { diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index edd642a..4ae887e 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -169,13 +169,6 @@ impl GdbStubImpl { if let Some(e) = err { return Err(e.into()); } - //// Write stop status - //status.state.write(res)?; - //// And then all the optional statistical information - //status.explanations(|explanation| { - // res.write_str(";")?; - // explanation.write(res)?; - //})?; } Tracepoints::qTP(qtp) => { let addr = ::Usize::from_be_bytes(qtp.addr) @@ -279,21 +272,35 @@ impl GdbStubImpl { // which means we can't really setup the state machine ourselves or // turn it into a nicer API. Tracepoints::qTfP(_) => { - let tp = ops.tracepoint_enumerate_start().handle_error()?; - if let Some(tp) = tp { - match tp { - TracepointItem::New(ctp) => ctp.write(res)?, - TracepointItem::Define(dtp) => dtp.write(res)?, + let mut err = None; + ops.tracepoint_enumerate_start(&mut |tp| { + let e = match tp { + TracepointItem::New(ctp) => ctp.write(res), + TracepointItem::Define(dtp) => dtp.write(res), + }; + if let Err(e) = e { + err = Some(e) } + }) + .handle_error()?; + if let Some(e) = err { + return Err(e.into()); } } Tracepoints::qTsP(_) => { - let tp = ops.tracepoint_enumerate_step().handle_error()?; - if let Some(tp) = tp { - match tp { - TracepointItem::New(ctp) => ctp.write(res)?, - TracepointItem::Define(dtp) => dtp.write(res)?, + let mut err = None; + ops.tracepoint_enumerate_step(&mut |tp| { + let e = match tp { + TracepointItem::New(ctp) => ctp.write(res), + TracepointItem::Define(dtp) => dtp.write(res), + }; + if let Err(e) = e { + err = Some(e) } + }) + .handle_error()?; + if let Some(e) = err { + return Err(e.into()); } } diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index e59b4d2..5c9393e 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -489,17 +489,20 @@ pub trait Tracepoints: Target { /// Begin enumerating tracepoints. The target implementation should /// initialize a state machine that is stepped by - /// [Tracepoints::tracepoint_enumerate_step], and returns TracepointItems - /// that correspond with the currently configured tracepoints. + /// [Tracepoints::tracepoint_enumerate_step], and call `f` with the first + /// [TracepointItem] that corresponds with the currently configured + /// tracepoints. fn tracepoint_enumerate_start( &mut self, - ) -> TargetResult::Usize>>, Self>; + f: &mut dyn FnMut(TracepointItem<'_, ::Usize>), + ) -> TargetResult<(), Self>; /// Step the tracepoint enumeration state machine. The target implementation - /// should return TracepointItems that correspond with the currently - /// configured tracepoints. + /// should call `f` with the next TracepointItem that correspond with the + /// currently configured tracepoints. fn tracepoint_enumerate_step( &mut self, - ) -> TargetResult::Usize>>, Self>; + f: &mut dyn FnMut(TracepointItem<'_, ::Usize>), + ) -> TargetResult<(), Self>; /// Reconfigure the trace buffer to include or modify an attribute. fn trace_buffer_configure(&mut self, config: TraceBufferConfig) -> TargetResult<(), Self>; From 7e367ed58f12670429ec95ff4563c9ddbb82c74b Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 16:09:59 -0500 Subject: [PATCH 16/29] remove arch_int helper --- src/stub/core_impl/tracepoints.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 4ae887e..58ef9ad 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -21,10 +21,9 @@ use managed::ManagedSlice; impl NewTracepoint { /// Parse from a raw CreateTDP packet. pub fn from_tdp(ctdp: CreateTDP<'_>) -> Option { - let arch_int = |b: &[u8]| -> Result { U::from_be_bytes(b).ok_or(()) }; Some(Self { number: ctdp.number, - addr: arch_int(ctdp.addr).ok()?, + addr: U::from_be_bytes(ctdp.addr)?, enabled: ctdp.enable, pass_count: ctdp.pass, step_count: ctdp.step, @@ -36,11 +35,9 @@ impl NewTracepoint { impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// Parse from a raw DefineTDP packet. pub fn from_tdp(dtdp: DefineTDP<'a>) -> Option { - let arch_int = |b: &[u8]| -> Result { U::from_be_bytes(b).ok_or(()) }; - Some(Self { number: dtdp.number, - addr: arch_int(dtdp.addr).ok()?, + addr: U::from_be_bytes(dtdp.addr)?, actions: TracepointActionList::Raw { data: ManagedSlice::Borrowed(dtdp.actions), }, From a65be610a811ef1eb52db434b4c2f6e3d906e775 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 17:55:50 -0500 Subject: [PATCH 17/29] move implementations, have actions method return an error for parsing failures --- examples/armv4t/gdb/tracepoints.rs | 18 +- src/stub/core_impl/tracepoints.rs | 319 +++++++++++++++++++++++++++-- src/target/ext/tracepoints.rs | 264 ------------------------ 3 files changed, 309 insertions(+), 292 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 2a1624b..0f45b60 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -37,14 +37,16 @@ impl target::ext::tracepoints::Tracepoints for Emu { fn tracepoint_define(&mut self, tp: DefineTracepoint<'_, u32>) -> TargetResult<(), Self> { let tp_copy = tp.get_owned(); let mut valid = true; - tp.actions(|action| { - if let TracepointAction::Registers { mask: _ } = action { - // we only handle register collection actions for the simple - // case - } else { - valid = false; - } - }); + let _more = tp + .actions(|action| { + if let TracepointAction::Registers { mask: _ } = action { + // we only handle register collection actions for the simple + // case + } else { + valid = false; + } + }) + .map_err(|_e| TargetError::Fatal("unable to parse actions"))?; if !valid { return Err(TargetError::NonFatal); } diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 58ef9ad..0ca4aab 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -8,8 +8,11 @@ use crate::protocol::commands::prelude::decode_hex_buf; use crate::protocol::commands::_QTDP::CreateTDP; use crate::protocol::commands::_QTDP::DefineTDP; use crate::protocol::commands::_QTDP::QTDP; +use crate::protocol::PacketParseError; +use crate::protocol::ResponseWriterError; use crate::target::ext::tracepoints::DefineTracepoint; use crate::target::ext::tracepoints::ExperimentExplanation; +use crate::target::ext::tracepoints::ExperimentStatus; use crate::target::ext::tracepoints::FrameDescription; use crate::target::ext::tracepoints::FrameRequest; use crate::target::ext::tracepoints::NewTracepoint; @@ -17,6 +20,7 @@ use crate::target::ext::tracepoints::TracepointAction; use crate::target::ext::tracepoints::TracepointActionList; use crate::target::ext::tracepoints::TracepointItem; use managed::ManagedSlice; +use num_traits::PrimInt; impl NewTracepoint { /// Parse from a raw CreateTDP packet. @@ -32,6 +36,34 @@ impl NewTracepoint { } } +#[cfg(feature = "alloc")] +impl NewTracepoint { + /// Allocate an owned copy of this structure. + pub fn get_owned(&self) -> NewTracepoint { + self.clone() + } +} + +impl NewTracepoint { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), Error> { + res.write_str("QTDP:")?; + res.write_num(self.number.0)?; + res.write_str(":")?; + res.write_num(self.addr)?; + res.write_str(":")?; + res.write_str(if self.enabled { "E" } else { "D" })?; + res.write_str(":")?; + res.write_num(self.step_count)?; + res.write_str(":")?; + res.write_num(self.pass_count)?; + + Ok(()) + } +} + impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// Parse from a raw DefineTDP packet. pub fn from_tdp(dtdp: DefineTDP<'a>) -> Option { @@ -51,14 +83,17 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// Return `Some(more)` on success, where `more` is a bool indicating if /// there will be additional "tracepoint define" packets for this /// tracepoint. - pub fn actions(self, mut f: impl FnMut(&TracepointAction<'_, U>)) -> Option { + pub fn actions( + self, + mut f: impl FnMut(&TracepointAction<'_, U>), + ) -> Result { match self.actions { TracepointActionList::Raw { mut data } => Self::parse_raw_actions(&mut data, f), TracepointActionList::Parsed { mut actions, more } => { for action in actions.iter_mut() { (f)(action); } - Some(more) + Ok(more) } } } @@ -66,11 +101,14 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { fn parse_raw_actions( actions: &mut [u8], mut f: impl FnMut(&TracepointAction<'_, U>), - ) -> Option { + ) -> Result { let (actions, more) = match actions { [rest @ .., b'-'] => (rest, true), x => (x, false), }; + // TODO: There's no "packet unsupported", so for now we stub out unimplemented + // functionality by reporting the commands malformed instead. + use crate::protocol::PacketParseError::MalformedCommand; let mut unparsed: Option<&mut [u8]> = Some(actions); loop { match unparsed { @@ -85,7 +123,7 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { // "normals" actions may still be "while stepping" actions, // just continued from the previous packet, which we forgot // about! - return None; + return Err(MalformedCommand); } Some([b'R', mask @ ..]) => { let mask_end = mask @@ -96,10 +134,10 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { let mask = if let Some(mask_end) = mask_end { let (mask_bytes, next) = mask.split_at_mut(mask_end.0); unparsed = Some(next); - decode_hex_buf(mask_bytes).ok()? + decode_hex_buf(mask_bytes).or(Err(MalformedCommand))? } else { unparsed = None; - decode_hex_buf(mask).ok()? + decode_hex_buf(mask).or(Err(MalformedCommand))? }; (f)(&TracepointAction::Registers { mask: ManagedSlice::Borrowed(mask), @@ -108,14 +146,20 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { Some([b'M', _mem_args @ ..]) => { // Unimplemented: even simple actions like `collect *(int*)0x0` // are actually assembled as `X` bytecode actions - return None; + return Err(MalformedCommand); } Some([b'X', eval_args @ ..]) => { let mut len_end = eval_args.splitn_mut(2, |b| *b == b','); - let (len_bytes, rem) = (len_end.next()?, len_end.next()?); - let len: usize = decode_hex(len_bytes).ok()?; + let (len_bytes, rem) = ( + len_end.next().ok_or(MalformedCommand)?, + len_end.next().ok_or(MalformedCommand)?, + ); + let len: usize = decode_hex(len_bytes).or(Err(MalformedCommand))?; + if rem.len() < len * 2 { + return Err(MalformedCommand); + } let (expr_bytes, next_bytes) = rem.split_at_mut(len * 2); - let expr = decode_hex_buf(expr_bytes).ok()?; + let expr = decode_hex_buf(expr_bytes).or(Err(MalformedCommand))?; (f)(&TracepointAction::Expression { expr: ManagedSlice::Borrowed(expr), }); @@ -124,11 +168,242 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { Some([]) | None => { break; } - _ => return None, + _ => return Err(MalformedCommand), } } - Some(more) + Ok(more) + } +} +#[cfg(feature = "alloc")] +impl<'a, U: Copy> DefineTracepoint<'a, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> DefineTracepoint<'b, U> { + DefineTracepoint { + number: self.number, + addr: self.addr, + actions: self.actions.get_owned(), + } + } +} + +#[cfg(feature = "alloc")] +impl<'a, U: Copy> TracepointAction<'a, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointAction<'b, U> { + use core::ops::Deref; + match self { + TracepointAction::Registers { mask } => TracepointAction::Registers { + mask: ManagedSlice::Owned(mask.deref().into()), + }, + TracepointAction::Memory { + basereg, + offset, + length, + } => TracepointAction::Memory { + basereg: *basereg, + offset: *offset, + length: *length, + }, + TracepointAction::Expression { expr } => TracepointAction::Expression { + expr: ManagedSlice::Owned(expr.deref().into()), + }, + } + } +} + +#[cfg(feature = "alloc")] +impl<'a, U: Copy> TracepointActionList<'a, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointActionList<'b, U> { + use core::ops::Deref; + match self { + TracepointActionList::Raw { data } => TracepointActionList::Raw { + data: ManagedSlice::Owned(data.deref().into()), + }, + TracepointActionList::Parsed { actions, more } => TracepointActionList::Parsed { + actions: ManagedSlice::Owned( + actions.iter().map(|action| action.get_owned()).collect(), + ), + more: *more, + }, + } + } +} + +#[cfg(feature = "alloc")] +impl<'a, U: Copy> TracepointItem<'a, U> { + /// Allocate an owned copy of this structure. + pub fn get_owned<'b>(&self) -> TracepointItem<'b, U> { + match self { + TracepointItem::New(n) => TracepointItem::New(n.get_owned()), + TracepointItem::Define(d) => TracepointItem::Define(d.get_owned()), + } + } +} + +impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepoint<'a, U> { + pub(crate) fn write( + self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), Error> { + res.write_str("QTDP:-")?; + res.write_num(self.number.0)?; + res.write_str(":")?; + res.write_num(self.addr)?; + res.write_str(":")?; + let mut err = None; + let more = self.actions(|action| { + if let Err(e) = action.write::(res) { + err = Some(e) + } + }); + if let Some(e) = err { + return Err(e.into()); + } + match more { + Ok(true) => Ok(res.write_str("-")?), + Ok(false) => Ok(()), + e => e.map(|_| ()).map_err(|e| Error::PacketParse(e)), + } + } +} + +impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAction<'a, U> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), Error> { + match self { + TracepointAction::Registers { mask } => { + res.write_str("R")?; + res.write_hex_buf(mask)?; + } + TracepointAction::Memory { + basereg, + offset, + length, + } => { + res.write_str("M")?; + match basereg { + Some(r) => res.write_num(*r), + None => res.write_str("-1"), + }?; + res.write_str(",")?; + res.write_num(*offset)?; + res.write_str(",")?; + res.write_num(*length)?; + } + TracepointAction::Expression { expr } => { + res.write_str("X")?; + res.write_num(expr.len())?; + res.write_str(",")?; + res.write_hex_buf(expr)?; + } + } + Ok(()) + } +} + +impl<'a> ExperimentStatus<'a> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), ResponseWriterError> { + use crate::target::ext::tracepoints::ExperimentStatus::*; + if let Running = self { + return res.write_str("T1"); + } + // We're stopped for some reason, and may have an explanation for why + res.write_str("T0")?; + match self { + Running => { + /* unreachable */ + () + } + NotRunning => { + /* no information */ + () + } + NotRun => res.write_str(";tnotrun:0")?, + Stop(ref t) => match t { + Some(text) => { + res.write_str(";tstop:")?; + res.write_hex_buf(text)?; + res.write_str(":0")?; + } + None => res.write_str(";tstop:0")?, + }, + Full => res.write_str(";tfull:0")?, + Disconnected => res.write_str(";tdisconnected:0")?, + PassCount(tpnum) => { + res.write_str(";tpasscount:")?; + res.write_num(tpnum.0)?; + } + Error(text, tpnum) => { + res.write_str(";terror:")?; + res.write_hex_buf(text)?; + res.write_str(":")?; + res.write_num(tpnum.0)?; + } + Unknown => res.write_str(";tunknown:0")?, + } + + Ok(()) + } +} + +impl<'a> ExperimentExplanation<'a> { + pub(crate) fn write( + &self, + res: &mut ResponseWriter<'_, C>, + ) -> Result<(), Error> { + use ExperimentExplanation::*; + match self { + Frames(u) => { + res.write_str("tframes:")?; + res.write_num(*u)?; + } + Created(u) => { + res.write_str("tcreated:")?; + res.write_num(*u)?; + } + Size(u) => { + res.write_str("tsize:")?; + res.write_num(*u)?; + } + Free(u) => { + res.write_str("tfree:")?; + res.write_num(*u)?; + } + Circular(u) => { + res.write_str("circular:")?; + res.write_num(if *u { 1 } else { 0 })?; + } + Disconn(dis) => match dis { + true => res.write_str("disconn:1")?, + false => res.write_str("disconn:0")?, + }, + Other(body) => res.write_str(body.as_ref())?, + }; + + Ok(()) + } +} + +impl<'a, U: crate::internal::BeBytes> From> for Option> { + fn from(s: FrameRequest<&'a mut [u8]>) -> Self { + Some(match s { + FrameRequest::Select(u) => FrameRequest::Select(u), + FrameRequest::AtPC(u) => FrameRequest::AtPC(U::from_be_bytes(u)?), + FrameRequest::Hit(tp) => FrameRequest::Hit(tp), + FrameRequest::Between(s, e) => { + FrameRequest::Between(U::from_be_bytes(s)?, U::from_be_bytes(e)?) + } + FrameRequest::Outside(s, e) => { + FrameRequest::Outside(U::from_be_bytes(s)?, U::from_be_bytes(e)?) + } + }) } } @@ -156,9 +431,13 @@ impl GdbStubImpl { Tracepoints::qTStatus(_) => { let status = ops.trace_experiment_status().handle_error()?; status.write(res)?; - let mut err = None; + let mut err: Option> = None; ops.trace_experiment_info(&mut |explanation: ExperimentExplanation<'_>| { - if let Err(e) = res.write_str(";").and_then(|()| explanation.write(res)) { + if let Err(e) = res + .write_str(";") + .map_err(|e| e.into()) + .and_then(|()| explanation.write::(res)) + { err = Some(e) } }) @@ -269,11 +548,11 @@ impl GdbStubImpl { // which means we can't really setup the state machine ourselves or // turn it into a nicer API. Tracepoints::qTfP(_) => { - let mut err = None; + let mut err: Option> = None; ops.tracepoint_enumerate_start(&mut |tp| { let e = match tp { - TracepointItem::New(ctp) => ctp.write(res), - TracepointItem::Define(dtp) => dtp.write(res), + TracepointItem::New(ctp) => ctp.write::(res), + TracepointItem::Define(dtp) => dtp.write::(res), }; if let Err(e) = e { err = Some(e) @@ -285,11 +564,11 @@ impl GdbStubImpl { } } Tracepoints::qTsP(_) => { - let mut err = None; + let mut err: Option> = None; ops.tracepoint_enumerate_step(&mut |tp| { let e = match tp { - TracepointItem::New(ctp) => ctp.write(res), - TracepointItem::Define(dtp) => dtp.write(res), + TracepointItem::New(ctp) => ctp.write::(res), + TracepointItem::Define(dtp) => dtp.write::(res), }; if let Err(e) = e { err = Some(e) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index 5c9393e..c3987df 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -1,12 +1,8 @@ //! Provide tracepoints for the target. -use crate::conn::Connection; -use crate::protocol::ResponseWriter; -use crate::protocol::ResponseWriterError; use crate::target::Arch; use crate::target::Target; use crate::target::TargetResult; use managed::ManagedSlice; -use num_traits::PrimInt; /// A tracepoint, identified by a unique number. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -35,34 +31,6 @@ pub struct NewTracepoint { pub more: bool, } -#[cfg(feature = "alloc")] -impl NewTracepoint { - /// Allocate an owned copy of this structure. - pub fn get_owned(&self) -> NewTracepoint { - self.clone() - } -} - -impl NewTracepoint { - pub(crate) fn write( - &self, - res: &mut ResponseWriter<'_, C>, - ) -> Result<(), ResponseWriterError> { - res.write_str("QTDP:")?; - res.write_num(self.number.0)?; - res.write_str(":")?; - res.write_num(self.addr)?; - res.write_str(":")?; - res.write_str(if self.enabled { "E" } else { "D" })?; - res.write_str(":")?; - res.write_num(self.step_count)?; - res.write_str(":")?; - res.write_num(self.pass_count)?; - - Ok(()) - } -} - /// Describes how to collect information for a trace frame when the tracepoint /// it is attached to is hit. A tracepoint may have more than one action /// attached. @@ -85,67 +53,6 @@ pub enum TracepointAction<'a, U> { Expression { expr: ManagedSlice<'a, u8> }, } -#[cfg(feature = "alloc")] -impl<'a, U: Copy> TracepointAction<'a, U> { - /// Allocate an owned copy of this structure. - pub fn get_owned<'b>(&self) -> TracepointAction<'b, U> { - use core::ops::Deref; - match self { - TracepointAction::Registers { mask } => TracepointAction::Registers { - mask: ManagedSlice::Owned(mask.deref().into()), - }, - TracepointAction::Memory { - basereg, - offset, - length, - } => TracepointAction::Memory { - basereg: *basereg, - offset: *offset, - length: *length, - }, - TracepointAction::Expression { expr } => TracepointAction::Expression { - expr: ManagedSlice::Owned(expr.deref().into()), - }, - } - } -} - -impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> TracepointAction<'a, U> { - pub(crate) fn write( - &self, - res: &mut ResponseWriter<'_, C>, - ) -> Result<(), ResponseWriterError> { - match self { - TracepointAction::Registers { mask } => { - res.write_str("R")?; - res.write_hex_buf(mask)?; - } - TracepointAction::Memory { - basereg, - offset, - length, - } => { - res.write_str("M")?; - match basereg { - Some(r) => res.write_num(*r), - None => res.write_str("-1"), - }?; - res.write_str(",")?; - res.write_num(*offset)?; - res.write_str(",")?; - res.write_num(*length)?; - } - TracepointAction::Expression { expr } => { - res.write_str("X")?; - res.write_num(expr.len())?; - res.write_str(",")?; - res.write_hex_buf(expr)?; - } - } - Ok(()) - } -} - /// A list of TracepointActions, either raw and unparsed from a GDB packet, or /// a slice of parsed structures like which may be returned from enumerating /// tracepoints. @@ -163,25 +70,6 @@ pub enum TracepointActionList<'a, U> { }, } -#[cfg(feature = "alloc")] -impl<'a, U: Copy> TracepointActionList<'a, U> { - /// Allocate an owned copy of this structure. - pub fn get_owned<'b>(&self) -> TracepointActionList<'b, U> { - use core::ops::Deref; - match self { - TracepointActionList::Raw { data } => TracepointActionList::Raw { - data: ManagedSlice::Owned(data.deref().into()), - }, - TracepointActionList::Parsed { actions, more } => TracepointActionList::Parsed { - actions: ManagedSlice::Owned( - actions.iter().map(|action| action.get_owned()).collect(), - ), - more: *more, - }, - } - } -} - /// Definition data for a tracepoint. A tracepoint may have more than one define /// structure for all of its data. GDB may ask for the state of current /// tracepoints, which are described with this same structure. @@ -195,45 +83,6 @@ pub struct DefineTracepoint<'a, U> { pub actions: TracepointActionList<'a, U>, } -#[cfg(feature = "alloc")] -impl<'a, U: Copy> DefineTracepoint<'a, U> { - /// Allocate an owned copy of this structure. - pub fn get_owned<'b>(&self) -> DefineTracepoint<'b, U> { - DefineTracepoint { - number: self.number, - addr: self.addr, - actions: self.actions.get_owned(), - } - } -} - -impl<'a, U: crate::internal::BeBytes + num_traits::Zero + PrimInt> DefineTracepoint<'a, U> { - pub(crate) fn write( - self, - res: &mut ResponseWriter<'_, C>, - ) -> Result<(), ResponseWriterError> { - res.write_str("QTDP:-")?; - res.write_num(self.number.0)?; - res.write_str(":")?; - res.write_num(self.addr)?; - res.write_str(":")?; - let mut err = None; - let more = self.actions(|action| { - if let Err(e) = action.write(res) { - err = Some(e) - } - }); - if let Some(e) = err { - return Err(e); - } - if let Some(true) = more { - res.write_str("-")?; - } - - Ok(()) - } -} - /// An item from a stream of tracepoint descriptions. Enumerating tracepoints /// should emit a sequence of Create and Define items for all the tracepoints /// that are loaded. @@ -251,17 +100,6 @@ pub enum TracepointItem<'a, U> { Define(DefineTracepoint<'a, U>), } -#[cfg(feature = "alloc")] -impl<'a, U: Copy> TracepointItem<'a, U> { - /// Allocate an owned copy of this structure. - pub fn get_owned<'b>(&self) -> TracepointItem<'b, U> { - match self { - TracepointItem::New(n) => TracepointItem::New(n.get_owned()), - TracepointItem::Define(d) => TracepointItem::Define(d.get_owned()), - } - } -} - /// The running state of a trace experiment. #[derive(Debug)] pub enum ExperimentStatus<'a> { @@ -314,92 +152,6 @@ pub enum ExperimentExplanation<'a> { Other(managed::Managed<'a, str>), } -impl<'a> ExperimentStatus<'a> { - pub(crate) fn write( - &self, - res: &mut ResponseWriter<'_, C>, - ) -> Result<(), ResponseWriterError> { - use ExperimentStatus::*; - if let Running = self { - return res.write_str("T1"); - } - // We're stopped for some reason, and may have an explanation for why - res.write_str("T0")?; - match self { - Running => { - /* unreachable */ - () - } - NotRunning => { - /* no information */ - () - } - NotRun => res.write_str(";tnotrun:0")?, - Stop(ref t) => match t { - Some(text) => { - res.write_str(";tstop:")?; - res.write_hex_buf(text)?; - res.write_str(":0")?; - } - None => res.write_str(";tstop:0")?, - }, - Full => res.write_str(";tfull:0")?, - Disconnected => res.write_str(";tdisconnected:0")?, - PassCount(tpnum) => { - res.write_str(";tpasscount:")?; - res.write_num(tpnum.0)?; - } - Error(text, tpnum) => { - res.write_str(";terror:")?; - res.write_hex_buf(text)?; - res.write_str(":")?; - res.write_num(tpnum.0)?; - } - Unknown => res.write_str(";tunknown:0")?, - } - - Ok(()) - } -} - -impl<'a> ExperimentExplanation<'a> { - pub(crate) fn write( - &self, - res: &mut ResponseWriter<'_, C>, - ) -> Result<(), ResponseWriterError> { - use ExperimentExplanation::*; - match self { - Frames(u) => { - res.write_str("tframes:")?; - res.write_num(*u)?; - } - Created(u) => { - res.write_str("tcreated:")?; - res.write_num(*u)?; - } - Size(u) => { - res.write_str("tsize:")?; - res.write_num(*u)?; - } - Free(u) => { - res.write_str("tfree:")?; - res.write_num(*u)?; - } - Circular(u) => { - res.write_str("circular:")?; - res.write_num(if *u { 1 } else { 0 })?; - } - Disconn(dis) => match dis { - true => res.write_str("disconn:1")?, - false => res.write_str("disconn:0")?, - }, - Other(body) => res.write_str(body.as_ref())?, - }; - - Ok(()) - } -} - /// Shape of the trace buffer #[derive(Debug)] pub enum BufferShape { @@ -438,22 +190,6 @@ pub enum FrameRequest { Outside(U, U), } -impl<'a, U: crate::internal::BeBytes> From> for Option> { - fn from(s: FrameRequest<&'a mut [u8]>) -> Self { - Some(match s { - FrameRequest::Select(u) => FrameRequest::Select(u), - FrameRequest::AtPC(u) => FrameRequest::AtPC(U::from_be_bytes(u)?), - FrameRequest::Hit(tp) => FrameRequest::Hit(tp), - FrameRequest::Between(s, e) => { - FrameRequest::Between(U::from_be_bytes(s)?, U::from_be_bytes(e)?) - } - FrameRequest::Outside(s, e) => { - FrameRequest::Outside(U::from_be_bytes(s)?, U::from_be_bytes(e)?) - } - }) - } -} - /// Describes a detail of a frame from the trace buffer #[derive(Debug)] pub enum FrameDescription { From 7631df446756865c207759be6ce13b236e4c84e2 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 18:38:06 -0500 Subject: [PATCH 18/29] fix actions documentation --- src/stub/core_impl/tracepoints.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 0ca4aab..da7a3b5 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -79,8 +79,8 @@ impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// Parse the actions that should be added to the definition of this /// tracepoint, calling `f` on each action. /// - /// Returns None if parsing of actions failed, or hit unsupported actions. - /// Return `Some(more)` on success, where `more` is a bool indicating if + /// Returns `Err` if parsing of actions failed, or hit unsupported actions. + /// Return `Ok(more)` on success, where `more` is a bool indicating if /// there will be additional "tracepoint define" packets for this /// tracepoint. pub fn actions( From 2b632ab721f820f08c49c9b86cf4454a0534117c Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 18:53:03 -0500 Subject: [PATCH 19/29] fix TracepointAction documentation --- src/target/ext/tracepoints.rs | 41 ++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index c3987df..e2a0364 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -35,37 +35,48 @@ pub struct NewTracepoint { /// it is attached to is hit. A tracepoint may have more than one action /// attached. #[derive(Debug)] -#[allow(missing_docs)] pub enum TracepointAction<'a, U> { - /// Collect the registers whose bits are set in `mask` (big endian). - /// Note that `mask` may be larger than the word length. - Registers { mask: ManagedSlice<'a, u8> }, - /// Collect `len` bytes of memory starting at the address in register number + /// Collect registers. + Registers { + /// A bitmask of which registers should be collected. The least significant + /// bit is numberered zero. Note that the mask may be larger than the word length. + mask: ManagedSlice<'a, u8> + }, + /// Collect memory.`len` bytes of memory starting at the address in register number /// `basereg`, plus `offset`. If `basereg` is None, then treat it as a fixed /// address. Memory { + /// If `Some`, then calculate the address of memory to collect relative to + /// the value of this register number. If `None` then memory should be + /// collected from a fixed address. basereg: Option, + /// The offset used to calculate the address to collect memory from. offset: U, + /// How many bytes of memory to collect. length: u64, }, - /// Evaluate `expr`, which is a GDB agent bytecode expression, and collect - /// memory as it directs. - Expression { expr: ManagedSlice<'a, u8> }, + /// Collect data according to an agent bytecode program. + Expression { + /// The GDB agent bytecode program to evaluate. + expr: ManagedSlice<'a, u8> + }, } -/// A list of TracepointActions, either raw and unparsed from a GDB packet, or -/// a slice of parsed structures like which may be returned from enumerating -/// tracepoints. +/// A list of TracepointActions. #[derive(Debug)] -#[allow(missing_docs)] pub enum TracepointActionList<'a, U> { /// Raw and unparsed actions, such as from GDB. - Raw { data: ManagedSlice<'a, u8> }, + Raw { + /// The unparsed action data. + data: ManagedSlice<'a, u8> + }, /// A slice of parsed actions, such as what may be returned by a target when - /// enumerating tracepoints. `more` must be set if there will be another - /// "tracepoint definition" with more actions for this tracepoint. + /// enumerating tracepoints. Parsed { + /// The parsed actions. actions: ManagedSlice<'a, TracepointAction<'a, U>>, + /// Indicate if there will be additional "tracepoint definitions" with + /// more actions for this tracepoint. more: bool, }, } From 0ea02456269a97c7482db80bd39782f1570e374d Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 19:19:18 -0500 Subject: [PATCH 20/29] break out tracepoint status information --- examples/armv4t/gdb/tracepoints.rs | 16 +++++++++----- src/stub/core_impl/tracepoints.rs | 10 ++++++--- src/target/ext/tracepoints.rs | 34 +++++++++++++++++++----------- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/examples/armv4t/gdb/tracepoints.rs b/examples/armv4t/gdb/tracepoints.rs index 0f45b60..7f9fc83 100644 --- a/examples/armv4t/gdb/tracepoints.rs +++ b/examples/armv4t/gdb/tracepoints.rs @@ -10,6 +10,7 @@ use gdbstub::target::ext::tracepoints::TraceBufferConfig; use gdbstub::target::ext::tracepoints::Tracepoint; use gdbstub::target::ext::tracepoints::TracepointAction; use gdbstub::target::ext::tracepoints::TracepointItem; +use gdbstub::target::ext::tracepoints::TracepointStatus; use gdbstub::target::TargetError; use gdbstub::target::TargetResult; @@ -59,18 +60,23 @@ impl target::ext::tracepoints::Tracepoints for Emu { .ok_or_else(move || TargetError::Fatal("define on non-existing tracepoint")) } - fn tracepoint_status(&self, tp: Tracepoint, _addr: u32) -> TargetResult<(u64, u64), Self> { + fn tracepoint_status( + &self, + tp: Tracepoint, + _addr: u32, + ) -> TargetResult { // We don't collect "real" trace buffer frames, so just report hit count // and say the number of bytes is always 0. // Because we don't implement "while-stepping" actions, we don't need to // also check that `addr` matches. - Ok(( - self.traceframes + Ok(TracepointStatus { + hit_count: self + .traceframes .iter() .filter(|frame| frame.number.0 == tp.0) .count() as u64, - 0, - )) + bytes_used: 0, + }) } fn tracepoint_enumerate_start( diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index da7a3b5..2c93a87 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -19,6 +19,7 @@ use crate::target::ext::tracepoints::NewTracepoint; use crate::target::ext::tracepoints::TracepointAction; use crate::target::ext::tracepoints::TracepointActionList; use crate::target::ext::tracepoints::TracepointItem; +use crate::target::ext::tracepoints::TracepointStatus; use managed::ManagedSlice; use num_traits::PrimInt; @@ -449,11 +450,14 @@ impl GdbStubImpl { Tracepoints::qTP(qtp) => { let addr = ::Usize::from_be_bytes(qtp.addr) .ok_or(Error::TargetMismatch)?; - let (hits, usage) = ops.tracepoint_status(qtp.tracepoint, addr).handle_error()?; + let TracepointStatus { + hit_count, + bytes_used, + } = ops.tracepoint_status(qtp.tracepoint, addr).handle_error()?; res.write_str("V")?; - res.write_num(hits)?; + res.write_num(hit_count)?; res.write_str(":")?; - res.write_num(usage)?; + res.write_num(bytes_used)?; } Tracepoints::QTDP(q) => { match q { diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index e2a0364..b6d9fb4 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -38,17 +38,18 @@ pub struct NewTracepoint { pub enum TracepointAction<'a, U> { /// Collect registers. Registers { - /// A bitmask of which registers should be collected. The least significant - /// bit is numberered zero. Note that the mask may be larger than the word length. - mask: ManagedSlice<'a, u8> + /// A bitmask of which registers should be collected. The least + /// significant bit is numberered zero. Note that the mask may + /// be larger than the word length. + mask: ManagedSlice<'a, u8>, }, - /// Collect memory.`len` bytes of memory starting at the address in register number - /// `basereg`, plus `offset`. If `basereg` is None, then treat it as a fixed - /// address. + /// Collect memory.`len` bytes of memory starting at the address in register + /// number `basereg`, plus `offset`. If `basereg` is None, then treat it + /// as a fixed address. Memory { - /// If `Some`, then calculate the address of memory to collect relative to - /// the value of this register number. If `None` then memory should be - /// collected from a fixed address. + /// If `Some`, then calculate the address of memory to collect relative + /// to the value of this register number. If `None` then memory + /// should be collected from a fixed address. basereg: Option, /// The offset used to calculate the address to collect memory from. offset: U, @@ -58,7 +59,7 @@ pub enum TracepointAction<'a, U> { /// Collect data according to an agent bytecode program. Expression { /// The GDB agent bytecode program to evaluate. - expr: ManagedSlice<'a, u8> + expr: ManagedSlice<'a, u8>, }, } @@ -68,7 +69,7 @@ pub enum TracepointActionList<'a, U> { /// Raw and unparsed actions, such as from GDB. Raw { /// The unparsed action data. - data: ManagedSlice<'a, u8> + data: ManagedSlice<'a, u8>, }, /// A slice of parsed actions, such as what may be returned by a target when /// enumerating tracepoints. @@ -210,6 +211,15 @@ pub enum FrameDescription { Hit(Tracepoint), } +/// The state of a tracepoint. +#[derive(Debug)] +pub struct TracepointStatus { + /// The number of times a tracepoint has been hit in a trace run. + pub hit_count: u64, + /// The number of bytes the tracepoint accounts for in the trace buffer. + pub bytes_used: u64, +} + /// Target Extension - Provide tracepoints. pub trait Tracepoints: Target { /// Clear any saved tracepoints and empty the trace frame buffer @@ -232,7 +242,7 @@ pub trait Tracepoints: Target { &self, tp: Tracepoint, addr: ::Usize, - ) -> TargetResult<(u64, u64), Self>; + ) -> TargetResult; /// Begin enumerating tracepoints. The target implementation should /// initialize a state machine that is stepped by From 1da19ab0bbcf7d6e868217174bf4adc9b08c5494 Mon Sep 17 00:00:00 2001 From: cczetier Date: Mon, 6 Jan 2025 19:20:21 -0500 Subject: [PATCH 21/29] fix tracepoint status documentation --- src/target/ext/tracepoints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index b6d9fb4..f504ccd 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -237,7 +237,7 @@ pub trait Tracepoints: Target { ) -> TargetResult<(), Self>; /// Request the status of tracepoint `tp` at address `addr`. /// - /// Returns `(number of tracepoint hits, number of bytes used for frames)`. + /// Returns a [TracepointStatus] with the requested information. fn tracepoint_status( &self, tp: Tracepoint, From ad297de51b5d8e696a5927b9813c72eadb59c983 Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 7 Jan 2025 15:47:32 -0500 Subject: [PATCH 22/29] rustfmt --- src/protocol/commands/_QTBuffer.rs | 12 ++++++------ src/protocol/commands/_QTDP.rs | 16 ++++++++++------ src/protocol/commands/_QTFrame.rs | 17 ++++++++--------- src/protocol/commands/_QTinit.rs | 9 ++++++--- src/protocol/commands/_qTBuffer.rs | 12 ++++++++---- src/protocol/commands/_qTP.rs | 7 ++----- src/protocol/commands/_qTStatus.rs | 9 ++++++--- 7 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/protocol/commands/_QTBuffer.rs b/src/protocol/commands/_QTBuffer.rs index f190e38..ed39c54 100644 --- a/src/protocol/commands/_QTBuffer.rs +++ b/src/protocol/commands/_QTBuffer.rs @@ -1,5 +1,6 @@ use super::prelude::*; -use crate::target::ext::tracepoints::{BufferShape, TraceBufferConfig}; +use crate::target::ext::tracepoints::BufferShape; +use crate::target::ext::tracepoints::TraceBufferConfig; #[derive(Debug)] pub struct QTBuffer(pub TraceBufferConfig); @@ -19,18 +20,17 @@ impl ParseCommand<'_> for QTBuffer { [b'0'] => Some(BufferShape::Linear), _ => None, }?))) - }, + } b"size" => { let size = s.next()?; Some(QTBuffer(TraceBufferConfig::Size(match size { [b'-', b'1'] => None, - i => Some(decode_hex(i).ok()?) + i => Some(decode_hex(i).ok()?), }))) - }, + } _ => None, } - - }, + } _ => None, } } diff --git a/src/protocol/commands/_QTDP.rs b/src/protocol/commands/_QTDP.rs index 751f70e..464650c 100644 --- a/src/protocol/commands/_QTDP.rs +++ b/src/protocol/commands/_QTDP.rs @@ -41,15 +41,15 @@ impl<'a> ParseCommand<'a> for QTDP<'a> { number, addr, while_stepping: false, - actions + actions, })) - }, + } [b':', tracepoint @ ..] => { // Strip off the trailing '-' that indicates if there will be // more packets after this let (tracepoint, more) = match tracepoint { [rest @ .., b'-'] => (rest, true), - x => (x, false) + x => (x, false), }; let mut params = tracepoint.splitn_mut(6, |b| *b == b':'); let n = Tracepoint(decode_hex(params.next()?).ok()?); @@ -64,7 +64,11 @@ impl<'a> ParseCommand<'a> for QTDP<'a> { return Some(QTDP::Create(CreateTDP { number: n, addr, - enable: match ena { [b'E'] => Some(true), [b'D'] => Some(false), _ => None }?, + enable: match ena { + [b'E'] => Some(true), + [b'D'] => Some(false), + _ => None, + }?, step, pass, more, @@ -72,8 +76,8 @@ impl<'a> ParseCommand<'a> for QTDP<'a> { // TODO: populate tracepoint conditions fast: None, condition: None, - })) - }, + })); + } _ => None, } } diff --git a/src/protocol/commands/_QTFrame.rs b/src/protocol/commands/_QTFrame.rs index 79002fb..beaad7d 100644 --- a/src/protocol/commands/_QTFrame.rs +++ b/src/protocol/commands/_QTFrame.rs @@ -1,5 +1,6 @@ use super::prelude::*; -use crate::target::ext::tracepoints::{Tracepoint, FrameRequest}; +use crate::target::ext::tracepoints::FrameRequest; +use crate::target::ext::tracepoints::Tracepoint; #[derive(Debug)] pub struct QTFrame<'a>(pub FrameRequest<&'a mut [u8]>); @@ -16,26 +17,24 @@ impl<'a> ParseCommand<'a> for QTFrame<'a> { b"pc" => { let addr = decode_hex_buf(s.next()?).ok()?; QTFrame(FrameRequest::AtPC(addr)) - }, + } b"tdp" => { let tp = Tracepoint(decode_hex(s.next()?).ok()?); QTFrame(FrameRequest::Hit(tp)) - }, + } b"range" => { let start = decode_hex_buf(s.next()?).ok()?; let end = decode_hex_buf(s.next()?).ok()?; QTFrame(FrameRequest::Between(start, end)) - }, + } b"outside" => { let start = decode_hex_buf(s.next()?).ok()?; let end = decode_hex_buf(s.next()?).ok()?; QTFrame(FrameRequest::Outside(start, end)) - }, - n => { - QTFrame(FrameRequest::Select(decode_hex(n).ok()?)) - }, + } + n => QTFrame(FrameRequest::Select(decode_hex(n).ok()?)), }) - }, + } _ => None, } } diff --git a/src/protocol/commands/_QTinit.rs b/src/protocol/commands/_QTinit.rs index 13d33fc..73a7e5d 100644 --- a/src/protocol/commands/_QTinit.rs +++ b/src/protocol/commands/_QTinit.rs @@ -1,11 +1,14 @@ use super::prelude::*; #[derive(Debug)] -pub struct QTinit { } +pub struct QTinit {} impl<'a> ParseCommand<'a> for QTinit { fn from_packet(buf: PacketBuf<'a>) -> Option { - if !buf.into_body().is_empty() { None } - else { Some(Self { }) } + if !buf.into_body().is_empty() { + None + } else { + Some(Self {}) + } } } diff --git a/src/protocol/commands/_qTBuffer.rs b/src/protocol/commands/_qTBuffer.rs index 531a4cb..a907581 100644 --- a/src/protocol/commands/_qTBuffer.rs +++ b/src/protocol/commands/_qTBuffer.rs @@ -4,7 +4,7 @@ use super::prelude::*; pub struct qTBuffer<'a> { pub offset: u64, pub length: usize, - pub data: &'a mut [u8] + pub data: &'a mut [u8], } impl<'a> ParseCommand<'a> for qTBuffer<'a> { @@ -22,9 +22,13 @@ impl<'a> ParseCommand<'a> for qTBuffer<'a> { // our packet size, which means we actually have half as much space // as our slice would indicate. let (front, _back) = buf.split_at_mut(buf.len() / 2); - Some(qTBuffer { offset, length, data: front }) - }, - _ => None + Some(qTBuffer { + offset, + length, + data: front, + }) + } + _ => None, } } } diff --git a/src/protocol/commands/_qTP.rs b/src/protocol/commands/_qTP.rs index 14520ba..7154348 100644 --- a/src/protocol/commands/_qTP.rs +++ b/src/protocol/commands/_qTP.rs @@ -16,11 +16,8 @@ impl<'a> ParseCommand<'a> for qTP<'a> { let mut s = body.split_mut(|b| *b == b':'); let tracepoint = Tracepoint(decode_hex(s.next()?).ok()?); let addr = decode_hex_buf(s.next()?).ok()?; - Some(qTP { - tracepoint, - addr - }) - }, + Some(qTP { tracepoint, addr }) + } _ => None, } } diff --git a/src/protocol/commands/_qTStatus.rs b/src/protocol/commands/_qTStatus.rs index 8463a57..8b611c4 100644 --- a/src/protocol/commands/_qTStatus.rs +++ b/src/protocol/commands/_qTStatus.rs @@ -1,11 +1,14 @@ use super::prelude::*; #[derive(Debug)] -pub struct qTStatus { } +pub struct qTStatus {} impl<'a> ParseCommand<'a> for qTStatus { fn from_packet(buf: PacketBuf<'a>) -> Option { - if !buf.into_body().is_empty() { None } - else { Some(Self { }) } + if !buf.into_body().is_empty() { + None + } else { + Some(Self {}) + } } } From 11d5fed9b7a8f4c1c94c860ed75e69377ba65ad4 Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 7 Jan 2025 15:50:50 -0500 Subject: [PATCH 23/29] standardize QTinit --- src/protocol/commands/_QTinit.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/protocol/commands/_QTinit.rs b/src/protocol/commands/_QTinit.rs index 73a7e5d..e1d48f4 100644 --- a/src/protocol/commands/_QTinit.rs +++ b/src/protocol/commands/_QTinit.rs @@ -1,14 +1,14 @@ use super::prelude::*; #[derive(Debug)] -pub struct QTinit {} +pub struct QTinit; impl<'a> ParseCommand<'a> for QTinit { + #[inline(always)] fn from_packet(buf: PacketBuf<'a>) -> Option { if !buf.into_body().is_empty() { - None - } else { - Some(Self {}) + return None; } + Some(QTinit) } } From 9ce35a30316849e74f011f73574a96c0fb571cdf Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 7 Jan 2025 15:51:55 -0500 Subject: [PATCH 24/29] standardize qTStatus --- src/protocol/commands/_qTStatus.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/protocol/commands/_qTStatus.rs b/src/protocol/commands/_qTStatus.rs index 8b611c4..a805315 100644 --- a/src/protocol/commands/_qTStatus.rs +++ b/src/protocol/commands/_qTStatus.rs @@ -1,14 +1,14 @@ use super::prelude::*; #[derive(Debug)] -pub struct qTStatus {} +pub struct qTStatus; impl<'a> ParseCommand<'a> for qTStatus { + #[inline(always)] fn from_packet(buf: PacketBuf<'a>) -> Option { if !buf.into_body().is_empty() { - None - } else { - Some(Self {}) + return None; } + Some(qTStatus) } } From 7ebd28fcb02d01fb8d8c44b7e3a76fde5fce6f44 Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 7 Jan 2025 16:21:20 -0500 Subject: [PATCH 25/29] remove extra derives --- src/protocol/commands.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/protocol/commands.rs b/src/protocol/commands.rs index 4431f84..0042b90 100644 --- a/src/protocol/commands.rs +++ b/src/protocol/commands.rs @@ -51,7 +51,6 @@ macro_rules! commands { pub mod ext { $( #[allow(non_camel_case_types, clippy::enum_variant_names)] - #[derive(Debug)] pub enum [<$ext:camel>] $(<$lt>)? { $($command(super::$mod::$command<$($lifetime)?>),)* } @@ -59,7 +58,6 @@ macro_rules! commands { use super::breakpoint::{BasicBreakpoint, BytecodeBreakpoint}; #[allow(non_camel_case_types)] - #[derive(Debug)] pub enum Breakpoints<'a> { z(BasicBreakpoint<'a>), Z(BasicBreakpoint<'a>), @@ -71,7 +69,6 @@ macro_rules! commands { } /// GDB commands - #[derive(Debug)] pub enum Command<'a> { $( [<$ext:camel>](ext::[<$ext:camel>]$(<$lt>)?), From 340b43feac9838a7cd383948f6064e4cbbb7ac2d Mon Sep 17 00:00:00 2001 From: cczetier Date: Tue, 7 Jan 2025 16:23:22 -0500 Subject: [PATCH 26/29] remove pub from from_tdp --- src/stub/core_impl/tracepoints.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index 2c93a87..c542214 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -25,7 +25,7 @@ use num_traits::PrimInt; impl NewTracepoint { /// Parse from a raw CreateTDP packet. - pub fn from_tdp(ctdp: CreateTDP<'_>) -> Option { + fn from_tdp(ctdp: CreateTDP<'_>) -> Option { Some(Self { number: ctdp.number, addr: U::from_be_bytes(ctdp.addr)?, @@ -67,7 +67,7 @@ impl NewTracepoint impl<'a, U: BeBytes> DefineTracepoint<'a, U> { /// Parse from a raw DefineTDP packet. - pub fn from_tdp(dtdp: DefineTDP<'a>) -> Option { + fn from_tdp(dtdp: DefineTDP<'a>) -> Option { Some(Self { number: dtdp.number, addr: U::from_be_bytes(dtdp.addr)?, From ce4acb49211d501d99a90c6ee0b0301c8a1ecc18 Mon Sep 17 00:00:00 2001 From: cczetier Date: Wed, 8 Jan 2025 16:11:57 -0500 Subject: [PATCH 27/29] explain tracepoint extensions, drop InstallInTrace --- src/stub/core_impl/base.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/stub/core_impl/base.rs b/src/stub/core_impl/base.rs index 969dfd4..840338f 100644 --- a/src/stub/core_impl/base.rs +++ b/src/stub/core_impl/base.rs @@ -173,7 +173,22 @@ impl GdbStubImpl { } if let Some(_ops) = target.support_tracepoints() { - res.write_str(";InstallInTrace+")?; + // There are a number of optional tracepoint extensions that + // gdbstub should eventually implement. + // * `StaticTracepoint` for static tracepoint support. + // * `EnableDisableTracepoints` for enabling/disabling tracepoints during a + // trace experiment. + // * `tracenz` for the tracenz agent bytecode operation. + // * The `Qbtrace:*` family for branch tracing. + // * `InstallInTrace` allows for gdbstub to deliver tracepoint configuration + // commands while the trace experiment is running instead of them only taking + // affect on the next `tstart` command. + // + // For now, gdbstub doesn't provide trait extensions for these + // options and so we don't report support. We do report support + // for one extension however: + // * `QTBuffer:size` for configuring the trace buffer size, since + // the target is allowed to implement it as a no-op. res.write_str(";QTBuffer:size+")?; } From 992d54a3d94ced35038a9567ab7fa1fb78429c11 Mon Sep 17 00:00:00 2001 From: cczetier Date: Wed, 8 Jan 2025 16:27:59 -0500 Subject: [PATCH 28/29] remove get_owned and just use clone derive --- src/stub/core_impl/tracepoints.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/stub/core_impl/tracepoints.rs b/src/stub/core_impl/tracepoints.rs index c542214..868aac7 100644 --- a/src/stub/core_impl/tracepoints.rs +++ b/src/stub/core_impl/tracepoints.rs @@ -37,14 +37,6 @@ impl NewTracepoint { } } -#[cfg(feature = "alloc")] -impl NewTracepoint { - /// Allocate an owned copy of this structure. - pub fn get_owned(&self) -> NewTracepoint { - self.clone() - } -} - impl NewTracepoint { pub(crate) fn write( &self, @@ -237,7 +229,7 @@ impl<'a, U: Copy> TracepointItem<'a, U> { /// Allocate an owned copy of this structure. pub fn get_owned<'b>(&self) -> TracepointItem<'b, U> { match self { - TracepointItem::New(n) => TracepointItem::New(n.get_owned()), + TracepointItem::New(n) => TracepointItem::New(n.clone()), TracepointItem::Define(d) => TracepointItem::Define(d.get_owned()), } } From de201ebd9d8107361eca9f321fce8299daa75a9a Mon Sep 17 00:00:00 2001 From: cczetier Date: Thu, 9 Jan 2025 15:53:40 -0500 Subject: [PATCH 29/29] restrict visibility for internal actions list, removed managed slice for explanation string --- src/target/ext/tracepoints.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/ext/tracepoints.rs b/src/target/ext/tracepoints.rs index f504ccd..73b7a2f 100644 --- a/src/target/ext/tracepoints.rs +++ b/src/target/ext/tracepoints.rs @@ -92,7 +92,7 @@ pub struct DefineTracepoint<'a, U> { /// The PC address of the tracepoint that is being defined. pub addr: U, /// A list of actions that should be appended to the tracepoint. - pub actions: TracepointActionList<'a, U>, + pub(crate) actions: TracepointActionList<'a, U>, } /// An item from a stream of tracepoint descriptions. Enumerating tracepoints @@ -161,7 +161,7 @@ pub enum ExperimentExplanation<'a> { Disconn(bool), /// Report a raw string as a trace status explanation. - Other(managed::Managed<'a, str>), + Other(&'a str), } /// Shape of the trace buffer