Revert [Windows] Disable TrapUnreachable for Win64, add SEH_NoReturn

This reverts r370525 (git commit 0bb1630685fba255fa93def92603f064c2ffd203) Also reverts r370543 (git commit 185ddc08eed6542781040b8499ef7ad15c8ae9f4) The approach I took only works for functions marked `noreturn`. In general, a call that is not known to be noreturn may be followed by unreachable for other reasons. For example, there could be multiple call sites to a function that throws sometimes, and at some call sites, it is known to always throw, so it is followed by unreachable. We need to insert an `int3` in these cases to pacify the Windows unwinder. I think this probably deserves its own standalone, Win64-only fixup pass that runs after block placement. Implementing that will take some time, so let's revert to TrapUnreachable in the mean time. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370829 91177308-0d34-0410-b5e6-96231b3b80d8
earl · Sep 3, 2019 · 798169e · 798169e
1 parent cb0fddb
commit 798169e
Show file tree

Hide file tree

Showing 21 changed files with 38 additions and 145 deletions.
diff --git a/lib/Target/X86/X86FrameLowering.cpp b/lib/Target/X86/X86FrameLowering.cpp
@@ -2558,7 +2558,7 @@ bool blockEndIsUnreachable(const MachineBasicBlock &MBB,
              MBB.succ_begin(), MBB.succ_end(),
              [](const MachineBasicBlock *Succ) { return Succ->isEHPad(); }) &&
          std::all_of(MBBI, MBB.end(), [](const MachineInstr &MI) {
-           return MI.isMetaInstruction() || MI.getOpcode() == X86::SEH_NoReturn;
+           return MI.isMetaInstruction();
          });
 }
 

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
@@ -4101,17 +4101,6 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
     InFlag = Chain.getValue(1);
   }
 
