Skip to content

Commit

Permalink
[Support] Assert that DomTree nodes share parent (llvm#101198)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aengelke authored and bwendling committed Aug 15, 2024
1 parent 450bd41 commit fbff163
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 2 deletions.
2 changes: 2 additions & 0 deletions llvm/include/llvm/Support/GenericDomTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ class DominatorTreeBase {
/// may (but is not required to) be null for a forward (backwards)
/// statically unreachable block.
DomTreeNodeBase<NodeT> *getNode(const NodeT *BB) const {
assert((!BB || Parent == NodeTrait::getParent(const_cast<NodeT *>(BB))) &&
"cannot get DomTreeNode of block with different parent");
if (auto Idx = getNodeIndex(BB); Idx && *Idx < DomTreeNodes.size())
return DomTreeNodes[*Idx].get();
return nullptr;
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Analysis/TypeMetadataUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ findCallsAtConstantOffset(SmallVectorImpl<DevirtCallSite> &DevirtCalls,
// after indirect call promotion and inlining, where we may have uses
// of the vtable pointer guarded by a function pointer check, and a fallback
// indirect call.
if (CI->getFunction() != User->getFunction())
continue;
if (!DT.dominates(CI, User))
continue;
if (isa<BitCastInst>(User)) {
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall,
continue;

if (Instruction *K = dyn_cast<Instruction>(J))
if (K->getFunction() == ACall->getFunction())
WorkList.push_back(K);
}

Expand Down
8 changes: 6 additions & 2 deletions llvm/lib/Transforms/Scalar/LoopFuse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1729,7 +1729,9 @@ struct LoopFuser {
// mergeLatch may remove the only block in FC1.
SE.forgetLoop(FC1.L);
SE.forgetLoop(FC0.L);
SE.forgetLoopDispositions();
// Forget block dispositions as well, so that there are no dangling
// pointers to erased/free'ed blocks.
SE.forgetBlockAndLoopDispositions();

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

// Move instructions from FC0.Latch to FC1.Latch.
// Note: mergeLatch requires an updated DT.
Expand Down
33 changes: 33 additions & 0 deletions llvm/test/Transforms/AlignmentFromAssumptions/domtree-crash.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -passes=alignment-from-assumptions -S < %s | FileCheck %s

; The alignment assumption is a global, which has users in a different
; function. Test that in this case the dominator tree is only queried with
; blocks from the same function.

@global = external constant [192 x i8]

define void @fn1() {
; CHECK-LABEL: define void @fn1() {
; CHECK-NEXT: call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
; CHECK-NEXT: ret void
;
call void @llvm.assume(i1 false) [ "align"(ptr @global, i64 1) ]
ret void
}

define void @fn2() {
; CHECK-LABEL: define void @fn2() {
; CHECK-NEXT: ret void
; CHECK: [[LOOP:.*]]:
; CHECK-NEXT: [[GEP:%.*]] = getelementptr inbounds i8, ptr @global, i64 0
; CHECK-NEXT: [[LOAD:%.*]] = load i64, ptr [[GEP]], align 1
; CHECK-NEXT: br label %[[LOOP]]
;
ret void

loop:
%gep = getelementptr inbounds i8, ptr @global, i64 0
%load = load i64, ptr %gep, align 1
br label %loop
}

0 comments on commit fbff163

Please sign in to comment.