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

Reid Kleckner rnk at google.com
Tue Apr 21 11:11:39 PDT 2015


================
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;
+
----------------
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.

================
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;
----------------
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?

================
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");
----------------
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.

http://reviews.llvm.org/D9158

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






More information about the llvm-commits mailing list