[guest/{entrypoint,host_fxn_call,interrupts},host/outb] brought back panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out8, out16, and out32 to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>

Author: danbugs

src/hyperlight_guest/src/entrypoint.rs

@@ -25,7 +25,7 @@ use spin::Once;
 use crate::gdt::load_gdt;
 use crate::guest_function_call::dispatch_function;
 use crate::guest_logger::init_logger;
-use crate::host_function_call::{debug_print, outb};
+use crate::host_function_call::outb;
 use crate::idtr::load_idt;
 use crate::{
     __security_cookie, HEAP_ALLOCATOR, MIN_STACK_ADDRESS, OS_PAGE_SIZE, OUTB_PTR,
@@ -43,11 +43,12 @@ pub fn halt() {
 
 #[no_mangle]
 pub extern "C" fn abort() -> ! {
-    abort_with_code(&[0])
+    abort_with_code(&[0, 0xFF])
 }
 
 pub fn abort_with_code(code: &[u8]) -> ! {
     outb(OutBAction::Abort as u16, code);
+    outb(OutBAction::Abort as u16, &[0xFF]); // send abort terminator (if not included in code)
     unreachable!()
 }
 
@@ -56,13 +57,19 @@ pub fn abort_with_code(code: &[u8]) -> ! {
 /// # Safety
 /// This function is unsafe because it dereferences a raw pointer.
 pub unsafe fn abort_with_code_and_message(code: &[u8], message_ptr: *const c_char) -> ! {
-    debug_print(
-        CStr::from_ptr(message_ptr)
-            .to_str()
-            .expect("Invalid UTF-8 string"),
-    );
-
+    // Step 1: Send abort code (typically 1 byte, but `code` allows flexibility)
     outb(OutBAction::Abort as u16, code);
+
+    // Step 2: Convert the C string to bytes
+    let message_bytes = CStr::from_ptr(message_ptr).to_bytes(); // excludes null terminator
+
+    // Step 3: Send the message itself in chunks
+    outb(OutBAction::Abort as u16, message_bytes);
+
+    // Step 4: Send abort terminator to signal completion (e.g., 0xFF)
+    outb(OutBAction::Abort as u16, &[0xFF]);
+
+    // This function never returns
     unreachable!()
 }
 

src/hyperlight_guest/src/host_function_call.rs

@@ -17,7 +17,7 @@ limitations under the License.
 use alloc::format;
 use alloc::string::ToString;
 use alloc::vec::Vec;
-use core::arch::global_asm;
+use core::arch;
 
 use hyperlight_common::flatbuffer_wrappers::function_call::{FunctionCall, FunctionCallType};
 use hyperlight_common::flatbuffer_wrappers::function_types::{
@@ -79,21 +79,31 @@ pub fn outb(port: u16, data: &[u8]) {
     unsafe {
         match RUNNING_MODE {
             RunMode::Hypervisor => {
-                for chunk in data.chunks(4) {
-                    // Process the data in chunks of 4 bytes. If a chunk has fewer than 4 bytes,
-                    // pad it with 0x7F to ensure it can be converted into a 4-byte array.
-                    // The choice of 0x7F as the padding value is arbitrary and does not carry
-                    // any special meaning; it simply ensures consistent chunk size.
-                    let val = match chunk {
-                        [a, b, c, d] => u32::from_le_bytes([*a, *b, *c, *d]),
-                        [a, b, c] => u32::from_le_bytes([*a, *b, *c, 0x7F]),
-                        [a, b] => u32::from_le_bytes([*a, *b, 0x7F, 0x7F]),
-                        [a] => u32::from_le_bytes([*a, 0x7F, 0x7F, 0x7F]),
-                        [] => break,
-                        _ => unreachable!(),
-                    };
-
-                    hloutd(val, port);
+                let mut i = 0;
+                while i < data.len() {
+                    let remaining = data.len() - i;
+                    match remaining {
+                        4.. => {
+                            let val = u32::from_le_bytes([
+                                data[i],
+                                data[i + 1],
+                                data[i + 2],
+                                data[i + 3],
+                            ]);
+                            out32(port, val);
+                            i += 4;
+                        }
+                        2..=3 => {
+                            let val = u16::from_le_bytes([data[i], data[i + 1]]);
+                            out16(port, val);
+                            i += 2;
+                        }
+                        1 => {
+                            out8(port, data[i]);
+                            i += 1;
+                        }
+                        _ => break,
+                    }
                 }
             }
             RunMode::InProcessLinux | RunMode::InProcessWindows => {
@@ -119,11 +129,33 @@ pub fn outb(port: u16, data: &[u8]) {
     }
 }
 
-extern "win64" {
-    fn hloutd(value: u32, port: u16);
+pub(crate) unsafe fn out8(port: u16, val: u8) {
+    arch::asm!("out dx, al", in("dx") port, in("al") val, options(preserves_flags, nomem, nostack));
+}
+
+pub(crate) unsafe fn out16(port: u16, val: u16) {
+    arch::asm!("out dx, ax", in("dx") port, in("ax") val, options(preserves_flags, nomem, nostack));
+}
+
+pub(crate) unsafe fn out32(port: u16, val: u32) {
+    arch::asm!("out dx, eax", in("dx") port, in("eax") val, options(preserves_flags, nomem, nostack));
+}
+
+/// Prints a message using `OutBAction::DebugPrint`. It transmits bytes of a message
+/// through several VMExists and, with such, it is slower than
+/// `print_output_with_host_print`.
+///
+/// This function should be used in debug mode only. This function does not
+/// require memory to be setup to be used.
+pub fn debug_print(msg: &str) {
+    outb(OutBAction::DebugPrint as u16, msg.as_bytes());
 }
 
-pub fn print_output_as_guest_function(function_call: &FunctionCall) -> Result<Vec<u8>> {
+/// Print a message using the host's print function.
+///
+/// This function requires memory to be setup to be used. In particular, the
+/// existence of the input and output memory regions.
+pub fn print_output_with_host_print(function_call: &FunctionCall) -> Result<Vec<u8>> {
     if let ParameterValue::String(message) = function_call.parameters.clone().unwrap()[0].clone() {
         call_host_function(
             "HostPrint",
@@ -135,20 +167,7 @@ pub fn print_output_as_guest_function(function_call: &FunctionCall) -> Result<Ve
     } else {
         Err(HyperlightGuestError::new(
             ErrorCode::GuestError,
-            "Wrong Parameters passed to print_output_as_guest_function".to_string(),
+            "Wrong Parameters passed to print_output_with_host_print".to_string(),
         ))
     }
 }
-
-pub fn debug_print(msg: &str) {
-    outb(OutBAction::DebugPrint as u16, msg.as_bytes());
-}
-
-global_asm!(
-    ".global hloutd
-     hloutd:
-        mov eax, ecx
-        mov dx, dx
-        out dx, eax
-        ret"
-);

src/hyperlight_guest/src/interrupt_handlers.rs

@@ -31,10 +31,9 @@ pub extern "sysv64" fn hl_exception_handler(
 ) {
     let exception = Exception::try_from(exception_number as u8).expect("Invalid exception number");
     let msg = format!(
-        "EXCEPTION: {:#?}\n\
-            Page Fault Address: {:#x}\n\
+        "Page Fault Address: {:#x}\n\
             Stack Pointer: {:#x}",
-        exception, page_fault_address, stack_pointer
+        page_fault_address, stack_pointer
     );
 
     unsafe {

src/hyperlight_guest/src/lib.rs

@@ -17,16 +17,13 @@ limitations under the License.
 #![no_std]
 // Deps
 use alloc::string::ToString;
-use core::hint::unreachable_unchecked;
 
 use buddy_system_allocator::LockedHeap;
 use guest_function_register::GuestFunctionRegister;
-use host_function_call::debug_print;
 use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use hyperlight_common::mem::{HyperlightPEB, RunMode};
-use hyperlight_common::outb::OutBAction;
 
-use crate::host_function_call::outb;
+use crate::entrypoint::abort_with_code_and_message;
 extern crate alloc;
 
 // Modules
@@ -73,9 +70,11 @@ pub(crate) static _fltused: i32 = 0;
 // to satisfy the clippy when cfg == test
 #[allow(dead_code)]
 fn panic(info: &core::panic::PanicInfo) -> ! {
-    debug_print(info.to_string().as_str());
-    outb(OutBAction::Abort as u16, &[ErrorCode::UnknownError as u8]);
-    unsafe { unreachable_unchecked() }
+    let msg = info.to_string();
+    let c_string = alloc::ffi::CString::new(msg)
+        .unwrap_or_else(|_| alloc::ffi::CString::new("panic (invalid utf8)").unwrap());
+
+    unsafe { abort_with_code_and_message(&[ErrorCode::UnknownError as u8], c_string.as_ptr()) }
 }
 
 // Globals

src/hyperlight_host/src/sandbox/outb.rs

@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+use std::cell::RefCell;
 use std::sync::{Arc, Mutex};
 
 use hyperlight_common::flatbuffer_wrappers::function_types::ParameterValue;
@@ -93,6 +94,58 @@ pub(super) fn outb_log(mgr: &mut SandboxMemoryManager<HostSharedMemory>) -> Resu
     Ok(())
 }
 
+const ABORT_TERMINATOR: u8 = 0xFF;
+
+thread_local! {
+    static GUEST_ABORT_BUFFER: RefCell<Vec<u8>> = const { RefCell::new(Vec::new()) }
+}
+
+fn outb_abort(data: &[u8]) -> Result<()> {
+    GUEST_ABORT_BUFFER.with(|buf| {
+        let mut buffer = buf.borrow_mut();
+
+        for &b in data {
+            if b == ABORT_TERMINATOR {
+                let full = &buffer[..];
+
+                let guest_error_code = *full.first().unwrap_or(&0);
+                let guest_error = ErrorCode::from(guest_error_code as u64);
+
+                match guest_error {
+                    ErrorCode::StackOverflow => {
+                        buffer.clear();
+                        return Err(HyperlightError::StackOverflow());
+                    }
+                    _ => {
+                        let message = if let Some(&maybe_exception_code) = full.get(1) {
+                            match Exception::try_from(maybe_exception_code) {
+                                Ok(exception) => {
+                                    let extra_msg = String::from_utf8_lossy(&full[2..]);
+                                    format!("Exception: {:?} | {}", exception, extra_msg)
+                                }
+                                Err(_) => {
+                                    // Not a valid exception, so treat full[1..] as message
+                                    String::from_utf8_lossy(&full[1..]).into()
+                                }
+                            }
+                        } else {
+                            // Only the code was sent, no additional data
+                            String::new()
+                        };
+
+                        buffer.clear();
+                        return Err(HyperlightError::GuestAborted(guest_error_code, message));
+                    }
+                }
+            } else {
+                buffer.push(b);
+            }
+        }
+
+        Ok(())
+    })
+}
+
 /// Handles OutB operations from the guest.
 #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
 fn handle_outb_impl(
@@ -117,27 +170,7 @@ fn handle_outb_impl(
 
             Ok(())
         }
-        OutBAction::Abort => {
-            let byte = u64::from(data[0]);
-            let guest_error = ErrorCode::from(byte);
-
-            match guest_error {
-                ErrorCode::StackOverflow => Err(HyperlightError::StackOverflow()),
-                _ => {
-                    let message = match data.get(1) {
-                        Some(&exception_code) => match Exception::try_from(exception_code) {
-                            Ok(exception) => format!("Exception: {:?}", exception),
-                            Err(e) => {
-                                format!("Unknown exception code: {:#x} ({})", exception_code, e)
-                            }
-                        },
-                        None => "See stderr for panic context".into(),
-                    };
-
-                    Err(HyperlightError::GuestAborted(byte as u8, message))
-                }
-            }
-        }
+        OutBAction::Abort => outb_abort(&data),
         OutBAction::DebugPrint => {
             let s = String::from_utf8_lossy(&data);
             eprint!("{}", s);

src/hyperlight_host/tests/integration_test.rs

@@ -63,7 +63,7 @@ fn guest_abort() {
         .unwrap_err();
     println!("{:?}", res);
     assert!(
-        matches!(res, HyperlightError::GuestAborted(code, message) if (code == error_code && message.contains("NoException")) )
+        matches!(res, HyperlightError::GuestAborted(code, message) if (code == error_code && message.is_empty()))
     );
 }
 
@@ -83,7 +83,7 @@ fn guest_abort_with_context1() {
         .unwrap_err();
     println!("{:?}", res);
     assert!(
-        matches!(res, HyperlightError::GuestAborted(code, context) if (code == 25 && context.contains("NoException")))
+        matches!(res, HyperlightError::GuestAborted(code, context) if (code == 25 && context == "Oh no"))
     );
 }
 
@@ -135,7 +135,7 @@ fn guest_abort_with_context2() {
         .unwrap_err();
     println!("{:?}", res);
     assert!(
-        matches!(res, HyperlightError::GuestAborted(_, context) if context.contains("NoException"))
+        matches!(res, HyperlightError::GuestAborted(_, context) if context.contains(&abort_message[..400]))
     );
 }
 
@@ -161,7 +161,7 @@ fn guest_abort_c_guest() {
         .unwrap_err();
     println!("{:?}", res);
     assert!(
-        matches!(res, HyperlightError::GuestAborted(code, message) if (code == 75 && message.contains("NoException")) )
+        matches!(res, HyperlightError::GuestAborted(code, message) if (code == 75 && message == "This is a test error message") )
     );
 }
 
@@ -181,7 +181,7 @@ fn guest_panic() {
         .unwrap_err();
     println!("{:?}", res);
     assert!(
-        matches!(res, HyperlightError::GuestAborted(code, context) if code == ErrorCode::UnknownError as u8 && context.contains("NoException"))
+        matches!(res, HyperlightError::GuestAborted(code, context) if code == ErrorCode::UnknownError as u8 && context.contains("\nError... error..."))
     )
 }
 
@@ -262,7 +262,7 @@ fn guest_malloc_abort() {
     assert!(matches!(
         res.unwrap_err(),
         // OOM memory errors in rust allocator are panics. Our panic handler returns ErrorCode::UnknownError on panic
-        HyperlightError::GuestAborted(code, msg) if code == ErrorCode::UnknownError as u8 && msg.contains("NoException")
+        HyperlightError::GuestAborted(code, msg) if code == ErrorCode::UnknownError as u8 && msg.contains("memory allocation of ")
     ));
 }
 

src/tests/rust_guests/callbackguest/src/main.rs

@@ -35,7 +35,7 @@ use hyperlight_guest::error::{HyperlightGuestError, Result};
 use hyperlight_guest::guest_function_definition::GuestFunctionDefinition;
 use hyperlight_guest::guest_function_register::register_function;
 use hyperlight_guest::host_function_call::{
-    call_host_function, get_host_return_value, print_output_as_guest_function,
+    call_host_function, get_host_return_value, print_output_with_host_print,
 };
 use hyperlight_guest::logging::log_message;
 
@@ -167,7 +167,7 @@ pub extern "C" fn hyperlight_main() {
         "PrintOutput".to_string(),
         Vec::from(&[ParameterType::String]),
         ReturnType::Int,
-        print_output_as_guest_function as usize,
+        print_output_with_host_print as usize,
     );
     register_function(print_output_def);