Index: docs/ExceptionHandling.rst =================================================================== --- docs/ExceptionHandling.rst +++ docs/ExceptionHandling.rst @@ -818,3 +818,20 @@ 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: docs/LangRef.rst =================================================================== --- docs/LangRef.rst +++ docs/LangRef.rst @@ -8657,10 +8657,6 @@ it is undefined behavior to execute a :ref:`call ` or :ref:`invoke ` that does not carry an appropriate :ref:`"funclet" bundle `. -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: lib/IR/Verifier.cpp =================================================================== --- lib/IR/Verifier.cpp +++ lib/IR/Verifier.cpp @@ -403,6 +403,7 @@ 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 @@ 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 @@ Assert(isa(ParentPad) || isa(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(U)) { - UnwindDest = CRI->getUnwindDest(); - } else if (isa(U) || isa(U)) { - continue; - } else if (CallSite(U)) { - continue; - } else { - Assert(false, "bogus cleanuppad use", &CPI); + Value *FirstUnwindPad = nullptr; + SmallVector 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(U)) { + UnwindDest = CRI->getUnwindDest(); + } else if (auto *CSI = dyn_cast(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(U)) { + UnwindDest = II->getUnwindDest(); + } else if (isa(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(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(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(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(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: test/CodeGen/WinEH/wineh-no-demotion.ll =================================================================== --- test/CodeGen/WinEH/wineh-no-demotion.ll +++ test/CodeGen/WinEH/wineh-no-demotion.ll @@ -33,7 +33,7 @@ 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 @@ shared: %x = call i32 @g() - invoke void @f() [ "funclet"(token %0) ] + invoke void @f() to label %shared.cont unwind label %inner shared.cont: Index: test/CodeGen/WinEH/wineh-statenumbering.ll =================================================================== --- test/CodeGen/WinEH/wineh-statenumbering.ll +++ test/CodeGen/WinEH/wineh-statenumbering.ll @@ -44,7 +44,7 @@ ; 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: test/Verifier/invalid-eh.ll =================================================================== --- test/Verifier/invalid-eh.ll +++ 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/.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 @@ ;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: }