Repository: bytecodealliance/wasmtime
Pull request #7478 (2023-11-03 19:30:25)

Reset sp_offset when resetting stack in Winch
<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
This is a fix for a fuzzer testcase triggering an assertion of `sp_offset` not equaling `locals_size` where there was a function which had spilled a local and ended with `unreachable`. I've added a minimal reproduction of the test case as the filetest `with_spilled_local.wat`. While investigating, it looks like something similar happens when `unreachable` is the last instruction in an `if` followed by an `else` (covered by the other filetest added). Having `reset_stack` also reset the `sp_offset` clears up both assertion failures.


Patch:
diff --git a/cranelift/codegen/src/isa/aarch64/abi.rs b/cranelift/codegen/src/isa/aarch64/abi.rs
index b5d6e56fc..d1279921d 100644
--- a/cranelift/codegen/src/isa/aarch64/abi.rs
+++ b/cranelift/codegen/src/isa/aarch64/abi.rs
@@ -677,43 +677,9 @@ impl ABIMachineSpec for AArch64MachineDeps {
 
         let probe_count = align_to(frame_size, guard_size) / guard_size;
         if probe_count <= PROBE_MAX_UNROLL {
-            // When manually unrolling stick an instruction that stores 0 at a
-            // constant offset relative to the stack pointer. This will
-            // turn into something like `movn tmp, #n ; stur xzr [sp, tmp]`.
-            //
-            // Note that this may actually store beyond the stack size for the
-            // last item but that's ok since it's unused stack space and if
-            // that faults accidentally we're so close to faulting it shouldn't
-            // make too much difference to fault there.
-            insts.reserve(probe_count as usize);
-            for i in 0..probe_count {
-                let offset = (guard_size * (i + 1)) as i64;
-                insts.push(Self::gen_store_stack(
-                    StackAMode::SPOffset(-offset, I8),
-                    zero_reg(),
-                    I32,
-                ));
-            }
+            Self::gen_probestack_unroll(insts, guard_size, probe_count)
         } else {
-            // The non-unrolled version uses two temporary registers. The
-            // `start` contains the current offset from sp and counts downwards
-            // during the loop by increments of `guard_size`. The `end` is
-            // the size of the frame and where we stop.
-            //
-            // Note that this emission is all post-regalloc so it should be ok
-            // to use the temporary registers here as input/output as the loop
-            // itself is not allowed to use the registers.
-            let start = writable_spilltmp_reg();
-            let end = writable_tmp2_reg();
-            // `gen_inline_probestack` is called after regalloc2, so it's acceptable to reuse
-            // `start` and `end` as temporaries in load_constant.
-            insts.extend(Inst::load_constant(start, 0, &mut |_| start));
-            insts.extend(Inst::load_constant(end, frame_size.into(), &mut |_| end));
-            insts.push(Inst::StackProbeLoop {
-                start,
-                end: end.to_reg(),
-                step: Imm12::maybe_from_u64(guard_size.into()).unwrap(),
-            });
+            Self::gen_probestack_loop(insts, frame_size, guard_size)
         }
     }
 
@@ -1210,6 +1176,52 @@ impl ABIMachineSpec for AArch64MachineDeps {
     }
 }
 
+impl AArch64MachineDeps {
+    fn gen_probestack_unroll(insts: &mut SmallInstVec<Inst>, guard_size: u32, probe_count: u32) {
+        // When manually unrolling adjust the stack pointer and then write a zero
+        // to the stack at that offset. This generates something like
+        // `sub sp, sp, #1, lsl #12` followed by `stur wzr, [sp]`.
+        //
+        // We do this because valgrind expects us to never write beyond the stack
+        // pointer and associated redzone.
+        // See: https://github.com/bytecodealliance/wasmtime/issues/7454
+        for _ in 0..probe_count {
+            insts.extend(Self::gen_sp_reg_adjust(-(guard_size as i32)));
+
+            insts.push(Self::gen_store_stack(
+                StackAMode::SPOffset(0, I8),
+                zero_reg(),
+                I32,
+            ));
+        }
+
+        // Restore the stack pointer to its original value
+        insts.extend(Self::gen_sp_reg_adjust((guard_size * probe_count) as i32));
+    }
+
+    fn gen_probestack_loop(insts: &mut SmallInstVec<Inst>, frame_size: u32, guard_size: u32) {
+        // The non-unrolled version uses two temporary registers. The
+        // `start` contains the current offset from sp and counts downwards
+        // during the loop by increments of `guard_size`. The `end` is
+        // the size of the frame and where we stop.
+        //
+        // Note that this emission is all post-regalloc so it should be ok
+        // to use the temporary registers here as input/output as the loop
+        // itself is not allowed to use the registers.
+        let start = writable_spilltmp_reg();
+        let end = writable_tmp2_reg();
+        // `gen_inline_probestack` is called after regalloc2, so it's acceptable to reuse
+        // `start` and `end` as temporaries in load_constant.
+        insts.extend(Inst::load_constant(start, 0, &mut |_| start));
+        insts.extend(Inst::load_constant(end, frame_size.into(), &mut |_| end));
+        insts.push(Inst::StackProbeLoop {
+            start,
+            end: end.to_reg(),
+            step: Imm12::maybe_from_u64(guard_size.into()).unwrap(),
+        });
+    }
+}
+
 fn select_api_key(
     isa_flags: &aarch64_settings::Flags,
     call_conv: isa::CallConv,
diff --git a/cranelift/codegen/src/isa/riscv64/abi.rs b/cranelift/codegen/src/isa/riscv64/abi.rs
index d119d0f8e..095ef13f1 100644
--- a/cranelift/codegen/src/isa/riscv64/abi.rs
+++ b/cranelift/codegen/src/isa/riscv64/abi.rs
@@ -718,10 +718,20 @@ impl ABIMachineSpec for Riscv64MachineDeps {
         // Number of probes that we need to perform
         let probe_count = align_to(frame_size, guard_size) / guard_size;
 
+        // Must be a caller-saved register that is not an argument.
+        let tmp = match call_conv {
+            isa::CallConv::Tail => Writable::from_reg(x_reg(1)),
+            _ => Writable::from_reg(x_reg(28)), // t3
+        };
+
         if probe_count <= PROBE_MAX_UNROLL {
-            Self::gen_probestack_unroll(insts, guard_size, probe_count)
+            Self::gen_probestack_unroll(insts, tmp, guard_size, probe_count)
         } else {
-            Self::gen_probestack_loop(insts, call_conv, guard_size, probe_count)
+            insts.push(Inst::StackProbeLoop {
+                guard_size,
+                probe_count,
+                tmp,
+            });
         }
     }
 }
