[WinEH] Verify consistent funclet unwind exits

Summary:
A funclet EH pad may be exited by an unwind edge, which may be a
cleanupret exiting its cleanuppad, an invoke exiting a funclet, or an
unwind out of a nested funclet transitively exiting its parent.  Funclet
EH personalities require all such exceptional exits from a given funclet to
have the same unwind destination, and EH preparation / state numbering /
table generation implicitly depends on this.  Formalize it as a rule of
the IR in the LangRef and verifier.

Reviewers: rnk, majnemer, andrew.w.kaylor

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D15962

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

diff --git a/docs/ExceptionHandling.rst b/docs/ExceptionHandling.rst
index 25248568b612fec4652421a06ffa023346b511eb..9f46094d79105932e89cba9f995b7cfd9e1f6da5 100644 (file)
--- a/docs/ExceptionHandling.rst
+++ b/docs/ExceptionHandling.rst
@@ -818,3 +818,20 @@ not-yet-exited pad (after exiting from any pads that the unwind edge exits),
 or "none" if there is no such pad.  This ensures that the stack of executing
 funclets at run-time always corresponds to some path in the funclet pad tree
 that the parent tokens encode.
 or "none" if there is no such pad.  This ensures that the stack of executing
 funclets at run-time always corresponds to some path in the funclet pad tree
 that the parent tokens encode.

+
+All unwind edges which exit any given funclet pad (including ``cleanupret``
+edges exiting their ``cleanuppad`` and ``catchswitch`` edges exiting their
+``catchswitch``) must share the same unwind destination.  Similarly, any
+funclet pad which may be exited by unwind to caller must not be exited by
+any exception edges which unwind anywhere other than the caller.  This
+ensures that each funclet as a whole has only one unwind destination, which
+EH tables for funclet personalities may require.  Note that any unwind edge
+which exits a ``catchpad`` also exits its parent ``catchswitch``, so this
+implies that for any given ``catchswitch``, its unwind destination must also
+be the unwind destination of any unwind edge that exits any of its constituent
+``catchpad``\s.  Because ``catchswitch`` has no ``nounwind`` variant, and
+because IR producers are not *required* to annotate calls which will not
+unwind as ``nounwind``, it is legal to nest a ``call`` or an "``unwind to
+caller``\ " ``catchswitch`` within a funclet pad that has an unwind
+destination other than caller; it is undefined behavior for such a ``call``
+or ``catchswitch`` to unwind.

diff --git a/docs/LangRef.rst b/docs/LangRef.rst
index 650d18b9ba464e5866e13d36cc2124c84f774d94..ff27f3c30c96f13ad53adaf379cc5b5a2bbceb99 100644 (file)
--- a/docs/LangRef.rst
+++ b/docs/LangRef.rst
@@ -8657,10 +8657,6 @@ described in the `EH documentation\ <ExceptionHandling.html#wineh-constraints>`_
 it is undefined behavior to execute a :ref:`call <i_call>` or :ref:`invoke <i_invoke>`
 that does not carry an appropriate :ref:`"funclet" bundle <ob_funclet>`.
 
 it is undefined behavior to execute a :ref:`call <i_call>` or :ref:`invoke <i_invoke>`
 that does not carry an appropriate :ref:`"funclet" bundle <ob_funclet>`.
 

-It is undefined behavior for the ``cleanuppad`` to exit via an unwind edge which
-does not transitively unwind to the same destination as a constituent
-``cleanupret``.
-

 Example:
 """"""""
 
 Example:
 """"""""
 

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index 44ced48e81d23a035bb61a6a8a5ebd7bb5b862fc..dc723f63ea981bdafdf3a1c75fe50a824e3b6146 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -403,6 +403,7 @@ private:
   void visitCatchPadInst(CatchPadInst &CPI);
   void visitCatchReturnInst(CatchReturnInst &CatchReturn);
   void visitCleanupPadInst(CleanupPadInst &CPI);
   void visitCatchPadInst(CatchPadInst &CPI);
   void visitCatchReturnInst(CatchReturnInst &CatchReturn);
   void visitCleanupPadInst(CleanupPadInst &CPI);

+  void visitFuncletPadInst(FuncletPadInst &FPI);

   void visitCatchSwitchInst(CatchSwitchInst &CatchSwitch);
   void visitCleanupReturnInst(CleanupReturnInst &CRI);
 
   void visitCatchSwitchInst(CatchSwitchInst &CatchSwitch);
   void visitCleanupReturnInst(CleanupReturnInst &CRI);
 


