[X86][Win64] Avoid statepoints prior to SEH epilogue

[zero9178]

Summary

This patchs purpose is very similar to https://reviews.llvm.org/D119644

The gist of the issue is that SEH unwinding has certain invariants around call instructions. One of those is that a call instruction must not be immediately followed by the function epilogue. Failing to do so leads to Windows' unwinder not recognizing the frame and skipping it when unwinding the stack.

LLVM ensures this invariant by inserting a noop after a call prior to an epilogue. The implementation however, makes the unfortunate assumption that pseudo instructions may not be calls, leading to statepoints being skipped and no noop being inserted.
This patch fixes that issue by only skipping over pseudo instructions that aren't calls.

[mstorsjo]

The gist of the issue is that SEH unwinding has certain invariants around call instructions. One of those is that a call instruction must not be immediately followed by the function epilogue. Failing to do so leads to Windows' unwinder not recognizing the frame and skipping it when unwinding the stack.

Hmm, I was aware that this was an issue if the function ended with a non-returning call, like the testcase added in D119644, but I wasn't aware that this was an issue also for a call directly followed by the epilogue in a regular function. I would expect that to happen a lot more often.

(I'm not contradicting the statement here - I'm just not very experienced with the x86_64 SEH cases. For ARM/ARM64 SEH, these details are much more tightly defined, and such tweaks aren't luckily really needed there.)

I think the change looks reasonable, but I'm not really very familiar in this layer. I guess @rnk would be the best authority here, but he's away... Adding @efriedma who usually is knowledgeable on these things.

[efriedma]

LGTM

Hmm, I was aware that this was an issue if the function ended with a non-returning call, like the testcase added in D119644, but I wasn't aware that this was an issue also for a call directly followed by the epilogue in a regular function. I would expect that to happen a lot more often.

If it's actually significant to codesize, we could look into reducing the number of cases where we emit these nops. As far as I know, we really only need the padding in the case where a function has a corresponding EH pad. (Compare MSVC vs clang for void f(int*); void g1() { int z; f(&z); f(&z); } void g2() { int z; try { f(&z); f(&z); } catch (...){} }.)

llvm/lib/Target/X86/X86MCInstLower.cpp

void X86AsmPrinter::emitInstruction(const MachineInstr *MI) {
 
   case X86::SEH_Epilogue: {
     assert(MF->hasWinCFI() && "SEH_ instruction in function without WinCFI?");
     MachineBasicBlock::const_iterator MBBI(MI);
     // Check if preceded by a call and emit nop if so.
     for (MBBI = PrevCrossBBInst(MBBI);
          MBBI != MachineBasicBlock::const_iterator();
          MBBI = PrevCrossBBInst(MBBI)) {
-      // Conservatively assume that pseudo instructions don't emit code and keep
-      // looking for a call. We may emit an unnecessary nop in some cases.
-      if (!MBBI->isPseudo()) {
+      // Pseudo instructions that aren't a call are assumed to not emit any
+      // code. If they do, we worst case generate unnecessary noops after a
+      // call.
+      if (MBBI->isCall() || !MBBI->isPseudo()) {
         if (MBBI->isCall())
           EmitAndCountInstruction(MCInstBuilder(X86::NOOP));
         break;
       }
     }
     return;
   }
   case X86::UBSAN_UD1:

llvm/test/CodeGen/X86/win64-seh-epilogue-statepoint.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-windows-gnu %s -o - | FileCheck %s
 
 define i32 @foobar() gc "statepoint-example" personality ptr @__gxx_personality_seh0 {
 ; CHECK-LABEL: foobar:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    subq $40, %rsp
 ; CHECK-NEXT:    .seh_stackalloc 40
 ; CHECK-NEXT:    .seh_endprologue
 ; CHECK-NEXT:    callq bar
 ; CHECK-NEXT:  .Ltmp0:
+; CHECK-NEXT:    nop
 ; CHECK-NEXT:    addq $40, %rsp
 ; CHECK-NEXT:    retq
 ; CHECK-NEXT:    .seh_endproc
     %statepoint_token = call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 2882400000, i32 0, ptr elementtype(i32 ()) @bar, i32 0, i32 0, i32 0, i32 0)
     %res = call i32 @llvm.experimental.gc.result.i32(token %statepoint_token)
     ret i32 %res
 }
 
 declare i32 @bar()
 
 declare dso_local i32 @__gxx_personality_seh0(...)
 
 declare token @llvm.experimental.gc.statepoint.p0(i64 immarg, i32 immarg, ptr, i32 immarg, i32 immarg, ...)
 declare i32 @llvm.experimental.gc.result.i32(token)