[PATCH] D102817: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 2

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/BranchFolding.cpp:1207
     // If it is dead, remove it.
-    if (MBB->pred_empty()) {
+    if (MBB->pred_empty() && !MBB->hasAddressTaken()) {
       RemoveDeadBlock(MBB);
----------------
tentzen wrote:
> efriedma wrote:
> > efriedma wrote:
> > > tentzen wrote:
> > > > efriedma wrote:
> > > > > tentzen wrote:
> > > > > > JosephTremoulet wrote:
> > > > > > > do the tests (either the new ones you're adding or existing ones) exercise the case where we need this?
> > > > > > Yes, the **Dtor one.  it's how we found this bug. thanks.
> > > > > We intentionally do not check for `hasAddressTaken()` here. Even if something refers to the address, it's not legal to branch to the block, so it can't contain any useful instructions.  AsmPrinter::emitFunctionBody has some code to print a fake label if there's a reference to an erased block.
> > > > Hmm, really? there are at least three places where hasAddressTaken() is used to guard optimizations in the same pass (BranchFolding.cpp).  why is OptimizeBranches() special?
> > > The other places in the pass are checking hasAddressTaken() to try to figure out if they can rewrite the predecessors.  Here, we can trivially rewrite all the predecessors: there aren't any.
> > I guess given that we have references to an MBB stored in FuncInfo, we can't really delete the block.  So I guess this is right, sort of.  But it would still be undefined behavior to jump to it, so I'm not sure this really solves whatever problem you ran into.
> it's MBB.pred_empty(), so there is no edge jumping to it.
> Without this change, an EH Cleanuppad block could be removed and EH runtime would not be able to dtor a live object during unwinding. 
That's a contradiction: you're saying there's no edge jumping to it, but somehow the "EH runtime" is executing the code in the block.

In particular, even if you preserve the block, anything involving dominance relationships, like register allocation, is likely to destroy unreachable code.  Maybe you can sort of get away with it in trivial cases.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102817/new/

https://reviews.llvm.org/D102817