-  // Insert a pseudo instruction after noreturn calls that expands to int3 if
-  // this would be the last instruction in the funclet. If the return address of
-  // a call refers to the last PC of a function, the Windows SEH machinery can
-  // get confused about which function or scope the return address belongs to.
-  // MSVC inserts int3 after every noreturn function call, but LLVM only places
-  // them when it would cause a problem otherwise.
-  if (CLI.DoesNotReturn && Subtarget.isTargetWin64()) {
-    Chain = DAG.getNode(X86ISD::SEH_NORETURN, dl, NodeTys, Chain, InFlag);
-    InFlag = Chain.getValue(1);
-  }
-
   // Handle result values, copying them out of physregs into vregs that we
   // return.
   return LowerCallResult(Chain, InFlag, CallConv, isVarArg, Ins, dl, DAG,
@@ -28729,7 +28718,6 @@ const char *X86TargetLowering::getTargetNodeName(unsigned Opcode) const {
   case X86ISD::VASTART_SAVE_XMM_REGS: return "X86ISD::VASTART_SAVE_XMM_REGS";
   case X86ISD::VAARG_64:           return "X86ISD::VAARG_64";
   case X86ISD::WIN_ALLOCA:         return "X86ISD::WIN_ALLOCA";
-  case X86ISD::SEH_NORETURN:       return "X86ISD::SEH_NORETURN";
   case X86ISD::MEMBARRIER:         return "X86ISD::MEMBARRIER";
   case X86ISD::MFENCE:             return "X86ISD::MFENCE";
   case X86ISD::SEG_ALLOCA:         return "X86ISD::SEG_ALLOCA";

diff --git a/lib/Target/X86/X86ISelLowering.h b/lib/Target/X86/X86ISelLowering.h
@@ -531,9 +531,6 @@ namespace llvm {
       // Windows's _chkstk call to do stack probing.
       WIN_ALLOCA,
 
-      // Expands to int3 or nothing, depending on basic block layout.
-      SEH_NORETURN,
-
       // For allocating variable amounts of stack space when using
       // segmented stacks. Check if the current stacklet has enough space, and
       // falls back to heap allocation if not.

diff --git a/lib/Target/X86/X86InstrCompiler.td b/lib/Target/X86/X86InstrCompiler.td
@@ -239,9 +239,6 @@ let isPseudo = 1, SchedRW = [WriteSystem] in {
                             "#SEH_EndPrologue", []>;
   def SEH_Epilogue : I<0, Pseudo, (outs), (ins),
                             "#SEH_Epilogue", []>;
-  let hasSideEffects = 1 in
-    def SEH_NoReturn : I<0, Pseudo, (outs), (ins),
-                         "#SEH_NoReturn", [(X86SehNoReturn)]>;
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/Target/X86/X86InstrInfo.td b/lib/Target/X86/X86InstrInfo.td
@@ -289,9 +289,6 @@ def X86mul_imm : SDNode<"X86ISD::MUL_IMM", SDTIntBinOp>;
 def X86WinAlloca : SDNode<"X86ISD::WIN_ALLOCA", SDT_X86WIN_ALLOCA,
                           [SDNPHasChain, SDNPOutGlue]>;
 
-def X86SehNoReturn : SDNode<"X86ISD::SEH_NORETURN", SDTX86Void,
-                            [SDNPHasChain, SDNPInGlue, SDNPOutGlue]>;
-
 def X86SegAlloca : SDNode<"X86ISD::SEG_ALLOCA", SDT_X86SEG_ALLOCA,
                           [SDNPHasChain]>;
 

diff --git a/lib/Target/X86/X86MCInstLower.cpp b/lib/Target/X86/X86MCInstLower.cpp
@@ -1921,20 +1921,6 @@ void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) {
     return;
   }
 
-  case X86::SEH_NoReturn: {
-    // Materialize an int3 if this instruction is in the last basic block in the
-    // function. The int3 serves the same purpose as the noop emitted above for
-    // SEH_Epilogue, which is to make the Win64 unwinder happy. If the return
-    // address of the preceding call appears to precede an epilogue or a new
-    // function, then the unwinder may get lost.
-    const MachineBasicBlock *MBB = MI->getParent();
-    const MachineBasicBlock *NextMBB = MBB->getNextNode();
-    if (!NextMBB || NextMBB->isEHPad()) {
-      EmitAndCountInstruction(MCInstBuilder(X86::INT3));
-    }
-    return;
-  }
-
   // Lower PSHUFB and VPERMILP normally but add a comment if we can find
   // a constant shuffle mask. We won't be able to do this at the MC layer
   // because the mask isn't an immediate.

diff --git a/lib/Target/X86/X86TargetMachine.cpp b/lib/Target/X86/X86TargetMachine.cpp
@@ -219,9 +219,17 @@ X86TargetMachine::X86TargetMachine(const Target &T, const Triple &TT,
           getEffectiveX86CodeModel(CM, JIT, TT.getArch() == Triple::x86_64),
           OL),
       TLOF(createTLOF(getTargetTriple())) {
+  // Windows stack unwinder gets confused when execution flow "falls through"
+  // after a call to 'noreturn' function.
+  // To prevent that, we emit a trap for 'unreachable' IR instructions.
+  // (which on X86, happens to be the 'ud2' instruction)
   // On PS4, the "return address" of a 'noreturn' call must still be within
   // the calling function, and TrapUnreachable is an easy way to get that.
-  if (TT.isPS4() || TT.isOSBinFormatMachO()) {
+  // The check here for 64-bit windows is a bit icky, but as we're unlikely
+  // to ever want to mix 32 and 64-bit windows code in a single module
+  // this should be fine.
+  if ((TT.isOSWindows() && TT.getArch() == Triple::x86_64) || TT.isPS4() ||
+      TT.isOSBinFormatMachO()) {
     this->Options.TrapUnreachable = true;
     this->Options.NoTrapAfterNoreturn = TT.isOSBinFormatMachO();
   }

diff --git a/test/CodeGen/WinEH/wineh-noret-cleanup.ll b/test/CodeGen/WinEH/wineh-noret-cleanup.ll
@@ -1,3 +1,4 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: sed -e s/.Cxx:// %s | llc -mtriple=x86_64-pc-windows-msvc | FileCheck %s --check-prefix=CXX
 ; RUN: sed -e s/.Seh:// %s | llc -mtriple=x86_64-pc-windows-msvc | FileCheck %s --check-prefix=SEH
 
@@ -68,13 +69,13 @@ catch.body.2:
 ; SEH-NEXT:    .long   .Ltmp0@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp1@IMGREL+1
 ; SEH-NEXT:    .long   dummy_filter@IMGREL
-; SEH-NEXT:    .long   .LBB0_5@IMGREL
+; SEH-NEXT:    .long   .LBB0_2@IMGREL
 ; SEH-NEXT:    .long   .Ltmp2@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp3@IMGREL+1
-; SEH-NEXT:    .long   "?dtor$2@?0?test@4HA"@IMGREL
+; SEH-NEXT:    .long   "?dtor$5@?0?test@4HA"@IMGREL 
 ; SEH-NEXT:    .long   0
 ; SEH-NEXT:    .long   .Ltmp2@IMGREL+1
 ; SEH-NEXT:    .long   .Ltmp3@IMGREL+1
 ; SEH-NEXT:    .long   dummy_filter@IMGREL
-; SEH-NEXT:    .long   .LBB0_5@IMGREL
+; SEH-NEXT:    .long   .LBB0_2@IMGREL
 ; SEH-NEXT:  .Llsda_end0:
diff --git a/test/CodeGen/X86/br-fold.ll b/test/CodeGen/X86/br-fold.ll
@@ -5,18 +5,18 @@
 ; RUN: llc -mtriple=x86_64-scei-ps4 < %s | FileCheck -check-prefix=PS4 %s
 
 ; X64_DARWIN: orq
-; X64_DARWIN-NEXT: ud2
+; X64-DARWIN-NEXT: ud2
 
 ; X64_LINUX: orq %rax, %rcx
 ; X64_LINUX-NEXT: jne
 ; X64_LINUX-NEXT: %bb8.i329
 
 ; X64_WINDOWS: orq %rax, %rcx
-; X64_WINDOWS-NEXT: jne
+; X64_WINDOWS-NEXT: ud2
 
 ; X64_WINDOWS_GNU: movq .refptr._ZN11xercesc_2_513SchemaSymbols21fgURI_SCHEMAFORSCHEMAE(%rip), %rax
 ; X64_WINDOWS_GNU: orq .refptr._ZN11xercesc_2_56XMLUni16fgNotationStringE(%rip), %rax
-; X64_WINDOWS_GNU-NEXT: jne
+; X64_WINDOWS_GNU-NEXT: ud2
 
 ; PS4: orq %rax, %rcx
 ; PS4-NEXT: ud2

diff --git a/test/CodeGen/X86/catchpad-lifetime.ll b/test/CodeGen/X86/catchpad-lifetime.ll
@@ -7,8 +7,6 @@ declare void @throw()
 
 declare i32 @__CxxFrameHandler3(...)
 
-declare void @llvm.trap()
-
 define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   %alloca2 = alloca i8*, align 4
@@ -32,7 +30,6 @@ catch.pad:                                        ; preds = %catch.dispatch
   %bc2 = bitcast i8** %alloca2 to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %bc2)
   store volatile i8* null, i8** %alloca1
-  call void @llvm.trap()
   unreachable
 
 ; CHECK-LABEL: "?catch$2@?0?test1@4HA"
@@ -70,7 +67,6 @@ catch.pad:                                        ; preds = %catch.dispatch
   %bc2 = bitcast i8** %alloca2 to i8*
   call void @llvm.lifetime.start.p0i8(i64 4, i8* %bc2)
   store volatile i8* null, i8** %alloca1
-  call void @llvm.trap()
   unreachable
 
 ; CHECK-LABEL: "?catch$2@?0?test2@4HA"

diff --git a/test/CodeGen/X86/catchpad-regmask.ll b/test/CodeGen/X86/catchpad-regmask.ll
@@ -75,7 +75,7 @@ unreachable:                                      ; preds = %entry
 ; CHECK: popq %rbp
 ; CHECK: retq
 
-; CHECK: "?catch${{[0-9]+}}@?0?global_array@4HA":
+; CHECK: "?catch$2@?0?global_array@4HA":
 ; CHECK: pushq   %rbp
 ; CHECK: movslq  {{.*}}, %[[idx:[^ ]*]]
 ; CHECK: leaq    array(%rip), %[[base:[^ ]*]]
@@ -122,7 +122,7 @@ unreachable:                                      ; preds = %entry
 ; CHECK: popq %rbp
 ; CHECK: retq
 
-; CHECK: "?catch${{[0-9]+}}@?0?access_imported@4HA":
+; CHECK: "?catch$2@?0?access_imported@4HA":
 ; CHECK: pushq   %rbp
 ; CHECK: movq    __imp_imported(%rip), %[[base:[^ ]*]]
 ; CHECK: movl    $222, (%[[base]])

diff --git a/test/CodeGen/X86/catchret-regmask.ll b/test/CodeGen/X86/catchret-regmask.ll
@@ -6,7 +6,6 @@ target triple = "x86_64-pc-windows-msvc"
 declare i32 @__CxxFrameHandler3(...)
 declare void @throw() noreturn uwtable
 declare i8* @getval()
-declare void @llvm.trap()
 
 define i8* @reload_out_of_pad(i8* %arg) #0 personality i32 (...)* @__CxxFrameHandler3 {
 assertPassed:
@@ -20,7 +19,6 @@ catch:
   ; This block *must* appear after the catchret to test the bug.
   ; FIXME: Make this an MIR test so we can control MBB layout.
 unreachable:
-  call void @llvm.trap()
   unreachable
 
 catch.dispatch:
@@ -37,7 +35,7 @@ return:
 ; CHECK: movq -[[arg_slot]](%rbp), %rax # 8-byte Reload
 ; CHECK: retq
 
-; CHECK: "?catch${{[0-9]+}}@?0?reload_out_of_pad@4HA":
+; CHECK: "?catch$3@?0?reload_out_of_pad@4HA":
 ; CHECK-NOT: Reload
 ; CHECK: retq
 
@@ -52,7 +50,6 @@ catch:
   catchret from %cp to label %return
 
 unreachable:
-  call void @llvm.trap()
   unreachable
 
 catch.dispatch:
@@ -68,7 +65,7 @@ return:
 ; CHECK: movq -[[val_slot:[0-9]+]](%rbp), %rax # 8-byte Reload
 ; CHECK: retq
 
-; CHECK: "?catch${{[0-9]+}}@?0?spill_in_pad@4HA":
+; CHECK: "?catch$3@?0?spill_in_pad@4HA":
 ; CHECK: callq getval
 ; CHECK: movq %rax, -[[val_slot]](%rbp) # 8-byte Spill
 ; CHECK: retq

diff --git a/test/CodeGen/X86/empty-function.ll b/test/CodeGen/X86/empty-function.ll
@@ -15,7 +15,7 @@ entry:
 
 ; CHECK-LABEL: f:
 ; WIN32: nop
-; WIN64: nop
+; WIN64: ud2
 ; LINUX-NOT: nop
 ; LINUX-NOT: ud2
 

diff --git a/test/CodeGen/X86/funclet-layout.ll b/test/CodeGen/X86/funclet-layout.ll
@@ -9,8 +9,6 @@ target triple = "x86_64-pc-windows-msvc"
 @"\01??_7type_info@@6B@" = external constant i8*
 @"\01??_R0H@8" = internal global %rtti.TypeDescriptor2 { i8** @"\01??_7type_info@@6B@", i8* null, [3 x i8] c".H\00" }
 
-declare void @llvm.trap()
-
 define void @test1(i1 %B) personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   invoke void @g()
@@ -33,7 +31,6 @@ try.cont:
   ret void
 
 unreachable:
-  call void @llvm.trap()
   unreachable
 }
 
@@ -79,7 +76,6 @@ try.cont.5:                                       ; preds = %try.cont
   ret i32 0
 
 unreachable:                                      ; preds = %catch, %entry
-  call void @llvm.trap()
   unreachable
 }
 
@@ -129,13 +125,11 @@ try.cont:                                         ; preds = %entry
   br i1 %V, label %exit_one, label %exit_two
 
 exit_one:
-  tail call void @g()
-  call void @llvm.trap()
+  tail call void @exit(i32 0)
   unreachable
 
 exit_two:
-  tail call void @g()
-  call void @llvm.trap()
+  tail call void @exit(i32 0)
   unreachable
 }
 
@@ -144,20 +138,20 @@ exit_two:
 ; The entry funclet contains %entry and %try.cont
 ; CHECK: # %entry
 ; CHECK: # %try.cont
-; CHECK: callq g
+; CHECK: callq exit
 ; CHECK-NOT: # exit_one
 ; CHECK-NOT: # exit_two
 ; CHECK: ud2
 
 ; The catch(...) funclet contains %catch.2
 ; CHECK: # %catch.2{{$}}
 ; CHECK: callq exit
-; CHECK-NEXT: int3
+; CHECK: ud2
 
 ; The catch(int) funclet contains %catch
 ; CHECK: # %catch{{$}}
 ; CHECK: callq exit
-; CHECK-NEXT: int3
+; CHECK: ud2
 
 declare void @exit(i32) noreturn nounwind
 declare void @_CxxThrowException(i8*, %eh.ThrowInfo*)

diff --git a/test/CodeGen/X86/noreturn-call-win64.ll b/test/CodeGen/X86/noreturn-call-win64.ll
diff --git a/test/CodeGen/X86/pr24374.ll b/test/CodeGen/X86/pr24374.ll
@@ -31,6 +31,6 @@ define void @g() {
   unreachable
 }
 ; CHECK-LABEL: g:
-; CHECK: nop
+; CHECK: ud2
 
 attributes #0 = { nounwind }
diff --git a/test/CodeGen/X86/trap.ll b/test/CodeGen/X86/trap.ll
@@ -1,19 +1,13 @@
 ; RUN: llc < %s -mtriple=i686-apple-darwin8 -mcpu=yonah | FileCheck %s -check-prefix=DARWIN
 ; RUN: llc < %s -mtriple=i686-unknown-linux -mcpu=yonah | FileCheck %s -check-prefix=LINUX
 ; RUN: llc < %s -mtriple=x86_64-scei-ps4 | FileCheck %s -check-prefix=PS4
-; RUN: llc < %s -mtriple=x86_64-windows-msvc | FileCheck %s -check-prefix=WIN64
 
 ; DARWIN-LABEL: test0:
 ; DARWIN: ud2
 ; LINUX-LABEL: test0:
 ; LINUX: ud2
-; FIXME: PS4 probably doesn't want two ud2s.
 ; PS4-LABEL: test0:
 ; PS4: ud2
-; PS4: ud2
-; WIN64-LABEL: test0:
-; WIN64: ud2
-; WIN64-NOT: ud2
 define i32 @test0() noreturn nounwind  {
 entry:
 	tail call void @llvm.trap( )
@@ -26,9 +20,6 @@ entry:
 ; LINUX: int3
 ; PS4-LABEL: test1:
 ; PS4: int     $65
-; WIN64-LABEL: test1:
-; WIN64: int3
-; WIN64-NOT: ud2
 define i32 @test1() noreturn nounwind  {
 entry:
 	tail call void @llvm.debugtrap( )