[WinEH] Verify consistent funclet unwind exits
authorJoseph Tremoulet <jotrem@microsoft.com>
Sun, 10 Jan 2016 04:30:02 +0000 (04:30 +0000)
committerJoseph Tremoulet <jotrem@microsoft.com>
Sun, 10 Jan 2016 04:30:02 +0000 (04:30 +0000)
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

docs/ExceptionHandling.rst
docs/LangRef.rst
lib/IR/Verifier.cpp
test/CodeGen/WinEH/wineh-no-demotion.ll
test/CodeGen/WinEH/wineh-statenumbering.ll
test/Verifier/invalid-eh.ll

index 2524856..9f46094 100644 (file)
@@ -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.
+
+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.
index 650d18b..ff27f3c 100644 (file)
@@ -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 for the ``cleanuppad`` to exit via an unwind edge which
-does not transitively unwind to the same destination as a constituent
-``cleanupret``.
-
 Example:
 """"""""
 
index 44ced48..dc723f6 100644 (file)
@@ -403,6 +403,7 @@ private:
   void visitCatchPadInst(CatchPadInst &CPI);
   void visitCatchReturnInst(CatchReturnInst &CatchReturn);
   void visitCleanupPadInst(CleanupPadInst &CPI);
+  void visitFuncletPadInst(FuncletPadInst &FPI);
   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);
 
-  visitInstruction(CPI);
+  visitFuncletPadInst(CPI);
 }
 
 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);
 
+  visitFuncletPadInst(CPI);
+}
+
+void Verifier::visitFuncletPadInst(FuncletPadInst &FPI) {
   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) {
index 4fb84db..0901e27 100644 (file)
@@ -33,7 +33,7 @@ right:
 
 shared:
   %x = call i32 @g()
-  invoke void @f() [ "funclet"(token %0) ]
+  invoke void @f()
           to label %shared.cont unwind label %inner
 
 shared.cont:
@@ -72,7 +72,7 @@ right:
 
 shared:
   %x = call i32 @g()
-  invoke void @f() [ "funclet"(token %0) ]
+  invoke void @f()
           to label %shared.cont unwind label %inner
 
 shared.cont:
index dab7fde..4e7c369 100644 (file)
@@ -44,7 +44,7 @@ catch:                                            ; preds = %catch.dispatch
   ; 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
index e43a676..e5ad2ab 100644 (file)
 ; 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()
 
@@ -183,3 +187,98 @@ declare void @g()
 ;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: }