Skip to content

Commit

Permalink
Return anyhow::Error from host functions instead of Trap
Browse files Browse the repository at this point in the history
This commit refactors how errors are modeled when returned from host
functions and additionally refactors how custom errors work with `Trap`.
At a high level functions in Wasmtime that previously worked with
`Result<T, Trap>` now work with `Result<T>` instead where the error is
`anyhow::Error`. This includes functions such as:

* Host-defined functions in a `Linker<T>`
* `TypedFunc::call`
* Host-related callbacks like call hooks

Errors are now modeled primarily as `anyhow::Error` throughout Wasmtime.
This subsequently removes the need for `Trap` to have the ability to
represent all host-defined errors as it previously did. Consequently the
`From` implementations for any error into a `Trap` have been removed
here and the only embedder-defined way to create a `Trap` is to use
`Trap::new` with a custom string.

After this commit the distinction between a `Trap` and a host error is
the wasm backtrace that it contains. Previously all errors in host
functions would flow through a `Trap` and get a wasm backtrace attached
to them, but now this only happens if a `Trap` itself is created meaning
that arbitrary host-defined errors flowing from a host import to the
other side won't get backtraces attached. Some internals of Wasmtime
itself were updated or preserved to use `Trap::new` to capture a
backtrace where it seemed useful, such as when fuel runs out.

The main motivation for this commit is that it now enables hosts to
thread a concrete error type from a host function all the way through to
where a wasm function was invoked. Previously this could not be done
since the host error was wrapped in a `Trap` that didn't provide the
ability to get at the internals.

A consequence of this commit is that when a host error is returned that
isn't a `Trap` we'll capture a backtrace and then won't have a `Trap` to
attach it to. To avoid losing the contextual information this commit
uses the `Error::context` method to attach the backtrace as contextual
information to ensure that the backtrace is itself not lost.

This is a breaking change for likely all users of Wasmtime, but it's
hoped to be a relatively minor change to workaround. Most use cases can
likely change `-> Result<T, Trap>` to `-> Result<T>` and otherwise
explicit creation of a `Trap` is largely no longer necessary.
  • Loading branch information
