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
95 changes: 57 additions & 38 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};
use wasmtime::{BacktraceContext, Trap};

#[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: Error::msg(message.into_owned()),
})
}

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: Error::msg(message.into_owned()),
})
}

#[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::<BacktraceContext>() {
Some(trap) => trap,
None => return None,
};
if trap.frames().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,37 +113,36 @@ 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() {
Some(c) => {
*code = match c {
TrapCode::StackOverflow => 0,
TrapCode::MemoryOutOfBounds => 1,
TrapCode::HeapMisaligned => 2,
TrapCode::TableOutOfBounds => 3,
TrapCode::IndirectCallToNull => 4,
TrapCode::BadSignature => 5,
TrapCode::IntegerOverflow => 6,
TrapCode::IntegerDivisionByZero => 7,
TrapCode::BadConversionToInteger => 8,
TrapCode::UnreachableCodeReached => 9,
TrapCode::Interrupt => 10,
_ => unreachable!(),
};
true
}
None => false,
}
let trap = match raw.error.downcast_ref::<Trap>() {
Some(trap) => trap,
None => return false,
};
*code = match trap {
Trap::StackOverflow => 0,
Trap::MemoryOutOfBounds => 1,
Trap::HeapMisaligned => 2,
Trap::TableOutOfBounds => 3,
Trap::IndirectCallToNull => 4,
Trap::BadSignature => 5,
Trap::IntegerOverflow => 6,
Trap::IntegerDivisionByZero => 7,
Trap::BadConversionToInteger => 8,
Trap::UnreachableCodeReached => 9,
Trap::Interrupt => 10,
_ => unreachable!(),
};
true
}

#[no_mangle]
pub extern "C" fn wasmtime_trap_exit_status(raw: &wasm_trap_t, status: &mut i32) -> bool {
match raw.trap.i32_exit_status() {
Some(i) => {
*status = i;
true
}
None => false,
#[cfg(feature = "wasi")]
if let Some(exit) = raw.error.downcast_ref::<wasmtime_wasi::I32Exit>() {
*status = exit.0;
return true;
}

false
}

#[no_mangle]
Expand Down
14 changes: 4 additions & 10 deletions crates/fuzzing/src/oracles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,7 @@ pub fn differential(
// falls through to checking the intermediate state otherwise.
(Err(lhs), Err(rhs)) => {
let err = rhs.downcast::<Trap>().expect("not a trap");
let poisoned = err.trap_code() == Some(TrapCode::StackOverflow)
|| lhs_engine.is_stack_overflow(&lhs);
let poisoned = err == Trap::StackOverflow || lhs_engine.is_stack_overflow(&lhs);

if poisoned {
return Ok(false);
Expand Down Expand Up @@ -675,14 +674,9 @@ pub fn table_ops(
.downcast::<Trap>()
.unwrap();

match trap.trap_code() {
Some(TrapCode::TableOutOfBounds) => {}
None if trap
.to_string()
.contains("all fuel consumed by WebAssembly") => {}
_ => {
panic!("unexpected trap: {}", trap);
}
match trap {
Trap::TableOutOfBounds | Trap::OutOfFuel => {}
_ => panic!("unexpected trap: {trap}"),
}

// Do a final GC after running the Wasm.
Expand Down
Loading