cranelift/pulley: assert continuation has single predecessor (debug only)

Make the structural invariant explicit: the funcref-dispatch fusion
absorbs the two `VMFuncRef` field loads from the continuation block,
which is only sound when the continuation has exactly one
predecessor (the brif we're replacing). Build a `ControlFlowGraph`
in `pre_lower_pulley` and `debug_assert_eq!(n_preds, 1)` before
sinking.

Both the CFG construction and the assertion are gated on
`#[cfg(debug_assertions)]` so they compile to nothing in release
builds (no runtime overhead).

Author: matthargett

cranelift/codegen/src/isa/pulley_shared/inst/emit.rs

@@ -407,12 +407,20 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xband32_s8_br_if_not_x32(
-                        &mut inverted, dst_writable, src_reg, mask_imm, 0,
+                        &mut inverted,
+                        dst_writable,
+                        src_reg,
+                        mask_imm,
+                        0,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xband64_s8_br_if_not_x64(
-                        &mut inverted, dst_writable, src_reg, mask_imm, 0,
+                        &mut inverted,
+                        dst_writable,
+                        src_reg,
+                        mask_imm,
+                        0,
                     );
                 }
             }
@@ -422,12 +430,20 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xband32_s8_br_if_not_x32(
-                        &mut inverted, dst_writable, src_reg, mask_imm, inv_rel,
+                        &mut inverted,
+                        dst_writable,
+                        src_reg,
+                        mask_imm,
+                        inv_rel,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xband64_s8_br_if_not_x64(
-                        &mut inverted, dst_writable, src_reg, mask_imm, inv_rel,
+                        &mut inverted,
+                        dst_writable,
+                        src_reg,
+                        mask_imm,
+                        inv_rel,
                     );
                 }
             }
@@ -482,12 +498,24 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xfuncref_dispatch_not_x32(
-                        &mut inverted, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0,
+                        &mut inverted,
+                        dst_code_w,
+                        dst_vmctx_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        0,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xfuncref_dispatch_not_x64(
-                        &mut inverted, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0,
+                        &mut inverted,
+                        dst_code_w,
+                        dst_vmctx_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        0,
                     );
                 }
             }
@@ -497,12 +525,24 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xfuncref_dispatch_not_x32(
-                        &mut inverted, dst_code_w, dst_vmctx_w, src_reg, oc, ov, inv_rel,
+                        &mut inverted,
+                        dst_code_w,
+                        dst_vmctx_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        inv_rel,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xfuncref_dispatch_not_x64(
-                        &mut inverted, dst_code_w, dst_vmctx_w, src_reg, oc, ov, inv_rel,
+                        &mut inverted,
+                        dst_code_w,
+                        dst_vmctx_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        inv_rel,
                     );
                 }
             }
@@ -513,12 +553,12 @@ fn pulley_emit<P>(
             sink.use_label_at_offset(taken_end - 4, *taken, LabelUse::PcRel);
             sink.add_cond_branch(*start_offset, taken_end, *taken, &inverted);
             patch_pc_rel_offset(sink, |sink| match size {
-                OperandSize::Size32 => enc::xfuncref_dispatch_x32(
-                    sink, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0,
-                ),
-                OperandSize::Size64 => enc::xfuncref_dispatch_x64(
-                    sink, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0,
-                ),
+                OperandSize::Size32 => {
+                    enc::xfuncref_dispatch_x32(sink, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0)
+                }
+                OperandSize::Size64 => {
+                    enc::xfuncref_dispatch_x64(sink, dst_code_w, dst_vmctx_w, src_reg, oc, ov, 0)
+                }
             });
             debug_assert_eq!(sink.cur_offset(), taken_end);
 
@@ -559,12 +599,26 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xband_funcref_dispatch_not_x32(
-                        &mut inverted, dm_w, dc_w, dv_w, src_reg, oc, ov, 0,
+                        &mut inverted,
+                        dm_w,
+                        dc_w,
+                        dv_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        0,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xband_funcref_dispatch_not_x64(
-                        &mut inverted, dm_w, dc_w, dv_w, src_reg, oc, ov, 0,
+                        &mut inverted,
+                        dm_w,
+                        dc_w,
+                        dv_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        0,
                     );
                 }
             }
