Unwind: separate the "set frame reg" and "define new frame" operations to fix metadata bug related to stack checks.

.github/workflows/main.yml

@@ -219,7 +219,8 @@ jobs:
           # defaults to x86_64-apple-darwin
           - os: macos-latest
           - os: windows-2019
-          - os: windows-2019
+          - os: windows-2022
+          - os: windows-2022
             target: x86_64-pc-windows-gnu
           - os: ubuntu-latest
             target: aarch64-unknown-linux-gnu

cranelift/codegen/src/isa/aarch64/abi.rs

@@ -671,6 +671,13 @@ impl ABIMachineSpec for AArch64MachineDeps {
                 shift12: false,
             },
         });
+
+        if flags.unwind_info() {
+            insts.push(Inst::Unwind {
+                inst: UnwindInst::SetFrameReg,
+            });
+        }
+
         insts
     }
 

cranelift/codegen/src/isa/unwind.rs

@@ -118,8 +118,8 @@ pub enum UnwindInfo {
 /// push rbp
 /// unwind UnwindInst::PushFrameRegs { offset_upward_to_caller_sp: 16 }
 /// mov rbp, rsp
-/// unwind UnwindInst::DefineNewFrame { offset_upward_to_caller_sp: 16,
-///                                     offset_downward_to_clobbers: 16 }
+/// unwind UnwindInst::SetFrameReg { offset_upward_to_caller_sp: 16 }
+/// unwind UnwindInst::DefineNewFrame { offset_downward_to_clobbers: 16 }
 /// sub rsp, 32
 /// mov [rsp+16], r12
 /// unwind UnwindInst::SaveReg { reg: R12, clobber_offset: 0 }
@@ -143,15 +143,20 @@ pub enum UnwindInst {
     },
     /// The frame-pointer register for this architecture has just been
     /// set to the current stack location. We wish to define a new
-    /// frame that is anchored on this new FP value. Offsets are provided
-    /// upward to the caller's stack frame and downward toward the clobber
-    /// area. We expect this pseudo-op to come after `PushFrameRegs`.
+    /// frame that is anchored on this new FP value. We expect this
+    /// pseudo-op to come after `PushFrameRegs`.
+    SetFrameReg,
+    /// We now know the size of clobbers, if any, and can define a
+    /// frame in which we will give their offsets. Offsets are
+    /// provided upward to the caller's stack frame and downward
+    /// toward the clobber area. We expect this pseudo op to come
+    /// after `SetFrameReg`.
     DefineNewFrame {
-        /// The offset from the current SP and FP value upward to the value of
-        /// SP at the callsite that invoked us.
+        /// The offset from the current SP and FP value upward to the
+        /// value of SP at the callsite that invoked us.
         offset_upward_to_caller_sp: u32,
-        /// The offset from the current SP and FP value downward to the start of
-        /// the clobber area.
+        /// The offset from the current SP and FP value downward to
+        /// the start of the clobber area.
         offset_downward_to_clobbers: u32,
     },
     /// The stack pointer was adjusted to allocate the stack.

cranelift/codegen/src/isa/unwind/systemv.rs

@@ -203,18 +203,21 @@ pub(crate) fn create_unwind_info_from_insts<MR: RegisterMapper<Reg>>(
                     ));
                 }
             }
-            &UnwindInst::DefineNewFrame {
-                offset_upward_to_caller_sp,
-                offset_downward_to_clobbers,
-            } => {
+            &UnwindInst::SetFrameReg => {
                 // Define CFA in terms of FP. Note that we assume it was already
                 // defined correctly in terms of the current SP, and FP has just
                 // been set to the current SP, so we do not need to change the
                 // offset, only the register.  (This is done only if the target
                 // defines a frame pointer register.)
-                if let Some(fp) = mr.fp() {
-                    instructions.push((instruction_offset, CallFrameInstruction::CfaRegister(fp)));
-                }
+                instructions.push((
+                    instruction_offset,
+                    CallFrameInstruction::CfaRegister(mr.fp().unwrap()),
+                ));
+            }
+            &UnwindInst::DefineNewFrame {
+                offset_upward_to_caller_sp,
+                offset_downward_to_clobbers,
+            } => {
                 // Record initial CFA offset.  This will be used with later
                 // StackAlloc calls if we do not have a frame pointer.
                 cfa_offset = offset_upward_to_caller_sp;

cranelift/codegen/src/isa/unwind/winx64.rs

@@ -277,12 +277,14 @@ pub(crate) fn create_unwind_info_from_insts<MR: RegisterMapper<crate::machinst::
                     reg: UNWIND_RBP_REG,
                 });
             }
