It turns out that implementing the rethrow-on-fallthrough

semantics of a ctor/dtor function-try-block catch handler
by pushing a normal cleanup is not just overkill but actually
actively wrong when the handler contains an explicit return
(which is only legal in a dtor).  Just emit the rethrow as
ordinary code at the fallthrough point.  Fixes PR13102.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158488 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 56 insertions, 18 deletions

diff --git a/lib/CodeGen/CGException.cpp b/lib/CodeGen/CGException.cpp
index 95e0030866..4cf7f12bd6 100644
--- a/lib/CodeGen/CGException.cpp
+++ b/lib/CodeGen/CGException.cpp
@@ -1127,14 +1127,6 @@ static void BeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *S) {
   CGF.EmitAutoVarCleanups(var);
 }
 
-namespace {
-  struct CallRethrow : EHScopeStack::Cleanup {
-    void Emit(CodeGenFunction &CGF, Flags flags) {
-      CGF.EmitCallOrInvoke(getReThrowFn(CGF));
-    }
-  };
-}
-
 /// Emit the structure of the dispatch block for the given catch scope.
 /// It is an invariant that the dispatch block already exists.
 static void emitCatchDispatchBlock(CodeGenFunction &CGF,
@@ -1246,11 +1238,12 @@ void CodeGenFunction::ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
   if (HaveInsertPoint())
     Builder.CreateBr(ContBB);
 
-  // Determine if we need an implicit rethrow for all these catch handlers.
-  bool ImplicitRethrow = false;
+  // Determine if we need an implicit rethrow for all these catch handlers;
+  // see the comment below.
+  bool doImplicitRethrow = false;
   if (IsFnTryBlock)
-    ImplicitRethrow = isa<CXXDestructorDecl>(CurCodeDecl) ||
-                      isa<CXXConstructorDecl>(CurCodeDecl);
+    doImplicitRethrow = isa<CXXDestructorDecl>(CurCodeDecl) ||
+                        isa<CXXConstructorDecl>(CurCodeDecl);
 
   // Perversely, we emit the handlers backwards precisely because we
   // want them to appear in source order.  In all of these cases, the
@@ -1273,15 +1266,24 @@ void CodeGenFunction::ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
     // Initialize the catch variable and set up the cleanups.
     BeginCatch(*this, C);
 
-    // If there's an implicit rethrow, push a normal "cleanup" to call
-    // _cxa_rethrow.  This needs to happen before __cxa_end_catch is
-    // called, and so it is pushed after BeginCatch.
-    if (ImplicitRethrow)
-      EHStack.pushCleanup<CallRethrow>(NormalCleanup);
-
     // Perform the body of the catch.
     EmitStmt(C->getHandlerBlock());
 
+    // [except.handle]p11:
+    //   The currently handled exception is rethrown if control
+    //   reaches the end of a handler of the function-try-block of a
+    //   constructor or destructor.
+
+    // It is important that we only do this on fallthrough and not on
+    // return.  Note that it's illegal to put a return in a
+    // constructor function-try-block's catch handler (p14), so this
+    // really only applies to destructors.
+    if (doImplicitRethrow && HaveInsertPoint()) {
+      EmitCallOrInvoke(getReThrowFn(*this));
+      Builder.CreateUnreachable();
+      Builder.ClearInsertionPoint();
+    }
+
     // Fall out through the catch cleanups.
     CatchScope.ForceCleanup();
 
diff --git a/test/CodeGenCXX/exceptions.cpp b/test/CodeGenCXX/exceptions.cpp
index 079c1e5e72..8c20c9e9ce 100644
--- a/test/CodeGenCXX/exceptions.cpp
+++ b/test/CodeGenCXX/exceptions.cpp
@@ -414,3 +414,39 @@ namespace test9 {
   // CHECK: [[TEST9_NEW:%.*]] = call noalias i8* @_Znam
   // CHECK: call void @_ZdaPv(i8* [[TEST9_NEW]])
 }
+
+// In a destructor with a function-try-block, a return statement in a
+// catch handler behaves differently from running off the end of the
+// catch handler.  PR13102.
+namespace test10 {
+  extern void cleanup();
+  extern bool suppress;
+
+  struct A { ~A(); };
+  A::~A() try { cleanup(); } catch (...) { return; }
+  // CHECK:    define void @_ZN6test101AD1Ev(
+  // CHECK:      invoke void @_ZN6test107cleanupEv()
+  // CHECK-NOT:  rethrow
+  // CHECK:      ret void
+
+  struct B { ~B(); };
+  B::~B() try { cleanup(); } catch (...) {}
+  // CHECK:    define void @_ZN6test101BD1Ev(
+  // CHECK:      invoke void @_ZN6test107cleanupEv()
+  // CHECK:      call i8* @__cxa_begin_catch
+  // CHECK-NEXT: invoke void @__cxa_rethrow()
+  // CHECK:      unreachable
+
+  struct C { ~C(); };
+  C::~C() try { cleanup(); } catch (...) { if (suppress) return; }
+  // CHECK:    define void @_ZN6test101CD1Ev(
+  // CHECK:      invoke void @_ZN6test107cleanupEv()
+  // CHECK:      call i8* @__cxa_begin_catch
+  // CHECK-NEXT: load i8* @_ZN6test108suppressE, align 1
+  // CHECK-NEXT: trunc
+  // CHECK-NEXT: br i1
+  // CHECK:      call void @__cxa_end_catch()
+  // CHECK-NEXT: br label
+  // CHECK:      invoke void @__cxa_rethrow()
+  // CHECK:      unreachable
+}