From a4468be1a97c24ee6bcd8aac67c8f18dbcfe493f Mon Sep 17 00:00:00 2001 From: Putta Khunchalee Date: Sun, 24 May 2026 18:46:36 +0700 Subject: [PATCH] Allows GDB handler to postpone response (#1455) --- Cargo.lock | 1 + gui/Cargo.toml | 1 + gui/src/gdb/client.rs | 101 ++++++++++++++++++++--------------- gui/src/gdb/mod.rs | 1 - gui/src/gdb/state.rs | 121 +++++++++++++++--------------------------- gui/src/main.rs | 25 +++++++-- 6 files changed, 126 insertions(+), 124 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d73d0ec..3db36d65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1619,6 +1619,7 @@ version = "0.1.0" dependencies = [ "aarch64", "anstyle-parse", + "arrayvec", "ash 0.37.3+1.3.251", "ashpd", "async-net", diff --git a/gui/Cargo.toml b/gui/Cargo.toml index 3c76f93e..cf428429 100644 --- a/gui/Cargo.toml +++ b/gui/Cargo.toml @@ -9,6 +9,7 @@ path = "src/main.rs" [dependencies] anstyle-parse = "0.2.6" +arrayvec = "0.7.6" async-net = "2.0.0" bytes = "1.9.0" clap = { version = "4.5.21", features = ["derive"] } diff --git a/gui/src/gdb/client.rs b/gui/src/gdb/client.rs index b6c8ace7..ae346dff 100644 --- a/gui/src/gdb/client.rs +++ b/gui/src/gdb/client.rs @@ -1,4 +1,5 @@ use super::{GdbError, GdbHandler, GdbSession}; +use arrayvec::ArrayVec; /// Implementation of [`GdbDispatcher`] to dispatch requests from GDB client. pub struct ClientDispatcher<'a, H> { @@ -11,10 +12,11 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { Self { session, handler } } - pub async fn pump(&mut self) -> Result + '_>, GdbError> { + pub async fn pump( + &mut self, + ) -> Result, Vec, ArrayVec)>, GdbError> { // Check if GDB packet. let req = &mut self.session.req; - let res = &mut self.session.res; let state = &mut self.session.state; match req.first().copied() { @@ -27,7 +29,8 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { } req.drain(..1); - return Ok(res.drain(0..0).into()); + + return Ok(Some(Default::default())); } Some(v) => return Err(GdbError::UnknownPacketPrefix(v)), None => return Ok(None), @@ -55,8 +58,11 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { Some(false) => return Err(GdbError::MissingAck), None => { // TODO: Should we consider this as an invalid packet instead? - res.push(b'-'); // Request retransmission. - return Ok(res.drain(..).into()); + let mut h = ArrayVec::new(); + + h.push(b'-'); // Request retransmission. + + return Ok(Some((h, Vec::new(), ArrayVec::new()))); } } } @@ -70,23 +76,26 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { Some(true) => return Err(GdbError::InvalidChecksum(checksum, expect)), Some(false) => return Err(GdbError::MissingAck), None => { - res.push(b'-'); // Request retransmission. - return Ok(res.drain(..).into()); + let mut h = ArrayVec::new(); + + h.push(b'-'); // Request retransmission. + + return Ok(Some((h, Vec::new(), ArrayVec::new()))); } } } // Push response prefix. + let mut head = ArrayVec::new(); + match state.no_ack() { Some(true) => (), Some(false) => return Err(GdbError::MissingAck), - None => res.push(b'+'), + None => head.push(b'+'), } - res.push(b'$'); - // Execute command. - let off = res.len(); + let body; macro_rules! parse { () => { @@ -94,8 +103,9 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { }; ($cmd:literal => $body:expr, $($rem:tt)*) => { if data == $cmd.as_bytes() { - if let Err(e) = $body { - return Err(GdbError::Parse($cmd, e)); + match $body { + Ok(v) => body = v, + Err(e) => return Err(GdbError::Parse($cmd, e)), } } else { parse!($($rem)*); @@ -103,8 +113,9 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { }; ($prefix:literal | $data:ident => $body:expr, $($rem:tt)*) => { if let Some($data) = data.strip_prefix($prefix.as_bytes()) { - if let Err(e) = $body { - return Err(GdbError::Parse($prefix, e)); + match $body { + Ok(v) => body = v, + Err(e) => return Err(GdbError::Parse($prefix, e)), } } else { parse!($($rem)*); @@ -116,55 +127,61 @@ impl<'a, H: GdbHandler> ClientDispatcher<'a, H> { // Queries the reason the target halted. Defined on the Packets page (search for "'?'" // near the top of the packet list). // See https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html - "?" => state.parse_stop_reason(res, self.handler).await, - "c" | data => state.parse_continue(data, res, self.handler), - "jThreadsInfo" => Ok(()), - "m" | data => state.parse_read_memory(data, res, self.handler, false).await, + "?" => state.parse_stop_reason(self.handler).await, + "c" | data => state.parse_continue(data, self.handler), + "jThreadsInfo" => Ok(Some(Vec::new())), + "m" | data => state.parse_read_memory(data, self.handler, false).await, // https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html - "p" | data => state.parse_read_register(data, res, self.handler).await, + "p" | data => state.parse_read_register(data, self.handler).await, // https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#index-qC-packet - "qC" => state.parse_current_thread(res), + "qC" => state.parse_current_thread(), // I think this does not worth for additional complexity on our side so we don't support // this. See https://lldb.llvm.org/resources/lldbgdbremote.html#qenableerrorstrings for // more details. - "QEnableErrorStrings" => Ok(()), + "QEnableErrorStrings" => Ok(Some(Vec::new())), // https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#index-qfThreadInfo-packet - "qfThreadInfo" => state.parse_first_thread_info(res, self.handler), + "qfThreadInfo" => state.parse_first_thread_info(self.handler), // https://lldb.llvm.org/resources/lldbgdbremote.html#qhostinfo - "qHostInfo" => state.parse_host_info(res), + "qHostInfo" => state.parse_host_info(), // https://lldb.llvm.org/resources/lldbgdbremote.html#qlistthreadsinstopreply - "QListThreadsInStopReply" => state.parse_enable_threads_in_stop_reply(res), + "QListThreadsInStopReply" => state.parse_enable_threads_in_stop_reply(), // The VMM already relocated the kernel. - "qOffsets" => Ok(()), + "qOffsets" => Ok(Some(Vec::new())), // https://lldb.llvm.org/resources/lldbgdbremote.html#qregisterinfo-hex-reg-id - "qRegisterInfo" | reg => state.parse_register_info(reg, res), + "qRegisterInfo" | reg => state.parse_register_info(reg), // https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html#index-qsThreadInfo-packet - "qsThreadInfo" => state.parse_subsequent_thread_info(res), + "qsThreadInfo" => state.parse_subsequent_thread_info(), // TODO: What is this? - "qStructuredDataPlugins" => Ok(()), + "qStructuredDataPlugins" => Ok(Some(Vec::new())), // This does not useful to us. See // https://lldb.llvm.org/resources/lldbgdbremote.html#qprocessinfo for more details. - "qProcessInfo" => Ok(()), - "QStartNoAckMode" => state.parse_start_no_ack_mode(res), + "qProcessInfo" => Ok(Some(Vec::new())), + "QStartNoAckMode" => state.parse_start_no_ack_mode(), // It is unclear if qSupported can sent from GDB without additional payload. - "qSupported" | rest => state.parse_supported(rest, res), + "qSupported" | rest => state.parse_supported(rest), // https://lldb.llvm.org/resources/lldbgdbremote.html#qthreadsuffixsupported - "QThreadSuffixSupported" => state.parse_thread_suffix_supported(res), + "QThreadSuffixSupported" => state.parse_thread_suffix_supported(), // TODO: https://github.com/obhq/obliteration/issues/1398 - "qVAttachOrWaitSupported" => Ok(()), - "vCont?" => state.parse_vcont(res), - "x" | data => state.parse_read_memory(data, res, self.handler, true).await, + "qVAttachOrWaitSupported" => Ok(Some(Vec::new())), + "vCont?" => state.parse_vcont(), + "x" | data => state.parse_read_memory(data, self.handler, true).await, } - // Push checksum. - let mut checksum = [0u8; 2]; + // Get checksum. + let mut tail = ArrayVec::new(); + let body = match body { + Some(v) => v, + None => return Ok(Some((head, Vec::new(), tail))), + }; - hex::encode_to_slice([Self::get_checksum(&res[off..])], &mut checksum).unwrap(); + head.push(b'$'); + tail.push(b'#'); + tail.push(0); + tail.push(0); - res.push(b'#'); - res.extend_from_slice(&checksum); + hex::encode_to_slice([Self::get_checksum(&body)], &mut tail[1..]).unwrap(); - Ok(res.drain(..).into()) + Ok(Some((head, body, tail))) } fn get_checksum(data: &[u8]) -> u8 { diff --git a/gui/src/gdb/mod.rs b/gui/src/gdb/mod.rs index 75aa3ce6..794d65e1 100644 --- a/gui/src/gdb/mod.rs +++ b/gui/src/gdb/mod.rs @@ -74,7 +74,6 @@ mod state; #[derive(Default)] pub struct GdbSession { req: Vec, - res: Vec, state: SessionState, } diff --git a/gui/src/gdb/state.rs b/gui/src/gdb/state.rs index 47bcf11d..17e987a0 100644 --- a/gui/src/gdb/state.rs +++ b/gui/src/gdb/state.rs @@ -18,13 +18,10 @@ impl SessionState { pub fn parse_start_no_ack_mode( &mut self, - res: &mut Vec, - ) -> Result<(), Box> { + ) -> Result>, Box> { self.no_ack = Some(false); - res.extend_from_slice(b"OK"); - - Ok(()) + Ok(Some(b"OK".into())) } pub fn parse_ack_no_ack(&mut self) { @@ -34,15 +31,16 @@ impl SessionState { pub fn parse_supported( &mut self, req: &[u8], - res: &mut Vec, - ) -> Result<(), Box> { + ) -> Result>, Box> { // Push features that we always supported. + let mut res = Vec::with_capacity(256); + res.extend_from_slice(b"QStartNoAckMode+"); // Parse GDB features. let req = match req.strip_prefix(b":") { Some(v) => v, - None => return Ok(()), + None => return Ok(Some(res)), }; for feat in req.split(|&b| b == b';') { @@ -80,32 +78,28 @@ impl SessionState { } } - Ok(()) + Ok(Some(res)) } pub fn parse_thread_suffix_supported( &mut self, - res: &mut Vec, - ) -> Result<(), Box> { + ) -> Result>, Box> { self.thread_suffix_supported = true; - res.extend_from_slice(b"OK"); - - Ok(()) + Ok(Some(b"OK".into())) } pub fn parse_enable_threads_in_stop_reply( &mut self, - res: &mut Vec, - ) -> Result<(), Box> { + ) -> Result>, Box> { self.threads_in_stop_reply = true; - res.extend_from_slice(b"OK"); - - Ok(()) + Ok(Some(b"OK".into())) } - pub fn parse_host_info(&mut self, res: &mut Vec) -> Result<(), Box> { + pub fn parse_host_info(&mut self) -> Result>, Box> { + let mut res = Vec::with_capacity(256); + // https://en.wikipedia.org/wiki/Mach-O if cfg!(target_arch = "aarch64") { res.extend_from_slice(b"cputype:16777228;cpusubtype:0"); // 0x100000C @@ -131,94 +125,78 @@ impl SessionState { // It is unlikely for us to support page size other than 16K in a near future. res.extend_from_slice(b";vm-page-size:16384"); - Ok(()) + Ok(Some(res)) } - pub fn parse_vcont(&mut self, res: &mut Vec) -> Result<(), Box> { + pub fn parse_vcont(&mut self) -> Result>, Box> { // Only Continue and Stop is supported at the moment. - res.extend_from_slice(b"vCont;c;t"); - - Ok(()) + Ok(Some(b"vCont;c;t".into())) } - pub fn parse_current_thread( - &mut self, - res: &mut Vec, - ) -> Result<(), Box> { - write!(res, "QC{:x}", self.current_thread).unwrap(); - - Ok(()) + pub fn parse_current_thread(&mut self) -> Result>, Box> { + Ok(Some(format!("QC{:x}", self.current_thread).into())) } pub async fn parse_stop_reason( &mut self, - res: &mut Vec, h: &mut H, - ) -> Result<(), Box> { + ) -> Result>, Box> { h.suspend_threads().await?; // https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=include/gdb/signals.def - res.extend_from_slice(b"S96"); // EXC_BREAKPOINT - - Ok(()) + Ok(Some(b"S96".into())) // EXC_BREAKPOINT } pub fn parse_continue( &mut self, req: &[u8], - _: &mut Vec, h: &mut H, - ) -> Result<(), Box> { + ) -> Result>, Box> { // TODO: It is unclear if this resume current thread or all threads. let addr = if req.is_empty() { None } else { todo!() }; h.resume_thread(self.current_thread, addr)?; - todo!("reply on next stop") + Ok(None) } pub fn parse_first_thread_info( &mut self, - res: &mut Vec, h: &mut H, - ) -> Result<(), Box> { + ) -> Result>, Box> { // We don't need to do anything special because the hexadecimal already in a big-endian // format. let mut iter = h.active_threads().into_iter(); + let mut res = Vec::with_capacity(128); match iter.next() { Some(v) => write!(res, "m{v:x}").unwrap(), - None => return Ok(()), + None => return Ok(Some(res)), } for id in iter { write!(res, ",{id:x}").unwrap(); } - Ok(()) + Ok(Some(res)) } pub fn parse_subsequent_thread_info( &mut self, - res: &mut Vec, - ) -> Result<(), Box> { - // No more threads. - res.extend_from_slice(b"l"); - - Ok(()) + ) -> Result>, Box> { + Ok(Some(b"l".into())) // No more threads. } pub fn parse_register_info( &mut self, reg: &[u8], - res: &mut Vec, - ) -> Result<(), Box> { + ) -> Result>, Box> { // Parse register number. let reg = Self::parse_hex(reg) .ok_or_else(|| format!("unknown register '{}'", String::from_utf8_lossy(reg)))?; let reg = match Register::try_from(reg) { Ok(v) => v, - Err(_) => return Ok(()), // No more registers. + Err(_) => return Ok(Some(Vec::new())), // No more registers. }; // Build response. @@ -239,17 +217,14 @@ impl SessionState { write!(info, "generic:{v};").unwrap(); } - res.extend_from_slice(info.as_bytes()); - - Ok(()) + Ok(Some(info.into())) } pub async fn parse_read_register( &mut self, req: &[u8], - res: &mut Vec, h: &mut H, - ) -> Result<(), Box> { + ) -> Result>, Box> { // Get target register and thread. let (reg, td) = if self.thread_suffix_supported { // https://lldb.llvm.org/resources/lldbgdbremote.html#qthreadsuffixsupported @@ -277,9 +252,7 @@ impl SessionState { // Read register. let val = Self::read_register(h, td, reg).await?; - res.extend_from_slice(val.as_bytes()); - - Ok(()) + Ok(Some(val.into())) } #[cfg(target_arch = "aarch64")] @@ -324,10 +297,9 @@ impl SessionState { pub async fn parse_read_memory( &mut self, req: &[u8], - res: &mut Vec, h: &mut H, bin: bool, - ) -> Result<(), Box> { + ) -> Result>, Box> { let mut iter = req.splitn(2, |&b| b == b','); let addr = iter.next().unwrap(); // This should always success. let len = iter.next().ok_or_else(|| "missing length")?; @@ -340,30 +312,25 @@ impl SessionState { // Special case for zero length. let len = match NonZero::new(len) { Some(v) => v, - None => { - if bin { - res.extend_from_slice(b"b"); - } - - return Ok(()); - } + None => match bin { + true => return Ok(Some(b"b".into())), + false => return Ok(Some(Vec::new())), + }, }; // Read memory. let data = h.read_memory(self.current_thread, addr, len).await?; if bin { + let mut res = Vec::with_capacity(1 + data.len()); + res.extend_from_slice(b"b"); res.extend_from_slice(&data); + + Ok(Some(res)) } else { - let off = res.len(); - - res.resize(off + data.len(), 0); - - hex::encode_to_slice(data, &mut res[off..]).unwrap(); + Ok(Some(hex::encode(data).into())) } - - Ok(()) } fn parse_hex(v: &[u8]) -> Option { diff --git a/gui/src/main.rs b/gui/src/main.rs index 8d25ff35..0956a822 100644 --- a/gui/src/main.rs +++ b/gui/src/main.rs @@ -22,6 +22,7 @@ use futures::{ use hv::{DebugEvent, Hypervisor}; use slint::{ComponentHandle, SharedString, ToSharedString}; use std::cell::{Cell, RefMut}; +use std::io::IoSlice; use std::net::SocketAddr; use std::num::NonZero; use std::path::PathBuf; @@ -912,17 +913,33 @@ impl Context { // Dispatch the requests. let mut dis = gdb.dispatch_client(&buf[..len], self); - while let Some(res) = dis + while let Some((head, body, tail)) = dis .pump() .await .map_err(|e| ProgramError::DispatchDebugger(Box::new(e)))? { - let res = res.as_ref(); + if head.is_empty() { + continue; + } - if !res.is_empty() { - con.write_all(res) + // Send the whole response. + let mut sent = 0; + let len = head.len() + body.len() + tail.len(); + let mut iov: &mut [_] = &mut [ + IoSlice::new(&head), + IoSlice::new(&body), + IoSlice::new(&tail), + ]; + + while sent != len { + let n = con + .write_vectored(iov) .await .map_err(ProgramError::WriteDebuggerSocket)?; + + IoSlice::advance_slices(&mut iov, n); + + sent += n; } }