From 9fe5fe9ea453eac231e6ece6b98d12edbf171ba2 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 1 May 2020 05:06:00 -0700 Subject: [PATCH] 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. --- crates/api/src/error.rs | 54 ++++++++++++ crates/api/src/exit.rs | 53 ++++++++++++ crates/api/src/externals.rs | 4 +- crates/api/src/func.rs | 72 ++++++++++++---- crates/api/src/instance.rs | 4 +- crates/api/src/lib.rs | 3 + crates/api/src/trampoline/func.rs | 2 +- crates/api/src/trap.rs | 44 +--------- crates/runtime/src/lib.rs | 3 +- crates/runtime/src/traphandlers.rs | 53 ++++++++++++ .../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/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 ++ 26 files changed, 400 insertions(+), 135 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/crates/api/src/error.rs b/crates/api/src/error.rs new file mode 100644 index 000000000000..4848b2e2bb54 --- /dev/null +++ b/crates/api/src/error.rs @@ -0,0 +1,54 @@ +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) => { + // 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` and `Exit` types (see the trampoline + // module with functions). + let e = match error.downcast::() { + Ok(exit) => Error::new(exit), + Err(e) => match e.downcast::() { + Ok(invalid) => Error::new(invalid), + Err(e) => Error::new(dbg!(e + .downcast::() + .expect("only `Trap` and `Exit` user errors are supported"))), + }, + }; + dbg!(&e); + dbg!(e.is::()); + e + } + 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..c370113deb29 --- /dev/null +++ b/crates/api/src/exit.rs @@ -0,0 +1,53 @@ +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(NonZeroI32::new(1).unwrap()); + /// assert_eq!(1, trap.status()); + /// ``` + 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 traditionally defined in , though somewhat generalized. + match self.status.get() { + 64 => write!(f, "command line usage error"), + 65 => write!(f, "input data format error"), + 66 => write!(f, "inadequate input"), + 67 => write!(f, "named identity not recognized"), + 68 => write!(f, "named resource could not be located"), + 69 => write!(f, "service unavailable"), + 70 => write!(f, "internal software error"), + 71 => write!(f, "system error"), + 72 => write!(f, "unexpected host environment configuration"), + 73 => write!(f, "inadequate output"), + 74 => write!(f, "input/output error"), + 75 => write!(f, "temporary failure; user is invited to retry"), + 76 => write!(f, "interactive protocol error"), + 77 => write!(f, "permission denied"), + 78 => write!(f, "application configuration error"), + _ => write!(f, "non-zero status {}", self.status), + } + } +} + +impl std::error::Error for Exit {} diff --git a/crates/api/src/externals.rs b/crates/api/src/externals.rs index 987219d9cdf8..ce140251805e 100644 --- a/crates/api/src/externals.rs +++ b/crates/api/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; @@ -419,7 +419,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/api/src/func.rs b/crates/api/src/func.rs index b1b18be7c175..ab21fd3255e9 100644 --- a/crates/api/src/func.rs +++ b/crates/api/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(Box::new(trap)) } } } @@ -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/api/src/instance.rs b/crates/api/src/instance.rs index 808f5d39790a..87c2700e839c 100644 --- a/crates/api/src/instance.rs +++ b/crates/api/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/api/src/lib.rs b/crates/api/src/lib.rs index 034283326935..f2463efba653 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/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/api/src/trampoline/func.rs b/crates/api/src/trampoline/func.rs index c54f5f0fcb7f..771694fb2f4c 100644 --- a/crates/api/src/trampoline/func.rs +++ b/crates/api/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 `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)), diff --git a/crates/api/src/trap.rs b/crates/api/src/trap.rs index c5ea37f96120..26079873bd39 100644 --- a/crates/api/src/trap.rs +++ b/crates/api/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/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 39513559638b..d23d932a09fa 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, raise_lib_trap, raise_user_trap, resume_panic, SignalHandler, Trap, + catch_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 d21568b90c7f..cbcf5495e7fb 100644 --- a/crates/runtime/src/traphandlers.rs +++ b/crates/runtime/src/traphandlers.rs @@ -6,7 +6,9 @@ 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; @@ -336,6 +338,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 { @@ -410,6 +418,7 @@ enum UnwindReason { UserTrap(Box), LibTrap(Trap), JitTrap { backtrace: Backtrace, pc: usize }, + Exit { status: i32 }, } impl<'a> CallThreadState<'a> { @@ -457,6 +466,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) @@ -783,3 +799,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(Box::new(InvalidWASIExitStatus {})) + } +} + +/// 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 23649b20b495..90fd3d74ba7c 100644 --- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs +++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs @@ -812,15 +812,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/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..afb204d94e49 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 573e102fec81..b56387f11e84 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -183,3 +183,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)) +)