Skip to content

Commit c5caf69

Browse files
committed
gdb: Update debugging to take into account the new threading model
Signed-off-by: Doru Blânzeanu <[email protected]>
1 parent ac81c5a commit c5caf69

File tree

11 files changed

+126
-1181
lines changed

11 files changed

+126
-1181
lines changed

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

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use gdbstub::stub::{
2020
BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason, run_blocking,
2121
};
2222
#[cfg(target_os = "linux")]
23-
use libc::{SIGRTMIN, pthread_kill};
23+
use libc::SIGRTMIN;
2424

2525
use super::x86_64_target::HyperlightSandboxTarget;
2626
use super::{DebugResponse, GdbTargetError, VcpuStopReason};
@@ -55,14 +55,14 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
5555
VcpuStopReason::EntryPointBp => BaseStopReason::HwBreak(()),
5656
// This is a consequence of the GDB client sending an interrupt signal
5757
// to the target thread
58-
VcpuStopReason::Interrupt => BaseStopReason::SignalWithThread {
59-
tid: (),
58+
VcpuStopReason::Interrupt => {
6059
#[cfg(target_os = "linux")]
61-
signal: Signal(SIGRTMIN() as u8),
60+
let signal = Signal(SIGRTMIN() as u8);
6261
#[cfg(target_os = "windows")]
63-
// TODO: Handle the signal properly
64-
signal: Signal(53u8),
65-
},
62+
let signal = Signal(53u8); // SIGINT on Windows
63+
64+
BaseStopReason::SignalWithThread { tid: (), signal }
65+
}
6666
VcpuStopReason::Unknown => {
6767
log::warn!("Unknown stop reason received");
6868

@@ -106,18 +106,9 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
106106
log::info!("Received interrupt from GDB client - sending signal to target thread");
107107

108108
// Send a signal to the target thread to interrupt it
109-
#[cfg(target_os = "linux")]
110-
let ret = unsafe { pthread_kill(target.get_thread_id(), SIGRTMIN()) };
111-
112-
#[cfg(target_os = "windows")]
113-
let ret = {
114-
// TODO: Implement Windows signal sending
115-
libc::ESRCH
116-
};
117-
118-
log::info!("pthread_kill returned {}", ret);
109+
let res = target.interrupt_vcpu();
119110

120-
if ret < 0 && ret != libc::ESRCH {
111+
if !res {
121112
log::error!("Failed to send signal to target thread");
122113
return Err(GdbTargetError::SendSignalError);
123114
}

src/hyperlight_host/src/hypervisor/gdb/hyperv_debug.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ use std::collections::HashMap;
1818

1919
use windows::Win32::System::Hypervisor::WHV_VP_EXCEPTION_CONTEXT;
2020

21-
use super::arch::{vcpu_stop_reason, MAX_NO_OF_HW_BP};
22-
use super::{GuestDebug, VcpuStopReason, X86_64Regs, SW_BP_SIZE};
21+
use super::arch::{MAX_NO_OF_HW_BP, vcpu_stop_reason};
22+
use super::{GuestDebug, SW_BP_SIZE, VcpuStopReason, X86_64Regs};
2323
use crate::hypervisor::windows_hypervisor_platform::VMProcessor;
2424
use crate::hypervisor::wrappers::{WHvDebugRegisters, WHvGeneralRegisters};
25-
use crate::{new_error, HyperlightError, Result};
25+
use crate::{HyperlightError, Result, new_error};
2626

2727
/// KVM Debug struct
2828
/// This struct is used to abstract the internal details of the kvm

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ pub(crate) use mshv_debug::MshvDebug;
4545
use thiserror::Error;
4646
use x86_64_target::HyperlightSandboxTarget;
4747

48+
use super::InterruptHandle;
4849
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
4950
use crate::mem::layout::SandboxMemoryLayout;
5051
use crate::{HyperlightError, new_error};
@@ -149,6 +150,7 @@ pub(crate) enum DebugResponse {
149150
DisableDebug,
150151
ErrorOccurred,
151152
GetCodeSectionOffset(u64),
153+
InterruptHandle(Arc<dyn InterruptHandle>),
152154
ReadAddr(Vec<u8>),
153155
ReadRegisters(X86_64Regs),
154156
RemoveHwBreakpoint(bool),
@@ -382,7 +384,6 @@ impl<T, U> DebugCommChannel<T, U> {
382384
/// Creates a thread that handles gdb protocol
383385
pub(crate) fn create_gdb_thread(
384386
port: u16,
385-
thread_id: u64,
386387
) -> Result<DebugCommChannel<DebugResponse, DebugMsg>, GdbTargetError> {
387388
let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
388389
let socket = format!("localhost:{}", port);
@@ -400,12 +401,23 @@ pub(crate) fn create_gdb_thread(
400401
let conn: Box<dyn ConnectionExt<Error = io::Error>> = Box::new(conn);
401402
let debugger = GdbStub::new(conn);
402403

403-
let mut target = HyperlightSandboxTarget::new(hyp_conn, thread_id);
404+
let mut target = HyperlightSandboxTarget::new(hyp_conn);
404405

405406
// Waits for vCPU to stop at entrypoint breakpoint
406-
let res = target.recv()?;
407-
if let DebugResponse::VcpuStopped(_) = res {
407+
let msg = target.recv()?;
408+
if let DebugResponse::InterruptHandle(handle) = msg {
409+
log::info!("Received interrupt handle: {:?}", handle);
410+
target.set_interrupt_handle(handle);
411+
} else {
412+
return Err(GdbTargetError::UnexpectedMessage);
413+
}
414+
415+
// Waits for vCPU to stop at entrypoint breakpoint
416+
let msg = target.recv()?;
417+
if let DebugResponse::VcpuStopped(_) = msg {
408418
event_loop_thread(debugger, &mut target);
419+
} else {
420+
return Err(GdbTargetError::UnexpectedMessage);
409421
}
410422

411423
Ok(())

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17+
use std::sync::Arc;
18+
1719
use crossbeam_channel::TryRecvError;
1820
use gdbstub::arch::Arch;
1921
use gdbstub::common::Signal;
@@ -30,21 +32,21 @@ use gdbstub::target::{Target, TargetError, TargetResult};
3032
use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch;
3133

3234
use super::{DebugCommChannel, DebugMsg, DebugResponse, GdbTargetError, X86_64Regs};
35+
use crate::hypervisor::InterruptHandle;
3336

3437
/// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation
3538
pub(crate) struct HyperlightSandboxTarget {
3639
/// Hypervisor communication channels
3740
hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>,
38-
/// Thread ID
39-
#[allow(dead_code)]
40-
thread_id: u64,
41+
/// Interrupt handle for the vCPU thread
42+
interrupt_handle: Option<Arc<dyn InterruptHandle>>,
4143
}
4244

4345
impl HyperlightSandboxTarget {
44-
pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>, thread_id: u64) -> Self {
46+
pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>) -> Self {
4547
HyperlightSandboxTarget {
4648
hyp_conn,
47-
thread_id,
49+
interrupt_handle: None,
4850
}
4951
}
5052

@@ -61,10 +63,9 @@ impl HyperlightSandboxTarget {
6163
self.hyp_conn.send(ev)
6264
}
6365

64-
/// Returns the thread ID
65-
#[allow(dead_code)]
66-
pub(crate) fn get_thread_id(&self) -> u64 {
67-
self.thread_id
66+
/// Set the interrupt handle for the vCPU thread
67+
pub(crate) fn set_interrupt_handle(&mut self, handle: Arc<dyn InterruptHandle>) {
68+
self.interrupt_handle = Some(handle);
6869
}
6970

7071
/// Waits for a response over the communication channel
@@ -109,6 +110,17 @@ impl HyperlightSandboxTarget {
109110
}
110111
}
111112
}
113+
114+
/// Interrupts the vCPU execution
115+
pub(crate) fn interrupt_vcpu(&mut self) -> bool {
116+
if let Some(handle) = &self.interrupt_handle {
117+
handle.kill()
118+
} else {
119+
log::warn!("No interrupt handle set, cannot interrupt vCPU");
120+
121+
false
122+
}
123+
}
112124
}
113125

114126
impl Target for HyperlightSandboxTarget {
@@ -418,7 +430,7 @@ mod tests {
418430
fn test_gdb_target() {
419431
let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
420432

421-
let mut target = HyperlightSandboxTarget::new(hyp_conn, 0);
433+
let mut target = HyperlightSandboxTarget::new(hyp_conn);
422434

423435
// Check response to read registers - send the response first to not be blocked
424436
// by the recv call in the target

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -395,29 +395,38 @@ impl HypervLinuxDriver {
395395

396396
Self::setup_initial_sregs(&mut vcpu_fd, pml4_ptr.absolute()?)?;
397397

398-
Ok(Self {
398+
let interrupt_handle = Arc::new(LinuxInterruptHandle {
399+
running: AtomicU64::new(0),
400+
cancel_requested: AtomicBool::new(false),
401+
tid: AtomicU64::new(unsafe { libc::pthread_self() }),
402+
retry_delay: config.get_interrupt_retry_delay(),
403+
sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
404+
dropped: AtomicBool::new(false),
405+
});
406+
407+
#[allow(unused_mut)]
408+
let mut hv = Self {
399409
_mshv: mshv,
400410
vm_fd,
401411
vcpu_fd,
402412
mem_regions,
403413
entrypoint: entrypoint_ptr.absolute()?,
404414
orig_rsp: rsp_ptr,
405-
interrupt_handle: Arc::new(LinuxInterruptHandle {
406-
running: AtomicU64::new(0),
407-
cancel_requested: AtomicBool::new(false),
408-
tid: AtomicU64::new(unsafe { libc::pthread_self() }),
409-
retry_delay: config.get_interrupt_retry_delay(),
410-
sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
411-
dropped: AtomicBool::new(false),
412-
}),
413-
415+
interrupt_handle: interrupt_handle.clone(),
414416
#[cfg(gdb)]
415417
debug,
416418
#[cfg(gdb)]
417419
gdb_conn,
418420
#[cfg(crashdump)]
419421
rt_cfg,
420-
})
422+
};
423+
424+
// Send the interrupt handle to the GDB thread if debugging is enabled
425+
// This is used to allow the GDB thread to stop the vCPU
426+
#[cfg(gdb)]
427+
hv.send_dbg_msg(DebugResponse::InterruptHandle(interrupt_handle))?;
428+
429+
Ok(hv)
421430
}
422431

423432
#[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,8 @@ impl HypervWindowsDriver {
300300
entrypoint: u64,
301301
rsp: u64,
302302
mmap_file_handle: HandleWrapper,
303-
#[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
304303
#[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
304+
#[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
305305
) -> Result<Self> {
306306
// create and setup hypervisor partition
307307
let mut partition = VMPartition::new(1)?;
@@ -332,27 +332,38 @@ impl HypervWindowsDriver {
332332
// subtract 2 pages for the guard pages, since when we copy memory to and from surrogate process,
333333
// we don't want to copy the guard pages themselves (that would cause access violation)
334334
let mem_size = raw_size - 2 * PAGE_SIZE_USIZE;
335-
Ok(Self {
335+
336+
let interrupt_handle = Arc::new(WindowsInterruptHandle {
337+
running: AtomicBool::new(false),
338+
cancel_requested: AtomicBool::new(false),
339+
partition_handle,
340+
dropped: AtomicBool::new(false),
341+
});
342+
343+
#[allow(unused_mut)]
344+
let mut hv = Self {
336345
size: mem_size,
337346
processor: proc,
338347
_surrogate_process: surrogate_process,
339348
source_address: raw_source_address,
340349
entrypoint,
341350
orig_rsp: GuestPtr::try_from(RawPtr::from(rsp))?,
342351
mem_regions,
343-
interrupt_handle: Arc::new(WindowsInterruptHandle {
344-
running: AtomicBool::new(false),
345-
cancel_requested: AtomicBool::new(false),
346-
partition_handle,
347-
dropped: AtomicBool::new(false),
348-
}),
352+
interrupt_handle: interrupt_handle.clone(),
349353
#[cfg(crashdump)]
350354
rt_cfg,
351355
#[cfg(gdb)]
352356
debug,
353357
#[cfg(gdb)]
354358
gdb_conn,
355-
})
359+
};
360+
361+
// Send the interrupt handle to the GDB thread if debugging is enabled
362+
// This is used to allow the GDB thread to stop the vCPU
363+
#[cfg(gdb)]
364+
hv.send_dbg_msg(DebugResponse::InterruptHandle(interrupt_handle))?;
365+
366+
Ok(hv)
356367
}
357368

358369
fn setup_initial_sregs(proc: &mut VMProcessor, pml4_addr: u64) -> Result<()> {
@@ -873,6 +884,7 @@ impl Drop for HypervWindowsDriver {
873884
}
874885
}
875886

887+
#[derive(Debug)]
876888
pub struct WindowsInterruptHandle {
877889
// `WHvCancelRunVirtualProcessor()` will return Ok even if the vcpu is not running, which is the reason we need this flag.
878890
running: AtomicBool,

0 commit comments

Comments
 (0)