@@ -574,12 +628,26 @@ fn pulley_emit<P>(
             match size {
                 OperandSize::Size32 => {
                     enc::xband_funcref_dispatch_not_x32(
-                        &mut inverted, dm_w, dc_w, dv_w, src_reg, oc, ov, inv_rel,
+                        &mut inverted,
+                        dm_w,
+                        dc_w,
+                        dv_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        inv_rel,
                     );
                 }
                 OperandSize::Size64 => {
                     enc::xband_funcref_dispatch_not_x64(
-                        &mut inverted, dm_w, dc_w, dv_w, src_reg, oc, ov, inv_rel,
+                        &mut inverted,
+                        dm_w,
+                        dc_w,
+                        dv_w,
+                        src_reg,
+                        oc,
+                        ov,
+                        inv_rel,
                     );
                 }
             }
@@ -589,12 +657,12 @@ fn pulley_emit<P>(
             sink.use_label_at_offset(taken_end - 4, *taken, LabelUse::PcRel);
             sink.add_cond_branch(*start_offset, taken_end, *taken, &inverted);
             patch_pc_rel_offset(sink, |sink| match size {
-                OperandSize::Size32 => enc::xband_funcref_dispatch_x32(
-                    sink, dm_w, dc_w, dv_w, src_reg, oc, ov, 0,
-                ),
-                OperandSize::Size64 => enc::xband_funcref_dispatch_x64(
-                    sink, dm_w, dc_w, dv_w, src_reg, oc, ov, 0,
-                ),
+                OperandSize::Size32 => {
+                    enc::xband_funcref_dispatch_x32(sink, dm_w, dc_w, dv_w, src_reg, oc, ov, 0)
+                }
+                OperandSize::Size64 => {
+                    enc::xband_funcref_dispatch_x64(sink, dm_w, dc_w, dv_w, src_reg, oc, ov, 0)
+                }
             });
             debug_assert_eq!(sink.cur_offset(), taken_end);
 

cranelift/codegen/src/isa/pulley_shared/lower.rs

@@ -460,6 +460,35 @@ where
             }
         }
     }
+
+    // Debug-only structural invariant: each continuation block we're about
+    // to absorb loads from must have exactly one predecessor (the brif that
+    // matched the pattern). If a second predecessor reaches the
+    // continuation, its control-flow path would observe the absorbed
+    // load destinations as uninitialized — exactly the bug the
+    // trap-on-null handler fix defends against. Building
+    // `ControlFlowGraph` is O(n) so we gate the whole thing on
+    // `#[cfg(debug_assertions)]`; the assertion + CFG construction
+    // compile to nothing in release builds (no runtime overhead).
+    #[cfg(debug_assertions)]
+    if !to_sink.is_empty() {
+        let cfg = crate::flowgraph::ControlFlowGraph::with_function(ctx.f);
+        for (l_code, _) in &to_sink {
+            let cont = ctx
+                .f
+                .layout
+                .inst_block(*l_code)
+                .expect("absorbed load belongs to a block");
+            let n_preds = cfg.pred_iter(cont).count();
+            debug_assert_eq!(
+                n_preds, 1,
+                "pulley funcref-dispatch fusion absorbs continuation-block \
+                 loads, so the continuation must have exactly one predecessor \
+                 (the brif being lowered); found {n_preds} predecessors for {cont:?}"
+            );
+        }
+    }
+
     for (l_code, l_vmctx) in to_sink {
         ctx.sink_pure_inst(l_code);
         ctx.sink_pure_inst(l_vmctx);

pulley/src/interp.rs

@@ -2474,8 +2474,14 @@ impl OpVisitor for Interpreter<'_> {
             // pointer, so the field loads are valid memory accesses.
             let base = s as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i64>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i64>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
                 self.state[dst_code].set_i64(code);
                 self.state[dst_vmctx].set_i64(vmctx);
             }
