[WinEH] Fix cleanup state numbering

[JosephTremoulet]

Summary

• Recurse from cleanupendpads to their cleanuppads, to make sure the cleanuppad is visited if it has a cleanupendpad but no cleanupret.
• Check for and avoid double-processing cleanuppads, to allow for them to have multiple cleanuprets (plus cleanupendpads).
• Update Cxx state numbering to visit toplevel cleanupendpads and to recurse from cleanupendpads to their preds, to ensure we number any funclets in inlined cleanups. SEH state numbering already did this.

[JosephTremoulet]

Just a few things I noticed when going over this code in the process of using it as a template for CLR state numbering. I wanted to make sure we'd handle these cases in a consistent way, and it would have taken longer to try to explain than to just send this patch.

[rnk]

I held off on doing this because Clang doesn't emit cleanupendpad for C++ destructors yet. This isn't usually problem because destructors are noexcept by default in C++11, so the cleanupendpad is unreachable *unless* someone explicitly declares a noexcept(false) destructor:

struct D {
  ~D() noexcept(false) { throw 42; }
};
int main() {
  D o;
  throw 42;
}

Let me deal with that and I'll get back to this.

[JosephTremoulet]

replace find(x) != end() with count(x) != 0

[rnk]

lgtm

[rnk]

I landed rL249748, which makes clang do the right thing with cleanupendpad and C++.

lib/CodeGen/WinEHPrepare.cpp