+            &UnwindInst::SetFrameReg => {
+                unwind_codes.push(UnwindCode::SetFPReg { instruction_offset });
+            }
             &UnwindInst::DefineNewFrame {
                 offset_downward_to_clobbers,
                 ..
             } => {
                 frame_register_offset = ensure_unwind_offset(offset_downward_to_clobbers)?;
-                unwind_codes.push(UnwindCode::SetFPReg { instruction_offset });
             }
             &UnwindInst::StackAlloc { size } => {
                 unwind_codes.push(UnwindCode::StackAlloc {

cranelift/codegen/src/isa/x64/abi.rs

@@ -463,6 +463,13 @@ impl ABIMachineSpec for X64ABIMachineSpec {
         // `mov %rsp, %rbp`
         // RSP is now 0 % 16
         insts.push(Inst::mov_r_r(OperandSize::Size64, r_rsp, w_rbp));
+
+        if flags.unwind_info() {
+            insts.push(Inst::Unwind {
+                inst: UnwindInst::SetFrameReg,
+            });
+        }
+
         insts
     }
 
@@ -530,6 +537,7 @@ impl ABIMachineSpec for X64ABIMachineSpec {
                 Writable::from_reg(regs::rsp()),
             ));
         }
+
         // Store each clobbered register in order at offsets from RSP,
         // placing them above the fixed frame slots.
         let mut cur_offset = fixed_frame_storage_size;

cranelift/filetests/filetests/isa/x64/fastcall.clif

@@ -11,6 +11,7 @@ block0(v0: i64, v1: i64, v2: i64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    %rcx, %rax
@@ -26,6 +27,7 @@ block0(v0: i64, v1: i64, v2: i64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    %rdx, %rax
@@ -41,6 +43,7 @@ block0(v0: i64, v1: i64, v2: i64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    %r8, %rax
@@ -56,6 +59,7 @@ block0(v0: i64, v1: i64, v2: i64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    %r9, %rax
@@ -71,6 +75,7 @@ block0(v0: i64, v1: i64, v2: f64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movdqa  %xmm2, %xmm0
@@ -86,6 +91,7 @@ block0(v0: i64, v1: i64, v2: f64, v3: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    %r9, %rax
@@ -111,6 +117,7 @@ block0(v0: i64, v1: i64, v2: i64, v3: i64, v4: i64, v5: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    48(%rbp), %r11
@@ -127,6 +134,7 @@ block0(v0: i128, v1: i64, v2: i128, v3: i128):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   movq    48(%rbp), %r11
@@ -149,6 +157,7 @@ block0(v0: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
 ; block0:
 ;   cvtsi2sd %rcx, %xmm2
@@ -220,6 +229,7 @@ block0(v0: i64):
 ;   pushq   %rbp
 ;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
 ;   movq    %rsp, %rbp
+;   unwind SetFrameReg
 ;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 144 }
 ;   subq    %rsp, $240, %rsp
 ;   movdqu  %xmm6, 96(%rsp)
@@ -305,3 +315,30 @@ block0(v0: i64):
 ;   popq    %rbp
 ;   ret
 
+function %f10(i64 vmctx, i64, i64, i64) -> i64 windows_fastcall {
+    gv0 = vmctx
+    gv1 = load.i64 notrap aligned gv0
+    stack_limit = gv1
+    ss0 = explicit_slot 20
+
+block0(v0: i64, v1: i64, v2: i64, v3: i64):
+  return v0
+}
+
+;   pushq   %rbp
+;   unwind PushFrameRegs { offset_upward_to_caller_sp: 16 }
+;   movq    %rsp, %rbp
+;   unwind SetFrameReg
+;   movq    0(%rcx), %r10
+;   addq    %r10, $32, %r10
+;   cmpq    %rsp, %r10
+;   jbe ; ud2 stk_ovf ;
+;   unwind DefineNewFrame { offset_upward_to_caller_sp: 16, offset_downward_to_clobbers: 0 }
+;   subq    %rsp, $32, %rsp
+; block0:
+;   movq    %rcx, %rax
+;   addq    %rsp, $32, %rsp
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+