[PATCH] [WinEH] Demote values live across exception handlers up front
David Majnemer
david.majnemer at gmail.com
Tue Apr 21 11:20:16 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;
+
----------------
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/
More information about the llvm-commits
mailing list