Skip to content

fix: host_bindgen! returns Result instead of panicking on guest errors #1317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jsturtevant wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: host_bindgen! returns Result instead of panicking on guest errors #1317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/hyperlight_component_util/src/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ pub struct Mod {
pub items: TokenStream,
pub traits: BTreeMap<Ident, Trait>,
pub impls: BTreeMap<(Vec<Ident>, Ident), TokenStream>,
pub emitted_type_names: BTreeSet<Ident>,
}
impl Mod {
pub fn empty() -> Self {
Expand All @@ -132,6 +133,7 @@ impl Mod {
items: TokenStream::new(),
traits: BTreeMap::new(),
impls: BTreeMap::new(),
emitted_type_names: BTreeSet::new(),
}
}
/// Get a reference to a sub-module, creating it if necessary
Expand Down
7 changes: 4 additions & 3 deletions src/hyperlight_component_util/src/host.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ fn emit_export_extern_decl<'a, 'b, 'c>(
.map(|p| rtypes::emit_func_param(s, p))
.collect::<Vec<_>>();
let result_decl = rtypes::emit_func_result(s, &ft.result);
let result_decl = quote! { ::std::result::Result<#result_decl, ::hyperlight_host::error::HyperlightError> };
let hln = emit_fn_hl_name(s, ed.kebab_name);
let ret = format_ident!("ret");
let marshal = ft
Expand All @@ -66,11 +67,11 @@ fn emit_export_extern_decl<'a, 'b, 'c>(
#hln,
marshalled,
);
let ::std::result::Result::Ok(#ret) = #ret else { panic!("bad return from guest {:?}", #ret) };
let #ret = #ret?;
#[allow(clippy::unused_unit)]
let mut rts = self.rt.lock().unwrap();
#[allow(clippy::unused_unit)]
#unmarshal
Ok(#unmarshal)
}
}
}
Expand Down Expand Up @@ -121,7 +122,7 @@ fn emit_export_instance<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, it: &

