Skip to content

Commit 8101d18

Browse files
authored
[Support] Assert that DomTree nodes share parent (llvm#101198)
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 9bb7c11 commit 8101d18

File tree

5 files changed

+44
-2
lines changed

5 files changed

+44
-2
lines changed

llvm/include/llvm/Support/GenericDomTree.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,8 @@ class DominatorTreeBase {
397397
/// may (but is not required to) be null for a forward (backwards)
398398
/// statically unreachable block.
399399
DomTreeNodeBase<NodeT> *getNode(const NodeT *BB) const {
400+
assert((!BB || Parent == NodeTrait::getParent(const_cast<NodeT *>(BB))) &&
401+
"cannot get DomTreeNode of block with different parent");
400402
if (auto Idx = getNodeIndex(BB); Idx && *Idx < DomTreeNodes.size())
401403
return DomTreeNodes[*Idx].get();
402404
return nullptr;

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/Transforms/Scalar/AlignmentFromAssumptions.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
208208
continue;
209209

210210
if (Instruction *K = dyn_cast<Instruction>(J))
211+
if (K->getFunction() == ACall->getFunction())
211212
WorkList.push_back(K);
212213
}
213214

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.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -passes=alignment-from-assumptions -S < %s | FileCheck %s
3+
4+
; The alignment assumption is a global, which has users in a different
5+
; function. Test that in this case the dominator tree is only queried with
6+
; blocks from the same function.
7+
8+
@global = external constant [192 x i8]
9+
10+
define void @fn1() {
11+
; CHECK-LABEL: define void @fn1() {
12+
; CHECK-NEXT: call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
13+
; CHECK-NEXT: ret void
14+
;
15+
call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
16+
ret void
17+
}
18+
19+
define void @fn2() {
20+
; CHECK-LABEL: define void @fn2() {
21+
; CHECK-NEXT: ret void
22+
; CHECK: [[LOOP:.*]]:
23+
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i8, ptr @global, i64 0
24+
; CHECK-NEXT: [[LOAD:%.*]] = load i64, ptr [[GEP]], align 1
25+
; CHECK-NEXT: br label %[[LOOP]]
26+
;
27+
ret void
28+
29+
loop:
30+
%gep = getelementptr inbounds i8, ptr @global, i64 0
31+
%load = load i64, ptr %gep, align 1
32+
br label %loop
33+
}

0 commit comments

Comments
 (0)