Merge pull request #572 from bytecodealliance/acf/panic-ferry

💢 Package unknown hostcall panics into termination details

Author: acfoltzer

lucet-runtime/include/lucet_types.h

@@ -50,6 +50,7 @@ enum lucet_terminated_reason {
     lucet_terminated_reason_borrow_error,
     lucet_terminated_reason_provided,
     lucet_terminated_reason_remote,
+    lucet_terminated_reason_other_panic,
 };
 
 enum lucet_trapcode {

lucet-runtime/lucet-runtime-internals/src/c_api.rs

@@ -309,15 +309,21 @@ pub mod lucet_result {
                             },
                             TerminationDetails::Provided(p) => lucet_terminated {
                                 reason: lucet_terminated_reason::Provided,
-                                provided: p
+                                provided: *p
                                     .downcast_ref()
-                                    .map(|CTerminationDetails { details }| *details)
-                                    .unwrap_or(ptr::null_mut()),
+                                    .map(|CTerminationDetails { details }| details)
+                                    .unwrap_or(&ptr::null_mut()),
                             },
                             TerminationDetails::Remote => lucet_terminated {
                                 reason: lucet_terminated_reason::Remote,
                                 provided: std::ptr::null_mut(),
                             },
+                            TerminationDetails::OtherPanic(p) => lucet_terminated {
+                                reason: lucet_terminated_reason::OtherPanic,
+                                // double box the panic payload so that the pointer passed to FFI
+                                // land is thin
+                                provided: Box::into_raw(Box::new(p)) as *mut _,
+                            },
                         },
                     },
                 },
@@ -372,6 +378,7 @@ pub mod lucet_result {
         BorrowError,
         Provided,
         Remote,
+        OtherPanic,
     }
 
     #[repr(C)]

lucet-runtime/lucet-runtime-internals/src/instance.rs

@@ -1284,6 +1284,25 @@ pub enum TerminationDetails {
     Provided(Box<dyn Any + 'static>),
     /// The instance was terminated by its `KillSwitch`.
     Remote,
+    /// A panic occurred during a hostcall other than the specialized panic used to implement
+    /// Lucet runtime features.
+    ///
+    /// Panics are raised by the Lucet runtime in order to unwind the hostcall before jumping back
+    /// to the host context for any of the reasons described by the variants of this type. The panic
+    /// payload in that case is a already a `TerminationDetails` value.
+    ///
+    /// This variant is created when any type other than `TerminationDetails` is the payload of a
+    /// panic arising during a hostcall, meaning it was not intentionally raised by the Lucet
+    /// runtime.
+    ///
+    /// The panic payload contained in this variant should be rethrown using
+    /// [`resume_unwind`](https://doc.rust-lang.org/std/panic/fn.resume_unwind.html) once returned
+    /// to the host context.
+    ///
+    /// Note that this variant will be removed once cross-FFI unwinding support lands in
+    /// [Rust](https://github.com/rust-lang/rfcs/pull/2945) and
+    /// [Lucet](https://github.com/bytecodealliance/lucet/pull/254).
+    OtherPanic(Box<dyn Any + Send + 'static>),
 }
 
 impl TerminationDetails {
@@ -1334,6 +1353,7 @@ impl std::fmt::Debug for TerminationDetails {
             TerminationDetails::YieldTypeMismatch => write!(f, "YieldTypeMismatch"),
             TerminationDetails::Provided(_) => write!(f, "Provided(Any)"),
             TerminationDetails::Remote => write!(f, "Remote"),
+            TerminationDetails::OtherPanic(_) => write!(f, "OtherPanic(Any)"),
         }
     }
 }

lucet-runtime/lucet-runtime-macros/src/lib.rs

@@ -111,12 +111,11 @@ pub fn lucet_hostcall(_attr: TokenStream, item: TokenStream) -> TokenStream {
                 match res {
                     Ok(res) => res,
                     Err(e) => {
-                        match e.downcast::<#termination_details>() {
-                            Ok(details) => {
-                                #vmctx_mod::Vmctx::from_raw(vmctx_raw).terminate_no_unwind(*details)
-                            },
-                            Err(e) => std::panic::resume_unwind(e),
-                        }
+                        let details = match e.downcast::<#termination_details>() {
+                            Ok(details) => *details,
+                            Err(e) => #termination_details::OtherPanic(e),
+                        };
+                        #vmctx_mod::Vmctx::from_raw(vmctx_raw).terminate_no_unwind(details)
                     }
                 }
             })