let ns = wn.namespace_path();
let nsi = wn.namespace_idents();
let trait_name = kebab_to_type(wn.name);
let trait_name = kebab_to_exports_name(wn.name);
let r#trait = s.r#trait(&nsi, trait_name.clone());
let tvs = r#trait
.tvs
Expand Down
2 changes: 1 addition & 1 deletion src/hyperlight_component_util/src/resource.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub fn emit_tables<'a, 'b, 'c>(
#sphantom
}
impl<I: #bound #sv> #rtsid<I #svs> {
fn new() -> Self {
pub(crate) fn new() -> Self {
#rtsid {
#(#inits,)*
_phantomI: ::core::marker::PhantomData,
Expand Down
38 changes: 34 additions & 4 deletions src/hyperlight_component_util/src/rtypes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,11 @@ fn emit_extern_decl<'a, 'b, 'c>(
.map(|p| emit_func_param(&mut s, p))
.collect::<Vec<_>>();
let result = emit_func_result(&mut s, &ft.result);
let result = if !s.is_guest && s.is_export {
quote! { ::std::result::Result<#result, ::hyperlight_host::error::HyperlightError> }
} else {
result
};
quote! {
fn #n(&mut self, #(#params),*) -> #result;
}
Expand All @@ -663,12 +668,22 @@ fn emit_extern_decl<'a, 'b, 'c>(
}
ResourceItemName::Method(n) => {
let result = emit_func_result(&mut sv, &ft.result);
let result = if !sv.is_guest && sv.is_export {
quote! { ::std::result::Result<#result, ::hyperlight_host::error::HyperlightError> }
} else {
result
};
sv.cur_trait().items.extend(quote! {
fn #n(&mut self, #(#params),*) -> #result;
});
}
ResourceItemName::Static(n) => {
let result = emit_func_result(&mut sv, &ft.result);
let result = if !sv.is_guest && sv.is_export {
quote! { ::std::result::Result<#result, ::hyperlight_host::error::HyperlightError> }
} else {
result
};
sv.cur_trait().items.extend(quote! {
fn #n(&mut self, #(#params),*) -> #result;
});
Expand All @@ -691,6 +706,9 @@ fn emit_extern_decl<'a, 'b, 'c>(
) -> TokenStream {
let id = kebab_to_type(ed.kebab_name);
let mut s = s.helper();
if !s.cur_mod().emitted_type_names.insert(id.clone()) {
return TokenStream::new();
}

let t = emit_defined(&mut s, v, id, t);
s.cur_mod().items.extend(t);
Expand All @@ -712,6 +730,9 @@ fn emit_extern_decl<'a, 'b, 'c>(
let rn = kebab_to_type(ed.kebab_name);
s.add_helper_supertrait(rn.clone());
let mut s = s.helper();
if !s.cur_mod().emitted_type_names.insert(rn.clone()) {
return quote! {};
}
s.cur_trait = Some(rn.clone());
s.cur_trait().items.extend(quote! {
type T: ::core::marker::Send;
Expand All @@ -729,7 +750,12 @@ fn emit_extern_decl<'a, 'b, 'c>(
emit_instance(&mut s, wn.clone(), it);

let nsids = wn.namespace_idents();
let repr = s.r#trait(&nsids, kebab_to_type(wn.name));
let trait_name = if !s.is_guest && s.is_export {
kebab_to_exports_name(wn.name)
} else {
kebab_to_type(wn.name)
};
let repr = s.r#trait(&nsids, trait_name.clone());
Comment thread
jsturtevant marked this conversation as resolved.
let vs = if !repr.tvs.is_empty() {
let vs = repr.tvs.clone();
let tvs = vs
Expand All @@ -745,9 +771,9 @@ fn emit_extern_decl<'a, 'b, 'c>(
let tns = wn.namespace_path();
let tn = kebab_to_type(wn.name);
let trait_bound = if tns.is_empty() {
quote! { #rp #tn }
quote! { #rp #trait_name }
} else {
quote! { #rp #tns::#tn }
quote! { #rp #tns::#trait_name }
};
quote! {
type #tn: #trait_bound #vs;
Expand All @@ -766,7 +792,11 @@ fn emit_instance<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, it: &'c Inst
tracing::debug!("emitting instance {:?}", wn);
let mut s = s.with_cursor(wn.namespace_idents());

let name = kebab_to_type(wn.name);
let name = if !s.is_guest && s.is_export {
kebab_to_exports_name(wn.name)
} else {
kebab_to_type(wn.name)
};

s.cur_helper_mod = Some(kebab_to_namespace(wn.name));
s.cur_trait = Some(name.clone());
Expand Down
29 changes: 21 additions & 8 deletions src/hyperlight_host/tests/wit_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,12 @@ fn sb() -> TestSandbox<Host, MultiUseSandbox> {

mod wit_test {

use hyperlight_host::error::HyperlightError;
use proptest::prelude::*;

use crate::bindings::test::wit::{Roundtrip, TestExports, TestHostResource, roundtrip};
use crate::bindings::test::wit::{
FailableExports, RoundtripExports, TestExports, TestHostResourceExports, roundtrip,
};
use crate::sb;

prop_compose! {
Expand Down Expand Up @@ -347,7 +350,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())
}
}
}
Expand Down Expand Up @@ -394,7 +397,7 @@ mod wit_test {

#[test]
fn test_roundtrip_no_result() {
sb().roundtrip().roundtrip_no_result(42);
sb().roundtrip().roundtrip_no_result(42).unwrap();
}

use std::sync::atomic::Ordering::Relaxed;
Expand All @@ -404,7 +407,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);
Expand All @@ -416,14 +419,24 @@ 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);
}

#[test]
fn test_guest_call_error_returns_error() {
let mut sb = sb();
let err = sb.failable().guest_panic().unwrap_err();
assert!(matches!(
err,
HyperlightError::GuestAborted(_, msg) if msg.contains("deliberate guest panic")
));
}
Comment thread
jsturtevant marked this conversation as resolved.
}

mod pick_world_bindings {
Expand Down
5 changes: 5 additions & 0 deletions src/tests/rust_guests/witguest/guest.wit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ world test {
import host-resource;
export roundtrip;
export test-host-resource;
export failable;
}

interface roundtrip {
Expand Down Expand Up @@ -89,4 +90,8 @@ interface test-host-resource {
test-accepts-borrow: func(x: borrow<testresource>);
test-accepts-own: func(x: own<testresource>);
test-returns: func() -> own<testresource>;
}

interface failable {
guest-panic: func() -> string;
}
10 changes: 10 additions & 0 deletions src/tests/rust_guests/witguest/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ impl test::wit::Roundtrip for Guest {
}
}

impl test::wit::Failable for Guest {
fn guest_panic(&mut self) -> alloc::string::String {
panic!("deliberate guest panic")
}
}

use alloc::string::ToString;

use test::wit::host_resource::Testresource;
Expand Down Expand Up @@ -214,6 +220,10 @@ impl test::wit::TestExports<Host> 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<Guest> = Mutex::new(Guest {
Expand Down
Loading