if (isa<CatchEndPadInst>(FirstNonPHI)) {
     addTryBlockMapEntry(FuncInfo, TryLow, TryHigh, CatchHigh, Handlers);
     DEBUG(dbgs() << "TryLow[" << FirstTryPad->getName() << "]: " << TryLow
                  << '\n');
     DEBUG(dbgs() << "TryHigh[" << FirstTryPad->getName() << "]: " << TryHigh
                  << '\n');
     DEBUG(dbgs() << "CatchHigh[" << FirstTryPad->getName() << "]: " << CatchHigh
                  << '\n');
   } else if (isa<CleanupPadInst>(FirstNonPHI)) {
+    // A cleanup can have multiple exits; don't re-process after the first.
+    if (FuncInfo.EHPadStateMap.count(FirstNonPHI))
+      return;
     int CleanupState = addUnwindMapEntry(FuncInfo, ParentState, &BB);
     FuncInfo.EHPadStateMap[FirstNonPHI] = CleanupState;
     DEBUG(dbgs() << "Assigning state #" << CleanupState << " to BB "
                  << BB.getName() << '\n');
     for (const BasicBlock *PredBlock : predecessors(&BB))
       if ((PredBlock = getEHPadFromPredecessor(PredBlock)))
         calculateExplicitCXXStateNumbers(FuncInfo, *PredBlock, CleanupState);
+  } else if (auto *CEPI = dyn_cast<CleanupEndPadInst>(FirstNonPHI)) {
+    // Propagate ParentState to the cleanuppad in case it doesn't have
+    // any cleanuprets.
+    BasicBlock *CleanupBlock = CEPI->getCleanupPad()->getParent();
+    calculateExplicitCXXStateNumbers(FuncInfo, *CleanupBlock, ParentState);
+    // Anything unwinding through CleanupEndPadInst is in ParentState.
+    for (const BasicBlock *PredBlock : predecessors(&BB))
+      if ((PredBlock = getEHPadFromPredecessor(PredBlock)))
+        calculateExplicitCXXStateNumbers(FuncInfo, *PredBlock, ParentState);
   } else if (isa<TerminatePadInst>(FirstNonPHI)) {
     report_fatal_error("Not yet implemented!");
   } else {
     llvm_unreachable("unexpected EH Pad!");
   }
 }
 
 static int addSEHExcept(WinEHFuncInfo &FuncInfo, int ParentState,
if (isa<CatchEndPadInst>(FirstNonPHI)) {
     // outside the __try.
     FuncInfo.EHPadStateMap[FirstNonPHI] = ParentState;
     DEBUG(dbgs() << "Assigning state #" << ParentState << " to BB "
                  << BB.getName() << '\n');
     for (const BasicBlock *PredBlock : predecessors(&BB))
       if ((PredBlock = getEHPadFromPredecessor(PredBlock)))
         calculateExplicitSEHStateNumbers(FuncInfo, *PredBlock, ParentState);
   } else if (isa<CleanupPadInst>(FirstNonPHI)) {
+    // A cleanup can have multiple exits; don't re-process after the first.
+    if (FuncInfo.EHPadStateMap.count(FirstNonPHI))
+      return;
     int CleanupState = addSEHFinally(FuncInfo, ParentState, &BB);
     FuncInfo.EHPadStateMap[FirstNonPHI] = CleanupState;
     DEBUG(dbgs() << "Assigning state #" << CleanupState << " to BB "
                  << BB.getName() << '\n');
     for (const BasicBlock *PredBlock : predecessors(&BB))
       if ((PredBlock = getEHPadFromPredecessor(PredBlock)))
         calculateExplicitSEHStateNumbers(FuncInfo, *PredBlock, CleanupState);
-  } else if (isa<CleanupEndPadInst>(FirstNonPHI)) {
+  } else if (auto *CEPI = dyn_cast<CleanupEndPadInst>(FirstNonPHI)) {
+    // Propagate ParentState to the cleanuppad in case it doesn't have
+    // any cleanuprets.
+    BasicBlock *CleanupBlock = CEPI->getCleanupPad()->getParent();
+    calculateExplicitSEHStateNumbers(FuncInfo, *CleanupBlock, ParentState);
     // Anything unwinding through CleanupEndPadInst is in ParentState.
     FuncInfo.EHPadStateMap[FirstNonPHI] = ParentState;
     DEBUG(dbgs() << "Assigning state #" << ParentState << " to BB "
                  << BB.getName() << '\n');
     for (const BasicBlock *PredBlock : predecessors(&BB))
       if ((PredBlock = getEHPadFromPredecessor(PredBlock)))
         calculateExplicitSEHStateNumbers(FuncInfo, *PredBlock, ParentState);
   } else if (isa<TerminatePadInst>(FirstNonPHI)) {
if (!FuncInfo.EHPadStateMap.empty())
     return;
 
   for (const BasicBlock &BB : *Fn) {
     if (!BB.isEHPad())
       continue;
     if (BB.isLandingPad())
       report_fatal_error("MSVC C++ EH cannot use landingpads");
     const Instruction *FirstNonPHI = BB.getFirstNonPHI();
-    // Skip cleanupendpads; they are exits, not entries.
-    if (isa<CleanupEndPadInst>(FirstNonPHI))
-      continue;
     if (!doesEHPadUnwindToCaller(FirstNonPHI))
       continue;
     calculateExplicitCXXStateNumbers(FuncInfo, BB, -1);
   }
 }
 
 static int addClrEHHandler(WinEHFuncInfo &FuncInfo, int ParentState,
                            ClrHandlerType HandlerType, uint32_t TypeToken,

test/CodeGen/WinEH/wineh-statenumbering-cleanups.ll

+; RUN: sed -e s/.Cxx:// %s | opt -mtriple=x86-pc-windows-msvc -S -x86-winehstate | FileCheck %s
+; RUN: sed -e s/.SEH:// %s | opt -mtriple=x86-pc-windows-msvc -S -x86-winehstate | FileCheck %s
+
+declare i32 @__CxxFrameHandler3(...)
+declare i32 @_except_handler3(...)
+declare void @dummy_filter()
+
+declare void @f(i32)
+
+; CHECK-LABEL: define void @test1(
+;Cxx: define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
+;SEH: define void @test1() personality i32 (...)* @_except_handler3 {
+entry:
+  ; CHECK: entry:
+  ; CHECK:  store i32 0
+  ; CHECK:  invoke void @f(i32 0)
+  invoke void @f(i32 0)
+    to label %exit unwind label %cleanup.pad
+cleanup.pad:
+  ; CHECK: cleanup.pad:
+  ; CHECK:   store i32 1
+  ; CHECK:   invoke void @f(i32 1)
+  %cleanup = cleanuppad []
+  invoke void @f(i32 1)
+    to label %cleanup.ret unwind label %catch.pad
+catch.pad:
+;Cxx: %catch = catchpad [i8* null, i32 u0x40, i8* null]
+;SEH: %catch = catchpad [void ()* @dummy_filter]
+        to label %catch.body unwind label %catch.end
+catch.body:
+  catchret %catch to label %cleanup.ret
+catch.end:
+  catchendpad unwind label %cleanup.end
+cleanup.ret:
+  cleanupret %cleanup unwind to caller
+cleanup.end:
+  cleanupendpad %cleanup unwind to caller
+exit:
+  ret void
+}
+
+; CHECK-LABEL: define void @test2(
+;Cxx: define void @test2(i1 %b) personality i32 (...)* @__CxxFrameHandler3 {
+;SEH: define void @test2(i1 %b) personality i32 (...)* @_except_handler3 {
+entry:
+  ; CHECK: entry:
+  ; CHECK:   store i32 1
+  ; CHECK:   invoke void @f(i32 1)
+  invoke void @f(i32 1)
+    to label %exit unwind label %cleanup.pad
+cleanup.pad:
+  %cleanup = cleanuppad []
+  br i1 %b, label %left, label %right
+left:
+  cleanupret %cleanup unwind label %catch.pad
+right:
+  cleanupret %cleanup unwind label %catch.pad
+catch.pad:
+;Cxx: %catch = catchpad [i8* null, i32 u0x40, i8* null]
+;SEH: %catch = catchpad [void ()* @dummy_filter]
+        to label %catch.body unwind label %catch.end
+catch.body:
+  catchret %catch to label %exit
+catch.end:
+  catchendpad unwind to caller
+exit:
+  ret void
+}
+
+; CHECK-LABEL: define void @test3(
+;Cxx: define void @test3() personality i32 (...)* @__CxxFrameHandler3 {
+;SEH: define void @test3() personality i32 (...)* @_except_handler3 {
+entry:
+  ; CHECK: entry:
+  ; CHECK:   store i32 1
+  ; CHECK:   invoke void @f(i32 1)
+  invoke void @f(i32 1)
+    to label %exit unwind label %cleanup.pad
+cleanup.pad:
+  ; CHECK: cleanup.pad:
+  ; CHECK:   store i32 0
+  ; CHECK:   invoke void @f(i32 0)
+  %cleanup = cleanuppad []
+  invoke void @f(i32 0)
+    to label %unreachable unwind label %cleanup.end
+unreachable:
+  unreachable
+cleanup.end:
+  cleanupendpad %cleanup unwind label %catch.pad
+catch.pad:
+;Cxx: %catch = catchpad [i8* null, i32 u0x40, i8* null]
+;SEH: %catch = catchpad [void ()* @dummy_filter]
+        to label %catch.body unwind label %catch.end
+catch.body:
+  catchret %catch to label %exit
+catch.end:
+  catchendpad unwind to caller
+exit:
+  ret void
+}