lucet-runtime/lucet-runtime-tests/src/guest_fault.rs

@@ -28,6 +28,14 @@ macro_rules! guest_fault_common_defs {
                 123
             }
 
+            pub struct OtherPanicPayload;
+
+            #[lucet_hostcall]
+            #[no_mangle]
+            pub fn raise_other_panic(_vmctx: &Vmctx) {
+                panic!(OtherPanicPayload);
+            }
+
             pub static mut RECOVERABLE_PTR: *mut libc::c_char = std::ptr::null_mut();
 
             #[no_mangle]
@@ -55,6 +63,17 @@ macro_rules! guest_fault_common_defs {
                     }
                 }
 
+                extern "C" fn raise_other_panic_main(vmctx: *const lucet_vmctx) {
+                    extern "C" {
+                        // actually is defined in this file
+                        fn raise_other_panic(vmctx: *const lucet_vmctx);
+                    }
+                    unsafe {
+                        raise_other_panic(vmctx);
+                        std::hint::unreachable_unchecked();
+                    }
+                }
+
                 extern "C" fn infinite_loop(_vmctx: *const lucet_vmctx) {
                     loop {}
                 }
@@ -156,6 +175,10 @@ macro_rules! guest_fault_common_defs {
                         "hostcall_main",
                         FunctionPointer::from_usize(hostcall_main as usize),
                     ))
+                    .with_export_func(MockExportBuilder::new(
+                        "raise_other_panic_main",
+                        FunctionPointer::from_usize(raise_other_panic_main as usize),
+                    ))
                     .with_export_func(MockExportBuilder::new(
                         "infinite_loop",
                         FunctionPointer::from_usize(infinite_loop as usize),
@@ -622,7 +645,8 @@ macro_rules! guest_fault_tests {
                     test_nonex(|| {
                         let module = mock_traps_module();
                         let region =
-                            <TestRegion as RegionCreate>::create(1, &Limits::default()).expect("region can be created");
+                            <TestRegion as RegionCreate>::create(1, &Limits::default())
+                            .expect("region can be created");
                         let mut inst = region
                             .new_instance(module)
                             .expect("instance can be created");
@@ -648,6 +672,32 @@ macro_rules! guest_fault_tests {
                     });
                 }
 
+                #[test]
+                fn raise_other_panic() {
+                    test_nonex(|| {
+                        let module = mock_traps_module();
+                        let region =
+                            <TestRegion as RegionCreate>::create(1, &Limits::default())
+                            .expect("region can be created");
+                        let mut inst = region
+                            .new_instance(module)
+                            .expect("instance can be created");
+
+                        match inst.run("raise_other_panic_main", &[]) {
+                            Err(Error::RuntimeTerminated(TerminationDetails::OtherPanic(payload))) => {
+                                assert!(payload.is::<crate::common::OtherPanicPayload>());
+                            }
+                            res => panic!("unexpected result: {:?}", res),
+                        }
+
+                        // after a fault, can reset and run a normal function; in practice we would
+                        // want to reraise the panic most of the time, but this should still work
+                        inst.reset().expect("instance resets");
+
+                        run_onetwothree(&mut inst);
+                    });
+                }
+
                 #[test]
                 fn fatal_continue_signal_handler() {
                     fn signal_handler_continue(