@@ -3028,7 +3029,7 @@ void Verifier::visitCatchPadInst(CatchPadInst &CPI) {
   Assert(BB->getFirstNonPHI() == &CPI,
          "CatchPadInst not the first non-PHI instruction in the block.", &CPI);
 
   Assert(BB->getFirstNonPHI() == &CPI,
          "CatchPadInst not the first non-PHI instruction in the block.", &CPI);
 

-  visitInstruction(CPI);
+  visitFuncletPadInst(CPI);

 }
 
 void Verifier::visitCatchReturnInst(CatchReturnInst &CatchReturn) {
 }
 
 void Verifier::visitCatchReturnInst(CatchReturnInst &CatchReturn) {


@@ -3058,33 +3059,156 @@ void Verifier::visitCleanupPadInst(CleanupPadInst &CPI) {
   Assert(isa<ConstantTokenNone>(ParentPad) || isa<FuncletPadInst>(ParentPad),
          "CleanupPadInst has an invalid parent.", &CPI);
 
   Assert(isa<ConstantTokenNone>(ParentPad) || isa<FuncletPadInst>(ParentPad),
          "CleanupPadInst has an invalid parent.", &CPI);
 

+  visitFuncletPadInst(CPI);
+}
+
+void Verifier::visitFuncletPadInst(FuncletPadInst &FPI) {

   User *FirstUser = nullptr;
   User *FirstUser = nullptr;

-  BasicBlock *FirstUnwindDest = nullptr;
-  for (User *U : CPI.users()) {
-    BasicBlock *UnwindDest;
-    if (CleanupReturnInst *CRI = dyn_cast<CleanupReturnInst>(U)) {
-      UnwindDest = CRI->getUnwindDest();
-    } else if (isa<CleanupPadInst>(U) || isa<CatchSwitchInst>(U)) {
-      continue;
-    } else if (CallSite(U)) {
-      continue;
-    } else {
-      Assert(false, "bogus cleanuppad use", &CPI);
+  Value *FirstUnwindPad = nullptr;
+  SmallVector<FuncletPadInst *, 8> Worklist({&FPI});
+  while (!Worklist.empty()) {
+    FuncletPadInst *CurrentPad = Worklist.pop_back_val();
+    Value *UnresolvedAncestorPad = nullptr;
+    for (User *U : CurrentPad->users()) {
+      BasicBlock *UnwindDest;
+      if (auto *CRI = dyn_cast<CleanupReturnInst>(U)) {
+        UnwindDest = CRI->getUnwindDest();
+      } else if (auto *CSI = dyn_cast<CatchSwitchInst>(U)) {
+        // We allow catchswitch unwind to caller to nest
+        // within an outer pad that unwinds somewhere else,
+        // because catchswitch doesn't have a nounwind variant.
+        // See e.g. SimplifyCFGOpt::SimplifyUnreachable.
+        if (CSI->unwindsToCaller())
+          continue;
+        UnwindDest = CSI->getUnwindDest();
+      } else if (auto *II = dyn_cast<InvokeInst>(U)) {
+        UnwindDest = II->getUnwindDest();
+      } else if (isa<CallInst>(U)) {
+        // Calls which don't unwind may be found inside funclet
+        // pads that unwind somewhere else.  We don't *require*
+        // such calls to be annotated nounwind.
+        continue;
+      } else if (auto *CPI = dyn_cast<CleanupPadInst>(U)) {
+        // The unwind dest for a cleanup can only be found by
+        // recursive search.  Add it to the worklist, and we'll
+        // search for its first use that determines where it unwinds.
+        Worklist.push_back(CPI);
+        continue;
+      } else {
+        Assert(isa<CatchReturnInst>(U), "Bogus funclet pad use", U);
+        continue;
+      }
+
+      Value *UnwindPad;
+      bool ExitsFPI;
+      if (UnwindDest) {
+        UnwindPad = UnwindDest->getFirstNonPHI();
+        Value *UnwindParent = getParentPad(UnwindPad);
+        // Ignore unwind edges that don't exit CurrentPad.
+        if (UnwindParent == CurrentPad)
+          continue;
+        // Determine whether the original funclet pad is exited,
+        // and if we are scanning nested pads determine how many
+        // of them are exited so we can stop searching their
+        // children.
+        Value *ExitedPad = CurrentPad;
+        ExitsFPI = false;
+        do {
+          if (ExitedPad == &FPI) {
+            ExitsFPI = true;
+            // Now we can resolve any ancestors of CurrentPad up to
+            // FPI, but not including FPI since we need to make sure
+            // to check all direct users of FPI for consistency.
+            UnresolvedAncestorPad = &FPI;
+            break;
+          }
+          Value *ExitedParent = getParentPad(ExitedPad);
+          if (ExitedParent == UnwindParent) {
+            // ExitedPad is the ancestor-most pad which this unwind
+            // edge exits, so we can resolve up to it, meaning that
+            // ExitedParent is the first ancestor still unresolved.
+            UnresolvedAncestorPad = ExitedParent;
+            break;
+          }
+          ExitedPad = ExitedParent;
+        } while (!isa<ConstantTokenNone>(ExitedPad));
+      } else {
+        // Unwinding to caller exits all pads.
+        UnwindPad = ConstantTokenNone::get(FPI.getContext());
+        ExitsFPI = true;
+        UnresolvedAncestorPad = &FPI;
+      }
+
+      if (ExitsFPI) {
+        // This unwind edge exits FPI.  Make sure it agrees with other
+        // such edges.
+        if (FirstUser) {
+          Assert(UnwindPad == FirstUnwindPad, "Unwind edges out of a funclet "
+                                              "pad must have the same unwind "
+                                              "dest",
+                 &FPI, U, FirstUser);
+        } else {
+          FirstUser = U;
+          FirstUnwindPad = UnwindPad;
+        }
+      }
+      // Make sure we visit all uses of FPI, but for nested pads stop as
+      // soon as we know where they unwind to.
+      if (CurrentPad != &FPI)
+        break;

     }
     }

+    if (UnresolvedAncestorPad) {
+      if (CurrentPad == UnresolvedAncestorPad) {
+        // When CurrentPad is FPI itself, we don't mark it as resolved even if
+        // we've found an unwind edge that exits it, because we need to verify
+        // all direct uses of FPI.
+        assert(CurrentPad == &FPI);
+        continue;
+      }
+      // Pop off the worklist any nested pads that we've found an unwind
+      // destination for.  The pads on the worklist are the uncles,
+      // great-uncles, etc. of CurrentPad.  We've found an unwind destination
+      // for all ancestors of CurrentPad up to but not including
+      // UnresolvedAncestorPad.
+      Value *ResolvedPad = CurrentPad;
+      while (!Worklist.empty()) {
+        Value *UnclePad = Worklist.back();
+        Value *AncestorPad = getParentPad(UnclePad);
+        // Walk ResolvedPad up the ancestor list until we either find the
+        // uncle's parent or the last resolved ancestor.
+        while (ResolvedPad != AncestorPad) {
+          Value *ResolvedParent = getParentPad(ResolvedPad);
+          if (ResolvedParent == UnresolvedAncestorPad) {
+            break;
+          }
+          ResolvedPad = ResolvedParent;
+        }
+        // If the resolved ancestor search didn't find the uncle's parent,
+        // then the uncle is not yet resolved.
+        if (ResolvedPad != AncestorPad)
+          break;
+        // This uncle is resolved, so pop it from the worklist.
+        Worklist.pop_back();
+      }
+    }
+  }

 
 

-    if (!FirstUser) {
-      FirstUser = U;
-      FirstUnwindDest = UnwindDest;
-    } else {
-      Assert(
-          UnwindDest == FirstUnwindDest,
-          "cleanupret instructions from the same cleanuppad must have the same "
-          "unwind destination",
-          FirstUser, U);
+  if (FirstUnwindPad) {
+    if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(FPI.getParentPad())) {
+      BasicBlock *SwitchUnwindDest = CatchSwitch->getUnwindDest();
+      Value *SwitchUnwindPad;
+      if (SwitchUnwindDest)
+        SwitchUnwindPad = SwitchUnwindDest->getFirstNonPHI();
+      else
+        SwitchUnwindPad = ConstantTokenNone::get(FPI.getContext());
+      Assert(SwitchUnwindPad == FirstUnwindPad,
+             "Unwind edges out of a catch must have the same unwind dest as "
+             "the parent catchswitch",
+             &FPI, FirstUser, CatchSwitch);

     }
   }
 
     }
   }
 

-  visitInstruction(CPI);
+  visitInstruction(FPI);

 }
 
 void Verifier::visitCatchSwitchInst(CatchSwitchInst &CatchSwitch) {
 }
 
 void Verifier::visitCatchSwitchInst(CatchSwitchInst &CatchSwitch) {

diff --git a/test/CodeGen/WinEH/wineh-no-demotion.ll b/test/CodeGen/WinEH/wineh-no-demotion.ll
index 4fb84db890930e6a35f159ae7017165f0c3eebbc..0901e27c301d0ecc35042dfe9914666e4b18a228 100644 (file)
--- a/test/CodeGen/WinEH/wineh-no-demotion.ll
+++ b/test/CodeGen/WinEH/wineh-no-demotion.ll
@@ -33,7 +33,7 @@ right:
 
 shared:
   %x = call i32 @g()
 
 shared:
   %x = call i32 @g()

-  invoke void @f() [ "funclet"(token %0) ]
+  invoke void @f()

           to label %shared.cont unwind label %inner
 
 shared.cont:
           to label %shared.cont unwind label %inner
 
 shared.cont:


@@ -72,7 +72,7 @@ right:
 
 shared:
   %x = call i32 @g()
 
 shared:
   %x = call i32 @g()

-  invoke void @f() [ "funclet"(token %0) ]
+  invoke void @f()

           to label %shared.cont unwind label %inner
 
 shared.cont:
           to label %shared.cont unwind label %inner
 
 shared.cont:

diff --git a/test/CodeGen/WinEH/wineh-statenumbering.ll b/test/CodeGen/WinEH/wineh-statenumbering.ll
index dab7fde61a6676d70656a11974dd8c9c7689e290..4e7c36943a01646490c1684f8b61e314e90937fd 100644 (file)
--- a/test/CodeGen/WinEH/wineh-statenumbering.ll
+++ b/test/CodeGen/WinEH/wineh-statenumbering.ll
@@ -44,7 +44,7 @@ catch:                                            ; preds = %catch.dispatch
   ; CHECK: catch:
   ; CHECK:   store i32 2
   ; CHECK:   invoke void @_CxxThrowException(
   ; CHECK: catch:
   ; CHECK:   store i32 2
   ; CHECK:   invoke void @_CxxThrowException(

-  invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null) #1
+  invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null) [ "funclet"(token %1) ]

           to label %unreachable unwind label %catch.dispatch.1
 
 catch.dispatch.1:                                 ; preds = %catch
           to label %unreachable unwind label %catch.dispatch.1
 
 catch.dispatch.1:                                 ; preds = %catch

diff --git a/test/Verifier/invalid-eh.ll b/test/Verifier/invalid-eh.ll
index e43a676495da0eee00621975434e81419c619b5f..e5ad2abfb5ed21d2d14cf1d492ca041ee3a6ec59 100644 (file)
--- a/test/Verifier/invalid-eh.ll
+++ b/test/Verifier/invalid-eh.ll
@@ -11,6 +11,10 @@
 ; RUN: sed -e s/.T11:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK11 %s
 ; RUN: sed -e s/.T12:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK12 %s
 ; RUN: sed -e s/.T13:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK13 %s
 ; RUN: sed -e s/.T11:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK11 %s
 ; RUN: sed -e s/.T12:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK12 %s
 ; RUN: sed -e s/.T13:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK13 %s

+; RUN: sed -e s/.T14:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK14 %s
+; RUN: sed -e s/.T15:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK15 %s
+; RUN: sed -e s/.T16:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK16 %s
+; RUN: sed -e s/.T17:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK17 %s

 
 declare void @g()
 
 
 declare void @g()
 


@@ -183,3 +187,98 @@ declare void @g()
 ;T13:     unreachable
 ;T13: }
 
 ;T13:     unreachable
 ;T13: }
 

