[WinEH] Fix problem with mapping handler blocks when multiple clauses share a handler implementation.

[andrew.w.kaylor]

Summary

This addresses a failure which could occur when optimized IR eliminated handler-specific blocks and created a single shared handler representation. The IR unconditionally flows into a handler block even when a selector dispatch would normally be expected. I believe this can only happen if the landing pad has a catch-all handler and the handlers for prior clauses are identical to the catch-all implementation with no intervening clean-up.

My proposed solution effectively eliminates the type-specific catch handlers from the final IR produced, but I think this is good because the resulting code will be more compact yet functionally equivalent.

[rnk]

lgtm

llvm/trunk/lib/CodeGen/WinEHPrepare.cpp

if (Pred == CmpInst::ICMP_NE) {
     CatchHandler = FBB;
     NextBB = TBB;
     return true;
   }
 
   return false;
 }
 
+static bool isCatchBlock(BasicBlock *BB) {
+  for (BasicBlock::iterator II = BB->getFirstNonPHIOrDbg(), IE = BB->end();
+       II != IE; ++II) {
+    if (match(cast<Value>(II), m_Intrinsic<Intrinsic::eh_begincatch>()))
+      return true;
+  }
+  return false;
+}
+
 static BasicBlock *createStubLandingPad(Function *Handler,
                                         Value *PersonalityFn) {
   // FIXME: Finish this!
   LLVMContext &Context = Handler->getContext();
   BasicBlock *StubBB = BasicBlock::Create(Context, "stub");
   Handler->getBasicBlockList().push_back(StubBB);
   IRBuilder<> Builder(StubBB);
   LandingPadInst *LPad = Builder.CreateLandingPad(
if (isa<ConstantPointerNull>(ExpectedSelector)) {
       ++HandlersFound;
 
       // Once we reach a catch-all, don't expect to hit a resume instruction.
       BB = nullptr;
       break;
     }
 
     CatchHandler *CatchAction = findCatchHandler(BB, NextBB, VisitedBlocks);
+    assert(CatchAction);
+
     // See if there is any interesting code executed before the dispatch.
     findCleanupHandlers(Actions, BB, CatchAction->getStartBlock());
 
-    assert(CatchAction);
-
     // When the source program contains multiple nested try blocks the catch
     // handlers can get strung together in such a way that we can encounter
     // a dispatch for a selector that we've already had a handler for.
     if (CatchAction->getSelector()->stripPointerCasts() == ExpectedSelector) {
       ++HandlersFound;
 
       // Add the catch handler to the action list.
       DEBUG(dbgs() << "  Found catch dispatch in block "
                    << CatchAction->getStartBlock()->getName() << "\n");
       Actions.insertCatchHandler(CatchAction);
     } else {
+      // Under some circumstances optimized IR will flow unconditionally into a
+      // handler block without checking the selector.  This can only happen if
+      // the landing pad has a catch-all handler and the handler for the
+      // preceeding catch clause is identical to the catch-call handler
+      // (typically an empty catch).  In this case, the handler must be shared
+      // by all remaining clauses.
+      if (isa<ConstantPointerNull>(
+              CatchAction->getSelector()->stripPointerCasts())) {
+        DEBUG(dbgs() << "  Applying early catch-all handler in block "
+                     << CatchAction->getStartBlock()->getName()
+                     << "  to all remaining clauses.\n");
+        Actions.insertCatchHandler(CatchAction);
+        return;
+      }
+
       DEBUG(dbgs() << "  Found extra catch dispatch in block "
                    << CatchAction->getStartBlock()->getName() << "\n");
     }
 
     // Move on to the block after the catch handler.
     BB = NextBB;
   }
 
CatchHandler *WinEHPrepare::findCatchHandler(BasicBlock *BB,
   // If this is the first time we've visited this block from any landing pad
   // look to see if it is a selector dispatch block.
   if (!CatchHandlerMap.count(BB)) {
     if (isSelectorDispatch(BB, CatchBlock, Selector, NextBB)) {
       CatchHandler *Action = new CatchHandler(BB, Selector, NextBB);
       CatchHandlerMap[BB] = Action;
       return Action;
     }
+    // If we encounter a block containing an llvm.eh.begincatch before we
+    // find a selector dispatch block, the handler is assumed to be
+    // reached unconditionally.  This happens for catch-all blocks, but
+    // it can also happen for other catch handlers that have been combined
+    // with the catch-all handler during optimization.
+    if (isCatchBlock(BB)) {
+      PointerType *Int8PtrTy = Type::getInt8PtrTy(BB->getContext());
+      Constant *NullSelector = ConstantPointerNull::get(Int8PtrTy);
+      CatchHandler *Action = new CatchHandler(BB, NullSelector, nullptr);
+      CatchHandlerMap[BB] = Action;
+      return Action;
+    }
   }
 
   // Visit each successor, looking for the dispatch.
   // FIXME: We expect to find the dispatch quickly, so this will probably
   //        work better as a breadth first search.
   for (BasicBlock *Succ : successors(BB)) {
     if (VisitedBlocks.count(Succ))
       continue;

llvm/trunk/test/CodeGen/WinEH/cppeh-shared-empty-catch.ll

+; RUN: opt -mtriple=x86_64-pc-windows-msvc -winehprepare -S -o - < %s | FileCheck %s
+
+; This test is based on the following source, built with -O2
+;
+; void f() {
+;   try {
+;     g();
+;     try {
+;       throw;
+;     } catch (int) {
+;     }
+;   } catch (...) {
+;   }
+; }
+;
+
+; ModuleID = '<stdin>'
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+%rtti.TypeDescriptor2 = type { i8**, i8*, [3 x i8] }
+%eh.CatchHandlerType = type { i32, i8* }
+%eh.ThrowInfo = type { i32, i32, i32, i32 }
+
+$"\01??_R0H@8" = comdat any
+
+@"\01??_7type_info@@6B@" = external constant i8*
+@"\01??_R0H@8" = linkonce_odr global %rtti.TypeDescriptor2 { i8** @"\01??_7type_info@@6B@", i8* null, [3 x i8] c".H\00" }, comdat
[e-mail].H.0 = private unnamed_addr constant %eh.CatchHandlerType { i32 0, i8* bitcast (%rtti.TypeDescriptor2* @"\01??_R0H@8" to i8*) }, section "llvm.metadata"
+
+; CHECK-LABEL: define void @"\01?f@@YAXXZ"()
+; CHECK: entry:
+; CHECK:   call void (...) @llvm.frameescape()
+; CHECK:   invoke void @"\01?g@@YAXXZ"()
+
+; Function Attrs: nounwind
+define void @"\01?f@@YAXXZ"() #0 {
+entry:
+  invoke void @"\01?g@@YAXXZ"()
+          to label %invoke.cont unwind label %lpad
+
+; CHECK-LABEL: invoke.cont:
+; CHECK:   invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null)
+; CHECK:           to label %unreachable unwind label %[[LPAD1_LABEL:lpad[0-9]+]]
+
+invoke.cont:                                      ; preds = %entry
+  invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null) #4
+          to label %unreachable unwind label %lpad1
+
+lpad:                                             ; preds = %entry
+  %0 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
+          catch i8* null
+  %1 = extractvalue { i8*, i32 } %0, 0
+  br label %catch2
+
+; Note: Even though this landing pad has two catch clauses, it only has one action because both
+;       handlers do the same thing.
+; CHECK: [[LPAD1_LABEL]]:
+; CHECK:   landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
+; CHECK-NEXT:           catch %eh.CatchHandlerType* @llvm.eh.handlertype.H.0
+; CHECK-NEXT:           catch i8* null
+; CHECK-NEXT:   [[RECOVER:\%.+]] = call i8* (...) @llvm.eh.actions(i32 1, i8* null, i32 -1, i8* (i8*, i8*)* @"\01?f@@YAXXZ.catch1")
+; CHECK-NEXT:   indirectbr i8* [[RECOVER]], [label %try.cont4]
+
+lpad1:                                            ; preds = %invoke.cont
+  %2 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*)
+          catch %eh.CatchHandlerType* @llvm.eh.handlertype.H.0
+          catch i8* null
+  %3 = extractvalue { i8*, i32 } %2, 0
+  br label %catch2
+
+catch2:                                           ; preds = %lpad1, %lpad
+  %exn.slot.0 = phi i8* [ %3, %lpad1 ], [ %1, %lpad ]
+  tail call void @llvm.eh.begincatch(i8* %exn.slot.0, i8* null) #3
+  tail call void @llvm.eh.endcatch() #3
+  br label %try.cont4
+
+try.cont4:                                        ; preds = %catch, %catch2
+  ret void
+
+unreachable:                                      ; preds = %invoke.cont
+  unreachable
+
+; CHECK: }
+}
+
+declare void @"\01?g@@YAXXZ"() #1
+
+declare i32 @__CxxFrameHandler3(...)
+
+declare void @_CxxThrowException(i8*, %eh.ThrowInfo*)
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.eh.typeid.for(i8*) #2
+
+; Function Attrs: nounwind
+declare void @llvm.eh.begincatch(i8* nocapture, i8* nocapture) #3
+
+; Function Attrs: nounwind
+declare void @llvm.eh.endcatch() #3
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-realign-stack" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-realign-stack" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #2 = { nounwind readnone }
+attributes #3 = { nounwind }
+attributes #4 = { noreturn }
+
+!llvm.ident = !{!0}
+
+!0 = !{!"clang version 3.7.0 (trunk 235112) (llvm/trunk 235121)"}