[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