From e6629e216491687d653e435ba6caa8dbea0bc624 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 1 May 2020 04:41:05 -0700 Subject: [PATCH 01/11] Remove Cranelift's OutOfBounds trap, which is no longer used. --- cranelift/codegen/src/ir/trapcode.rs | 8 +------- cranelift/filetests/filetests/parser/flags.clif | 4 ++-- cranelift/filetests/filetests/parser/tiny.clif | 4 ++-- cranelift/filetests/filetests/regalloc/iterate.clif | 2 +- cranelift/filetests/filetests/wasm/control.clif | 2 +- crates/wasmtime/src/trap.rs | 1 - 6 files changed, 7 insertions(+), 14 deletions(-) diff --git a/cranelift/codegen/src/ir/trapcode.rs b/cranelift/codegen/src/ir/trapcode.rs index 3f8ffe3fb508..0f1f62e3b6d0 100644 --- a/cranelift/codegen/src/ir/trapcode.rs +++ b/cranelift/codegen/src/ir/trapcode.rs @@ -27,9 +27,6 @@ pub enum TrapCode { /// A `table_addr` instruction detected an out-of-bounds error. TableOutOfBounds, - /// Other bounds checking error. - OutOfBounds, - /// Indirect call to a null table entry. IndirectCallToNull, @@ -63,7 +60,6 @@ impl Display for TrapCode { StackOverflow => "stk_ovf", HeapOutOfBounds => "heap_oob", TableOutOfBounds => "table_oob", - OutOfBounds => "oob", IndirectCallToNull => "icall_null", BadSignature => "bad_sig", IntegerOverflow => "int_ovf", @@ -86,7 +82,6 @@ impl FromStr for TrapCode { "stk_ovf" => Ok(StackOverflow), "heap_oob" => Ok(HeapOutOfBounds), "table_oob" => Ok(TableOutOfBounds), - "oob" => Ok(OutOfBounds), "icall_null" => Ok(IndirectCallToNull), "bad_sig" => Ok(BadSignature), "int_ovf" => Ok(IntegerOverflow), @@ -106,11 +101,10 @@ mod tests { use alloc::string::ToString; // Everything but user-defined codes. - const CODES: [TrapCode; 11] = [ + const CODES: [TrapCode; 10] = [ TrapCode::StackOverflow, TrapCode::HeapOutOfBounds, TrapCode::TableOutOfBounds, - TrapCode::OutOfBounds, TrapCode::IndirectCallToNull, TrapCode::BadSignature, TrapCode::IntegerOverflow, diff --git a/cranelift/filetests/filetests/parser/flags.clif b/cranelift/filetests/filetests/parser/flags.clif index c8d6e78912a2..f8d9cd2b7b59 100644 --- a/cranelift/filetests/filetests/parser/flags.clif +++ b/cranelift/filetests/filetests/parser/flags.clif @@ -25,7 +25,7 @@ block201: return block202: - trap oob + trap heap_oob } ; check: v1 = ifcmp_imm v0, 17 ; check: brif eq v1, block201 @@ -56,7 +56,7 @@ block201: return block202: - trap oob + trap heap_oob } ; check: v2 = ffcmp v0, v1 ; check: brff eq v2, block201 diff --git a/cranelift/filetests/filetests/parser/tiny.clif b/cranelift/filetests/filetests/parser/tiny.clif index 98f477f80866..3f825e6eaca4 100644 --- a/cranelift/filetests/filetests/parser/tiny.clif +++ b/cranelift/filetests/filetests/parser/tiny.clif @@ -223,7 +223,7 @@ function %cond_traps(i32) { block0(v0: i32): trapz v0, stk_ovf v1 = ifcmp_imm v0, 5 - trapif ugt v1, oob + trapif ugt v1, heap_oob v2 = bitcast.f32 v1 v3 = ffcmp v2, v2 trapff uno v3, int_ovf @@ -233,7 +233,7 @@ block0(v0: i32): ; nextln: block0(v0: i32): ; nextln: trapz v0, stk_ovf ; nextln: v1 = ifcmp_imm v0, 5 -; nextln: trapif ugt v1, oob +; nextln: trapif ugt v1, heap_oob ; nextln: v2 = bitcast.f32 v1 ; nextln: v3 = ffcmp v2, v2 ; nextln: trapff uno v3, int_ovf diff --git a/cranelift/filetests/filetests/regalloc/iterate.clif b/cranelift/filetests/filetests/regalloc/iterate.clif index 2c7d69176518..f3ed963d7031 100644 --- a/cranelift/filetests/filetests/regalloc/iterate.clif +++ b/cranelift/filetests/filetests/regalloc/iterate.clif @@ -139,7 +139,7 @@ block0(v0: i64): jump block4 block4: - trap oob + trap heap_oob block2: v8 = load.i64 v5+8 diff --git a/cranelift/filetests/filetests/wasm/control.clif b/cranelift/filetests/filetests/wasm/control.clif index d00c5b3166cc..4f40ddffb93b 100644 --- a/cranelift/filetests/filetests/wasm/control.clif +++ b/cranelift/filetests/filetests/wasm/control.clif @@ -54,7 +54,7 @@ block0(v0: i32): br_table v0, block4, jt0 block4: - trap oob + trap heap_oob block1: return diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index b1bdd6afde6c..c5ea37f96120 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -85,7 +85,6 @@ impl Trap { StackOverflow => "call stack exhausted", HeapOutOfBounds => "out of bounds memory access", TableOutOfBounds => "undefined element: out of bounds table access", - OutOfBounds => "out of bounds", IndirectCallToNull => "uninitialized element", BadSignature => "indirect call type mismatch", IntegerOverflow => "integer overflow", From 4ab43d19773fef1c6d4f579e08336cc591a1d98e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 1 May 2020 05:06:00 -0700 Subject: [PATCH 02/11] Change proc_exit to unwind instead of exit the host process. This implements the semantics in https://github.com/WebAssembly/WASI/pull/235. Fixes #783. Fixes #993. --- Cargo.lock | 1 + crates/api/src/error.rs | 37 ++++++++ crates/api/src/exit.rs | 40 +++++++++ crates/runtime/Cargo.toml | 1 + crates/runtime/src/lib.rs | 3 +- crates/runtime/src/traphandlers.rs | 61 +++++++++++++- .../src/old/snapshot_0/hostcalls_impl/misc.rs | 7 -- .../src/snapshots/wasi_snapshot_preview1.rs | 9 -- crates/wasi-common/wig/src/hostcalls.rs | 6 ++ crates/wasi-common/wig/src/wasi.rs | 16 ++++ crates/wasmtime/src/externals.rs | 4 +- crates/wasmtime/src/func.rs | 72 ++++++++++++---- crates/wasmtime/src/instance.rs | 4 +- crates/wasmtime/src/lib.rs | 3 + crates/wasmtime/src/runtime.rs | 2 +- crates/wasmtime/src/trampoline/func.rs | 4 +- crates/wasmtime/src/trap.rs | 44 +--------- crates/wiggle/generate/src/funcs.rs | 6 ++ crates/wiggle/generate/src/module_trait.rs | 84 ++++++++++--------- crates/wiggle/tests/wasi.rs | 4 - examples/interrupt.rs | 8 +- src/commands/run.rs | 13 ++- tests/all/cli_tests.rs | 40 +++++++++ tests/all/iloop.rs | 14 ++-- tests/all/stack_overflow.rs | 4 +- tests/wasm/exit125_wasi_snapshot0.wat | 8 ++ tests/wasm/exit125_wasi_snapshot1.wat | 8 ++ tests/wasm/exit126_wasi_snapshot0.wat | 8 ++ tests/wasm/exit126_wasi_snapshot1.wat | 8 ++ 29 files changed, 378 insertions(+), 141 deletions(-) create mode 100644 crates/api/src/error.rs create mode 100644 crates/api/src/exit.rs create mode 100644 tests/wasm/exit125_wasi_snapshot0.wat create mode 100644 tests/wasm/exit125_wasi_snapshot1.wat create mode 100644 tests/wasm/exit126_wasi_snapshot0.wat create mode 100644 tests/wasm/exit126_wasi_snapshot1.wat diff --git a/Cargo.lock b/Cargo.lock index c1432510963b..998ffaa53acf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2273,6 +2273,7 @@ dependencies = [ name = "wasmtime-runtime" version = "0.16.0" dependencies = [ + "anyhow", "backtrace", "cc", "cfg-if", diff --git a/crates/api/src/error.rs b/crates/api/src/error.rs new file mode 100644 index 000000000000..dfac504fad48 --- /dev/null +++ b/crates/api/src/error.rs @@ -0,0 +1,37 @@ +use crate::frame_info::FRAME_INFO; +use crate::{Exit, Trap}; +use anyhow::Error; +use wasmtime_environ::ir::TrapCode; + +/// Convert from an internal unwinding code into an `Error`. +pub(crate) fn from_runtime(jit: wasmtime_runtime::Trap) -> Error { + let info = FRAME_INFO.read().unwrap(); + match jit { + wasmtime_runtime::Trap::User(error) => error, + wasmtime_runtime::Trap::Jit { + pc, + backtrace, + maybe_interrupted, + } => { + let mut code = info + .lookup_trap_info(pc) + .map(|info| info.trap_code) + .unwrap_or(TrapCode::StackOverflow); + if maybe_interrupted && code == TrapCode::StackOverflow { + code = TrapCode::Interrupt; + } + Error::new(Trap::new_wasm(&info, Some(pc), code, backtrace)) + } + wasmtime_runtime::Trap::Wasm { + trap_code, + backtrace, + } => Error::new(Trap::new_wasm(&info, None, trap_code, backtrace)), + wasmtime_runtime::Trap::OOM { backtrace } => Error::new(Trap::new_with_trace( + &info, + None, + "out of memory".to_string(), + backtrace, + )), + wasmtime_runtime::Trap::Exit { status } => Error::new(Exit::new(status)), + } +} diff --git a/crates/api/src/exit.rs b/crates/api/src/exit.rs new file mode 100644 index 000000000000..866b83f0449b --- /dev/null +++ b/crates/api/src/exit.rs @@ -0,0 +1,40 @@ +use std::fmt; +use std::num::NonZeroI32; + +/// A struct representing an explicit program exit, with a code +/// indicating the status. +#[derive(Clone, Debug)] +pub struct Exit { + status: NonZeroI32, +} + +impl Exit { + /// Creates a new `Exit` with `status`. The status value must be in the range [1..126]. + /// # Example + /// ``` + /// let exit = wasmtime::Exit::new(std::num::NonZeroI32::new(1).unwrap()); + /// assert_eq!(1, exit.status().get()); + /// ``` + pub fn new(status: NonZeroI32) -> Self { + assert!(status.get() > 0 && status.get() < 126); + Self { status } + } + + /// Returns a reference the `status` stored in `Exit`. + pub fn status(&self) -> NonZeroI32 { + self.status + } +} + +impl fmt::Display for Exit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Error codes used by WASI libc. + match self.status.get() { + 70 => write!(f, "internal software error"), + 71 => write!(f, "system error"), + _ => write!(f, "non-zero status {}", self.status), + } + } +} + +impl std::error::Error for Exit {} diff --git a/crates/runtime/Cargo.toml b/crates/runtime/Cargo.toml index a3ea52cc32a3..b7a84af5d14a 100644 --- a/crates/runtime/Cargo.toml +++ b/crates/runtime/Cargo.toml @@ -21,6 +21,7 @@ thiserror = "1.0.4" more-asserts = "0.2.1" cfg-if = "0.1.9" backtrace = "0.3.42" +anyhow = "1.0.19" [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3.7", features = ["winbase", "memoryapi", "errhandlingapi"] } diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 97983cde2d4f..89d07223140b 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -44,7 +44,8 @@ pub use crate::mmap::Mmap; pub use crate::sig_registry::SignatureRegistry; pub use crate::table::Table; pub use crate::traphandlers::{ - catch_traps, init_traps, raise_lib_trap, raise_user_trap, resume_panic, SignalHandler, Trap, + catch_traps, init_traps, raise_lib_trap, raise_user_trap, resume_panic, wasi_proc_exit, + InvalidWASIExitStatus, SignalHandler, Trap, }; pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index 6b2bae92ee64..2b0d3eb8351f 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -2,11 +2,13 @@ //! signalhandling mechanisms. use crate::VMContext; +use anyhow::Error; use backtrace::Backtrace; use std::any::Any; use std::cell::Cell; -use std::error::Error; +use std::fmt; use std::io; +use std::num::NonZeroI32; use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use std::sync::Once; @@ -271,7 +273,7 @@ fn real_init() { /// Only safe to call when wasm code is on the stack, aka `catch_traps` must /// have been previously called. Additionally no Rust destructors can be on the /// stack. They will be skipped and not executed. -pub unsafe fn raise_user_trap(data: Box) -> ! { +pub unsafe fn raise_user_trap(data: Error) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -305,7 +307,7 @@ pub unsafe fn resume_panic(payload: Box) -> ! { #[derive(Debug)] pub enum Trap { /// A user-raised trap through `raise_user_trap`. - User(Box), + User(Error), /// A trap raised from jit code Jit { @@ -332,6 +334,12 @@ pub enum Trap { /// Native stack backtrace at the time the OOM occurred backtrace: Backtrace, }, + + /// A program exit was requested with a non-zero exit status. + Exit { + /// The exit status + status: NonZeroI32, + }, } impl Trap { @@ -403,9 +411,10 @@ pub struct CallThreadState<'a> { enum UnwindReason { None, Panic(Box), - UserTrap(Box), + UserTrap(Error), LibTrap(Trap), JitTrap { backtrace: Backtrace, pc: usize }, + Exit { status: i32 }, } impl<'a> CallThreadState<'a> { @@ -453,6 +462,13 @@ impl<'a> CallThreadState<'a> { maybe_interrupted, }) } + UnwindReason::Exit { status } => { + if let Some(status) = NonZeroI32::new(status) { + Err(Trap::Exit { status }) + } else { + Ok(()) + } + } UnwindReason::Panic(panic) => { debug_assert_eq!(ret, 0); std::panic::resume_unwind(panic) @@ -779,3 +795,40 @@ fn setup_unix_sigaltstack() -> Result<(), Trap> { } } } + +/// Perform a program exit by unwinding the stack to the point where wasm +/// as most recently entered, carrying an exit status value. +/// +/// This is implemented in `wasmtime_runtime` rather than with the rest of the WASI +/// functions as it's essentially an unwinding operation. +/// +/// # Safety +/// +/// Only safe to call when wasm code is on the stack, aka `catch_traps` must +/// have been previously called. Additionally no Rust destructors can be on the +/// stack. They will be skipped and not executed. +pub unsafe extern "C" fn wasi_proc_exit( + _vmctx: *mut VMContext, + _caller_vmctx: *mut VMContext, + status: i32, +) -> ! { + // Check that the status is within WASI's range. + if status >= 0 && status < 126 { + tls::with(|info| info.unwrap().unwind_with(UnwindReason::Exit { status })) + } else { + raise_user_trap(InvalidWASIExitStatus {}.into()) + } +} + +/// An error type for indicating an exit was requested using an exit status outside +/// of WASI's valid range of [0..126]. +#[derive(Debug)] +pub struct InvalidWASIExitStatus {} + +impl fmt::Display for InvalidWASIExitStatus { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "invalid WASI exit status; values must be in [0..126]") + } +} + +impl std::error::Error for InvalidWASIExitStatus {} diff --git a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs index 922d8b304c11..d6f4d2468484 100644 --- a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs +++ b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs @@ -342,13 +342,6 @@ pub(crate) struct FdEventData<'a> { pub(crate) userdata: wasi::__wasi_userdata_t, } -pub(crate) fn proc_exit(_wasi_ctx: &WasiCtx, _memory: &mut [u8], rval: wasi::__wasi_exitcode_t) { - trace!("proc_exit(rval={:?})", rval); - // TODO: Rather than call std::process::exit here, we should trigger a - // stack unwind similar to a trap. - std::process::exit(rval as i32); -} - pub(crate) fn proc_raise( _wasi_ctx: &WasiCtx, _memory: &mut [u8], diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 4ab53f5d6ef8..b2f06e3b1b1c 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -813,15 +813,6 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { Ok(nevents) } - // This is just a temporary to ignore the warning which becomes a hard error - // in the CI. Once we figure out non-returns in `wiggle`, this should be gone. - #[allow(unreachable_code)] - fn proc_exit(&self, rval: types::Exitcode) -> std::result::Result<(), ()> { - // TODO: Rather than call std::process::exit here, we should trigger a - // stack unwind similar to a trap. - std::process::exit(rval as i32); - } - fn proc_raise(&self, _sig: types::Signal) -> Result<()> { unimplemented!("proc_raise") } diff --git a/crates/wasi-common/wig/src/hostcalls.rs b/crates/wasi-common/wig/src/hostcalls.rs index 0b73cae6d7f8..f3912e91004e 100644 --- a/crates/wasi-common/wig/src/hostcalls.rs +++ b/crates/wasi-common/wig/src/hostcalls.rs @@ -17,6 +17,12 @@ pub fn define(args: TokenStream) -> TokenStream { for module in doc.modules() { for func in module.funcs() { + // `proc_exit` is special; it's essentially an unwinding primitive, + // so we implement it in the runtime rather than in wasi-common. + if func.name.as_str() == "proc_exit" { + continue; + } + ret.extend(generate_wrappers(&func, old)); } } diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index b87484e53dda..57c95b52de2b 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -46,6 +46,14 @@ pub fn define_struct(args: TokenStream) -> TokenStream { linker_add.push(quote! { linker.define(#module_name, #name, self.#name_ident.clone())?; }); + // `proc_exit` is special; it's essentially an unwinding primitive, + // so we implement it in the runtime rather than in wasi-common. + if name == "proc_exit" { + ctor_externs.push(quote! { + let #name_ident = wasmtime::Func::exit_func(store); + }); + continue; + } let mut shim_arg_decls = Vec::new(); let mut params = Vec::new(); @@ -291,6 +299,14 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { linker_add.push(quote! { linker.define(#module_name, #name, self.#name_ident.clone())?; }); + // `proc_exit` is special; it's essentially an unwinding primitive, + // so we implement it in the runtime rather than in wasi-common. + if name == "proc_exit" { + ctor_externs.push(quote! { + let #name_ident = wasmtime::Func::exit_func(store); + }); + continue; + } let mut shim_arg_decls = Vec::new(); let mut params = Vec::new(); diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 6536f017fcef..b14ff291dd43 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -2,8 +2,8 @@ use crate::trampoline::{ generate_global_export, generate_memory_export, generate_table_export, StoreInstanceHandle, }; use crate::values::{from_checked_anyfunc, into_checked_anyfunc, Val}; +use crate::{error, Func, Store}; use crate::{ExternType, GlobalType, MemoryType, Mutability, TableType, ValType}; -use crate::{Func, Store, Trap}; use anyhow::{anyhow, bail, Result}; use std::slice; use wasmtime_environ::wasm; @@ -420,7 +420,7 @@ impl Table { let src_table = src_table.instance.get_defined_table(src_table_index); runtime::Table::copy(dst_table, src_table, dst_index, src_index, len) - .map_err(Trap::from_jit)?; + .map_err(error::from_runtime)?; Ok(()) } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index b1b18be7c175..c113df63c3ce 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1,6 +1,6 @@ use crate::runtime::StoreInner; use crate::trampoline::StoreInstanceHandle; -use crate::{Extern, FuncType, Memory, Store, Trap, Val, ValType}; +use crate::{error, Extern, FuncType, Memory, Store, Trap, Val, ValType}; use anyhow::{bail, ensure, Context as _, Result}; use std::cmp::max; use std::fmt; @@ -8,8 +8,8 @@ use std::mem; use std::panic::{self, AssertUnwindSafe}; use std::ptr; use std::rc::Weak; +use wasmtime_runtime::{raise_user_trap, wasi_proc_exit, ExportFunction, VMTrampoline}; use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; -use wasmtime_runtime::{ExportFunction, VMTrampoline}; /// A WebAssembly function which can be called. /// @@ -151,7 +151,7 @@ macro_rules! getters { $(#[$doc])* #[allow(non_snake_case)] pub fn $name<$($args,)* R>(&self) - -> anyhow::Result Result> + -> anyhow::Result Result> where $($args: WasmTy,)* R: WasmTy, @@ -181,7 +181,7 @@ macro_rules! getters { // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! - Ok(move |$($args: $args),*| -> Result { + Ok(move |$($args: $args),*| -> Result { unsafe { let fnptr = mem::transmute::< *const VMFunctionBody, @@ -474,6 +474,46 @@ impl Func { func.into_func(store) } + /// Creates a new `Func` for a function which performs a program exit, + /// unwinding the stack up the point where wasm was most recently entered. + pub fn exit_func(store: &Store) -> Func { + unsafe extern "C" fn trampoline( + callee_vmctx: *mut VMContext, + caller_vmctx: *mut VMContext, + ptr: *const VMFunctionBody, + args: *mut u128, + ) { + let ptr = mem::transmute::< + *const VMFunctionBody, + unsafe extern "C" fn(*mut VMContext, *mut VMContext, i32) -> !, + >(ptr); + + let mut next = args as *const u128; + let status = i32::load(&mut next); + ptr(callee_vmctx, caller_vmctx, status) + } + + let mut args = Vec::new(); + ::push(&mut args); + let ret = Vec::new(); + let ty = FuncType::new(args.into(), ret.into()); + let (instance, export) = unsafe { + crate::trampoline::generate_raw_func_export( + &ty, + std::slice::from_raw_parts_mut(wasi_proc_exit as *mut _, 0), + trampoline, + store, + Box::new(()), + ) + .expect("failed to generate export") + }; + Func { + instance, + export, + trampoline, + } + } + /// Returns the underlying wasm type that this `Func` has. pub fn ty(&self) -> FuncType { // Signatures should always be registered in the store's registry of @@ -737,7 +777,7 @@ pub(crate) fn catch_traps( vmctx: *mut VMContext, store: &Store, closure: impl FnMut(), -) -> Result<(), Trap> { +) -> Result<()> { let signalhandler = store.signal_handler(); unsafe { wasmtime_runtime::catch_traps( @@ -747,7 +787,7 @@ pub(crate) fn catch_traps( signalhandler.as_deref(), closure, ) - .map_err(Trap::from_jit) + .map_err(error::from_runtime) } } @@ -941,7 +981,7 @@ unsafe impl WasmRet for Result { } fn handle_trap(trap: Trap) -> ! { - unsafe { wasmtime_runtime::raise_user_trap(Box::new(trap)) } + unsafe { raise_user_trap(trap.into()) } } } @@ -1133,9 +1173,9 @@ macro_rules! impl_into_func { R::push(&mut ret); let ty = FuncType::new(_args.into(), ret.into()); let store_weak = store.weak(); - unsafe { - let trampoline = trampoline::<$($args,)* R>; - let (instance, export) = crate::trampoline::generate_raw_func_export( + let trampoline = trampoline::<$($args,)* R>; + let (instance, export) = unsafe { + crate::trampoline::generate_raw_func_export( &ty, std::slice::from_raw_parts_mut( shim:: as *mut _, @@ -1145,12 +1185,12 @@ macro_rules! impl_into_func { store, Box::new((self, store_weak)), ) - .expect("failed to generate export"); - Func { - instance, - export, - trampoline, - } + .expect("failed to generate export") + }; + Func { + instance, + export, + trampoline, } } } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 808f5d39790a..87c2700e839c 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,5 +1,5 @@ use crate::trampoline::StoreInstanceHandle; -use crate::{Export, Extern, Func, Global, Memory, Module, Store, Table, Trap}; +use crate::{error, Export, Extern, Func, Global, Memory, Module, Store, Table}; use anyhow::{bail, Error, Result}; use std::any::Any; use std::mem; @@ -52,7 +52,7 @@ fn instantiate( .map_err(|e| -> Error { match e { InstantiationError::StartTrap(trap) | InstantiationError::Trap(trap) => { - Trap::from_jit(trap).into() + error::from_runtime(trap).into() } other => other.into(), } diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index 034283326935..f2463efba653 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -10,6 +10,8 @@ #![doc(test(attr(deny(warnings))))] #![doc(test(attr(allow(dead_code, unused_variables, unused_mut))))] +mod error; +mod exit; mod externals; mod frame_info; mod func; @@ -23,6 +25,7 @@ mod trap; mod types; mod values; +pub use crate::exit::Exit; pub use crate::externals::*; pub use crate::frame_info::FrameInfo; pub use crate::func::*; diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index f1a0b7892a15..94337379790e 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -905,7 +905,7 @@ impl Store { /// }); /// /// let trap = run().unwrap_err(); - /// assert!(trap.message().contains("wasm trap: interrupt")); + /// assert!(trap.to_string().contains("wasm trap: interrupt")); /// # Ok(()) /// # } /// ``` diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index c54f5f0fcb7f..ba64a6420644 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -54,10 +54,10 @@ unsafe extern "C" fn stub_fn( // If a trap was raised (an error returned from the imported function) // then we smuggle the trap through `Box` through to the - // call-site, which gets unwrapped in `Trap::from_jit` later on as we + // call-site, which gets unwrapped in `error::from_runtime` later on as we // convert from the internal `Trap` type to our own `Trap` type in this // crate. - Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(Box::new(trap)), + Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(trap.into()), // And finally if the imported function panicked, then we trigger the // form of unwinding that's safe to jump over wasm code on all diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index c5ea37f96120..26079873bd39 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -34,47 +34,7 @@ impl Trap { Trap::new_with_trace(&info, None, message.into(), Backtrace::new_unresolved()) } - pub(crate) fn from_jit(jit: wasmtime_runtime::Trap) -> Self { - let info = FRAME_INFO.read().unwrap(); - match jit { - wasmtime_runtime::Trap::User(error) => { - // Since we're the only one using the wasmtime internals (in - // theory) we should only see user errors which were originally - // created from our own `Trap` type (see the trampoline module - // with functions). - // - // If this unwrap trips for someone we'll need to tweak the - // return type of this function to probably be `anyhow::Error` - // or something like that. - *error - .downcast() - .expect("only `Trap` user errors are supported") - } - wasmtime_runtime::Trap::Jit { - pc, - backtrace, - maybe_interrupted, - } => { - let mut code = info - .lookup_trap_info(pc) - .map(|info| info.trap_code) - .unwrap_or(TrapCode::StackOverflow); - if maybe_interrupted && code == TrapCode::StackOverflow { - code = TrapCode::Interrupt; - } - Trap::new_wasm(&info, Some(pc), code, backtrace) - } - wasmtime_runtime::Trap::Wasm { - trap_code, - backtrace, - } => Trap::new_wasm(&info, None, trap_code, backtrace), - wasmtime_runtime::Trap::OOM { backtrace } => { - Trap::new_with_trace(&info, None, "out of memory".to_string(), backtrace) - } - } - } - - fn new_wasm( + pub(crate) fn new_wasm( info: &GlobalFrameInfo, trap_pc: Option, code: TrapCode, @@ -98,7 +58,7 @@ impl Trap { Trap::new_with_trace(info, trap_pc, msg, backtrace) } - fn new_with_trace( + pub(crate) fn new_with_trace( info: &GlobalFrameInfo, trap_pc: Option, message: String, diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index 6bb5beed40f4..0881c1b04820 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -12,6 +12,12 @@ pub fn define_func( ) -> TokenStream { let funcname = func.name.as_str(); + // `proc_exit` is special; it's essentially an unwinding primitive, + // so we implement it in the runtime rather than in wasi-common. + if funcname == "proc_exit" { + return quote!(); + } + let ident = names.func(&func.name); let rt = names.runtime_mod(); let ctx_type = names.ctx_type(); diff --git a/crates/wiggle/generate/src/module_trait.rs b/crates/wiggle/generate/src/module_trait.rs index b09f23702831..b413c28795f5 100644 --- a/crates/wiggle/generate/src/module_trait.rs +++ b/crates/wiggle/generate/src/module_trait.rs @@ -22,48 +22,54 @@ pub fn passed_by_reference(ty: &witx::Type) -> bool { pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let traitname = names.trait_name(&m.name); - let traitmethods = m.funcs().map(|f| { - // Check if we're returning an entity anotated with a lifetime, - // in which case, we'll need to annotate the function itself, and - // hence will need an explicit lifetime (rather than anonymous) - let (lifetime, is_anonymous) = if f - .params - .iter() - .chain(&f.results) - .any(|ret| ret.tref.needs_lifetime()) - { - (quote!('a), false) - } else { - (anon_lifetime(), true) - }; - let funcname = names.func(&f.name); - let args = f.params.iter().map(|arg| { - let arg_name = names.func_param(&arg.name); - let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); - let arg_type = if passed_by_reference(&*arg.tref.type_()) { - quote!(&#arg_typename) + let is_wasi = m.name.as_str().starts_with("wasi"); + let traitmethods = m + .funcs() + // `proc_exit` is special; it's essentially an unwinding primitive, + // so we implement it in the runtime rather than in wasi-common. + .filter(|f| !is_wasi || f.name.as_str() != "proc_exit") + .map(|f| { + // Check if we're returning an entity anotated with a lifetime, + // in which case, we'll need to annotate the function itself, and + // hence will need an explicit lifetime (rather than anonymous) + let (lifetime, is_anonymous) = if f + .params + .iter() + .chain(&f.results) + .any(|ret| ret.tref.needs_lifetime()) + { + (quote!('a), false) } else { - quote!(#arg_typename) + (anon_lifetime(), true) }; - quote!(#arg_name: #arg_type) - }); - let rets = f - .results - .iter() - .skip(1) - .map(|ret| names.type_ref(&ret.tref, lifetime.clone())); - let err = f - .results - .get(0) - .map(|err_result| names.type_ref(&err_result.tref, lifetime.clone())) - .unwrap_or(quote!(())); + let funcname = names.func(&f.name); + let args = f.params.iter().map(|arg| { + let arg_name = names.func_param(&arg.name); + let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); + let arg_type = if passed_by_reference(&*arg.tref.type_()) { + quote!(&#arg_typename) + } else { + quote!(#arg_typename) + }; + quote!(#arg_name: #arg_type) + }); + let rets = f + .results + .iter() + .skip(1) + .map(|ret| names.type_ref(&ret.tref, lifetime.clone())); + let err = f + .results + .get(0) + .map(|err_result| names.type_ref(&err_result.tref, lifetime.clone())) + .unwrap_or(quote!(())); - if is_anonymous { - quote!(fn #funcname(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) - } else { - quote!(fn #funcname<#lifetime>(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) - } - }); + if is_anonymous { + quote!(fn #funcname(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) + } else { + quote!(fn #funcname<#lifetime>(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) + } + }); quote! { pub trait #traitname { #(#traitmethods)* diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 5985d4b16b31..8d5dc0f9db39 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -316,10 +316,6 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { unimplemented!("poll_oneoff") } - fn proc_exit(&self, _rval: types::Exitcode) -> std::result::Result<(), ()> { - unimplemented!("proc_exit") - } - fn proc_raise(&self, _sig: types::Signal) -> Result<()> { unimplemented!("proc_raise") } diff --git a/examples/interrupt.rs b/examples/interrupt.rs index 696d8a2d952d..276c51320add 100644 --- a/examples/interrupt.rs +++ b/examples/interrupt.rs @@ -29,10 +29,14 @@ fn main() -> Result<()> { }); println!("Entering infinite loop ..."); - let trap = run().unwrap_err(); + let error = run().unwrap_err(); println!("trap received..."); - assert!(trap.message().contains("wasm trap: interrupt")); + assert!(error + .downcast_ref::() + .unwrap() + .message() + .contains("wasm trap: interrupt")); Ok(()) } diff --git a/src/commands/run.rs b/src/commands/run.rs index d30785eb68e6..123669c6697d 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -12,7 +12,7 @@ use std::{ }; use structopt::{clap::AppSettings, StructOpt}; use wasi_common::preopen_dir; -use wasmtime::{Engine, Instance, Module, Store, Trap, Val, ValType}; +use wasmtime::{Engine, Exit, Instance, Module, Store, Trap, Val, ValType}; use wasmtime_wasi::{old::snapshot_0::Wasi as WasiSnapshot0, Wasi}; fn parse_module(s: &OsStr) -> Result { @@ -142,6 +142,17 @@ impl RunCommand { { Ok(()) => (), Err(e) => { + // If the program exited because of a non-zero exit status, print + // a message and exit. + if let Some(exit) = e.downcast_ref::() { + eprintln!("Error: {}", exit); + if cfg!(unix) { + // On Unix, if it's a normal exit status, return it. + process::exit(exit.status().get()); + } + process::exit(1); + } + // 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. diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 04500f213a08..3d6e02174d32 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -182,3 +182,43 @@ fn timeout_in_invoke() -> Result<()> { ); Ok(()) } + +// Exit with a valid non-zero exit code, snapshot0 edition. +#[test] +fn exit125_wasi_snapshot0() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit125_wasi_snapshot0.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 125); + Ok(()) +} + +// Exit with a valid non-zero exit code, snapshot1 edition. +#[test] +fn exit125_wasi_snapshot1() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit125_wasi_snapshot1.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 125); + Ok(()) +} + +// Exit with an invalid non-zero exit code, snapshot0 edition. +#[test] +fn exit126_wasi_snapshot0() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit126_wasi_snapshot0.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 1); + assert!(output.stdout.is_empty()); + assert!(String::from_utf8_lossy(&output.stderr).contains("invalid WASI exit status")); + Ok(()) +} + +// Exit with an invalid non-zero exit code, snapshot1 edition. +#[test] +fn exit126_wasi_snapshot1() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit126_wasi_snapshot1.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 1); + assert!(output.stdout.is_empty()); + assert!(String::from_utf8_lossy(&output.stderr).contains("invalid WASI exit status")); + Ok(()) +} diff --git a/tests/all/iloop.rs b/tests/all/iloop.rs index 9e0f5aa851d2..abfe5e4b3de2 100644 --- a/tests/all/iloop.rs +++ b/tests/all/iloop.rs @@ -30,7 +30,7 @@ fn loops_interruptable() -> anyhow::Result<()> { let iloop = instance.get_func("loop").unwrap().get0::<()>()?; store.interrupt_handle()?.interrupt(); let trap = iloop().unwrap_err(); - assert!(trap.message().contains("wasm trap: interrupt")); + assert!(trap.to_string().contains("wasm trap: interrupt")); Ok(()) } @@ -44,9 +44,9 @@ fn functions_interruptable() -> anyhow::Result<()> { store.interrupt_handle()?.interrupt(); let trap = iloop().unwrap_err(); assert!( - trap.message().contains("wasm trap: interrupt"), + trap.to_string().contains("wasm trap: interrupt"), "{}", - trap.message() + trap.to_string() ); Ok(()) } @@ -91,9 +91,9 @@ fn loop_interrupt_from_afar() -> anyhow::Result<()> { let trap = iloop().unwrap_err(); thread.join().unwrap(); assert!( - trap.message().contains("wasm trap: interrupt"), + trap.to_string().contains("wasm trap: interrupt"), "bad message: {}", - trap.message() + trap.to_string() ); Ok(()) } @@ -127,9 +127,9 @@ fn function_interrupt_from_afar() -> anyhow::Result<()> { let trap = iloop().unwrap_err(); thread.join().unwrap(); assert!( - trap.message().contains("wasm trap: interrupt"), + trap.to_string().contains("wasm trap: interrupt"), "bad message: {}", - trap.message() + trap.to_string() ); Ok(()) } diff --git a/tests/all/stack_overflow.rs b/tests/all/stack_overflow.rs index 283426130ec5..9d63dbdc4ca8 100644 --- a/tests/all/stack_overflow.rs +++ b/tests/all/stack_overflow.rs @@ -30,9 +30,9 @@ fn host_always_has_some_stack() -> anyhow::Result<()> { // has been exhausted. let trap = foo().unwrap_err(); assert!( - trap.message().contains("call stack exhausted"), + trap.to_string().contains("call stack exhausted"), "{}", - trap.message() + trap.to_string() ); // Additionally, however, and this is the crucial test, make sure that the diff --git a/tests/wasm/exit125_wasi_snapshot0.wat b/tests/wasm/exit125_wasi_snapshot0.wat new file mode 100644 index 000000000000..03e2ae9004b0 --- /dev/null +++ b/tests/wasm/exit125_wasi_snapshot0.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_unstable" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 125)) + ) + (export "_start" (func $_start)) +) diff --git a/tests/wasm/exit125_wasi_snapshot1.wat b/tests/wasm/exit125_wasi_snapshot1.wat new file mode 100644 index 000000000000..6d2dbbfd3857 --- /dev/null +++ b/tests/wasm/exit125_wasi_snapshot1.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_snapshot_preview1" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 125)) + ) + (export "_start" (func $_start)) +) diff --git a/tests/wasm/exit126_wasi_snapshot0.wat b/tests/wasm/exit126_wasi_snapshot0.wat new file mode 100644 index 000000000000..091e61001117 --- /dev/null +++ b/tests/wasm/exit126_wasi_snapshot0.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_unstable" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 126)) + ) + (export "_start" (func $_start)) +) diff --git a/tests/wasm/exit126_wasi_snapshot1.wat b/tests/wasm/exit126_wasi_snapshot1.wat new file mode 100644 index 000000000000..e481533c5928 --- /dev/null +++ b/tests/wasm/exit126_wasi_snapshot1.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_snapshot_preview1" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 126)) + ) + (export "_start" (func $_start)) +) From 36a2f9f7b080a8997a533f21a2b615532d73ff46 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 1 May 2020 16:49:45 -0700 Subject: [PATCH 03/11] Fix exit-status tests on Windows. --- src/commands/run.rs | 9 +++++---- tests/all/cli_tests.rs | 12 ++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/commands/run.rs b/src/commands/run.rs index 123669c6697d..1e94cc08a899 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -146,11 +146,12 @@ impl RunCommand { // a message and exit. if let Some(exit) = e.downcast_ref::() { eprintln!("Error: {}", exit); - if cfg!(unix) { - // On Unix, if it's a normal exit status, return it. - process::exit(exit.status().get()); + // On Windows, exit status 3 indicates an abort (see below), + // so just return 1 indicating a non-zero status. + if cfg!(windows) { + process::exit(1); } - process::exit(1); + process::exit(exit.status().get()); } // If the program exited because of a trap, return an error code diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 3d6e02174d32..58a50c720c3a 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -188,7 +188,11 @@ fn timeout_in_invoke() -> Result<()> { fn exit125_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/wasm/exit125_wasi_snapshot0.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - assert_eq!(output.status.code().unwrap(), 125); + if cfg!(windows) { + assert_eq!(output.status.code().unwrap(), 1); + } else { + assert_eq!(output.status.code().unwrap(), 125); + } Ok(()) } @@ -197,7 +201,11 @@ fn exit125_wasi_snapshot0() -> Result<()> { fn exit125_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/wasm/exit125_wasi_snapshot1.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - assert_eq!(output.status.code().unwrap(), 125); + if cfg!(windows) { + assert_eq!(output.status.code().unwrap(), 1); + } else { + assert_eq!(output.status.code().unwrap(), 125); + } Ok(()) } From d1700cbb3ae4d1b2847a0bd807ad03ced2aa680b Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 11 May 2020 14:16:44 -0700 Subject: [PATCH 04/11] Revert the wiggle changes and re-introduce the wasi-common implementations. --- Cargo.lock | 1 - crates/api/src/error.rs | 37 -------- crates/api/src/exit.rs | 40 -------- crates/c-api/src/trap.rs | 2 +- crates/runtime/Cargo.toml | 1 - crates/runtime/src/lib.rs | 2 +- crates/runtime/src/traphandlers.rs | 38 ++++---- .../src/old/snapshot_0/hostcalls_impl/misc.rs | 11 +++ .../src/snapshots/wasi_snapshot_preview1.rs | 12 +++ crates/wasi-common/wig/src/hostcalls.rs | 3 +- crates/wasi-common/wig/src/wasi.rs | 6 +- crates/wasmtime/src/externals.rs | 4 +- crates/wasmtime/src/func.rs | 10 +- crates/wasmtime/src/instance.rs | 4 +- crates/wasmtime/src/lib.rs | 5 +- crates/wasmtime/src/runtime.rs | 2 +- crates/wasmtime/src/trampoline/func.rs | 4 +- crates/wasmtime/src/trap.rs | 92 +++++++++++++++++-- crates/wast/src/wast.rs | 4 +- crates/wiggle/generate/src/funcs.rs | 6 -- crates/wiggle/generate/src/module_trait.rs | 84 ++++++++--------- crates/wiggle/tests/wasi.rs | 4 + examples/interrupt.rs | 8 +- src/commands/run.rs | 29 +++--- tests/all/cli_tests.rs | 16 +++- tests/all/custom_signal_handler.rs | 8 +- tests/all/func.rs | 6 +- tests/all/import_calling_export.rs | 2 +- tests/all/traps.rs | 20 ++-- tests/spec_testsuite | 2 +- 30 files changed, 242 insertions(+), 221 deletions(-) delete mode 100644 crates/api/src/error.rs delete mode 100644 crates/api/src/exit.rs diff --git a/Cargo.lock b/Cargo.lock index 998ffaa53acf..c1432510963b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2273,7 +2273,6 @@ dependencies = [ name = "wasmtime-runtime" version = "0.16.0" dependencies = [ - "anyhow", "backtrace", "cc", "cfg-if", diff --git a/crates/api/src/error.rs b/crates/api/src/error.rs deleted file mode 100644 index dfac504fad48..000000000000 --- a/crates/api/src/error.rs +++ /dev/null @@ -1,37 +0,0 @@ -use crate::frame_info::FRAME_INFO; -use crate::{Exit, Trap}; -use anyhow::Error; -use wasmtime_environ::ir::TrapCode; - -/// Convert from an internal unwinding code into an `Error`. -pub(crate) fn from_runtime(jit: wasmtime_runtime::Trap) -> Error { - let info = FRAME_INFO.read().unwrap(); - match jit { - wasmtime_runtime::Trap::User(error) => error, - wasmtime_runtime::Trap::Jit { - pc, - backtrace, - maybe_interrupted, - } => { - let mut code = info - .lookup_trap_info(pc) - .map(|info| info.trap_code) - .unwrap_or(TrapCode::StackOverflow); - if maybe_interrupted && code == TrapCode::StackOverflow { - code = TrapCode::Interrupt; - } - Error::new(Trap::new_wasm(&info, Some(pc), code, backtrace)) - } - wasmtime_runtime::Trap::Wasm { - trap_code, - backtrace, - } => Error::new(Trap::new_wasm(&info, None, trap_code, backtrace)), - wasmtime_runtime::Trap::OOM { backtrace } => Error::new(Trap::new_with_trace( - &info, - None, - "out of memory".to_string(), - backtrace, - )), - wasmtime_runtime::Trap::Exit { status } => Error::new(Exit::new(status)), - } -} diff --git a/crates/api/src/exit.rs b/crates/api/src/exit.rs deleted file mode 100644 index 866b83f0449b..000000000000 --- a/crates/api/src/exit.rs +++ /dev/null @@ -1,40 +0,0 @@ -use std::fmt; -use std::num::NonZeroI32; - -/// A struct representing an explicit program exit, with a code -/// indicating the status. -#[derive(Clone, Debug)] -pub struct Exit { - status: NonZeroI32, -} - -impl Exit { - /// Creates a new `Exit` with `status`. The status value must be in the range [1..126]. - /// # Example - /// ``` - /// let exit = wasmtime::Exit::new(std::num::NonZeroI32::new(1).unwrap()); - /// assert_eq!(1, exit.status().get()); - /// ``` - pub fn new(status: NonZeroI32) -> Self { - assert!(status.get() > 0 && status.get() < 126); - Self { status } - } - - /// Returns a reference the `status` stored in `Exit`. - pub fn status(&self) -> NonZeroI32 { - self.status - } -} - -impl fmt::Display for Exit { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // Error codes used by WASI libc. - match self.status.get() { - 70 => write!(f, "internal software error"), - 71 => write!(f, "system error"), - _ => write!(f, "non-zero status {}", self.status), - } - } -} - -impl std::error::Error for Exit {} diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 67ca334d7bb3..83289a348f71 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -53,7 +53,7 @@ pub extern "C" fn wasm_trap_new( #[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.borrow().message().as_bytes()); + buffer.extend_from_slice(trap.trap.borrow().reason().to_string().as_bytes()); buffer.reserve_exact(1); buffer.push(0); out.set_buffer(buffer); diff --git a/crates/runtime/Cargo.toml b/crates/runtime/Cargo.toml index b7a84af5d14a..a3ea52cc32a3 100644 --- a/crates/runtime/Cargo.toml +++ b/crates/runtime/Cargo.toml @@ -21,7 +21,6 @@ thiserror = "1.0.4" more-asserts = "0.2.1" cfg-if = "0.1.9" backtrace = "0.3.42" -anyhow = "1.0.19" [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3.7", features = ["winbase", "memoryapi", "errhandlingapi"] } diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 89d07223140b..a3c110de043e 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -45,7 +45,7 @@ pub use crate::sig_registry::SignatureRegistry; pub use crate::table::Table; pub use crate::traphandlers::{ catch_traps, init_traps, raise_lib_trap, raise_user_trap, resume_panic, wasi_proc_exit, - InvalidWASIExitStatus, SignalHandler, Trap, + SignalHandler, Trap, }; pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index 2b0d3eb8351f..aaded85b5969 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -2,11 +2,10 @@ //! signalhandling mechanisms. use crate::VMContext; -use anyhow::Error; use backtrace::Backtrace; use std::any::Any; use std::cell::Cell; -use std::fmt; +use std::error::Error; use std::io; use std::num::NonZeroI32; use std::ptr; @@ -273,7 +272,7 @@ fn real_init() { /// Only safe to call when wasm code is on the stack, aka `catch_traps` must /// have been previously called. Additionally no Rust destructors can be on the /// stack. They will be skipped and not executed. -pub unsafe fn raise_user_trap(data: Error) -> ! { +pub unsafe fn raise_user_trap(data: Box) -> ! { tls::with(|info| info.unwrap().unwind_with(UnwindReason::UserTrap(data))) } @@ -307,7 +306,7 @@ pub unsafe fn resume_panic(payload: Box) -> ! { #[derive(Debug)] pub enum Trap { /// A user-raised trap through `raise_user_trap`. - User(Error), + User(Box), /// A trap raised from jit code Jit { @@ -340,6 +339,12 @@ pub enum Trap { /// The exit status status: NonZeroI32, }, + + /// A program exit was requested with an invalid exit status. + InvalidExitStatus { + /// Native stack backtrace at the time the error occurred + backtrace: Backtrace, + }, } impl Trap { @@ -361,6 +366,14 @@ impl Trap { let backtrace = Backtrace::new_unresolved(); Trap::OOM { backtrace } } + + /// Construct a new InvalidExitStatus trap with the given source location. + /// + /// Internally saves a backtrace when constructed. + pub fn invalid_exit_status() -> Self { + let backtrace = Backtrace::new_unresolved(); + Trap::InvalidExitStatus { backtrace } + } } /// Catches any wasm traps that happen within the execution of `closure`, @@ -411,7 +424,7 @@ pub struct CallThreadState<'a> { enum UnwindReason { None, Panic(Box), - UserTrap(Error), + UserTrap(Box), LibTrap(Trap), JitTrap { backtrace: Backtrace, pc: usize }, Exit { status: i32 }, @@ -816,19 +829,6 @@ pub unsafe extern "C" fn wasi_proc_exit( if status >= 0 && status < 126 { tls::with(|info| info.unwrap().unwind_with(UnwindReason::Exit { status })) } else { - raise_user_trap(InvalidWASIExitStatus {}.into()) + raise_lib_trap(Trap::invalid_exit_status()) } } - -/// An error type for indicating an exit was requested using an exit status outside -/// of WASI's valid range of [0..126]. -#[derive(Debug)] -pub struct InvalidWASIExitStatus {} - -impl fmt::Display for InvalidWASIExitStatus { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "invalid WASI exit status; values must be in [0..126]") - } -} - -impl std::error::Error for InvalidWASIExitStatus {} diff --git a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs index d6f4d2468484..6b75a24414d5 100644 --- a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs +++ b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs @@ -342,6 +342,17 @@ pub(crate) struct FdEventData<'a> { pub(crate) userdata: wasi::__wasi_userdata_t, } +// Not all implementations use this implementation. +#[allow(dead_code)] +pub(crate) fn proc_exit(_wasi_ctx: &WasiCtx, _memory: &mut [u8], rval: wasi::__wasi_exitcode_t) { + trace!("proc_exit(rval={:?})", rval); + + // Note that this is just a default implementation. Host environments + // will often override this `proc_exit` implementation and do other + // things other than exit the host process. + std::process::exit(rval as i32); +} + pub(crate) fn proc_raise( _wasi_ctx: &WasiCtx, _memory: &mut [u8], diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index b2f06e3b1b1c..63c05f4c6d98 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -813,6 +813,18 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { Ok(nevents) } + // Not all implementations use this implementation. + #[allow(dead_code)] + // This is just a temporary to ignore the warning which becomes a hard error + // in the CI. Once we figure out non-returns in `wiggle`, this should be gone. + #[allow(unreachable_code)] + fn proc_exit(&self, rval: types::Exitcode) -> std::result::Result<(), ()> { + // Note that this is just a default implementation. Host environments + // will often override this `proc_exit` implementation and do other + // things other than exit the host process. + std::process::exit(rval as i32); + } + fn proc_raise(&self, _sig: types::Signal) -> Result<()> { unimplemented!("proc_raise") } diff --git a/crates/wasi-common/wig/src/hostcalls.rs b/crates/wasi-common/wig/src/hostcalls.rs index f3912e91004e..2ea4e6e5eb7c 100644 --- a/crates/wasi-common/wig/src/hostcalls.rs +++ b/crates/wasi-common/wig/src/hostcalls.rs @@ -18,7 +18,8 @@ pub fn define(args: TokenStream) -> TokenStream { for module in doc.modules() { for func in module.funcs() { // `proc_exit` is special; it's essentially an unwinding primitive, - // so we implement it in the runtime rather than in wasi-common. + // so we implement it in the runtime rather than use the implementation + // in wasi-common. if func.name.as_str() == "proc_exit" { continue; } diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index 57c95b52de2b..51af787193df 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -47,7 +47,8 @@ pub fn define_struct(args: TokenStream) -> TokenStream { linker.define(#module_name, #name, self.#name_ident.clone())?; }); // `proc_exit` is special; it's essentially an unwinding primitive, - // so we implement it in the runtime rather than in wasi-common. + // so we implement it in the runtime rather than use the implementation + // in wasi-common. if name == "proc_exit" { ctor_externs.push(quote! { let #name_ident = wasmtime::Func::exit_func(store); @@ -300,7 +301,8 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { linker.define(#module_name, #name, self.#name_ident.clone())?; }); // `proc_exit` is special; it's essentially an unwinding primitive, - // so we implement it in the runtime rather than in wasi-common. + // so we implement it in the runtime rather than use the implementation + // in wasi-common. if name == "proc_exit" { ctor_externs.push(quote! { let #name_ident = wasmtime::Func::exit_func(store); diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index b14ff291dd43..6536f017fcef 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -2,8 +2,8 @@ use crate::trampoline::{ generate_global_export, generate_memory_export, generate_table_export, StoreInstanceHandle, }; use crate::values::{from_checked_anyfunc, into_checked_anyfunc, Val}; -use crate::{error, Func, Store}; use crate::{ExternType, GlobalType, MemoryType, Mutability, TableType, ValType}; +use crate::{Func, Store, Trap}; use anyhow::{anyhow, bail, Result}; use std::slice; use wasmtime_environ::wasm; @@ -420,7 +420,7 @@ impl Table { let src_table = src_table.instance.get_defined_table(src_table_index); runtime::Table::copy(dst_table, src_table, dst_index, src_index, len) - .map_err(error::from_runtime)?; + .map_err(Trap::from_jit)?; Ok(()) } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index c113df63c3ce..8f5093970a07 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -1,6 +1,6 @@ use crate::runtime::StoreInner; use crate::trampoline::StoreInstanceHandle; -use crate::{error, Extern, FuncType, Memory, Store, Trap, Val, ValType}; +use crate::{Extern, FuncType, Memory, Store, Trap, Val, ValType}; use anyhow::{bail, ensure, Context as _, Result}; use std::cmp::max; use std::fmt; @@ -151,7 +151,7 @@ macro_rules! getters { $(#[$doc])* #[allow(non_snake_case)] pub fn $name<$($args,)* R>(&self) - -> anyhow::Result Result> + -> anyhow::Result Result> where $($args: WasmTy,)* R: WasmTy, @@ -181,7 +181,7 @@ macro_rules! getters { // ... and then once we've passed the typechecks we can hand out our // object since our `transmute` below should be safe! - Ok(move |$($args: $args),*| -> Result { + Ok(move |$($args: $args),*| -> Result { unsafe { let fnptr = mem::transmute::< *const VMFunctionBody, @@ -777,7 +777,7 @@ pub(crate) fn catch_traps( vmctx: *mut VMContext, store: &Store, closure: impl FnMut(), -) -> Result<()> { +) -> Result<(), Trap> { let signalhandler = store.signal_handler(); unsafe { wasmtime_runtime::catch_traps( @@ -787,7 +787,7 @@ pub(crate) fn catch_traps( signalhandler.as_deref(), closure, ) - .map_err(error::from_runtime) + .map_err(Trap::from_jit) } } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 87c2700e839c..808f5d39790a 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -1,5 +1,5 @@ use crate::trampoline::StoreInstanceHandle; -use crate::{error, Export, Extern, Func, Global, Memory, Module, Store, Table}; +use crate::{Export, Extern, Func, Global, Memory, Module, Store, Table, Trap}; use anyhow::{bail, Error, Result}; use std::any::Any; use std::mem; @@ -52,7 +52,7 @@ fn instantiate( .map_err(|e| -> Error { match e { InstantiationError::StartTrap(trap) | InstantiationError::Trap(trap) => { - error::from_runtime(trap).into() + Trap::from_jit(trap).into() } other => other.into(), } diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index f2463efba653..da450c58bde0 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -10,8 +10,6 @@ #![doc(test(attr(deny(warnings))))] #![doc(test(attr(allow(dead_code, unused_variables, unused_mut))))] -mod error; -mod exit; mod externals; mod frame_info; mod func; @@ -25,7 +23,6 @@ mod trap; mod types; mod values; -pub use crate::exit::Exit; pub use crate::externals::*; pub use crate::frame_info::FrameInfo; pub use crate::func::*; @@ -34,7 +31,7 @@ pub use crate::linker::*; pub use crate::module::Module; pub use crate::r#ref::{AnyRef, HostRef}; pub use crate::runtime::*; -pub use crate::trap::Trap; +pub use crate::trap::{Trap, TrapReason}; pub use crate::types::*; pub use crate::values::*; diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index 94337379790e..0c85628b79fc 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -905,7 +905,7 @@ impl Store { /// }); /// /// let trap = run().unwrap_err(); - /// assert!(trap.to_string().contains("wasm trap: interrupt")); + /// assert!(trap.reason().to_string().contains("wasm trap: interrupt")); /// # Ok(()) /// # } /// ``` diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index ba64a6420644..c54f5f0fcb7f 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -54,10 +54,10 @@ unsafe extern "C" fn stub_fn( // If a trap was raised (an error returned from the imported function) // then we smuggle the trap through `Box` through to the - // call-site, which gets unwrapped in `error::from_runtime` later on as we + // call-site, which gets unwrapped in `Trap::from_jit` later on as we // convert from the internal `Trap` type to our own `Trap` type in this // crate. - Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(trap.into()), + Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(Box::new(trap)), // And finally if the imported function panicked, then we trigger the // form of unwinding that's safe to jump over wasm code on all diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 26079873bd39..05467ab8e064 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -2,6 +2,7 @@ use crate::frame_info::{GlobalFrameInfo, FRAME_INFO}; use crate::FrameInfo; use backtrace::Backtrace; use std::fmt; +use std::num::NonZeroI32; use std::sync::Arc; use wasmtime_environ::ir::TrapCode; @@ -12,8 +13,27 @@ pub struct Trap { inner: Arc, } +/// State describing the occasion which evoked a trap. +#[derive(Debug)] +pub enum TrapReason { + /// An error message describing a trap. + Message(String), + + /// A non-zero exit status describing an explicit program exit. + Exit(NonZeroI32), +} + +impl fmt::Display for TrapReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TrapReason::Message(s) => write!(f, "{}", s), + TrapReason::Exit(status) => write!(f, "Exited with non-zero exit status {}", status), + } + } +} + struct TrapInner { - message: String, + reason: TrapReason, wasm_trace: Vec, native_trace: Backtrace, } @@ -27,14 +47,58 @@ impl Trap { /// # Example /// ``` /// let trap = wasmtime::Trap::new("unexpected error"); - /// assert_eq!("unexpected error", trap.message()); + /// assert_eq!("unexpected error", trap.reason().to_string()); /// ``` pub fn new>(message: I) -> Self { let info = FRAME_INFO.read().unwrap(); Trap::new_with_trace(&info, None, message.into(), Backtrace::new_unresolved()) } - pub(crate) fn new_wasm( + pub(crate) fn from_jit(jit: wasmtime_runtime::Trap) -> Self { + let info = FRAME_INFO.read().unwrap(); + match jit { + wasmtime_runtime::Trap::User(error) => { + // Since we're the only one using the wasmtime internals (in + // theory) we should only see user errors which were originally + // created from our own `Trap` type (see the trampoline module + // with functions). + // + // If this unwrap trips for someone we'll need to tweak the + // return type of this function to probably be `anyhow::Error` + // or something like that. + *error + .downcast() + .expect("only `Trap` user errors are supported") + } + wasmtime_runtime::Trap::Jit { + pc, + backtrace, + maybe_interrupted, + } => { + let mut code = info + .lookup_trap_info(pc) + .map(|info| info.trap_code) + .unwrap_or(TrapCode::StackOverflow); + if maybe_interrupted && code == TrapCode::StackOverflow { + code = TrapCode::Interrupt; + } + Trap::new_wasm(&info, Some(pc), code, backtrace) + } + wasmtime_runtime::Trap::Wasm { + trap_code, + backtrace, + } => Trap::new_wasm(&info, None, trap_code, backtrace), + wasmtime_runtime::Trap::OOM { backtrace } => { + Trap::new_with_trace(&info, None, "out of memory".to_string(), backtrace) + } + wasmtime_runtime::Trap::Exit { status } => Trap::new_exit(status), + wasmtime_runtime::Trap::InvalidExitStatus { backtrace } => { + Trap::new_with_trace(&info, None, "invalid exit status".to_string(), backtrace) + } + } + } + + fn new_wasm( info: &GlobalFrameInfo, trap_pc: Option, code: TrapCode, @@ -58,7 +122,7 @@ impl Trap { Trap::new_with_trace(info, trap_pc, msg, backtrace) } - pub(crate) fn new_with_trace( + fn new_with_trace( info: &GlobalFrameInfo, trap_pc: Option, message: String, @@ -87,16 +151,26 @@ impl Trap { } Trap { inner: Arc::new(TrapInner { - message, + reason: TrapReason::Message(message), wasm_trace, native_trace, }), } } + fn new_exit(status: NonZeroI32) -> Self { + Trap { + inner: Arc::new(TrapInner { + reason: TrapReason::Exit(status), + wasm_trace: Vec::new(), + native_trace: Backtrace::from(Vec::new()), + }), + } + } + /// Returns a reference the `message` stored in `Trap`. - pub fn message(&self) -> &str { - &self.inner.message + pub fn reason(&self) -> &TrapReason { + &self.inner.reason } /// Returns a list of function frames in WebAssembly code that led to this @@ -109,7 +183,7 @@ impl Trap { impl fmt::Debug for Trap { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Trap") - .field("message", &self.inner.message) + .field("reason", &self.inner.reason) .field("wasm_trace", &self.inner.wasm_trace) .field("native_trace", &self.inner.native_trace) .finish() @@ -118,7 +192,7 @@ impl fmt::Debug for Trap { impl fmt::Display for Trap { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.inner.message)?; + write!(f, "{}", self.inner.reason)?; let trace = self.trace(); if trace.is_empty() { return Ok(()); diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 83450bac9e2b..14717fbaf7b7 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -123,7 +123,7 @@ impl WastContext { fn module(&mut self, instance_name: Option<&str>, module: &[u8]) -> Result<()> { let instance = match self.instantiate(module)? { Outcome::Ok(i) => i, - Outcome::Trap(e) => bail!("instantiation failed with: {}", e.message()), + Outcome::Trap(e) => bail!("instantiation failed: {}", e.reason()), }; if let Some(name) = instance_name { self.linker.instance(name, &instance)?; @@ -189,7 +189,7 @@ impl WastContext { Outcome::Ok(values) => bail!("expected trap, got {:?}", values), Outcome::Trap(t) => t, }; - let actual = trap.message(); + let actual = trap.reason().to_string(); 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/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index 0881c1b04820..6bb5beed40f4 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -12,12 +12,6 @@ pub fn define_func( ) -> TokenStream { let funcname = func.name.as_str(); - // `proc_exit` is special; it's essentially an unwinding primitive, - // so we implement it in the runtime rather than in wasi-common. - if funcname == "proc_exit" { - return quote!(); - } - let ident = names.func(&func.name); let rt = names.runtime_mod(); let ctx_type = names.ctx_type(); diff --git a/crates/wiggle/generate/src/module_trait.rs b/crates/wiggle/generate/src/module_trait.rs index b413c28795f5..b09f23702831 100644 --- a/crates/wiggle/generate/src/module_trait.rs +++ b/crates/wiggle/generate/src/module_trait.rs @@ -22,54 +22,48 @@ pub fn passed_by_reference(ty: &witx::Type) -> bool { pub fn define_module_trait(names: &Names, m: &Module) -> TokenStream { let traitname = names.trait_name(&m.name); - let is_wasi = m.name.as_str().starts_with("wasi"); - let traitmethods = m - .funcs() - // `proc_exit` is special; it's essentially an unwinding primitive, - // so we implement it in the runtime rather than in wasi-common. - .filter(|f| !is_wasi || f.name.as_str() != "proc_exit") - .map(|f| { - // Check if we're returning an entity anotated with a lifetime, - // in which case, we'll need to annotate the function itself, and - // hence will need an explicit lifetime (rather than anonymous) - let (lifetime, is_anonymous) = if f - .params - .iter() - .chain(&f.results) - .any(|ret| ret.tref.needs_lifetime()) - { - (quote!('a), false) + let traitmethods = m.funcs().map(|f| { + // Check if we're returning an entity anotated with a lifetime, + // in which case, we'll need to annotate the function itself, and + // hence will need an explicit lifetime (rather than anonymous) + let (lifetime, is_anonymous) = if f + .params + .iter() + .chain(&f.results) + .any(|ret| ret.tref.needs_lifetime()) + { + (quote!('a), false) + } else { + (anon_lifetime(), true) + }; + let funcname = names.func(&f.name); + let args = f.params.iter().map(|arg| { + let arg_name = names.func_param(&arg.name); + let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); + let arg_type = if passed_by_reference(&*arg.tref.type_()) { + quote!(&#arg_typename) } else { - (anon_lifetime(), true) + quote!(#arg_typename) }; - let funcname = names.func(&f.name); - let args = f.params.iter().map(|arg| { - let arg_name = names.func_param(&arg.name); - let arg_typename = names.type_ref(&arg.tref, lifetime.clone()); - let arg_type = if passed_by_reference(&*arg.tref.type_()) { - quote!(&#arg_typename) - } else { - quote!(#arg_typename) - }; - quote!(#arg_name: #arg_type) - }); - let rets = f - .results - .iter() - .skip(1) - .map(|ret| names.type_ref(&ret.tref, lifetime.clone())); - let err = f - .results - .get(0) - .map(|err_result| names.type_ref(&err_result.tref, lifetime.clone())) - .unwrap_or(quote!(())); - - if is_anonymous { - quote!(fn #funcname(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) - } else { - quote!(fn #funcname<#lifetime>(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) - } + quote!(#arg_name: #arg_type) }); + let rets = f + .results + .iter() + .skip(1) + .map(|ret| names.type_ref(&ret.tref, lifetime.clone())); + let err = f + .results + .get(0) + .map(|err_result| names.type_ref(&err_result.tref, lifetime.clone())) + .unwrap_or(quote!(())); + + if is_anonymous { + quote!(fn #funcname(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) + } else { + quote!(fn #funcname<#lifetime>(&self, #(#args),*) -> Result<(#(#rets),*), #err>;) + } + }); quote! { pub trait #traitname { #(#traitmethods)* diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 8d5dc0f9db39..5985d4b16b31 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -316,6 +316,10 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { unimplemented!("poll_oneoff") } + fn proc_exit(&self, _rval: types::Exitcode) -> std::result::Result<(), ()> { + unimplemented!("proc_exit") + } + fn proc_raise(&self, _sig: types::Signal) -> Result<()> { unimplemented!("proc_raise") } diff --git a/examples/interrupt.rs b/examples/interrupt.rs index 276c51320add..38031ac6ce60 100644 --- a/examples/interrupt.rs +++ b/examples/interrupt.rs @@ -29,14 +29,10 @@ fn main() -> Result<()> { }); println!("Entering infinite loop ..."); - let error = run().unwrap_err(); + let trap = run().unwrap_err(); println!("trap received..."); - assert!(error - .downcast_ref::() - .unwrap() - .message() - .contains("wasm trap: interrupt")); + assert!(trap.reason().to_string().contains("wasm trap: interrupt")); Ok(()) } diff --git a/src/commands/run.rs b/src/commands/run.rs index 1e94cc08a899..93fdc53d6130 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -12,7 +12,7 @@ use std::{ }; use structopt::{clap::AppSettings, StructOpt}; use wasi_common::preopen_dir; -use wasmtime::{Engine, Exit, Instance, Module, Store, Trap, Val, ValType}; +use wasmtime::{Engine, Instance, Module, Store, Trap, TrapReason, Val, ValType}; use wasmtime_wasi::{old::snapshot_0::Wasi as WasiSnapshot0, Wasi}; fn parse_module(s: &OsStr) -> Result { @@ -144,23 +144,22 @@ impl RunCommand { Err(e) => { // If the program exited because of a non-zero exit status, print // a message and exit. - if let Some(exit) = e.downcast_ref::() { - eprintln!("Error: {}", exit); - // On Windows, exit status 3 indicates an abort (see below), - // so just return 1 indicating a non-zero status. - if cfg!(windows) { - process::exit(1); - } - process::exit(exit.status().get()); - } - - // 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::() { + if let Some(trap) = e.downcast_ref::() { // Print the error message in the usual way. eprintln!("Error: {:?}", e); + if let TrapReason::Exit(status) = trap.reason() { + // On Windows, exit status 3 indicates an abort (see below), + // so just return 1 indicating a non-zero status. + if cfg!(windows) { + process::exit(1); + } + process::exit(status.get()); + } + + // 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); diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 58a50c720c3a..48da74eb53f2 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -214,9 +214,13 @@ fn exit125_wasi_snapshot1() -> Result<()> { fn exit126_wasi_snapshot0() -> Result<()> { let wasm = build_wasm("tests/wasm/exit126_wasi_snapshot0.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - assert_eq!(output.status.code().unwrap(), 1); + if cfg!(windows) { + assert_eq!(output.status.code().unwrap(), 3); + } else { + assert_eq!(output.status.code().unwrap(), 128 + libc::SIGABRT); + } assert!(output.stdout.is_empty()); - assert!(String::from_utf8_lossy(&output.stderr).contains("invalid WASI exit status")); + assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status")); Ok(()) } @@ -225,8 +229,12 @@ fn exit126_wasi_snapshot0() -> Result<()> { fn exit126_wasi_snapshot1() -> Result<()> { let wasm = build_wasm("tests/wasm/exit126_wasi_snapshot1.wat")?; let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; - assert_eq!(output.status.code().unwrap(), 1); + if cfg!(windows) { + assert_eq!(output.status.code().unwrap(), 3); + } else { + assert_eq!(output.status.code().unwrap(), 128 + libc::SIGABRT); + } assert!(output.stdout.is_empty()); - assert!(String::from_utf8_lossy(&output.stderr).contains("invalid WASI exit status")); + assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status")); Ok(()) } diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 990c2af3781b..70186a0eff56 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -113,10 +113,11 @@ mod tests { .unwrap_err() .downcast::()?; assert!( - trap.message() + trap.reason() + .to_string() .starts_with("wasm trap: out of bounds memory access"), "bad trap message: {:?}", - trap.message() + trap.reason().to_string() ); } @@ -140,7 +141,8 @@ mod tests { .unwrap_err() .downcast::()?; assert!(trap - .message() + .reason() + .to_string() .starts_with("wasm trap: out of bounds memory access")); } Ok(()) diff --git a/tests/all/func.rs b/tests/all/func.rs index ea5584056417..96f838a24e56 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -181,7 +181,7 @@ fn trap_smoke() -> Result<()> { let store = Store::default(); let f = Func::wrap(&store, || -> Result<(), Trap> { Err(Trap::new("test")) }); let err = f.call(&[]).unwrap_err().downcast::()?; - assert_eq!(err.message(), "test"); + assert_eq!(err.reason().to_string(), "test"); Ok(()) } @@ -202,7 +202,7 @@ fn trap_import() -> Result<()> { .err() .unwrap() .downcast::()?; - assert_eq!(trap.message(), "foo"); + assert_eq!(trap.reason().to_string(), "foo"); Ok(()) } @@ -397,7 +397,7 @@ fn func_write_nothing() -> anyhow::Result<()> { let f = Func::new(&store, ty, |_, _, _| Ok(())); let err = f.call(&[]).unwrap_err().downcast::()?; assert_eq!( - err.message(), + err.reason().to_string(), "function attempted to return an incompatible value" ); Ok(()) diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 7affdd8759bc..3ca18df68ac3 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -91,7 +91,7 @@ fn test_returns_incorrect_type() -> Result<()> { .expect_err("the execution should fail") .downcast::()?; assert_eq!( - trap.message(), + trap.reason().to_string(), "function attempted to return an incompatible value" ); Ok(()) diff --git a/tests/all/traps.rs b/tests/all/traps.rs index f4ae458f4804..02fb281d989d 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -25,7 +25,7 @@ fn test_trap_return() -> Result<()> { .expect("error calling function") .downcast::()?; - assert_eq!(e.message(), "test 123"); + assert_eq!(e.reason().to_string(), "test 123"); Ok(()) } @@ -64,9 +64,9 @@ fn test_trap_trace() -> Result<()> { assert_eq!(trace[1].func_offset(), 1); assert_eq!(trace[1].module_offset(), 0x21); assert!( - e.message().contains("unreachable"), + e.reason().to_string().contains("unreachable"), "wrong message: {}", - e.message() + e.reason().to_string() ); Ok(()) @@ -103,7 +103,7 @@ fn test_trap_trace_cb() -> Result<()> { assert_eq!(trace[0].func_index(), 2); assert_eq!(trace[1].module_name().unwrap(), "hello_mod"); assert_eq!(trace[1].func_index(), 1); - assert_eq!(e.message(), "cb throw"); + assert_eq!(e.reason().to_string(), "cb throw"); Ok(()) } @@ -135,7 +135,7 @@ fn test_trap_stack_overflow() -> Result<()> { assert_eq!(trace[i].func_index(), 0); assert_eq!(trace[i].func_name(), Some("run")); } - assert!(e.message().contains("call stack exhausted")); + assert!(e.reason().to_string().contains("call stack exhausted")); Ok(()) } @@ -234,7 +234,10 @@ fn trap_start_function_import() -> Result<()> { let sig = FuncType::new(Box::new([]), Box::new([])); let func = Func::new(&store, sig, |_, _, _| Err(Trap::new("user trap"))); let err = Instance::new(&module, &[func.into()]).err().unwrap(); - assert_eq!(err.downcast_ref::().unwrap().message(), "user trap"); + assert_eq!( + err.downcast_ref::().unwrap().reason().to_string(), + "user trap" + ); Ok(()) } @@ -373,7 +376,10 @@ fn call_signature_mismatch() -> Result<()> { .unwrap() .downcast::() .unwrap(); - assert_eq!(err.message(), "wasm trap: indirect call type mismatch"); + assert_eq!( + err.reason().to_string(), + "wasm trap: indirect call type mismatch" + ); Ok(()) } diff --git a/tests/spec_testsuite b/tests/spec_testsuite index da56298dddb4..c70c3c8b136e 160000 --- a/tests/spec_testsuite +++ b/tests/spec_testsuite @@ -1 +1 @@ -Subproject commit da56298dddb441d1af38492ee98fe001e625d156 +Subproject commit c70c3c8b136e5e7193135d40ec3960f4ef1cb20a From bd5191cd4f363ed34d0c04f31c73ec5a7135c577 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 07:19:34 -0700 Subject: [PATCH 05/11] Move `wasi_proc_exit` into the wasmtime-wasi crate. --- crates/runtime/src/lib.rs | 3 +- crates/runtime/src/traphandlers.rs | 53 -------------------------- crates/wasi-common/wig/src/wasi.rs | 4 +- crates/wasi/src/lib.rs | 16 ++++++++ crates/wasmtime/src/externals.rs | 2 +- crates/wasmtime/src/func.rs | 44 +-------------------- crates/wasmtime/src/instance.rs | 2 +- crates/wasmtime/src/trampoline/func.rs | 2 +- crates/wasmtime/src/trap.rs | 37 +++++++++--------- src/commands/run.rs | 4 +- 10 files changed, 43 insertions(+), 124 deletions(-) diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index a3c110de043e..97983cde2d4f 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -44,8 +44,7 @@ pub use crate::mmap::Mmap; pub use crate::sig_registry::SignatureRegistry; pub use crate::table::Table; pub use crate::traphandlers::{ - catch_traps, init_traps, raise_lib_trap, raise_user_trap, resume_panic, wasi_proc_exit, - SignalHandler, Trap, + catch_traps, init_traps, raise_lib_trap, raise_user_trap, resume_panic, SignalHandler, Trap, }; pub use crate::vmcontext::{ VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport, VMGlobalDefinition, diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs index aaded85b5969..6b2bae92ee64 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -7,7 +7,6 @@ use std::any::Any; use std::cell::Cell; use std::error::Error; use std::io; -use std::num::NonZeroI32; use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use std::sync::Once; @@ -333,18 +332,6 @@ pub enum Trap { /// Native stack backtrace at the time the OOM occurred backtrace: Backtrace, }, - - /// A program exit was requested with a non-zero exit status. - Exit { - /// The exit status - status: NonZeroI32, - }, - - /// A program exit was requested with an invalid exit status. - InvalidExitStatus { - /// Native stack backtrace at the time the error occurred - backtrace: Backtrace, - }, } impl Trap { @@ -366,14 +353,6 @@ impl Trap { let backtrace = Backtrace::new_unresolved(); Trap::OOM { backtrace } } - - /// Construct a new InvalidExitStatus trap with the given source location. - /// - /// Internally saves a backtrace when constructed. - pub fn invalid_exit_status() -> Self { - let backtrace = Backtrace::new_unresolved(); - Trap::InvalidExitStatus { backtrace } - } } /// Catches any wasm traps that happen within the execution of `closure`, @@ -427,7 +406,6 @@ enum UnwindReason { UserTrap(Box), LibTrap(Trap), JitTrap { backtrace: Backtrace, pc: usize }, - Exit { status: i32 }, } impl<'a> CallThreadState<'a> { @@ -475,13 +453,6 @@ impl<'a> CallThreadState<'a> { maybe_interrupted, }) } - UnwindReason::Exit { status } => { - if let Some(status) = NonZeroI32::new(status) { - Err(Trap::Exit { status }) - } else { - Ok(()) - } - } UnwindReason::Panic(panic) => { debug_assert_eq!(ret, 0); std::panic::resume_unwind(panic) @@ -808,27 +779,3 @@ fn setup_unix_sigaltstack() -> Result<(), Trap> { } } } - -/// Perform a program exit by unwinding the stack to the point where wasm -/// as most recently entered, carrying an exit status value. -/// -/// This is implemented in `wasmtime_runtime` rather than with the rest of the WASI -/// functions as it's essentially an unwinding operation. -/// -/// # Safety -/// -/// Only safe to call when wasm code is on the stack, aka `catch_traps` must -/// have been previously called. Additionally no Rust destructors can be on the -/// stack. They will be skipped and not executed. -pub unsafe extern "C" fn wasi_proc_exit( - _vmctx: *mut VMContext, - _caller_vmctx: *mut VMContext, - status: i32, -) -> ! { - // Check that the status is within WASI's range. - if status >= 0 && status < 126 { - tls::with(|info| info.unwrap().unwind_with(UnwindReason::Exit { status })) - } else { - raise_lib_trap(Trap::invalid_exit_status()) - } -} diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs index 51af787193df..39e263c7a67a 100644 --- a/crates/wasi-common/wig/src/wasi.rs +++ b/crates/wasi-common/wig/src/wasi.rs @@ -51,7 +51,7 @@ pub fn define_struct(args: TokenStream) -> TokenStream { // in wasi-common. if name == "proc_exit" { ctor_externs.push(quote! { - let #name_ident = wasmtime::Func::exit_func(store); + let #name_ident = wasmtime::Func::wrap(store, crate::wasi_proc_exit); }); continue; } @@ -305,7 +305,7 @@ pub fn define_struct_for_wiggle(args: TokenStream) -> TokenStream { // in wasi-common. if name == "proc_exit" { ctor_externs.push(quote! { - let #name_ident = wasmtime::Func::exit_func(store); + let #name_ident = wasmtime::Func::wrap(store, crate::wasi_proc_exit); }); continue; } diff --git a/crates/wasi/src/lib.rs b/crates/wasi/src/lib.rs index d8e8be07fde0..e92b1d21d99f 100644 --- a/crates/wasi/src/lib.rs +++ b/crates/wasi/src/lib.rs @@ -1,3 +1,5 @@ +use wasmtime::Trap; + pub mod old; pub use wasi_common::{WasiCtx, WasiCtxBuilder}; @@ -12,3 +14,17 @@ pub fn is_wasi_module(name: &str) -> bool { // trick. name.starts_with("wasi") } + +/// Implement the WASI `proc_exit` function. This function is implemented here +/// instead of in wasi-common so that we can use the runtime to perform an +/// unwind rather than exiting the host process. +fn wasi_proc_exit(status: i32) -> Result<(), Trap> { + // Check that the status is within WASI's range. + if status >= 0 && status < 126 { + Err(Trap::i32_exit(status)) + } else { + Err(Trap::new( + "exit with invalid exit status outside of [0..126)", + )) + } +} diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs index 6536f017fcef..d97ccb22f94e 100644 --- a/crates/wasmtime/src/externals.rs +++ b/crates/wasmtime/src/externals.rs @@ -420,7 +420,7 @@ impl Table { let src_table = src_table.instance.get_defined_table(src_table_index); runtime::Table::copy(dst_table, src_table, dst_index, src_index, len) - .map_err(Trap::from_jit)?; + .map_err(Trap::from_runtime)?; Ok(()) } diff --git a/crates/wasmtime/src/func.rs b/crates/wasmtime/src/func.rs index 8f5093970a07..dda4fcee73ad 100644 --- a/crates/wasmtime/src/func.rs +++ b/crates/wasmtime/src/func.rs @@ -8,7 +8,7 @@ use std::mem; use std::panic::{self, AssertUnwindSafe}; use std::ptr; use std::rc::Weak; -use wasmtime_runtime::{raise_user_trap, wasi_proc_exit, ExportFunction, VMTrampoline}; +use wasmtime_runtime::{raise_user_trap, ExportFunction, VMTrampoline}; use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody}; /// A WebAssembly function which can be called. @@ -474,46 +474,6 @@ impl Func { func.into_func(store) } - /// Creates a new `Func` for a function which performs a program exit, - /// unwinding the stack up the point where wasm was most recently entered. - pub fn exit_func(store: &Store) -> Func { - unsafe extern "C" fn trampoline( - callee_vmctx: *mut VMContext, - caller_vmctx: *mut VMContext, - ptr: *const VMFunctionBody, - args: *mut u128, - ) { - let ptr = mem::transmute::< - *const VMFunctionBody, - unsafe extern "C" fn(*mut VMContext, *mut VMContext, i32) -> !, - >(ptr); - - let mut next = args as *const u128; - let status = i32::load(&mut next); - ptr(callee_vmctx, caller_vmctx, status) - } - - let mut args = Vec::new(); - ::push(&mut args); - let ret = Vec::new(); - let ty = FuncType::new(args.into(), ret.into()); - let (instance, export) = unsafe { - crate::trampoline::generate_raw_func_export( - &ty, - std::slice::from_raw_parts_mut(wasi_proc_exit as *mut _, 0), - trampoline, - store, - Box::new(()), - ) - .expect("failed to generate export") - }; - Func { - instance, - export, - trampoline, - } - } - /// Returns the underlying wasm type that this `Func` has. pub fn ty(&self) -> FuncType { // Signatures should always be registered in the store's registry of @@ -787,7 +747,7 @@ pub(crate) fn catch_traps( signalhandler.as_deref(), closure, ) - .map_err(Trap::from_jit) + .map_err(Trap::from_runtime) } } diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 808f5d39790a..a048eecaf5db 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -52,7 +52,7 @@ fn instantiate( .map_err(|e| -> Error { match e { InstantiationError::StartTrap(trap) | InstantiationError::Trap(trap) => { - Trap::from_jit(trap).into() + Trap::from_runtime(trap).into() } other => other.into(), } diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index c54f5f0fcb7f..41d6ac61b378 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -54,7 +54,7 @@ unsafe extern "C" fn stub_fn( // If a trap was raised (an error returned from the imported function) // then we smuggle the trap through `Box` through to the - // call-site, which gets unwrapped in `Trap::from_jit` later on as we + // call-site, which gets unwrapped in `Trap::from_runtime` later on as we // convert from the internal `Trap` type to our own `Trap` type in this // crate. Ok(Err(trap)) => wasmtime_runtime::raise_user_trap(Box::new(trap)), diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 05467ab8e064..d97aaa883434 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -2,7 +2,6 @@ use crate::frame_info::{GlobalFrameInfo, FRAME_INFO}; use crate::FrameInfo; use backtrace::Backtrace; use std::fmt; -use std::num::NonZeroI32; use std::sync::Arc; use wasmtime_environ::ir::TrapCode; @@ -19,15 +18,15 @@ pub enum TrapReason { /// An error message describing a trap. Message(String), - /// A non-zero exit status describing an explicit program exit. - Exit(NonZeroI32), + /// An `i32` exit status describing an explicit program exit. + I32Exit(i32), } impl fmt::Display for TrapReason { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { TrapReason::Message(s) => write!(f, "{}", s), - TrapReason::Exit(status) => write!(f, "Exited with non-zero exit status {}", status), + TrapReason::I32Exit(status) => write!(f, "Exited with i32 exit status {}", status), } } } @@ -54,9 +53,21 @@ impl Trap { Trap::new_with_trace(&info, None, message.into(), Backtrace::new_unresolved()) } - pub(crate) fn from_jit(jit: wasmtime_runtime::Trap) -> Self { + /// Creates a new `Trap` representing an explicit program exit with a classic `i32` + /// exit status value. + pub fn i32_exit(status: i32) -> Self { + Trap { + inner: Arc::new(TrapInner { + reason: TrapReason::I32Exit(status), + wasm_trace: Vec::new(), + native_trace: Backtrace::from(Vec::new()), + }), + } + } + + pub(crate) fn from_runtime(runtime_trap: wasmtime_runtime::Trap) -> Self { let info = FRAME_INFO.read().unwrap(); - match jit { + match runtime_trap { wasmtime_runtime::Trap::User(error) => { // Since we're the only one using the wasmtime internals (in // theory) we should only see user errors which were originally @@ -91,10 +102,6 @@ impl Trap { wasmtime_runtime::Trap::OOM { backtrace } => { Trap::new_with_trace(&info, None, "out of memory".to_string(), backtrace) } - wasmtime_runtime::Trap::Exit { status } => Trap::new_exit(status), - wasmtime_runtime::Trap::InvalidExitStatus { backtrace } => { - Trap::new_with_trace(&info, None, "invalid exit status".to_string(), backtrace) - } } } @@ -158,16 +165,6 @@ impl Trap { } } - fn new_exit(status: NonZeroI32) -> Self { - Trap { - inner: Arc::new(TrapInner { - reason: TrapReason::Exit(status), - wasm_trace: Vec::new(), - native_trace: Backtrace::from(Vec::new()), - }), - } - } - /// Returns a reference the `message` stored in `Trap`. pub fn reason(&self) -> &TrapReason { &self.inner.reason diff --git a/src/commands/run.rs b/src/commands/run.rs index 93fdc53d6130..8f5c199d4791 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -148,13 +148,13 @@ impl RunCommand { // Print the error message in the usual way. eprintln!("Error: {:?}", e); - if let TrapReason::Exit(status) = trap.reason() { + if let TrapReason::I32Exit(status) = trap.reason() { // On Windows, exit status 3 indicates an abort (see below), // so just return 1 indicating a non-zero status. if cfg!(windows) { process::exit(1); } - process::exit(status.get()); + process::exit(*status); } // If the program exited because of a trap, return an error code From 994f4b28c95956f2d8d78ce4269140a19de03f62 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 12:04:24 -0700 Subject: [PATCH 06/11] Revert the spec_testsuite change. --- tests/spec_testsuite | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/spec_testsuite b/tests/spec_testsuite index c70c3c8b136e..da56298dddb4 160000 --- a/tests/spec_testsuite +++ b/tests/spec_testsuite @@ -1 +1 @@ -Subproject commit c70c3c8b136e5e7193135d40ec3960f4ef1cb20a +Subproject commit da56298dddb441d1af38492ee98fe001e625d156 From 27d03274e3b40192abdfb9eb08e7c0b89b178b37 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 12:10:40 -0700 Subject: [PATCH 07/11] Remove the old proc_exit implementations. --- .../src/old/snapshot_0/hostcalls_impl/misc.rs | 11 ----------- .../src/snapshots/wasi_snapshot_preview1.rs | 14 ++++---------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs index 6b75a24414d5..d6f4d2468484 100644 --- a/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs +++ b/crates/wasi-common/src/old/snapshot_0/hostcalls_impl/misc.rs @@ -342,17 +342,6 @@ pub(crate) struct FdEventData<'a> { pub(crate) userdata: wasi::__wasi_userdata_t, } -// Not all implementations use this implementation. -#[allow(dead_code)] -pub(crate) fn proc_exit(_wasi_ctx: &WasiCtx, _memory: &mut [u8], rval: wasi::__wasi_exitcode_t) { - trace!("proc_exit(rval={:?})", rval); - - // Note that this is just a default implementation. Host environments - // will often override this `proc_exit` implementation and do other - // things other than exit the host process. - std::process::exit(rval as i32); -} - pub(crate) fn proc_raise( _wasi_ctx: &WasiCtx, _memory: &mut [u8], diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs index 63c05f4c6d98..31117243a629 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -813,16 +813,10 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx { Ok(nevents) } - // Not all implementations use this implementation. - #[allow(dead_code)] - // This is just a temporary to ignore the warning which becomes a hard error - // in the CI. Once we figure out non-returns in `wiggle`, this should be gone. - #[allow(unreachable_code)] - fn proc_exit(&self, rval: types::Exitcode) -> std::result::Result<(), ()> { - // Note that this is just a default implementation. Host environments - // will often override this `proc_exit` implementation and do other - // things other than exit the host process. - std::process::exit(rval as i32); + fn proc_exit(&self, _rval: types::Exitcode) -> std::result::Result<(), ()> { + // proc_exit is special in that it's expected to unwind the stack, which + // typically requires runtime-specific logic. + unimplemented!("runtimes are expected to override this implementation") } fn proc_raise(&self, _sig: types::Signal) -> Result<()> { From 5c0cc178e364131f40232ad63bcb75245b2d827e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 12:25:53 -0700 Subject: [PATCH 08/11] Make `TrapReason` an implementation detail. --- crates/c-api/src/trap.rs | 2 +- crates/wasmtime/src/lib.rs | 2 +- crates/wasmtime/src/runtime.rs | 2 +- crates/wasmtime/src/trap.rs | 23 +++++++++++++++++++---- crates/wast/src/wast.rs | 4 ++-- examples/interrupt.rs | 2 +- src/commands/run.rs | 6 +++--- tests/all/custom_signal_handler.rs | 8 +++----- tests/all/func.rs | 7 ++++--- tests/all/import_calling_export.rs | 2 +- tests/all/traps.rs | 17 +++++++---------- 11 files changed, 43 insertions(+), 32 deletions(-) diff --git a/crates/c-api/src/trap.rs b/crates/c-api/src/trap.rs index 83289a348f71..67ca334d7bb3 100644 --- a/crates/c-api/src/trap.rs +++ b/crates/c-api/src/trap.rs @@ -53,7 +53,7 @@ pub extern "C" fn wasm_trap_new( #[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.borrow().reason().to_string().as_bytes()); + buffer.extend_from_slice(trap.trap.borrow().message().as_bytes()); buffer.reserve_exact(1); buffer.push(0); out.set_buffer(buffer); diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index da450c58bde0..034283326935 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -31,7 +31,7 @@ pub use crate::linker::*; pub use crate::module::Module; pub use crate::r#ref::{AnyRef, HostRef}; pub use crate::runtime::*; -pub use crate::trap::{Trap, TrapReason}; +pub use crate::trap::Trap; pub use crate::types::*; pub use crate::values::*; diff --git a/crates/wasmtime/src/runtime.rs b/crates/wasmtime/src/runtime.rs index 0c85628b79fc..f1a0b7892a15 100644 --- a/crates/wasmtime/src/runtime.rs +++ b/crates/wasmtime/src/runtime.rs @@ -905,7 +905,7 @@ impl Store { /// }); /// /// let trap = run().unwrap_err(); - /// assert!(trap.reason().to_string().contains("wasm trap: interrupt")); + /// assert!(trap.message().contains("wasm trap: interrupt")); /// # Ok(()) /// # } /// ``` diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index d97aaa883434..9d055add31bc 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -14,7 +14,7 @@ pub struct Trap { /// State describing the occasion which evoked a trap. #[derive(Debug)] -pub enum TrapReason { +enum TrapReason { /// An error message describing a trap. Message(String), @@ -46,7 +46,7 @@ impl Trap { /// # Example /// ``` /// let trap = wasmtime::Trap::new("unexpected error"); - /// assert_eq!("unexpected error", trap.reason().to_string()); + /// assert_eq!("unexpected error", trap.message()); /// ``` pub fn new>(message: I) -> Self { let info = FRAME_INFO.read().unwrap(); @@ -166,8 +166,23 @@ impl Trap { } /// Returns a reference the `message` stored in `Trap`. - pub fn reason(&self) -> &TrapReason { - &self.inner.reason + /// + /// In the case of an explicit exit, the exit status can be obtained by + /// calling [`exit_code`](Self::exit_code). + pub fn message(&self) -> &str { + match &self.inner.reason { + TrapReason::Message(message) => message, + TrapReason::I32Exit(_) => "explicitly exited", + } + } + + /// 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, + } } /// Returns a list of function frames in WebAssembly code that led to this diff --git a/crates/wast/src/wast.rs b/crates/wast/src/wast.rs index 14717fbaf7b7..5c3c2253111f 100644 --- a/crates/wast/src/wast.rs +++ b/crates/wast/src/wast.rs @@ -123,7 +123,7 @@ impl WastContext { fn module(&mut self, instance_name: Option<&str>, module: &[u8]) -> Result<()> { let instance = match self.instantiate(module)? { Outcome::Ok(i) => i, - Outcome::Trap(e) => bail!("instantiation failed: {}", e.reason()), + Outcome::Trap(e) => bail!("instantiation failed: {}", e.message()), }; if let Some(name) = instance_name { self.linker.instance(name, &instance)?; @@ -189,7 +189,7 @@ impl WastContext { Outcome::Ok(values) => bail!("expected trap, got {:?}", values), Outcome::Trap(t) => t, }; - let actual = trap.reason().to_string(); + let actual = trap.message(); 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/examples/interrupt.rs b/examples/interrupt.rs index 38031ac6ce60..696d8a2d952d 100644 --- a/examples/interrupt.rs +++ b/examples/interrupt.rs @@ -32,7 +32,7 @@ fn main() -> Result<()> { let trap = run().unwrap_err(); println!("trap received..."); - assert!(trap.reason().to_string().contains("wasm trap: interrupt")); + assert!(trap.message().contains("wasm trap: interrupt")); Ok(()) } diff --git a/src/commands/run.rs b/src/commands/run.rs index 8f5c199d4791..ca5e46051b6d 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -12,7 +12,7 @@ use std::{ }; use structopt::{clap::AppSettings, StructOpt}; use wasi_common::preopen_dir; -use wasmtime::{Engine, Instance, Module, Store, Trap, TrapReason, Val, ValType}; +use wasmtime::{Engine, Instance, Module, Store, Trap, Val, ValType}; use wasmtime_wasi::{old::snapshot_0::Wasi as WasiSnapshot0, Wasi}; fn parse_module(s: &OsStr) -> Result { @@ -148,13 +148,13 @@ impl RunCommand { // Print the error message in the usual way. eprintln!("Error: {:?}", e); - if let TrapReason::I32Exit(status) = trap.reason() { + if let Some(status) = trap.i32_exit_status() { // On Windows, exit status 3 indicates an abort (see below), // so just return 1 indicating a non-zero status. if cfg!(windows) { process::exit(1); } - process::exit(*status); + process::exit(status); } // If the program exited because of a trap, return an error code diff --git a/tests/all/custom_signal_handler.rs b/tests/all/custom_signal_handler.rs index 70186a0eff56..990c2af3781b 100644 --- a/tests/all/custom_signal_handler.rs +++ b/tests/all/custom_signal_handler.rs @@ -113,11 +113,10 @@ mod tests { .unwrap_err() .downcast::()?; assert!( - trap.reason() - .to_string() + trap.message() .starts_with("wasm trap: out of bounds memory access"), "bad trap message: {:?}", - trap.reason().to_string() + trap.message() ); } @@ -141,8 +140,7 @@ mod tests { .unwrap_err() .downcast::()?; assert!(trap - .reason() - .to_string() + .message() .starts_with("wasm trap: out of bounds memory access")); } Ok(()) diff --git a/tests/all/func.rs b/tests/all/func.rs index 96f838a24e56..16da165bab56 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -181,7 +181,8 @@ fn trap_smoke() -> Result<()> { let store = Store::default(); let f = Func::wrap(&store, || -> Result<(), Trap> { Err(Trap::new("test")) }); let err = f.call(&[]).unwrap_err().downcast::()?; - assert_eq!(err.reason().to_string(), "test"); + assert_eq!(err.message(), "test"); + assert!(err.i32_exit_status().is_none()); Ok(()) } @@ -202,7 +203,7 @@ fn trap_import() -> Result<()> { .err() .unwrap() .downcast::()?; - assert_eq!(trap.reason().to_string(), "foo"); + assert_eq!(trap.message(), "foo"); Ok(()) } @@ -397,7 +398,7 @@ fn func_write_nothing() -> anyhow::Result<()> { let f = Func::new(&store, ty, |_, _, _| Ok(())); let err = f.call(&[]).unwrap_err().downcast::()?; assert_eq!( - err.reason().to_string(), + err.message(), "function attempted to return an incompatible value" ); Ok(()) diff --git a/tests/all/import_calling_export.rs b/tests/all/import_calling_export.rs index 3ca18df68ac3..7affdd8759bc 100644 --- a/tests/all/import_calling_export.rs +++ b/tests/all/import_calling_export.rs @@ -91,7 +91,7 @@ fn test_returns_incorrect_type() -> Result<()> { .expect_err("the execution should fail") .downcast::()?; assert_eq!( - trap.reason().to_string(), + trap.message(), "function attempted to return an incompatible value" ); Ok(()) diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 02fb281d989d..3e13dafe92b1 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -25,7 +25,7 @@ fn test_trap_return() -> Result<()> { .expect("error calling function") .downcast::()?; - assert_eq!(e.reason().to_string(), "test 123"); + assert_eq!(e.message(), "test 123"); Ok(()) } @@ -64,9 +64,9 @@ fn test_trap_trace() -> Result<()> { assert_eq!(trace[1].func_offset(), 1); assert_eq!(trace[1].module_offset(), 0x21); assert!( - e.reason().to_string().contains("unreachable"), + e.message().contains("unreachable"), "wrong message: {}", - e.reason().to_string() + e.message() ); Ok(()) @@ -103,7 +103,7 @@ fn test_trap_trace_cb() -> Result<()> { assert_eq!(trace[0].func_index(), 2); assert_eq!(trace[1].module_name().unwrap(), "hello_mod"); assert_eq!(trace[1].func_index(), 1); - assert_eq!(e.reason().to_string(), "cb throw"); + assert_eq!(e.message(), "cb throw"); Ok(()) } @@ -135,7 +135,7 @@ fn test_trap_stack_overflow() -> Result<()> { assert_eq!(trace[i].func_index(), 0); assert_eq!(trace[i].func_name(), Some("run")); } - assert!(e.reason().to_string().contains("call stack exhausted")); + assert!(e.message().contains("call stack exhausted")); Ok(()) } @@ -235,7 +235,7 @@ fn trap_start_function_import() -> Result<()> { let func = Func::new(&store, sig, |_, _, _| Err(Trap::new("user trap"))); let err = Instance::new(&module, &[func.into()]).err().unwrap(); assert_eq!( - err.downcast_ref::().unwrap().reason().to_string(), + err.downcast_ref::().message().to_string(), "user trap" ); Ok(()) @@ -376,10 +376,7 @@ fn call_signature_mismatch() -> Result<()> { .unwrap() .downcast::() .unwrap(); - assert_eq!( - err.reason().to_string(), - "wasm trap: indirect call type mismatch" - ); + assert_eq!(err.message(), "wasm trap: indirect call type mismatch"); Ok(()) } From e6dc85f822d296a92028eb097fbff45f6b1d9f1f Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 12:37:14 -0700 Subject: [PATCH 09/11] Allow exit status 2 on Windows too. --- src/commands/run.rs | 4 ++-- tests/all/cli_tests.rs | 18 ++++++++++++++++++ tests/all/traps.rs | 5 +---- tests/wasm/exit2_wasi_snapshot0.wat | 8 ++++++++ tests/wasm/exit2_wasi_snapshot1.wat | 8 ++++++++ 5 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 tests/wasm/exit2_wasi_snapshot0.wat create mode 100644 tests/wasm/exit2_wasi_snapshot1.wat diff --git a/src/commands/run.rs b/src/commands/run.rs index ca5e46051b6d..d253402097f5 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -150,8 +150,8 @@ impl RunCommand { if let Some(status) = trap.i32_exit_status() { // On Windows, exit status 3 indicates an abort (see below), - // so just return 1 indicating a non-zero status. - if cfg!(windows) { + // so return 1 indicating a non-zero status to avoid ambiguity. + if cfg!(windows) && status >= 3 { process::exit(1); } process::exit(status); diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index 48da74eb53f2..c8edbd1fb6e5 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -183,6 +183,24 @@ fn timeout_in_invoke() -> Result<()> { Ok(()) } +// Exit with a valid non-zero exit code, snapshot0 edition. +#[test] +fn exit2_wasi_snapshot0() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit2_wasi_snapshot0.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 2); + Ok(()) +} + +// Exit with a valid non-zero exit code, snapshot1 edition. +#[test] +fn exit2_wasi_snapshot1() -> Result<()> { + let wasm = build_wasm("tests/wasm/exit2_wasi_snapshot1.wat")?; + let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?; + assert_eq!(output.status.code().unwrap(), 2); + Ok(()) +} + // Exit with a valid non-zero exit code, snapshot0 edition. #[test] fn exit125_wasi_snapshot0() -> Result<()> { diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 3e13dafe92b1..f4ae458f4804 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -234,10 +234,7 @@ fn trap_start_function_import() -> Result<()> { let sig = FuncType::new(Box::new([]), Box::new([])); let func = Func::new(&store, sig, |_, _, _| Err(Trap::new("user trap"))); let err = Instance::new(&module, &[func.into()]).err().unwrap(); - assert_eq!( - err.downcast_ref::().message().to_string(), - "user trap" - ); + assert_eq!(err.downcast_ref::().unwrap().message(), "user trap"); Ok(()) } diff --git a/tests/wasm/exit2_wasi_snapshot0.wat b/tests/wasm/exit2_wasi_snapshot0.wat new file mode 100644 index 000000000000..244d7b604109 --- /dev/null +++ b/tests/wasm/exit2_wasi_snapshot0.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_unstable" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 2)) + ) + (export "_start" (func $_start)) +) diff --git a/tests/wasm/exit2_wasi_snapshot1.wat b/tests/wasm/exit2_wasi_snapshot1.wat new file mode 100644 index 000000000000..75f64407aaae --- /dev/null +++ b/tests/wasm/exit2_wasi_snapshot1.wat @@ -0,0 +1,8 @@ +(module + (import "wasi_snapshot_preview1" "proc_exit" + (func $__wasi_proc_exit (param i32))) + (func $_start + (call $__wasi_proc_exit (i32.const 2)) + ) + (export "_start" (func $_start)) +) From 38a479c10909b70aac0fbf31f03095a08dbd0a98 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 12 May 2020 14:25:43 -0700 Subject: [PATCH 10/11] Fix a documentation link. --- crates/wasmtime/src/trap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index 9d055add31bc..f4ca1af14981 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -168,7 +168,7 @@ impl Trap { /// Returns a reference the `message` stored in `Trap`. /// /// In the case of an explicit exit, the exit status can be obtained by - /// calling [`exit_code`](Self::exit_code). + /// calling [`i32_exit_status`](Self::i32_exit_status). pub fn message(&self) -> &str { match &self.inner.reason { TrapReason::Message(message) => message, From 59e816da77e80927ed3ee2d71eb1e156a653712e Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 13 May 2020 13:06:16 -0700 Subject: [PATCH 11/11] Really fix a documentation link. --- crates/wasmtime/src/trap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs index f4ca1af14981..4c160b4cfe0a 100644 --- a/crates/wasmtime/src/trap.rs +++ b/crates/wasmtime/src/trap.rs @@ -168,7 +168,7 @@ impl Trap { /// Returns a reference the `message` stored in `Trap`. /// /// In the case of an explicit exit, the exit status can be obtained by - /// calling [`i32_exit_status`](Self::i32_exit_status). + /// calling `i32_exit_status`. pub fn message(&self) -> &str { match &self.inner.reason { TrapReason::Message(message) => message,