gdb: Update debugging to take into account the new threading model

Signed-off-by: Doru Blânzeanu <[email protected]>

Author: dblnz

src/hyperlight_host/examples/guest-debugging/main.rs

@@ -39,6 +39,7 @@ fn get_sandbox_cfg() -> Option<SandboxConfiguration> {
 }
 
 fn main() -> hyperlight_host::Result<()> {
+    env_logger::init();
     let cfg = get_sandbox_cfg();
 
     // Create an uninitialized sandbox with a guest binary

src/hyperlight_host/src/hypervisor/gdb/event_loop.rs

@@ -20,7 +20,7 @@ use gdbstub::stub::{
     run_blocking, BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason,
 };
 #[cfg(target_os = "linux")]
-use libc::{pthread_kill, SIGRTMIN};
+use libc::SIGRTMIN;
 
 use super::x86_64_target::HyperlightSandboxTarget;
 use super::{DebugResponse, GdbTargetError, VcpuStopReason};
@@ -55,14 +55,14 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
                         VcpuStopReason::EntryPointBp => BaseStopReason::HwBreak(()),
                         // This is a consequence of the GDB client sending an interrupt signal
                         // to the target thread
-                        VcpuStopReason::Interrupt => BaseStopReason::SignalWithThread {
-                            tid: (),
+                        VcpuStopReason::Interrupt => {
                             #[cfg(target_os = "linux")]
-                            signal: Signal(SIGRTMIN() as u8),
+                            let signal = Signal(SIGRTMIN() as u8);
                             #[cfg(target_os = "windows")]
-                            // TODO: Handle the signal properly
-                            signal: Signal(53u8),
-                        },
+                            let signal = Signal(53u8); // SIGINT on Windows
+
+                            BaseStopReason::SignalWithThread { tid: (), signal }
+                        }
                         VcpuStopReason::Unknown => {
                             log::warn!("Unknown stop reason received");
 
@@ -106,18 +106,9 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
         log::info!("Received interrupt from GDB client - sending signal to target thread");
 
         // Send a signal to the target thread to interrupt it
-        #[cfg(target_os = "linux")]
-        let ret = unsafe { pthread_kill(target.get_thread_id(), SIGRTMIN()) };
-
-        #[cfg(target_os = "windows")]
-        let ret = {
-            // TODO: Implement Windows signal sending
-            libc::ESRCH
-        };
-
-        log::info!("pthread_kill returned {}", ret);
+        let res = target.interrupt_vcpu();
 
-        if ret < 0 && ret != libc::ESRCH {
+        if !res {
             log::error!("Failed to send signal to target thread");
             return Err(GdbTargetError::SendSignalError);
         }

src/hyperlight_host/src/hypervisor/gdb/mod.rs

@@ -49,6 +49,8 @@ use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
 use crate::mem::layout::SandboxMemoryLayout;
 use crate::{new_error, HyperlightError};
 
+use super::InterruptHandle;
+
 #[derive(Debug, Error)]
 pub(crate) enum GdbTargetError {
     #[error("Error encountered while binding to address and port")]
@@ -149,6 +151,7 @@ pub(crate) enum DebugResponse {
     DisableDebug,
     ErrorOccurred,
     GetCodeSectionOffset(u64),
+    InterruptHandle(Arc<dyn InterruptHandle>),
     ReadAddr(Vec<u8>),
     ReadRegisters(X86_64Regs),
     RemoveHwBreakpoint(bool),
@@ -382,7 +385,6 @@ impl<T, U> DebugCommChannel<T, U> {
 /// Creates a thread that handles gdb protocol
 pub(crate) fn create_gdb_thread(
     port: u16,
-    thread_id: u64,
 ) -> Result<DebugCommChannel<DebugResponse, DebugMsg>, GdbTargetError> {
     let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
     let socket = format!("localhost:{}", port);
@@ -400,12 +402,23 @@ pub(crate) fn create_gdb_thread(
             let conn: Box<dyn ConnectionExt<Error = io::Error>> = Box::new(conn);
             let debugger = GdbStub::new(conn);
 
-            let mut target = HyperlightSandboxTarget::new(hyp_conn, thread_id);
+            let mut target = HyperlightSandboxTarget::new(hyp_conn);
+
+            // Waits for vCPU to stop at entrypoint breakpoint
+            let msg = target.recv()?;
+            if let DebugResponse::InterruptHandle(handle) = msg {
+                log::info!("Received interrupt handle: {:?}", handle);
+                target.set_interrupt_handle(handle);
+            } else {
+                return Err(GdbTargetError::UnexpectedMessage);
+            }
 
             // Waits for vCPU to stop at entrypoint breakpoint
-            let res = target.recv()?;
-            if let DebugResponse::VcpuStopped(_) = res {
+            let msg = target.recv()?;
+            if let DebugResponse::VcpuStopped(_) = msg {
                 event_loop_thread(debugger, &mut target);
+            } else {
+                return Err(GdbTargetError::UnexpectedMessage);
             }
 
             Ok(())

src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs

@@ -28,23 +28,25 @@ use gdbstub::target::ext::breakpoints::{
 use gdbstub::target::ext::section_offsets::{Offsets, SectionOffsets};
 use gdbstub::target::{Target, TargetError, TargetResult};
 use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch;
+use std::sync::Arc;
+
+use crate::hypervisor::InterruptHandle;
 
 use super::{DebugCommChannel, DebugMsg, DebugResponse, GdbTargetError, X86_64Regs};
 
 /// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation
 pub(crate) struct HyperlightSandboxTarget {
     /// Hypervisor communication channels
     hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>,
-    /// Thread ID
-    #[allow(dead_code)]
-    thread_id: u64,
+    /// Interrupt handle for the vCPU thread
+    interrupt_handle: Option<Arc<dyn InterruptHandle>>,
 }
 
 impl HyperlightSandboxTarget {
-    pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>, thread_id: u64) -> Self {
+    pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>) -> Self {
         HyperlightSandboxTarget {
             hyp_conn,
-            thread_id,
+            interrupt_handle: None,
         }
     }
 
@@ -61,10 +63,9 @@ impl HyperlightSandboxTarget {
         self.hyp_conn.send(ev)
     }
 
-    /// Returns the thread ID
-    #[allow(dead_code)]
-    pub(crate) fn get_thread_id(&self) -> u64 {
-        self.thread_id
+    /// Set the interrupt handle for the vCPU thread
+    pub(crate) fn set_interrupt_handle(&mut self, handle: Arc<dyn InterruptHandle>) {
+        self.interrupt_handle = Some(handle);
     }
 
     /// Waits for a response over the communication channel
@@ -109,6 +110,17 @@ impl HyperlightSandboxTarget {
             }
         }
     }
+
+    /// Interrupts the vCPU execution
+    pub(crate) fn interrupt_vcpu(&mut self) -> bool {
+        if let Some(handle) = &self.interrupt_handle {
+            handle.kill()
+        } else {
+            log::warn!("No interrupt handle set, cannot interrupt vCPU");
+
+            false
+        }
+    }
 }
 
 impl Target for HyperlightSandboxTarget {
@@ -418,7 +430,7 @@ mod tests {
     fn test_gdb_target() {
         let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
 
-        let mut target = HyperlightSandboxTarget::new(hyp_conn, 0);
+        let mut target = HyperlightSandboxTarget::new(hyp_conn);
 
         // Check response to read registers - send the response first to not be blocked
         // by the recv call in the target

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -388,27 +388,36 @@ impl HypervLinuxDriver {
 
         Self::setup_initial_sregs(&mut vcpu_fd, pml4_ptr.absolute()?)?;
 
-        Ok(Self {
+        let interrupt_handle = Arc::new(LinuxInterruptHandle {
+            running: AtomicU64::new(0),
+            cancel_requested: AtomicBool::new(false),
+            tid: AtomicU64::new(unsafe { libc::pthread_self() }),
+            retry_delay: config.get_interrupt_retry_delay(),
+            sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
+            dropped: AtomicBool::new(false),
+        });
+
+        #[allow(unused_mut)]
+        let mut hv = Self {
             _mshv: mshv,
             vm_fd,
             vcpu_fd,
             mem_regions,
             entrypoint: entrypoint_ptr.absolute()?,
             orig_rsp: rsp_ptr,
-            interrupt_handle: Arc::new(LinuxInterruptHandle {
-                running: AtomicU64::new(0),
-                cancel_requested: AtomicBool::new(false),
-                tid: AtomicU64::new(unsafe { libc::pthread_self() }),
-                retry_delay: config.get_interrupt_retry_delay(),
-                sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
-                dropped: AtomicBool::new(false),
-            }),
-
+            interrupt_handle: interrupt_handle.clone(),
             #[cfg(gdb)]
             debug,
             #[cfg(gdb)]
             gdb_conn,
-        })
+        };
+
+        // Send the interrupt handle to the GDB thread if debugging is enabled
+        // This is used to allow the GDB thread to stop the vCPU
+        #[cfg(gdb)]
+        hv.send_dbg_msg(DebugResponse::InterruptHandle(interrupt_handle))?;
+
+        Ok(hv)
     }
 
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

@@ -36,7 +36,7 @@ use {
     super::handlers::DbgMemAccessHandlerWrapper,
     crate::hypervisor::handlers::DbgMemAccessHandlerCaller,
     crate::{log_then_return, HyperlightError},
-    std::sync::{Arc, Mutex},
+    std::sync::Mutex,
 };
 
 use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT};
@@ -327,25 +327,36 @@ impl HypervWindowsDriver {
         // subtract 2 pages for the guard pages, since when we copy memory to and from surrogate process,
         // we don't want to copy the guard pages themselves (that would cause access violation)
         let mem_size = raw_size - 2 * PAGE_SIZE_USIZE;
-        Ok(Self {
+
+        let interrupt_handle = Arc::new(WindowsInterruptHandle {
+            running: AtomicBool::new(false),
+            cancel_requested: AtomicBool::new(false),
+            partition_handle,
+            dropped: AtomicBool::new(false),
+        });
+
+        #[allow(unused_mut)]
+        let mut hv = Self {
             size: mem_size,
             processor: proc,
             _surrogate_process: surrogate_process,
             source_address: raw_source_address,
             entrypoint,
             orig_rsp: GuestPtr::try_from(RawPtr::from(rsp))?,
             mem_regions,
-            interrupt_handle: Arc::new(WindowsInterruptHandle {
-                running: AtomicBool::new(false),
-                cancel_requested: AtomicBool::new(false),
-                partition_handle,
-                dropped: AtomicBool::new(false),
-            }),
+            interrupt_handle: interrupt_handle.clone(),
             #[cfg(gdb)]
             debug,
             #[cfg(gdb)]
             gdb_conn,
-        })
+        };
+
+        // Send the interrupt handle to the GDB thread if debugging is enabled
+        // This is used to allow the GDB thread to stop the vCPU
+        #[cfg(gdb)]
+        hv.send_dbg_msg(DebugResponse::InterruptHandle(interrupt_handle))?;
+
+        Ok(hv)
     }
 
     fn setup_initial_sregs(proc: &mut VMProcessor, pml4_addr: u64) -> Result<()> {
@@ -813,6 +824,7 @@ impl Drop for HypervWindowsDriver {
     }
 }
 
+#[derive(Debug)]
 pub struct WindowsInterruptHandle {
     // `WHvCancelRunVirtualProcessor()` will return Ok even if the vcpu is not running, which is the reason we need this flag.
     running: AtomicBool,