[PATCH] D14454: [WinEH] Fix mutli-parent cloning

[Andy Kaylor via llvm-commits]

andrew.w.kaylor created this revision.
andrew.w.kaylor added reviewers: JosephTremoulet, rnk, majnemer.
andrew.w.kaylor added a subscriber: llvm-commits.
andrew.w.kaylor set the repository for this revision to rL LLVM.

This fixes the bug in the earlier multi-parent cloning implementation that was leading to non-deterministic (and incorrect) output.

The problem is that while updating sibling-to-sibling unwind edges for cloned funclets, the code to handle invokes was attempting to walk the unwind edges of the cloned funclets.  As a result, it was likely to walk the catch-to-catch unwind chain before that chain was properly updated.  This change just creates a second loop in updateSiblingToSiblingUnwind() so that the invoke processing happens after all of the other unwind edges are updated.

I've included just the changes since the previously committed revision (as reviewed in D13274), but since that commit has been reverted, I will be committing the previous change set in addition to these modifications when the review is complete.

Repository:
  rL LLVM

http://reviews.llvm.org/D14454

Files:
  lib/CodeGen/WinEHPrepare.cpp

Index: lib/CodeGen/WinEHPrepare.cpp
===================================================================
--- lib/CodeGen/WinEHPrepare.cpp
+++ lib/CodeGen/WinEHPrepare.cpp
@@ -1280,7 +1280,6 @@
   // Remap any bad sibling-to-sibling transitions for funclets that
   // we just cloned.
   for (BasicBlock *ChildFunclet : FuncletChildren[CurFunclet]) {
-    bool NeedOrigInvokeRemapping = false;
     for (auto *BB : FuncletBlocks[ChildFunclet]) {
       TerminatorInst *Terminator = BB->getTerminator();
       if (!Terminator->isExceptional())
@@ -1290,34 +1289,7 @@
       // Note that catchendpads are handled when the associated catchpad
       // is cloned.  They don't fit the pattern we're looking for here.
       BasicBlock *UnwindDest = nullptr;
-      if (auto *II = dyn_cast<InvokeInst>(Terminator)) {
-        UnwindDest = II->getUnwindDest();
-        assert(UnwindDest && "Invoke unwinds to a null destination.");
-        assert(UnwindDest->isEHPad() && "Invoke does not unwind to an EH pad.");
-        auto *EHPadInst = UnwindDest->getFirstNonPHI();
-        if (isa<CatchEndPadInst>(EHPadInst)) {
-          // If the invoke unwind destination is the unwind destination for
-          // the current child catch pad funclet, there is nothing to be done.
-          auto *CurCatch = cast<CatchPadInst>(ChildFunclet->getFirstNonPHI());
-          if (CurCatch->getUnwindDest() == UnwindDest)
-            continue;
-
-          // Otherwise, the invoke unwinds to a catch end pad that is the unwind
-          // destination another catch pad in the unwind chain from either the
-          // current catch pad or one of its clones.  If it is already the
-          // catch end pad at the end unwind chain from the current catch pad,
-          // we'll need to check the invoke instructions in the original funclet
-          // later.  Otherwise, we need to remap this invoke now.
-          BasicBlock *CurCatchEnd = getEndPadForCatch(CurCatch);
-          if (CurCatchEnd == UnwindDest)
-            NeedOrigInvokeRemapping = true;
-          else
-            II->setUnwindDest(CurCatchEnd);
-          continue;
-        }
-        // All other unwind scenarios for the invoke are handled elsewhere.
-        continue;
-      } else if (auto *I = dyn_cast<CatchPadInst>(Terminator)) {
+      if (auto *I = dyn_cast<CatchPadInst>(Terminator)) {
         UnwindDest = I->getUnwindDest();
         // The catchendpad is not a sibling destination.  This case should
         // have been handled in cloneFuncletForParent().
@@ -1380,6 +1352,43 @@
       else if (auto *I = dyn_cast<CleanupEndPadInst>(Terminator))
         I->setUnwindDest(TargetSibling);
     }
+  }
+  
+  // Invoke remapping can't be done correctly until after all of their
+  // other sibling-to-sibling unwinds have been remapped.
+  for (BasicBlock *ChildFunclet : FuncletChildren[CurFunclet]) {
+    bool NeedOrigInvokeRemapping = false;
+    for (auto *BB : FuncletBlocks[ChildFunclet]) {
+      TerminatorInst *Terminator = BB->getTerminator();
+      auto *II = dyn_cast<InvokeInst>(Terminator);
+      if (!II)
+        continue;
+
+      BasicBlock *UnwindDest = II->getUnwindDest();
+      assert(UnwindDest && "Invoke unwinds to a null destination.");
+      assert(UnwindDest->isEHPad() && "Invoke does not unwind to an EH pad.");
+      auto *EHPadInst = UnwindDest->getFirstNonPHI();
+      if (isa<CatchEndPadInst>(EHPadInst)) {
+        // If the invoke unwind destination is the unwind destination for
+        // the current child catch pad funclet, there is nothing to be done.
+        auto *CurCatch = cast<CatchPadInst>(ChildFunclet->getFirstNonPHI());
+        if (CurCatch->getUnwindDest() == UnwindDest)
+          continue;
+
+        // Otherwise, the invoke unwinds to a catch end pad that is the unwind
+        // destination another catch pad in the unwind chain from either the
+        // current catch pad or one of its clones.  If it is already the
+        // catch end pad at the end unwind chain from the current catch pad,
+        // we'll need to check the invoke instructions in the original funclet
+        // later.  Otherwise, we need to remap this invoke now.
+        BasicBlock *CurCatchEnd = getEndPadForCatch(CurCatch);
+        if (CurCatchEnd == UnwindDest)
+          NeedOrigInvokeRemapping = true;
+        else
+          II->setUnwindDest(CurCatchEnd);
+      }
+      // All other unwind scenarios for the invoke are handled elsewhere.
+    }
     if (NeedOrigInvokeRemapping) {
       // To properly remap invoke instructions that unwind to catch end pads
       // that are not the unwind destination of the catch pad funclet in which


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14454.39556.patch
Type: text/x-patch
Size: 4683 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151106/e8460fe5/attachment.bin>