From 375c9d15a1cc4797af93da359581a5a3e6c59d96 Mon Sep 17 00:00:00 2001 From: djole Date: Tue, 22 Sep 2026 13:19:33 +0200 Subject: [PATCH] debugger: expose SBPF interpreter call stack Upgrade gdbstub and implement its typed qSBPFCallStack extension so debugger clients can recover the active PC, saved return PCs, and r10 from interpreter state whose return addresses live outside guest memory. Follow the WebAssembly qWasmCallStack approach: expose runtime-owned call frames through a dedicated remote protocol extension. Exercise a real nested call through the remote protocol. Cover both dynamic-stack versions with recursive calls sharing return PCs and r10, callee adjustments to r10, and restoration of caller state on return. Use the same version-specific PC base for current and saved frames, including SBPFv3 virtual addresses. Extend the SBPFv3 protocol test to check caller PCs. Adapt reads and stepping to the current gdbstub API. Consume the walnuthq/gdbstub feature branch until the extension is released upstream. --- Cargo.lock | 17 +++-- Cargo.toml | 2 +- cli/Cargo.lock | 23 ++++--- src/debugger.rs | 63 +++++++++++++++---- src/interpreter.rs | 10 ++- tests/vm.rs | 152 +++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 228 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88ed609f7..810e7e646 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -52,9 +52,9 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "bitflags" -version = "1.3.2" +version = "2.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" [[package]] name = "bitvec" @@ -265,16 +265,15 @@ checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" [[package]] name = "gdbstub" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32c95766e0414f8bfc1d07055574c621b67739466d6ba516c4fef8e99d30d2e6" +version = "0.7.10" +source = "git+https://github.com/walnuthq/gdbstub?branch=feat%2Fsbpf-call-stack#23f9924327cb85a72d288df643cc5b02c5e955b7" dependencies = [ "bitflags", "cfg-if", "log", "managed", "num-traits", - "paste", + "pastey", ] [[package]] @@ -397,10 +396,10 @@ dependencies = [ ] [[package]] -name = "paste" -version = "1.0.9" +name = "pastey" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1de2e551fb905ac83f73f7aedf2f0cb4a0da7e35efa24a202a936269f1f18e1" +checksum = "2ee67f1008b1ba2321834326597b8e186293b049a023cdef258527550b9935b4" [[package]] name = "pin-project-lite" diff --git a/Cargo.toml b/Cargo.toml index 9feca196e..53ef3fc68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ bench = false arbitrary = { version = "1.0", optional = true, features = ["derive"] } byteorder = "1.2" combine = "3.8.1" -gdbstub = { version = "0.6.2", optional = true } +gdbstub = { version = "0.7.10", git = "https://github.com/walnuthq/gdbstub", branch = "feat/sbpf-call-stack", optional = true } hash32 = "0.3.1" log = "0.4.2" rand = { version = "0.8.5", features = ["small_rng"], optional = true } diff --git a/cli/Cargo.lock b/cli/Cargo.lock index 5e8b74ca1..712bec3ba 100644 --- a/cli/Cargo.lock +++ b/cli/Cargo.lock @@ -31,6 +31,12 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "bitflags" +version = "2.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" + [[package]] name = "byteorder" version = "1.3.4" @@ -56,7 +62,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4bd1061998a501ee7d4b6d449020df3266ca3124b941ec56cf2005c3779ca142" dependencies = [ "atty", - "bitflags", + "bitflags 1.3.2", "clap_derive", "indexmap", "lazy_static", @@ -102,16 +108,15 @@ checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" [[package]] name = "gdbstub" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32c95766e0414f8bfc1d07055574c621b67739466d6ba516c4fef8e99d30d2e6" +version = "0.7.10" +source = "git+https://github.com/walnuthq/gdbstub?branch=feat%2Fsbpf-call-stack#23f9924327cb85a72d288df643cc5b02c5e955b7" dependencies = [ - "bitflags", + "bitflags 2.13.2", "cfg-if 1.0.0", "log", "managed", "num-traits", - "paste", + "pastey", ] [[package]] @@ -217,10 +222,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "afb2e1c3ee07430c2cf76151675e583e0f19985fa6efae47d6848a3e2c824f85" [[package]] -name = "paste" -version = "1.0.9" +name = "pastey" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1de2e551fb905ac83f73f7aedf2f0cb4a0da7e35efa24a202a936269f1f18e1" +checksum = "2ee67f1008b1ba2321834326597b8e186293b049a023cdef258527550b9935b4" [[package]] name = "ppv-lite86" diff --git a/src/debugger.rs b/src/debugger.rs index e1403cc54..ca6a6bb1b 100644 --- a/src/debugger.rs +++ b/src/debugger.rs @@ -2,14 +2,17 @@ use std::net::{TcpListener, TcpStream}; -use gdbstub::common::Signal; +use gdbstub::common::{Signal, Tid}; use gdbstub::conn::ConnectionExt; use gdbstub::stub::{state_machine, GdbStub, SingleThreadStopReason}; use gdbstub::arch::lldb::{Encoding, Format, Generic, Register}; use gdbstub::arch::RegId; -use gdbstub::target::{ext::monitor_cmd::MonitorCmd, Target, TargetError, TargetResult}; +use gdbstub::target::{ + ext::{monitor_cmd::MonitorCmd, sbpf::Sbpf}, + Target, TargetError, TargetResult, +}; use gdbstub::{outputln, target}; use core::convert::{TryFrom, TryInto}; @@ -166,6 +169,11 @@ impl<'a, 'b, 'c, C: ContextObject> Target for Interpreter<'a, 'b, 'c, C> { fn support_monitor_cmd(&mut self) -> Option> { Some(self) } + + #[inline(always)] + fn support_sbpf(&mut self) -> Option> { + Some(self) + } } fn get_host_ptr( @@ -232,16 +240,19 @@ impl<'a, 'b, 'c, C: ContextObject> SingleThreadBase for Interpreter<'a, 'b, 'c, Some(self) } - fn read_addrs(&mut self, start_addr: u64, data: &mut [u8]) -> TargetResult<(), Self> { + fn read_addrs(&mut self, start_addr: u64, data: &mut [u8]) -> TargetResult { + let mut read: usize = 0; for (vm_addr, val) in (start_addr..).zip(data.iter_mut()) { let host_ptr = match get_host_ptr(self, vm_addr) { Ok(host_ptr) => host_ptr, - // The debugger is sometimes requesting more data than we have access to, just skip these - _ => continue, + // Report a partial read when the request crosses the end of a + // mapped guest-memory region. + _ => break, }; *val = unsafe { *host_ptr }; + read = read.saturating_add(1); } - Ok(()) + Ok(read) } fn write_addrs(&mut self, start_addr: u64, data: &[u8]) -> TargetResult<(), Self> { @@ -444,7 +455,7 @@ impl<'a, 'b, 'c, C: ContextObject> } mod bpf_arch { - use gdbstub::arch::{Arch, SingleStepGdbBehavior}; + use gdbstub::arch::Arch; /// BPF-specific breakpoint kinds. /// @@ -468,17 +479,11 @@ mod bpf_arch { /// Implements `Arch` for BPF. pub enum Bpf {} - #[allow(deprecated)] impl Arch for Bpf { type Usize = u64; type Registers = reg::BpfRegs; type RegId = reg::id::BpfRegId; type BreakpointKind = BpfBreakpointKind; - - #[inline(always)] - fn single_step_gdb_behavior() -> SingleStepGdbBehavior { - SingleStepGdbBehavior::Required - } } pub mod reg { @@ -706,3 +711,35 @@ impl<'a, 'b, 'c, C: ContextObject> MonitorCmd for Interpreter<'a, 'b, 'c, C> { Ok(()) } } + +impl<'a, 'b, 'c, C: ContextObject> Sbpf for Interpreter<'a, 'b, 'c, C> { + fn sbpf_call_stack( + &self, + _tid: Tid, + next_frame: &mut dyn FnMut(u64, u64), + ) -> Result<(), Self::Error> { + // Return addresses live in interpreter-owned host memory and cannot be + // recovered through ordinary guest-memory reads. + next_frame(self.get_dbg_pc(), self.reg[ebpf::FRAME_PTR_REG]); + + let call_depth: usize = self + .vm + .call_depth + .try_into() + .map_err(|_| "invalid SBPF call depth")?; + let call_frames = self + .call_frames + .get(..call_depth) + .ok_or("invalid SBPF call depth")?; + let pc_base = self.get_dbg_pc_base(); + for frame in call_frames.iter().rev() { + let return_pc = frame + .target_pc + .checked_mul(ebpf::INSN_SIZE as u64) + .and_then(|pc| pc.checked_add(pc_base)) + .ok_or("invalid SBPF return PC")?; + next_frame(return_pc, frame.frame_pointer); + } + Ok(()) + } +} diff --git a/src/interpreter.rs b/src/interpreter.rs index c4a0fbb84..e4ce3511a 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -132,11 +132,16 @@ impl<'a, 'b, 'c, C: ContextObject> Interpreter<'a, 'b, 'c, C> { /// Translate between the virtual machines' pc value and the pc value used by the debugger #[cfg(feature = "debugger")] pub fn get_dbg_pc(&self) -> u64 { + self.get_dbg_pc_base() + self.reg[11] * ebpf::INSN_SIZE as u64 + } + + #[cfg(feature = "debugger")] + pub(crate) fn get_dbg_pc_base(&self) -> u64 { // V3+ ELFs place .text at its real vaddr (e.g. 0x1_0000_0000) in the on-disk // sh_addr/p_vaddr, which is also what LLDB uses when it loads the .so.debug. // Pre-V3 ELFs have .text at sh_addr=0 in the file (the runtime adds MM_REGION_SIZE // itself), so reporting a file-offset-style PC happens to line up with LLDB's view. - let base = if self + if self .executable .get_sbpf_version() .enable_lower_rodata_vaddr() @@ -144,8 +149,7 @@ impl<'a, 'b, 'c, C: ContextObject> Interpreter<'a, 'b, 'c, C> { self.executable.get_text_section_vaddr() } else { self.executable.get_text_section_offset() - }; - base + self.reg[11] * ebpf::INSN_SIZE as u64 + } } fn push_frame(&mut self, config: &Config) -> bool { diff --git a/tests/vm.rs b/tests/vm.rs index 2f6ef3a73..2c2171a4e 100644 --- a/tests/vm.rs +++ b/tests/vm.rs @@ -51,6 +51,42 @@ fn test_gdbstub_architecture() { Ok(reply) } + fn send_packet(writer: &mut impl Write, payload: &str) -> std::io::Result<()> { + let checksum = payload + .bytes() + .fold(0u8, |checksum, byte| checksum.wrapping_add(byte)); + write!(writer, "${payload}#{checksum:02x}") + } + + fn decode_hex_packet(reply: &str) -> Vec { + let packet = reply.strip_prefix('+').unwrap_or(reply); + let encoded = packet + .strip_prefix('$') + .and_then(|packet| packet.split_once('#')) + .map(|(payload, _)| payload) + .expect("GDB response packet"); + let encoded = encoded.strip_prefix('O').unwrap_or(encoded); + + let mut expanded = Vec::new(); + let mut bytes = encoded.bytes(); + while let Some(byte) = bytes.next() { + if byte == b'*' { + let repeat = bytes.next().expect("RLE repeat count"); + let previous = *expanded.last().expect("RLE previous byte"); + expanded.extend(std::iter::repeat_n(previous, (repeat - 29) as usize)); + } else { + expanded.push(byte); + } + } + + expanded + .as_chunks::<2>() + .0 + .iter() + .map(|digits| u8::from_str_radix(std::str::from_utf8(digits).unwrap(), 16).unwrap()) + .collect() + } + // Should there are conflicts with the default debug port for this test // provide an option to the user to actually alter it. let debug_port = std::env::var("GDBSTUB_TEST_DEBUG_PORT") @@ -121,19 +157,44 @@ fn test_gdbstub_architecture() { // Check the icount_remain pseudo register is 10_000_000_000 (0x2540BE400). writer.write_all(b"$pc#d3")?; let reply = read_reply(&mut reader)?; - assert_eq!("+$00e40b540200*!#01", reply); + assert_eq!(decode_hex_packet(&reply), 10_000_000_000u64.to_le_bytes()); // Check the monitor command returns the expected metadata. writer.write_all(b"$qRcmd,6d65746164617461#9d")?; let reply = read_reply(&mut reader)?; assert_eq!( - "+$O3643536d6956694d614167754b67784e5677553854574d5056695162744c3\ -54b4b6f4672447757* 4594e520a#9e", - reply + decode_hex_packet(&reply), + format!("{METADATA}\n").as_bytes() ); let reply = read_reply(&mut reader)?; assert_eq!("$OK#9a", reply); + // Stop in the function called by entrypoint so the runtime + // contains an actual saved caller frame. + send_packet(&mut writer, "P1=0000000002000000")?; + let reply = read_reply(&mut reader)?; + assert!(reply.contains("$OK#")); + send_packet(&mut writer, "Z0,120,0")?; + let reply = read_reply(&mut reader)?; + assert!(reply.contains("$OK#")); + send_packet(&mut writer, "c")?; + let reply = read_reply(&mut reader)?; + assert!(reply.contains("$T05")); + + send_packet(&mut writer, "qSBPFCallStack:1")?; + let reply = read_reply(&mut reader)?; + let response = decode_hex_packet(&reply); + assert_eq!(response.len(), 2 * 2 * std::mem::size_of::()); + let values: Vec<_> = response + .as_chunks::<{ std::mem::size_of::() }>() + .0 + .iter() + .map(|value| u64::from_le_bytes(*value)) + .collect(); + assert_eq!(values[0], 0x120); + assert_eq!(values[2], 0x168); + assert!(values[1] > values[3]); + // Gracefully shutdown the remote gdbstub. writer.write_all(b"$D#44")?; let reply = read_reply(&mut reader)?; @@ -184,6 +245,9 @@ fn test_gdbstub_sbpfv3_pc_and_text() { let mut context_object = TestContextObject::default(); let mut call_frames = vec![CallFrame::default(); Config::default().max_call_depth]; + // Seed a caller to verify that the packet uses virtual addresses for both + // current and historical PCs, not the legacy ELF file-offset convention. + call_frames[0].target_pc = executable.get_entrypoint_instruction_offset() as u64 + 1; create_vm!( vm, &executable, @@ -194,7 +258,9 @@ fn test_gdbstub_sbpfv3_pc_and_text() { None ); let mut registers = vm.registers; + vm.call_depth = 1; registers[11] = executable.get_entrypoint_instruction_offset() as u64; + let expected_fp = registers[10]; let mut interpreter = Interpreter::new(&mut vm, &executable, registers, &mut call_frames); // Drive the real protocol parser and target handlers synchronously. Each @@ -256,6 +322,15 @@ fn test_gdbstub_sbpfv3_pc_and_text() { expected_pc ); + let frames = request("qSBPFCallStack:1"); + let expected_frames: String = [expected_pc, expected_fp, expected_pc + 8, 0] + .iter() + .copied() + .flat_map(u64::to_le_bytes) + .map(|byte| format!("{byte:02x}")) + .collect(); + assert_eq!(frames, expected_frames); + let instruction = request(&format!("m{expected_pc:x},8")); let expected_hex: String = expected_instruction .iter() @@ -275,3 +350,72 @@ fn test_gdbstub_sbpfv3_pc_and_text() { assert_eq!(request("D"), "OK"); assert!(matches!(state, Some(GdbStubStateMachine::Disconnected(_)))); } + +#[cfg(feature = "debugger")] +#[test] +fn test_debugger_dynamic_recursive_frames() { + use gdbstub::{common::Tid, target::ext::sbpf::Sbpf}; + use solana_sbpf::{ + assembler::assemble, ebpf, interpreter::Interpreter, program::SBPFVersion, vm::CallFrame, + }; + use std::sync::Arc; + use test_utils::{create_vm, TestContextObject}; + + for version in [SBPFVersion::V1, SBPFVersion::V2] { + let config = Config { + enabled_sbpf_versions: version..=version, + enable_instruction_meter: false, + ..Config::default() + }; + let initial_fp = ebpf::MM_STACK_START + config.stack_size() as u64; + let executable = assemble::( + "mov64 r0, 2 + call function_recurse + exit + function_recurse: + jeq r0, 0, done + add64 r0, -1 + call function_recurse + exit + done: + add64 r10, -64 + exit", + Arc::new(BuiltinProgram::new_loader(config)), + ) + .unwrap(); + let mut context = TestContextObject::default(); + create_vm!(vm, &executable, &mut context, stack, heap, Vec::new(), None); + let mut registers = [0; 12]; + registers[10] = initial_fp; + registers[11] = executable.get_entrypoint_instruction_offset() as u64; + let mut frames = vec![CallFrame::default(); executable.get_config().max_call_depth]; + let mut interpreter = Interpreter::new(&mut vm, &executable, registers, &mut frames); + let snapshot = |interpreter: &Interpreter| { + let mut result = Vec::new(); + interpreter + .sbpf_call_stack(Tid::new(1).unwrap(), &mut |pc, fp| result.push((pc, fp))) + .unwrap(); + result + }; + // Reach the deepest recursive invocation, before it adjusts r10. + for _ in 0..20 { + if interpreter.reg[11] == 7 { + break; + } + assert!(interpreter.step()); + } + assert_eq!(interpreter.reg[11], 7); + let before = snapshot(&interpreter); + assert_eq!(before.len(), 4); + assert!(before.iter().all(|(_, fp)| *fp == initial_fp)); + // Recursive callers can share both their return PC and r10. + assert_eq!(before[1], before[2]); + assert!(interpreter.step()); + let adjusted = snapshot(&interpreter); + assert_eq!(adjusted[0].1, initial_fp - 64); + assert_eq!(&adjusted[1..], &before[1..]); + // Returning restores the caller's register state and removes one frame. + assert!(interpreter.step()); + assert_eq!(snapshot(&interpreter), adjusted[1..]); + } +}