From 7ca01944c233a1d8ffb535b2bc55a7ee6227a499 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 28 Oct 2022 08:03:07 -0700 Subject: [PATCH 01/12] Return `anyhow::Error` from host functions instead of `Trap` 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` now work with `Result` instead where the error is `anyhow::Error`. This includes functions such as: * Host-defined functions in a `Linker` * `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` to `-> Result` and otherwise explicit creation of a `Trap` is largely no longer necessary. --- Cargo.lock | 1 + crates/c-api/src/func.rs | 22 +- crates/c-api/src/instance.rs | 4 +- crates/c-api/src/trap.rs | 54 ++- crates/fuzzing/src/oracles/stacks.rs | 6 +- crates/wasi-common/src/snapshots/preview_0.rs | 9 +- crates/wasi-common/src/snapshots/preview_1.rs | 17 +- crates/wasi-nn/src/witx.rs | 6 +- crates/wasmtime/src/component/func/host.rs | 4 +- crates/wasmtime/src/func.rs | 84 ++-- crates/wasmtime/src/func/typed.rs | 12 +- crates/wasmtime/src/lib.rs | 6 +- crates/wasmtime/src/linker.rs | 10 +- crates/wasmtime/src/store.rs | 56 +-- crates/wasmtime/src/trampoline/func.rs | 4 +- crates/wasmtime/src/trap.rs | 416 +++++++++--------- crates/wiggle/generate/src/funcs.rs | 4 +- crates/wiggle/generate/src/lib.rs | 2 +- crates/wiggle/generate/src/module_trait.rs | 2 +- crates/wiggle/generate/src/wasmtime.rs | 8 +- crates/wiggle/src/lib.rs | 8 - crates/wiggle/test-helpers/Cargo.toml | 1 + .../wiggle/test-helpers/examples/tracing.rs | 6 +- crates/wiggle/tests/errors.rs | 16 +- crates/wiggle/tests/wasi.rs | 2 +- examples/interrupt.rs | 6 +- tests/all/async_functions.rs | 11 +- tests/all/call_hook.rs | 58 +-- tests/all/component_model/import.rs | 18 +- tests/all/component_model/strings.rs | 9 +- tests/all/func.rs | 37 +- tests/all/host_funcs.rs | 25 +- tests/all/iloop.rs | 8 +- tests/all/import_calling_export.rs | 7 +- tests/all/linker.rs | 2 +- tests/all/traps.rs | 103 +++-- 36 files changed, 505 insertions(+), 539 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 114e632f6ad8..11948cb718fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3876,6 +3876,7 @@ dependencies = [ name = "wiggle-test" version = "0.21.0" dependencies = [ + "anyhow", "env_logger 0.9.0", "proptest", "thiserror", diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index 3d3f5ee3cf3a..79024fd495bc 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -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}; @@ -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(¶ms, &mut out_results); if let Some(trap) = out { - return Err(trap.trap.clone()); + return Err(trap.error); } let out_results = out_results.as_slice(); @@ -152,10 +153,7 @@ pub unsafe extern "C" fn wasm_func_call( } ptr::null_mut() } - Ok(Err(trap)) => match trap.downcast::() { - 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::() { Trap::new(msg) @@ -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) } } @@ -235,7 +233,7 @@ pub(crate) unsafe fn c_callback_to_rust_fn( callback: wasmtime_func_callback_t, data: *mut c_void, finalizer: Option, -) -> 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 @@ -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 @@ -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, -) -> 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), } } } @@ -350,7 +348,7 @@ pub unsafe extern "C" fn wasmtime_func_call( } Ok(Err(trap)) => match trap.downcast::() { 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))), @@ -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 } } diff --git a/crates/c-api/src/instance.rs b/crates/c-api/src/instance.rs index 4897520bedc4..2f93661ecddd 100644 --- a/crates/c-api/src/instance.rs +++ b/crates/c-api/src/instance.rs @@ -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 } @@ -100,7 +100,7 @@ pub(crate) fn handle_instantiate( } Err(e) => match e.downcast::() { 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())), diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 6f60709fe4e6..cf91ce152118 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -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 } } } @@ -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(), }) } @@ -49,14 +61,14 @@ 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); @@ -64,9 +76,13 @@ pub extern "C" fn wasm_trap_message(trap: &wasm_trap_t, out: &mut wasm_message_t #[no_mangle] pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option> { - if raw.trap.trace().unwrap_or(&[]).len() > 0 { + let trap = match raw.error.downcast_ref::() { + 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(), @@ -78,10 +94,14 @@ pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option() { + 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(), @@ -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::() { + Some(trap) => trap, + None => return false, + }; + match trap.trap_code() { Some(c) => { *code = match c { TrapCode::StackOverflow => 0, @@ -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::() { + Some(trap) => trap, + None => return false, + }; + match trap.i32_exit_status() { Some(i) => { *status = i; true diff --git a/crates/fuzzing/src/oracles/stacks.rs b/crates/fuzzing/src/oracles/stacks.rs index 54dde5801fba..810f1eb7b1da 100644 --- a/crates/fuzzing/src/oracles/stacks.rs +++ b/crates/fuzzing/src/oracles/stacks.rs @@ -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 @@ -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`") @@ -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(); @@ -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::().unwrap(); log::debug!("trap: {}", trap); let get_stack = instance .get_typed_func::<(), (u32, u32), _>(&mut store, "get_stack") diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs index 3fbf9379bbb5..d04b471c1847 100644 --- a/crates/wasi-common/src/snapshots/preview_0.rs +++ b/crates/wasi-common/src/snapshots/preview_0.rs @@ -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}; @@ -28,10 +29,10 @@ impl wiggle::GuestErrorType for types::Errno { } impl types::UserErrorConversion for WasiCtx { - fn errno_from_error(&mut self, e: Error) -> Result { + fn errno_from_error(&mut self, e: Error) -> Result { debug!("Error: {:?}", e); - e.try_into() - .map_err(|e| wasmtime::Trap::new(format!("{:?}", e))) + let errno = e.try_into()?; + Ok(errno) } } @@ -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 } diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 664a0508e1c6..19a67affd16a 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -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}; @@ -35,10 +35,10 @@ impl wiggle::GuestErrorType for types::Errno { } impl types::UserErrorConversion for WasiCtx { - fn errno_from_error(&mut self, e: Error) -> Result { + fn errno_from_error(&mut self, e: Error) -> Result { debug!("Error: {:?}", e); - e.try_into() - .map_err(|e| wasmtime::Trap::new(format!("{:?}", e))) + let errno = e.try_into()?; + Ok(errno) } } @@ -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> { diff --git a/crates/wasi-nn/src/witx.rs b/crates/wasi-nn/src/witx.rs index b4c27734c60f..e7c877bd907e 100644 --- a/crates/wasi-nn/src/witx.rs +++ b/crates/wasi-nn/src/witx.rs @@ -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!({ @@ -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 { + fn nn_errno_from_wasi_nn_error(&mut self, e: WasiNnError) -> Result { eprintln!("Host error: {:?}", e); match e { WasiNnError::BackendError(_) => unimplemented!(), diff --git a/crates/wasmtime/src/component/func/host.rs b/crates/wasmtime/src/component/func/host.rs index 023acba474fc..087f11af5029 100644 --- a/crates/wasmtime/src/component/func/host.rs +++ b/crates/wasmtime/src/component/func/host.rs @@ -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 @@ -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; diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 571ab528d9a6..3d285a854183 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -3,7 +3,7 @@ use crate::{ AsContext, AsContextMut, CallHook, Engine, Extern, FuncType, Instance, StoreContext, StoreContextMut, Trap, Val, ValRaw, ValType, }; -use anyhow::{bail, Context as _, Result}; +use anyhow::{bail, Context as _, Error, Result}; use std::future::Future; use std::mem; use std::panic::{self, AssertUnwindSafe}; @@ -290,7 +290,7 @@ macro_rules! generate_wrap_async_func { match unsafe { async_cx.block_on(future.as_mut()) } { Ok(ret) => ret.into_fallible(), - Err(e) => R::fallible_from_trap(e), + Err(e) => R::fallible_from_error(e), } }) } @@ -332,7 +332,7 @@ impl Func { pub fn new( store: impl AsContextMut, ty: FuncType, - func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<()> + Send + Sync + 'static, ) -> Self { let ty_clone = ty.clone(); unsafe { @@ -365,7 +365,7 @@ impl Func { pub unsafe fn new_unchecked( mut store: impl AsContextMut, ty: FuncType, - func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, ) -> Self { let store = store.as_context_mut().0; let host = HostFunc::new_unchecked(store.engine(), ty, func); @@ -443,7 +443,7 @@ impl Func { Caller<'a, T>, &'a [Val], &'a mut [Val], - ) -> Box> + Send + 'a> + ) -> Box> + Send + 'a> + Send + Sync + 'static, @@ -505,7 +505,7 @@ impl Func { /// | `T` | `T` | a single return value | /// | `(T1, T2, ...)` | `T1 T2 ...` | multiple returns | /// - /// Note that all return types can also be wrapped in `Result<_, Trap>` to + /// Note that all return types can also be wrapped in `Result<_>` to /// indicate that the host function can generate a trap as well as possibly /// returning a value. /// @@ -581,7 +581,7 @@ impl Func { /// let add = Func::wrap(&mut store, |a: i32, b: i32| { /// match a.checked_add(b) { /// Some(i) => Ok(i), - /// None => Err(Trap::new("overflow")), + /// None => anyhow::bail!("overflow"), /// } /// }); /// let module = Module::new( @@ -652,7 +652,7 @@ impl Func { /// let log_str = Func::wrap(&mut store, |mut caller: Caller<'_, ()>, ptr: i32, len: i32| { /// let mem = match caller.get_export("memory") { /// Some(Extern::Memory(mem)) => mem, - /// _ => return Err(Trap::new("failed to find host memory")), + /// _ => anyhow::bail!("failed to find host memory"), /// }; /// let data = mem.data(&caller) /// .get(ptr as u32 as usize..) @@ -660,9 +660,9 @@ impl Func { /// let string = match data { /// Some(data) => match str::from_utf8(data) { /// Ok(s) => s, - /// Err(_) => return Err(Trap::new("invalid utf-8")), + /// Err(_) => anyhow::bail!("invalid utf-8"), /// }, - /// None => return Err(Trap::new("pointer/length out of bounds")), + /// None => anyhow::bail!("pointer/length out of bounds"), /// }; /// assert_eq!(string, "Hello, world!"); /// println!("{}", string); @@ -808,7 +808,7 @@ impl Func { &self, mut store: impl AsContextMut, params_and_returns: *mut ValRaw, - ) -> Result<(), Trap> { + ) -> Result<()> { let mut store = store.as_context_mut(); let data = &store.0.store_data()[self.0]; let anyfunc = data.export().anyfunc; @@ -821,7 +821,7 @@ impl Func { anyfunc: NonNull, trampoline: VMTrampoline, params_and_returns: *mut ValRaw, - ) -> Result<(), Trap> { + ) -> Result<()> { invoke_wasm_and_catch_traps(store, |caller| { let trampoline = wasmtime_runtime::prepare_host_to_wasm_trampoline(caller, trampoline); trampoline( @@ -1024,8 +1024,8 @@ impl Func { mut caller: Caller<'_, T>, ty: &FuncType, values_vec: &mut [ValRaw], - func: &dyn Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap>, - ) -> Result<(), Trap> { + func: &dyn Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<()>, + ) -> Result<()> { // Translate the raw JIT arguments in `values_vec` into a `Val` which // we'll be passing as a slice. The storage for our slice-of-`Val` we'll // be taking from the `Store`. We preserve our slice back into the @@ -1064,14 +1064,10 @@ impl Func { // values, and we need to catch that here. for (i, (ret, ty)) in results.iter().zip(ty.results()).enumerate() { if ret.ty() != ty { - return Err(Trap::new( - "function attempted to return an incompatible value", - )); + bail!("function attempted to return an incompatible value"); } if !ret.comes_from_same_store(caller.store.0) { - return Err(Trap::new( - "cross-`Store` values are not currently supported", - )); + bail!("cross-`Store` values are not currently supported"); } unsafe { values_vec[i] = ret.to_raw(&mut caller.store); @@ -1224,7 +1220,7 @@ impl Func { pub(crate) fn invoke_wasm_and_catch_traps( store: &mut StoreContextMut<'_, T>, closure: impl FnMut(*mut VMContext), -) -> Result<(), Trap> { +) -> Result<()> { unsafe { let exit = enter_wasm(store); @@ -1342,7 +1338,7 @@ pub unsafe trait WasmRet { self, store: &mut StoreOpaque, ptr: Self::Retptr, - ) -> Result; + ) -> Result; #[doc(hidden)] fn func_type(params: impl Iterator) -> FuncType; @@ -1358,7 +1354,7 @@ pub unsafe trait WasmRet { #[doc(hidden)] fn into_fallible(self) -> Self::Fallible; #[doc(hidden)] - fn fallible_from_trap(trap: Trap) -> Self::Fallible; + fn fallible_from_error(error: Error) -> Self::Fallible; } unsafe impl WasmRet for T @@ -1367,17 +1363,13 @@ where { type Abi = ::Abi; type Retptr = (); - type Fallible = Result; + type Fallible = Result; fn compatible_with_store(&self, store: &StoreOpaque) -> bool { ::compatible_with_store(self, store) } - unsafe fn into_abi_for_ret( - self, - store: &mut StoreOpaque, - _retptr: (), - ) -> Result { + unsafe fn into_abi_for_ret(self, store: &mut StoreOpaque, _retptr: ()) -> Result { Ok(::into_abi(self, store)) } @@ -1389,16 +1381,16 @@ where T::abi_into_raw(f(()), ptr); } - fn into_fallible(self) -> Result { + fn into_fallible(self) -> Result { Ok(self) } - fn fallible_from_trap(trap: Trap) -> Result { - Err(trap) + fn fallible_from_error(error: Error) -> Result { + Err(error) } } -unsafe impl WasmRet for Result +unsafe impl WasmRet for Result where T: WasmRet, { @@ -1417,7 +1409,7 @@ where self, store: &mut StoreOpaque, retptr: Self::Retptr, - ) -> Result { + ) -> Result { self.and_then(|val| val.into_abi_for_ret(store, retptr)) } @@ -1429,12 +1421,12 @@ where T::wrap_trampoline(ptr, f) } - fn into_fallible(self) -> Result { + fn into_fallible(self) -> Result { self } - fn fallible_from_trap(trap: Trap) -> Result { - Err(trap) + fn fallible_from_error(error: Error) -> Result { + Err(error) } } @@ -1448,7 +1440,7 @@ macro_rules! impl_wasm_host_results { { type Abi = <($($t::Abi,)*) as HostAbi>::Abi; type Retptr = <($($t::Abi,)*) as HostAbi>::Retptr; - type Fallible = Result; + type Fallible = Result; #[inline] fn compatible_with_store(&self, _store: &StoreOpaque) -> bool { @@ -1457,7 +1449,7 @@ macro_rules! impl_wasm_host_results { } #[inline] - unsafe fn into_abi_for_ret(self, _store: &mut StoreOpaque, ptr: Self::Retptr) -> Result { + unsafe fn into_abi_for_ret(self, _store: &mut StoreOpaque, ptr: Self::Retptr) -> Result { let ($($t,)*) = self; let abi = ($($t.into_abi(_store),)*); Ok(<($($t::Abi,)*) as HostAbi>::into_abi(abi, ptr)) @@ -1480,13 +1472,13 @@ macro_rules! impl_wasm_host_results { } #[inline] - fn into_fallible(self) -> Result { + fn into_fallible(self) -> Result { Ok(self) } #[inline] - fn fallible_from_trap(trap: Trap) -> Result { - Err(trap) + fn fallible_from_error(error: Error) -> Result { + Err(error) } } ) @@ -1844,7 +1836,7 @@ macro_rules! impl_into_func { let ret = { panic::catch_unwind(AssertUnwindSafe(|| { if let Err(trap) = caller.store.0.call_hook(CallHook::CallingHost) { - return R::fallible_from_trap(trap); + return R::fallible_from_error(trap); } $(let $args = $args::from_abi($args, caller.store.0);)* let r = func( @@ -1852,7 +1844,7 @@ macro_rules! impl_into_func { $( $args, )* ); if let Err(trap) = caller.store.0.call_hook(CallHook::ReturningFromHost) { - return R::fallible_from_trap(trap); + return R::fallible_from_error(trap); } r.into_fallible() })) @@ -1986,7 +1978,7 @@ impl HostFunc { pub fn new( engine: &Engine, ty: FuncType, - func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<()> + Send + Sync + 'static, ) -> Self { let ty_clone = ty.clone(); unsafe { @@ -2001,7 +1993,7 @@ impl HostFunc { pub unsafe fn new_unchecked( engine: &Engine, ty: FuncType, - func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, ) -> Self { let func = move |caller_vmctx, values: &mut [ValRaw]| { Caller::::with(caller_vmctx, |mut caller| { diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index 83565829e0f7..55eef9a26ee5 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -1,6 +1,6 @@ use super::{invoke_wasm_and_catch_traps, HostAbi}; use crate::store::{AutoAssertNoGc, StoreOpaque}; -use crate::{AsContextMut, ExternRef, Func, FuncType, StoreContextMut, Trap, ValRaw, ValType}; +use crate::{AsContextMut, ExternRef, Func, FuncType, StoreContextMut, ValRaw, ValType}; use anyhow::{bail, Result}; use std::marker; use std::mem::{self, MaybeUninit}; @@ -72,7 +72,7 @@ where /// /// This function will panic if it is called when the underlying [`Func`] is /// connected to an asynchronous store. - pub fn call(&self, mut store: impl AsContextMut, params: Params) -> Result { + pub fn call(&self, mut store: impl AsContextMut, params: Params) -> Result { let mut store = store.as_context_mut(); assert!( !store.0.async_support(), @@ -99,7 +99,7 @@ where &self, mut store: impl AsContextMut, params: Params, - ) -> Result + ) -> Result where T: Send, { @@ -120,7 +120,7 @@ where store: &mut StoreContextMut<'_, T>, func: ptr::NonNull, params: Params, - ) -> Result { + ) -> Result { // double-check that params/results match for this function's type in // debug mode. if cfg!(debug_assertions) { @@ -150,9 +150,7 @@ where match params.into_abi(&mut store) { Some(abi) => abi, None => { - return Err(Trap::new( - "attempt to pass cross-`Store` value to Wasm as function argument", - )) + bail!("attempt to pass cross-`Store` value to Wasm as function argument") } } }; diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index c86fbeab1065..04ad0d988d14 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -340,7 +340,7 @@ //! // module which called this host function. //! let mem = match caller.get_export("memory") { //! Some(Extern::Memory(mem)) => mem, -//! _ => return Err(Trap::new("failed to find host memory")), +//! _ => anyhow::bail!("failed to find host memory"), //! }; //! //! // Use the `ptr` and `len` values to get a subslice of the wasm-memory @@ -351,9 +351,9 @@ //! let string = match data { //! Some(data) => match str::from_utf8(data) { //! Ok(s) => s, -//! Err(_) => return Err(Trap::new("invalid utf-8")), +//! Err(_) => anyhow::bail!("invalid utf-8"), //! }, -//! None => return Err(Trap::new("pointer/length out of bounds")), +//! None => anyhow::bail!("pointer/length out of bounds"), //! }; //! assert_eq!(string, "Hello, world!"); //! println!("{}", string); diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index 2c269c1d2399..a37a70115224 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -149,7 +149,7 @@ macro_rules! generate_wrap_async_func { let mut future = Pin::from(func(caller, $($args),*)); match unsafe { async_cx.block_on(future.as_mut()) } { Ok(ret) => ret.into_fallible(), - Err(e) => R::fallible_from_trap(e), + Err(e) => R::fallible_from_error(e), } }) } @@ -271,7 +271,7 @@ impl Linker { import.name(), ); self.func_new(import.module(), import.name(), func_ty, move |_, _, _| { - Err(Trap::new(err_msg.clone())) + bail!("{err_msg}") })?; } } @@ -348,7 +348,7 @@ impl Linker { module: &str, name: &str, ty: FuncType, - func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &[Val], &mut [Val]) -> Result<()> + Send + Sync + 'static, ) -> Result<&mut Self> { let func = HostFunc::new(&self.engine, ty, func); let key = self.import_key(module, Some(name)); @@ -366,7 +366,7 @@ impl Linker { module: &str, name: &str, ty: FuncType, - func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<(), Trap> + Send + Sync + 'static, + func: impl Fn(Caller<'_, T>, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, ) -> Result<&mut Self> { let func = HostFunc::new_unchecked(&self.engine, ty, func); let key = self.import_key(module, Some(name)); @@ -391,7 +391,7 @@ impl Linker { Caller<'a, T>, &'a [Val], &'a mut [Val], - ) -> Box> + Send + 'a> + ) -> Box> + Send + 'a> + Send + Sync + 'static, diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 5a77d408b833..519d636ef9b8 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -79,7 +79,7 @@ use crate::linker::Definition; use crate::module::BareModuleInfo; use crate::{module::ModuleRegistry, Engine, Module, Trap, Val, ValRaw}; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use std::cell::UnsafeCell; use std::collections::HashMap; use std::convert::TryFrom; @@ -219,11 +219,11 @@ enum ResourceLimiterInner { pub trait CallHookHandler: Send { /// A callback to run when wasmtime is about to enter a host call, or when about to /// exit the hostcall. - async fn handle_call_event(&self, t: &mut T, ch: CallHook) -> Result<(), crate::Trap>; + async fn handle_call_event(&self, t: &mut T, ch: CallHook) -> Result<()>; } enum CallHookInner { - Sync(Box Result<(), crate::Trap> + Send + Sync>), + Sync(Box Result<()> + Send + Sync>), #[cfg(feature = "async")] Async(Box + Send + Sync>), } @@ -331,8 +331,7 @@ pub struct StoreOpaque { #[cfg(feature = "async")] struct AsyncState { - current_suspend: - UnsafeCell<*const wasmtime_fiber::Suspend, (), Result<(), Trap>>>, + current_suspend: UnsafeCell<*const wasmtime_fiber::Suspend, (), Result<()>>>, current_poll_cx: UnsafeCell<*mut Context<'static>>, } @@ -722,7 +721,7 @@ impl Store { /// to host or wasm code as the trap propagates to the root call. pub fn call_hook( &mut self, - hook: impl FnMut(&mut T, CallHook) -> Result<(), Trap> + Send + Sync + 'static, + hook: impl FnMut(&mut T, CallHook) -> Result<()> + Send + Sync + 'static, ) { self.inner.call_hook = Some(CallHookInner::Sync(Box::new(hook))); } @@ -1094,7 +1093,7 @@ impl StoreInner { &mut self.data } - pub fn call_hook(&mut self, s: CallHook) -> Result<(), Trap> { + pub fn call_hook(&mut self, s: CallHook) -> Result<()> { match &mut self.call_hook { Some(CallHookInner::Sync(hook)) => hook(&mut self.data, s), @@ -1103,7 +1102,7 @@ impl StoreInner { Ok(self .inner .async_cx() - .ok_or(Trap::new("couldn't grab async_cx for call hook"))? + .ok_or_else(|| anyhow!("couldn't grab async_cx for call hook"))? .block_on(handler.handle_call_event(&mut self.data, s).as_mut())??) }, @@ -1354,7 +1353,7 @@ impl StoreOpaque { /// This only works on async futures and stores, and assumes that we're /// executing on a fiber. This will yield execution back to the caller once. #[cfg(feature = "async")] - fn async_yield_impl(&mut self) -> Result<(), Trap> { + fn async_yield_impl(&mut self) -> Result<()> { // Small future that yields once and then returns () #[derive(Default)] struct Yield { @@ -1380,7 +1379,7 @@ impl StoreOpaque { let mut future = Yield::default(); - // When control returns, we have a `Result<(), Trap>` passed + // When control returns, we have a `Result<()>` passed // in from the host fiber. If this finished successfully then // we were resumed normally via a `poll`, so keep going. If // the future was dropped while we were yielded, then we need @@ -1518,7 +1517,7 @@ impl StoreContextMut<'_, T> { pub(crate) async fn on_fiber( &mut self, func: impl FnOnce(&mut StoreContextMut<'_, T>) -> R + Send, - ) -> Result + ) -> Result where T: Send, { @@ -1530,11 +1529,7 @@ impl StoreContextMut<'_, T> { let future = { let current_poll_cx = self.0.async_state.current_poll_cx.get(); let current_suspend = self.0.async_state.current_suspend.get(); - let stack = self - .engine() - .allocator() - .allocate_fiber_stack() - .map_err(|e| Trap::from(anyhow::Error::from(e)))?; + let stack = self.engine().allocator().allocate_fiber_stack()?; let engine = self.engine().clone(); let slot = &mut slot; @@ -1558,8 +1553,7 @@ impl StoreContextMut<'_, T> { *slot = Some(func(self)); Ok(()) } - }) - .map_err(|e| Trap::from(anyhow::Error::from(e)))?; + })?; // Once we have the fiber representing our synchronous computation, we // wrap that in a custom future implementation which does the @@ -1575,7 +1569,7 @@ impl StoreContextMut<'_, T> { return Ok(slot.unwrap()); struct FiberFuture<'a> { - fiber: wasmtime_fiber::Fiber<'a, Result<(), Trap>, (), Result<(), Trap>>, + fiber: wasmtime_fiber::Fiber<'a, Result<()>, (), Result<()>>, current_poll_cx: *mut *mut Context<'static>, engine: Engine, } @@ -1644,7 +1638,7 @@ impl StoreContextMut<'_, T> { unsafe impl Send for FiberFuture<'_> {} impl Future for FiberFuture<'_> { - type Output = Result<(), Trap>; + type Output = Result<()>; fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll { // We need to carry over this `cx` into our fiber's runtime @@ -1699,7 +1693,7 @@ impl StoreContextMut<'_, T> { impl Drop for FiberFuture<'_> { fn drop(&mut self) { if !self.fiber.done() { - let result = self.fiber.resume(Err(Trap::new("future dropped"))); + let result = self.fiber.resume(Err(anyhow!("future dropped"))); // This resumption with an error should always complete the // fiber. While it's technically possible for host code to catch // the trap and re-resume, we'd ideally like to signal that to @@ -1719,7 +1713,7 @@ impl StoreContextMut<'_, T> { #[cfg(feature = "async")] pub struct AsyncCx { - current_suspend: *mut *const wasmtime_fiber::Suspend, (), Result<(), Trap>>, + current_suspend: *mut *const wasmtime_fiber::Suspend, (), Result<()>>, current_poll_cx: *mut *mut Context<'static>, } @@ -1748,7 +1742,7 @@ impl AsyncCx { pub unsafe fn block_on( &self, mut future: Pin<&mut (dyn Future + Send)>, - ) -> Result { + ) -> Result { // Take our current `Suspend` context which was configured as soon as // our fiber started. Note that we must load it at the front here and // save it on our stack frame. While we're polling the future other @@ -1895,15 +1889,16 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { + let trap = || anyhow::Error::from(Trap::new("all fuel consumed by WebAssembly")); return match &mut self.out_of_gas_behavior { - OutOfGas::Trap => Err(anyhow::Error::new(OutOfGasError)), + OutOfGas::Trap => Err(trap()), #[cfg(feature = "async")] OutOfGas::InjectFuel { injection_count, fuel_to_inject, } => { if *injection_count == 0 { - return Err(anyhow::Error::new(OutOfGasError)); + return Err(trap()); } *injection_count -= 1; let fuel = *fuel_to_inject; @@ -1916,17 +1911,6 @@ unsafe impl wasmtime_runtime::Store for StoreInner { #[cfg(not(feature = "async"))] OutOfGas::InjectFuel { .. } => unreachable!(), }; - - #[derive(Debug)] - struct OutOfGasError; - - impl fmt::Display for OutOfGasError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("all fuel consumed by WebAssembly") - } - } - - impl std::error::Error for OutOfGasError {} } fn new_epoch(&mut self) -> Result { diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 840687370849..15f147064117 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -21,7 +21,7 @@ unsafe extern "C" fn stub_fn( values_vec: *mut ValRaw, values_vec_len: usize, ) where - F: Fn(*mut VMContext, &mut [ValRaw]) -> Result<(), Trap> + 'static, + F: Fn(*mut VMContext, &mut [ValRaw]) -> Result<()> + 'static, { // Here we are careful to use `catch_unwind` to ensure Rust panics don't // unwind past us. The primary reason for this is that Rust considers it UB @@ -105,7 +105,7 @@ pub fn create_function( engine: &Engine, ) -> Result<(Box, VMSharedSignatureIndex, VMTrampoline)> where - F: Fn(*mut VMContext, &mut [ValRaw]) -> Result<(), Trap> + Send + Sync + 'static, + F: Fn(*mut VMContext, &mut [ValRaw]) -> Result<()> + Send + Sync + 'static, { let mut obj = engine.compiler().object()?; let (t1, t2) = engine.compiler().emit_trampoline_obj( diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 086fac434a3f..802f41ae70d9 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -1,5 +1,6 @@ use crate::store::StoreOpaque; use crate::Module; +use anyhow::Error; use once_cell::sync::OnceCell; use std::fmt; use std::sync::Arc; @@ -13,6 +14,11 @@ pub struct Trap { inner: Arc, } +struct TrapInner { + reason: TrapReason, + backtrace: OnceCell, +} + /// State describing the occasion which evoked a trap. #[derive(Debug)] enum TrapReason { @@ -22,22 +28,15 @@ enum TrapReason { /// An `i32` exit status describing an explicit program exit. I32Exit(i32), - /// A structured error describing a trap. - Error(Box), - /// A specific code for a trap triggered while executing WASM. InstructionTrap(TrapCode), } -impl fmt::Display for TrapReason { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - TrapReason::Message(s) => write!(f, "{}", s), - TrapReason::I32Exit(status) => write!(f, "Exited with i32 exit status {}", status), - TrapReason::Error(e) => write!(f, "{}", e), - TrapReason::InstructionTrap(code) => write!(f, "wasm trap: {}", code), - } - } +#[derive(Debug)] +pub(crate) struct TrapBacktrace { + wasm_trace: Vec, + runtime_trace: wasmtime_runtime::Backtrace, + hint_wasm_backtrace_details_env: bool, } /// A trap code describing the reason for a trap. @@ -115,117 +114,6 @@ impl TrapCode { } } -impl fmt::Display for TrapCode { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use TrapCode::*; - let desc = match self { - StackOverflow => "call stack exhausted", - MemoryOutOfBounds => "out of bounds memory access", - HeapMisaligned => "misaligned memory access", - TableOutOfBounds => "undefined element: out of bounds table access", - IndirectCallToNull => "uninitialized element", - BadSignature => "indirect call type mismatch", - IntegerOverflow => "integer overflow", - IntegerDivisionByZero => "integer divide by zero", - BadConversionToInteger => "invalid conversion to integer", - UnreachableCodeReached => "wasm `unreachable` instruction executed", - Interrupt => "interrupt", - AlwaysTrapAdapter => "degenerate component adapter called", - }; - write!(f, "{}", desc) - } -} - -#[derive(Debug)] -pub(crate) struct TrapBacktrace { - wasm_trace: Vec, - runtime_trace: wasmtime_runtime::Backtrace, - hint_wasm_backtrace_details_env: bool, -} - -impl TrapBacktrace { - pub fn new( - store: &StoreOpaque, - runtime_trace: wasmtime_runtime::Backtrace, - trap_pc: Option, - ) -> Self { - let mut wasm_trace = Vec::::with_capacity(runtime_trace.frames().len()); - let mut hint_wasm_backtrace_details_env = false; - let wasm_backtrace_details_env_used = - store.engine().config().wasm_backtrace_details_env_used; - - for frame in runtime_trace.frames() { - debug_assert!(frame.pc() != 0); - - // Note that we need to be careful about the pc we pass in - // here to lookup frame information. This program counter is - // used to translate back to an original source location in - // the origin wasm module. If this pc is the exact pc that - // the trap happened at, then we look up that pc precisely. - // Otherwise backtrace information typically points at the - // pc *after* the call instruction (because otherwise it's - // likely a call instruction on the stack). In that case we - // want to lookup information for the previous instruction - // (the call instruction) so we subtract one as the lookup. - let pc_to_lookup = if Some(frame.pc()) == trap_pc { - frame.pc() - } else { - frame.pc() - 1 - }; - - // NB: The PC we are looking up _must_ be a Wasm PC since - // `wasmtime_runtime::Backtrace` only contains Wasm frames. - // - // However, consider the case where we have multiple, nested calls - // across stores (with host code in between, by necessity, since - // only things in the same store can be linked directly together): - // - // | ... | - // | Host | | - // +-----------------+ | stack - // | Wasm in store A | | grows - // +-----------------+ | down - // | Host | | - // +-----------------+ | - // | Wasm in store B | V - // +-----------------+ - // - // In this scenario, the `wasmtime_runtime::Backtrace` will contain - // two frames: Wasm in store B followed by Wasm in store A. But - // `store.modules()` will only have the module information for - // modules instantiated within this store. Therefore, we use `if let - // Some(..)` instead of the `unwrap` you might otherwise expect and - // we ignore frames from modules that were not registered in this - // store's module registry. - if let Some((info, module)) = store.modules().lookup_frame_info(pc_to_lookup) { - wasm_trace.push(info); - - // If this frame has unparsed debug information and the - // store's configuration indicates that we were - // respecting the environment variable of whether to - // do this then we will print out a helpful note in - // `Display` to indicate that more detailed information - // in a trap may be available. - let has_unparsed_debuginfo = module.compiled_module().has_unparsed_debuginfo(); - if has_unparsed_debuginfo && wasm_backtrace_details_env_used { - hint_wasm_backtrace_details_env = true; - } - } - } - - Self { - wasm_trace, - runtime_trace, - hint_wasm_backtrace_details_env, - } - } -} - -struct TrapInner { - reason: TrapReason, - backtrace: OnceCell, -} - fn _assert_trap_is_sync_and_send(t: &Trap) -> (&dyn Sync, &dyn Send) { (t, t) } @@ -265,25 +153,42 @@ impl Trap { pub(crate) fn from_runtime_box( store: &StoreOpaque, runtime_trap: Box, - ) -> Self { - Self::from_runtime(store, *runtime_trap) - } - - #[cold] // see Trap::new - pub(crate) fn from_runtime(store: &StoreOpaque, runtime_trap: wasmtime_runtime::Trap) -> Self { - let wasmtime_runtime::Trap { reason, backtrace } = runtime_trap; + ) -> Error { + let wasmtime_runtime::Trap { reason, backtrace } = *runtime_trap; match reason { + // For user-defined errors they're already an `anyhow::Error` so no + // conversion is really necessary here, but a `backtrace` may have + // been captured so it's attempted to get inserted here. + // + // If the error is actually a `Trap` then the backtrace is inserted + // directly into the `Trap` since there's storage there for it. + // Otherwise though this represents a host-defined error which isn't + // using a `Trap` but instead some other condition that was fatal to + // wasm itself. In that situation the backtrace is inserted as + // contextual information on error using `error.context(...)` to + // provide useful information to debug with for the embedder/caller, + // otherwise the information about what the wasm was doing when the + // error was generated would be lost. wasmtime_runtime::TrapReason::User { - error, + mut error, needs_backtrace, } => { - let trap = Trap::from(error); if let Some(backtrace) = backtrace { debug_assert!(needs_backtrace); - debug_assert!(trap.inner.backtrace.get().is_none()); - trap.record_backtrace(TrapBacktrace::new(store, backtrace, None)); + let bt = TrapBacktrace::new(store, backtrace, None); + match error.downcast_mut::() { + Some(trap) => { + debug_assert!(trap.inner.backtrace.get().is_none()); + trap.record_backtrace(bt); + } + None => { + if !bt.wasm_trace.is_empty() { + error = error.context(BacktraceContext(bt)); + } + } + } } - trap + error } wasmtime_runtime::TrapReason::Jit(pc) => { let code = store @@ -291,11 +196,11 @@ impl Trap { .lookup_trap_code(pc) .unwrap_or(EnvTrapCode::StackOverflow); let backtrace = backtrace.map(|bt| TrapBacktrace::new(store, bt, Some(pc))); - Trap::new_wasm(code, backtrace) + Trap::new_wasm(code, backtrace).into() } wasmtime_runtime::TrapReason::Wasm(trap_code) => { let backtrace = backtrace.map(|bt| TrapBacktrace::new(store, bt, None)); - Trap::new_wasm(trap_code, backtrace) + Trap::new_wasm(trap_code, backtrace).into() } } } @@ -401,100 +306,183 @@ impl fmt::Display for Trap { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.inner.reason)?; - if let Some(trace) = self.trace() { - if trace.is_empty() { - return Ok(()); + if let Some(trace) = self.inner.backtrace.get().as_ref() { + if !trace.wasm_trace.is_empty() { + write!(f, "\n{trace}")?; } - writeln!(f, "\nwasm backtrace:")?; + } + Ok(()) + } +} - for (i, frame) in trace.iter().enumerate() { - let name = frame.module_name().unwrap_or(""); - write!(f, " {:>3}: ", i)?; +impl std::error::Error for Trap {} + +impl fmt::Display for TrapReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TrapReason::Message(s) => write!(f, "{}", s), + TrapReason::I32Exit(status) => write!(f, "Exited with i32 exit status {}", status), + TrapReason::InstructionTrap(code) => write!(f, "wasm trap: {}", code), + } + } +} - if let Some(offset) = frame.module_offset() { - write!(f, "{:#6x} - ", offset)?; +impl fmt::Display for TrapCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use TrapCode::*; + let desc = match self { + StackOverflow => "call stack exhausted", + MemoryOutOfBounds => "out of bounds memory access", + HeapMisaligned => "misaligned memory access", + TableOutOfBounds => "undefined element: out of bounds table access", + IndirectCallToNull => "uninitialized element", + BadSignature => "indirect call type mismatch", + IntegerOverflow => "integer overflow", + IntegerDivisionByZero => "integer divide by zero", + BadConversionToInteger => "invalid conversion to integer", + UnreachableCodeReached => "wasm `unreachable` instruction executed", + Interrupt => "interrupt", + AlwaysTrapAdapter => "degenerate component adapter called", + }; + write!(f, "{}", desc) + } +} + +impl TrapBacktrace { + pub fn new( + store: &StoreOpaque, + runtime_trace: wasmtime_runtime::Backtrace, + trap_pc: Option, + ) -> Self { + let mut wasm_trace = Vec::::with_capacity(runtime_trace.frames().len()); + let mut hint_wasm_backtrace_details_env = false; + let wasm_backtrace_details_env_used = + store.engine().config().wasm_backtrace_details_env_used; + + for frame in runtime_trace.frames() { + debug_assert!(frame.pc() != 0); + + // Note that we need to be careful about the pc we pass in + // here to lookup frame information. This program counter is + // used to translate back to an original source location in + // the origin wasm module. If this pc is the exact pc that + // the trap happened at, then we look up that pc precisely. + // Otherwise backtrace information typically points at the + // pc *after* the call instruction (because otherwise it's + // likely a call instruction on the stack). In that case we + // want to lookup information for the previous instruction + // (the call instruction) so we subtract one as the lookup. + let pc_to_lookup = if Some(frame.pc()) == trap_pc { + frame.pc() + } else { + frame.pc() - 1 + }; + + // NB: The PC we are looking up _must_ be a Wasm PC since + // `wasmtime_runtime::Backtrace` only contains Wasm frames. + // + // However, consider the case where we have multiple, nested calls + // across stores (with host code in between, by necessity, since + // only things in the same store can be linked directly together): + // + // | ... | + // | Host | | + // +-----------------+ | stack + // | Wasm in store A | | grows + // +-----------------+ | down + // | Host | | + // +-----------------+ | + // | Wasm in store B | V + // +-----------------+ + // + // In this scenario, the `wasmtime_runtime::Backtrace` will contain + // two frames: Wasm in store B followed by Wasm in store A. But + // `store.modules()` will only have the module information for + // modules instantiated within this store. Therefore, we use `if let + // Some(..)` instead of the `unwrap` you might otherwise expect and + // we ignore frames from modules that were not registered in this + // store's module registry. + if let Some((info, module)) = store.modules().lookup_frame_info(pc_to_lookup) { + wasm_trace.push(info); + + // If this frame has unparsed debug information and the + // store's configuration indicates that we were + // respecting the environment variable of whether to + // do this then we will print out a helpful note in + // `Display` to indicate that more detailed information + // in a trap may be available. + let has_unparsed_debuginfo = module.compiled_module().has_unparsed_debuginfo(); + if has_unparsed_debuginfo && wasm_backtrace_details_env_used { + hint_wasm_backtrace_details_env = true; } + } + } + + Self { + wasm_trace, + runtime_trace, + hint_wasm_backtrace_details_env, + } + } +} - let write_raw_func_name = |f: &mut fmt::Formatter<'_>| { - demangle_function_name_or_index( - f, - frame.func_name(), - frame.func_index() as usize, - ) - }; - if frame.symbols().is_empty() { - write!(f, "{}!", name)?; - write_raw_func_name(f)?; +impl fmt::Display for TrapBacktrace { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "wasm backtrace:")?; + + for (i, frame) in self.wasm_trace.iter().enumerate() { + let name = frame.module_name().unwrap_or(""); + write!(f, " {:>3}: ", i)?; + + if let Some(offset) = frame.module_offset() { + write!(f, "{:#6x} - ", offset)?; + } + + let write_raw_func_name = |f: &mut fmt::Formatter<'_>| { + demangle_function_name_or_index(f, frame.func_name(), frame.func_index() as usize) + }; + if frame.symbols().is_empty() { + write!(f, "{}!", name)?; + write_raw_func_name(f)?; + writeln!(f, "")?; + } else { + for (i, symbol) in frame.symbols().iter().enumerate() { + if i > 0 { + write!(f, " - ")?; + } else { + // ... + } + match symbol.name() { + Some(name) => demangle_function_name(f, name)?, + None if i == 0 => write_raw_func_name(f)?, + None => write!(f, "")?, + } writeln!(f, "")?; - } else { - for (i, symbol) in frame.symbols().iter().enumerate() { - if i > 0 { - write!(f, " - ")?; - } else { - // ... - } - match symbol.name() { - Some(name) => demangle_function_name(f, name)?, - None if i == 0 => write_raw_func_name(f)?, - None => write!(f, "")?, - } - writeln!(f, "")?; - if let Some(file) = symbol.file() { - write!(f, " at {}", file)?; - if let Some(line) = symbol.line() { - write!(f, ":{}", line)?; - if let Some(col) = symbol.column() { - write!(f, ":{}", col)?; - } + if let Some(file) = symbol.file() { + write!(f, " at {}", file)?; + if let Some(line) = symbol.line() { + write!(f, ":{}", line)?; + if let Some(col) = symbol.column() { + write!(f, ":{}", col)?; } } - writeln!(f, "")?; } + writeln!(f, "")?; } } - if self - .inner - .backtrace - .get() - .map(|t| t.hint_wasm_backtrace_details_env) - .unwrap_or(false) - { - writeln!(f, "note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information")?; - } } - Ok(()) - } -} - -impl std::error::Error for Trap { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match &self.inner.reason { - TrapReason::Error(e) => e.source(), - TrapReason::I32Exit(_) | TrapReason::Message(_) | TrapReason::InstructionTrap(_) => { - None - } + if self.hint_wasm_backtrace_details_env { + writeln!(f, "note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information")?; } + Ok(()) } } -impl From for Trap { - fn from(e: anyhow::Error) -> Trap { - match e.downcast::() { - Ok(trap) => trap, - Err(e) => Box::::from(e).into(), - } - } -} +struct BacktraceContext(TrapBacktrace); -impl From> for Trap { - fn from(e: Box) -> Trap { - // If the top-level error is already a trap, don't be redundant and just return it. - if let Some(trap) = e.downcast_ref::() { - trap.clone() - } else { - let reason = TrapReason::Error(e.into()); - Trap::new_with_trace(reason, None) - } +impl fmt::Display for BacktraceContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "error while executing at {}", self.0) } } diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index bdcc7a0ea426..7fd595796927 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -101,7 +101,7 @@ fn _define_func( ctx: &mut (impl #(#bounds)+*), memory: &dyn #rt::GuestMemory, #(#abi_params),* - ) -> Result<#abi_ret, #rt::wasmtime_crate::Trap> { + ) -> #rt::anyhow::Result<#abi_ret> { use std::convert::TryFrom as _; #traced_body } @@ -127,7 +127,7 @@ fn _define_func( ctx: &'a mut (impl #(#bounds)+*), memory: &'a dyn #rt::GuestMemory, #(#abi_params),* - ) -> impl std::future::Future> + 'a { + ) -> impl std::future::Future> + 'a { use std::convert::TryFrom as _; #traced_body } diff --git a/crates/wiggle/generate/src/lib.rs b/crates/wiggle/generate/src/lib.rs index 09698d0227f9..9b5cea7579ce 100644 --- a/crates/wiggle/generate/src/lib.rs +++ b/crates/wiggle/generate/src/lib.rs @@ -43,7 +43,7 @@ pub fn generate(doc: &witx::Document, names: &Names, settings: &CodegenSettings) 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(&mut self, e: super::#user_typename) -> Result<#abi_typename, #rt::wasmtime_crate::Trap>;) + quote!(fn #methodname(&mut self, e: super::#user_typename) -> #rt::anyhow::Result<#abi_typename>;) }); let user_error_conversion = quote! { pub trait UserErrorConversion { diff --git a/crates/wiggle/generate/src/module_trait.rs b/crates/wiggle/generate/src/module_trait.rs index cd277bbf17a8..be9a47ad3525 100644 --- a/crates/wiggle/generate/src/module_trait.rs +++ b/crates/wiggle/generate/src/module_trait.rs @@ -45,7 +45,7 @@ pub fn define_module_trait(names: &Names, m: &Module, settings: &CodegenSettings }); let result = match f.results.len() { - 0 if f.noreturn => quote!(#rt::wasmtime_crate::Trap), + 0 if f.noreturn => quote!(#rt::anyhow::Error), 0 => quote!(()), 1 => { let (ok, err) = match &**f.results[0].tref.type_() { diff --git a/crates/wiggle/generate/src/wasmtime.rs b/crates/wiggle/generate/src/wasmtime.rs index d80bacec44b9..50ece48207df 100644 --- a/crates/wiggle/generate/src/wasmtime.rs +++ b/crates/wiggle/generate/src/wasmtime.rs @@ -114,9 +114,7 @@ fn generate_func( let body = quote! { let mem = match caller.get_export("memory") { Some(#rt::wasmtime_crate::Extern::Memory(m)) => m, - _ => { - return Err(#rt::wasmtime_crate::Trap::new("missing required memory export")); - } + _ => #rt::anyhow::bail!("missing required memory export"), }; let (mem , ctx) = mem.data_and_store_mut(&mut caller); let ctx = get_cx(ctx); @@ -143,7 +141,7 @@ fn generate_func( linker.func_wrap( #module_str, #field_str, - move |mut caller: #rt::wasmtime_crate::Caller<'_, T> #(, #arg_decls)*| -> Result<#ret_ty, #rt::wasmtime_crate::Trap> { + move |mut caller: #rt::wasmtime_crate::Caller<'_, T> #(, #arg_decls)*| -> #rt::anyhow::Result<#ret_ty> { let result = async { #body }; #rt::run_in_dummy_executor(result)? }, @@ -156,7 +154,7 @@ fn generate_func( linker.func_wrap( #module_str, #field_str, - move |mut caller: #rt::wasmtime_crate::Caller<'_, T> #(, #arg_decls)*| -> Result<#ret_ty, #rt::wasmtime_crate::Trap> { + move |mut caller: #rt::wasmtime_crate::Caller<'_, T> #(, #arg_decls)*| -> #rt::anyhow::Result<#ret_ty> { #body }, )?; diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index d5f232bb313f..369f842ab5e0 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -909,14 +909,6 @@ impl Pointee for str { } } -impl From for wasmtime_crate::Trap { - fn from(err: GuestError) -> wasmtime_crate::Trap { - wasmtime_crate::Trap::from( - Box::new(err) as Box - ) - } -} - pub fn run_in_dummy_executor( future: F, ) -> Result { diff --git a/crates/wiggle/test-helpers/Cargo.toml b/crates/wiggle/test-helpers/Cargo.toml index 058cf06f6a51..a09a7a510b75 100644 --- a/crates/wiggle/test-helpers/Cargo.toml +++ b/crates/wiggle/test-helpers/Cargo.toml @@ -16,6 +16,7 @@ proptest = "1.0.0" wiggle = { path = "..", features = ["tracing_log"] } [dev-dependencies] +anyhow = { workspace = true } thiserror = "1.0" tracing = "0.1.26" tracing-subscriber = { version = "0.3.1", default-features = false, features = ['fmt'] } diff --git a/crates/wiggle/test-helpers/examples/tracing.rs b/crates/wiggle/test-helpers/examples/tracing.rs index dd2556a43ac8..07c361d985b6 100644 --- a/crates/wiggle/test-helpers/examples/tracing.rs +++ b/crates/wiggle/test-helpers/examples/tracing.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use wiggle_test::{impl_errno, HostMemory, WasiCtx}; /// The `errors` argument to the wiggle gives us a hook to map a rich error @@ -32,10 +33,7 @@ impl_errno!(types::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( - &mut self, - e: RichError, - ) -> Result { + fn errno_from_rich_error(&mut self, e: RichError) -> Result { wiggle::tracing::debug!( rich_error = wiggle::tracing::field::debug(&e), "error conversion" diff --git a/crates/wiggle/tests/errors.rs b/crates/wiggle/tests/errors.rs index 5b10a5479b0a..737b7ea5a4b7 100644 --- a/crates/wiggle/tests/errors.rs +++ b/crates/wiggle/tests/errors.rs @@ -1,5 +1,6 @@ /// Execute the wiggle guest conversion code to exercise it mod convert_just_errno { + use anyhow::Result; use wiggle_test::{impl_errno, HostMemory, WasiCtx}; /// The `errors` argument to the wiggle gives us a hook to map a rich error @@ -31,10 +32,7 @@ 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( - &mut self, - e: RichError, - ) -> Result { + fn errno_from_rich_error(&mut self, e: RichError) -> Result { // WasiCtx can collect a Vec 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()); @@ -105,6 +103,7 @@ mod convert_just_errno { /// we use two distinct error types. mod convert_multiple_error_types { pub use super::convert_just_errno::RichError; + use anyhow::Result; use wiggle_test::{impl_errno, WasiCtx}; /// Test that we can map multiple types of errors. @@ -143,16 +142,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( - &mut self, - _e: RichError, - ) -> Result { + fn errno_from_rich_error(&mut self, _e: RichError) -> Result { unimplemented!() } fn errno2_from_another_rich_error( &mut self, _e: AnotherRichError, - ) -> Result { + ) -> Result { unimplemented!() } } @@ -165,7 +161,7 @@ mod convert_multiple_error_types { fn bar(&mut self, _: u32) -> Result<(), AnotherRichError> { unimplemented!() } - fn baz(&mut self, _: u32) -> wiggle::wasmtime_crate::Trap { + fn baz(&mut self, _: u32) -> anyhow::Error { unimplemented!() } } diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index f51897ca9108..9a0783c1555c 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -314,7 +314,7 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { unimplemented!("poll_oneoff") } - fn proc_exit(&mut self, _rval: types::Exitcode) -> wiggle::wasmtime_crate::Trap { + fn proc_exit(&mut self, _rval: types::Exitcode) -> anyhow::Error { unimplemented!("proc_exit") } diff --git a/examples/interrupt.rs b/examples/interrupt.rs index 70d7965a3aa9..1fa639060b5a 100644 --- a/examples/interrupt.rs +++ b/examples/interrupt.rs @@ -26,7 +26,11 @@ fn main() -> Result<()> { }); println!("Entering infinite loop ..."); - let trap = run.call(&mut store, ()).unwrap_err(); + let trap = run + .call(&mut store, ()) + .unwrap_err() + .downcast::() + .unwrap(); println!("trap received..."); assert!(trap.trap_code().unwrap() == TrapCode::Interrupt); diff --git a/tests/all/async_functions.rs b/tests/all/async_functions.rs index f741d86e073c..d6c85bc4c9cd 100644 --- a/tests/all/async_functions.rs +++ b/tests/all/async_functions.rs @@ -440,7 +440,7 @@ async fn resume_separate_thread() { let func = Func::wrap0_async(&mut store, |_| { Box::new(async { tokio::task::yield_now().await; - Err::<(), _>(wasmtime::Trap::new("test")) + Err::<(), _>(wasmtime::Trap::new("test").into()) }) }); let result = Instance::new_async(&mut store, &module, &[func.into()]).await; @@ -536,7 +536,7 @@ async fn resume_separate_thread3() { // ... all in all this function will need access to the original TLS // information to raise the trap. This TLS information should be // restored even though the asynchronous execution is suspended. - Err::<(), _>(wasmtime::Trap::new("")) + Err::<(), _>(wasmtime::Trap::new("").into()) }); assert!(f.call(&mut store, &[], &mut []).is_err()); } @@ -561,7 +561,12 @@ async fn recursive_async() -> Result<()> { // ... but calls that actually stack overflow should indeed stack // overflow - let err = overflow.call_async(&mut caller, ()).await.unwrap_err(); + let err = overflow + .call_async(&mut caller, ()) + .await + .unwrap_err() + .downcast::() + .unwrap(); assert_eq!(err.trap_code(), Some(TrapCode::StackOverflow)); Ok(()) }) diff --git a/tests/all/call_hook.rs b/tests/all/call_hook.rs index d30125181a41..f28771a2d265 100644 --- a/tests/all/call_hook.rs +++ b/tests/all/call_hook.rs @@ -1,4 +1,4 @@ -use anyhow::Error; +use anyhow::{bail, Error, Result}; use std::future::Future; use std::pin::Pin; use std::task::{self, Poll}; @@ -424,12 +424,12 @@ fn trapping() -> Result<(), Error> { linker.func_wrap( "host", "f", - |mut caller: Caller, action: i32, recur: i32| -> Result<(), Trap> { + |mut caller: Caller, action: i32, recur: i32| -> Result<()> { assert_eq!(caller.data().context.last(), Some(&Context::Host)); assert_eq!(caller.data().calls_into_host, caller.data().calls_into_wasm); match action { - TRAP_IN_F => return Err(Trap::new("trapping in f")), + TRAP_IN_F => bail!("trapping in f"), TRAP_NEXT_CALL_HOST => caller.data_mut().trap_next_call_host = true, TRAP_NEXT_RETURN_HOST => caller.data_mut().trap_next_return_host = true, TRAP_NEXT_CALL_WASM => caller.data_mut().trap_next_call_wasm = true, @@ -485,7 +485,7 @@ fn trapping() -> Result<(), Error> { }; let (s, e) = run(TRAP_IN_F, false); - assert!(e.unwrap().to_string().starts_with("trapping in f")); + assert!(format!("{:?}", e.unwrap()).contains("trapping in f")); assert_eq!(s.calls_into_host, 1); assert_eq!(s.returns_from_host, 1); assert_eq!(s.calls_into_wasm, 1); @@ -501,10 +501,7 @@ fn trapping() -> Result<(), Error> { // trap in next call to host. recur, so the second call into host traps: let (s, e) = run(TRAP_NEXT_CALL_HOST, true); - assert!(e - .unwrap() - .to_string() - .starts_with("call_hook: trapping on CallingHost")); + assert!(format!("{:?}", e.unwrap()).contains("call_hook: trapping on CallingHost")); assert_eq!(s.calls_into_host, 2); assert_eq!(s.returns_from_host, 1); assert_eq!(s.calls_into_wasm, 2); @@ -512,10 +509,7 @@ fn trapping() -> Result<(), Error> { // trap in the return from host. should trap right away, without recursion let (s, e) = run(TRAP_NEXT_RETURN_HOST, false); - assert!(e - .unwrap() - .to_string() - .starts_with("call_hook: trapping on ReturningFromHost")); + assert!(format!("{:?}", e.unwrap()).contains("call_hook: trapping on ReturningFromHost")); assert_eq!(s.calls_into_host, 1); assert_eq!(s.returns_from_host, 1); assert_eq!(s.calls_into_wasm, 1); @@ -531,10 +525,7 @@ fn trapping() -> Result<(), Error> { // trap in next call to wasm. recur, so the second call into wasm traps: let (s, e) = run(TRAP_NEXT_CALL_WASM, true); - assert!(e - .unwrap() - .to_string() - .starts_with("call_hook: trapping on CallingWasm")); + assert!(format!("{:?}", e.unwrap()).contains("call_hook: trapping on CallingWasm")); assert_eq!(s.calls_into_host, 1); assert_eq!(s.returns_from_host, 1); assert_eq!(s.calls_into_wasm, 2); @@ -542,10 +533,7 @@ fn trapping() -> Result<(), Error> { // trap in the return from wasm. should trap right away, without recursion let (s, e) = run(TRAP_NEXT_RETURN_WASM, false); - assert!(e - .unwrap() - .to_string() - .starts_with("call_hook: trapping on ReturningFromWasm")); + assert!(format!("{:?}", e.unwrap()).contains("call_hook: trapping on ReturningFromWasm")); assert_eq!(s.calls_into_host, 1); assert_eq!(s.returns_from_host, 1); assert_eq!(s.calls_into_wasm, 1); @@ -560,11 +548,7 @@ async fn basic_async_hook() -> Result<(), Error> { #[async_trait::async_trait] impl CallHookHandler for HandlerR { - async fn handle_call_event( - &self, - obj: &mut State, - ch: CallHook, - ) -> Result<(), wasmtime::Trap> { + async fn handle_call_event(&self, obj: &mut State, ch: CallHook) -> Result<()> { State::call_hook(obj, ch) } } @@ -638,13 +622,9 @@ async fn timeout_async_hook() -> Result<(), Error> { #[async_trait::async_trait] impl CallHookHandler for HandlerR { - async fn handle_call_event( - &self, - obj: &mut State, - ch: CallHook, - ) -> Result<(), wasmtime::Trap> { + async fn handle_call_event(&self, obj: &mut State, ch: CallHook) -> Result<()> { if obj.calls_into_host > 200 { - return Err(wasmtime::Trap::new("timeout")); + bail!("timeout"); } match ch { @@ -718,11 +698,7 @@ async fn drop_suspended_async_hook() -> Result<(), Error> { #[async_trait::async_trait] impl CallHookHandler for Handler { - async fn handle_call_event( - &self, - state: &mut u32, - _ch: CallHook, - ) -> Result<(), wasmtime::Trap> { + async fn handle_call_event(&self, state: &mut u32, _ch: CallHook) -> Result<()> { assert_eq!(*state, 0); *state += 1; let _dec = Decrement(state); @@ -861,12 +837,12 @@ impl Default for State { impl State { // This implementation asserts that hooks are always called in a stack-like manner. - fn call_hook(&mut self, s: CallHook) -> Result<(), Trap> { + fn call_hook(&mut self, s: CallHook) -> Result<()> { match s { CallHook::CallingHost => { self.calls_into_host += 1; if self.trap_next_call_host { - return Err(Trap::new("call_hook: trapping on CallingHost")); + bail!("call_hook: trapping on CallingHost"); } else { self.context.push(Context::Host); } @@ -875,7 +851,7 @@ impl State { Some(Context::Host) => { self.returns_from_host += 1; if self.trap_next_return_host { - return Err(Trap::new("call_hook: trapping on ReturningFromHost")); + bail!("call_hook: trapping on ReturningFromHost"); } } c => panic!( @@ -886,7 +862,7 @@ impl State { CallHook::CallingWasm => { self.calls_into_wasm += 1; if self.trap_next_call_wasm { - return Err(Trap::new("call_hook: trapping on CallingWasm")); + bail!("call_hook: trapping on CallingWasm"); } else { self.context.push(Context::Wasm); } @@ -895,7 +871,7 @@ impl State { Some(Context::Wasm) => { self.returns_from_wasm += 1; if self.trap_next_return_wasm { - return Err(Trap::new("call_hook: trapping on ReturningFromWasm")); + bail!("call_hook: trapping on ReturningFromWasm"); } } c => panic!( diff --git a/tests/all/component_model/import.rs b/tests/all/component_model/import.rs index 79cabe6ba353..c5f47071998e 100644 --- a/tests/all/component_model/import.rs +++ b/tests/all/component_model/import.rs @@ -733,16 +733,22 @@ fn bad_import_alignment() -> Result<()> { .instantiate(&mut store, &component)? .get_typed_func::<(), (), _>(&mut store, "unaligned-retptr")? .call(&mut store, ()) - .unwrap_err() - .downcast::()?; - assert!(trap.to_string().contains("pointer not aligned"), "{}", trap); + .unwrap_err(); + assert!( + format!("{:?}", trap).contains("pointer not aligned"), + "{}", + trap + ); let trap = linker .instantiate(&mut store, &component)? .get_typed_func::<(), (), _>(&mut store, "unaligned-argptr")? .call(&mut store, ()) - .unwrap_err() - .downcast::()?; - assert!(trap.to_string().contains("pointer not aligned"), "{}", trap); + .unwrap_err(); + assert!( + format!("{:?}", trap).contains("pointer not aligned"), + "{}", + trap + ); Ok(()) } diff --git a/tests/all/component_model/strings.rs b/tests/all/component_model/strings.rs index fca18ebc49cc..bda6172e37b5 100644 --- a/tests/all/component_model/strings.rs +++ b/tests/all/component_model/strings.rs @@ -498,9 +498,8 @@ fn test_invalid_string_encoding( let trap = test_raw_when_encoded(engine, src, dst, bytes, len)?.unwrap(); let src = src.replace("latin1+", ""); assert!( - trap.to_string() - .contains(&format!("invalid {src} encoding")), - "bad error: {}", + format!("{:?}", trap).contains(&format!("invalid {src} encoding")), + "bad error: {:?}", trap, ); Ok(()) @@ -524,7 +523,7 @@ fn test_raw_when_encoded( dst: &str, bytes: &[u8], len: u32, -) -> Result> { +) -> Result> { let component = format!( r#" (component @@ -574,6 +573,6 @@ fn test_raw_when_encoded( let func = instance.get_typed_func::<(&[u8], u32), (), _>(&mut store, "f")?; match func.call(&mut store, (bytes, len)) { Ok(_) => Ok(None), - Err(e) => Ok(Some(e.downcast()?)), + Err(e) => Ok(Some(e)), } } diff --git a/tests/all/func.rs b/tests/all/func.rs index 79a0efbbe491..8d91e95ed10a 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -17,15 +17,13 @@ fn func_constructors() { Func::wrap(&mut store, || -> Option { None }); Func::wrap(&mut store, || -> Option { None }); - Func::wrap(&mut store, || -> Result<(), Trap> { loop {} }); - Func::wrap(&mut store, || -> Result { loop {} }); - Func::wrap(&mut store, || -> Result { loop {} }); - Func::wrap(&mut store, || -> Result { loop {} }); - Func::wrap(&mut store, || -> Result { loop {} }); - Func::wrap(&mut store, || -> Result, Trap> { - loop {} - }); - Func::wrap(&mut store, || -> Result, Trap> { loop {} }); + Func::wrap(&mut store, || -> Result<()> { loop {} }); + Func::wrap(&mut store, || -> Result { loop {} }); + Func::wrap(&mut store, || -> Result { loop {} }); + Func::wrap(&mut store, || -> Result { loop {} }); + Func::wrap(&mut store, || -> Result { loop {} }); + Func::wrap(&mut store, || -> Result> { loop {} }); + Func::wrap(&mut store, || -> Result> { loop {} }); } #[test] @@ -222,8 +220,8 @@ fn import_works() -> Result<()> { #[test] fn trap_smoke() -> Result<()> { let mut store = Store::<()>::default(); - let f = Func::wrap(&mut store, || -> Result<(), Trap> { - Err(Trap::new("test")) + let f = Func::wrap(&mut store, || -> Result<()> { + Err(Trap::new("test").into()) }); let err = f .call(&mut store, &[], &mut []) @@ -244,7 +242,9 @@ fn trap_import() -> Result<()> { )?; let mut store = Store::<()>::default(); let module = Module::new(store.engine(), &wasm)?; - let import = Func::wrap(&mut store, || -> Result<(), Trap> { Err(Trap::new("foo")) }); + let import = Func::wrap(&mut store, || -> Result<()> { + Err(Trap::new("foo").into()) + }); let trap = Instance::new(&mut store, &module, &[import.into()]) .err() .unwrap() @@ -451,10 +451,7 @@ fn func_write_nothing() -> anyhow::Result<()> { let mut store = Store::<()>::default(); let ty = FuncType::new(None, Some(ValType::I32)); let f = Func::new(&mut store, ty, |_, _, _| Ok(())); - let err = f - .call(&mut store, &[], &mut [Val::I32(0)]) - .unwrap_err() - .downcast::()?; + let err = f.call(&mut store, &[], &mut [Val::I32(0)]).unwrap_err(); assert!(err .to_string() .contains("function attempted to return an incompatible value")); @@ -488,7 +485,7 @@ fn return_cross_store_value() -> anyhow::Result<()> { let run = instance.get_func(&mut store1, "run").unwrap(); let result = run.call(&mut store1, &[], &mut [Val::I32(0)]); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("cross-`Store`")); + assert!(format!("{:?}", result.unwrap_err()).contains("cross-`Store`")); Ok(()) } @@ -620,9 +617,9 @@ fn trap_doesnt_leak() -> anyhow::Result<()> { // test that `Func::wrap` is correct let canary1 = Canary::default(); let dtor1_run = canary1.0.clone(); - let f1 = Func::wrap(&mut store, move || -> Result<(), Trap> { + let f1 = Func::wrap(&mut store, move || -> Result<()> { drop(&canary1); - Err(Trap::new("")) + Err(Trap::new("").into()) }); assert!(f1.typed::<(), (), _>(&store)?.call(&mut store, ()).is_err()); assert!(f1.call(&mut store, &[], &mut []).is_err()); @@ -632,7 +629,7 @@ fn trap_doesnt_leak() -> anyhow::Result<()> { let dtor2_run = canary2.0.clone(); let f2 = Func::new(&mut store, FuncType::new(None, None), move |_, _, _| { drop(&canary2); - Err(Trap::new("")) + Err(Trap::new("").into()) }); assert!(f2.typed::<(), (), _>(&store)?.call(&mut store, ()).is_err()); assert!(f2.call(&mut store, &[], &mut []).is_err()); diff --git a/tests/all/host_funcs.rs b/tests/all/host_funcs.rs index 0c33c60a90f8..84c04d54a9b0 100644 --- a/tests/all/host_funcs.rs +++ b/tests/all/host_funcs.rs @@ -33,13 +33,13 @@ fn wrap_func() -> Result<()> { linker.func_wrap("m3", "", || -> Option { None })?; linker.func_wrap("m3", "f", || -> Option { None })?; - linker.func_wrap("", "f1", || -> Result<(), Trap> { loop {} })?; - linker.func_wrap("", "f2", || -> Result { loop {} })?; - linker.func_wrap("", "f3", || -> Result { loop {} })?; - linker.func_wrap("", "f4", || -> Result { loop {} })?; - linker.func_wrap("", "f5", || -> Result { loop {} })?; - linker.func_wrap("", "f6", || -> Result, Trap> { loop {} })?; - linker.func_wrap("", "f7", || -> Result, Trap> { loop {} })?; + linker.func_wrap("", "f1", || -> Result<()> { loop {} })?; + linker.func_wrap("", "f2", || -> Result { loop {} })?; + linker.func_wrap("", "f3", || -> Result { loop {} })?; + linker.func_wrap("", "f4", || -> Result { loop {} })?; + linker.func_wrap("", "f5", || -> Result { loop {} })?; + linker.func_wrap("", "f6", || -> Result> { loop {} })?; + linker.func_wrap("", "f7", || -> Result> { loop {} })?; Ok(()) } @@ -444,7 +444,7 @@ fn call_wasm_many_args() -> Result<()> { fn trap_smoke() -> Result<()> { let engine = Engine::default(); let mut linker = Linker::<()>::new(&engine); - linker.func_wrap("", "", || -> Result<(), Trap> { Err(Trap::new("test")) })?; + linker.func_wrap("", "", || -> Result<()> { Err(Trap::new("test").into()) })?; let mut store = Store::new(&engine, ()); @@ -472,7 +472,7 @@ fn trap_import() -> Result<()> { let engine = Engine::default(); let mut linker = Linker::new(&engine); - linker.func_wrap("", "", || -> Result<(), Trap> { Err(Trap::new("foo")) })?; + linker.func_wrap("", "", || -> Result<()> { Err(Trap::new("foo").into()) })?; let module = Module::new(&engine, &wasm)?; let mut store = Store::new(&engine, ()); @@ -607,10 +607,7 @@ fn func_return_nothing() -> Result<()> { let mut store = Store::new(&engine, ()); let f = linker.get(&mut store, "", "").unwrap().into_func().unwrap(); - let err = f - .call(&mut store, &[], &mut [Val::I32(0)]) - .unwrap_err() - .downcast::()?; + let err = f.call(&mut store, &[], &mut [Val::I32(0)]).unwrap_err(); assert!(err .to_string() .contains("function attempted to return an incompatible value")); @@ -725,7 +722,7 @@ fn wasi_imports() -> Result<()> { let instance = linker.instantiate(&mut store, &module)?; let start = instance.get_typed_func::<(), (), _>(&mut store, "_start")?; - let trap = start.call(&mut store, ()).unwrap_err(); + let trap = start.call(&mut store, ()).unwrap_err().downcast::()?; assert_eq!(trap.i32_exit_status(), Some(123)); Ok(()) diff --git a/tests/all/iloop.rs b/tests/all/iloop.rs index 1947a59c4446..1e6227edd154 100644 --- a/tests/all/iloop.rs +++ b/tests/all/iloop.rs @@ -31,7 +31,7 @@ fn loops_interruptable() -> anyhow::Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; store.engine().increment_epoch(); - let trap = iloop.call(&mut store, ()).unwrap_err(); + let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; assert!( trap.trap_code().unwrap() == TrapCode::Interrupt, "bad message: {}", @@ -48,7 +48,7 @@ fn functions_interruptable() -> anyhow::Result<()> { let instance = Instance::new(&mut store, &module, &[func.into()])?; let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; store.engine().increment_epoch(); - let trap = iloop.call(&mut store, ()).unwrap_err(); + let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; assert!( trap.trap_code().unwrap() == TrapCode::Interrupt, "{}", @@ -98,7 +98,7 @@ fn loop_interrupt_from_afar() -> anyhow::Result<()> { // Enter the infinitely looping function and assert that our interrupt // handle does indeed actually interrupt the function. let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; - let trap = iloop.call(&mut store, ()).unwrap_err(); + let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; STOP.store(true, SeqCst); thread.join().unwrap(); assert!(HITS.load(SeqCst) > NUM_HITS); @@ -138,7 +138,7 @@ fn function_interrupt_from_afar() -> anyhow::Result<()> { // Enter the infinitely looping function and assert that our interrupt // handle does indeed actually interrupt the function. let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; - let trap = iloop.call(&mut store, ()).unwrap_err(); + let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; STOP.store(true, SeqCst); thread.join().unwrap(); assert!(HITS.load(SeqCst) > NUM_HITS); diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index dd7342a05446..5b17d6937e15 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -82,10 +82,7 @@ fn test_returns_incorrect_type() -> Result<()> { let mut result = [Val::I32(0)]; let trap = run_func .call(&mut store, &[], &mut result) - .expect_err("the execution should fail") - .downcast::()?; - assert!(trap - .to_string() - .contains("function attempted to return an incompatible value")); + .expect_err("the execution should fail"); + assert!(format!("{:?}", trap).contains("function attempted to return an incompatible value")); Ok(()) } diff --git a/tests/all/linker.rs b/tests/all/linker.rs index cc8e060afd05..9728d757f5d0 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -32,7 +32,7 @@ fn link_twice_bad() -> Result<()> { linker.func_wrap("f", "", || {})?; assert!(linker.func_wrap("f", "", || {}).is_err()); assert!(linker - .func_wrap("f", "", || -> Result<(), Trap> { loop {} }) + .func_wrap("f", "", || -> Result<()> { loop {} }) .is_err()); // globals diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 74831fb554dc..d8e4dbf97846 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -15,15 +15,17 @@ fn test_trap_return() -> Result<()> { let module = Module::new(store.engine(), wat)?; let hello_type = FuncType::new(None, None); - let hello_func = Func::new(&mut store, hello_type, |_, _, _| Err(Trap::new("test 123"))); + let hello_func = Func::new(&mut store, hello_type, |_, _, _| { + Err(Trap::new("test 123").into()) + }); let instance = Instance::new(&mut store, &module, &[hello_func.into()])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; assert!(e.to_string().contains("test 123")); Ok(()) @@ -45,8 +47,8 @@ fn test_trap_trace() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; let trace = e.trace().expect("backtrace is available"); assert_eq!(trace.len(), 2); @@ -123,10 +125,8 @@ fn test_trap_through_host() -> Result<()> { &module, &[host_func_a.into(), host_func_b.into()], )?; - let a = instance - .get_typed_func::<(), (), _>(&mut store, "a") - .unwrap(); - let err = a.call(&mut store, ()).unwrap_err(); + let a = instance.get_typed_func::<(), (), _>(&mut store, "a")?; + let err = a.call(&mut store, ()).unwrap_err().downcast::()?; let trace = err.trace().expect("backtrace is available"); assert_eq!(trace.len(), 3); assert_eq!(trace[0].func_name(), Some("c")); @@ -155,8 +155,8 @@ fn test_trap_backtrace_disabled() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; assert!(e.trace().is_none(), "backtraces should be disabled"); Ok(()) @@ -174,7 +174,9 @@ fn test_trap_trace_cb() -> Result<()> { "#; let fn_type = FuncType::new(None, None); - let fn_func = Func::new(&mut store, fn_type, |_, _, _| Err(Trap::new("cb throw"))); + let fn_func = Func::new(&mut store, fn_type, |_, _, _| { + Err(Trap::new("cb throw").into()) + }); let module = Module::new(store.engine(), wat)?; let instance = Instance::new(&mut store, &module, &[fn_func.into()])?; @@ -182,8 +184,8 @@ fn test_trap_trace_cb() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; let trace = e.trace().expect("backtrace is available"); assert_eq!(trace.len(), 2); @@ -211,8 +213,8 @@ fn test_trap_stack_overflow() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; let trace = e.trace().expect("backtrace is available"); assert!(trace.len() >= 32); @@ -244,8 +246,8 @@ fn trap_display_pretty() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; assert_eq!( e.to_string(), "\ @@ -321,7 +323,11 @@ fn trap_start_function_import() -> Result<()> { let module = Module::new(store.engine(), &binary)?; let sig = FuncType::new(None, None); - let func = Func::new(&mut store, sig, |_, _, _| Err(Trap::new("user trap"))); + let func = Func::new( + &mut store, + sig, + |_, _, _| Err(Trap::new("user trap").into()), + ); let err = Instance::new(&mut store, &module, &[func.into()]) .err() .unwrap(); @@ -416,7 +422,7 @@ fn rust_catch_panic_import() -> Result<()> { let instance = Instance::new(&mut store, &module, &[panic.into(), catch_panic.into()])?; let run = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let trap = run.call(&mut store, ()).unwrap_err(); + let trap = run.call(&mut store, ()).unwrap_err().downcast::()?; let trace = trap.trace().unwrap(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_index(), 3); @@ -561,11 +567,11 @@ fn present_after_module_drop() -> Result<()> { let func = instance.get_typed_func::<(), (), _>(&mut store, "foo")?; println!("asserting before we drop modules"); - assert_trap(func.call(&mut store, ()).unwrap_err()); + assert_trap(func.call(&mut store, ()).unwrap_err().downcast()?); drop((instance, module)); println!("asserting after drop"); - assert_trap(func.call(&mut store, ()).unwrap_err()); + assert_trap(func.call(&mut store, ()).unwrap_err().downcast()?); return Ok(()); fn assert_trap(t: Trap) { @@ -796,8 +802,8 @@ fn traps_without_address_map() -> Result<()> { let e = run_func .call(&mut store, ()) - .err() - .expect("error calling function"); + .unwrap_err() + .downcast::()?; let trace = e.trace().expect("backtrace is available"); assert_eq!(trace.len(), 2); @@ -848,8 +854,8 @@ fn catch_trap_calling_across_stores() -> Result<()> { let trap = func .call(&mut data.child_store, ()) - .err() - .expect("should trap"); + .unwrap_err() + .downcast::()?; assert!( trap.to_string().contains("unreachable"), "trap should contain 'unreachable', got: {trap}" @@ -963,7 +969,11 @@ async fn async_then_sync_trap() -> Result<()> { let a = async_instance .get_typed_func::<(), (), _>(&mut async_store, "a") .unwrap(); - let trap = a.call_async(&mut async_store, ()).await.unwrap_err(); + let trap = a + .call_async(&mut async_store, ()) + .await + .unwrap_err() + .downcast::()?; let trace = trap.trace().unwrap(); // We don't support cross-store or cross-engine symbolication currently, so @@ -1022,25 +1032,21 @@ async fn sync_then_async_trap() -> Result<()> { let sync_module = Module::new(sync_store.engine(), wat)?; let mut sync_linker = Linker::new(sync_store.engine()); - sync_linker.func_wrap( - "", - "b", - move |mut caller: Caller| -> Result<(), Trap> { - log::info!("Called `b`..."); - let async_instance = caller.data().async_instance; - let async_store = &mut caller.data_mut().async_store; - - log::info!("Calling `c`..."); - let c = async_instance - .get_typed_func::<(), (), _>(&mut *async_store, "c") - .unwrap(); - tokio::task::block_in_place(|| { - tokio::runtime::Handle::current() - .block_on(async move { c.call_async(async_store, ()).await }) - })?; - Ok(()) - }, - )?; + sync_linker.func_wrap("", "b", move |mut caller: Caller| -> Result<()> { + log::info!("Called `b`..."); + let async_instance = caller.data().async_instance; + let async_store = &mut caller.data_mut().async_store; + + log::info!("Calling `c`..."); + let c = async_instance + .get_typed_func::<(), (), _>(&mut *async_store, "c") + .unwrap(); + tokio::task::block_in_place(|| { + tokio::runtime::Handle::current() + .block_on(async move { c.call_async(async_store, ()).await }) + })?; + Ok(()) + })?; let sync_instance = sync_linker.instantiate(&mut sync_store, &sync_module)?; @@ -1048,7 +1054,10 @@ async fn sync_then_async_trap() -> Result<()> { let a = sync_instance .get_typed_func::<(), (), _>(&mut sync_store, "a") .unwrap(); - let trap = a.call(&mut sync_store, ()).unwrap_err(); + let trap = a + .call(&mut sync_store, ()) + .unwrap_err() + .downcast::()?; let trace = trap.trace().unwrap(); // We don't support cross-store or cross-engine symbolication currently, so From 46d0ce809324fee690fb2092b963c1afaba286e5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 31 Oct 2022 09:20:24 -0700 Subject: [PATCH 02/12] Fix some doc links --- cranelift/codegen/src/ir/immediates.rs | 4 ++-- crates/wasmtime/src/func/typed.rs | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cranelift/codegen/src/ir/immediates.rs b/cranelift/codegen/src/ir/immediates.rs index 4421f1fdaa9d..3b3f7032353b 100644 --- a/cranelift/codegen/src/ir/immediates.rs +++ b/cranelift/codegen/src/ir/immediates.rs @@ -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: /// /// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but /// [PartialOrd] respects IEEE754 semantics. @@ -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: /// /// The [PartialEq] and [Hash] implementations are over the underlying bit pattern, but /// [PartialOrd] respects IEEE754 semantics. diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index 55eef9a26ee5..ecc5c15c5b32 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -72,6 +72,8 @@ where /// /// This function will panic if it is called when the underlying [`Func`] is /// connected to an asynchronous store. + /// + /// [`Trap`]: crate::Trap pub fn call(&self, mut store: impl AsContextMut, params: Params) -> Result { let mut store = store.as_context_mut(); assert!( @@ -93,6 +95,8 @@ where /// /// This function will panic if it is called when the underlying [`Func`] is /// connected to a synchronous store. + /// + /// [`Trap`]: crate::Trap #[cfg(feature = "async")] #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))] pub async fn call_async( From 61eb19b0bf720fac77039428e133f2ab56eb53cc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 1 Nov 2022 07:14:35 -0700 Subject: [PATCH 03/12] add some tests and make a backtrace type public (#55) * Trap: avoid a trailing newline in the Display impl which in turn ends up with three newlines between the end of the backtrace and the `Caused by` in the anyhow Debug impl * make BacktraceContext pub, and add tests showing downcasting behavior of anyhow::Error to traps or backtraces --- crates/wasmtime/src/trap.rs | 32 +++++++++++--- tests/all/traps.rs | 87 +++++++++++++++++++++++++++++++++++-- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 802f41ae70d9..631607670055 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -430,7 +430,14 @@ impl fmt::Display for TrapBacktrace { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "wasm backtrace:")?; + let mut needs_newline = false; for (i, frame) in self.wasm_trace.iter().enumerate() { + // Avoid putting a trailing newline on the output + if needs_newline { + writeln!(f, "")?; + } else { + needs_newline = true; + } let name = frame.module_name().unwrap_or(""); write!(f, " {:>3}: ", i)?; @@ -444,7 +451,6 @@ impl fmt::Display for TrapBacktrace { if frame.symbols().is_empty() { write!(f, "{}!", name)?; write_raw_func_name(f)?; - writeln!(f, "")?; } else { for (i, symbol) in frame.symbols().iter().enumerate() { if i > 0 { @@ -457,8 +463,8 @@ impl fmt::Display for TrapBacktrace { None if i == 0 => write_raw_func_name(f)?, None => write!(f, "")?, } - writeln!(f, "")?; if let Some(file) = symbol.file() { + writeln!(f, "")?; write!(f, " at {}", file)?; if let Some(line) = symbol.line() { write!(f, ":{}", line)?; @@ -467,7 +473,6 @@ impl fmt::Display for TrapBacktrace { } } } - writeln!(f, "")?; } } } @@ -478,7 +483,9 @@ impl fmt::Display for TrapBacktrace { } } -struct BacktraceContext(TrapBacktrace); +/// Describes the context (backtrace) at which a user's error terminated (trapped) +/// WebAssembly execution +pub struct BacktraceContext(TrapBacktrace); impl fmt::Display for BacktraceContext { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -486,13 +493,28 @@ impl fmt::Display for BacktraceContext { } } -/// Description of a frame in a backtrace for a [`Trap`]. +impl fmt::Debug for BacktraceContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", self.0) + } +} + +impl BacktraceContext { + /// Returns a list of function frames in WebAssembly code that led to this + /// trap happening. + pub fn frames(&self) -> &[FrameInfo] { + self.0.wasm_trace.as_slice() + } +} + +/// Description of a frame in a backtrace for a [`Trap`] or [`BacktraceContext`]. /// /// Whenever a WebAssembly trap occurs an instance of [`Trap`] is created. Each /// [`Trap`] has a backtrace of the WebAssembly frames that led to the trap, and /// each frame is described by this structure. /// /// [`Trap`]: crate::Trap +/// [`BacktraceContext`]: crate::BacktraceContext #[derive(Debug)] pub struct FrameInfo { module_name: Option, diff --git a/tests/all/traps.rs b/tests/all/traps.rs index d8e4dbf97846..d719edcf2125 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -22,11 +22,92 @@ fn test_trap_return() -> Result<()> { let instance = Instance::new(&mut store, &module, &[hello_func.into()])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; + let e = run_func.call(&mut store, ()).unwrap_err(); + let trap = e.downcast_ref::().expect("error is a Trap"); + assert!(trap.to_string().contains("test 123")); + + assert!( + e.downcast_ref::().is_none(), + "error should not contain a BacktraceContext" + ); + + Ok(()) +} + +#[test] +fn test_anyhow_error_return() -> Result<()> { + let mut store = Store::<()>::default(); + let wat = r#" + (module + (func $hello (import "" "hello")) + (func (export "run") (call $hello)) + ) + "#; + + let module = Module::new(store.engine(), wat)?; + let hello_type = FuncType::new(None, None); + let hello_func = Func::new(&mut store, hello_type, |_, _, _| { + Err(anyhow::Error::msg("test 1234")) + }); + + let instance = Instance::new(&mut store, &module, &[hello_func.into()])?; + let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; + + let e = run_func.call(&mut store, ()).unwrap_err(); + assert!(!e.to_string().contains("test 1234")); + assert!(format!("{:?}", e).contains("Caused by:\n test 1234")); + + assert!(e.downcast_ref::().is_none()); + assert!(e.downcast_ref::().is_some()); + + Ok(()) +} + +#[test] +fn test_trap_return_downcast() -> Result<()> { + let mut store = Store::<()>::default(); + let wat = r#" + (module + (func $hello (import "" "hello")) + (func (export "run") (call $hello)) + ) + "#; + + #[derive(Debug)] + struct MyTrap; + impl std::fmt::Display for MyTrap { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "my trap") + } + } + impl std::error::Error for MyTrap {} + + let module = Module::new(store.engine(), wat)?; + let hello_type = FuncType::new(None, None); + let hello_func = Func::new(&mut store, hello_type, |_, _, _| { + Err(anyhow::Error::from(MyTrap)) + }); + + let instance = Instance::new(&mut store, &module, &[hello_func.into()])?; + let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; + let e = run_func .call(&mut store, ()) - .unwrap_err() - .downcast::()?; - assert!(e.to_string().contains("test 123")); + .err() + .expect("error calling function"); + let dbg = format!("{:?}", e); + println!("{}", dbg); + + assert!(!e.to_string().contains("my trap")); + assert!(dbg.contains("Caused by:\n my trap")); + + e.downcast_ref::() + .expect("error downcasts to MyTrap"); + let bt = e + .downcast_ref::() + .expect("error downcasts to BacktraceContext"); + assert_eq!(bt.frames().len(), 1); + println!("{:?}", bt); Ok(()) } From 1ff3d78dfe38bc84354841b13d6797d7eeb5987d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 07:18:07 -0700 Subject: [PATCH 04/12] Remove now-unnecesary `Trap` downcasts in `Linker::module` --- crates/wasmtime/src/linker.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/wasmtime/src/linker.rs b/crates/wasmtime/src/linker.rs index a37a70115224..ba7890898397 100644 --- a/crates/wasmtime/src/linker.rs +++ b/crates/wasmtime/src/linker.rs @@ -3,7 +3,7 @@ use crate::instance::InstancePre; use crate::store::StoreOpaque; use crate::{ AsContextMut, Caller, Engine, Extern, ExternType, Func, FuncType, ImportType, Instance, - IntoFunc, Module, StoreContextMut, Trap, Val, ValRaw, + IntoFunc, Module, StoreContextMut, Val, ValRaw, }; use anyhow::{anyhow, bail, Context, Result}; use log::warn; @@ -714,8 +714,7 @@ impl Linker { .unwrap() .into_func() .unwrap() - .call(&mut caller, params, results) - .map_err(|error| error.downcast::().unwrap())?; + .call(&mut caller, params, results)?; Ok(()) }, @@ -781,8 +780,7 @@ impl Linker { .into_func() .unwrap() .call_async(&mut caller, params, results) - .await - .map_err(|error| error.downcast::().unwrap())?; + .await?; Ok(()) }) }, From 35d68e13f5dcb4dbc44927ea96a190723e4a3ba7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 07:22:02 -0700 Subject: [PATCH 05/12] Fix test output expectations --- crates/wasmtime/src/trap.rs | 2 +- tests/all/traps.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 631607670055..11d382c3c186 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -477,7 +477,7 @@ impl fmt::Display for TrapBacktrace { } } if self.hint_wasm_backtrace_details_env { - writeln!(f, "note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information")?; + write!(f, "\nnote: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information")?; } Ok(()) } diff --git a/tests/all/traps.rs b/tests/all/traps.rs index d719edcf2125..00be807a2fb0 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -337,7 +337,7 @@ wasm backtrace: 0: 0x23 - m!die 1: 0x27 - m! 2: 0x2c - m!foo - 3: 0x31 - m! + 3: 0x31 - m!\ " ); Ok(()) @@ -384,7 +384,7 @@ wasm backtrace: 2: 0x2c - a!foo 3: 0x31 - a! 4: 0x29 - b!middle - 5: 0x2e - b! + 5: 0x2e - b!\ " ); Ok(()) @@ -634,7 +634,7 @@ wasm backtrace: 0: 0x1d - m!die 1: 0x21 - m! 2: 0x26 - m!foo - 3: 0x2b - m!start + 3: 0x2b - m!start\ " ); Ok(()) @@ -794,7 +794,7 @@ fn no_hint_even_with_dwarf_info() -> Result<()> { "\ wasm trap: wasm `unreachable` instruction executed wasm backtrace: - 0: 0x1a - !start + 0: 0x1a - !start\ " ); Ok(()) @@ -829,7 +829,7 @@ fn hint_with_dwarf_info() -> Result<()> { wasm trap: wasm `unreachable` instruction executed wasm backtrace: 0: 0x1a - !start -note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information +note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information\ " ); Ok(()) From 9fff030784e11960d9c92d05bb49a81d1619bae4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 07:36:40 -0700 Subject: [PATCH 06/12] Remove `Trap::i32_exit` This commit removes special-handling in the `wasmtime::Trap` type for the i32 exit code required by WASI. This is now instead modeled as a specific `I32Exit` error type in the `wasmtime-wasi` crate which is returned by the `proc_exit` hostcall. Embedders which previously tested for i32 exits now downcast to the `I32Exit` value. --- crates/bench-api/src/lib.rs | 17 +++++----- crates/c-api/src/trap.rs | 16 ++++------ crates/wasi-common/src/error.rs | 16 ++++++++++ crates/wasi-common/src/lib.rs | 2 +- crates/wasi-common/src/snapshots/preview_1.rs | 11 +++---- crates/wasi/src/lib.rs | 2 +- crates/wasmtime/src/trap.rs | 20 ------------ src/commands/run.rs | 31 +++++++++++-------- tests/all/func.rs | 1 - tests/all/host_funcs.rs | 9 ++++-- 10 files changed, 61 insertions(+), 64 deletions(-) diff --git a/crates/bench-api/src/lib.rs b/crates/bench-api/src/lib.rs index a946fdb6b551..498236f64d4f 100644 --- a/crates/bench-api/src/lib.rs +++ b/crates/bench-api/src/lib.rs @@ -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; @@ -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::() { + if exit.0 == 0 { + return Ok(()); + } } + + Err(trap) } } } diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index cf91ce152118..8334074920f9 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -141,17 +141,13 @@ 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 { - let trap = match raw.error.downcast_ref::() { - Some(trap) => trap, - None => return false, - }; - match trap.i32_exit_status() { - Some(i) => { - *status = i; - true - } - None => false, + #[cfg(feature = "wasi")] + if let Some(exit) = raw.error.downcast_ref::() { + *status = exit.0; + return true; } + + false } #[no_mangle] diff --git a/crates/wasi-common/src/error.rs b/crates/wasi-common/src/error.rs index eed892ed063e..d16909e03157 100644 --- a/crates/wasi-common/src/error.rs +++ b/crates/wasi-common/src/error.rs @@ -24,6 +24,7 @@ //! `anyhow::Result::context` to aid in debugging of errors. pub use anyhow::{Context, Error}; +use std::fmt; /// Internal error type for the `wasi-common` crate. /// @@ -138,3 +139,18 @@ impl ErrorExt for Error { ErrorKind::Perm.into() } } + +/// An error returned from the `proc_exit` host syscall. +/// +/// Embedders can test if an error returned from wasm is this error, in which +/// case it may signal a non-fatal trap. +#[derive(Debug)] +pub struct I32Exit(pub i32); + +impl fmt::Display for I32Exit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Exited with i32 exit status {}", self.0) + } +} + +impl std::error::Error for I32Exit {} diff --git a/crates/wasi-common/src/lib.rs b/crates/wasi-common/src/lib.rs index 0f86c560caf3..6f9e949cbec9 100644 --- a/crates/wasi-common/src/lib.rs +++ b/crates/wasi-common/src/lib.rs @@ -66,7 +66,7 @@ pub use cap_rand::RngCore; pub use clocks::{SystemTimeSpec, WasiClocks, WasiMonotonicClock, WasiSystemClock}; pub use ctx::WasiCtx; pub use dir::WasiDir; -pub use error::{Context, Error, ErrorExt, ErrorKind}; +pub use error::{Context, Error, ErrorExt, ErrorKind, I32Exit}; pub use file::WasiFile; pub use sched::{Poll, WasiSched}; pub use string_array::StringArrayError; diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 19a67affd16a..38e3dc14220b 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -8,7 +8,7 @@ use crate::{ subscription::{RwEventFlags, SubscriptionResult}, Poll, Userdata, }, - Error, ErrorExt, ErrorKind, SystemTimeSpec, WasiCtx, + Error, ErrorExt, ErrorKind, I32Exit, SystemTimeSpec, WasiCtx, }; use anyhow::{Context, Result}; use cap_std::time::{Duration, SystemClock}; @@ -1216,12 +1216,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { async fn proc_exit(&mut self, status: types::Exitcode) -> anyhow::Error { // Check that the status is within WASI's range. - let trap = if status < 126 { - wasmtime::Trap::i32_exit(status as i32).into() + if status < 126 { + I32Exit(status as i32).into() } else { - wasmtime::Trap::new("exit with invalid exit status outside of [0..126)") - }; - trap.into() + wasmtime::Trap::new("exit with invalid exit status outside of [0..126)").into() + } } async fn proc_raise(&mut self, _sig: types::Signal) -> Result<(), Error> { diff --git a/crates/wasi/src/lib.rs b/crates/wasi/src/lib.rs index 17744239783b..308fc2d0d421 100644 --- a/crates/wasi/src/lib.rs +++ b/crates/wasi/src/lib.rs @@ -7,7 +7,7 @@ //! Individual snapshots are available through //! `wasmtime_wasi::snapshots::preview_{0, 1}::Wasi::new(&Store, Rc>)`. -pub use wasi_common::{Error, WasiCtx, WasiDir, WasiFile}; +pub use wasi_common::{Error, I32Exit, WasiCtx, WasiDir, WasiFile}; /// Re-export the commonly used wasi-cap-std-sync crate here. This saves /// consumers of this library from having to keep additional dependencies diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 11d382c3c186..e7b726885b48 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -25,9 +25,6 @@ enum TrapReason { /// An error message describing a trap. Message(String), - /// An `i32` exit status describing an explicit program exit. - I32Exit(i32), - /// A specific code for a trap triggered while executing WASM. InstructionTrap(TrapCode), } @@ -133,13 +130,6 @@ impl Trap { Trap::new_with_trace(reason, None) } - /// Creates a new `Trap` representing an explicit program exit with a classic `i32` - /// exit status value. - #[cold] // see Trap::new - pub fn i32_exit(status: i32) -> Self { - Trap::new_with_trace(TrapReason::I32Exit(status), None) - } - // Same safety requirements and caveats as // `wasmtime_runtime::raise_user_trap`. pub(crate) unsafe fn raise(error: anyhow::Error) -> ! { @@ -229,15 +219,6 @@ impl Trap { } } - /// If the trap was the result of an explicit program exit with a classic - /// `i32` exit status value, return the value, otherwise return `None`. - pub fn i32_exit_status(&self) -> Option { - match self.inner.reason { - TrapReason::I32Exit(status) => Some(status), - _ => None, - } - } - /// Displays the error reason for this trap. /// /// In particular, it differs from this struct's `Display` by *only* @@ -321,7 +302,6 @@ impl fmt::Display for TrapReason { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { TrapReason::Message(s) => write!(f, "{}", s), - TrapReason::I32Exit(status) => write!(f, "Exited with i32 exit status {}", status), TrapReason::InstructionTrap(code) => write!(f, "wasm trap: {}", code), } } diff --git a/src/commands/run.rs b/src/commands/run.rs index 8185bf7e0053..4dc9732d594f 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -15,6 +15,7 @@ use std::{ use wasmtime::{Engine, Func, Linker, Module, Store, Trap, Val, ValType}; use wasmtime_cli_flags::{CommonOptions, WasiModules}; use wasmtime_wasi::sync::{ambient_authority, Dir, TcpListener, WasiCtxBuilder}; +use wasmtime_wasi::I32Exit; #[cfg(feature = "wasi-nn")] use wasmtime_wasi_nn::WasiNnCtx; @@ -211,24 +212,25 @@ impl RunCommand { { Ok(()) => (), Err(e) => { - // If the program exited because of a non-zero exit status, print - // a message and exit. - if let Some(trap) = e.downcast_ref::() { + // If a specific WASI error code was requested then that's + // forwarded through to the process here without printing any + // extra error information. + if let Some(exit) = e.downcast_ref::() { // Print the error message in the usual way. - if let Some(status) = trap.i32_exit_status() { - // On Windows, exit status 3 indicates an abort (see below), - // so return 1 indicating a non-zero status to avoid ambiguity. - if cfg!(windows) && status >= 3 { - process::exit(1); - } - process::exit(status); + // On Windows, exit status 3 indicates an abort (see below), + // so return 1 indicating a non-zero status to avoid ambiguity. + if cfg!(windows) && exit.0 >= 3 { + process::exit(1); } + process::exit(exit.0); + } + // If the program exited because of a trap, return an error code + // to the outside environment indicating a more severe problem + // than a simple failure. + if e.is::() { eprintln!("Error: {:?}", e); - // If the program exited because of a trap, return an error code - // to the outside environment indicating a more severe problem - // than a simple failure. if cfg!(unix) { // On Unix, return the error code of an abort. process::exit(128 + libc::SIGABRT); @@ -238,6 +240,9 @@ impl RunCommand { process::exit(3); } } + + // Otherwise fall back on Rust's default error printing/return + // code. return Err(e); } } diff --git a/tests/all/func.rs b/tests/all/func.rs index 8d91e95ed10a..924de965f0fc 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -228,7 +228,6 @@ fn trap_smoke() -> Result<()> { .unwrap_err() .downcast::()?; assert!(err.to_string().contains("test")); - assert!(err.i32_exit_status().is_none()); Ok(()) } diff --git a/tests/all/host_funcs.rs b/tests/all/host_funcs.rs index 84c04d54a9b0..665eb4f6181c 100644 --- a/tests/all/host_funcs.rs +++ b/tests/all/host_funcs.rs @@ -2,6 +2,7 @@ use anyhow::Result; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use wasmtime::*; use wasmtime_wasi::sync::WasiCtxBuilder; +use wasmtime_wasi::I32Exit; #[test] #[should_panic = "cannot use `func_new_async` without enabling async support"] @@ -456,7 +457,6 @@ fn trap_smoke() -> Result<()> { .downcast::()?; assert!(err.to_string().contains("test")); - assert!(err.i32_exit_status().is_none()); Ok(()) } @@ -722,8 +722,11 @@ fn wasi_imports() -> Result<()> { let instance = linker.instantiate(&mut store, &module)?; let start = instance.get_typed_func::<(), (), _>(&mut store, "_start")?; - let trap = start.call(&mut store, ()).unwrap_err().downcast::()?; - assert_eq!(trap.i32_exit_status(), Some(123)); + let exit = start + .call(&mut store, ()) + .unwrap_err() + .downcast::()?; + assert_eq!(exit.0, 123); Ok(()) } From 444c0d2a49b94152d71d8b7354e344587af64a75 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 08:20:56 -0700 Subject: [PATCH 07/12] Remove the `Trap::new` constructor This commit removes the ability to create a trap with an arbitrary error message. The purpose of this commit is to continue the prior trend of leaning into the `anyhow::Error` type instead of trying to recreate it with `Trap`. A subsequent simplification to `Trap` after this commit is that `Trap` will simply be an `enum` of trap codes with no extra information. This commit is doubly-motivated by the desire to always use the new `BacktraceContext` type instead of sometimes using that and sometimes using `Trap`. Most of the changes here were around updating `Trap::new` calls to `bail!` calls instead. Tests which assert particular error messages additionally often needed to use the `:?` formatter instead of the `{}` formatter because the prior formats the whole `anyhow::Error` and the latter only formats the top-most error, which now contains the backtrace. --- crates/c-api/src/func.rs | 44 ++++++++-------- crates/c-api/src/trap.rs | 4 +- crates/fuzzing/src/oracles.rs | 5 +- crates/fuzzing/src/oracles/stacks.rs | 17 ++++--- crates/wasi-common/src/snapshots/preview_1.rs | 4 +- crates/wasmtime/src/component/func/host.rs | 4 +- crates/wasmtime/src/component/func/options.rs | 5 +- crates/wasmtime/src/store.rs | 5 +- crates/wasmtime/src/trap.rs | 39 +++++++-------- crates/wast/src/wast.rs | 34 +++++++------ crates/wiggle/src/lib.rs | 7 ++- crates/wiggle/tests/wasmtime_sync.rs | 2 +- tests/all/async_functions.rs | 8 +-- tests/all/cli_tests.rs | 12 +---- tests/all/component_model/import.rs | 32 +++++------- tests/all/fuel.rs | 9 ++-- tests/all/func.rs | 24 +++------ tests/all/host_funcs.rs | 17 ++----- tests/all/traps.rs | 50 ++++++------------- 19 files changed, 133 insertions(+), 189 deletions(-) diff --git a/crates/c-api/src/func.rs b/crates/c-api/src/func.rs index 79024fd495bc..68cebd0c427d 100644 --- a/crates/c-api/src/func.rs +++ b/crates/c-api/src/func.rs @@ -3,7 +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::Result; +use anyhow::{Error, Result}; +use std::any::Any; use std::ffi::c_void; use std::mem::{self, MaybeUninit}; use std::panic::{self, AssertUnwindSafe}; @@ -155,19 +156,23 @@ pub unsafe extern "C" fn wasm_func_call( } Ok(Err(err)) => Box::into_raw(Box::new(wasm_trap_t::new(err))), Err(panic) => { - let trap = if let Some(msg) = panic.downcast_ref::() { - 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.into())); + 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) -> Error { + if let Some(msg) = panic.downcast_ref::() { + 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 { Box::new(wasm_functype_t::new(f.func().ty(f.ext.store.context()))) @@ -346,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::() { - Ok(trap) => { - *trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(trap.into()))); + Ok(Err(trap)) => { + if trap.is::() { + *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::() { - 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.into()))); + let err = error_from_panic(panic); + *trap_ret = Box::into_raw(Box::new(wasm_trap_t::new(err))); None } } diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 8334074920f9..676e49fce6cc 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -52,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 { - error: Trap::new(message).into(), + error: Error::msg(message.into_owned()), }) } @@ -61,7 +61,7 @@ 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 { - error: Trap::new(message).into(), + error: Error::msg(message.into_owned()), }) } diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 21e35723f07b..78ae1f0d50e7 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -676,10 +676,7 @@ pub fn table_ops( .unwrap(); match trap.trap_code() { - Some(TrapCode::TableOutOfBounds) => {} - None if trap - .to_string() - .contains("all fuel consumed by WebAssembly") => {} + Some(TrapCode::TableOutOfBounds) | Some(TrapCode::OutOfFuel) => {} _ => { panic!("unexpected trap: {}", trap); } diff --git a/crates/fuzzing/src/oracles/stacks.rs b/crates/fuzzing/src/oracles/stacks.rs index 810f1eb7b1da..8aaf4effd133 100644 --- a/crates/fuzzing/src/oracles/stacks.rs +++ b/crates/fuzzing/src/oracles/stacks.rs @@ -1,5 +1,5 @@ use crate::generators::Stacks; -use anyhow::Result; +use anyhow::{bail, Result}; use wasmtime::*; /// Run the given `Stacks` test case and assert that the host's view of the Wasm @@ -27,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").into()); + bail!("out of fuel") } fuel.set(&mut caller, Val::I32(fuel_left - 1)).unwrap(); @@ -60,8 +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::().unwrap(); - log::debug!("trap: {}", trap); + log::debug!("trap: {:?}", trap); let get_stack = instance .get_typed_func::<(), (u32, u32), _>(&mut store, "get_stack") .expect("should export `get_stack` function as expected"); @@ -74,9 +73,15 @@ pub fn check_stacks(stacks: Stacks) -> usize { .get_memory(&mut store, "memory") .expect("should have `memory` export"); - let host_trace = trap.trace().unwrap(); + let (host_trace, code) = match trap.downcast_ref::() { + Some(trap) => (trap.trace().unwrap(), trap.trap_code()), + None => ( + trap.downcast_ref::().unwrap().frames(), + None, + ), + }; max_stack_depth = max_stack_depth.max(host_trace.len()); - assert_stack_matches(&mut store, memory, ptr, len, host_trace, trap.trap_code()); + assert_stack_matches(&mut store, memory, ptr, len, host_trace, code); } } max_stack_depth diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 38e3dc14220b..ca7dde519d10 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -10,7 +10,7 @@ use crate::{ }, Error, ErrorExt, ErrorKind, I32Exit, SystemTimeSpec, WasiCtx, }; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use cap_std::time::{Duration, SystemClock}; use std::convert::{TryFrom, TryInto}; use std::io::{IoSlice, IoSliceMut}; @@ -1219,7 +1219,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { if status < 126 { I32Exit(status as i32).into() } else { - wasmtime::Trap::new("exit with invalid exit status outside of [0..126)").into() + anyhow!("exit with invalid exit status outside of [0..126)") } } diff --git a/crates/wasmtime/src/component/func/host.rs b/crates/wasmtime/src/component/func/host.rs index 087f11af5029..023acba474fc 100644 --- a/crates/wasmtime/src/component/func/host.rs +++ b/crates/wasmtime/src/component/func/host.rs @@ -193,7 +193,7 @@ where // the component is disallowed, for example, when the `realloc` function // calls a canonical import. if !flags.may_leave() { - return Err(Trap::new("cannot leave component instance").into()); + bail!("cannot leave component instance"); } // There's a 2x2 matrix of whether parameters and results are stored on the @@ -303,7 +303,7 @@ where // the component is disallowed, for example, when the `realloc` function // calls a canonical import. if !flags.may_leave() { - return Err(Trap::new("cannot leave component instance").into()); + bail!("cannot leave component instance"); } let args; diff --git a/crates/wasmtime/src/component/func/options.rs b/crates/wasmtime/src/component/func/options.rs index 7c07e59004ae..f2df2bba5a33 100644 --- a/crates/wasmtime/src/component/func/options.rs +++ b/crates/wasmtime/src/component/func/options.rs @@ -1,6 +1,5 @@ use crate::store::{StoreId, StoreOpaque}; use crate::StoreContextMut; -use crate::Trap; use anyhow::{bail, Result}; use std::ptr::NonNull; use wasmtime_environ::component::StringEncoding; @@ -97,7 +96,7 @@ impl Options { }; if result % old_align != 0 { - bail!(Trap::new("realloc return: result not aligned")); + bail!("realloc return: result not aligned"); } let result = usize::try_from(result)?; @@ -105,7 +104,7 @@ impl Options { let result_slice = match memory.get_mut(result..).and_then(|s| s.get_mut(..new_size)) { Some(end) => end, - None => bail!(Trap::new("realloc return: beyond end of memory")), + None => bail!("realloc return: beyond end of memory"), }; Ok((result_slice, result)) diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 519d636ef9b8..d35b3c987cc6 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1889,16 +1889,15 @@ unsafe impl wasmtime_runtime::Store for StoreInner { } fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { - let trap = || anyhow::Error::from(Trap::new("all fuel consumed by WebAssembly")); return match &mut self.out_of_gas_behavior { - OutOfGas::Trap => Err(trap()), + OutOfGas::Trap => Err(Trap::out_of_fuel().into()), #[cfg(feature = "async")] OutOfGas::InjectFuel { injection_count, fuel_to_inject, } => { if *injection_count == 0 { - return Err(trap()); + return Err(Trap::out_of_fuel().into()); } *injection_count -= 1; let fuel = *fuel_to_inject; diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index e7b726885b48..6065f5a717e1 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -22,9 +22,6 @@ struct TrapInner { /// State describing the occasion which evoked a trap. #[derive(Debug)] enum TrapReason { - /// An error message describing a trap. - Message(String), - /// A specific code for a trap triggered while executing WASM. InstructionTrap(TrapCode), } @@ -89,6 +86,13 @@ pub enum TrapCode { /// generates a function that always traps and, when called, produces this /// flavor of trap. AlwaysTrapAdapter, + + /// When wasm code is configured to consume fuel and it runs out of fuel + /// then this trap will be raised. + /// + /// For more information see + /// [`Config::consume_fuel`](crate::Config::consume_fuel). + OutOfFuel, } impl TrapCode { @@ -116,26 +120,13 @@ fn _assert_trap_is_sync_and_send(t: &Trap) -> (&dyn Sync, &dyn Send) { } impl Trap { - /// Creates a new `Trap` with `message`. - /// - /// # Example - /// - /// ``` - /// let trap = wasmtime::Trap::new("unexpected error"); - /// assert!(trap.to_string().contains("unexpected error")); - /// ``` - #[cold] // traps are exceptional, this helps move handling off the main path - pub fn new>(message: I) -> Self { - let reason = TrapReason::Message(message.into()); - Trap::new_with_trace(reason, None) - } - // Same safety requirements and caveats as // `wasmtime_runtime::raise_user_trap`. pub(crate) unsafe fn raise(error: anyhow::Error) -> ! { - let needs_backtrace = error - .downcast_ref::() - .map_or(true, |trap| trap.trace().is_none()); + let needs_backtrace = match error.downcast_ref::() { + Some(trap) => trap.trace().is_none(), + None => error.downcast_ref::().is_none(), + }; wasmtime_runtime::raise_user_trap(error, needs_backtrace) } @@ -201,6 +192,11 @@ impl Trap { Trap::new_with_trace(TrapReason::InstructionTrap(code), backtrace) } + #[cold] // see Trap::new + pub(crate) fn out_of_fuel() -> Self { + Trap::new_with_trace(TrapReason::InstructionTrap(TrapCode::OutOfFuel), None) + } + /// Creates a new `Trap`. /// * `reason` - this is the wasmtime-internal reason for why this trap is /// being created. @@ -252,7 +248,6 @@ impl Trap { pub fn trap_code(&self) -> Option { match self.inner.reason { TrapReason::InstructionTrap(code) => Some(code), - _ => None, } } @@ -301,7 +296,6 @@ impl std::error::Error for Trap {} impl fmt::Display for TrapReason { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - TrapReason::Message(s) => write!(f, "{}", s), TrapReason::InstructionTrap(code) => write!(f, "wasm trap: {}", code), } } @@ -323,6 +317,7 @@ impl fmt::Display for TrapCode { UnreachableCodeReached => "wasm `unreachable` instruction executed", Interrupt => "interrupt", AlwaysTrapAdapter => "degenerate component adapter called", + OutOfFuel => "all fuel consumed by WebAssembly", }; write!(f, "{}", desc) } diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index a8460ab2c508..0a4754840e49 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -2,7 +2,7 @@ use crate::component; use crate::core; use crate::spectest::*; -use anyhow::{anyhow, bail, Context as _, Result}; +use anyhow::{anyhow, bail, Context as _, Error, Result}; use std::path::Path; use std::str; use wasmtime::*; @@ -24,7 +24,7 @@ pub struct WastContext { enum Outcome { Ok(T), - Trap(Trap), + Trap(Error), } impl Outcome { @@ -35,7 +35,7 @@ impl Outcome { } } - fn into_result(self) -> Result { + fn into_result(self) -> Result { match self { Outcome::Ok(t) => Ok(t), Outcome::Trap(t) => Err(t), @@ -111,22 +111,24 @@ impl WastContext { fn instantiate_module(&mut self, module: &[u8]) -> Result> { let module = Module::new(self.store.engine(), module)?; - let instance = match self.core_linker.instantiate(&mut self.store, &module) { - Ok(i) => i, - Err(e) => return e.downcast::().map(Outcome::Trap), - }; - Ok(Outcome::Ok(instance)) + Ok( + match self.core_linker.instantiate(&mut self.store, &module) { + Ok(i) => Outcome::Ok(i), + Err(e) => Outcome::Trap(e), + }, + ) } #[cfg(feature = "component-model")] fn instantiate_component(&mut self, module: &[u8]) -> Result> { let engine = self.store.engine(); let module = component::Component::new(engine, module)?; - let instance = match self.component_linker.instantiate(&mut self.store, &module) { - Ok(i) => i, - Err(e) => return e.downcast::().map(Outcome::Trap), - }; - Ok(Outcome::Ok(instance)) + Ok( + match self.component_linker.instantiate(&mut self.store, &module) { + Ok(i) => Outcome::Ok(i), + Err(e) => Outcome::Trap(e), + }, + ) } /// Register "spectest" which is used by the spec testsuite. @@ -174,7 +176,7 @@ impl WastContext { let mut results = vec![Val::null(); func.ty(&self.store).results().len()]; Ok(match func.call(&mut self.store, &values, &mut results) { Ok(()) => Outcome::Ok(Results::Core(results.into())), - Err(e) => Outcome::Trap(e.downcast()?), + Err(e) => Outcome::Trap(e), }) } #[cfg(feature = "component-model")] @@ -200,7 +202,7 @@ impl WastContext { func.post_return(&mut self.store)?; Outcome::Ok(Results::Component(results.into())) } - Err(e) => Outcome::Trap(e.downcast()?), + Err(e) => Outcome::Trap(e), }) } } @@ -330,7 +332,7 @@ impl WastContext { Outcome::Ok(values) => bail!("expected trap, got {:?}", values), Outcome::Trap(t) => t, }; - let actual = trap.to_string(); + let actual = format!("{trap:?}"); if actual.contains(expected) // `bulk-memory-operations/bulk.wast` checks for a message that // specifies which element is uninitialized, but our traps don't diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index 369f842ab5e0..9867e893f00c 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -1,3 +1,4 @@ +use anyhow::{bail, Result}; use std::fmt; use std::slice; use std::str; @@ -909,9 +910,7 @@ impl Pointee for str { } } -pub fn run_in_dummy_executor( - future: F, -) -> Result { +pub fn run_in_dummy_executor(future: F) -> Result { use std::pin::Pin; use std::task::{Context, Poll, RawWaker, RawWakerVTable, Waker}; @@ -921,7 +920,7 @@ pub fn run_in_dummy_executor( match f.as_mut().poll(&mut cx) { Poll::Ready(val) => return Ok(val), Poll::Pending => - return Err(wasmtime_crate::Trap::new("Cannot wait on pending future: must enable wiggle \"async\" future and execute on an async Store")) + bail!("Cannot wait on pending future: must enable wiggle \"async\" future and execute on an async Store"), } fn dummy_waker() -> Waker { diff --git a/crates/wiggle/tests/wasmtime_sync.rs b/crates/wiggle/tests/wasmtime_sync.rs index 3410e530ea4b..43c303286bc3 100644 --- a/crates/wiggle/tests/wasmtime_sync.rs +++ b/crates/wiggle/tests/wasmtime_sync.rs @@ -132,7 +132,7 @@ fn test_async_host_func_pending() { ) .unwrap_err(); assert!( - format!("{}", trap).contains("Cannot wait on pending future"), + format!("{:?}", trap).contains("Cannot wait on pending future"), "expected get a pending future Trap from dummy executor, got: {}", trap ); diff --git a/tests/all/async_functions.rs b/tests/all/async_functions.rs index d6c85bc4c9cd..b341c69dbe05 100644 --- a/tests/all/async_functions.rs +++ b/tests/all/async_functions.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{anyhow, bail, Result}; use std::future::Future; use std::pin::Pin; use std::task::{Context, Poll, RawWaker, RawWakerVTable, Waker}; @@ -440,7 +440,7 @@ async fn resume_separate_thread() { let func = Func::wrap0_async(&mut store, |_| { Box::new(async { tokio::task::yield_now().await; - Err::<(), _>(wasmtime::Trap::new("test").into()) + Err::<(), _>(anyhow!("test")) }) }); let result = Instance::new_async(&mut store, &module, &[func.into()]).await; @@ -493,7 +493,7 @@ async fn resume_separate_thread3() { // situation we'll set up the TLS info so it's in place while the body of // the function executes... let mut store = Store::new(&Engine::default(), None); - let f = Func::wrap(&mut store, move |mut caller: Caller<'_, _>| { + let f = Func::wrap(&mut store, move |mut caller: Caller<'_, _>| -> Result<()> { // ... and the execution of this host-defined function (while the TLS // info is initialized), will set up a recursive call into wasm. This // recursive call will be done asynchronously so we can suspend it @@ -536,7 +536,7 @@ async fn resume_separate_thread3() { // ... all in all this function will need access to the original TLS // information to raise the trap. This TLS information should be // restored even though the asynchronous execution is suspended. - Err::<(), _>(wasmtime::Trap::new("").into()) + bail!("") }); assert!(f.call(&mut store, &[], &mut []).is_err()); } diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 42520a523862..0e002bd267e3 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -252,11 +252,7 @@ fn exit125_wasi_snapshot1() -> Result<()> { fn exit126_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot0.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - if cfg!(windows) { - assert_eq!(output.status.code().unwrap(), 3); - } else { - assert_eq!(output.status.code().unwrap(), 128 + libc::SIGABRT); - } + assert_eq!(output.status.code().unwrap(), 1); assert!(output.stdout.is_empty()); assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status")); Ok(()) @@ -267,11 +263,7 @@ fn exit126_wasi_snapshot0() -> Result<()> { fn exit126_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot1.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - if cfg!(windows) { - assert_eq!(output.status.code().unwrap(), 3); - } else { - assert_eq!(output.status.code().unwrap(), 128 + libc::SIGABRT); - } + assert_eq!(output.status.code().unwrap(), 1); assert!(output.stdout.is_empty()); assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status")); Ok(()) diff --git a/tests/all/component_model/import.rs b/tests/all/component_model/import.rs index c5f47071998e..4c72f5f2304e 100644 --- a/tests/all/component_model/import.rs +++ b/tests/all/component_model/import.rs @@ -2,7 +2,7 @@ use super::REALLOC_AND_FREE; use anyhow::Result; use std::ops::Deref; use wasmtime::component::*; -use wasmtime::{Store, StoreContextMut, Trap}; +use wasmtime::{BacktraceContext, Store, StoreContextMut}; #[test] fn can_compile() -> Result<()> { @@ -256,15 +256,13 @@ fn attempt_to_leave_during_malloc() -> Result<()> { .instantiate(&mut store, &component)? .get_typed_func::<(), (), _>(&mut store, "run")? .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + .unwrap_err(); assert!( - trap.to_string().contains("cannot leave component instance"), - "bad trap: {}", - trap, + format!("{trap:?}").contains("cannot leave component instance"), + "bad trap: {trap:?}", ); - let trace = trap.trace().unwrap(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 4); // This was our entry point... @@ -294,12 +292,10 @@ fn attempt_to_leave_during_malloc() -> Result<()> { .instantiate(&mut store, &component)? .get_typed_func::<(&str,), (), _>(&mut store, "take-string")? .call(&mut store, ("x",)) - .unwrap_err() - .downcast::()?; + .unwrap_err(); assert!( - trap.to_string().contains("cannot leave component instance"), - "bad trap: {}", - trap, + format!("{trap:?}").contains("cannot leave component instance"), + "bad trap: {trap:?}", ); Ok(()) } @@ -344,10 +340,8 @@ fn attempt_to_reenter_during_host() -> Result<()> { let func = store.data_mut().func.take().unwrap(); let trap = func.call(&mut store, ()).unwrap_err(); assert!( - trap.to_string() - .contains("cannot reenter component instance"), - "bad trap: {}", - trap, + format!("{trap:?}").contains("cannot reenter component instance"), + "bad trap: {trap:?}", ); Ok(()) }, @@ -372,10 +366,8 @@ fn attempt_to_reenter_during_host() -> Result<()> { let func = store.data_mut().func.take().unwrap(); let trap = func.call(&mut store, &[], &mut []).unwrap_err(); assert!( - trap.to_string() - .contains("cannot reenter component instance"), - "bad trap: {}", - trap, + format!("{trap:?}").contains("cannot reenter component instance"), + "bad trap: {trap:?}", ); Ok(()) }, diff --git a/tests/all/fuel.rs b/tests/all/fuel.rs index ccc683e63d71..a42c3084d9b7 100644 --- a/tests/all/fuel.rs +++ b/tests/all/fuel.rs @@ -117,7 +117,7 @@ fn iloop() { store.add_fuel(10_000).unwrap(); let error = Instance::new(&mut store, &module, &[]).err().unwrap(); assert!( - error.to_string().contains("all fuel consumed"), + format!("{:?}", error).contains("all fuel consumed"), "bad error: {}", error ); @@ -173,8 +173,11 @@ fn host_function_consumes_all() { let export = instance .get_typed_func::<(), (), _>(&mut store, "") .unwrap(); - let trap = export.call(&mut store, ()).err().unwrap().to_string(); - assert!(trap.contains("all fuel consumed"), "bad error: {}", trap); + let trap = export.call(&mut store, ()).unwrap_err(); + assert!( + format!("{trap:?}").contains("all fuel consumed"), + "bad error: {trap:?}" + ); } #[test] diff --git a/tests/all/func.rs b/tests/all/func.rs index 924de965f0fc..d56d44e5008f 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst}; use std::sync::Arc; use wasmtime::*; @@ -220,13 +220,8 @@ fn import_works() -> Result<()> { #[test] fn trap_smoke() -> Result<()> { let mut store = Store::<()>::default(); - let f = Func::wrap(&mut store, || -> Result<()> { - Err(Trap::new("test").into()) - }); - let err = f - .call(&mut store, &[], &mut []) - .unwrap_err() - .downcast::()?; + let f = Func::wrap(&mut store, || -> Result<()> { bail!("test") }); + let err = f.call(&mut store, &[], &mut []).unwrap_err(); assert!(err.to_string().contains("test")); Ok(()) } @@ -241,13 +236,8 @@ fn trap_import() -> Result<()> { )?; let mut store = Store::<()>::default(); let module = Module::new(store.engine(), &wasm)?; - let import = Func::wrap(&mut store, || -> Result<()> { - Err(Trap::new("foo").into()) - }); - let trap = Instance::new(&mut store, &module, &[import.into()]) - .err() - .unwrap() - .downcast::()?; + let import = Func::wrap(&mut store, || -> Result<()> { bail!("foo") }); + let trap = Instance::new(&mut store, &module, &[import.into()]).unwrap_err(); assert!(trap.to_string().contains("foo")); Ok(()) } @@ -618,7 +608,7 @@ fn trap_doesnt_leak() -> anyhow::Result<()> { let dtor1_run = canary1.0.clone(); let f1 = Func::wrap(&mut store, move || -> Result<()> { drop(&canary1); - Err(Trap::new("").into()) + bail!("") }); assert!(f1.typed::<(), (), _>(&store)?.call(&mut store, ()).is_err()); assert!(f1.call(&mut store, &[], &mut []).is_err()); @@ -628,7 +618,7 @@ fn trap_doesnt_leak() -> anyhow::Result<()> { let dtor2_run = canary2.0.clone(); let f2 = Func::new(&mut store, FuncType::new(None, None), move |_, _, _| { drop(&canary2); - Err(Trap::new("").into()) + bail!("") }); assert!(f2.typed::<(), (), _>(&store)?.call(&mut store, ()).is_err()); assert!(f2.call(&mut store, &[], &mut []).is_err()); diff --git a/tests/all/host_funcs.rs b/tests/all/host_funcs.rs index 665eb4f6181c..9b96b69ee7e4 100644 --- a/tests/all/host_funcs.rs +++ b/tests/all/host_funcs.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use wasmtime::*; use wasmtime_wasi::sync::WasiCtxBuilder; @@ -445,16 +445,13 @@ fn call_wasm_many_args() -> Result<()> { fn trap_smoke() -> Result<()> { let engine = Engine::default(); let mut linker = Linker::<()>::new(&engine); - linker.func_wrap("", "", || -> Result<()> { Err(Trap::new("test").into()) })?; + linker.func_wrap("", "", || -> Result<()> { bail!("test") })?; let mut store = Store::new(&engine, ()); let f = linker.get(&mut store, "", "").unwrap().into_func().unwrap(); - let err = f - .call(&mut store, &[], &mut []) - .unwrap_err() - .downcast::()?; + let err = f.call(&mut store, &[], &mut []).unwrap_err(); assert!(err.to_string().contains("test")); @@ -472,16 +469,12 @@ fn trap_import() -> Result<()> { let engine = Engine::default(); let mut linker = Linker::new(&engine); - linker.func_wrap("", "", || -> Result<()> { Err(Trap::new("foo").into()) })?; + linker.func_wrap("", "", || -> Result<()> { bail!("foo") })?; let module = Module::new(&engine, &wasm)?; let mut store = Store::new(&engine, ()); - let trap = linker - .instantiate(&mut store, &module) - .err() - .unwrap() - .downcast::()?; + let trap = linker.instantiate(&mut store, &module).unwrap_err(); assert!(trap.to_string().contains("foo")); diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 00be807a2fb0..93be69f16462 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use std::panic::{self, AssertUnwindSafe}; use std::process::Command; use wasmtime::*; @@ -15,20 +15,17 @@ fn test_trap_return() -> Result<()> { let module = Module::new(store.engine(), wat)?; let hello_type = FuncType::new(None, None); - let hello_func = Func::new(&mut store, hello_type, |_, _, _| { - Err(Trap::new("test 123").into()) - }); + let hello_func = Func::new(&mut store, hello_type, |_, _, _| bail!("test 123")); let instance = Instance::new(&mut store, &module, &[hello_func.into()])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; let e = run_func.call(&mut store, ()).unwrap_err(); - let trap = e.downcast_ref::().expect("error is a Trap"); - assert!(trap.to_string().contains("test 123")); + assert!(format!("{e:?}").contains("test 123")); assert!( - e.downcast_ref::().is_none(), - "error should not contain a BacktraceContext" + e.downcast_ref::().is_some(), + "error should contain a BacktraceContext" ); Ok(()) @@ -234,12 +231,8 @@ fn test_trap_backtrace_disabled() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; - - assert!(e.trace().is_none(), "backtraces should be disabled"); + let e = run_func.call(&mut store, ()).unwrap_err(); + assert!(e.downcast_ref::().is_none()); Ok(()) } @@ -255,26 +248,21 @@ fn test_trap_trace_cb() -> Result<()> { "#; let fn_type = FuncType::new(None, None); - let fn_func = Func::new(&mut store, fn_type, |_, _, _| { - Err(Trap::new("cb throw").into()) - }); + let fn_func = Func::new(&mut store, fn_type, |_, _, _| bail!("cb throw")); let module = Module::new(store.engine(), wat)?; let instance = Instance::new(&mut store, &module, &[fn_func.into()])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.trace().expect("backtrace is available"); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].module_name().unwrap(), "hello_mod"); assert_eq!(trace[0].func_index(), 2); assert_eq!(trace[1].module_name().unwrap(), "hello_mod"); assert_eq!(trace[1].func_index(), 1); - assert!(e.to_string().contains("cb throw")); + assert!(format!("{e:?}").contains("cb throw")); Ok(()) } @@ -404,19 +392,9 @@ fn trap_start_function_import() -> Result<()> { let module = Module::new(store.engine(), &binary)?; let sig = FuncType::new(None, None); - let func = Func::new( - &mut store, - sig, - |_, _, _| Err(Trap::new("user trap").into()), - ); - let err = Instance::new(&mut store, &module, &[func.into()]) - .err() - .unwrap(); - assert!(err - .downcast_ref::() - .unwrap() - .to_string() - .contains("user trap")); + let func = Func::new(&mut store, sig, |_, _, _| bail!("user trap")); + let err = Instance::new(&mut store, &module, &[func.into()]).unwrap_err(); + assert!(err.to_string().contains("user trap")); Ok(()) } From 5819d258486c6d21fbb1f74c389b722ab51000c4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 10:52:47 -0700 Subject: [PATCH 08/12] Merge `Trap` and `TrapCode` With prior refactorings there's no more need for `Trap` to be opaque or otherwise contain a backtrace. This commit parse down `Trap` to simply an `enum` which was the old `TrapCode`. All various tests and such were updated to handle this. The main consequence of this commit is that all errors have a `BacktraceContext` context attached to them. This unfortunately means that the backtrace is printed first before the error message or trap code, but given all the prior simplifications that seems worth it at this time. --- crates/c-api/src/trap.rs | 41 ++- crates/fuzzing/src/oracles.rs | 11 +- crates/fuzzing/src/oracles/diff_v8.rs | 19 +- crates/fuzzing/src/oracles/diff_wasmi.rs | 33 ++- crates/fuzzing/src/oracles/diff_wasmtime.rs | 12 +- crates/fuzzing/src/oracles/stacks.rs | 17 +- crates/wasmtime/src/externals.rs | 6 +- crates/wasmtime/src/instance.rs | 2 +- crates/wasmtime/src/store.rs | 9 +- crates/wasmtime/src/trap.rs | 266 +++++--------------- examples/interrupt.rs | 8 +- tests/all/async_functions.rs | 5 +- tests/all/component_model/async.rs | 4 +- tests/all/component_model/func.rs | 14 +- tests/all/component_model/post_return.rs | 4 +- tests/all/component_model/strings.rs | 8 +- tests/all/custom_signal_handler.rs | 16 +- tests/all/iloop.rs | 24 +- tests/all/pooling_allocator.rs | 32 ++- tests/all/stack_overflow.rs | 8 +- tests/all/traps.rs | 165 +++++------- 21 files changed, 244 insertions(+), 460 deletions(-) diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 676e49fce6cc..21e68ec81552 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -1,7 +1,7 @@ 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)] pub struct wasm_trap_t { @@ -76,11 +76,11 @@ pub extern "C" fn wasm_trap_message(trap: &wasm_trap_t, out: &mut wasm_message_t #[no_mangle] pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option> { - let trap = match raw.error.downcast_ref::() { + let trap = match raw.error.downcast_ref::() { Some(trap) => trap, None => return None, }; - if trap.trace().unwrap_or(&[]).len() > 0 { + if trap.frames().len() > 0 { Some(Box::new(wasm_frame_t { trap: trap.clone(), idx: 0, @@ -117,26 +117,21 @@ pub extern "C" fn wasmtime_trap_code(raw: &wasm_trap_t, code: &mut i32) -> bool Some(trap) => trap, None => return false, }; - match 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, - } + *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] diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs index 78ae1f0d50e7..90fa8b6edfda 100644 --- a/crates/fuzzing/src/oracles.rs +++ b/crates/fuzzing/src/oracles.rs @@ -374,8 +374,7 @@ pub fn differential( // falls through to checking the intermediate state otherwise. (Err(lhs), Err(rhs)) => { let err = rhs.downcast::().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); @@ -675,11 +674,9 @@ pub fn table_ops( .downcast::() .unwrap(); - match trap.trap_code() { - Some(TrapCode::TableOutOfBounds) | Some(TrapCode::OutOfFuel) => {} - _ => { - panic!("unexpected trap: {}", trap); - } + match trap { + Trap::TableOutOfBounds | Trap::OutOfFuel => {} + _ => panic!("unexpected trap: {trap}"), } // Do a final GC after running the Wasm. diff --git a/crates/fuzzing/src/oracles/diff_v8.rs b/crates/fuzzing/src/oracles/diff_v8.rs index 8b8efd79d275..1d03c200411c 100644 --- a/crates/fuzzing/src/oracles/diff_v8.rs +++ b/crates/fuzzing/src/oracles/diff_v8.rs @@ -5,7 +5,6 @@ use std::cell::RefCell; use std::rc::Rc; use std::sync::Once; use wasmtime::Trap; -use wasmtime::TrapCode; pub struct V8Engine { isolate: Rc>, @@ -91,14 +90,14 @@ impl DiffEngine for V8Engine { v8 ); }; - match wasmtime.trap_code() { - Some(TrapCode::MemoryOutOfBounds) => { + match wasmtime { + Trap::MemoryOutOfBounds => { return verify_v8(&[ "memory access out of bounds", "data segment is out of bounds", ]) } - Some(TrapCode::UnreachableCodeReached) => { + Trap::UnreachableCodeReached => { return verify_v8(&[ "unreachable", // All the wasms we test use wasm-smith's @@ -113,10 +112,10 @@ impl DiffEngine for V8Engine { "Maximum call stack size exceeded", ]); } - Some(TrapCode::IntegerDivisionByZero) => { + Trap::IntegerDivisionByZero => { return verify_v8(&["divide by zero", "remainder by zero"]) } - Some(TrapCode::StackOverflow) => { + Trap::StackOverflow => { return verify_v8(&[ "call stack size exceeded", // Similar to the above comment in `UnreachableCodeReached` @@ -128,15 +127,15 @@ impl DiffEngine for V8Engine { "unreachable", ]); } - Some(TrapCode::IndirectCallToNull) => return verify_v8(&["null function"]), - Some(TrapCode::TableOutOfBounds) => { + Trap::IndirectCallToNull => return verify_v8(&["null function"]), + Trap::TableOutOfBounds => { return verify_v8(&[ "table initializer is out of bounds", "table index is out of bounds", ]) } - Some(TrapCode::BadSignature) => return verify_v8(&["function signature mismatch"]), - Some(TrapCode::IntegerOverflow) | Some(TrapCode::BadConversionToInteger) => { + Trap::BadSignature => return verify_v8(&["function signature mismatch"]), + Trap::IntegerOverflow | Trap::BadConversionToInteger => { return verify_v8(&[ "float unrepresentable in integer range", "divide result unrepresentable", diff --git a/crates/fuzzing/src/oracles/diff_wasmi.rs b/crates/fuzzing/src/oracles/diff_wasmi.rs index 01ff817c4b16..0864d34d5431 100644 --- a/crates/fuzzing/src/oracles/diff_wasmi.rs +++ b/crates/fuzzing/src/oracles/diff_wasmi.rs @@ -3,7 +3,7 @@ use crate::generators::{Config, DiffValue, DiffValueType}; use crate::oracles::engine::{DiffEngine, DiffInstance}; use anyhow::{Context, Error, Result}; -use wasmtime::{Trap, TrapCode}; +use wasmtime::Trap; /// A wrapper for `wasmi` as a [`DiffEngine`]. pub struct WasmiEngine { @@ -55,8 +55,8 @@ impl DiffEngine for WasmiEngine { // Wasmtime reports as a `MemoryOutOfBounds`. Some(wasmi::Error::Memory(msg)) => { assert_eq!( - trap.trap_code(), - Some(TrapCode::MemoryOutOfBounds), + *trap, + Trap::MemoryOutOfBounds, "wasmtime error did not match wasmi: {msg}" ); return; @@ -77,10 +77,7 @@ impl DiffEngine for WasmiEngine { .expect(&format!("not a trap: {:?}", err)), }; assert!(wasmi.as_code().is_some()); - assert_eq!( - wasmi.as_code().map(wasmi_to_wasmtime_trap_code), - trap.trap_code(), - ); + assert_eq!(wasmi_to_wasmtime_trap_code(wasmi.as_code().unwrap()), *trap); } fn is_stack_overflow(&self, err: &Error) -> bool { @@ -97,18 +94,18 @@ impl DiffEngine for WasmiEngine { } /// Converts `wasmi` trap code to `wasmtime` trap code. -fn wasmi_to_wasmtime_trap_code(trap: wasmi::core::TrapCode) -> wasmtime::TrapCode { - use wasmi::core::TrapCode as WasmiTrapCode; +fn wasmi_to_wasmtime_trap_code(trap: wasmi::core::TrapCode) -> Trap { + use wasmi::core::TrapCode; match trap { - WasmiTrapCode::Unreachable => TrapCode::UnreachableCodeReached, - WasmiTrapCode::MemoryAccessOutOfBounds => TrapCode::MemoryOutOfBounds, - WasmiTrapCode::TableAccessOutOfBounds => TrapCode::TableOutOfBounds, - WasmiTrapCode::ElemUninitialized => TrapCode::IndirectCallToNull, - WasmiTrapCode::DivisionByZero => TrapCode::IntegerDivisionByZero, - WasmiTrapCode::IntegerOverflow => TrapCode::IntegerOverflow, - WasmiTrapCode::InvalidConversionToInt => TrapCode::BadConversionToInteger, - WasmiTrapCode::StackOverflow => TrapCode::StackOverflow, - WasmiTrapCode::UnexpectedSignature => TrapCode::BadSignature, + TrapCode::Unreachable => Trap::UnreachableCodeReached, + TrapCode::MemoryAccessOutOfBounds => Trap::MemoryOutOfBounds, + TrapCode::TableAccessOutOfBounds => Trap::TableOutOfBounds, + TrapCode::ElemUninitialized => Trap::IndirectCallToNull, + TrapCode::DivisionByZero => Trap::IntegerDivisionByZero, + TrapCode::IntegerOverflow => Trap::IntegerOverflow, + TrapCode::InvalidConversionToInt => Trap::BadConversionToInteger, + TrapCode::StackOverflow => Trap::StackOverflow, + TrapCode::UnexpectedSignature => Trap::BadSignature, } } diff --git a/crates/fuzzing/src/oracles/diff_wasmtime.rs b/crates/fuzzing/src/oracles/diff_wasmtime.rs index 29d0e0847545..f2a31aee1fd1 100644 --- a/crates/fuzzing/src/oracles/diff_wasmtime.rs +++ b/crates/fuzzing/src/oracles/diff_wasmtime.rs @@ -6,7 +6,7 @@ use crate::oracles::engine::DiffInstance; use crate::oracles::{compile_module, engine::DiffEngine, StoreLimits}; use anyhow::{Context, Error, Result}; use arbitrary::Unstructured; -use wasmtime::{Extern, FuncType, Instance, Module, Store, Trap, TrapCode, Val}; +use wasmtime::{Extern, FuncType, Instance, Module, Store, Trap, Val}; /// A wrapper for using Wasmtime as a [`DiffEngine`]. pub struct WasmtimeEngine { @@ -45,18 +45,12 @@ impl DiffEngine for WasmtimeEngine { let trap2 = err .downcast_ref::() .expect(&format!("not a trap: {:?}", err)); - assert_eq!( - trap.trap_code(), - trap2.trap_code(), - "{}\nis not equal to\n{}", - trap, - trap2 - ); + assert_eq!(trap, trap2, "{}\nis not equal to\n{}", trap, trap2); } fn is_stack_overflow(&self, err: &Error) -> bool { match err.downcast_ref::() { - Some(trap) => trap.trap_code() == Some(TrapCode::StackOverflow), + Some(trap) => *trap == Trap::StackOverflow, None => false, } } diff --git a/crates/fuzzing/src/oracles/stacks.rs b/crates/fuzzing/src/oracles/stacks.rs index 8aaf4effd133..f397614952b3 100644 --- a/crates/fuzzing/src/oracles/stacks.rs +++ b/crates/fuzzing/src/oracles/stacks.rs @@ -27,7 +27,7 @@ pub fn check_stacks(stacks: Stacks) -> usize { let fuel_left = fuel.get(&mut caller).unwrap_i32(); if fuel_left == 0 { - bail!("out of fuel") + bail!(Trap::OutOfFuel); } fuel.set(&mut caller, Val::I32(fuel_left - 1)).unwrap(); @@ -73,15 +73,10 @@ pub fn check_stacks(stacks: Stacks) -> usize { .get_memory(&mut store, "memory") .expect("should have `memory` export"); - let (host_trace, code) = match trap.downcast_ref::() { - Some(trap) => (trap.trace().unwrap(), trap.trap_code()), - None => ( - trap.downcast_ref::().unwrap().frames(), - None, - ), - }; + let host_trace = trap.downcast_ref::().unwrap().frames(); + let trap = trap.downcast_ref::().unwrap(); max_stack_depth = max_stack_depth.max(host_trace.len()); - assert_stack_matches(&mut store, memory, ptr, len, host_trace, code); + assert_stack_matches(&mut store, memory, ptr, len, host_trace, *trap); } } max_stack_depth @@ -94,7 +89,7 @@ fn assert_stack_matches( ptr: u32, len: u32, host_trace: &[FrameInfo], - trap_code: Option, + trap: Trap, ) { let mut data = vec![0; len as usize]; memory @@ -115,7 +110,7 @@ fn assert_stack_matches( // be able to see the exact function that triggered the stack overflow. In // this situation the host trace is asserted to be one larger and then the // top frame (first) of the host trace is discarded. - let host_trace = if trap_code == Some(TrapCode::StackOverflow) { + let host_trace = if trap == Trap::StackOverflow { assert_eq!(host_trace.len(), wasm_trace.len() + 1); &host_trace[1..] } else { diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 953801d2739c..11368216c2ed 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -464,7 +464,7 @@ impl Table { let table = Table::from_wasmtime_table(wasmtime_export, store); (*table.wasmtime_table(store, std::iter::empty())) .fill(0, init, ty.minimum()) - .map_err(|c| Trap::new_wasm(c, None))?; + .map_err(|c| Trap::from_env(c))?; Ok(table) } @@ -653,7 +653,7 @@ impl Table { let src_table = src_table.wasmtime_table(store, src_range); unsafe { runtime::Table::copy(dst_table, src_table, dst_index, src_index, len) - .map_err(|c| Trap::new_wasm(c, None))?; + .map_err(|c| Trap::from_env(c))?; } Ok(()) } @@ -683,7 +683,7 @@ impl Table { unsafe { (*table) .fill(dst, val, len) - .map_err(|c| Trap::new_wasm(c, None))?; + .map_err(|c| Trap::from_env(c))?; } Ok(()) diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 3a7523efdfec..de6b33f50cf2 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -324,7 +324,7 @@ impl Instance { ) .map_err(|e| -> Error { match e { - InstantiationError::Trap(trap) => Trap::new_wasm(trap, None).into(), + InstantiationError::Trap(trap) => Trap::from_env(trap).into(), other => other.into(), } })?; diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index d35b3c987cc6..ae0753ad3a24 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1890,14 +1890,14 @@ unsafe impl wasmtime_runtime::Store for StoreInner { fn out_of_gas(&mut self) -> Result<(), anyhow::Error> { return match &mut self.out_of_gas_behavior { - OutOfGas::Trap => Err(Trap::out_of_fuel().into()), + OutOfGas::Trap => Err(Trap::OutOfFuel.into()), #[cfg(feature = "async")] OutOfGas::InjectFuel { injection_count, fuel_to_inject, } => { if *injection_count == 0 { - return Err(Trap::out_of_fuel().into()); + return Err(Trap::OutOfFuel.into()); } *injection_count -= 1; let fuel = *fuel_to_inject; @@ -1914,10 +1914,7 @@ unsafe impl wasmtime_runtime::Store for StoreInner { fn new_epoch(&mut self) -> Result { return match &mut self.epoch_deadline_behavior { - EpochDeadline::Trap => { - let trap = Trap::new_wasm(wasmtime_environ::TrapCode::Interrupt, None); - Err(anyhow::Error::from(trap)) - } + EpochDeadline::Trap => Err(Trap::Interrupt.into()), EpochDeadline::Callback(callback) => { let delta = callback(&mut self.data)?; // Set a new deadline and return the new epoch deadline so diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 6065f5a717e1..5d3b832dee6d 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -1,52 +1,28 @@ use crate::store::StoreOpaque; use crate::Module; use anyhow::Error; -use once_cell::sync::OnceCell; use std::fmt; -use std::sync::Arc; -use wasmtime_environ::{EntityRef, FilePos, TrapCode as EnvTrapCode}; +use wasmtime_environ::{EntityRef, FilePos, TrapCode}; use wasmtime_jit::{demangle_function_name, demangle_function_name_or_index}; -/// A struct representing an aborted instruction execution, with a message -/// indicating the cause. -#[derive(Clone)] -pub struct Trap { - inner: Arc, -} - -struct TrapInner { - reason: TrapReason, - backtrace: OnceCell, -} - -/// State describing the occasion which evoked a trap. -#[derive(Debug)] -enum TrapReason { - /// A specific code for a trap triggered while executing WASM. - InstructionTrap(TrapCode), -} - -#[derive(Debug)] -pub(crate) struct TrapBacktrace { - wasm_trace: Vec, - runtime_trace: wasmtime_runtime::Backtrace, - hint_wasm_backtrace_details_env: bool, -} - -/// A trap code describing the reason for a trap. -/// -/// All trap instructions have an explicit trap code. -/// -/// The code can be accessed from the c-api, where the possible values are translated -/// into enum values defined there: +/// Description of a trap which can happen in WebAssembly, halting wasm +/// execution. /// -/// * `wasm_trap_code` in c-api/src/trap.rs, and -/// * `wasmtime_trap_code_enum` in c-api/include/wasmtime/trap.h. -/// -/// These need to be kept in sync. +/// This enumeration is a list of all possible traps that can happen in wasm, in +/// addition to some Wasmtime-specific trap codes listed here as well. This type +/// will be returned as an error when a wasm function hits one of these trap +/// conditions through the [`anyhow::Error`] wrapper. +// +// The code can be accessed from the c-api, where the possible values are translated +// into enum values defined there: +// +// * `wasm_trap_code` in c-api/src/trap.rs, and +// * `wasmtime_trap_code_enum` in c-api/include/wasmtime/trap.h. +// +// These need to be kept in sync. #[non_exhaustive] #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] -pub enum TrapCode { +pub enum Trap { /// The current stack space was exhausted. StackOverflow, @@ -95,38 +71,19 @@ pub enum TrapCode { OutOfFuel, } -impl TrapCode { - /// Panics if `code` is `EnvTrapCode::User`. - fn from_non_user(code: EnvTrapCode) -> Self { - match code { - EnvTrapCode::StackOverflow => TrapCode::StackOverflow, - EnvTrapCode::HeapOutOfBounds => TrapCode::MemoryOutOfBounds, - EnvTrapCode::HeapMisaligned => TrapCode::HeapMisaligned, - EnvTrapCode::TableOutOfBounds => TrapCode::TableOutOfBounds, - EnvTrapCode::IndirectCallToNull => TrapCode::IndirectCallToNull, - EnvTrapCode::BadSignature => TrapCode::BadSignature, - EnvTrapCode::IntegerOverflow => TrapCode::IntegerOverflow, - EnvTrapCode::IntegerDivisionByZero => TrapCode::IntegerDivisionByZero, - EnvTrapCode::BadConversionToInteger => TrapCode::BadConversionToInteger, - EnvTrapCode::UnreachableCodeReached => TrapCode::UnreachableCodeReached, - EnvTrapCode::Interrupt => TrapCode::Interrupt, - EnvTrapCode::AlwaysTrapAdapter => TrapCode::AlwaysTrapAdapter, - } - } -} - -fn _assert_trap_is_sync_and_send(t: &Trap) -> (&dyn Sync, &dyn Send) { - (t, t) +#[derive(Debug)] +pub(crate) struct TrapBacktrace { + wasm_trace: Vec, + #[allow(dead_code)] + runtime_trace: wasmtime_runtime::Backtrace, + hint_wasm_backtrace_details_env: bool, } impl Trap { // Same safety requirements and caveats as // `wasmtime_runtime::raise_user_trap`. pub(crate) unsafe fn raise(error: anyhow::Error) -> ! { - let needs_backtrace = match error.downcast_ref::() { - Some(trap) => trap.trace().is_none(), - None => error.downcast_ref::().is_none(), - }; + let needs_backtrace = error.downcast_ref::().is_none(); wasmtime_runtime::raise_user_trap(error, needs_backtrace) } @@ -136,7 +93,7 @@ impl Trap { runtime_trap: Box, ) -> Error { let wasmtime_runtime::Trap { reason, backtrace } = *runtime_trap; - match reason { + let (error, pc) = match reason { // For user-defined errors they're already an `anyhow::Error` so no // conversion is really necessary here, but a `backtrace` may have // been captured so it's attempted to get inserted here. @@ -151,159 +108,59 @@ impl Trap { // otherwise the information about what the wasm was doing when the // error was generated would be lost. wasmtime_runtime::TrapReason::User { - mut error, + error, needs_backtrace, } => { - if let Some(backtrace) = backtrace { - debug_assert!(needs_backtrace); - let bt = TrapBacktrace::new(store, backtrace, None); - match error.downcast_mut::() { - Some(trap) => { - debug_assert!(trap.inner.backtrace.get().is_none()); - trap.record_backtrace(bt); - } - None => { - if !bt.wasm_trace.is_empty() { - error = error.context(BacktraceContext(bt)); - } - } - } - } - error + debug_assert!(needs_backtrace == backtrace.is_some()); + (error, None) } wasmtime_runtime::TrapReason::Jit(pc) => { let code = store .modules() .lookup_trap_code(pc) - .unwrap_or(EnvTrapCode::StackOverflow); - let backtrace = backtrace.map(|bt| TrapBacktrace::new(store, bt, Some(pc))); - Trap::new_wasm(code, backtrace).into() + .unwrap_or(TrapCode::StackOverflow); + (Trap::from_env(code).into(), Some(pc)) } wasmtime_runtime::TrapReason::Wasm(trap_code) => { - let backtrace = backtrace.map(|bt| TrapBacktrace::new(store, bt, None)); - Trap::new_wasm(trap_code, backtrace).into() + (Trap::from_env(trap_code).into(), None) } - } - } - - #[cold] // see Trap::new - pub(crate) fn new_wasm(code: EnvTrapCode, backtrace: Option) -> Self { - let code = TrapCode::from_non_user(code); - Trap::new_with_trace(TrapReason::InstructionTrap(code), backtrace) - } - - #[cold] // see Trap::new - pub(crate) fn out_of_fuel() -> Self { - Trap::new_with_trace(TrapReason::InstructionTrap(TrapCode::OutOfFuel), None) - } - - /// Creates a new `Trap`. - /// * `reason` - this is the wasmtime-internal reason for why this trap is - /// being created. - /// - /// * `backtrace` - this is a captured backtrace from when the trap - /// occurred. Contains the native backtrace, and the backtrace of - /// WebAssembly frames. - fn new_with_trace(reason: TrapReason, backtrace: Option) -> Self { - let backtrace = if let Some(bt) = backtrace { - OnceCell::with_value(bt) - } else { - OnceCell::new() }; - Trap { - inner: Arc::new(TrapInner { reason, backtrace }), - } - } - - /// Displays the error reason for this trap. - /// - /// In particular, it differs from this struct's `Display` by *only* - /// showing the reason, and not the full backtrace. This is useful to - /// customize the way the trap is reported, for instance to display a short - /// message for user-facing errors. - pub fn display_reason<'a>(&'a self) -> impl fmt::Display + 'a { - &self.inner.reason - } - - /// Returns a list of function frames in WebAssembly code that led to this - /// trap happening. - /// - /// This function return an `Option` of a list of frames to indicate that - /// wasm frames are not always available. Frames will never be available if - /// backtraces are disabled via - /// [`Config::wasm_backtrace`](crate::Config::wasm_backtrace). Frames will - /// also not be available for freshly-created traps. WebAssembly frames are - /// currently only captured when the trap reaches wasm itself to get raised - /// across a wasm boundary. - pub fn trace(&self) -> Option<&[FrameInfo]> { - self.inner - .backtrace - .get() - .as_ref() - .map(|bt| bt.wasm_trace.as_slice()) - } - - /// Code of a trap that happened while executing a WASM instruction. - /// If the trap was triggered by a host export this will be `None`. - pub fn trap_code(&self) -> Option { - match self.inner.reason { - TrapReason::InstructionTrap(code) => Some(code), + match backtrace { + Some(bt) => { + let bt = TrapBacktrace::new(store, bt, pc); + if bt.wasm_trace.is_empty() { + error + } else { + error.context(BacktraceContext(bt)) + } + } + None => error, } } - fn record_backtrace(&self, backtrace: TrapBacktrace) { - // When a trap is created on top of the wasm stack, the trampoline will - // re-raise it via - // `wasmtime_runtime::raise_user_trap(trap.into::>(), - // ..)` after `panic::catch_unwind`. We don't want to overwrite the - // first backtrace recorded, as it is most precise. However, this should - // never happen in the first place because we thread `needs_backtrace` - // booleans throuch all calls to `raise_user_trap` to avoid capturing - // unnecessary backtraces! So debug assert that we don't ever capture - // unnecessary backtraces. - let result = self.inner.backtrace.try_insert(backtrace); - debug_assert!(result.is_ok()); - } -} - -impl fmt::Debug for Trap { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut f = f.debug_struct("Trap"); - f.field("reason", &self.inner.reason); - if let Some(backtrace) = self.inner.backtrace.get() { - f.field("wasm_trace", &backtrace.wasm_trace) - .field("runtime_trace", &backtrace.runtime_trace); + /// Panics if `code` is `TrapCode::User`. + pub(crate) fn from_env(code: TrapCode) -> Self { + match code { + TrapCode::StackOverflow => Trap::StackOverflow, + TrapCode::HeapOutOfBounds => Trap::MemoryOutOfBounds, + TrapCode::HeapMisaligned => Trap::HeapMisaligned, + TrapCode::TableOutOfBounds => Trap::TableOutOfBounds, + TrapCode::IndirectCallToNull => Trap::IndirectCallToNull, + TrapCode::BadSignature => Trap::BadSignature, + TrapCode::IntegerOverflow => Trap::IntegerOverflow, + TrapCode::IntegerDivisionByZero => Trap::IntegerDivisionByZero, + TrapCode::BadConversionToInteger => Trap::BadConversionToInteger, + TrapCode::UnreachableCodeReached => Trap::UnreachableCodeReached, + TrapCode::Interrupt => Trap::Interrupt, + TrapCode::AlwaysTrapAdapter => Trap::AlwaysTrapAdapter, } - f.finish() } } impl fmt::Display for Trap { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.inner.reason)?; + use Trap::*; - if let Some(trace) = self.inner.backtrace.get().as_ref() { - if !trace.wasm_trace.is_empty() { - write!(f, "\n{trace}")?; - } - } - Ok(()) - } -} - -impl std::error::Error for Trap {} - -impl fmt::Display for TrapReason { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - TrapReason::InstructionTrap(code) => write!(f, "wasm trap: {}", code), - } - } -} - -impl fmt::Display for TrapCode { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use TrapCode::*; let desc = match self { StackOverflow => "call stack exhausted", MemoryOutOfBounds => "out of bounds memory access", @@ -319,10 +176,12 @@ impl fmt::Display for TrapCode { AlwaysTrapAdapter => "degenerate component adapter called", OutOfFuel => "all fuel consumed by WebAssembly", }; - write!(f, "{}", desc) + write!(f, "wasm trap: {desc}") } } +impl std::error::Error for Trap {} + impl TrapBacktrace { pub fn new( store: &StoreOpaque, @@ -460,6 +319,7 @@ impl fmt::Display for TrapBacktrace { /// Describes the context (backtrace) at which a user's error terminated (trapped) /// WebAssembly execution +#[derive(Debug)] pub struct BacktraceContext(TrapBacktrace); impl fmt::Display for BacktraceContext { @@ -468,12 +328,6 @@ impl fmt::Display for BacktraceContext { } } -impl fmt::Debug for BacktraceContext { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", self.0) - } -} - impl BacktraceContext { /// Returns a list of function frames in WebAssembly code that led to this /// trap happening. diff --git a/examples/interrupt.rs b/examples/interrupt.rs index 1fa639060b5a..749a63304584 100644 --- a/examples/interrupt.rs +++ b/examples/interrupt.rs @@ -26,14 +26,10 @@ fn main() -> Result<()> { }); println!("Entering infinite loop ..."); - let trap = run - .call(&mut store, ()) - .unwrap_err() - .downcast::() - .unwrap(); + let err = run.call(&mut store, ()).unwrap_err(); println!("trap received..."); - assert!(trap.trap_code().unwrap() == TrapCode::Interrupt); + assert_eq!(err.downcast::()?, Trap::Interrupt); Ok(()) } diff --git a/tests/all/async_functions.rs b/tests/all/async_functions.rs index b341c69dbe05..ee57764fa019 100644 --- a/tests/all/async_functions.rs +++ b/tests/all/async_functions.rs @@ -565,9 +565,8 @@ async fn recursive_async() -> Result<()> { .call_async(&mut caller, ()) .await .unwrap_err() - .downcast::() - .unwrap(); - assert_eq!(err.trap_code(), Some(TrapCode::StackOverflow)); + .downcast::()?; + assert_eq!(err, Trap::StackOverflow); Ok(()) }) }); diff --git a/tests/all/component_model/async.rs b/tests/all/component_model/async.rs index 4098d84caed7..816a02f1e137 100644 --- a/tests/all/component_model/async.rs +++ b/tests/all/component_model/async.rs @@ -1,6 +1,6 @@ use anyhow::Result; use wasmtime::component::*; -use wasmtime::{Store, StoreContextMut, Trap, TrapCode}; +use wasmtime::{Store, StoreContextMut, Trap}; /// This is super::func::thunks, except with an async store. #[tokio::test] @@ -38,7 +38,7 @@ async fn smoke() -> Result<()> { .call_async(&mut store, ()) .await .unwrap_err(); - assert!(err.downcast::()?.trap_code() == Some(TrapCode::UnreachableCodeReached)); + assert_eq!(err.downcast::()?, Trap::UnreachableCodeReached); Ok(()) } diff --git a/tests/all/component_model/func.rs b/tests/all/component_model/func.rs index 5c3027a5efbe..fded264cbc91 100644 --- a/tests/all/component_model/func.rs +++ b/tests/all/component_model/func.rs @@ -3,7 +3,7 @@ use anyhow::Result; use std::rc::Rc; use std::sync::Arc; use wasmtime::component::*; -use wasmtime::{Store, StoreContextMut, Trap, TrapCode}; +use wasmtime::{Store, StoreContextMut, Trap}; const CANON_32BIT_NAN: u32 = 0b01111111110000000000000000000000; const CANON_64BIT_NAN: u64 = 0b0111111111111000000000000000000000000000000000000000000000000000; @@ -37,7 +37,7 @@ fn thunks() -> Result<()> { .get_typed_func::<(), (), _>(&mut store, "thunk-trap")? .call(&mut store, ()) .unwrap_err(); - assert!(err.downcast::()?.trap_code() == Some(TrapCode::UnreachableCodeReached)); + assert_eq!(err.downcast::()?, Trap::UnreachableCodeReached); Ok(()) } @@ -1099,7 +1099,7 @@ fn some_traps() -> Result<()> { .call(&mut store, (&[],)) .unwrap_err() .downcast::()?; - assert_eq!(err.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(err, Trap::UnreachableCodeReached); // This should fail when calling the allocator function for the argument let err = instance(&mut store)? @@ -1107,7 +1107,7 @@ fn some_traps() -> Result<()> { .call(&mut store, ("",)) .unwrap_err() .downcast::()?; - assert_eq!(err.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(err, Trap::UnreachableCodeReached); // This should fail when calling the allocator function for the space // to store the arguments (before arguments are even lowered) @@ -1119,7 +1119,7 @@ fn some_traps() -> Result<()> { .call(&mut store, ("", "", "", "", "", "", "", "", "", "")) .unwrap_err() .downcast::()?; - assert_eq!(err.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(err, Trap::UnreachableCodeReached); // Assert that when the base pointer returned by malloc is out of bounds // that errors are reported as such. Both empty and lists with contents @@ -2375,8 +2375,8 @@ fn errors_that_poison_instance() -> Result<()> { Err(e) => e, }; assert_eq!( - err.downcast::().unwrap().trap_code(), - Some(TrapCode::UnreachableCodeReached) + err.downcast::().unwrap(), + Trap::UnreachableCodeReached ); } diff --git a/tests/all/component_model/post_return.rs b/tests/all/component_model/post_return.rs index 9079cc78d2c1..56bfa516618b 100644 --- a/tests/all/component_model/post_return.rs +++ b/tests/all/component_model/post_return.rs @@ -1,6 +1,6 @@ use anyhow::Result; use wasmtime::component::*; -use wasmtime::{Store, StoreContextMut, Trap, TrapCode}; +use wasmtime::{Store, StoreContextMut, Trap}; #[test] fn invalid_api() -> Result<()> { @@ -284,7 +284,7 @@ fn trap_in_post_return_poisons_instance() -> Result<()> { let f = instance.get_typed_func::<(), (), _>(&mut store, "f")?; f.call(&mut store, ())?; let trap = f.post_return(&mut store).unwrap_err().downcast::()?; - assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(trap, Trap::UnreachableCodeReached); let err = f.call(&mut store, ()).unwrap_err(); assert!( err.to_string() diff --git a/tests/all/component_model/strings.rs b/tests/all/component_model/strings.rs index bda6172e37b5..7f85b67fea58 100644 --- a/tests/all/component_model/strings.rs +++ b/tests/all/component_model/strings.rs @@ -1,7 +1,7 @@ use super::REALLOC_AND_FREE; use anyhow::Result; use wasmtime::component::{Component, Linker}; -use wasmtime::{Engine, Store, StoreContextMut, Trap, TrapCode}; +use wasmtime::{Engine, Store, StoreContextMut, Trap}; const UTF16_TAG: u32 = 1 << 31; @@ -248,7 +248,7 @@ fn test_ptr_out_of_bounds(engine: &Engine, src: &str, dst: &str) -> Result<()> { .err() .unwrap() .downcast::()?; - assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(trap, Trap::UnreachableCodeReached); Ok(()) }; @@ -322,7 +322,7 @@ fn test_ptr_overflow(engine: &Engine, src: &str, dst: &str) -> Result<()> { .call(&mut store, (size,)) .unwrap_err() .downcast::()?; - assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(trap, Trap::UnreachableCodeReached); Ok(()) }; @@ -422,7 +422,7 @@ fn test_realloc_oob(engine: &Engine, src: &str, dst: &str) -> Result<()> { let instance = Linker::new(engine).instantiate(&mut store, &component)?; let func = instance.get_typed_func::<(), (), _>(&mut store, "f")?; let trap = func.call(&mut store, ()).unwrap_err().downcast::()?; - assert_eq!(trap.trap_code(), Some(TrapCode::UnreachableCodeReached)); + assert_eq!(trap, Trap::UnreachableCodeReached); Ok(()) } diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 91077b12b91e..22cf65416209 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -170,12 +170,7 @@ mod tests { let trap = invoke_export(&mut store, instance, "read_out_of_bounds") .unwrap_err() .downcast::()?; - assert!( - trap.to_string() - .contains("wasm trap: out of bounds memory access"), - "bad trap message: {:?}", - trap.to_string() - ); + assert_eq!(trap, Trap::MemoryOutOfBounds); } // these invoke wasmtime_call_trampoline from callable.rs @@ -192,10 +187,11 @@ mod tests { let read_out_of_bounds_func = instance.get_typed_func::<(), i32, _>(&mut store, "read_out_of_bounds")?; println!("calling read_out_of_bounds..."); - let trap = read_out_of_bounds_func.call(&mut store, ()).unwrap_err(); - assert!(trap - .to_string() - .contains("wasm trap: out of bounds memory access")); + let trap = read_out_of_bounds_func + .call(&mut store, ()) + .unwrap_err() + .downcast::()?; + assert_eq!(trap, Trap::MemoryOutOfBounds); } Ok(()) } diff --git a/tests/all/iloop.rs b/tests/all/iloop.rs index 1e6227edd154..68614ddd7719 100644 --- a/tests/all/iloop.rs +++ b/tests/all/iloop.rs @@ -32,11 +32,7 @@ fn loops_interruptable() -> anyhow::Result<()> { let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; store.engine().increment_epoch(); let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; - assert!( - trap.trap_code().unwrap() == TrapCode::Interrupt, - "bad message: {}", - trap - ); + assert_eq!(trap, Trap::Interrupt); Ok(()) } @@ -49,11 +45,7 @@ fn functions_interruptable() -> anyhow::Result<()> { let iloop = instance.get_typed_func::<(), (), _>(&mut store, "loop")?; store.engine().increment_epoch(); let trap = iloop.call(&mut store, ()).unwrap_err().downcast::()?; - assert!( - trap.trap_code().unwrap() == TrapCode::Interrupt, - "{}", - trap.to_string() - ); + assert_eq!(trap, Trap::Interrupt); Ok(()) } @@ -102,11 +94,7 @@ fn loop_interrupt_from_afar() -> anyhow::Result<()> { STOP.store(true, SeqCst); thread.join().unwrap(); assert!(HITS.load(SeqCst) > NUM_HITS); - assert!( - trap.trap_code().unwrap() == TrapCode::Interrupt, - "bad message: {}", - trap.to_string() - ); + assert_eq!(trap, Trap::Interrupt); Ok(()) } @@ -142,10 +130,6 @@ fn function_interrupt_from_afar() -> anyhow::Result<()> { STOP.store(true, SeqCst); thread.join().unwrap(); assert!(HITS.load(SeqCst) > NUM_HITS); - assert!( - trap.trap_code().unwrap() == TrapCode::Interrupt, - "bad message: {}", - trap.to_string() - ); + assert_eq!(trap, Trap::Interrupt); Ok(()) } diff --git a/tests/all/pooling_allocator.rs b/tests/all/pooling_allocator.rs index e00a55b2f84a..9b389cfd1c6c 100644 --- a/tests/all/pooling_allocator.rs +++ b/tests/all/pooling_allocator.rs @@ -167,20 +167,32 @@ fn memory_guard_page_trap() -> Result<()> { let m = instance.get_memory(&mut store, "m").unwrap(); let f = instance.get_typed_func::(&mut store, "f")?; - let trap = f.call(&mut store, 0).expect_err("function should trap"); - assert!(trap.to_string().contains("out of bounds")); - - let trap = f.call(&mut store, 1).expect_err("function should trap"); - assert!(trap.to_string().contains("out of bounds")); + let trap = f + .call(&mut store, 0) + .expect_err("function should trap") + .downcast::()?; + assert_eq!(trap, Trap::MemoryOutOfBounds); + + let trap = f + .call(&mut store, 1) + .expect_err("function should trap") + .downcast::()?; + assert_eq!(trap, Trap::MemoryOutOfBounds); m.grow(&mut store, 1).expect("memory should grow"); f.call(&mut store, 0).expect("function should not trap"); - let trap = f.call(&mut store, 65536).expect_err("function should trap"); - assert!(trap.to_string().contains("out of bounds")); - - let trap = f.call(&mut store, 65537).expect_err("function should trap"); - assert!(trap.to_string().contains("out of bounds")); + let trap = f + .call(&mut store, 65536) + .expect_err("function should trap") + .downcast::()?; + assert_eq!(trap, Trap::MemoryOutOfBounds); + + let trap = f + .call(&mut store, 65537) + .expect_err("function should trap") + .downcast::()?; + assert_eq!(trap, Trap::MemoryOutOfBounds); m.grow(&mut store, 1).expect("memory should grow"); f.call(&mut store, 65536).expect("function should not trap"); diff --git a/tests/all/stack_overflow.rs b/tests/all/stack_overflow.rs index 52ff8e238162..9cd90eb9c8ed 100644 --- a/tests/all/stack_overflow.rs +++ b/tests/all/stack_overflow.rs @@ -28,12 +28,8 @@ fn host_always_has_some_stack() -> anyhow::Result<()> { // Make sure that our function traps and the trap says that the call stack // has been exhausted. - let trap = foo.call(&mut store, ()).unwrap_err(); - assert!( - trap.to_string().contains("call stack exhausted"), - "{}", - trap.to_string() - ); + let trap = foo.call(&mut store, ()).unwrap_err().downcast::()?; + assert_eq!(trap, Trap::StackOverflow); // Additionally, however, and this is the crucial test, make sure that the // host function actually completed. If HITS is 1 then we entered but didn't diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 93be69f16462..b03328e8c9d5 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, Result}; +use anyhow::{bail, Error, Result}; use std::panic::{self, AssertUnwindSafe}; use std::process::Command; use wasmtime::*; @@ -123,12 +123,9 @@ fn test_trap_trace() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.trace().expect("backtrace is available"); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].module_name().unwrap(), "hello_mod"); assert_eq!(trace[0].func_index(), 1); @@ -140,11 +137,7 @@ fn test_trap_trace() -> Result<()> { assert_eq!(trace[1].func_name(), None); assert_eq!(trace[1].func_offset(), Some(1)); assert_eq!(trace[1].module_offset(), Some(0x21)); - assert!( - e.to_string().contains("unreachable"), - "wrong message: {}", - e.to_string() - ); + assert_eq!(e.downcast::()?, Trap::UnreachableCodeReached); Ok(()) } @@ -204,8 +197,8 @@ fn test_trap_through_host() -> Result<()> { &[host_func_a.into(), host_func_b.into()], )?; let a = instance.get_typed_func::<(), (), _>(&mut store, "a")?; - let err = a.call(&mut store, ()).unwrap_err().downcast::()?; - let trace = err.trace().expect("backtrace is available"); + let err = a.call(&mut store, ()).unwrap_err(); + let trace = err.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 3); assert_eq!(trace[0].func_name(), Some("c")); assert_eq!(trace[1].func_name(), Some("b")); @@ -280,19 +273,16 @@ fn test_trap_stack_overflow() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.trace().expect("backtrace is available"); + let trace = e.downcast_ref::().unwrap().frames(); assert!(trace.len() >= 32); for i in 0..trace.len() { assert_eq!(trace[i].module_name().unwrap(), "rec_mod"); assert_eq!(trace[i].func_index(), 0); assert_eq!(trace[i].func_name(), Some("run")); } - assert!(e.to_string().contains("call stack exhausted")); + assert_eq!(e.downcast::()?, Trap::StackOverflow); Ok(()) } @@ -313,19 +303,18 @@ fn trap_display_pretty() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "bar")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + let e = run_func.call(&mut store, ()).unwrap_err(); assert_eq!( - e.to_string(), + format!("{:?}", e), "\ -wasm trap: wasm `unreachable` instruction executed -wasm backtrace: +error while executing at wasm backtrace: 0: 0x23 - m!die 1: 0x27 - m! 2: 0x2c - m!foo - 3: 0x31 - m!\ + 3: 0x31 - m! + +Caused by: + wasm trap: wasm `unreachable` instruction executed\ " ); Ok(()) @@ -358,21 +347,20 @@ fn trap_display_multi_module() -> Result<()> { let instance = Instance::new(&mut store, &module, &[bar])?; let bar2 = instance.get_typed_func::<(), (), _>(&mut store, "bar2")?; - let e = bar2 - .call(&mut store, ()) - .err() - .expect("error calling function"); + let e = bar2.call(&mut store, ()).unwrap_err(); assert_eq!( - e.to_string(), + format!("{e:?}"), "\ -wasm trap: wasm `unreachable` instruction executed -wasm backtrace: +error while executing at wasm backtrace: 0: 0x23 - a!die 1: 0x27 - a! 2: 0x2c - a!foo 3: 0x31 - a! 4: 0x29 - b!middle - 5: 0x2e - b!\ + 5: 0x2e - b! + +Caused by: + wasm trap: wasm `unreachable` instruction executed\ " ); Ok(()) @@ -394,7 +382,7 @@ fn trap_start_function_import() -> Result<()> { let sig = FuncType::new(None, None); let func = Func::new(&mut store, sig, |_, _, _| bail!("user trap")); let err = Instance::new(&mut store, &module, &[func.into()]).unwrap_err(); - assert!(err.to_string().contains("user trap")); + assert!(format!("{err:?}").contains("user trap")); Ok(()) } @@ -481,8 +469,8 @@ fn rust_catch_panic_import() -> Result<()> { let instance = Instance::new(&mut store, &module, &[panic.into(), catch_panic.into()])?; let run = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let trap = run.call(&mut store, ()).unwrap_err().downcast::()?; - let trace = trap.trace().unwrap(); + let trap = run.call(&mut store, ()).unwrap_err(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_index(), 3); assert_eq!(num_panics.load(std::sync::atomic::Ordering::SeqCst), 2); @@ -601,18 +589,20 @@ fn start_trap_pretty() -> Result<()> { let module = Module::new(store.engine(), wat)?; let e = match Instance::new(&mut store, &module, &[]) { Ok(_) => panic!("expected failure"), - Err(e) => e.downcast::()?, + Err(e) => e, }; assert_eq!( - e.to_string(), + format!("{e:?}"), "\ -wasm trap: wasm `unreachable` instruction executed -wasm backtrace: +error while executing at wasm backtrace: 0: 0x1d - m!die 1: 0x21 - m! 2: 0x26 - m!foo - 3: 0x2b - m!start\ + 3: 0x2b - m!start + +Caused by: + wasm trap: wasm `unreachable` instruction executed\ " ); Ok(()) @@ -626,22 +616,22 @@ fn present_after_module_drop() -> Result<()> { let func = instance.get_typed_func::<(), (), _>(&mut store, "foo")?; println!("asserting before we drop modules"); - assert_trap(func.call(&mut store, ()).unwrap_err().downcast()?); + assert_trap(func.call(&mut store, ()).unwrap_err()); drop((instance, module)); println!("asserting after drop"); - assert_trap(func.call(&mut store, ()).unwrap_err().downcast()?); + assert_trap(func.call(&mut store, ()).unwrap_err()); return Ok(()); - fn assert_trap(t: Trap) { - println!("{}", t); - let trace = t.trace().expect("backtrace is available"); + fn assert_trap(t: Error) { + println!("{:?}", t); + let trace = t.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_index(), 0); } } -fn assert_trap_code(wat: &str, code: wasmtime::TrapCode) { +fn assert_trap_code(wat: &str, code: wasmtime::Trap) { let mut store = Store::<()>::default(); let module = Module::new(store.engine(), wat).unwrap(); @@ -650,7 +640,7 @@ fn assert_trap_code(wat: &str, code: wasmtime::TrapCode) { Err(e) => e, }; let trap = err.downcast_ref::().unwrap(); - assert_eq!(trap.trap_code(), Some(code)); + assert_eq!(*trap, code); } #[test] @@ -663,7 +653,7 @@ fn heap_out_of_bounds_trap() { (start $start) ) "#, - TrapCode::MemoryOutOfBounds, + Trap::MemoryOutOfBounds, ); assert_trap_code( @@ -674,7 +664,7 @@ fn heap_out_of_bounds_trap() { (start $start) ) "#, - TrapCode::MemoryOutOfBounds, + Trap::MemoryOutOfBounds, ); } @@ -725,13 +715,11 @@ fn parse_dwarf_info() -> Result<()> { ); linker.module(&mut store, "", &module)?; let run = linker.get_default(&mut store, "")?; - let trap = run - .call(&mut store, &[], &mut []) - .unwrap_err() - .downcast::()?; + let trap = run.call(&mut store, &[], &mut []).unwrap_err(); let mut found = false; - for frame in trap.trace().expect("backtrace is available") { + let frames = trap.downcast_ref::().unwrap().frames(); + for frame in frames { for symbol in frame.symbols() { if let Some(file) = symbol.file() { if file.ends_with("input.rs") { @@ -763,16 +751,15 @@ fn no_hint_even_with_dwarf_info() -> Result<()> { ) "#, )?; - let trap = Instance::new(&mut store, &module, &[]) - .err() - .unwrap() - .downcast::()?; + let trap = Instance::new(&mut store, &module, &[]).unwrap_err(); assert_eq!( - trap.to_string(), + format!("{trap:?}"), "\ -wasm trap: wasm `unreachable` instruction executed -wasm backtrace: - 0: 0x1a - !start\ +error while executing at wasm backtrace: + 0: 0x1a - !start + +Caused by: + wasm trap: wasm `unreachable` instruction executed\ " ); Ok(()) @@ -797,17 +784,16 @@ fn hint_with_dwarf_info() -> Result<()> { ) "#, )?; - let trap = Instance::new(&mut store, &module, &[]) - .err() - .unwrap() - .downcast::()?; + let trap = Instance::new(&mut store, &module, &[]).unwrap_err(); assert_eq!( - trap.to_string(), + format!("{trap:?}"), "\ -wasm trap: wasm `unreachable` instruction executed -wasm backtrace: +error while executing at wasm backtrace: 0: 0x1a - !start -note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information\ +note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information + +Caused by: + wasm trap: wasm `unreachable` instruction executed\ " ); Ok(()) @@ -859,12 +845,9 @@ fn traps_without_address_map() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; - let e = run_func - .call(&mut store, ()) - .unwrap_err() - .downcast::()?; + let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.trace().expect("backtrace is available"); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].func_name(), Some("hello")); assert_eq!(trace[0].func_index(), 1); @@ -911,16 +894,13 @@ fn catch_trap_calling_across_stores() -> Result<()> { .get_typed_func::<(), (), _>(&mut data.child_store, "trap") .expect("trap function should be exported"); - let trap = func - .call(&mut data.child_store, ()) - .unwrap_err() - .downcast::()?; + let trap = func.call(&mut data.child_store, ()).unwrap_err(); assert!( - trap.to_string().contains("unreachable"), - "trap should contain 'unreachable', got: {trap}" + format!("{trap:?}").contains("unreachable"), + "trap should contain 'unreachable', got: {trap:?}" ); - let trace = trap.trace().unwrap(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_name(), Some("trap")); @@ -1028,13 +1008,9 @@ async fn async_then_sync_trap() -> Result<()> { let a = async_instance .get_typed_func::<(), (), _>(&mut async_store, "a") .unwrap(); - let trap = a - .call_async(&mut async_store, ()) - .await - .unwrap_err() - .downcast::()?; + let trap = a.call_async(&mut async_store, ()).await.unwrap_err(); - let trace = trap.trace().unwrap(); + let trace = trap.downcast_ref::().unwrap().frames(); // We don't support cross-store or cross-engine symbolication currently, so // the other frames are ignored. assert_eq!(trace.len(), 1); @@ -1113,12 +1089,9 @@ async fn sync_then_async_trap() -> Result<()> { let a = sync_instance .get_typed_func::<(), (), _>(&mut sync_store, "a") .unwrap(); - let trap = a - .call(&mut sync_store, ()) - .unwrap_err() - .downcast::()?; + let trap = a.call(&mut sync_store, ()).unwrap_err(); - let trace = trap.trace().unwrap(); + let trace = trap.downcast_ref::().unwrap().frames(); // We don't support cross-store or cross-engine symbolication currently, so // the other frames are ignored. assert_eq!(trace.len(), 1); From 9b0275e932d22993652b3ac755e4f3b56c33db0d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 11:32:41 -0700 Subject: [PATCH 09/12] Rename `BacktraceContext` to `WasmBacktrace` This feels like a better name given how this has turned out, and additionally this commit removes having both `WasmBacktrace` and `BacktraceContext`. --- crates/fuzzing/src/oracles/stacks.rs | 2 +- crates/wasmtime/src/trap.rs | 58 +++++++++++----------------- tests/all/component_model/import.rs | 4 +- tests/all/traps.rs | 34 ++++++++-------- 4 files changed, 43 insertions(+), 55 deletions(-) diff --git a/crates/fuzzing/src/oracles/stacks.rs b/crates/fuzzing/src/oracles/stacks.rs index f397614952b3..3216ed10c802 100644 --- a/crates/fuzzing/src/oracles/stacks.rs +++ b/crates/fuzzing/src/oracles/stacks.rs @@ -73,7 +73,7 @@ pub fn check_stacks(stacks: Stacks) -> usize { .get_memory(&mut store, "memory") .expect("should have `memory` export"); - let host_trace = trap.downcast_ref::().unwrap().frames(); + let host_trace = trap.downcast_ref::().unwrap().frames(); let trap = trap.downcast_ref::().unwrap(); max_stack_depth = max_stack_depth.max(host_trace.len()); assert_stack_matches(&mut store, memory, ptr, len, host_trace, *trap); diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 5d3b832dee6d..7840726d6a36 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -71,23 +71,15 @@ pub enum Trap { OutOfFuel, } -#[derive(Debug)] -pub(crate) struct TrapBacktrace { - wasm_trace: Vec, - #[allow(dead_code)] - runtime_trace: wasmtime_runtime::Backtrace, - hint_wasm_backtrace_details_env: bool, -} - impl Trap { // Same safety requirements and caveats as // `wasmtime_runtime::raise_user_trap`. pub(crate) unsafe fn raise(error: anyhow::Error) -> ! { - let needs_backtrace = error.downcast_ref::().is_none(); + let needs_backtrace = error.downcast_ref::().is_none(); wasmtime_runtime::raise_user_trap(error, needs_backtrace) } - #[cold] // see Trap::new + #[cold] // traps are exceptional, this helps move handling off the main path pub(crate) fn from_runtime_box( store: &StoreOpaque, runtime_trap: Box, @@ -127,11 +119,11 @@ impl Trap { }; match backtrace { Some(bt) => { - let bt = TrapBacktrace::new(store, bt, pc); + let bt = WasmBacktrace::new(store, bt, pc); if bt.wasm_trace.is_empty() { error } else { - error.context(BacktraceContext(bt)) + error.context(bt) } } None => error, @@ -182,8 +174,17 @@ impl fmt::Display for Trap { impl std::error::Error for Trap {} -impl TrapBacktrace { - pub fn new( +/// todo +#[derive(Debug)] +pub struct WasmBacktrace { + wasm_trace: Vec, + #[allow(dead_code)] + runtime_trace: wasmtime_runtime::Backtrace, + hint_wasm_backtrace_details_env: bool, +} + +impl WasmBacktrace { + fn new( store: &StoreOpaque, runtime_trace: wasmtime_runtime::Backtrace, trap_pc: Option, @@ -258,11 +259,17 @@ impl TrapBacktrace { hint_wasm_backtrace_details_env, } } + + /// Returns a list of function frames in WebAssembly this backtrace + /// represents. + pub fn frames(&self) -> &[FrameInfo] { + self.wasm_trace.as_slice() + } } -impl fmt::Display for TrapBacktrace { +impl fmt::Display for WasmBacktrace { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "wasm backtrace:")?; + writeln!(f, "error while executing at wasm backtrace:")?; let mut needs_newline = false; for (i, frame) in self.wasm_trace.iter().enumerate() { @@ -317,25 +324,6 @@ impl fmt::Display for TrapBacktrace { } } -/// Describes the context (backtrace) at which a user's error terminated (trapped) -/// WebAssembly execution -#[derive(Debug)] -pub struct BacktraceContext(TrapBacktrace); - -impl fmt::Display for BacktraceContext { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "error while executing at {}", self.0) - } -} - -impl BacktraceContext { - /// Returns a list of function frames in WebAssembly code that led to this - /// trap happening. - pub fn frames(&self) -> &[FrameInfo] { - self.0.wasm_trace.as_slice() - } -} - /// Description of a frame in a backtrace for a [`Trap`] or [`BacktraceContext`]. /// /// Whenever a WebAssembly trap occurs an instance of [`Trap`] is created. Each diff --git a/tests/all/component_model/import.rs b/tests/all/component_model/import.rs index 4c72f5f2304e..9f2654ba7482 100644 --- a/tests/all/component_model/import.rs +++ b/tests/all/component_model/import.rs @@ -2,7 +2,7 @@ use super::REALLOC_AND_FREE; use anyhow::Result; use std::ops::Deref; use wasmtime::component::*; -use wasmtime::{BacktraceContext, Store, StoreContextMut}; +use wasmtime::{Store, StoreContextMut, WasmBacktrace}; #[test] fn can_compile() -> Result<()> { @@ -262,7 +262,7 @@ fn attempt_to_leave_during_malloc() -> Result<()> { "bad trap: {trap:?}", ); - let trace = trap.downcast_ref::().unwrap().frames(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 4); // This was our entry point... diff --git a/tests/all/traps.rs b/tests/all/traps.rs index b03328e8c9d5..c01ddb88e633 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -24,8 +24,8 @@ fn test_trap_return() -> Result<()> { assert!(format!("{e:?}").contains("test 123")); assert!( - e.downcast_ref::().is_some(), - "error should contain a BacktraceContext" + e.downcast_ref::().is_some(), + "error should contain a WasmBacktrace" ); Ok(()) @@ -55,7 +55,7 @@ fn test_anyhow_error_return() -> Result<()> { assert!(format!("{:?}", e).contains("Caused by:\n test 1234")); assert!(e.downcast_ref::().is_none()); - assert!(e.downcast_ref::().is_some()); + assert!(e.downcast_ref::().is_some()); Ok(()) } @@ -101,8 +101,8 @@ fn test_trap_return_downcast() -> Result<()> { e.downcast_ref::() .expect("error downcasts to MyTrap"); let bt = e - .downcast_ref::() - .expect("error downcasts to BacktraceContext"); + .downcast_ref::() + .expect("error downcasts to WasmBacktrace"); assert_eq!(bt.frames().len(), 1); println!("{:?}", bt); @@ -125,7 +125,7 @@ fn test_trap_trace() -> Result<()> { let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.downcast_ref::().unwrap().frames(); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].module_name().unwrap(), "hello_mod"); assert_eq!(trace[0].func_index(), 1); @@ -198,7 +198,7 @@ fn test_trap_through_host() -> Result<()> { )?; let a = instance.get_typed_func::<(), (), _>(&mut store, "a")?; let err = a.call(&mut store, ()).unwrap_err(); - let trace = err.downcast_ref::().unwrap().frames(); + let trace = err.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 3); assert_eq!(trace[0].func_name(), Some("c")); assert_eq!(trace[1].func_name(), Some("b")); @@ -225,7 +225,7 @@ fn test_trap_backtrace_disabled() -> Result<()> { let run_func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; let e = run_func.call(&mut store, ()).unwrap_err(); - assert!(e.downcast_ref::().is_none()); + assert!(e.downcast_ref::().is_none()); Ok(()) } @@ -249,7 +249,7 @@ fn test_trap_trace_cb() -> Result<()> { let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.downcast_ref::().unwrap().frames(); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].module_name().unwrap(), "hello_mod"); assert_eq!(trace[0].func_index(), 2); @@ -275,7 +275,7 @@ fn test_trap_stack_overflow() -> Result<()> { let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.downcast_ref::().unwrap().frames(); + let trace = e.downcast_ref::().unwrap().frames(); assert!(trace.len() >= 32); for i in 0..trace.len() { assert_eq!(trace[i].module_name().unwrap(), "rec_mod"); @@ -470,7 +470,7 @@ fn rust_catch_panic_import() -> Result<()> { let instance = Instance::new(&mut store, &module, &[panic.into(), catch_panic.into()])?; let run = instance.get_typed_func::<(), (), _>(&mut store, "run")?; let trap = run.call(&mut store, ()).unwrap_err(); - let trace = trap.downcast_ref::().unwrap().frames(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_index(), 3); assert_eq!(num_panics.load(std::sync::atomic::Ordering::SeqCst), 2); @@ -625,7 +625,7 @@ fn present_after_module_drop() -> Result<()> { fn assert_trap(t: Error) { println!("{:?}", t); - let trace = t.downcast_ref::().unwrap().frames(); + let trace = t.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_index(), 0); } @@ -718,7 +718,7 @@ fn parse_dwarf_info() -> Result<()> { let trap = run.call(&mut store, &[], &mut []).unwrap_err(); let mut found = false; - let frames = trap.downcast_ref::().unwrap().frames(); + let frames = trap.downcast_ref::().unwrap().frames(); for frame in frames { for symbol in frame.symbols() { if let Some(file) = symbol.file() { @@ -847,7 +847,7 @@ fn traps_without_address_map() -> Result<()> { let e = run_func.call(&mut store, ()).unwrap_err(); - let trace = e.downcast_ref::().unwrap().frames(); + let trace = e.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 2); assert_eq!(trace[0].func_name(), Some("hello")); assert_eq!(trace[0].func_index(), 1); @@ -900,7 +900,7 @@ fn catch_trap_calling_across_stores() -> Result<()> { "trap should contain 'unreachable', got: {trap:?}" ); - let trace = trap.downcast_ref::().unwrap().frames(); + let trace = trap.downcast_ref::().unwrap().frames(); assert_eq!(trace.len(), 1); assert_eq!(trace[0].func_name(), Some("trap")); @@ -1010,7 +1010,7 @@ async fn async_then_sync_trap() -> Result<()> { .unwrap(); let trap = a.call_async(&mut async_store, ()).await.unwrap_err(); - let trace = trap.downcast_ref::().unwrap().frames(); + let trace = trap.downcast_ref::().unwrap().frames(); // We don't support cross-store or cross-engine symbolication currently, so // the other frames are ignored. assert_eq!(trace.len(), 1); @@ -1091,7 +1091,7 @@ async fn sync_then_async_trap() -> Result<()> { .unwrap(); let trap = a.call(&mut sync_store, ()).unwrap_err(); - let trace = trap.downcast_ref::().unwrap().frames(); + let trace = trap.downcast_ref::().unwrap().frames(); // We don't support cross-store or cross-engine symbolication currently, so // the other frames are ignored. assert_eq!(trace.len(), 1); From a651f236866dfe6ce43c22e7d61e12f7d0be00bb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 1 Nov 2022 12:15:18 -0700 Subject: [PATCH 10/12] Soup up documentation for errors and traps --- crates/wasmtime/src/config.rs | 40 ++++++--- crates/wasmtime/src/func.rs | 79 +++++++++++++++++- crates/wasmtime/src/func/typed.rs | 8 ++ crates/wasmtime/src/instance.rs | 7 +- crates/wasmtime/src/trap.rs | 130 ++++++++++++++++++++++++++---- 5 files changed, 231 insertions(+), 33 deletions(-) diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index e5f3fde34145..b29fe2322683 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -381,15 +381,30 @@ impl Config { self } - /// Configures whether backtraces exist in a `Trap`. - /// - /// Enabled by default, this feature builds in support to - /// generate backtraces at runtime for WebAssembly modules. This means that - /// unwinding information is compiled into wasm modules and necessary runtime - /// dependencies are enabled as well. - /// - /// When disabled, wasm backtrace details are ignored, and [`crate::Trap::trace()`] - /// will always return `None`. + /// Configures whether [`WasmBacktrace`] will be present in the context of + /// errors returned from Wasmtime. + /// + /// A backtrace may be collected whenever an error is returned from a host + /// function call through to WebAssembly or when WebAssembly itself hits a + /// trap condition, such as an out-of-bounds memory access. This flag + /// indicates, in these conditions, whether the backtrace is collected or + /// not. + /// + /// Currently wasm backtraces are implemented through frame pointer walking. + /// This means that collecting a backtrace is expected to be a fast and + /// relatively cheap operation. Additionally backtrace collection is + /// suitable in concurrent environments since one thread capturing a + /// backtrace won't block other threads. + /// + /// Collected backtraces are attached via [`anyhow::Error::context`] to + /// errors returned from host functions. The [`WasmBacktrace`] type can be + /// acquired via [`anyhow::Error::downcast_ref`] to inspect the backtrace. + /// When this option is disabled then this context is never applied to + /// errors coming out of wasm. + /// + /// This option is `true` by default. + /// + /// [`WasmBacktrace`]: crate::WasmBacktrace #[deprecated = "Backtraces will always be enabled in future Wasmtime releases; if this \ causes problems for you, please file an issue."] pub fn wasm_backtrace(&mut self, enable: bool) -> &mut Self { @@ -429,13 +444,16 @@ impl Config { /// This configuration option only exists to help third-party stack /// capturing mechanisms, such as the system's unwinder or the `backtrace` /// crate, determine how to unwind through Wasm frames. It does not affect - /// whether Wasmtime can capture Wasm backtraces or not, or whether - /// [`Trap::trace`][crate::Trap::trace] returns `Some` or `None`. + /// whether Wasmtime can capture Wasm backtraces or not. The presence of + /// [`WasmBacktrace`] is controlled by the [`Config::wasm_backtrace`] + /// option. /// /// Note that native unwind information is always generated when targeting /// Windows, since the Windows ABI requires it. /// /// This option defaults to `true`. + /// + /// [`WasmBacktrace`]: crate::WasmBacktrace pub fn native_unwind_info(&mut self, enable: bool) -> &mut Self { self.native_unwind_info = enable; self diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 3d285a854183..587fd8e566bd 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -327,6 +327,22 @@ impl Func { /// /// For more information about `Send + Sync + 'static` requirements on the /// `func`, see [`Func::wrap`](#why-send--sync--static). + /// + /// # Errors + /// + /// The host-provided function here returns a + /// [`Result<()>`](anyhow::Result). If the function returns `Ok(())` then + /// that indicates that the host function completed successfully and wrote + /// the result into the `&mut [Val]` argument. + /// + /// If the function returns `Err(e)`, however, then this is equivalent to + /// the host function triggering a trap for wasm. WebAssembly execution is + /// immediately halted and the original caller of [`Func::call`], for + /// example, will receive the error returned here (possibly with + /// [`WasmBacktrace`](crate::WasmBacktrace) context information attached). + /// + /// For more information about errors in Wasmtime see the [`Trap`] + /// documentation. #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn new( @@ -355,6 +371,11 @@ impl Func { /// [`Func::new`] or [`Func::wrap`]. The [`Func::wrap`] API, in particular, /// is both safer and faster than this API. /// + /// # Errors + /// + /// See [`Func::new`] for the behavior of returning an error from the host + /// function provided here. + /// /// # Unsafety /// /// This function is not safe because it's not known at compile time that @@ -392,6 +413,11 @@ impl Func { /// This function will panic if `store` is not associated with an [async /// config](crate::Config::async_support). /// + /// # Errors + /// + /// See [`Func::new`] for the behavior of returning an error from the host + /// function provided here. + /// /// # Examples /// /// ``` @@ -543,6 +569,18 @@ impl Func { /// actually closing over any values. These zero-sized types will use the /// context from [`Caller`] for host-defined information. /// + /// # Errors + /// + /// The closure provided here to `wrap` can optionally return a + /// [`Result`](anyhow::Result). Returning `Ok(t)` represents the host + /// function successfully completing with the `t` result. Returning + /// `Err(e)`, however, is equivalent to raising a custom wasm trap. + /// Execution of WebAssembly does not resume and the stack is unwound to the + /// original caller of the function where the error is returned. + /// + /// For more information about errors in Wasmtime see the [`Trap`] + /// documentation. + /// /// # Examples /// /// First up we can see how simple wasm imports can be implemented, such @@ -750,16 +788,41 @@ impl Func { /// Invokes this function with the `params` given and writes returned values /// to `results`. /// - /// The `params` here must match the type signature of this `Func`, or a - /// trap will occur. If a trap occurs while executing this function, then a - /// trap will also be returned. Additionally `results` must have the same - /// length as the number of results for this function. + /// The `params` here must match the type signature of this `Func`, or an + /// error will occur. Additionally `results` must have the same + /// length as the number of results for this function. Calling this function + /// will synchronously execute the WebAssembly function referenced to get + /// the results. + /// + /// This function will return `Ok(())` if execution completed without a trap + /// or error of any kind. In this situation the results will be written to + /// the provided `results` array. + /// + /// # Errors + /// + /// Any error which occurs throughout the execution of the function will be + /// returned as `Err(e)`. The [`Error`](anyhow::Error) type can be inspected + /// for the precise error cause such as: + /// + /// * [`Trap`] - indicates that a wasm trap happened and execution was + /// halted. + /// * [`WasmBacktrace`] - optionally included on errors for backtrace + /// information of the trap/error. + /// * Other string-based errors to indicate issues such as type errors with + /// `params`. + /// * Any host-originating error originally returned from a function defined + /// via [`Func::new`], for example. + /// + /// Errors typically indicate that execution of WebAssembly was halted + /// mid-way and did not complete after the error condition happened. /// /// # Panics /// /// This function will panic if called on a function belonging to an async /// store. Asynchronous stores must always use `call_async`. /// initiates a panic. Also panics if `store` does not own this function. + /// + /// [`WasmBacktrace`]: crate::WasmBacktrace pub fn call( &self, mut store: impl AsContextMut, @@ -788,6 +851,10 @@ impl Func { /// invoked many times with new `ExternRef` values and no other GC happens /// via any other means then no values will get collected. /// + /// # Errors + /// + /// For more information about errors see the [`Func::call`] documentation. + /// /// # Unsafety /// /// This function is unsafe because the `params_and_returns` argument is not @@ -878,6 +945,10 @@ impl Func { /// For more information see the documentation on [asynchronous /// configs](crate::Config::async_support). /// + /// # Errors + /// + /// For more information on errors see the [`Func::call`] documentation. + /// /// # Panics /// /// Panics if this is called on a function in a synchronous store. This diff --git a/crates/wasmtime/src/func/typed.rs b/crates/wasmtime/src/func/typed.rs index ecc5c15c5b32..321aa3663056 100644 --- a/crates/wasmtime/src/func/typed.rs +++ b/crates/wasmtime/src/func/typed.rs @@ -68,6 +68,10 @@ where /// For more information, see the [`Func::typed`] and [`Func::call`] /// documentation. /// + /// # Errors + /// + /// For more information on errors see the documentation on [`Func::call`]. + /// /// # Panics /// /// This function will panic if it is called when the underlying [`Func`] is @@ -91,6 +95,10 @@ where /// For more information, see the [`Func::typed`] and [`Func::call_async`] /// documentation. /// + /// # Errors + /// + /// For more information on errors see the documentation on [`Func::call`]. + /// /// # Panics /// /// This function will panic if it is called when the underlying [`Func`] is diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index de6b33f50cf2..cb7897ad3abe 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -88,7 +88,8 @@ impl Instance { /// /// When instantiation fails it's recommended to inspect the return value to /// see why it failed, or bubble it upwards. If you'd like to specifically - /// check for trap errors, you can use `error.downcast::()`. + /// check for trap errors, you can use `error.downcast::()`. For more + /// about error handling see the [`Trap`] documentation. /// /// # Panics /// @@ -102,7 +103,7 @@ impl Instance { mut store: impl AsContextMut, module: &Module, imports: &[Extern], - ) -> Result { + ) -> Result { let mut store = store.as_context_mut(); let imports = Instance::typecheck_externs(store.0, module, imports)?; // Note that the unsafety here should be satisfied by the call to @@ -134,7 +135,7 @@ impl Instance { mut store: impl AsContextMut, module: &Module, imports: &[Extern], - ) -> Result + ) -> Result where T: Send, { diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 7840726d6a36..7f6faa13bfa7 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -5,13 +5,66 @@ use std::fmt; use wasmtime_environ::{EntityRef, FilePos, TrapCode}; use wasmtime_jit::{demangle_function_name, demangle_function_name_or_index}; -/// Description of a trap which can happen in WebAssembly, halting wasm -/// execution. +/// Representation of a WebAssembly trap and what caused it to occur. /// -/// This enumeration is a list of all possible traps that can happen in wasm, in -/// addition to some Wasmtime-specific trap codes listed here as well. This type -/// will be returned as an error when a wasm function hits one of these trap -/// conditions through the [`anyhow::Error`] wrapper. +/// WebAssembly traps happen explicitly for instructions such as `unreachable` +/// but can also happen as side effects of other instructions such as `i32.load` +/// loading an out-of-bounds address. Traps halt the execution of WebAssembly +/// and cause an error to be returned to the host. This enumeration is a list of +/// all possible traps that can happen in wasm, in addition to some +/// Wasmtime-specific trap codes listed here as well. +/// +/// # Errors in Wasmtime +/// +/// Error-handling in Wasmtime is primarily done through the [`anyhow`] crate +/// where most results are a [`Result`](anyhow::Result) which is an alias for +/// [`Result`](std::result::Result). Errors in Wasmtime are +/// represented with [`anyhow::Error`] which acts as a container for any type of +/// error in addition to optional context for this error. The "base" error or +/// [`anyhow::Error::root_cause`] is a [`Trap`] whenever WebAssembly hits a +/// trap, or otherwise it's whatever the host created the error with when +/// returning an error for a host call. +/// +/// Any error which happens while WebAssembly is executing will also, by +/// default, capture a backtrace of the wasm frames while executing. This +/// backtrace is represented with a [`WasmBacktrace`] instance and is attached +/// to the [`anyhow::Error`] return value as a +/// [`context`](anyhow::Error::context). Inspecting a [`WasmBacktrace`] can be +/// done with the [`downcast_ref`](anyhow::Error::downcast_ref) function. For +/// information on this see the [`WasmBacktrace`] documentation. +/// +/// # Examples +/// +/// ``` +/// # use wasmtime::*; +/// # use anyhow::Result; +/// # fn main() -> Result<()> { +/// let engine = Engine::default(); +/// let module = Module::new( +/// &engine, +/// r#" +/// (module +/// (func (export "trap") +/// unreachable) +/// (func $overflow (export "overflow") +/// call $overflow) +/// ) +/// "#, +/// )?; +/// let mut store = Store::new(&engine, ()); +/// let instance = Instance::new(&mut store, &module, &[])?; +/// +/// let trap = instance.get_typed_func::<(), (), _>(&mut store, "trap")?; +/// let error = trap.call(&mut store, ()).unwrap_err(); +/// assert_eq!(*error.downcast_ref::().unwrap(), Trap::UnreachableCodeReached); +/// assert!(error.root_cause().is::()); +/// +/// let overflow = instance.get_typed_func::<(), (), _>(&mut store, "overflow")?; +/// let error = overflow.call(&mut store, ()).unwrap_err(); +/// assert_eq!(*error.downcast_ref::().unwrap(), Trap::StackOverflow); +/// # Ok(()) +/// # } +/// ``` // // The code can be accessed from the c-api, where the possible values are translated // into enum values defined there: @@ -174,13 +227,63 @@ impl fmt::Display for Trap { impl std::error::Error for Trap {} -/// todo +/// Representation of a backtrace of function frames in a WebAssembly module for +/// where an error happened. +/// +/// This structure is attached to the [`anyhow::Error`] returned from many +/// Wasmtime functions that execute WebAssembly such as [`Instance::new`] or +/// [`Func::call`]. This can be acquired with the [`anyhow::Error::downcast`] +/// family of methods to programmatically inspect the backtrace. Otherwise since +/// it's part of the error returned this will get printed along with the rest of +/// the error when the error is logged. +/// +/// Capturing of wasm backtraces can be configured through the +/// [`Config::wasm_backtrace`](crate::Config::wasm_backtrace) method. +/// +/// For more information about errors in wasmtime see the documentation of the +/// [`Trap`] type. +/// +/// [`Func::call`]: crate::Func::call +/// [`Instance::new`]: crate::Instance::new +/// +/// # Examples +/// +/// ``` +/// # use wasmtime::*; +/// # use anyhow::Result; +/// # fn main() -> Result<()> { +/// let engine = Engine::default(); +/// let module = Module::new( +/// &engine, +/// r#" +/// (module +/// (func $start (export "run") +/// call $trap) +/// (func $trap +/// unreachable) +/// ) +/// "#, +/// )?; +/// let mut store = Store::new(&engine, ()); +/// let instance = Instance::new(&mut store, &module, &[])?; +/// let func = instance.get_typed_func::<(), (), _>(&mut store, "run")?; +/// let error = func.call(&mut store, ()).unwrap_err(); +/// let bt = error.downcast_ref::().unwrap(); +/// let frames = bt.frames(); +/// assert_eq!(frames.len(), 2); +/// assert_eq!(frames[0].func_name(), Some("trap")); +/// assert_eq!(frames[1].func_name(), Some("start")); +/// # Ok(()) +/// # } +/// ``` #[derive(Debug)] pub struct WasmBacktrace { wasm_trace: Vec, + hint_wasm_backtrace_details_env: bool, + // This is currently only present for the `Debug` implementation for extra + // context. #[allow(dead_code)] runtime_trace: wasmtime_runtime::Backtrace, - hint_wasm_backtrace_details_env: bool, } impl WasmBacktrace { @@ -324,14 +427,11 @@ impl fmt::Display for WasmBacktrace { } } -/// Description of a frame in a backtrace for a [`Trap`] or [`BacktraceContext`]. -/// -/// Whenever a WebAssembly trap occurs an instance of [`Trap`] is created. Each -/// [`Trap`] has a backtrace of the WebAssembly frames that led to the trap, and -/// each frame is described by this structure. +/// Description of a frame in a backtrace for a [`WasmBacktrace`]. /// -/// [`Trap`]: crate::Trap -/// [`BacktraceContext`]: crate::BacktraceContext +/// Whenever an error happens while WebAssembly is executing a +/// [`WasmBacktrace`] will be attached to the error returned which can be used +/// to acquire this `FrameInfo`. For more information see [`WasmBacktrace`]. #[derive(Debug)] pub struct FrameInfo { module_name: Option, From 053943af64036d03b3156f64b3763e0eea42d9a4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 2 Nov 2022 07:29:21 -0700 Subject: [PATCH 11/12] Fix build of the C API --- crates/c-api/src/trap.rs | 50 ++++++++++++++++++++++------------------ crates/c-api/src/vec.rs | 27 ++++++++++++---------- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 21e68ec81552..23476fbf01a4 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -1,7 +1,7 @@ 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::{BacktraceContext, Trap}; +use wasmtime::{Trap, WasmBacktrace}; #[repr(C)] pub struct wasm_trap_t { @@ -30,8 +30,8 @@ impl wasm_trap_t { #[repr(C)] #[derive(Clone)] -pub struct wasm_frame_t { - trap: Trap, +pub struct wasm_frame_t<'a> { + trace: &'a WasmBacktrace, idx: usize, func_name: OnceCell>, module_name: OnceCell>, @@ -75,14 +75,14 @@ pub extern "C" fn wasm_trap_message(trap: &wasm_trap_t, out: &mut wasm_message_t } #[no_mangle] -pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option> { - let trap = match raw.error.downcast_ref::() { +pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option>> { + let trace = match raw.error.downcast_ref::() { Some(trap) => trap, None => return None, }; - if trap.frames().len() > 0 { + if trace.frames().len() > 0 { Some(Box::new(wasm_frame_t { - trap: trap.clone(), + trace, idx: 0, func_name: OnceCell::new(), module_name: OnceCell::new(), @@ -93,15 +93,15 @@ pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option() { +pub extern "C" fn wasm_trap_trace<'a>(raw: &'a wasm_trap_t, out: &mut wasm_frame_vec_t<'a>) { + let trace = match raw.error.downcast_ref::() { Some(trap) => trap, None => return out.set_buffer(Vec::new()), }; - let vec = (0..trap.trace().unwrap_or(&[]).len()) + let vec = (0..trace.frames().len()) .map(|idx| { Some(Box::new(wasm_frame_t { - trap: trap.clone(), + trace, idx, func_name: OnceCell::new(), module_name: OnceCell::new(), @@ -146,16 +146,18 @@ pub extern "C" fn wasmtime_trap_exit_status(raw: &wasm_trap_t, status: &mut i32) } #[no_mangle] -pub extern "C" fn wasm_frame_func_index(frame: &wasm_frame_t) -> u32 { - frame.trap.trace().expect("backtraces are always enabled")[frame.idx].func_index() +pub extern "C" fn wasm_frame_func_index(frame: &wasm_frame_t<'_>) -> u32 { + frame.trace.frames()[frame.idx].func_index() } #[no_mangle] -pub extern "C" fn wasmtime_frame_func_name(frame: &wasm_frame_t) -> Option<&wasm_name_t> { +pub extern "C" fn wasmtime_frame_func_name<'a>( + frame: &'a wasm_frame_t<'_>, +) -> Option<&'a wasm_name_t> { frame .func_name .get_or_init(|| { - frame.trap.trace().expect("backtraces are always enabled")[frame.idx] + frame.trace.frames()[frame.idx] .func_name() .map(|s| wasm_name_t::from(s.to_string().into_bytes())) }) @@ -163,11 +165,13 @@ pub extern "C" fn wasmtime_frame_func_name(frame: &wasm_frame_t) -> Option<&wasm } #[no_mangle] -pub extern "C" fn wasmtime_frame_module_name(frame: &wasm_frame_t) -> Option<&wasm_name_t> { +pub extern "C" fn wasmtime_frame_module_name<'a>( + frame: &'a wasm_frame_t<'_>, +) -> Option<&'a wasm_name_t> { frame .module_name .get_or_init(|| { - frame.trap.trace().expect("backtraces are always enabled")[frame.idx] + frame.trace.frames()[frame.idx] .module_name() .map(|s| wasm_name_t::from(s.to_string().into_bytes())) }) @@ -175,25 +179,25 @@ pub extern "C" fn wasmtime_frame_module_name(frame: &wasm_frame_t) -> Option<&wa } #[no_mangle] -pub extern "C" fn wasm_frame_func_offset(frame: &wasm_frame_t) -> usize { - frame.trap.trace().expect("backtraces are always enabled")[frame.idx] +pub extern "C" fn wasm_frame_func_offset(frame: &wasm_frame_t<'_>) -> usize { + frame.trace.frames()[frame.idx] .func_offset() .unwrap_or(usize::MAX) } #[no_mangle] -pub extern "C" fn wasm_frame_instance(_arg1: *const wasm_frame_t) -> *mut wasm_instance_t { +pub extern "C" fn wasm_frame_instance(_arg1: *const wasm_frame_t<'_>) -> *mut wasm_instance_t { unimplemented!("wasm_frame_instance") } #[no_mangle] -pub extern "C" fn wasm_frame_module_offset(frame: &wasm_frame_t) -> usize { - frame.trap.trace().expect("backtraces are always enabled")[frame.idx] +pub extern "C" fn wasm_frame_module_offset(frame: &wasm_frame_t<'_>) -> usize { + frame.trace.frames()[frame.idx] .module_offset() .unwrap_or(usize::MAX) } #[no_mangle] -pub extern "C" fn wasm_frame_copy(frame: &wasm_frame_t) -> Box { +pub extern "C" fn wasm_frame_copy<'a>(frame: &wasm_frame_t<'a>) -> Box> { Box::new(frame.clone()) } diff --git a/crates/c-api/src/vec.rs b/crates/c-api/src/vec.rs index 77835b415000..7a5aae734ffa 100644 --- a/crates/c-api/src/vec.rs +++ b/crates/c-api/src/vec.rs @@ -19,7 +19,7 @@ impl wasm_name_t { macro_rules! declare_vecs { ( $(( - name: $name:ident, + name: $name:ident $(<$lt:tt>)?, ty: $elem_ty:ty, new: $new:ident, empty: $empty:ident, @@ -29,12 +29,12 @@ macro_rules! declare_vecs { ))* ) => {$( #[repr(C)] - pub struct $name { + pub struct $name $(<$lt>)? { size: usize, data: *mut $elem_ty, } - impl $name { + impl$(<$lt>)? $name $(<$lt>)? { pub fn set_buffer(&mut self, buffer: Vec<$elem_ty>) { let mut vec = buffer.into_boxed_slice(); self.size = vec.len(); @@ -79,13 +79,13 @@ macro_rules! declare_vecs { } } - impl Clone for $name { + impl$(<$lt>)? Clone for $name $(<$lt>)? { fn clone(&self) -> Self { self.as_slice().to_vec().into() } } - impl From> for $name { + impl$(<$lt>)? From> for $name $(<$lt>)? { fn from(vec: Vec<$elem_ty>) -> Self { let mut vec = vec.into_boxed_slice(); let result = $name { @@ -97,7 +97,7 @@ macro_rules! declare_vecs { } } - impl Drop for $name { + impl$(<$lt>)? Drop for $name $(<$lt>)? { fn drop(&mut self) { drop(self.take()); } @@ -115,8 +115,8 @@ macro_rules! declare_vecs { } #[no_mangle] - pub unsafe extern "C" fn $new( - out: &mut $name, + pub unsafe extern "C" fn $new $(<$lt>)? ( + out: &mut $name $(<$lt>)?, size: usize, ptr: *const $elem_ty, ) { @@ -125,12 +125,15 @@ macro_rules! declare_vecs { } #[no_mangle] - pub extern "C" fn $copy(out: &mut $name, src: &$name) { + pub extern "C" fn $copy $(<$lt>)? ( + out: &mut $name $(<$lt>)?, + src: &$name $(<$lt>)?, + ) { out.set_buffer(src.as_slice().to_vec()); } #[no_mangle] - pub extern "C" fn $delete(out: &mut $name) { + pub extern "C" fn $delete $(<$lt>)? (out: &mut $name $(<$lt>)?) { out.take(); } )*}; @@ -228,8 +231,8 @@ declare_vecs! { delete: wasm_val_vec_delete, ) ( - name: wasm_frame_vec_t, - ty: Option>, + name: wasm_frame_vec_t<'a>, + ty: Option>>, new: wasm_frame_vec_new, empty: wasm_frame_vec_new_empty, uninit: wasm_frame_vec_new_uninitialized, From a15448e07c0a1052bfdb6ee7031d00da01b00ddf Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 2 Nov 2022 07:49:19 -0700 Subject: [PATCH 12/12] Fix a build warning --- crates/c-api/src/trap.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 23476fbf01a4..f6881c5454de 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -142,6 +142,9 @@ pub extern "C" fn wasmtime_trap_exit_status(raw: &wasm_trap_t, status: &mut i32) return true; } + // Squash unused warnings in wasi-disabled builds. + drop((raw, status)); + false }