Change proc_exit to unwind the stack rather than exiting the host p…

…rocess. (#1646)

* Remove Cranelift's OutOfBounds trap, which is no longer used.

* Change proc_exit to unwind instead of exit the host process.

This implements the semantics in WebAssembly/WASI#235.

Fixes #783.
Fixes #993.

* Fix exit-status tests on Windows.

* Revert the wiggle changes and re-introduce the wasi-common implementations.

* Move `wasi_proc_exit` into the wasmtime-wasi crate.

* Revert the spec_testsuite change.

* Remove the old proc_exit implementations.

* Make `TrapReason` an implementation detail.

* Allow exit status 2 on Windows too.

* Fix a documentation link.

* Really fix a documentation link.

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,

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

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

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

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

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],

crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs

@@ -813,13 +813,10 @@ 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_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<()> {

crates/wasi-common/wig/src/hostcalls.rs

@@ -17,6 +17,13 @@ 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 use the implementation
+            // in wasi-common.
+            if func.name.as_str() == "proc_exit" {
+                continue;
+            }
+
             ret.extend(generate_wrappers(&func, old));
         }
     }

crates/wasi-common/wig/src/wasi.rs

@@ -46,6 +46,15 @@ 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 use the implementation
+            // in wasi-common.
+            if name == "proc_exit" {
+                ctor_externs.push(quote! {
+                    let #name_ident = wasmtime::Func::wrap(store, crate::wasi_proc_exit);
+                });
+                continue;
+            }
 
             let mut shim_arg_decls = Vec::new();
             let mut params = Vec::new();
@@ -291,6 +300,15 @@ 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 use the implementation
+            // in wasi-common.
+            if name == "proc_exit" {
+                ctor_externs.push(quote! {
+                    let #name_ident = wasmtime::Func::wrap(store, crate::wasi_proc_exit);
+                });
+                continue;
+            }
 
             let mut shim_arg_decls = Vec::new();
             let mut params = Vec::new();

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)",
+        ))
+    }
+}

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(())
     }
 

crates/wasmtime/src/func.rs

@@ -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, ExportFunction, VMTrampoline};
 use wasmtime_runtime::{Export, InstanceHandle, VMContext, VMFunctionBody};
-use wasmtime_runtime::{ExportFunction, VMTrampoline};
 
 /// A WebAssembly function which can be called.
 ///
@@ -747,7 +747,7 @@ pub(crate) fn catch_traps(
             signalhandler.as_deref(),
             closure,
         )
-        .map_err(Trap::from_jit)
+        .map_err(Trap::from_runtime)
     }
 }
 
@@ -941,7 +941,7 @@ unsafe impl<T: WasmTy> WasmRet for Result<T, Trap> {
         }
 
         fn handle_trap(trap: Trap) -> ! {
-            unsafe { wasmtime_runtime::raise_user_trap(Box::new(trap)) }
+            unsafe { raise_user_trap(trap.into()) }
         }
     }
 
@@ -1133,9 +1133,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::<F, $($args,)* R> as *mut _,
@@ -1145,12 +1145,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,
                 }
             }
         }

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(),
                 }

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<dyn Error>` 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)),

crates/wasmtime/src/trap.rs

@@ -12,8 +12,27 @@ pub struct Trap {
     inner: Arc<TrapInner>,
 }
 
+/// State describing the occasion which evoked a trap.
+#[derive(Debug)]
+enum TrapReason {
+    /// An error message describing a trap.
+    Message(String),
+
+    /// 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::I32Exit(status) => write!(f, "Exited with i32 exit status {}", status),
+        }
+    }
+}
+
 struct TrapInner {
-    message: String,
+    reason: TrapReason,
     wasm_trace: Vec<FrameInfo>,
     native_trace: Backtrace,
 }
@@ -34,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
@@ -85,7 +116,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",
@@ -128,16 +158,31 @@ impl Trap {
         }
         Trap {
             inner: Arc::new(TrapInner {
-                message,
+                reason: TrapReason::Message(message),
                 wasm_trace,
                 native_trace,
             }),
         }
     }
 
     /// 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`.
     pub fn message(&self) -> &str {
-        &self.inner.message
+        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<i32> {
+        match self.inner.reason {
+            TrapReason::I32Exit(status) => Some(status),
+            _ => None,
+        }
     }
 
     /// Returns a list of function frames in WebAssembly code that led to this
@@ -150,7 +195,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()
@@ -159,7 +204,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(());