From 12c65cd4f6ce65cca5bd95280405d82157b022eb Mon Sep 17 00:00:00 2001 From: SuchAFuriousDeath <48620541+SuchAFuriousDeath@users.noreply.github.com> Date: Wed, 20 Nov 2024 20:45:24 +0100 Subject: [PATCH] Refactors debugging-related code (#1123) --- gui/src/vmm/debug/mod.rs | 143 ++++++++++++++++++++++----------------- gui/src/vmm/ffi.rs | 11 ++- gui/src/vmm/mod.rs | 44 ++++++++---- 3 files changed, 121 insertions(+), 77 deletions(-) diff --git a/gui/src/vmm/debug/mod.rs b/gui/src/vmm/debug/mod.rs index 1c1fee710..c059088de 100644 --- a/gui/src/vmm/debug/mod.rs +++ b/gui/src/vmm/debug/mod.rs @@ -1,79 +1,96 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use super::cpu::CpuManager; +use super::cpu::{CpuManager, GdbError}; use crate::debug::DebugClient; -use crate::error::RustError; use crate::hv::Hypervisor; use crate::screen::Screen; use gdbstub::stub::state_machine::state::{Idle, Running}; use gdbstub::stub::state_machine::{GdbStubStateMachine, GdbStubStateMachineInner}; use gdbstub::stub::MultiThreadStopReason; +use thiserror::Error; -pub fn dispatch_idle( - target: &mut CpuManager, - mut state: GdbStubStateMachineInner< - 'static, - Idle>, - CpuManager, - DebugClient, - >, -) -> Result< - Result< - GdbStubStateMachine<'static, CpuManager, DebugClient>, - GdbStubStateMachineInner<'static, Idle>, CpuManager, DebugClient>, - >, - RustError, -> { - // Check for pending command. - let b = match state.borrow_conn().read() { - Ok(v) => v, - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => return Ok(Err(state)), - Err(e) => { - return Err(RustError::with_source( - "couldn't read data from the debugger", - e, - )); +impl CpuManager { + pub(super) fn dispatch_gdb_idle( + &mut self, + mut state: GdbStubStateMachineInner< + 'static, + Idle>, + CpuManager, + DebugClient, + >, + ) -> Result< + Result< + GdbStubStateMachine<'static, CpuManager, DebugClient>, + GdbStubStateMachineInner< + 'static, + Idle>, + CpuManager, + DebugClient, + >, + >, + DispatchGdbIdleError, + > { + let b = match state.borrow_conn().read() { + Ok(v) => v, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => return Ok(Err(state)), + Err(e) => return Err(DispatchGdbIdleError::ReadData(e)), + }; + + state + .incoming_data(self, b) + .map(Ok) + .map_err(DispatchGdbIdleError::ProcessData) + } + + pub(super) fn dispatch_gdb_running( + &mut self, + mut state: GdbStubStateMachineInner<'static, Running, CpuManager, DebugClient>, + stop: Option>, + ) -> Result< + Result< + GdbStubStateMachine<'static, CpuManager, DebugClient>, + GdbStubStateMachineInner<'static, Running, CpuManager, DebugClient>, + >, + DispatchGdbRunningError, + > { + // Check If we are here because of a breakpoint. + if let Some(r) = stop { + return state + .report_stop(self, r) + .map(Ok) + .map_err(DispatchGdbRunningError::ReportStopReason); } - }; - state - .incoming_data(target, b) - .map(Ok) - .map_err(|e| RustError::with_source("couldn't process data from the debugger", e)) -} + // Check for pending command. + let b = match state.borrow_conn().read() { + Ok(v) => v, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => return Ok(Err(state)), + Err(e) => return Err(DispatchGdbRunningError::ReadData(e)), + }; -pub fn dispatch_running( - target: &mut CpuManager, - mut state: GdbStubStateMachineInner<'static, Running, CpuManager, DebugClient>, - stop: Option>, -) -> Result< - Result< - GdbStubStateMachine<'static, CpuManager, DebugClient>, - GdbStubStateMachineInner<'static, Running, CpuManager, DebugClient>, - >, - RustError, -> { - // Check If we are here because of a breakpoint. - if let Some(r) = stop { - return state - .report_stop(target, r) + state + .incoming_data(self, b) .map(Ok) - .map_err(|e| RustError::with_source("couldn't report stop reason to the debugger", e)); + .map_err(DispatchGdbRunningError::ProcessData) } +} - // Check for pending command. - let b = match state.borrow_conn().read() { - Ok(v) => v, - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => return Ok(Err(state)), - Err(e) => { - return Err(RustError::with_source( - "couldn't read data from the debugger", - e, - )); - } - }; +#[derive(Debug, Error)] +pub(super) enum DispatchGdbIdleError { + #[error("couldn't read data from the debugger")] + ReadData(#[source] std::io::Error), + + #[error("couldn't process data from the debugger")] + ProcessData(#[source] gdbstub::stub::GdbStubError), +} + +#[derive(Debug, Error)] +pub(super) enum DispatchGdbRunningError { + #[error("couldn't report stop reason to the debugger")] + ReportStopReason(#[source] gdbstub::stub::GdbStubError), + + #[error("couldn't read data from the debugger")] + ReadData(#[source] std::io::Error), - state - .incoming_data(target, b) - .map(Ok) - .map_err(|e| RustError::with_source("couldn't process data from the debugger", e)) + #[error("couldn't process data from the debugger")] + ProcessData(#[source] gdbstub::stub::GdbStubError), } diff --git a/gui/src/vmm/ffi.rs b/gui/src/vmm/ffi.rs index 2ac94573c..01c70107e 100644 --- a/gui/src/vmm/ffi.rs +++ b/gui/src/vmm/ffi.rs @@ -1,4 +1,4 @@ -use super::{DebugResult, KernelStop, Vmm, VmmEvent, VmmScreen}; +use super::{DebugResult, DispatchDebugResult, KernelStop, Vmm, VmmEvent, VmmScreen}; use crate::debug::DebugClient; use crate::error::RustError; use crate::profile::Profile; @@ -82,7 +82,14 @@ pub unsafe extern "C" fn vmm_dispatch_debug(vmm: *mut Vmm, stop: *mut KernelStop Some(Box::from_raw(stop).0) }; - vmm.dispatch_debug(stop) + match vmm.dispatch_debug(stop) { + Ok(DispatchDebugResult::Ok) => DebugResult::Ok, + Ok(DispatchDebugResult::Disconnected) => DebugResult::Disconnected, + Err(e) => { + let e = RustError::wrap(e).into_c(); + DebugResult::Error { reason: e } + } + } } #[no_mangle] diff --git a/gui/src/vmm/mod.rs b/gui/src/vmm/mod.rs index bb11911a3..6b73d44ce 100644 --- a/gui/src/vmm/mod.rs +++ b/gui/src/vmm/mod.rs @@ -347,30 +347,33 @@ impl Vmm { Ok(vmm) } - fn dispatch_debug(&mut self, mut stop: Option>) -> DebugResult { + fn dispatch_debug( + &mut self, + mut stop: Option>, + ) -> Result { loop { // Check current state. let r = match self.gdb.take().unwrap() { GdbStubStateMachine::Idle(s) => { - match debug::dispatch_idle(&mut self.cpu, s) { + match self.cpu.dispatch_gdb_idle(s) { Ok(Ok(v)) => Ok(v), Ok(Err(v)) => { // No pending data from the debugger. self.gdb = Some(v.into()); - return DebugResult::Ok; + return Ok(DispatchDebugResult::Ok); } - Err(e) => Err(e), + Err(e) => Err(DispatchDebugError::DispatchIdle(e)), } } GdbStubStateMachine::Running(s) => { - match debug::dispatch_running(&mut self.cpu, s, stop.take()) { + match self.cpu.dispatch_gdb_running(s, stop.take()) { Ok(Ok(v)) => Ok(v), Ok(Err(v)) => { // No pending data from the debugger. self.gdb = Some(v.into()); - return DebugResult::Ok; + return Ok(DispatchDebugResult::Ok); } - Err(e) => Err(e), + Err(e) => Err(DispatchDebugError::DispatchRunning(e)), } } GdbStubStateMachine::CtrlCInterrupt(s) => { @@ -380,16 +383,16 @@ impl Vmm { &mut self.cpu, Some(MultiThreadStopReason::Signal(Signal::SIGINT)), ) - .map_err(|e| { - RustError::with_source("couldn't handle CTRL+C from a debugger", e) - }) + .map_err(DispatchDebugError::HandleInterrupt) + } + GdbStubStateMachine::Disconnected(_) => { + return Ok(DispatchDebugResult::Disconnected) } - GdbStubStateMachine::Disconnected(_) => return DebugResult::Disconnected, }; match r { Ok(v) => self.gdb = Some(v), - Err(e) => return DebugResult::Error { reason: e.into_c() }, + Err(e) => return Err(e), } } } @@ -403,6 +406,23 @@ impl Drop for Vmm { } } +pub enum DispatchDebugResult { + Ok, + Disconnected, +} + +#[derive(Debug, Error)] +enum DispatchDebugError { + #[error("couldn't dispatch idle state")] + DispatchIdle(#[source] debug::DispatchGdbIdleError), + + #[error("couldn't dispatch running state")] + DispatchRunning(#[source] debug::DispatchGdbRunningError), + + #[error("couldn't handle CTRL+C interrupt")] + HandleInterrupt(#[source] gdbstub::stub::GdbStubError), +} + /// Contains objects required to render the screen. #[repr(C)] pub struct VmmScreen {