dhil requested alexcrichton for a review on PR #11717.
dhil opened PR #11717 from dhil:continuation-trapping to bytecodealliance:main:
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).
dhil requested wasmtime-core-reviewers for a review on PR #11717.
posborne submitted PR review.
posborne created PR review comment:
Should be able to be simplified slightly to just
matches!(*stack_chain, VMStackChain::Continuation(_))
alexcrichton requested fitzgen for a review on PR #11717.
alexcrichton commented on PR #11717:
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
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_trapsframes, suggesting that a "bubbling" semantics of trapping up through continuation stacks may be the right thing to do.
fitzgen submitted PR review:
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)
Last updated: Dec 06 2025 at 06:05 UTC