diff --git a/src/hyperlight_common/src/component.rs b/src/hyperlight_common/src/component.rs index 524d90f11..27cbc084f 100644 --- a/src/hyperlight_common/src/component.rs +++ b/src/hyperlight_common/src/component.rs @@ -41,6 +41,8 @@ mod private { /// imports. pub trait Positivity: private::Sealed { type NegativeOfThis: Positivity; + /// How a call to one of the interface's functions returns. + type CallResult; /// How a borrowed resource handle reaches the implementation. type Borrow<'a, T: 'a>; } @@ -58,6 +60,8 @@ impl private::Sealed for Positive {} impl Positivity for Negative { type NegativeOfThis = Positive; + /// A host implementation is called directly, so it cannot fail. + type CallResult = T; /// A handle arrives as an index into the resource table, held borrowed /// for the duration of the call. type Borrow<'a, T: 'a> = BorrowedResourceGuard<'a, T>; @@ -65,6 +69,14 @@ impl Positivity for Negative { impl Positivity for Positive { type NegativeOfThis = Negative; + /// Every call from the host crosses into the VM, where the guest can trap. + #[cfg(feature = "std")] + type CallResult = anyhow::Result; + /// The guest is not permitted to semantically enlarge its + /// functions to include kinds of failures other than the usual + /// trap/VM issue + #[cfg(not(feature = "std"))] + type CallResult = T; /// The host owns the value, so it hands out a plain reference. type Borrow<'a, T: 'a> = &'a T; } diff --git a/src/hyperlight_component_util/src/host.rs b/src/hyperlight_component_util/src/host.rs index 96f6043c9..d5f40f594 100644 --- a/src/hyperlight_component_util/src/host.rs +++ b/src/hyperlight_component_util/src/host.rs @@ -59,19 +59,22 @@ fn emit_export_extern_decl<'a, 'b, 'c>( fn #n(&mut self, #(#param_decls),*) -> #result_decl { let mut to_cleanup = Vec::>::new(); let marshalled = { - let mut rts = self.rt.lock().unwrap(); + let mut rts = self.rt + .lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #[allow(clippy::unused_unit)] (#(#marshal,)*) }; let #ret = ::hyperlight_host::sandbox::Callable::call::<::std::vec::Vec::>(&mut self.sb, #hln, marshalled, - ); - let ::std::result::Result::Ok(#ret) = #ret else { panic!("bad return from guest {:?}", #ret) }; + )?; #[allow(clippy::unused_unit)] - let mut rts = self.rt.lock().unwrap(); + let mut rts = self.rt + .lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #[allow(clippy::unused_unit)] - #unmarshal + ::std::result::Result::Ok(#unmarshal) } } } @@ -167,7 +170,8 @@ impl SelfInfo { orig_id, type_id: vec![(format_ident!("I"), imports_trait_bound)], inner_preamble: quote! { - let mut #inner_id = #outer_id.lock().unwrap(); + let mut #inner_id = #outer_id.lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; let mut #inner_id = ::std::ops::DerefMut::deref_mut(&mut #inner_id); }, outer_id, @@ -268,7 +272,8 @@ fn emit_import_extern_decl<'a, 'b, 'c>( let #outer_id = #orig_id.clone(); let captured_rts = rts.clone(); sb.register_host_function(#hln, move |#(#pds),*| { - let mut rts = captured_rts.lock().unwrap(); + let mut rts = captured_rts.lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #inner_preamble let #ret = #callname( ::std::borrow::BorrowMut::<#type_inst>::borrow_mut( @@ -277,8 +282,7 @@ fn emit_import_extern_decl<'a, 'b, 'c>( #(#pus),* ); Ok(#marshal_result) - }) - .unwrap(); + })?; } } ExternDesc::Type(_) => { @@ -407,24 +411,24 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com pub(crate) sb: S, pub(crate) rt: ::std::sync::Arc<::std::sync::Mutex<#rtsid>>, } - pub(crate) fn register_host_functions + ::std::marker::Send + 'static, S: ::hyperlight_host::func::Registerable>(sb: &mut S, i: I) -> ::std::sync::Arc<::std::sync::Mutex<#rtsid>> { + pub(crate) fn register_host_functions + ::std::marker::Send + 'static, S: ::hyperlight_host::func::Registerable>(sb: &mut S, i: I) -> <::hyperlight_common::component::Positive as ::hyperlight_common::component::Positivity>::CallResult<::std::sync::Arc<::std::sync::Mutex<#rtsid>>> { let rts = ::std::sync::Arc::new(::std::sync::Mutex::new(#rtsid::new())); let #import_id = ::std::sync::Arc::new(::std::sync::Mutex::new(i)); #(#imports)* - rts + Ok(rts) } impl + ::std::marker::Send, S: ::hyperlight_host::sandbox::Callable> #ns::#export_trait<::hyperlight_common::component::Positive, I> for #wrapper_name { #(#exports)* } impl #ns::#r#trait<::hyperlight_common::component::Positive> for ::hyperlight_host::sandbox::UninitializedSandbox { type Exports + ::std::marker::Send> = #wrapper_name; - fn instantiate + ::std::marker::Send + 'static>(mut self, i: I) -> Self::Exports { - let rts = register_host_functions(&mut self, i); - let sb = self.evolve().unwrap(); - #wrapper_name { + fn instantiate + ::std::marker::Send + 'static>(mut self, i: I) -> <::hyperlight_common::component::Positive as ::hyperlight_common::component::Positivity>::CallResult> { + let rts = register_host_functions(&mut self, i)?; + let sb = self.evolve()?; + Ok(#wrapper_name { sb, rt: rts, - } + }) } } }); diff --git a/src/hyperlight_component_util/src/rtypes.rs b/src/hyperlight_component_util/src/rtypes.rs index 4ff6ea351..c216cc371 100644 --- a/src/hyperlight_component_util/src/rtypes.rs +++ b/src/hyperlight_component_util/src/rtypes.rs @@ -607,10 +607,12 @@ pub fn emit_func_param(s: &mut State, p: &Param) -> TokenStream { /// Precondition: the result type must only be a named result if there /// are no names in it (i.e. a unit type) pub fn emit_func_result(s: &mut State, r: &etypes::Result<'_>) -> TokenStream { - match r { + let inner = match r { Some(vt) => emit_value(s, vt), None => quote! { () }, - } + }; + let p = s.positivity_param.clone().unwrap_or(quote! { P }); + quote! { <#p as ::hyperlight_common::component::Positivity>::CallResult<#inner> } } /// Emit a Rust typeversion of a component function type. This is only @@ -966,7 +968,7 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com type Exports + ::core::marker::Send>: #export_name; // todo: can/should this 'static bound be avoided? // it is important right now because this is closed over in host functions - fn instantiate + ::core::marker::Send + 'static>(self, imports: I) -> Self::Exports; + fn instantiate + ::core::marker::Send + 'static>(self, imports: I) ->

::CallResult>; } }); } diff --git a/src/hyperlight_host/tests/wit_test.rs b/src/hyperlight_host/tests/wit_test.rs index 983804842..a3073ade3 100644 --- a/src/hyperlight_host/tests/wit_test.rs +++ b/src/hyperlight_host/tests/wit_test.rs @@ -289,14 +289,15 @@ fn sb() -> TestSandbox { let path = wit_guest_as_pathbuf(); let guest_path = GuestBinary::FilePath(path); let uninit = UninitializedSandbox::new(guest_path, None).unwrap(); - test::wit::Test::instantiate(uninit, Host {}) + test::wit::Test::instantiate(uninit, Host {}).unwrap() } mod wit_test { - use proptest::prelude::*; - use crate::bindings::test::wit::{Roundtrip, TestExports, TestHostResource, roundtrip}; + use crate::bindings::test::wit::{ + Failable, Roundtrip, TestExports, TestHostResource, roundtrip, + }; use crate::sb; prop_compose! { @@ -348,7 +349,7 @@ mod wit_test { proptest! { #[test] fn $fn(x $($ty)*) { - assert_eq!(x, sb().roundtrip().$fn(x.clone())) + assert_eq!(x, sb().roundtrip().$fn(x.clone()).unwrap()) } } } @@ -395,7 +396,16 @@ mod wit_test { #[test] fn test_roundtrip_no_result() { - sb().roundtrip().roundtrip_no_result(42); + sb().roundtrip().roundtrip_no_result(42).unwrap(); + } + + #[test] + fn test_guest_trap_returns_error() { + let err = sb().failable().will_trap().unwrap_err(); + assert!( + format!("{err:?}").contains("Guest aborted"), + "unexpected error: {err:?}" + ); } use std::sync::atomic::Ordering::Relaxed; @@ -405,7 +415,7 @@ mod wit_test { let guard = crate::SERIALIZE_TEST_RESOURCE_TESTS.lock(); crate::HAS_BEEN_DROPPED.store(false, Relaxed); { - sb().test_host_resource().test_uses_locally(); + sb().test_host_resource().test_uses_locally().unwrap(); } assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); drop(guard); @@ -417,10 +427,10 @@ mod wit_test { { let mut sb = sb(); let inst = sb.test_host_resource(); - let r = inst.test_makes(); - inst.test_accepts_borrow(&r); - inst.test_accepts_own(r); - inst.test_returns(); + let r = inst.test_makes().unwrap(); + inst.test_accepts_borrow(&r).unwrap(); + inst.test_accepts_own(r).unwrap(); + inst.test_returns().unwrap(); } assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); drop(guard); @@ -498,18 +508,20 @@ mod bindgen_test_cases { struct ExportHost; impl test::bindgen_test_cases::Executor for ExportHost { - fn execute(&mut self) -> test::bindgen_test_cases::executor::ExecutionResult { - test::bindgen_test_cases::executor::ExecutionResult { + fn execute( + &mut self, + ) -> anyhow::Result { + Ok(test::bindgen_test_cases::executor::ExecutionResult { message: String::from("executed"), - } + }) } } impl test::bindgen_test_cases::Types for ExportHost { - fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { - test::bindgen_test_cases::types::Status { + fn get_status(&mut self) -> anyhow::Result { + Ok(test::bindgen_test_cases::types::Status { message: String::from("ok"), - } + }) } } @@ -519,10 +531,10 @@ mod bindgen_test_cases { test::bindgen_test_cases::types::Status, > for ExportHost { - fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { - test::bindgen_test_cases::types::Status { + fn get_status(&mut self) -> anyhow::Result { + Ok(test::bindgen_test_cases::types::Status { message: String::from("ok"), - } + }) } } diff --git a/src/tests/rust_guests/witguest/guest.wit b/src/tests/rust_guests/witguest/guest.wit index 9ed164b80..3000e0105 100644 --- a/src/tests/rust_guests/witguest/guest.wit +++ b/src/tests/rust_guests/witguest/guest.wit @@ -5,6 +5,7 @@ world test { import host-resource; export roundtrip; export test-host-resource; + export failable; } interface roundtrip { @@ -89,4 +90,8 @@ interface test-host-resource { test-accepts-borrow: func(x: borrow); test-accepts-own: func(x: own); test-returns: func() -> own; +} + +interface failable { + will-trap: func() -> string; } \ No newline at end of file diff --git a/src/tests/rust_guests/witguest/src/main.rs b/src/tests/rust_guests/witguest/src/main.rs index 37ad9ca89..51da0bdea 100644 --- a/src/tests/rust_guests/witguest/src/main.rs +++ b/src/tests/rust_guests/witguest/src/main.rs @@ -206,6 +206,12 @@ impl test::wit::TestHostResource>::T> } } +impl test::wit::Failable for Guest { + fn will_trap(&mut self) -> String { + panic!("deliberate guest crash") + } +} + #[allow(refining_impl_trait)] impl test::wit::TestExports for Guest { type Roundtrip = Self; @@ -216,6 +222,10 @@ impl test::wit::TestExports for Guest { fn test_host_resource(&mut self) -> &mut Self { self } + type Failable = Self; + fn failable(&mut self) -> &mut Self { + self + } } static GUEST_STATE: Mutex = Mutex::new(Guest {