pulley: trap on null in 8 fused funcref-dispatch handlers

Codex review on the rebeckerspecialties wasmtime fork PR pointed out
that phase-2/3's continuation-block load absorption breaks the
lazy-init slow path's correctness: the slow path's libcall rejoins
`continuation_block` via a block param, and after absorption the
loads are gone — `call_indirect` would see uninitialized
`dst_code`/`dst_vmctx` if the slow path is ever reached.

Fusion is gated on `is_eagerly_initialized_funcref_table` so the
slow path is unreachable at runtime, but the previous handler's
`ControlFlow::Continue(())` on null was advertised as defence-in-
depth and was itself broken. Replace it with `done_trap` in the 8
affected handlers (4 forward + 4 `_not` variants across x64/x32 ×
xfuncref_dispatch/xband_funcref_dispatch). `offset` on the `_not`
variants becomes vestigial; kept for encoding-shape parity.

Author: matthargett

pulley/src/interp.rs

@@ -2449,15 +2449,25 @@ impl OpVisitor for Interpreter<'_> {
     ) -> ControlFlow<Done> {
         // `src` is the ALREADY-MASKED funcref (`band v, -2` upstream — the
         // band stays as a separate Pulley op; this fusion absorbs only the
-        // brif + the two field loads). The branch fires when src != 0,
-        // matching the brif's original semantics in the eager-init
-        // predicate's IR rewrite (`brif value_masked, taken, null`). Under
-        // the predicate `src` is never zero at runtime, but the handler
-        // still has to match the brif's null fall-through as
-        // defence-in-depth.
+        // brif + the two field loads from the continuation block). The
+        // branch fires when src != 0, matching the brif's original
+        // semantics in the eager-init predicate's IR rewrite
+        // (`brif value_masked, continuation, null`).
+        //
+        // The null path TRAPS rather than falling through to the original
+        // lazy-init `null_block`. The fusion absorbed the continuation
+        // block's loads into this op, so the slow path's
+        // `null_block → lazy_init → jump continuation([returned_entry])`
+        // edge now lands in a continuation block whose loads are gone —
+        // call_indirect there would see uninitialized dst_code/dst_vmctx.
+        // The fusion is gated on `is_eagerly_initialized_funcref_table`,
+        // which guarantees `src != 0` at runtime, so trapping here is
+        // unreachable in correct code. If we ever do reach it (predicate
+        // bug, runtime invariant violation), we fail closed with a clean
+        // trap rather than misdispatching.
         let s = self.state[src].get_u64();
         if s == 0 {
-            ControlFlow::Continue(())
+            self.done_trap::<crate::XfuncrefDispatchX64>()
         } else {
             // SAFETY: under the eager-init predicate, the wasmtime runtime
             // enforces that the funcref slot contains a real VMFuncRef
@@ -2482,9 +2492,15 @@ impl OpVisitor for Interpreter<'_> {
         offset_vmctx: i8,
         offset: PcRelOffset,
     ) -> ControlFlow<Done> {
+        // Inverted form: fast path falls through, null path used to
+        // branch to `offset`. After the trap-on-null fix the branch
+        // target is dead — `offset` is kept in the encoding only for
+        // bytecode-shape compatibility with the forward variant. See
+        // `xfuncref_dispatch_x64` for the soundness rationale.
+        let _ = offset;
         let s = self.state[src].get_u64();
         if s == 0 {
-            self.pc_rel_jump::<crate::XfuncrefDispatchNotX64>(offset)
+            self.done_trap::<crate::XfuncrefDispatchNotX64>()
         } else {
             let base = s as *const u8;
             unsafe {
@@ -2508,7 +2524,7 @@ impl OpVisitor for Interpreter<'_> {
     ) -> ControlFlow<Done> {
         let s = self.state[src].get_u32();
         if s == 0 {
-            ControlFlow::Continue(())
+            self.done_trap::<crate::XfuncrefDispatchX32>()
         } else {
             let base = s as usize as *const u8;
             unsafe {
@@ -2530,9 +2546,10 @@ impl OpVisitor for Interpreter<'_> {
         offset_vmctx: i8,
         offset: PcRelOffset,
     ) -> ControlFlow<Done> {
+        let _ = offset;
         let s = self.state[src].get_u32();
         if s == 0 {
-            self.pc_rel_jump::<crate::XfuncrefDispatchNotX32>(offset)
+            self.done_trap::<crate::XfuncrefDispatchNotX32>()
         } else {
             let base = s as usize as *const u8;
             unsafe {
@@ -2558,10 +2575,15 @@ impl OpVisitor for Interpreter<'_> {
         // Phase-3 fusion: combines the standalone xband64_s8 (mask init
         // bit) with the phase-2 brif+xload+xload dispatch into one op.
         // The masked value is written unconditionally to dst_masked so
-        // the brif's block-call-arg machinery still finds it; the two
-        // loads only fire on the non-null side. Soundness under the
-        // eager-init predicate: `src` is provably non-zero at runtime,
-        // so the loads are valid memory accesses.
+        // the brif's block-call-arg machinery still finds it.
+        //
+        // The null path TRAPS rather than falling through to the original
+        // `null_block`. See `xfuncref_dispatch_x64` for the rationale: the
+        // fusion absorbed the continuation block's loads, so the slow
+        // path's rejoin to the continuation would see uninitialized
+        // dst_code/dst_vmctx. Gated on `is_eagerly_initialized_funcref_table`,
+        // so `src != 0` at runtime; trapping here is unreachable in correct
+        // code but fails closed if the runtime invariant is ever violated.
         let s = self.state[src].get_u64();
         let masked = s & !1u64;
         self.state[dst_masked].set_u64(masked);
@@ -2575,7 +2597,7 @@ impl OpVisitor for Interpreter<'_> {
             }
             self.pc_rel_jump::<crate::XbandFuncrefDispatchX64>(offset)
         } else {
-            ControlFlow::Continue(())
+            self.done_trap::<crate::XbandFuncrefDispatchX64>()
         }
     }
 
@@ -2589,11 +2611,13 @@ impl OpVisitor for Interpreter<'_> {
         offset_vmctx: i8,
         offset: PcRelOffset,
     ) -> ControlFlow<Done> {
+        // Inverted form; `offset` is vestigial after the trap-on-null fix.
+        let _ = offset;
         let s = self.state[src].get_u64();
         let masked = s & !1u64;
         self.state[dst_masked].set_u64(masked);
         if s == 0 {
-            self.pc_rel_jump::<crate::XbandFuncrefDispatchNotX64>(offset)
+            self.done_trap::<crate::XbandFuncrefDispatchNotX64>()
         } else {
             let base = masked as *const u8;
             unsafe {
@@ -2629,7 +2653,7 @@ impl OpVisitor for Interpreter<'_> {
             }
             self.pc_rel_jump::<crate::XbandFuncrefDispatchX32>(offset)
         } else {
-            ControlFlow::Continue(())
+            self.done_trap::<crate::XbandFuncrefDispatchX32>()
         }
     }
 
@@ -2643,11 +2667,12 @@ impl OpVisitor for Interpreter<'_> {
         offset_vmctx: i8,
         offset: PcRelOffset,
     ) -> ControlFlow<Done> {
+        let _ = offset;
         let s = self.state[src].get_u32();
         let masked = s & !1u32;
         self.state[dst_masked].set_u32(masked);
         if s == 0 {
-            self.pc_rel_jump::<crate::XbandFuncrefDispatchNotX32>(offset)
+            self.done_trap::<crate::XbandFuncrefDispatchNotX32>()
         } else {
             let base = masked as usize as *const u8;
             unsafe {

pulley/src/lib.rs

@@ -603,16 +603,24 @@ macro_rules! for_each_op {
             /// already-masked funcref pointer (`band v, -2` upstream).
             ///
             /// Forward form: loads-and-branch fire on the non-null side
-            /// (`src != 0`); the null side falls through. Used at the
+            /// (`src != 0`); the **null side TRAPS**. Used at the
             /// call_indirect lazy-init brif site under
             /// `is_eagerly_initialized_funcref_table` AND when the signature
             /// check is statically elided — under those conditions the only
             /// uses of the masked funcref pointer are the two `VMFuncRef`
             /// field loads, and the brif's null branch is provably
-            /// unreachable at runtime. The handler's runtime null check is
-            /// defence-in-depth (matching the original brif's role); it MUST
-            /// fall through on null so the slow path's lazy-init builtin
-            /// stays callable in the (provably-unreachable) error case.
+            /// unreachable at runtime.
+            ///
+            /// The handler traps on null (rather than falling through to
+            /// the original `null_block`'s lazy-init libcall) because the
+            /// fusion absorbs the two field loads from the continuation
+            /// block — and the lazy-init slow path rejoins THAT same
+            /// continuation. If we fell through and the slow path ran,
+            /// call_indirect would observe uninitialized `dst_code` /
+            /// `dst_vmctx`. The predicate guarantees the null path is
+            /// unreachable in correct code; trapping there is a fail-closed
+            /// defence-in-depth that protects against future predicate bugs
+            /// or runtime invariant violations.
             ///
             /// Fused form of `br_if + xload64 + xload64` (the preceding
             /// `xband64_s8` stays as a separate op since `src` is consumed
@@ -623,10 +631,11 @@ macro_rules! for_each_op {
             /// pattern matches), so the per-new-opcode predictor cost
             /// stays at one new op family rather than two.
             xfuncref_dispatch_x64 = XfuncrefDispatchX64 { dst_code: XReg, dst_vmctx: XReg, src: XReg, offset_code: i8, offset_vmctx: i8, offset: PcRelOffset };
-            /// Inverted form of `xfuncref_dispatch_x64`: the null side
-            /// branches and the loads-and-fall-through fire on `src != 0`.
-            /// Used by MachBuffer's branch-direction-flip fallthrough
-            /// optimization when the fast path is the natural fall-through.
+            /// Inverted form of `xfuncref_dispatch_x64`: fast-path
+            /// (loads-and-fall-through) fires on `src != 0`; the null side
+            /// **traps** (same rationale as the forward variant). `offset`
+            /// is vestigial after the trap-on-null fix; kept in the
+            /// encoding for shape parity with the forward variant.
             xfuncref_dispatch_not_x64 = XfuncrefDispatchNotX64 { dst_code: XReg, dst_vmctx: XReg, src: XReg, offset_code: i8, offset_vmctx: i8, offset: PcRelOffset };
             /// 32-bit pointer-width form of `xfuncref_dispatch_x64`. Same
             /// semantics; `src`, `dst_code`, `dst_vmctx` are i32 loaded into
@@ -646,7 +655,9 @@ macro_rules! for_each_op {
             /// if `src != 0`, load `wasm_call` from `dst_masked + offset_code`
             /// into `dst_code`, load callee `vmctx` from `dst_masked +
             /// offset_vmctx` into `dst_vmctx`, and branch by `offset`. The
-            /// null side falls through to the slow path.
+            /// null side **traps** (same rationale as `xfuncref_dispatch_*`:
+            /// the continuation-block loads were absorbed, so the slow path
+            /// would misdispatch if we fell through).
             ///
             /// Soundness: same as `xfuncref_dispatch_*` — gated on
             /// `is_eagerly_initialized_funcref_table` so `src` is provably
@@ -656,11 +667,12 @@ macro_rules! for_each_op {
             /// match_loop dispatch per call_indirect site vs phase 2 (the
             /// preceding standalone `xband64_s8` is absorbed).
             xband_funcref_dispatch_x64 = XbandFuncrefDispatchX64 { dst_masked: XReg, dst_code: XReg, dst_vmctx: XReg, src: XReg, offset_code: i8, offset_vmctx: i8, offset: PcRelOffset };
-            /// Inverted form: branch when `src == 0`, loads-and-fall-through
-            /// on `src != 0`. Used by MachBuffer's branch-direction flip
-            /// when the fast path is the natural fall-through. The
-            /// `dst_masked = src & -2` write is unconditional in both
-            /// forms.
+            /// Inverted form: fast-path (loads-and-fall-through) on
+            /// `src != 0`; null path traps. Used by MachBuffer's
+            /// branch-direction flip when the fast path is the natural
+            /// fall-through. The `dst_masked = src & -2` write is
+            /// unconditional in both forms. `offset` is vestigial after
+            /// the trap-on-null fix; kept for encoding-shape parity.
             xband_funcref_dispatch_not_x64 = XbandFuncrefDispatchNotX64 { dst_masked: XReg, dst_code: XReg, dst_vmctx: XReg, src: XReg, offset_code: i8, offset_vmctx: i8, offset: PcRelOffset };
             /// 32-bit pointer-width form of `xband_funcref_dispatch_x64`.
             /// Used on `pulley32` / arm64_32-apple-watchos.