@@ -1052,33 +1062,41 @@ fn create_reg_enviroment() -> MachineEnv {
 }
 
 impl Riscv64MachineDeps {
-    fn gen_probestack_unroll(insts: &mut SmallInstVec<Inst>, guard_size: u32, probe_count: u32) {
-        insts.reserve(probe_count as usize);
-        for i in 0..probe_count {
-            let offset = (guard_size * (i + 1)) as i64;
+    fn gen_probestack_unroll(
+        insts: &mut SmallInstVec<Inst>,
+        tmp: Writable<Reg>,
+        guard_size: u32,
+        probe_count: u32,
+    ) {
+        // When manually unrolling adjust the stack pointer and then write a zero
+        // to the stack at that offset.
+        //
+        // We do this because valgrind expects us to never write beyond the stack
+        // pointer and associated redzone.
+        // See: https://github.com/bytecodealliance/wasmtime/issues/7454
+
+        // Store the adjust amount in a register upfront, so we don't have to
+        // reload it for each probe. It's worth loading this as a negative and
+        // using an `add` instruction since we have compressed versions of `add`
+        // but not the `sub` instruction.
+        insts.extend(Inst::load_constant_u64(tmp, (-(guard_size as i64)) as u64));
+
+        for _ in 0..probe_count {
+            insts.push(Inst::AluRRR {
+                alu_op: AluOPRRR::Add,
+                rd: writable_stack_reg(),
+                rs1: stack_reg(),
+                rs2: tmp.to_reg(),
+            });
+
             insts.push(Self::gen_store_stack(
-                StackAMode::SPOffset(-offset, I8),
+                StackAMode::SPOffset(0, I8),
                 zero_reg(),
                 I32,
             ));
         }
-    }
 
-    fn gen_probestack_loop(
-        insts: &mut SmallInstVec<Inst>,
-        call_conv: isa::CallConv,
-        guard_size: u32,
-        probe_count: u32,
-    ) {
-        // Must be a caller-saved register that is not an argument.
-        let tmp = match call_conv {
-            isa::CallConv::Tail => Writable::from_reg(x_reg(1)),
-            _ => Writable::from_reg(x_reg(28)), // t3
-        };
-        insts.push(Inst::StackProbeLoop {
-            guard_size,
-            probe_count,
-            tmp,
-        });
+        // Restore the stack pointer to its original value
+        insts.extend(Self::gen_sp_reg_adjust((guard_size * probe_count) as i32));
     }
 }
diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs
index d99933500..49c54d529 100644
--- a/cranelift/codegen/src/isa/x64/abi.rs
+++ b/cranelift/codegen/src/isa/x64/abi.rs
@@ -33,17 +33,23 @@ pub struct X64ABIMachineSpec;
 impl X64ABIMachineSpec {
     fn gen_probestack_unroll(insts: &mut SmallInstVec<Inst>, guard_size: u32, probe_count: u32) {
         insts.reserve(probe_count as usize);
-        for i in 0..probe_count {
-            let offset = (guard_size * (i + 1)) as i64;
+        for _ in 0..probe_count {
+            // "Allocate" stack space for the probe by decrementing the stack pointer before
+            // the write. This is required to make valgrind happy.
+            // See: https://github.com/bytecodealliance/wasmtime/issues/7454
+            insts.extend(Self::gen_sp_reg_adjust(-(guard_size as i32)));
 
             // TODO: It would be nice if we could store the imm 0, but we don't have insts for those
             // so store the stack pointer. Any register will do, since the stack is undefined at this point
             insts.push(Self::gen_store_stack(
-                StackAMode::SPOffset(-offset, I8),
+                StackAMode::SPOffset(0, I8),
                 regs::rsp(),
                 I32,
             ));
         }
+
+        // Restore the stack pointer to its original value
+        insts.extend(Self::gen_sp_reg_adjust((guard_size * probe_count) as i32));
     }
 
     fn gen_probestack_loop(
@@ -523,8 +529,8 @@ impl ABIMachineSpec for X64ABIMachineSpec {
         // Unroll at most n consecutive probes, before falling back to using a loop
         //
         // This was number was picked because the loop version is 38 bytes long. We can fit
-        // 5 inline probes in that space, so unroll if its beneficial in terms of code size.
-        const PROBE_MAX_UNROLL: u32 = 5;
+        // 4 inline probes in that space, so unroll if its beneficial in terms of code size.
+        const PROBE_MAX_UNROLL: u32 = 4;
 
         // Number of probes that we need to perform
         let probe_count = align_to(frame_size, guard_size) / guard_size;
diff --git a/cranelift/codegen/src/opts/shifts.isle b/cranelift/codegen/src/opts/shifts.isle
index acf53946c..272c5c90b 100644
--- a/cranelift/codegen/src/opts/shifts.isle
+++ b/cranelift/codegen/src/opts/shifts.isle
@@ -268,3 +268,43 @@
       (rotl ty x (isub_uextend y z)))
 (rule (simplify (rotl ty (rotr ty x y @ (value_type kty)) z @ (value_type kty)))
       (rotr ty x (isub_uextend y z)))
+
+;; Convert shifts into rotates. We always normalize into a rotate left.
+;;
+;; (bor (ishl x k1) (ushr x k2)) == (rotl x k1) if k2 == ty_bits - k1
+;; (bor (ushr x k2) (ishl x k1)) == (rotl x k1) if k2 == ty_bits - k1
+;;
+;; TODO: This rule is restricted to scalars since no backend currently
+;; supports SIMD rotates.
+(rule (simplify (bor (ty_int ty)
+                      (ishl ty x k @ (iconst _ (u64_from_imm64 k1)))
+                      (ushr ty x (iconst _ (u64_from_imm64 k2)))))
+      (if-let $true (u64_eq k2 (u64_sub (ty_bits_u64 (lane_type ty)) k1)))
+      (rotl ty x k))
+(rule (simplify (bor (ty_int ty)
+                      (ushr ty x (iconst _ (u64_from_imm64 k2)))
+                      (ishl ty x k @ (iconst _ (u64_from_imm64 k1)))))
+      (if-let $true (u64_eq k2 (u64_sub (ty_bits_u64 (lane_type ty)) k1)))
+      (rotl ty x k))
+
+;; Normalize the shift amount. Some rules can't fire unless the shift amount
+;; is normalized. This also helps us materialize fewer and smaller constants.
+;;
+;; (op x k) == (op x (and k (ty_shift_mask ty)))
+;;
+;; where `op` is one of ishl, ushr, sshr, rotl, rotr
+(rule (simplify (ishl ty x (iconst kty (u64_from_imm64 k))))
+      (if-let $false (u64_eq k (u64_and k (ty_shift_mask ty))))
+      (ishl ty x (iconst kty (imm64 (u64_and k (ty_shift_mask ty))))))
+(rule (simplify (ushr ty x (iconst kty (u64_from_imm64 k))))
+      (if-let $false (u64_eq k (u64_and k (ty_shift_mask ty))))
+      (ushr ty x (iconst kty (imm64 (u64_and k (ty_shift_mask ty))))))
+(rule (simplify (sshr ty x (iconst kty (u64_from_imm64 k))))
+      (if-let $false (u64_eq k (u64_and k (ty_shift_mask ty))))
+      (sshr ty x (iconst kty (imm64 (u64_and k (ty_shift_mask ty))))))
+(rule (simplify (rotr ty x (iconst kty (u64_from_imm64 k))))
+      (if-let $false (u64_eq k (u64_and k (ty_shift_mask ty))))
+      (rotr ty x (iconst kty (imm64 (u64_and k (ty_shift_mask ty))))))
+(rule (simplify (rotl ty x (iconst kty (u64_from_imm64 k))))
+      (if-let $false (u64_eq k (u64_and k (ty_shift_mask ty))))
+      (rotl ty x (iconst kty (imm64 (u64_and k (ty_shift_mask ty))))))
diff --git a/src/commands/serve.rs b/src/commands/serve.rs
index 31276a1aa..927cc755b 100644
--- a/src/commands/serve.rs
+++ b/src/commands/serve.rs
@@ -373,12 +373,16 @@ impl hyper::service::Service<Request> for ProxyHandler {
                 wasmtime_wasi_http::proxy::Proxy::instantiate_pre(&mut store, &inner.instance_pre)
                     .await?;
 
-            proxy
+            if let Err(e) = proxy
                 .wasi_http_incoming_handler()
                 .call_handle(store, req, out)
-                .await?;
+                .await
+            {
+                log::error!("[{req_id}] :: {:#?}", e);
+                return Err(e);
+            }
 
-            Ok::<_, anyhow::Error>(())
+            Ok(())
         });
 
         Box::pin(async move {
diff --git a/winch/codegen/src/codegen/mod.rs b/winch/codegen/src/codegen/mod.rs
index ab688a61b..bb1a8fead 100644
--- a/winch/codegen/src/codegen/mod.rs
+++ b/winch/codegen/src/codegen/mod.rs
@@ -104,6 +104,7 @@ where
             ControlStackFrame::If {
                 reachable,
                 original_stack_len,
+                original_sp_offset,
                 ..
             } => {
                 if *reachable {
@@ -112,7 +113,12 @@ where
                     // entry, the if-else branch will be reachable.
                     self.context.reachable = true;
                     // Reset the stack to the original length and offset.
-                    Self::reset_stack(&mut self.context, *original_stack_len);
+                    Self::reset_stack(
+                        &mut self.context,
+                        self.masm,
+                        *original_stack_len,
+                        *original_sp_offset,
+                    );
                     frame.bind_else(self.masm, self.context.reachable);
                 }
             }
@@ -127,9 +133,9 @@ where
         if frame.is_next_sequence_reachable() {
             self.context.reachable = true;
 
-            let (value_stack_len, _) = frame.original_stack_len_and_sp_offset();
+            let (value_stack_len, target_sp) = frame.original_stack_len_and_sp_offset();
             // Reset the stack to the original length and offset.
-            Self::reset_stack(&mut self.context, value_stack_len);
+            Self::reset_stack(&mut self.context, self.masm, value_stack_len, target_sp);
             // If the current frame is the outermost frame, which corresponds to the
             // current function's body, only bind the exit label as we don't need to
             // push any more values to the value stack, else perform the entire `bind_end`
@@ -145,27 +151,36 @@ where
             // and SP in the expected state.  The compiler can enter
             // in this state through an infinite loop.
             let (value_stack_len, target_sp) = frame.original_stack_len_and_sp_offset();
-            Self::reset_stack(&mut self.context, value_stack_len);
-            if self.masm.sp_offset() > target_sp {
-                self.masm.free_stack(self.masm.sp_offset() - target_sp);
-            }
+            Self::reset_stack(&mut self.context, self.masm, value_stack_len, target_sp);
         }
     }
 
     /// Helper function to reset value and stack pointer to the given length and stack pointer
     /// offset respectively. This function is only used when restoring the code generation's
     /// reachabiliy state when handling an unreachable `end` or `else`.
-    pub fn reset_stack(context: &mut CodeGenContext, target_stack_len: usize) {
+    pub fn reset_stack(
+        context: &mut CodeGenContext,
+        masm: &mut M,
+        target_stack_len: usize,
+        target_sp: u32,
+    ) {
         // `CodeGenContext::reset_stack` only gets called when
         // handling unreachable end or unreachable else, so we only
         // care about freeing any registers in the provided range.
+        let mut bytes_freed = 0;
         context.drop_last(
             context.stack.len() - target_stack_len,
             |regalloc, val| match val {
                 Val::Reg(tr) => regalloc.free(tr.reg),
+                Val::Memory(m) => bytes_freed += m.slot.size,
                 _ => {}
             },
         );
+        if masm.sp_offset() > target_sp {
+            let bytes = masm.sp_offset() - target_sp;
+            assert!(bytes_freed == bytes);
+            masm.free_stack(bytes);
+        }
     }
 
     fn emit_body(


Test patch:
diff --git a/cranelift/filetests/filetests/egraph/shifts.clif b/cranelift/filetests/filetests/egraph/shifts.clif
index dbe984c84..fbe308ddb 100644
--- a/cranelift/filetests/filetests/egraph/shifts.clif
+++ b/cranelift/filetests/filetests/egraph/shifts.clif
@@ -74,9 +74,9 @@ block0(v0: i8):
     v2 = sshr v0, v1
     v3 = ishl v2, v1
     return v3
-    ; check: v4 = iconst.i8 224
-    ; check: v5 = band v0, v4
-    ; check: return v5
+    ; check: v8 = iconst.i8 224
+    ; check: v9 = band v0, v8
+    ; check: return v9
 }
 
 function %sextend_shift_32_64_unsigned(i32) -> i64 {
@@ -596,9 +596,9 @@ block0(v0: i128):
     return v3
 }
 
-; check: v4 = iconst.i64 0
-; nextln: v5 = uextend.i128 v4  ; v4 = 0
-; nextln: return v5
+; check: v7 = iconst.i64 0
+; nextln: v8 = uextend.i128 v7
+; nextln: return v8
 
 
 function %i128_ishl_overflow_becomes_0(i128) -> i128 {
@@ -609,9 +609,9 @@ block0(v0: i128):
     return v3
 }
 
-; check: v4 = iconst.i64 0
-; nextln: v5 = uextend.i128 v4  ; v4 = 0
-; nextln: return v5
+; check: v7 = iconst.i64 0
+; nextln: v8 = uextend.i128 v7
+; nextln: return v8
 
 function %simd_shift_does_not_panic(i8x16) -> i8x16 {
 block0(v0: i8x16):
@@ -636,11 +636,10 @@ block0(v0: i8x16):
     return v5
 }
 
-; check: v1 = iconst.i16 30
-; nextln: v3 = sshr v0, v1  ; v1 = 30
-; nextln: v6 = iconst.i16 6
-; nextln: v7 = sshr v3, v6  ; v6 = 6
-; check: return v7
+; check: v6 = iconst.i16 6
+; check: v7 = sshr v0, v6
+; check: v9 = sshr v7, v6
+; check: return v9
 
 
 function %rotr_rotr_prop(i8) -> i8 {
@@ -758,3 +757,57 @@ block0(v0: i8, v1: i8):
 }
 
 ; check: return v0
+
+function %shifts_to_rotl(i64) -> i64 {
+block0(v0: i64):
+    v1 = iconst.i16 3
+    v2 = iconst.i32 61
+    v3 = ishl v0, v1
+    v4 = ushr v0, v2
+    v5 = bor v3, v4
+    return v5
+}
+
+; check: v1 = iconst.i16 3
+; nextln: v6 = rotl v0, v1  ; v1 = 3
+; check: return v6
+
+
+function %shifts_to_rotl_reverse(i64) -> i64 {
+block0(v0: i64):
+    v1 = iconst.i16 32
+    v2 = iconst.i32 32
+    v3 = ishl v0, v1
+    v4 = ushr v0, v2
+    v5 = bor v4, v3
+    return v5
+}
+
+; check: v1 = iconst.i16 32
+; nextln: v6 = rotl v0, v1  ; v1 = 32
+; check: return v6
+
+function %shifts_to_rotr_equivalent(i64) -> i64 {
+block0(v0: i64):
+    v1 = iconst.i16 3
+    v2 = iconst.i32 61
+    v3 = ishl v0, v2
+    v4 = ushr v0, v1
+    v5 = bor v3, v4
+    return v5
+}
+
+; check: v2 = iconst.i32 61
+; nextln: v6 = rotl v0, v2  ; v2 = 61
+; check: return v6
+
+function %ishl_normalize_amount(i64) -> i64 {
+block0(v0: i64):
+    v1 = iconst.i32 99
+    v2 = ishl v0, v1
+    return v2
+}
+
+; check: v3 = iconst.i32 35
+; check: v4 = ishl v0, v3
+; check: return v4
diff --git a/cranelift/filetests/filetests/isa/aarch64/inline-probestack.clif b/cranelift/filetests/filetests/isa/aarch64/inline-probestack.clif
index 2ec7431b9..eee45fd76 100644
--- a/cranelift/filetests/filetests/isa/aarch64/inline-probestack.clif
+++ b/cranelift/filetests/filetests/isa/aarch64/inline-probestack.clif
@@ -47,9 +47,13 @@ block0:
 ; VCode:
 ;   stp fp, lr, [sp, #-16]!
 ;   mov fp, sp
-;   movn x16, #4095 ; str wzr, [sp, x16, SXTX]
-;   movn x16, #8191 ; str wzr, [sp, x16, SXTX]
-;   movn x16, #12287 ; str wzr, [sp, x16, SXTX]
+;   sub sp, sp, #4096
+;   str wzr, [sp]
+;   sub sp, sp, #4096
+;   str wzr, [sp]
+;   sub sp, sp, #4096
+;   str wzr, [sp]
+;   add sp, sp, #12288
 ;   sub sp, sp, #12288
 ; block0:
 ;   mov x0, sp
@@ -61,14 +65,15 @@ block0:
 ; block0: ; offset 0x0
 ;   stp x29, x30, [sp, #-0x10]!
 ;   mov x29, sp
-;   mov x16, #-0x1000
-;   str wzr, [sp, x16, sxtx]
-;   mov x16, #-0x2000
-;   str wzr, [sp, x16, sxtx]
-;   mov x16, #-0x3000
-;   str wzr, [sp, x16, sxtx]
+;   sub sp, sp, #1, lsl #12
+;   stur wzr, [sp]
+;   sub sp, sp, #1, lsl #12
+;   stur wzr, [sp]
+;   sub sp, sp, #1, lsl #12
+;   stur wzr, [sp]
+;   add sp, sp, #3, lsl #12
 ;   sub sp, sp, #3, lsl #12
-; block1: ; offset 0x24
+; block1: ; offset 0x28
 ;   mov x0, sp
 ;   add sp, sp, #3, lsl #12
 ;   ldp x29, x30, [sp], #0x10
diff --git a/cranelift/filetests/filetests/isa/riscv64/c-inline-probestack.clif b/cranelift/filetests/filetests/isa/riscv64/c-inline-probestack.clif
new file mode 100644
index 000000000..6d94c2f81
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/riscv64/c-inline-probestack.clif
@@ -0,0 +1,162 @@
+test compile precise-output
+set enable_probestack=true
+set probestack_strategy=inline
+; This is the default and is equivalent to a page size of 4096
+set probestack_size_log2=12
+target riscv64 has_c
+
+
+; If the stack size is just one page, we can avoid the stack probe entirely
+function %single_page() -> i64 system_v {
+ss0 = explicit_slot 2048
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   addi sp,sp,-2048
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,1
+;   addi t6,t6,-2048
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   c.addi16sp sp, -0x10
+;   c.sdsp ra, 8(sp)
+;   c.sdsp s0, 0(sp)
+;   c.mv s0, sp
+;   addi sp, sp, -0x800
+; block1: ; offset 0xc
+;   c.mv a0, sp
+;   c.lui t6, 1
+;   addi t6, t6, -0x800
+;   add sp, t6, sp
+;   c.ldsp ra, 8(sp)
+;   c.ldsp s0, 0(sp)
+;   c.addi16sp sp, 0x10
+;   c.jr ra
+
+function %unrolled() -> i64 system_v {
+ss0 = explicit_slot 12288
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   lui t3,-1
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   lui t6,3
+;   add sp,t6,sp
+;   lui t6,-3
+;   add sp,t6,sp
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,3
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   c.addi16sp sp, -0x10
+;   c.sdsp ra, 8(sp)
+;   c.sdsp s0, 0(sp)
+;   c.mv s0, sp
+;   c.lui t3, 0xfffff
+;   c.add sp, t3
+;   c.swsp zero, 0(sp)
+;   c.add sp, t3
+;   c.swsp zero, 0(sp)
+;   c.add sp, t3
+;   c.swsp zero, 0(sp)
+;   c.lui t6, 3
+;   add sp, t6, sp
+;   c.lui t6, 0xffffd
+;   add sp, t6, sp
+; block1: ; offset 0x22
+;   c.mv a0, sp
+;   c.lui t6, 3
+;   add sp, t6, sp
+;   c.ldsp ra, 8(sp)
+;   c.ldsp s0, 0(sp)
+;   c.addi16sp sp, 0x10
+;   c.jr ra
+
+function %large() -> i64 system_v {
+ss0 = explicit_slot 100000
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   inline_stack_probe##guard_size=4096 probe_count=25 tmp=t3
+;   lui t6,-24
+;   addi t6,t6,-1696
+;   add sp,t6,sp
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,24
+;   addi t6,t6,1696
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   c.addi16sp sp, -0x10
+;   c.sdsp ra, 8(sp)
+;   c.sdsp s0, 0(sp)
+;   c.mv s0, sp
+;   c.lui t6, 0x19
+;   c.lui t3, 1
+;   bgeu t3, t6, 0x12
+;   sub t5, sp, t6
+;   sb zero, 0(t5)
+;   sub t6, t6, t3
+;   c.j -0x10
+;   c.lui t6, 0xfffe8
+;   addi t6, t6, -0x6a0
+;   add sp, t6, sp
+; block1: ; offset 0x28
+;   c.mv a0, sp
+;   c.lui t6, 0x18
+;   addi t6, t6, 0x6a0
+;   add sp, t6, sp
+;   c.ldsp ra, 8(sp)
+;   c.ldsp s0, 0(sp)
+;   c.addi16sp sp, 0x10
+;   c.jr ra
+
diff --git a/cranelift/filetests/filetests/isa/riscv64/inline-probestack.clif b/cranelift/filetests/filetests/isa/riscv64/inline-probestack.clif
new file mode 100644
index 000000000..6b8177668
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/riscv64/inline-probestack.clif
@@ -0,0 +1,162 @@
+test compile precise-output
+set enable_probestack=true
+set probestack_strategy=inline
+; This is the default and is equivalent to a page size of 4096
+set probestack_size_log2=12
+target riscv64
+
+
+; If the stack size is just one page, we can avoid the stack probe entirely
+function %single_page() -> i64 system_v {
+ss0 = explicit_slot 2048
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   addi sp,sp,-2048
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,1
+;   addi t6,t6,-2048
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   addi sp, sp, -0x10
+;   sd ra, 8(sp)
+;   sd s0, 0(sp)
+;   mv s0, sp
+;   addi sp, sp, -0x800
+; block1: ; offset 0x14
+;   mv a0, sp
+;   lui t6, 1
+;   addi t6, t6, -0x800
+;   add sp, t6, sp
+;   ld ra, 8(sp)
+;   ld s0, 0(sp)
+;   addi sp, sp, 0x10
+;   ret
+
+function %unrolled() -> i64 system_v {
+ss0 = explicit_slot 12288
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   lui t3,-1
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   add sp,sp,t3
+;   sw zero,0(sp)
+;   lui t6,3
+;   add sp,t6,sp
+;   lui t6,-3
+;   add sp,t6,sp
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,3
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   addi sp, sp, -0x10
+;   sd ra, 8(sp)
+;   sd s0, 0(sp)
+;   mv s0, sp
+;   lui t3, 0xfffff
+;   add sp, sp, t3
+;   sw zero, 0(sp)
+;   add sp, sp, t3
+;   sw zero, 0(sp)
+;   add sp, sp, t3
+;   sw zero, 0(sp)
+;   lui t6, 3
+;   add sp, t6, sp
+;   lui t6, 0xffffd
+;   add sp, t6, sp
+; block1: ; offset 0x3c
+;   mv a0, sp
+;   lui t6, 3
+;   add sp, t6, sp
+;   ld ra, 8(sp)
+;   ld s0, 0(sp)
+;   addi sp, sp, 0x10
+;   ret
+
+function %large() -> i64 system_v {
+ss0 = explicit_slot 100000
+
+block0:
+  v1 = stack_addr.i64 ss0
+  return v1
+}
+
+; VCode:
+;   addi sp,sp,-16
+;   sd ra,8(sp)
+;   sd fp,0(sp)
+;   mv fp,sp
+;   inline_stack_probe##guard_size=4096 probe_count=25 tmp=t3
+;   lui t6,-24
+;   addi t6,t6,-1696
+;   add sp,t6,sp
+; block0:
+;   load_addr a0,0(nominal_sp)
+;   lui t6,24
+;   addi t6,t6,1696
+;   add sp,t6,sp
+;   ld ra,8(sp)
+;   ld fp,0(sp)
+;   addi sp,sp,16
+;   ret
+;
+; Disassembled:
+; block0: ; offset 0x0
+;   addi sp, sp, -0x10
+;   sd ra, 8(sp)
+;   sd s0, 0(sp)
+;   mv s0, sp
+;   lui t6, 0x19
+;   lui t3, 1
+;   bgeu t3, t6, 0x14
+;   sub t5, sp, t6
+;   sb zero, 0(t5)
+;   sub t6, t6, t3
+;   j -0x10
+;   lui t6, 0xfffe8
+;   addi t6, t6, -0x6a0
+;   add sp, t6, sp
+; block1: ; offset 0x38
+;   mv a0, sp
+;   lui t6, 0x18
+;   addi t6, t6, 0x6a0
+;   add sp, t6, sp
+;   ld ra, 8(sp)
+;   ld s0, 0(sp)
+;   addi sp, sp, 0x10
+;   ret
+
diff --git a/cranelift/filetests/filetests/isa/riscv64/wasm/zbb.wat b/cranelift/filetests/filetests/isa/riscv64/wasm/zbb.wat
index e307e6a6a..f0cc3f4cc 100644
--- a/cranelift/filetests/filetests/isa/riscv64/wasm/zbb.wat
+++ b/cranelift/filetests/filetests/isa/riscv64/wasm/zbb.wat
@@ -76,7 +76,7 @@
 ;; block0:
 ;;   j label1
 ;; block1:
-;;   roriw a0,a0,100
+;;   roriw a0,a0,4
 ;;   ret
 ;;
 ;; function u0:7:
diff --git a/cranelift/filetests/filetests/isa/x64/inline-probestack-large.clif b/cranelift/filetests/filetests/isa/x64/inline-probestack-large.clif
index adeab023c..c2193fd97 100644
--- a/cranelift/filetests/filetests/isa/x64/inline-probestack-large.clif
+++ b/cranelift/filetests/filetests/isa/x64/inline-probestack-large.clif
@@ -50,9 +50,13 @@ block0:
 ; VCode:
 ;   pushq   %rbp
 ;   movq    %rsp, %rbp
-;   movl    %esp, -65536(%rsp)
-;   movl    %esp, -131072(%rsp)
-;   movl    %esp, -196608(%rsp)
+;   subq    %rsp, $65536, %rsp
+;   movl    %esp, 0(%rsp)
+;   subq    %rsp, $65536, %rsp
+;   movl    %esp, 0(%rsp)
+;   subq    %rsp, $65536, %rsp
+;   movl    %esp, 0(%rsp)
+;   addq    %rsp, $196608, %rsp
 ;   subq    %rsp, $196608, %rsp
 ; block0:
 ;   lea     rsp(0 + virtual offset), %rax
@@ -65,11 +69,15 @@ block0:
 ; block0: ; offset 0x0
 ;   pushq %rbp
 ;   movq %rsp, %rbp
-;   movl %esp, -0x10000(%rsp)
-;   movl %esp, -0x20000(%rsp)
-;   movl %esp, -0x30000(%rsp)
+;   subq $0x10000, %rsp
+;   movl %esp, (%rsp)
+;   subq $0x10000, %rsp
+;   movl %esp, (%rsp)
+;   subq $0x10000, %rsp
+;   movl %esp, (%rsp)
+;   addq $0x30000, %rsp
 ;   subq $0x30000, %rsp
-; block1: ; offset 0x20
+; block1: ; offset 0x30
 ;   leaq (%rsp), %rax
 ;   addq $0x30000, %rsp
 ;   movq %rbp, %rsp
diff --git a/cranelift/filetests/filetests/isa/x64/inline-probestack.clif b/cranelift/filetests/filetests/isa/x64/inline-probestack.clif
index c55c87000..afb5b93fe 100644
--- a/cranelift/filetests/filetests/isa/x64/inline-probestack.clif
+++ b/cranelift/filetests/filetests/isa/x64/inline-probestack.clif
@@ -49,9 +49,13 @@ block0:
 ; VCode:
 ;   pushq   %rbp
 ;   movq    %rsp, %rbp
-;   movl    %esp, -4096(%rsp)
-;   movl    %esp, -8192(%rsp)
-;   movl    %esp, -12288(%rsp)
+;   subq    %rsp, $4096, %rsp
+;   movl    %esp, 0(%rsp)
+;   subq    %rsp, $4096, %rsp
+;   movl    %esp, 0(%rsp)
+;   subq    %rsp, $4096, %rsp
+;   movl    %esp, 0(%rsp)
+;   addq    %rsp, $12288, %rsp
 ;   subq    %rsp, $12288, %rsp
 ; block0:
 ;   lea     rsp(0 + virtual offset), %rax
@@ -64,11 +68,15 @@ block0:
 ; block0: ; offset 0x0
 ;   pushq %rbp
 ;   movq %rsp, %rbp
-;   movl %esp, -0x1000(%rsp)
-;   movl %esp, -0x2000(%rsp)
-;   movl %esp, -0x3000(%rsp)
+;   subq $0x1000, %rsp
+;   movl %esp, (%rsp)
+;   subq $0x1000, %rsp
+;   movl %esp, (%rsp)
+;   subq $0x1000, %rsp
+;   movl %esp, (%rsp)
+;   addq $0x3000, %rsp
 ;   subq $0x3000, %rsp
-; block1: ; offset 0x20
+; block1: ; offset 0x30
 ;   leaq (%rsp), %rax
 ;   addq $0x3000, %rsp
 ;   movq %rbp, %rsp
diff --git a/winch/filetests/filetests/x64/unreachable/with_spilled_local.wat b/winch/filetests/filetests/x64/unreachable/with_spilled_local.wat
new file mode 100644
index 000000000..66848d1ab
--- /dev/null
+++ b/winch/filetests/filetests/x64/unreachable/with_spilled_local.wat
@@ -0,0 +1,24 @@
+;;! target = "x86_64"
+
+(module
+  (func (export "")
+    (local i32)
+    local.get 0
+    block
+    end
+    unreachable
+  )
+)
+;;    0:	 55                   	push	rbp
+;;    1:	 4889e5               	mov	rbp, rsp
+;;    4:	 4883ec10             	sub	rsp, 0x10
+;;    8:	 48c744240800000000   	
+;; 				mov	qword ptr [rsp + 8], 0
+;;   11:	 4c89742404           	mov	qword ptr [rsp + 4], r14
+;;   16:	 448b5c240c           	mov	r11d, dword ptr [rsp + 0xc]
+;;   1b:	 4153                 	push	r11
+;;   1d:	 0f0b                 	ud2	
+;;   1f:	 4883c408             	add	rsp, 8
+;;   23:	 4883c410             	add	rsp, 0x10
+;;   27:	 5d                   	pop	rbp
+;;   28:	 c3                   	ret	
diff --git a/winch/filetests/filetests/x64/unreachable/with_spilled_local_in_if.wat b/winch/filetests/filetests/x64/unreachable/with_spilled_local_in_if.wat
new file mode 100644
index 000000000..b201d1748
--- /dev/null
+++ b/winch/filetests/filetests/x64/unreachable/with_spilled_local_in_if.wat
@@ -0,0 +1,32 @@
+;;! target = "x86_64"
+
+(module
+  (func (export "")
+    (local i32)
+    local.get 0
+    if
+      local.get 0
+      block
+      end
+      unreachable
+    else
+      nop
+    end
+  )
+)
+;;    0:	 55                   	push	rbp
+;;    1:	 4889e5               	mov	rbp, rsp
+;;    4:	 4883ec10             	sub	rsp, 0x10
+;;    8:	 48c744240800000000   	
+;; 				mov	qword ptr [rsp + 8], 0
+;;   11:	 4c89742404           	mov	qword ptr [rsp + 4], r14
+;;   16:	 8b44240c             	mov	eax, dword ptr [rsp + 0xc]
+;;   1a:	 85c0                 	test	eax, eax
+;;   1c:	 0f840d000000         	je	0x2f
+;;   22:	 448b5c240c           	mov	r11d, dword ptr [rsp + 0xc]
+;;   27:	 4153                 	push	r11
+;;   29:	 0f0b                 	ud2	
+;;   2b:	 4883c408             	add	rsp, 8
+;;   2f:	 4883c410             	add	rsp, 0x10
+;;   33:	 5d                   	pop	rbp
+;;   34:	 c3                   	ret