T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixes #2418: Enhance wiggle to generate its UserErrorConverstion trait with a function that returns Result<abi_err, String> (#2419)

Browse Source 
* Enhance wiggle to generate its UserErrorConverstion trait with a function that returns
a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.

* Enhance wiggle to generate its UserErrorConverstion trait with a function that returns
a Result<abi_err, String>.  This enhancement allows hostcall implementations using wiggle
to return an actionable error to the instance (the abi_err) or to terminate the instance
using the String as fatal error information.

* Enhance the wiggle/wasmtime integration to leverage new work in ab7e9c6.  Hostcall
implementations generated by wiggle now return an Result<abi_error, Trap>.  As a
result, hostcalls experiencing fatal errors may trap, thereby terminating the
wasmtime instance.  This enhancement has been performed for both wasi snapshot1
and wasi snapshot0.

* Update wasi-nn crate to reflect enhancement in issue #2418.

* Update wiggle test-helpers for wiggle enhancement made in issue #2418.

* Address PR feedback; omit verbose return statement.

* Address PR feedback; manually format within a proc macro.

* Address PR feedback; manually format proc macro.

* Restore return statements to wasi.rs.

* Restore return statements in funcs.rs.

* Address PR feedback; omit TODO and fix formatting.

* Ok-wrap error type in assert statement.
This commit is contained in:
Tanya L. Crenshaw
2020-11-24 12:06:57 -08:00
committed by GitHub
parent 128c3bd749
commit b06ed39c1e
18 changed files with 84 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
crates/wasi-common/src/wasi.rs
View File

@@ -23,9 +23,9 @@ impl types::GuestErrorConversion for WasiCtx {
}
impl types::UserErrorConversion for WasiCtx {
fn errno_from_error(&self, e: Error) -> Errno {
fn errno_from_error(&self, e: Error) -> Result<Errno, String> {
debug!("Error: {:?}", e);
e.into()
Ok(e.into())
}
}

30
crates/wasi-common/wig/src/hostcalls.rs
View File

@@ -13,8 +13,6 @@ pub fn define(args: TokenStream) -> TokenStream {
let mut ret = TokenStream::new();
let old = true;
for module in doc.modules() {
for func in module.funcs() {
// `proc_exit` is special; it's essentially an unwinding primitive,
@@ -24,14 +22,14 @@ pub fn define(args: TokenStream) -> TokenStream {
continue;
}
ret.extend(generate_wrappers(&func, old));
ret.extend(generate_wrappers(&func));
}
}
return ret;
}
fn generate_wrappers(func: &witx::InterfaceFunc, old: bool) -> TokenStream {
fn generate_wrappers(func: &witx::InterfaceFunc) -> TokenStream {
let name = format_ident!("{}", func.name.as_str());
let mut arg_declarations = Vec::new();
let mut arg_names = Vec::new();
@@ -101,33 +99,13 @@ fn generate_wrappers(func: &witx::InterfaceFunc, old: bool) -> TokenStream {
}
};
let c_abi_name = if old {
format_ident!("old_wasi_common_{}", name)
} else {
format_ident!("wasi_common_{}", name)
};
quote! {
pub unsafe fn #name(
wasi_ctx: &mut super::WasiCtx,
memory: &mut [u8],
#(#arg_declarations,)*
) -> #ret {
#body
}
#[no_mangle]
pub unsafe fn #c_abi_name(
wasi_ctx: *mut super::WasiCtx,
memory: *mut u8,
memory_len: usize,
#(#arg_declarations,)*
) -> #ret {
#name(
&mut *wasi_ctx,
std::slice::from_raw_parts_mut(memory, memory_len),
#(#arg_names,)*
)
) -> Result<#ret, String> {
Ok({#body})
}
}
}

12
crates/wasi-common/wig/src/wasi.rs
View File

