Skip to content

Commit c8bec47

Browse files
committed
[Support] Assert that DomTree nodes share parent
A dominance query of a block that is in a different function is ill-defined, so assert that getNode() is only called for blocks that are in the same function. There are two cases, where this behavior did occur. LoopFuse didn't explicitly do this, but didn't invalidate the SCEV block dispositions, leaving dangling pointers to free'ed basic blocks behind, causing use-after-free. We do, however, want to be able to dereference basic blocks inside the dominator tree, so that we can refer to them by a number stored inside the basic block.
1 parent 7fc945e commit c8bec47

File tree

4 files changed

+14
-2
lines changed

4 files changed

+14
-2
lines changed

llvm/include/llvm/Support/GenericDomTree.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,8 @@ class DominatorTreeBase {
359359
/// may (but is not required to) be null for a forward (backwards)
360360
/// statically unreachable block.
361361
DomTreeNodeBase<NodeT> *getNode(const NodeT *BB) const {
362+
assert((!BB || Parent == NodeTrait::getParent(const_cast<NodeT *>(BB))) &&
363+
"cannot get DomTreeNode of block with different parent");
362364
auto I = DomTreeNodes.find(BB);
363365
if (I != DomTreeNodes.end())
364366
return I->second.get();

llvm/lib/Analysis/TypeMetadataUtils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
3333
// after indirect call promotion and inlining, where we may have uses
3434
// of the vtable pointer guarded by a function pointer check, and a fallback
3535
// indirect call.
36+
if (CI->getFunction() != User->getFunction())
37+
continue;
3638
if (!DT.dominates(CI, User))
3739
continue;
3840
if (isa<BitCastInst>(User)) {

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,10 @@ bool llvm::isValidAssumeForContext(const Instruction *Inv,
526526
return AllowEphemerals || !isEphemeralValueOf(Inv, CxtI);
527527
}
528528

529+
// Inv and CxtI are in different functions.
530+
if (Inv->getFunction() != CxtI->getFunction())
531+
return false;
532+
529533
// Inv and CxtI are in different blocks.
530534
if (DT) {
531535
if (DT->dominates(Inv, CxtI))

llvm/lib/Transforms/Scalar/LoopFuse.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1729,7 +1729,9 @@ struct LoopFuser {
17291729
// mergeLatch may remove the only block in FC1.
17301730
SE.forgetLoop(FC1.L);
17311731
SE.forgetLoop(FC0.L);
1732-
SE.forgetLoopDispositions();
1732+
// Forget block dispositions as well, so that there are no dangling
1733+
// pointers to erased/free'ed blocks.
1734+
SE.forgetBlockAndLoopDispositions();
17331735

17341736
// Move instructions from FC0.Latch to FC1.Latch.
17351737
// Note: mergeLatch requires an updated DT.
@@ -2023,7 +2025,9 @@ struct LoopFuser {
20232025
// mergeLatch may remove the only block in FC1.
20242026
SE.forgetLoop(FC1.L);
20252027
SE.forgetLoop(FC0.L);
2026-
SE.forgetLoopDispositions();
2028+
// Forget block dispositions as well, so that there are no dangling
2029+
// pointers to erased/free'ed blocks.
2030+
SE.forgetBlockAndLoopDispositions();
20272031

20282032
// Move instructions from FC0.Latch to FC1.Latch.
20292033
// Note: mergeLatch requires an updated DT.

0 commit comments

Comments
 (0)