[WinEH] Don't inline an 'unwinds to caller' cleanupret into funclets which locally unwind

[majnemer]

Summary

It is problematic if the inlinee has a cleanupret which unwinds to
caller and we inline it into a call site which doesn't unwind.

If the funclet unwinds anywhere other than to the caller,
then we will give the funclet two unwind destinations.
This will result in a verifier failure.

Seeing as how the caller wasn't an invoke (which would locally unwind)
and that the funclet cannot unwind to caller, we must conclude that an
'unwind to caller' cleanupret is dynamically unreachable.

This fixes PR26698.

[JosephTremoulet]

HandleInlinedEHPad builds up the FuncletUnwindMap looking at the pre-inlining callee IR. Sharing it with the new call to getUnwindDestToken might be benign, but I'm not sure. I think it would be safer to use a separate FuncletUnwindMap for the new call. Probably also safer to compute the unwind dest in the caller before the code on line 1759 has had a chance to give the caller EH pad new uses in the partially-rewritten inlinee which might confuse the computation.

[majnemer]

• Address review feedback

[JosephTremoulet]

LGTM

[rnk]

belated lgtm

I thought we had some conversations about this situation or something like it.

llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp

if (isFuncletEHPersonality(Personality)) {
             if (CalledBB.isEHPad())
               return false;
           }
         }
       }
     }
   }
 
+  // Determine if we are dealing with a call in an EHPad which does not unwind
+  // to caller.
+  bool EHPadForCallUnwindsLocally = false;
+  if (CallSiteEHPad && CS.isCall()) {
+    UnwindDestMemoTy FuncletUnwindMap;
+    Value *CallSiteUnwindDestToken =
+        getUnwindDestToken(CallSiteEHPad, FuncletUnwindMap);
+
+    EHPadForCallUnwindsLocally =
+        CallSiteUnwindDestToken &&
+        !isa<ConstantTokenNone>(CallSiteUnwindDestToken);
+  }
+
   // Get an iterator to the last basic block in the function, which will have
   // the new function inlined after it.
   Function::iterator LastBlock = --Caller->end();
 
   // Make sure to capture all of the return instructions from the cloned
   // function.
   SmallVector<ReturnInst*, 8> Returns;
   ClonedCodeInfo InlinedFunctionInfo;
for (Function::iterator BB = FirstNewBlock->getIterator(),
           NewInst = InvokeInst::Create(cast<InvokeInst>(I), OpBundles, I);
         NewInst->takeName(I);
         I->replaceAllUsesWith(NewInst);
         I->eraseFromParent();
 
         OpBundles.clear();
       }
 
+      // It is problematic if the inlinee has a cleanupret which unwinds to
+      // caller and we inline it into a call site which doesn't unwind but into
+      // an EH pad that does.  Such an edge must be dynamically unreachable.
+      // As such, we replace the cleanupret with unreachable.
+      if (auto *CleanupRet = dyn_cast<CleanupReturnInst>(BB->getTerminator()))
+        if (CleanupRet->unwindsToCaller() && EHPadForCallUnwindsLocally)
+          changeToUnreachable(CleanupRet, /*UseLLVMTrap=*/false);
+
       Instruction *I = BB->getFirstNonPHI();
       if (!I->isEHPad())
         continue;
 
       if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(I)) {
         if (isa<ConstantTokenNone>(CatchSwitch->getParentPad()))
           CatchSwitch->setParentPad(CallSiteEHPad);
       } else {

llvm/trunk/test/Transforms/Inline/pr26698.ll

+; RUN: opt -S -inline < %s | FileCheck %s
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-pc-windows-msvc18.0.0"
+
+declare void @g(i32)
+
+define void @f() personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void @g(i32 0)
+          to label %invoke.cont unwind label %cs.bb
+
+invoke.cont:
+  ret void
+
+cs.bb:
+  %cs = catchswitch within none [label %cp.bb] unwind label %cleanup.bb
+
+cp.bb:
+  %cpouter1 = catchpad within %cs [i8* null, i32 0, i8* null]
+  call void @dtor() #1 [ "funclet"(token %cpouter1) ]
+  catchret from %cpouter1 to label %invoke.cont
+
+cleanup.bb:
+  %cpouter2 = cleanuppad within none []
+  call void @g(i32 1) [ "funclet"(token %cpouter2) ]
+  cleanupret from %cpouter2 unwind to caller
+}
+
+declare i32 @__CxxFrameHandler3(...)
+
+; Function Attrs: nounwind
+define internal void @dtor() #1 personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void @g(i32 2)
+          to label %invoke.cont unwind label %ehcleanup1
+
+invoke.cont:
+  ret void
+
+ehcleanup1:
+  %cpinner1 = cleanuppad within none []
+  invoke void @g(i32 3) [ "funclet" (token %cpinner1) ]
+          to label %done unwind label %ehcleanup2
+done:
+  unreachable
+
+ehcleanup2:
+  %cpinner2 = cleanuppad within %cpinner1 []
+  call void @g(i32 4) [ "funclet" (token %cpinner2) ]
+  cleanupret from %cpinner2 unwind to caller
+}
+
+; CHECK-LABEL: define void @f(
+
+; CHECK:      %[[cs:.*]] = catchswitch within none
+
+; CHECK:      %[[cpouter1:.*]] = catchpad within %[[cs]]
+
+; CHECK:      %[[cpinner1:.*]] = cleanuppad within %[[cpouter1]]
+
+; CHECK:      %[[cpinner2:.*]] = cleanuppad within %[[cpinner1]]
+; CHECK-NEXT: call void @g(i32 4) #0 [ "funclet"(token %[[cpinner2]]) ]
+; CHECK-NEXT: unreachable
+
+attributes #1 = { nounwind }