From 75f1e8842d3d4124cd6a547b97c5491c839068f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 30 Jul 2025 17:50:20 +0300 Subject: [PATCH 1/2] fix ci tracing tests silently failing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .github/workflows/dep_rust.yml | 2 +- Justfile | 12 ++++++++---- src/tests/rust_guests/witguest/Cargo.lock | 8 ++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/dep_rust.yml b/.github/workflows/dep_rust.yml index e625eb88a..fc68e3e4b 100644 --- a/.github/workflows/dep_rust.yml +++ b/.github/workflows/dep_rust.yml @@ -175,7 +175,7 @@ jobs: env: CARGO_TERM_COLOR: always RUST_LOG: debug - run: just test-rust-tracing ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv2' && 'mshv2' || ''}} + run: just test-rust-tracing ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv' && 'mshv2' || ''}} - name: Download benchmarks from "latest" run: just bench-download ${{ runner.os }} ${{ matrix.hypervisor }} ${{ matrix.cpu}} dev-latest # compare to prerelease diff --git a/Justfile b/Justfile index d478e6a2a..e6cf371bb 100644 --- a/Justfile +++ b/Justfile @@ -204,10 +204,14 @@ test-rust-tracing target=default-target features="": just build-rust-guests {{ target }} trace_guest just move-rust-guests {{ target }} # Run hello-world example with tracing enabled to get the trace output - # Capture the trace file path and print use it afterwards to run cargo run -p trace_dump - cargo run --profile={{ if target == "debug" { "dev" } else { target } }} --example hello-world --features {{ if features =="" {'trace_guest'} else { "trace_guest," + features } }} \ - | sed -n 's/.*Creating trace file at: \(.*\)/\1/p' \ - | xargs -I {} cargo run -p trace_dump ./{{ simpleguest_source }}/{{ target }}/simpleguest {} list_frames + TRACE_OUTPUT="$(cargo run --profile={{ if target == "debug" { "dev" } else { target } }} --example hello-world --features {{ if features =="" {"trace_guest"} else { "trace_guest," + features } }})" && \ + TRACE_FILE="$(echo "$TRACE_OUTPUT" | grep -oE 'Creating trace file at: [^ ]+' | awk -F': ' '{print $2}')" && \ + echo "$TRACE_OUTPUT" && \ + if [ -z "$TRACE_FILE" ]; then \ + echo "Error: Could not extract trace file path from output." >&2 ; \ + exit 1 ; \ + fi && \ + cargo run -p trace_dump ./{{ simpleguest_source }}/{{ target }}/simpleguest "$TRACE_FILE" list_frames # Rebuild the tracing guests without the tracing feature # This is to ensure that the tracing feature does not affect the other tests diff --git a/src/tests/rust_guests/witguest/Cargo.lock b/src/tests/rust_guests/witguest/Cargo.lock index ddd6f659e..ea2537ea3 100644 --- a/src/tests/rust_guests/witguest/Cargo.lock +++ b/src/tests/rust_guests/witguest/Cargo.lock @@ -359,9 +359,9 @@ dependencies = [ [[package]] name = "prettyplease" -version = "0.2.35" +version = "0.2.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "061c1221631e079b26479d25bbf2275bfe5917ae8419cd7e34f13bfc2aa7539a" +checksum = "ff24dfcda44452b9816fff4cd4227e1bb73ff5a2f1bc1105aa92fb8565ce44d2" dependencies = [ "proc-macro2", "syn", @@ -522,9 +522,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "wasmparser" -version = "0.235.0" +version = "0.236.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "161296c618fa2d63f6ed5fffd1112937e803cb9ec71b32b01a76321555660917" +checksum = "16d1eee846a705f6f3cb9d7b9f79b54583810f1fb57a1e3aea76d1742db2e3d2" dependencies = [ "bitflags", "hashbrown", From a32e8b475868e70d4a46757f717442a439c3ad8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 30 Jul 2025 18:54:07 +0300 Subject: [PATCH 2/2] fix mem_mgr not initialized when tracing enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/hyperv_linux.rs | 27 +++++++------------ .../src/hypervisor/hyperv_windows.rs | 27 +++++++------------ src/hyperlight_host/src/hypervisor/kvm.rs | 27 +++++++------------ src/hyperlight_host/src/hypervisor/mod.rs | 7 ++--- .../src/sandbox/initialized_multi_use.rs | 1 - src/hyperlight_host/src/sandbox/mem_access.rs | 7 +++-- 6 files changed, 39 insertions(+), 57 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index df3073a09..62bce6425 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -609,24 +609,11 @@ impl Hypervisor for HypervLinuxDriver { }; self.vcpu_fd.set_regs(®s)?; - // Extract mem_mgr to avoid borrowing conflicts - let mem_mgr = self - .mem_mgr - .take() - .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; - - let result = VirtualCPU::run( + VirtualCPU::run( self.as_mut_hypervisor(), - &mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, - ); - - // Put mem_mgr back - self.mem_mgr = Some(mem_mgr); - result?; - - Ok(()) + ) } #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -664,7 +651,6 @@ impl Hypervisor for HypervLinuxDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -688,7 +674,6 @@ impl Hypervisor for HypervLinuxDriver { // run VirtualCPU::run( self.as_mut_hypervisor(), - mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, )?; @@ -1162,6 +1147,14 @@ impl Hypervisor for HypervLinuxDriver { Ok(()) } + fn check_stack_guard(&self) -> Result { + if let Some(mgr) = self.mem_mgr.as_ref() { + mgr.check_stack_guard() + } else { + Err(new_error!("Memory manager is not initialized")) + } + } + #[cfg(feature = "trace_guest")] fn read_trace_reg(&self, reg: TraceRegister) -> Result { let mut assoc = [hv_register_assoc { diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 595372306..d4a0c06cd 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -623,24 +623,11 @@ impl Hypervisor for HypervWindowsDriver { }; self.processor.set_general_purpose_registers(®s)?; - // Extract mem_mgr to avoid borrowing conflicts - let mem_mgr = self - .mem_mgr - .take() - .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; - - let result = VirtualCPU::run( + VirtualCPU::run( self.as_mut_hypervisor(), - &mem_mgr, #[cfg(gdb)] dbg_mem_access_hdl, - ); - - // Put mem_mgr back - self.mem_mgr = Some(mem_mgr); - result?; - - Ok(()) + ) } #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -662,7 +649,6 @@ impl Hypervisor for HypervWindowsDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -684,7 +670,6 @@ impl Hypervisor for HypervWindowsDriver { VirtualCPU::run( self.as_mut_hypervisor(), - mem_mgr, #[cfg(gdb)] dbg_mem_access_hdl, )?; @@ -1094,6 +1079,14 @@ impl Hypervisor for HypervWindowsDriver { Ok(()) } + fn check_stack_guard(&self) -> Result { + if let Some(mgr) = self.mem_mgr.as_ref() { + mgr.check_stack_guard() + } else { + Err(new_error!("Memory manager is not initialized")) + } + } + #[cfg(feature = "trace_guest")] fn read_trace_reg(&self, reg: TraceRegister) -> Result { let regs = self.processor.get_regs()?; diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 761020c50..8a2de0d40 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -492,24 +492,11 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_regs(®s)?; - // Extract mem_mgr to avoid borrowing conflicts - let mem_mgr = self - .mem_mgr - .take() - .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; - - let result = VirtualCPU::run( + VirtualCPU::run( self.as_mut_hypervisor(), - &mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, - ); - - // Put mem_mgr back - self.mem_mgr = Some(mem_mgr); - result?; - - Ok(()) + ) } #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -555,7 +542,6 @@ impl Hypervisor for KVMDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -578,7 +564,6 @@ impl Hypervisor for KVMDriver { // run VirtualCPU::run( self.as_mut_hypervisor(), - mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, )?; @@ -1012,6 +997,14 @@ impl Hypervisor for KVMDriver { Ok(()) } + fn check_stack_guard(&self) -> Result { + if let Some(mgr) = self.mem_mgr.as_ref() { + mgr.check_stack_guard() + } else { + Err(new_error!("Memory manager is not initialized")) + } + } + #[cfg(feature = "trace_guest")] fn read_trace_reg(&self, reg: TraceRegister) -> Result { let regs = self.vcpu_fd.get_regs()?; diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 801a354be..0fe452b6e 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -170,7 +170,6 @@ pub(crate) trait Hypervisor: Debug + Send { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; @@ -269,6 +268,9 @@ pub(crate) trait Hypervisor: Debug + Send { unimplemented!() } + /// Check stack guard to see if the stack is still valid + fn check_stack_guard(&self) -> Result; + /// Read a register for trace/unwind purposes #[cfg(feature = "trace_guest")] fn read_trace_reg(&self, reg: TraceRegister) -> Result; @@ -289,7 +291,6 @@ impl VirtualCPU { #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub(crate) fn run( hv: &mut dyn Hypervisor, - mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: Arc>, ) -> Result<()> { loop { @@ -311,7 +312,7 @@ impl VirtualCPU { #[cfg(crashdump)] crashdump::generate_crashdump(hv)?; - handle_mem_access(mem_mgr)?; + handle_mem_access(hv)?; log_then_return!("MMIO access address {:#x}", addr); } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index ada0ea174..b3cee64f8 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -230,7 +230,6 @@ impl MultiUseSandbox { self.vm.dispatch_call_from_host( self.dispatch_ptr.clone(), - &self.mem_mgr, #[cfg(gdb)] self.dbg_mem_access_fn.clone(), )?; diff --git a/src/hyperlight_host/src/sandbox/mem_access.rs b/src/hyperlight_host/src/sandbox/mem_access.rs index ea0d3a3fe..e9de96e99 100644 --- a/src/hyperlight_host/src/sandbox/mem_access.rs +++ b/src/hyperlight_host/src/sandbox/mem_access.rs @@ -19,16 +19,19 @@ use std::sync::{Arc, Mutex}; use tracing::{Span, instrument}; +#[cfg(gdb)] use super::mem_mgr::MemMgrWrapper; use crate::error::HyperlightError::StackOverflow; +use crate::hypervisor::Hypervisor; #[cfg(gdb)] use crate::hypervisor::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper}; +#[cfg(gdb)] use crate::mem::shared_mem::HostSharedMemory; use crate::{Result, log_then_return}; #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] -pub(crate) fn handle_mem_access(wrapper: &MemMgrWrapper) -> Result<()> { - if !wrapper.check_stack_guard()? { +pub(crate) fn handle_mem_access(hv: &dyn Hypervisor) -> Result<()> { + if !hv.check_stack_guard()? { log_then_return!(StackOverflow()); }