Continuation trapping semantics

[dhil]

This patch fixes a problem with traps on continuations, which would otherwise allow a Wasm program to continue running after invoking a trapping instruction. Currently, a fresh trap handler is installed per continuation stack, meaning that the effects of a trap is delimited by the stack segment on which the trap occurred -- whereas it really ought to be delimited by the top-level of the program (i.e. the part just before host/engine frames).

[posborne]

Should be able to be simplified slightly to just matches!(*stack_chain, VMStackChain::Continuation(_))

[alexcrichton]

I'm not familiar enough with the stack-switching code currently to review this myself. For example I don't know if this is accidentally skipping over native frames at the base of other continuations. Given that I'm going to defer to @fitzgen and @posborne as they're more familiar with the details

[dhil]

For example I don't know if this is accidentally skipping over native frames at the base of other continuations.

Excellent point. I think it may skip over intermediate invoke_wasm_and_catch_traps frames, suggesting that a "bubbling" semantics of trapping up through continuation stacks may be the right thing to do.

[fitzgen]

Thanks for fixing this bug!

Can we add a test that spawns an N deep stack chain with M frames where every other frame is a host frame, and the last frame (whether host or Wasm) triggers a trap? Then we can run that test exhaustively for small N and M.

Something like

(module
  ;; The imported host function.
  (import "host" "func" (func $host_func (param i32 i32)))

  ;; A global that is incremented after calling the host
  ;; function, which should trap, and therefore the
  ;; increment should never happen.
  (global $g (export "g") (mut i32) (i32.const 0))

  (func (export "run") (param $frames-per-stack i32) (param $fuel i32)
    ;; Trap on out-of-fuel for frames.
    if (i32.eqz (local.get $fuel))
      unreachable
    end

    ;; Decrement frame fuel.
    (local.set $fuel (i32.sub (local.get $fuel) (i32.const 1)))

    if (i32.eqz (i32.rem (local.get $fuel) (local.get $frames-per-stack))))
      ;; TODO: Spawn a new stack, starting either with `run`
      ;; or our host function (based on another param or a
      ;; global or something), and switch to it...
    else
      ;; Call the host function to continue our mutual recursion.
      (call $host_func (local.get $frames-per-stack) (local.get $frame-fuel))
    end

    ;; Increment the global. Should never execute, dynamically.
    (global.set $g (i32.add (global.get $g) (i32.const 1))
  )
)

let host_func = Func::wrap(
    &mut store,
    |mut caller: Caller<'_, ()>, frames_per_stack: u32, fuel: u32| -> Result<()> {
        ;; Trap on out-of-fuel for frames.
        if fuel == 0 {
            bail!("out of frame fuel");
        }

        ;; Mutual recursion back into the Wasm function.
        let run = instance.get_typed_func::<(u32, u32)>(&mut caller).unwrap();
        run.call(&mut caller, (frames_per_stack, fuel - 1))?;

        ;; Increment the global. Should never execute, dynamically.
        let g = instance.get_global("g").unwrap();
        let g_val = g.get(&mut caller).unwrap_i32();
        g.set(&mut caller, Val::I32(g_val + 1))?;

        Ok(())
    },
);

// ...

for frames_per_stack in 1..4 {
    for fuel in 0..frames_per_stack * 3 {
        let mut store = Store::new(&engine, ());
        let instance = Instance::new(...)?;

        let run = instance.get_typed_func::<(u32, u32)>(&mut store)?;
        run.call(&mut store, (frames_per_stack, fuel))?;

        let g = instance.get_global(&mut store, "g").unwrap();
        assert_eq!(g.unwrap_i32(), 0);
    }
}

This would give me a lot more confidence that we are properly handling traps across stacks, regardless of the stack chain, host functions, and what kind of frame is youngest or oldest.

(And when we add embedder API support for spawning stacks, we should also extend the host function in this new test to use that support)