@@ -168,7 +168,7 @@ pub fn define_struct(args: TokenStream) -> TokenStream {
// The first result is returned bare right now...
if let Some(ret) = results.next() {
handle_early_error = quote! { return e.into() };
handle_early_error = quote! { return Ok(e.into()) };
match &*ret.tref.type_() {
// Eventually we'll want to add support for more returned
// types, but for now let's just conform to what `*.witx`
@@ -200,7 +200,7 @@ pub fn define_struct(args: TokenStream) -> TokenStream {
let my_cx = cx.clone();
let #name_ident = wasmtime::Func::wrap(
store,
move |caller: wasmtime::Caller<'_> #(,#shim_arg_decls)*| -> #ret_ty {
move |caller: wasmtime::Caller<'_> #(,#shim_arg_decls)*| -> Result<#ret_ty, wasmtime::Trap> {
tracing::trace!(
#format_str,
#(#format_args),*
@@ -214,11 +214,15 @@ pub fn define_struct(args: TokenStream) -> TokenStream {
#handle_early_error
}
};
hostcalls::#name_ident(
let result = hostcalls::#name_ident(
&mut my_cx.borrow_mut(),
memory.data_unchecked_mut(),
#(#hostcall_args),*
) #cvt_ret
);
match result {
Ok(r) => { return Ok(r #cvt_ret); },
Err(err) => { return Err(wasmtime::Trap::new(err)); },
}
}
}
);

2
crates/wasi-nn/src/witx.rs
View File

@@ -22,7 +22,7 @@ impl types::GuestErrorConversion for WasiNnCtx {
}
impl<'a> types::UserErrorConversion for WasiNnCtx {
fn errno_from_wasi_nn_error(&self, e: WasiNnError) -> Errno {
fn errno_from_wasi_nn_error(&self, e: WasiNnError) -> Result<Errno, String> {
eprintln!("Host error: {:?}", e);
match e {
WasiNnError::OpenvinoError(_) => unimplemented!(),

15
crates/wiggle/generate/src/funcs.rs
View File

@@ -55,7 +55,7 @@ pub fn define_func(
let method = names.user_error_conversion_method(&user_err);
quote!(UserErrorConversion::#method(ctx, e))
} else {
quote!(e)
quote!(Ok(e))
};
quote! {
let e = #conversion;
@@ -63,7 +63,10 @@ pub fn define_func(
#rt::tracing::Level::TRACE,
#name = #rt::tracing::field::debug(&e),
);
return #abi_ret::from(e);
match e {
Ok(e) => { return Ok(#abi_ret::from(e)); },
Err(e) => { return Err(e); },
}
}
})
.unwrap_or_else(|| quote!(()));
@@ -79,7 +82,7 @@ pub fn define_func(
quote! {
let e = #rt::GuestError::InFunc { funcname: #funcname, location: #location, err: Box::new(e.into()) };
let err: #err_typename = GuestErrorConversion::#err_method(ctx, e);
return #abi_ret::from(err);
return Ok(#abi_ret::from(err));
}
} else {
quote! {
@@ -161,17 +164,17 @@ pub fn define_func(
#rt::tracing::Level::TRACE,
success=#rt::tracing::field::display(&success)
);
#abi_ret::from(success)
Ok(#abi_ret::from(success))
}
} else {
quote!()
quote!(Ok(()))
};
let trait_name = names.trait_name(&module.name);
let mod_name = &module.name.as_str();
let func_name = &func.name.as_str();
quote!(pub fn #ident(#abi_args) -> #abi_ret {
quote!(pub fn #ident(#abi_args) -> Result<#abi_ret, String> {
let _span = #rt::tracing::span!(
#rt::tracing::Level::TRACE,
"wiggle abi",

2
crates/wiggle/generate/src/lib.rs
View File

@@ -40,7 +40,7 @@ pub fn generate(doc: &witx::Document, names: &Names, errs: &ErrorTransform) -> T
let abi_typename = names.type_ref(&errtype.abi_type(), anon_lifetime());
let user_typename = errtype.typename();
let methodname = names.user_error_conversion_method(&errtype);
quote!(fn #methodname(&self, e: super::#user_typename) -> #abi_typename;)
quote!(fn #methodname(&self, e: super::#user_typename) -> Result<#abi_typename, String>;)
});
let user_error_conversion = quote! {
pub trait UserErrorConversion {

12
crates/wiggle/test-helpers/examples/tracing.rs
View File

@@ -36,7 +36,7 @@ impl_errno!(types::Errno, types::GuestErrorConversion);
/// When the `errors` mapping in witx is non-empty, we need to impl the
/// types::UserErrorConversion trait that wiggle generates from that mapping.
impl<'a> types::UserErrorConversion for WasiCtx<'a> {
fn errno_from_rich_error(&self, e: RichError) -> types::Errno {
fn errno_from_rich_error(&self, e: RichError) -> Result<types::Errno, String> {
wiggle::tracing::debug!(
rich_error = wiggle::tracing::field::debug(&e),
"error conversion"
@@ -46,8 +46,8 @@ impl<'a> types::UserErrorConversion for WasiCtx<'a> {
self.log.borrow_mut().push(e.to_string());
// Then do the trivial mapping down to the flat enum.
match e {
RichError::InvalidArg { .. } => types::Errno::InvalidArg,
RichError::PicketLine { .. } => types::Errno::PicketLine,
RichError::InvalidArg { .. } => Ok(types::Errno::InvalidArg),
RichError::PicketLine { .. } => Ok(types::Errno::PicketLine),
}
}
}
@@ -90,7 +90,7 @@ fn main() {
let r0 = one_error_conversion::foo(&ctx, &host_memory, 0, 0, 8);
assert_eq!(
r0,
i32::from(types::Errno::Ok),
Ok(i32::from(types::Errno::Ok)),
"Expected return value for strike=0"
);
assert!(ctx.log.borrow().is_empty(), "No error log for strike=0");
@@ -99,7 +99,7 @@ fn main() {
let r1 = one_error_conversion::foo(&ctx, &host_memory, 1, 0, 8);
assert_eq!(
r1,
i32::from(types::Errno::PicketLine),
Ok(i32::from(types::Errno::PicketLine)),
"Expected return value for strike=1"
);
assert_eq!(
@@ -112,7 +112,7 @@ fn main() {
let r2 = one_error_conversion::foo(&ctx, &host_memory, 2, 0, 8);
assert_eq!(
r2,
i32::from(types::Errno::InvalidArg),
Ok(i32::from(types::Errno::InvalidArg)),
"Expected return value for strike=2"
);
assert_eq!(

14
crates/wiggle/tests/arrays.rs
View File

@@ -105,7 +105,7 @@ impl ReduceExcusesExcercise {
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "reduce excuses errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "reduce excuses errno");
let expected = *self
.excuse_values
@@ -183,7 +183,7 @@ impl PopulateExcusesExcercise {
self.array_ptr_loc.ptr as i32,
self.elements.len() as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "populate excuses errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "populate excuses errno");
let arr: GuestPtr<'_, [GuestPtr<'_, types::Excuse>]> =
host_memory.ptr((self.array_ptr_loc.ptr, self.elements.len() as u32));
@@ -309,7 +309,7 @@ impl SumElementsExercise {
self.start_ix as i32,
self.return_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "sum_of_element errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "sum_of_element errno");
let result_ptr = host_memory.ptr::<i32>(self.return_loc.ptr);
let result = result_ptr.read().expect("read result");
@@ -330,7 +330,7 @@ impl SumElementsExercise {
);
assert_eq!(
res,
types::Errno::InvalidArg.into(),
Ok(types::Errno::InvalidArg.into()),
"out of bounds sum_of_element errno"
);
@@ -346,7 +346,7 @@ impl SumElementsExercise {
if self.start_ix <= self.end_ix {
assert_eq!(
res,
types::Errno::Ok.into(),
Ok(types::Errno::Ok.into()),
"expected ok sum_of_elements errno"
);
let result_ptr = host_memory.ptr::<i32>(self.return_loc.ptr);
@@ -367,7 +367,7 @@ impl SumElementsExercise {
} else {
assert_eq!(
res,
types::Errno::InvalidArg.into(),
Ok(types::Errno::InvalidArg.into()),
"expected error out-of-bounds sum_of_elements"
);
}
@@ -384,7 +384,7 @@ impl SumElementsExercise {
);
assert_eq!(
res,
types::Errno::InvalidArg.into(),
Ok(types::Errno::InvalidArg.into()),
"out of bounds sum_of_elements errno"
);
}

4
crates/wiggle/tests/atoms.rs
View File

@@ -34,7 +34,7 @@ impl IntFloatExercise {
let e = atoms::int_float_args(&ctx, &host_memory, self.an_int as i32, self.an_float);
assert_eq!(e, types::Errno::Ok.into(), "int_float_args error");
assert_eq!(e, Ok(types::Errno::Ok.into()), "int_float_args error");
}
pub fn strat() -> BoxedStrategy<Self> {
@@ -72,7 +72,7 @@ impl DoubleIntExercise {
.ptr::<types::AliasToFloat>(self.return_loc.ptr)
.read()
.expect("failed to read return");
assert_eq!(e, types::Errno::Ok.into(), "errno");
assert_eq!(e, Ok(types::Errno::Ok.into()), "errno");
assert_eq!(return_val, (self.input as f32) * 2.0, "return val");
}

19
crates/wiggle/tests/errors.rs
View File

@@ -34,14 +34,14 @@ mod convert_just_errno {
/// When the `errors` mapping in witx is non-empty, we need to impl the
/// types::UserErrorConversion trait that wiggle generates from that mapping.
impl<'a> types::UserErrorConversion for WasiCtx<'a> {
fn errno_from_rich_error(&self, e: RichError) -> types::Errno {
fn errno_from_rich_error(&self, e: RichError) -> Result<types::Errno, String> {
// WasiCtx can collect a Vec<String> log so we can test this. We're
// logging the Display impl that `thiserror::Error` provides us.
self.log.borrow_mut().push(e.to_string());
// Then do the trivial mapping down to the flat enum.
match e {
RichError::InvalidArg { .. } => types::Errno::InvalidArg,
RichError::PicketLine { .. } => types::Errno::PicketLine,
RichError::InvalidArg { .. } => Ok(types::Errno::InvalidArg),
RichError::PicketLine { .. } => Ok(types::Errno::PicketLine),
}
}
}
@@ -68,7 +68,7 @@ mod convert_just_errno {
let r0 = one_error_conversion::foo(&ctx, &host_memory, 0);
assert_eq!(
r0,
i32::from(types::Errno::Ok),
Ok(i32::from(types::Errno::Ok)),
"Expected return value for strike=0"
);
assert!(ctx.log.borrow().is_empty(), "No error log for strike=0");
@@ -77,7 +77,7 @@ mod convert_just_errno {
let r1 = one_error_conversion::foo(&ctx, &host_memory, 1);
assert_eq!(
r1,
i32::from(types::Errno::PicketLine),
Ok(i32::from(types::Errno::PicketLine)),
"Expected return value for strike=1"
);
assert_eq!(
@@ -90,7 +90,7 @@ mod convert_just_errno {
let r2 = one_error_conversion::foo(&ctx, &host_memory, 2);
assert_eq!(
r2,
i32::from(types::Errno::InvalidArg),
Ok(i32::from(types::Errno::InvalidArg)),
"Expected return value for strike=2"
);
assert_eq!(
@@ -159,10 +159,13 @@ mod convert_multiple_error_types {
// each member of the `errors` mapping.
// Bodies elided.
impl<'a> types::UserErrorConversion for WasiCtx<'a> {
fn errno_from_rich_error(&self, _e: RichError) -> types::Errno {
fn errno_from_rich_error(&self, _e: RichError) -> Result<types::Errno, String> {
unimplemented!()
}
fn errno2_from_another_rich_error(&self, _e: AnotherRichError) -> types::Errno2 {
fn errno2_from_another_rich_error(
&self,
_e: AnotherRichError,
) -> Result<types::Errno2, String> {
unimplemented!()
}
}

2
crates/wiggle/tests/flags.rs
View File

@@ -79,7 +79,7 @@ impl ConfigureCarExercise {
self.other_config_by_ptr.ptr as i32,
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "configure car errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "configure car errno");
let res_config = host_memory
.ptr::<types::CarConfig>(self.return_ptr_loc.ptr)

6
crates/wiggle/tests/handles.rs
View File

@@ -37,7 +37,7 @@ impl HandleExercise {
let e = handle_examples::fd_create(&ctx, &host_memory, self.return_loc.ptr as i32);
assert_eq!(e, types::Errno::Ok.into(), "fd_create error");
assert_eq!(e, Ok(types::Errno::Ok.into()), "fd_create error");
let h_got: u32 = host_memory
.ptr(self.return_loc.ptr)
@@ -48,13 +48,13 @@ impl HandleExercise {
let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32);
assert_eq!(e, types::Errno::Ok.into(), "fd_consume error");
assert_eq!(e, Ok(types::Errno::Ok.into()), "fd_consume error");
let e = handle_examples::fd_consume(&ctx, &host_memory, h_got as i32 + 1);
assert_eq!(
e,
types::Errno::InvalidArg.into(),
Ok(types::Errno::InvalidArg.into()),
"fd_consume invalid error"
);
}

2
crates/wiggle/tests/ints.rs
View File

@@ -53,7 +53,7 @@ impl CookieCutterExercise {
self.cookie.into(),
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "cookie cutter errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "cookie cutter errno");
let is_cookie_start = host_memory
.ptr::<types::Bool>(self.return_ptr_loc.ptr)

2
crates/wiggle/tests/pointers.rs
View File

@@ -157,7 +157,7 @@ impl PointersAndEnumsExercise {
self.input3_loc.ptr as i32,
self.input4_ptr_loc.ptr as i32,
);
assert_eq!(e, types::Errno::Ok.into(), "errno");
assert_eq!(e, Ok(types::Errno::Ok.into()), "errno");
// Implementation of pointers_and_enums writes input3 to the input2_loc:
let written_to_input2_loc: i32 = host_memory

6
crates/wiggle/tests/strings.rs
View File

@@ -88,7 +88,7 @@ impl HelloStringExercise {
self.test_word.len() as i32,
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "hello string errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "hello string errno");
let given = host_memory
.ptr::<u32>(self.return_ptr_loc.ptr)
@@ -209,7 +209,7 @@ impl MultiStringExercise {
self.c.len() as i32,
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "multi string errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "multi string errno");
let given = host_memory
.ptr::<u32>(self.return_ptr_loc.ptr)
@@ -287,7 +287,7 @@ impl OverlappingStringExercise {
a_len - self.offset_c as i32,
self.return_ptr_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "multi string errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "multi string errno");
let given = host_memory
.ptr::<u32>(self.return_ptr_loc.ptr)

20
crates/wiggle/tests/structs.rs
View File

@@ -118,7 +118,7 @@ impl SumOfPairExercise {
self.return_loc.ptr as i32,
);
assert_eq!(sum_err, types::Errno::Ok.into(), "sum errno");
assert_eq!(sum_err, Ok(types::Errno::Ok.into()), "sum errno");
let return_val: i64 = host_memory
.ptr(self.return_loc.ptr)
@@ -216,7 +216,11 @@ impl SumPairPtrsExercise {
self.return_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "sum of pair of ptrs errno");
assert_eq!(
res,
Ok(types::Errno::Ok.into()),
"sum of pair of ptrs errno"
);
let doubled: i64 = host_memory
.ptr(self.return_loc.ptr)
@@ -295,7 +299,7 @@ impl SumIntAndPtrExercise {
self.return_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "sum of int and ptr errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "sum of int and ptr errno");
let doubled: i64 = host_memory
.ptr(self.return_loc.ptr)
@@ -334,7 +338,7 @@ impl ReturnPairInts {
let err = structs::return_pair_ints(&ctx, &host_memory, self.return_loc.ptr as i32);
assert_eq!(err, types::Errno::Ok.into(), "return struct errno");
assert_eq!(err, Ok(types::Errno::Ok.into()), "return struct errno");
let return_struct: types::PairInts = host_memory
.ptr(self.return_loc.ptr)
@@ -414,7 +418,11 @@ impl ReturnPairPtrsExercise {
self.return_loc.ptr as i32,
);
assert_eq!(res, types::Errno::Ok.into(), "return pair of ptrs errno");
assert_eq!(
res,
Ok(types::Errno::Ok.into()),
"return pair of ptrs errno"
);
let ptr_pair_int_ptrs: types::PairIntPtrs<'_> = host_memory
.ptr(self.return_loc.ptr)
@@ -522,7 +530,7 @@ impl SumArrayExercise {
);
// should be no error - if hostcall did a GuestError it should eprintln it.
assert_eq!(res, types::Errno::Ok.into(), "reduce excuses errno");
assert_eq!(res, Ok(types::Errno::Ok.into()), "reduce excuses errno");
// Sum is inputs upcasted to u16
let expected: u16 = self.inputs.iter().map(|v| *v as u16).sum();

4
crates/wiggle/tests/union.rs
View File

@@ -133,7 +133,7 @@ impl GetTagExercise {
self.return_loc.ptr as i32,
);
assert_eq!(e, types::Errno::Ok.into(), "get_tag errno");
assert_eq!(e, Ok(types::Errno::Ok.into()), "get_tag errno");
let return_val: types::Excuse = host_memory
.ptr(self.return_loc.ptr)
@@ -217,7 +217,7 @@ impl ReasonMultExercise {
self.multiply_by as i32,
);
assert_eq!(e, types::Errno::Ok.into(), "reason_mult errno");
assert_eq!(e, Ok(types::Errno::Ok.into()), "reason_mult errno");
match self.input {
types::Reason::DogAte(f) => {

12
crates/wiggle/wasmtime/macro/src/lib.rs
View File

@@ -185,7 +185,7 @@ fn generate_func(
witx::CoreParamSignifies::Value(atom) => names.atom_type(atom),
_ => unreachable!("coretype ret should always be passed by value"),
};
(quote! { #ret_ty }, quote! { return e.into(); })
(quote! { #ret_ty }, quote! { return Ok(e.into()); })
} else {
(
quote! {()},
@@ -199,7 +199,7 @@ fn generate_func(
let my_cx = cx.clone();
let #name_ident = wasmtime::Func::wrap(
store,
move |caller: wasmtime::Caller<'_> #(,#arg_decls)*| -> #ret_ty {
move |caller: wasmtime::Caller<'_> #(,#arg_decls)*| -> Result<#ret_ty, wasmtime::Trap> {
unsafe {
let mem = match caller.get_export("memory") {
Some(wasmtime::Extern::Memory(m)) => m,
@@ -210,11 +210,15 @@ fn generate_func(
}
};
let mem = #runtime::WasmtimeGuestMemory::new(mem);
#target_module::#name_ident(
let result = #target_module::#name_ident(
&mut my_cx.borrow_mut(),
&mem,
#(#arg_names),*
)
);
match result {
Ok(r) => {return Ok(r.into());},
Err(err) => { return Err(wasmtime::Trap::new(err)); },
}
}
}
);