alexcrichton committed Oct 31, 2022
1 parent 95ecb7e commit 74784cd
Show file tree
Hide file tree
Showing 36 changed files with 505 additions and 539 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 10 additions & 12 deletions crates/c-api/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
wasm_extern_t, wasm_functype_t, wasm_store_t, wasm_val_t, wasm_val_vec_t, wasmtime_error_t,
wasmtime_extern_t, wasmtime_val_t, wasmtime_val_union, CStoreContext, CStoreContextMut,
};
use anyhow::Result;
use std::ffi::c_void;
use std::mem::{self, MaybeUninit};
use std::panic::{self, AssertUnwindSafe};
Expand Down Expand Up @@ -67,7 +68,7 @@ unsafe fn create_function(
let mut out_results: wasm_val_vec_t = vec![wasm_val_t::default(); results.len()].into();
let out = func(&params, &mut out_results);
if let Some(trap) = out {
return Err(trap.trap.clone());
return Err(trap.error);
}

let out_results = out_results.as_slice();
Expand Down Expand Up @@ -152,10 +153,7 @@ pub unsafe extern "C" fn wasm_func_call(
}
ptr::null_mut()
}
Ok(Err(trap)) => match trap.downcast::<Trap>() {
Ok(trap) => Box::into_raw(Box::new(wasm_trap_t::new(trap))),
Err(err) => Box::into_raw(Box::new(wasm_trap_t::new(err.into()))),
},
Ok(Err(err)) => Box::into_raw(Box::new(wasm_trap_t::new(err))),
Err(panic) => {
let trap = if let Some(msg) = panic.downcast_ref::<String>() {
Trap::new(msg)
Expand All @@ -164,7 +162,7 @@ pub unsafe extern "C" fn wasm_func_call(
} else {
Trap::new("rust panic happened")
};
let trap = Box::new(wasm_trap_t::new(trap));
let trap = Box::new(wasm_trap_t::new(trap.into()));
Box::into_raw(trap)
}
}
Expand Down Expand Up @@ -235,7 +233,7 @@ pub(crate) unsafe fn c_callback_to_rust_fn(
callback: wasmtime_func_callback_t,
data: *mut c_void,
finalizer: Option<extern "C" fn(*mut std::ffi::c_void)>,
) -> impl Fn(Caller<'_, crate::StoreData>, &[Val], &mut [Val]) -> Result<(), Trap> {
) -> impl Fn(Caller<'_, crate::StoreData>, &[Val], &mut [Val]) -> Result<()> {
let foreign = crate::ForeignData { data, finalizer };
move |mut caller, params, results| {
drop(&foreign); // move entire foreign into this closure
Expand Down Expand Up @@ -264,7 +262,7 @@ pub(crate) unsafe fn c_callback_to_rust_fn(
out_results.len(),
);
if let Some(trap) = out {
return Err(trap.trap);
return Err(trap.error);
}

// Translate the `wasmtime_val_t` results into the `results` space
Expand Down Expand Up @@ -299,14 +297,14 @@ pub(crate) unsafe fn c_unchecked_callback_to_rust_fn(
callback: wasmtime_func_unchecked_callback_t,
data: *mut c_void,
finalizer: Option<extern "C" fn(*mut std::ffi::c_void)>,
) -> impl Fn(Caller<'_, crate::StoreData>, &mut [ValRaw]) -> Result<(), Trap> {
) -> impl Fn(Caller<'_, crate::StoreData>, &mut [ValRaw]) -> Result<()> {
let foreign = crate::ForeignData { data, finalizer };
move |caller, values| {
drop(&foreign); // move entire foreign into this closure
let mut caller = wasmtime_caller_t { caller };
match callback(foreign.data, &mut caller, values.as_mut_ptr(), values.len()) {
None => Ok(()),
Some(trap) => Err(trap.trap),
Some(trap) => Err(trap.error),
}
}
}
Expand Down Expand Up @@ -350,7 +348,7 @@ pub unsafe extern "C" fn wasmtime_func_call(
}
Ok(Err(trap)) => match trap.downcast::<Trap>() {
Ok(trap) => {
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap)));
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap.into())));
None
}
Err(err) => Some(Box::new(wasmtime_error_t::from(err))),
Expand All @@ -363,7 +361,7 @@ pub unsafe extern "C" fn wasmtime_func_call(
} else {
Trap::new("rust panic happened")
};
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap)));
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap.into())));
None
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/c-api/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub unsafe extern "C" fn wasm_instance_new(
))),
Err(e) => {
if let Some(ptr) = result {
*ptr = Box::into_raw(Box::new(wasm_trap_t::new(e.into())));
*ptr = Box::into_raw(Box::new(wasm_trap_t::new(e)));
}
None
}
Expand Down Expand Up @@ -100,7 +100,7 @@ pub(crate) fn handle_instantiate(
}
Err(e) => match e.downcast::<Trap>() {
Ok(trap) => {
*trap_ptr = Box::into_raw(Box::new(wasm_trap_t::new(trap)));
*trap_ptr = Box::into_raw(Box::new(wasm_trap_t::new(trap.into())));
None
}
Err(e) => Some(Box::new(e.into())),
Expand Down
54 changes: 41 additions & 13 deletions crates/c-api/src/trap.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
use crate::{wasm_frame_vec_t, wasm_instance_t, wasm_name_t, wasm_store_t};
use anyhow::{anyhow, Error};
use once_cell::unsync::OnceCell;
use wasmtime::{Trap, TrapCode};

#[repr(C)]
#[derive(Clone)]
pub struct wasm_trap_t {
pub(crate) trap: Trap,
pub(crate) error: Error,
}

// This is currently only needed for the `wasm_trap_copy` API in the C API.
//
// For now the impl here is "fake it til you make it" since this is losing
// context by only cloning the error string.
impl Clone for wasm_trap_t {
fn clone(&self) -> wasm_trap_t {
wasm_trap_t {
error: anyhow!("{:?}", self.error),
}
}
}

wasmtime_c_api_macros::declare_ref!(wasm_trap_t);

impl wasm_trap_t {
pub(crate) fn new(trap: Trap) -> wasm_trap_t {
wasm_trap_t { trap: trap }
pub(crate) fn new(error: Error) -> wasm_trap_t {
wasm_trap_t { error }
}
}

Expand Down Expand Up @@ -40,7 +52,7 @@ pub extern "C" fn wasm_trap_new(
}
let message = String::from_utf8_lossy(&message[..message.len() - 1]);
Box::new(wasm_trap_t {
trap: Trap::new(message),
error: Trap::new(message).into(),
})
}

Expand All @@ -49,24 +61,28 @@ pub unsafe extern "C" fn wasmtime_trap_new(message: *const u8, len: usize) -> Bo
let bytes = crate::slice_from_raw_parts(message, len);
let message = String::from_utf8_lossy(&bytes);
Box::new(wasm_trap_t {
trap: Trap::new(message),
error: Trap::new(message).into(),
})
}