+;T14: define void @f() personality void ()* @g {
+;T14:   entry:
+;T14:     ret void
+;T14:   cleanup:
+;T14:     %cp = cleanuppad within none []
+;T14:     unreachable
+;T14:   left:
+;T14:     cleanupret from %cp unwind label %switch
+;T14:   right:
+;T14:     cleanupret from %cp unwind to caller
+;T14:     ; CHECK14: Unwind edges out of a funclet pad must have the same unwind dest
+;T14:     ; CHECK14-NEXT: %cp = cleanuppad within none []
+;T14:     ; CHECK14-NEXT: cleanupret from %cp unwind label %switch
+;T14:     ; CHECK14-NEXT: cleanupret from %cp unwind to caller
+;T14:   switch:
+;T14:     %cs = catchswitch within none [label %catch] unwind to caller
+;T14:   catch:
+;T14:     catchpad within %cs [i32 1]
+;T14:     unreachable
+;T14: }
+
+;T15: define void @f() personality void ()* @g {
+;T15:   entry:
+;T15:     ret void
+;T15:   switch:
+;T15:     %cs = catchswitch within none [label %catch] unwind to caller
+;T15:   catch:
+;T15:     %catch.pad = catchpad within %cs [i32 1]
+;T15:     invoke void @g() [ "funclet"(token %catch.pad) ]
+;T15:       to label %unreachable unwind label %target1
+;T15:   unreachable:
+;T15:     unreachable
+;T15:   target1:
+;T15:     cleanuppad within none []
+;T15:     unreachable
+;T15:   target2:
+;T15:     cleanuppad within none []
+;T15:     unreachable
+;T15:   nested.1:
+;T15:     %nested.pad.1 = cleanuppad within %catch.pad []
+;T15:     unreachable
+;T15:   nested.2:
+;T15:     %nested.pad.2 = cleanuppad within %nested.pad.1 []
+;T15:     cleanupret from %nested.pad.2 unwind label %target2
+;T15:     ; CHECK15: Unwind edges out of a funclet pad must have the same unwind dest
+;T15:     ; CHECK15-NEXT: %catch.pad = catchpad within %cs [i32 1]
+;T15:     ; CHECK15-NEXT: cleanupret from %nested.pad.2 unwind label %target2
+;T15:     ; CHECK15-NEXT: invoke void @g() [ "funclet"(token %catch.pad) ]
+;T15:     ; CHECK15-NEXT:   to label %unreachable unwind label %target1
+;T15: }
+
+;T16: define void @f() personality void ()* @g {
+;T16:   entry:
+;T16:     ret void
+;T16:   switch:
+;T16:     %cs = catchswitch within none [label %catch] unwind to caller
+;T16:   catch:
+;T16:     %catch.pad = catchpad within %cs [i32 1]
+;T16:     invoke void @g() [ "funclet"(token %catch.pad) ]
+;T16:       to label %unreachable unwind label %target1
+;T16:     ; CHECK16: Unwind edges out of a catch must have the same unwind dest as the parent catchswitch
+;T16:     ; CHECK16-NEXT:   %catch.pad = catchpad within %cs [i32 1]
+;T16:     ; CHECK16-NEXT:  invoke void @g() [ "funclet"(token %catch.pad) ]
+;T16:     ; CHECK16-NEXT:          to label %unreachable unwind label %target1
+;T16:     ; CHECK16-NEXT:  %cs = catchswitch within none [label %catch] unwind to caller
+;T16:   unreachable:
+;T16:     unreachable
+;T16:   target1:
+;T16:     cleanuppad within none []
+;T16:     unreachable
+;T16: }
+
+;T17: define void @f() personality void ()* @g {
+;T17:   entry:
+;T17:     ret void
+;T17:   switch:
+;T17:     %cs = catchswitch within none [label %catch] unwind label %target1
+;T17:   catch:
+;T17:     %catch.pad = catchpad within %cs [i32 1]
+;T17:     invoke void @g() [ "funclet"(token %catch.pad) ]
+;T17:       to label %unreachable unwind label %target2
+;T17:     ; CHECK17: Unwind edges out of a catch must have the same unwind dest as the parent catchswitch
+;T17:     ; CHECK17-NEXT:  %catch.pad = catchpad within %cs [i32 1]
+;T17:     ; CHECK17-NEXT:  invoke void @g() [ "funclet"(token %catch.pad) ]
+;T17:     ; CHECK17-NEXT:          to label %unreachable unwind label %target2
+;T17:     ; CHECK17-NEXT:  %cs = catchswitch within none [label %catch] unwind label %target1
+;T17:   unreachable:
+;T17:     unreachable
+;T17:   target1:
+;T17:     cleanuppad within none []
+;T17:     unreachable
+;T17:   target2:
+;T17:     cleanuppad within none []
+;T17:     unreachable
+;T17: }