Enable limited stack overflow checks while running inside continuatio…

…ns (#136)

Currently, we can overflow the stack while running inside a
continuation, without the runtime having any way of detecting this.
This PR partially rectifies this, by making the existing stack limit
checks that get emitted by cranelift in every wasm function prelude work
correctly while running inside a continuation.

All that was required to enable the stack limit checks was the
following:
1. Stop zero-ing out the `stack_limit` value in `VMRuntimeLimits`
whenever we `resume` a continuation.
2. When creating a continuation, set a reasonable value for the
`stack_limits` value in its `StackLimits` object.

Note that all the required infrastructure to make sure that whenever we
switch stacks, we save and restore the `stack_limits` value inside
`VMRuntimeLimits` and the `StackLimits` object of the involved stacks
was already implemented in #98 and #99. In this sense, enabling these
checks is "free": The limits were already checked, but previously using
a limit of 0.

The only remaining question is what the "reasonable value" for the stack
limits value mentioned above is. As discussed in #122, the stack limit
checks that cranelift emits in function preludes are rather limited, and
these limitations are reflected in the checks that this PR provides:
When entering a wasm function, they check that the current stack pointer
is larger than the `stack_limit` value in `VMRuntimeLimits`. They do not
take into account how much stack space the function itself will occupy.
No stack limit checks are performed when calling a host function.

Thus, this PR defines a config option `wasmfx_red_zone_size`. The idea
is that we define the stack limit as `bottom_of_fiber_stack` +
`wasmfx_red_zone_size`. Thus, the stack checks boil down to the
following:
Whenever we enter a wasm function while inside a continuation, we ensure
that there are at least `wasmfx_red_zone_size` bytes of stack space
left.

I've set the default value for `wasmfx_red_zone_size` to 32k. To get a
rough idea for a sensible value, I determined that a call to the
`fd_write` WASI function occupies ~21k of stack space, and generously
rounded this up to 32k.

**Important**: This means that these stack limit checks are incomplete:
Calling a wasm or host function that occupies more than
`wasmfx_red_zone_size` of stack space may still result in an undetected
stack overflow!

crates/cli-flags/src/lib.rs

@@ -211,6 +211,10 @@ wasmtime_option_group! {
         pub timeout: Option<Duration>,
         /// Size of stacks created with cont.new instructions
         pub wasmfx_stack_size: Option<usize>,
+        /// Space that must be left on stack when starting execution of a
+        /// function while running on a continuation stack.
+        /// Must be smaller than the `wasmfx_stack_size` option above.
+        pub wasmfx_red_zone_size: Option<usize>,
         /// Configures support for all WebAssembly proposals implemented.
         pub all_proposals: Option<bool>,
         /// Configure support for the bulk memory proposal.
@@ -552,6 +556,9 @@ impl CommonOptions {
         if let Some(wasmfx_stack_size) = self.wasm.wasmfx_stack_size {
             config.wasmfx_stack_size(wasmfx_stack_size);
         }
+        if let Some(wasmfx_red_zone_size) = self.wasm.wasmfx_red_zone_size {
+            config.wasmfx_red_zone_size(wasmfx_red_zone_size);
+        }
 
         match_feature! {
             ["pooling-allocator" : self.opts.pooling_allocator]

crates/continuations/src/lib.rs

@@ -3,6 +3,12 @@ use std::ptr;
 /// Default size for continuation stacks
 pub const DEFAULT_FIBER_SIZE: usize = 2097152; // 2MB = 512 pages of 4k
 
+/// Default size of the red zone at the bottom of a fiber stack. This means that
+/// whenever we are executing on a fiber stack and starting (!) execution of a
+/// wasm (!) function, the stack pointer must be at least this many bytes away
+/// from the bottom of the fiber stack.
+pub const DEFAULT_RED_ZONE_SIZE: usize = 32768; // 32K = 8 pages of 4k size
+
 /// TODO
 #[allow(dead_code)]
 pub const ENABLE_DEBUG_PRINTING: bool = false;
@@ -46,6 +52,11 @@ pub mod types {
 #[derive(Clone)]
 pub struct WasmFXConfig {
     pub stack_size: usize,
+
+    /// Space that must be left on stack when starting execution of a
+    /// function while running on a continuation stack.
+    /// Must be smaller than the value of `stack_size` above.
+    pub red_zone_size: usize,
 }
 
 /// This type is used to save (and subsequently restore) a subset of the data in

crates/runtime/src/continuation.rs

@@ -314,9 +314,14 @@ pub fn cont_new(
     let capacity = cmp::max(param_count, result_count);
     let payload = Payloads::new(capacity);
 
+    let wasmfx_config = unsafe { &*(*instance.store()).wasmfx_config() };
+    // TODO(frank-emrich) Currently, the general `stack_limit` configuration
+    // option of wasmtime is unrelated to the stack size of our fiber stack.
+    let stack_size = wasmfx_config.stack_size;
+    let red_zone_size = wasmfx_config.red_zone_size;
+
     let fiber = {
-        let wasmfx_config = unsafe { &*(*instance.store()).wasmfx_config() };
-        let stack = FiberStack::malloc(wasmfx_config.stack_size)
+        let stack = FiberStack::malloc(stack_size)
             .map_err(|error| TrapReason::user_without_backtrace(error.into()))?;
         let args_ptr = payload.data;
         let fiber = Fiber::new(stack, move |_first_val: (), _suspend: &Yield| unsafe {
@@ -332,8 +337,9 @@ pub fn cont_new(
     };
 
     let tsp = fiber.stack().top().unwrap();
+    let stack_limit = unsafe { tsp.sub(stack_size - red_zone_size) } as usize;
     let contobj = Box::new(ContinuationObject {
-        limits: StackLimits::with_stack_limit(unsafe { tsp.sub(DEFAULT_FIBER_SIZE) } as usize),
+        limits: StackLimits::with_stack_limit(stack_limit),
         fiber: Box::into_raw(fiber),
         parent_chain: StackChain::Absent,
         args: payload,
@@ -413,12 +419,6 @@ pub fn resume(
         *runtime_limits.last_wasm_entry_sp.get() = (*contobj).limits.last_wasm_entry_sp;
     }
 
-    unsafe {
-        (*(*(*instance.store()).vmruntime_limits())
-            .stack_limit
-            .get_mut()) = 0
-    };
-
     Ok(fiber.resume())
 }
 

crates/runtime/src/traphandlers/backtrace.rs

@@ -230,11 +230,7 @@ impl Backtrace {
                         let stack_range = (*cont.fiber).stack().range().unwrap();
                         debug_assert!(stack_range.contains(&limits.last_wasm_exit_fp));
                         debug_assert!(stack_range.contains(&limits.last_wasm_entry_sp));
-                        // TODO(frank-emrich) Enable this assertion once we stop
-                        // zero-ing the stack limit in
-                        // `wasmtime_runtime::continuation::resume`
-                        //
-                        // debug_assert_eq!(stack_range.end, limits.stack_limit);
+                        debug_assert!(stack_range.contains(&limits.stack_limit));
                     },
                     None => {
                         // reached stack information for main stack

crates/wasmtime/src/config.rs

@@ -223,6 +223,7 @@ impl Config {
             compiler_config: CompilerConfig::default(),
             wasmfx_config: WasmFXConfig {
                 stack_size: wasmtime_continuations::DEFAULT_FIBER_SIZE,
+                red_zone_size: wasmtime_continuations::DEFAULT_RED_ZONE_SIZE,
             },
             #[cfg(feature = "cache")]
             cache_config: CacheConfig::new_cache_disabled(),
@@ -699,6 +700,13 @@ impl Config {
         self
     }
 
+    /// Configures the amount of space that must be left on stack when starting
+    /// execution of a function while running on a continuation stack.
+    pub fn wasmfx_red_zone_size(&mut self, size: usize) -> &mut Self {
+        self.wasmfx_config.red_zone_size = size;
+        self
+    }
+
     /// Configures whether the WebAssembly tail calls proposal will be enabled
     /// for compilation or not.
     ///

tests/all/typed_continuations.rs

@@ -1116,4 +1116,49 @@ mod traps {
 
         Ok(())
     }
+
+    #[test]
+    #[cfg_attr(feature = "typed_continuations_baseline_implementation", ignore)]
+    fn stack_overflow_in_continuation() -> Result<()> {
+        let wat = r#"
+        (module
+            (type $ft (func (param i32)))
+            (type $ct (cont $ft))
+
+            (func $entry (export "entry")
+                (call $a)
+            )
+
+            (func $a (export "a")
+                ;; We ask for a billion recursive calls
+                (i32.const 1_000_000_000)
+
+                (resume $ct (cont.new $ct (ref.func $overflow)))
+            )
+
+            (func $overflow (export "overflow") (param $i i32)
+                (block $continue
+                    (local.get $i)
+                    ;; return if $i == 0
+                    (br_if $continue)
+                    (return)
+                )
+                (i32.sub (local.get $i) (i32.const 1))
+                (call $overflow)
+            )
+
+        )
+    "#;
+
+        let runner = Runner::new();
+
+        let error = runner
+            .run_test::<()>(wat, &[])
+            .expect_err("Expecting execution to yield error");
+
+        assert!(error.root_cause().is::<Trap>());
+        assert_eq!(*error.downcast_ref::<Trap>().unwrap(), Trap::StackOverflow);
+
+        Ok(())
+    }
 }