[PATCH] [WinEH] Demote values live across exception handlers up front

[David Majnemer]

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:452-457
@@ +451,8 @@
+      for (Value *Op : I.operands()) {
+        // Don't demote static allocas, constants, and labels.
+        if (isa<Constant>(Op) || isa<BasicBlock>(Op))
+          continue;
+        auto *AI = dyn_cast<AllocaInst>(Op);
+        if (AI && AI->isStaticAlloca())
+          continue;
+
----------------
rnk wrote:
> majnemer wrote:
> > Is it possible to demote a dynamic alloca in the exceptional path?
> Yes, it will produce a dangling pointer. It has the same semantics as the source one would write for it:
> ```
> void f() {
>   int *data = nullptr;
>   try {
>     throw 42;
>   } catch (int e) {
>     data = (int*)alloca(e);
>   }
>   data[0] = 42; // store to dead stack memory
> }
> ```
> Rejecting this isn't our job, it's Clang's.
What if the programmer wasn't at fault?

Let's say we had:
  int n;
  void f() {
    int *x = (int *)alloca(n);
    try {
      throw 42;
    } catch (int e) {
      do_something(x);
      return;
    }
  }

Is it unreasonable for LLVM to sink the dynamic alloca into the catch block?

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:521-522
@@ +520,4 @@
+    for (Instruction &Inst : *LPadBB) {
+      if (auto *IntrinCall = dyn_cast<IntrinsicInst>(&Inst)) {
+        if (IntrinCall->getIntrinsicID() == Intrinsic::eh_actions)
+          return false;
----------------
rnk wrote:
> majnemer wrote:
> > You could make this just:
> >   if (match(&Inst, m_Intrinsic<Intrinsic::eh_actions>())
> >     return false;
> Won't that require that the intrinsic call has no arguments?
No: http://llvm.org/docs/doxygen/html/PatternMatch_8h_source.html#l01193

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:1468-1469
@@ -1348,6 +1467,4 @@
   if (isa<Instruction>(V) || isa<Argument>(V)) {
-    AllocaInst *NewAlloca =
-        Builder.CreateAlloca(V->getType(), nullptr, "eh.temp.alloca");
-    FrameVarInfo[V].push_back(NewAlloca);
-    LoadInst *NewLoad = Builder.CreateLoad(NewAlloca, V->getName() + ".reload");
-    return NewLoad;
+    dbgs() << "Failed to demote instruction used in exception handler:\n";
+    dbgs() << "  " << *V << '\n';
+    report_fatal_error("WinEHPrepare failed to demote instruction");
----------------
rnk wrote:
> majnemer wrote:
> > Wrap these in DEBUG?
> Let's make this errs(). I really want this check to stay in for a release build. It's not expensive, and it's the #1 place where outlining can fall down and go off the rails. We should emit a reasonable crash diagnostic.
Sounds good.

http://reviews.llvm.org/D9158

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/