Skip to content
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.

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

Return anyhow::Error from host functions instead of Trap, redesign Trap #5149

Merged
merged 12 commits into from
Nov 2, 2022
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.

4 changes: 2 additions & 2 deletions cranelift/codegen/src/ir/immediates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ impl FromStr for Offset32 {
/// containing the bit pattern.
///
/// We specifically avoid using a f32 here since some architectures may silently alter floats.
/// See: https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646
/// See: <https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646>
///
/// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but
/// [PartialOrd] respects IEEE754 semantics.
Expand All @@ -488,7 +488,7 @@ pub struct Ieee32(u32);
/// containing the bit pattern.
///
/// We specifically avoid using a f64 here since some architectures may silently alter floats.
/// See: https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646
/// See: <https://github.com/bytecodealliance/wasmtime/pull/2251#discussion_r498508646>
///
/// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but
/// [PartialOrd] respects IEEE754 semantics.
Expand Down
17 changes: 8 additions & 9 deletions crates/bench-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@
mod unsafe_send_sync;

use crate::unsafe_send_sync::UnsafeSendSync;
use anyhow::{anyhow, Context, Result};
use anyhow::{Context, Result};
use std::os::raw::{c_int, c_void};
use std::slice;
use std::{env, path::PathBuf};
use target_lexicon::Triple;
use wasmtime::{Config, Engine, Instance, Linker, Module, Store};
use wasmtime_cli_flags::{CommonOptions, WasiModules};
use wasmtime_wasi::{sync::WasiCtxBuilder, WasiCtx};
use wasmtime_wasi::{sync::WasiCtxBuilder, I32Exit, WasiCtx};

pub type ExitCode = c_int;
pub const OK: ExitCode = 0;
Expand Down Expand Up @@ -539,14 +539,13 @@ impl BenchState {
Err(trap) => {
// Since _start will likely return by using the system `exit` call, we must
// check the trap code to see if it actually represents a successful exit.
match trap.i32_exit_status() {
Some(0) => Ok(()),
Some(n) => Err(anyhow!("_start exited with a non-zero code: {}", n)),
None => Err(anyhow!(
"executing the benchmark resulted in a trap: {}",
trap
)),
if let Some(exit) = trap.downcast_ref::<I32Exit>() {
if exit.0 == 0 {
return Ok(());
}
}

Err(trap)
}
}
}
Expand Down
56 changes: 27 additions & 29 deletions crates/c-api/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ 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::{Error, Result};
use std::any::Any;
use std::ffi::c_void;
use std::mem::{self, MaybeUninit};
use std::panic::{self, AssertUnwindSafe};
Expand Down Expand Up @@ -67,7 +69,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,24 +154,25 @@ 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)
} else if let Some(msg) = panic.downcast_ref::<&'static str>() {
Trap::new(*msg)
} else {
Trap::new("rust panic happened")
};
let trap = Box::new(wasm_trap_t::new(trap));
let err = error_from_panic(panic);
let trap = Box::new(wasm_trap_t::new(err));
Box::into_raw(trap)
}
}
}

fn error_from_panic(panic: Box<dyn Any + Send>) -> Error {
if let Some(msg) = panic.downcast_ref::<String>() {
Error::msg(msg.clone())
} else if let Some(msg) = panic.downcast_ref::<&'static str>() {
Error::msg(*msg)
} else {
Error::msg("rust panic happened")
}
}

#[no_mangle]
pub unsafe extern "C" fn wasm_func_type(f: &wasm_func_t) -> Box<wasm_functype_t> {
Box::new(wasm_functype_t::new(f.func().ty(f.ext.store.context())))
Expand Down Expand Up @@ -235,7 +238,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 +267,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 +302,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 @@ -348,22 +351,17 @@ pub unsafe extern "C" fn wasmtime_func_call(
store.data_mut().wasm_val_storage = params;
None
}
Ok(Err(trap)) => match trap.downcast::<Trap>() {
Ok(trap) => {
Ok(Err(trap)) => {
if trap.is::<Trap>() {
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap)));
None
} else {
Some(Box::new(wasmtime_error_t::from(trap)))
}
Err(err) => Some(Box::new(wasmtime_error_t::from(err))),
},
}
Err(panic) => {
let trap = if let Some(msg) = panic.downcast_ref::<String>() {
Trap::new(msg)
} else if let Some(msg) = panic.downcast_ref::<&'static str>() {
Trap::new(*msg)
} else {
Trap::new("rust panic happened")
};
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap)));
let err = error_from_panic(panic);
*trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(err)));
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
Loading