[PATCH] D15962: [WinEH] Verify consistent funclet unwind exits

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added inline comments.

================
Comment at: docs/ExceptionHandling.rst:837
@@ +836,2 @@
+destination other than caller; it is undefined behavior for such a ``call``
+or ``catchswitch`` to unwind.
----------------
JosephTremoulet wrote:
> Yes, you're understanding correctly.
> 
> When we have catchswitch A unwinding to pad B and we discover that pad B is immediately `unreachable`, to me it makes more sense to allow stomping out the edge and leaving catchswitch A as "unwinds to caller" like we do than it does to require that code to snoop around and see if catchswitch A has a parent pad and if an unwind edge which leaves the parent can be found and redirect catchswitch A to that edge's successor for consistency -- in addition to requiring a not-rocket-science but nonetheless nontrivial search for the parent's unwind successor, it also requires suddenly giving that successor a new predecessor and doing who-knows-what updates to maintain SSA etc in the face of that.
> 
> I absolutely agree that it would be nicer to have a nounwind variant of catchswitch, not just for this but also to be able to express catch-all over funclet EH in a way that the CFG reflects naturally.  I think it's a nontrivial undertaking (though easier now that we have parent tokens and funclet bundles than it would have been before), and not the top of my priority list.  Happy to review patches, though :).
OK.  That's a reasonable explanation.

Another nice-to-have would be some pass that would insert a cleanup pad with a trap instruction in this situation for debugging/testing purposes.

================
Comment at: lib/IR/Verifier.cpp:3117
@@ +3116,3 @@
+        do {
+          if (ExitedPad == &FPI) {
+            ExitsFPI = true;
----------------
I haven't taken the time to think through it yet but you may know, is it possible to skip over FPI and/or UnwindParent below while walking the pad-parent chain?  I think I could make some IR that would do that for just this function.  I just don't know if it would be flagged as invalid somewhere else.


http://reviews.llvm.org/D15962