@@ -2504,8 +2510,14 @@ impl OpVisitor for Interpreter<'_> {
         } else {
             let base = s as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i64>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i64>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
                 self.state[dst_code].set_i64(code);
                 self.state[dst_vmctx].set_i64(vmctx);
             }
@@ -2528,8 +2540,14 @@ impl OpVisitor for Interpreter<'_> {
         } else {
             let base = s as usize as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i32>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i32>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
                 self.state[dst_code].set_i32(code);
                 self.state[dst_vmctx].set_i32(vmctx);
             }
@@ -2553,8 +2571,14 @@ impl OpVisitor for Interpreter<'_> {
         } else {
             let base = s as usize as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i32>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i32>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
                 self.state[dst_code].set_i32(code);
                 self.state[dst_vmctx].set_i32(vmctx);
             }
@@ -2590,8 +2614,14 @@ impl OpVisitor for Interpreter<'_> {
         if s != 0 {
             let base = masked as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i64>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i64>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
                 self.state[dst_code].set_i64(code);
                 self.state[dst_vmctx].set_i64(vmctx);
             }
@@ -2621,8 +2651,14 @@ impl OpVisitor for Interpreter<'_> {
         } else {
             let base = masked as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i64>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i64>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i64>()
+                    .read_unaligned();
                 self.state[dst_code].set_i64(code);
                 self.state[dst_vmctx].set_i64(vmctx);
             }
@@ -2646,8 +2682,14 @@ impl OpVisitor for Interpreter<'_> {
         if s != 0 {
             let base = masked as usize as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i32>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i32>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
                 self.state[dst_code].set_i32(code);
                 self.state[dst_vmctx].set_i32(vmctx);
             }
@@ -2676,8 +2718,14 @@ impl OpVisitor for Interpreter<'_> {
         } else {
             let base = masked as usize as *const u8;
             unsafe {
-                let code = base.byte_offset(offset_code as isize).cast::<i32>().read_unaligned();
-                let vmctx = base.byte_offset(offset_vmctx as isize).cast::<i32>().read_unaligned();
+                let code = base
+                    .byte_offset(offset_code as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
+                let vmctx = base
+                    .byte_offset(offset_vmctx as isize)
+                    .cast::<i32>()
+                    .read_unaligned();
                 self.state[dst_code].set_i32(code);
                 self.state[dst_vmctx].set_i32(vmctx);
             }

tests/all/pulley.rs

@@ -729,7 +729,11 @@ fn fusion_call_indirect_with_host_swap() -> Result<()> {
     let bytes = wat::parse_str(wat)?;
 
     for use_pulley in [true, false] {
-        let cfg = if use_pulley { pulley_trap_safe_config() } else { Config::new() };
+        let cfg = if use_pulley {
+            pulley_trap_safe_config()
+        } else {
+            Config::new()
+        };
         let engine = Engine::new(&cfg)?;
         let module = Module::new(&engine, &bytes)?;
         let mut store = Store::new(&engine, ());
@@ -782,7 +786,11 @@ fn fusion_call_indirect_imported_table() -> Result<()> {
     let bytes_b = wat::parse_str(wat_b)?;
 
     for use_pulley in [true, false] {
-        let cfg = if use_pulley { pulley_trap_safe_config() } else { Config::new() };
+        let cfg = if use_pulley {
+            pulley_trap_safe_config()
+        } else {
+            Config::new()
+        };
         let engine = Engine::new(&cfg)?;
         let module_a = Module::new(&engine, &bytes_a)?;
         let module_b = Module::new(&engine, &bytes_b)?;