Revert "Avoid emitting unreachable SP adjustments after throw"

This reverts commit 65b2128. Breaks sanitizer bots (https://reviews.llvm.org/D75712#1927668) and causes https://crbug.com/1062021 (which may or may not be a compiler bug, not clear yet).
llvm · Mar 18, 2020 · 4e0fe03 · 4e0fe03
1 parent a9e05d5
commit 4e0fe03
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 65 deletions.
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -3009,12 +3009,6 @@ eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
   I = MBB.erase(I);
   auto InsertPos = skipDebugInstructionsForward(I, MBB.end());
 
-  // Try to avoid emitting dead SP adjustments if the block end is unreachable,
-  // typically because the function is marked noreturn (abort, throw,
-  // assert_fail, etc).
-  if (isDestroy && blockEndIsUnreachable(MBB, I))
-    return I;
-
   if (!reserveCallFrame) {
     // If the stack pointer can be changed after prologue, turn the
     // adjcallstackup instruction into a 'sub ESP, <amt>' and the
@@ -3097,7 +3091,13 @@ eliminateCallFramePseudoInstr(MachineFunction &MF, MachineBasicBlock &MBB,
     return I;
   }
 
-  if (InternalAmt) {
+  if (isDestroy && InternalAmt && !blockEndIsUnreachable(MBB, I)) {
+    // If we are performing frame pointer elimination and if the callee pops
+    // something off the stack pointer, add it back.  We do this until we have
+    // more advanced stack pointer tracking ability.
+    // We are not tracking the stack pointer adjustment by the callee, so make
+    // sure we restore the stack pointer immediately after the call, there may
+    // be spill code inserted between the CALL and ADJCALLSTACKUP instructions.
     MachineBasicBlock::iterator CI = I;
     MachineBasicBlock::iterator B = MBB.begin();
     while (CI != B && !std::prev(CI)->isCall())

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -4366,6 +4366,12 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   else
     NumBytesForCalleeToPop = 0;  // Callee pops nothing.
 
+  if (CLI.DoesNotReturn && !getTargetMachine().Options.TrapUnreachable) {
+    // No need to reset the stack after the call if the call doesn't return. To
+    // make the MI verify, we'll pretend the callee does it for us.
+    NumBytesForCalleeToPop = NumBytes;
+  }
+
   // Returns a flag for retval copy to use.
   if (!IsSibcall) {
     Chain = DAG.getCALLSEQ_END(Chain,

diff --git a/llvm/test/CodeGen/X86/noreturn-call-win64.ll b/llvm/test/CodeGen/X86/noreturn-call-win64.ll
@@ -1,8 +1,5 @@
 ; RUN: llc < %s -mtriple=x86_64-windows-msvc | FileCheck %s
 
-%struct.MakeCleanup = type { i8 }
-%eh.ThrowInfo = type { i32, i32, i32, i32 }
-
 ; Function Attrs: noinline nounwind optnone uwtable
 define dso_local i32 @foo() {
 entry:
@@ -54,58 +51,3 @@ declare dso_local i32 @cond()
 declare dso_local void @abort1() noreturn
 declare dso_local void @abort2() noreturn
 declare dso_local void @abort3() noreturn
-
-define dso_local void @throw_exception() uwtable personality i32 (...)* @__CxxFrameHandler3 {
-entry:
-  %o = alloca %struct.MakeCleanup, align 1
-  %call = invoke i32 @cond()
-          to label %invoke.cont unwind label %ehcleanup
-
-invoke.cont:                                      ; preds = %entry
-  %cmp1 = icmp eq i32 0, %call
-  br i1 %cmp1, label %if.then, label %if.end
-
-if.then:                                          ; preds = %invoke.cont
-  invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null)
-          to label %unreachable unwind label %ehcleanup
-
-if.end:                                           ; preds = %invoke.cont
-  %call2 = invoke i32 @cond()
-          to label %invoke.cont1 unwind label %ehcleanup
-
-invoke.cont1:                                     ; preds = %if.end
-  %cmp2 = icmp eq i32 0, %call2
-  br i1 %cmp2, label %if.then3, label %if.end4
-
-if.then3:                                         ; preds = %invoke.cont1
-  invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null)
-          to label %unreachable unwind label %ehcleanup
-
-if.end4:                                          ; preds = %invoke.cont1
-  call void @"??1MakeCleanup@@QEAA@XZ"(%struct.MakeCleanup* nonnull %o)
-  ret void
-
-ehcleanup:                                        ; preds = %if.then3, %if.end, %if.then, %entry
-  %cp = cleanuppad within none []
-  call void @"??1MakeCleanup@@QEAA@XZ"(%struct.MakeCleanup* nonnull %o) [ "funclet"(token %cp) ]
-  cleanupret from %cp unwind to caller
-
-unreachable:                                      ; preds = %if.then3, %if.then
-  unreachable
-}
-
-declare dso_local i32 @__CxxFrameHandler3(...)
-declare dso_local void @_CxxThrowException(i8*, %eh.ThrowInfo*)
-declare dso_local void @"??1MakeCleanup@@QEAA@XZ"(%struct.MakeCleanup*)
-
-; CHECK-LABEL: throw_exception:
-; CHECK: callq cond
-; CHECK: je
-; CHECK: callq cond
-; CHECK: je
-; CHECK: retq
-; CHECK: callq _CxxThrowException
-; CHECK-NOT: {{(addq|subq) .*, %rsp}}
-; CHECK: callq _CxxThrowException
-; CHECK-NOT: {{(addq|subq) .*, %rsp}}
-; CHECK: .seh_handlerdata