#[no_mangle]
pub extern "C" fn wasm_trap_message(trap: &wasm_trap_t, out: &mut wasm_message_t) {
let mut buffer = Vec::new();
buffer.extend_from_slice(trap.trap.to_string().as_bytes());
buffer.extend_from_slice(format!("{:?}", trap.error).as_bytes());
buffer.reserve_exact(1);
buffer.push(0);
out.set_buffer(buffer);
}

#[no_mangle]
pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option<Box<wasm_frame_t>> {
if raw.trap.trace().unwrap_or(&[]).len() > 0 {
let trap = match raw.error.downcast_ref::<Trap>() {
Some(trap) => trap,
None => return None,
};
if trap.trace().unwrap_or(&[]).len() > 0 {
Some(Box::new(wasm_frame_t {
trap: raw.trap.clone(),
trap: trap.clone(),
idx: 0,
func_name: OnceCell::new(),
module_name: OnceCell::new(),
Expand All @@ -78,10 +94,14 @@ pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option<Box<wasm_frame_t

#[no_mangle]
pub extern "C" fn wasm_trap_trace(raw: &wasm_trap_t, out: &mut wasm_frame_vec_t) {
let vec = (0..raw.trap.trace().unwrap_or(&[]).len())
let trap = match raw.error.downcast_ref::<Trap>() {
Some(trap) => trap,
None => return out.set_buffer(Vec::new()),
};
let vec = (0..trap.trace().unwrap_or(&[]).len())
.map(|idx| {
Some(Box::new(wasm_frame_t {
trap: raw.trap.clone(),
trap: trap.clone(),
idx,
func_name: OnceCell::new(),
module_name: OnceCell::new(),
Expand All @@ -93,7 +113,11 @@ pub extern "C" fn wasm_trap_trace(raw: &wasm_trap_t, out: &mut wasm_frame_vec_t)

#[no_mangle]
pub extern "C" fn wasmtime_trap_code(raw: &wasm_trap_t, code: &mut i32) -> bool {
match raw.trap.trap_code() {
let trap = match raw.error.downcast_ref::<Trap>() {
Some(trap) => trap,
None => return false,
};
match trap.trap_code() {
Some(c) => {
*code = match c {
TrapCode::StackOverflow => 0,
Expand All @@ -117,7 +141,11 @@ pub extern "C" fn wasmtime_trap_code(raw: &wasm_trap_t, code: &mut i32) -> bool

#[no_mangle]
pub extern "C" fn wasmtime_trap_exit_status(raw: &wasm_trap_t, status: &mut i32) -> bool {
match raw.trap.i32_exit_status() {
let trap = match raw.error.downcast_ref::<Trap>() {
Some(trap) => trap,
None => return false,
};
match trap.i32_exit_status() {
Some(i) => {
*status = i;
true
Expand Down
6 changes: 4 additions & 2 deletions crates/fuzzing/src/oracles/stacks.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::generators::Stacks;
use anyhow::Result;
use wasmtime::*;

/// Run the given `Stacks` test case and assert that the host's view of the Wasm
Expand All @@ -17,7 +18,7 @@ pub fn check_stacks(stacks: Stacks) -> usize {
.func_wrap(
"host",
"check_stack",
|mut caller: Caller<'_, ()>| -> Result<(), Trap> {
|mut caller: Caller<'_, ()>| -> Result<()> {
let fuel = caller
.get_export("fuel")
.expect("should export `fuel`")
Expand All @@ -26,7 +27,7 @@ pub fn check_stacks(stacks: Stacks) -> usize {

let fuel_left = fuel.get(&mut caller).unwrap_i32();
if fuel_left == 0 {
return Err(Trap::new("out of fuel"));
return Err(Trap::new("out of fuel").into());
}

fuel.set(&mut caller, Val::I32(fuel_left - 1)).unwrap();
Expand Down Expand Up @@ -59,6 +60,7 @@ pub fn check_stacks(stacks: Stacks) -> usize {
for input in stacks.inputs().iter().copied() {
log::debug!("input: {}", input);
if let Err(trap) = run.call(&mut store, (input.into(),)) {
let trap = trap.downcast::<Trap>().unwrap();
log::debug!("trap: {}", trap);
let get_stack = instance
.get_typed_func::<(), (u32, u32), _>(&mut store, "get_stack")
Expand Down
9 changes: 5 additions & 4 deletions crates/wasi-common/src/snapshots/preview_0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::sched::{
use crate::snapshots::preview_1::types as snapshot1_types;
use crate::snapshots::preview_1::wasi_snapshot_preview1::WasiSnapshotPreview1 as Snapshot1;
use crate::{Error, ErrorExt, WasiCtx};
use anyhow::Result;
use cap_std::time::Duration;
use std::collections::HashSet;
use std::convert::{TryFrom, TryInto};
Expand All @@ -28,10 +29,10 @@ impl wiggle::GuestErrorType for types::Errno {
}

impl types::UserErrorConversion for WasiCtx {
fn errno_from_error(&mut self, e: Error) -> Result<types::Errno, wasmtime::Trap> {
fn errno_from_error(&mut self, e: Error) -> Result<types::Errno> {
debug!("Error: {:?}", e);
e.try_into()
.map_err(|e| wasmtime::Trap::new(format!("{:?}", e)))
let errno = e.try_into()?;
Ok(errno)
}
}

Expand Down Expand Up @@ -932,7 +933,7 @@ impl wasi_unstable::WasiUnstable for WasiCtx {
Ok(num_results.try_into().expect("results fit into memory"))
}

async fn proc_exit(&mut self, status: types::Exitcode) -> wasmtime::Trap {
async fn proc_exit(&mut self, status: types::Exitcode) -> anyhow::Error {
Snapshot1::proc_exit(self, status).await
}

Expand Down
17 changes: 9 additions & 8 deletions crates/wasi-common/src/snapshots/preview_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
},
Error, ErrorExt, ErrorKind, SystemTimeSpec, WasiCtx,
};
use anyhow::Context;
use anyhow::{Context, Result};
use cap_std::time::{Duration, SystemClock};
use std::convert::{TryFrom, TryInto};
use std::io::{IoSlice, IoSliceMut};
Expand All @@ -35,10 +35,10 @@ impl wiggle::GuestErrorType for types::Errno {
}

impl types::UserErrorConversion for WasiCtx {
fn errno_from_error(&mut self, e: Error) -> Result<types::Errno, wasmtime::Trap> {
fn errno_from_error(&mut self, e: Error) -> Result<types::Errno> {
debug!("Error: {:?}", e);
e.try_into()
.map_err(|e| wasmtime::Trap::new(format!("{:?}", e)))
let errno = e.try_into()?;
Ok(errno)
}
}

Expand Down Expand Up @@ -1214,13 +1214,14 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
Ok(num_results.try_into().expect("results fit into memory"))
}

async fn proc_exit(&mut self, status: types::Exitcode) -> wasmtime::Trap {
async fn proc_exit(&mut self, status: types::Exitcode) -> anyhow::Error {
// Check that the status is within WASI's range.
if status < 126 {
wasmtime::Trap::i32_exit(status as i32)
let trap = if status < 126 {
wasmtime::Trap::i32_exit(status as i32).into()
} else {
wasmtime::Trap::new("exit with invalid exit status outside of [0..126)")
}
};
trap.into()
}

async fn proc_raise(&mut self, _sig: types::Signal) -> Result<(), Error> {
Expand Down
6 changes: 2 additions & 4 deletions crates/wasi-nn/src/witx.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Contains the macro-generated implementation of wasi-nn from the its witx definition file.
use crate::ctx::WasiNnCtx;
use crate::ctx::WasiNnError;
use anyhow::Result;

// Generate the traits and types of wasi-nn in several Rust modules (e.g. `types`).
wiggle::from_witx!({
Expand All @@ -11,10 +12,7 @@ wiggle::from_witx!({
use types::NnErrno;

impl<'a> types::UserErrorConversion for WasiNnCtx {
fn nn_errno_from_wasi_nn_error(
&mut self,
e: WasiNnError,
) -> Result<NnErrno, wiggle::wasmtime_crate::Trap> {
fn nn_errno_from_wasi_nn_error(&mut self, e: WasiNnError) -> Result<NnErrno> {
eprintln!("Host error: {:?}", e);
match e {
WasiNnError::BackendError(_) => unimplemented!(),
Expand Down
4 changes: 2 additions & 2 deletions crates/wasmtime/src/component/func/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ where
// the component is disallowed, for example, when the `realloc` function
// calls a canonical import.
if !flags.may_leave() {
bail!("cannot leave component instance");
return Err(Trap::new("cannot leave component instance").into());
}

// There's a 2x2 matrix of whether parameters and results are stored on the
Expand Down Expand Up @@ -303,7 +303,7 @@ where
// the component is disallowed, for example, when the `realloc` function
// calls a canonical import.
if !flags.may_leave() {
bail!("cannot leave component instance");
return Err(Trap::new("cannot leave component instance").into());
}

let args;
Expand Down
Loading

0 comments on commit 74784cd

Please sign in to comment.