Skip to content

Commit ece2d37

Browse files
spaitsspaits
committed
Reland [BasicBlockUtils] Handle funclets when detaching EH pad blocks (#157363)
Fixes #148052 . When removing EH Pad blocks, the value defined by them becomes poison. These poison values are then used by `catchret` and `cleanupret`, which is invalid. This commit replaces those unreachable `catchret` and `cleanupret` instructions with `unreachable`.
1 parent b30c29c commit ece2d37

File tree

2 files changed

+270
-24
lines changed

2 files changed

+270
-24
lines changed

‎llvm/lib/Transforms/Utils/BasicBlockUtils.cpp‎

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,37 +58,77 @@ static cl::opt<unsigned> MaxDeoptOrUnreachableSuccessorCheckDepth(
5858
"is followed by a block that either has a terminating "
5959
"deoptimizing call or is terminated with an unreachable"));
6060

61-
void llvm::detachDeadBlocks(
62-
ArrayRef<BasicBlock *> BBs,
63-
SmallVectorImpl<DominatorTree::UpdateType> *Updates,
61+
static void zapAllInstructionInDeadBasicBlock(BasicBlock *BB) {
62+
while (!BB->empty()) {
63+
Instruction &I = BB->back();
64+
// If this instruction is used, replace uses with an arbitrary value.
65+
// Because control flow can't get here, we don't care what we replace the
66+
// value with. Note that since this block is unreachable, and all values
67+
// contained within it must dominate their uses, that all uses will
68+
// eventually be removed (they are themselves dead).
69+
if (!I.use_empty())
70+
I.replaceAllUsesWith(PoisonValue::get(I.getType()));
71+
BB->back().eraseFromParent();
72+
}
73+
new UnreachableInst(BB->getContext(), BB);
74+
assert(BB->size() == 1 && isa<UnreachableInst>(BB->getTerminator()) &&
75+
"The successor list of BB isn't empty before "
76+
"applying corresponding DTU updates.");
77+
}
78+
79+
static void deleteBasicBlockFromSuccessor(
80+
BasicBlock *BB, SmallVectorImpl<DominatorTree::UpdateType> *Updates,
6481
bool KeepOneInputPHIs) {
82+
SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
83+
for (BasicBlock *Succ : successors(BB)) {
84+
Succ->removePredecessor(BB, KeepOneInputPHIs);
85+
if (Updates && UniqueSuccessors.insert(Succ).second)
86+
Updates->push_back({DominatorTree::Delete, BB, Succ});
87+
}
88+
}
89+
90+
static void replaceFuncletPadsRetWithUnreachable(
91+
Instruction &I, SmallVectorImpl<DominatorTree::UpdateType> *Updates,
92+
bool KeepOneInputPHIs) {
93+
assert(isa<FuncletPadInst>(I) && "Instruction must be a funclet pad!");
94+
for (User *User : make_early_inc_range(I.users())) {
95+
Instruction *ReturnInstr = dyn_cast<Instruction>(User);
96+
// If we have a cleanupret or catchret block, replace it with just an
97+
// unreachable.
98+
if (isa<CatchReturnInst>(ReturnInstr) ||
99+
isa<CleanupReturnInst>(ReturnInstr)) {
100+
BasicBlock *ReturnInstrBB = ReturnInstr->getParent();
101+
// This catchret or catchpad basic block is detached now. Let the
102+
// successors know it.
103+
deleteBasicBlockFromSuccessor(ReturnInstrBB, Updates, KeepOneInputPHIs);
104+
zapAllInstructionInDeadBasicBlock(ReturnInstrBB);
105+
}
106+
}
107+
}
108+
109+
void llvm::detachDeadBlocks(ArrayRef<BasicBlock *> BBs,
110+
SmallVectorImpl<DominatorTree::UpdateType> *Updates,
111+
bool KeepOneInputPHIs) {
65112
for (auto *BB : BBs) {
66113
// Loop through all of our successors and make sure they know that one
67114
// of their predecessors is going away.
68115
SmallPtrSet<BasicBlock *, 4> UniqueSuccessors;
69-
for (BasicBlock *Succ : successors(BB)) {
70-
Succ->removePredecessor(BB, KeepOneInputPHIs);
71-
if (Updates && UniqueSuccessors.insert(Succ).second)
72-
Updates->push_back({DominatorTree::Delete, BB, Succ});
116+
auto NonFirstPhiIt = BB->getFirstNonPHIIt();
117+
if (NonFirstPhiIt != BB->end()) {
118+
Instruction &I = *NonFirstPhiIt;
119+
// Exception handling funclets need to be explicitly addressed.
120+
// These funclets must begin with cleanuppad or catchpad and end with
121+
// cleanupred or catchret. The return instructions can be in different
122+
// basic blocks than the pad instruction. If we would only delete the
123+
// first block, the we would have possible cleanupret and catchret
124+
// instructions with poison arguments, which wouldn't be valid.
125+
if (isa<FuncletPadInst>(I))
126+
replaceFuncletPadsRetWithUnreachable(I, Updates, KeepOneInputPHIs);
73127
}
74128

75-
// Zap all the instructions in the block.
76-
while (!BB->empty()) {
77-
Instruction &I = BB->back();
78-
// If this instruction is used, replace uses with an arbitrary value.
79-
// Because control flow can't get here, we don't care what we replace the
80-
// value with. Note that since this block is unreachable, and all values
81-
// contained within it must dominate their uses, that all uses will
82-
// eventually be removed (they are themselves dead).
83-
if (!I.use_empty())
84-
I.replaceAllUsesWith(PoisonValue::get(I.getType()));
85-
BB->back().eraseFromParent();
86-
}
87-
new UnreachableInst(BB->getContext(), BB);
88-
assert(BB->size() == 1 &&
89-
isa<UnreachableInst>(BB->getTerminator()) &&
90-
"The successor list of BB isn't empty before "
91-
"applying corresponding DTU updates.");
129+
// Detaching and emptying the current basic block.
130+
deleteBasicBlockFromSuccessor(BB, Updates, KeepOneInputPHIs);
131+
zapAllInstructionInDeadBasicBlock(BB);
92132
}
93133
}
94134

‎llvm/test/Transforms/SimplifyCFG/unreachable-multi-basic-block-funclet.ll‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -passes=simplifycfg -S < %s | FileCheck %s
3+
4+
; cleanuppad/cleanupret
5+
6+
define void @unreachable_cleanuppad_linear(i64 %shapes.1) personality ptr null {
7+
; CHECK-LABEL: define void @unreachable_cleanuppad_linear(
8+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
9+
; CHECK-NEXT: [[START:.*:]]
10+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
11+
; CHECK-NEXT: ret void
12+
;
13+
start:
14+
%_7 = icmp ult i64 0, %shapes.1
15+
ret void
16+
17+
funclet:
18+
%cleanuppad = cleanuppad within none []
19+
br label %funclet_end
20+
21+
funclet_end:
22+
cleanupret from %cleanuppad unwind to caller
23+
}
24+
25+
define void @unreachable_cleanuppad_multiple_predecessors(i64 %shapes.1) personality ptr null {
26+
; CHECK-LABEL: define void @unreachable_cleanuppad_multiple_predecessors(
27+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
28+
; CHECK-NEXT: [[START:.*:]]
29+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
30+
; CHECK-NEXT: ret void
31+
;
32+
start:
33+
%_7 = icmp ult i64 0, %shapes.1
34+
ret void
35+
36+
funclet:
37+
%cleanuppad = cleanuppad within none []
38+
switch i64 %shapes.1, label %otherwise [ i64 0, label %one
39+
i64 1, label %two
40+
i64 42, label %three ]
41+
one:
42+
br label %funclet_end
43+
44+
two:
45+
br label %funclet_end
46+
47+
three:
48+
br label %funclet_end
49+
50+
otherwise:
51+
br label %funclet_end
52+
53+
funclet_end:
54+
cleanupret from %cleanuppad unwind to caller
55+
}
56+
57+
; catchpad/catchret
58+
59+
define void @unreachable_catchpad_linear(i64 %shapes.1) personality ptr null {
60+
; CHECK-LABEL: define void @unreachable_catchpad_linear(
61+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
62+
; CHECK-NEXT: [[START:.*:]]
63+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
64+
; CHECK-NEXT: ret void
65+
;
66+
start:
67+
%_7 = icmp ult i64 0, %shapes.1
68+
ret void
69+
70+
dispatch:
71+
%cs = catchswitch within none [label %funclet] unwind to caller
72+
73+
funclet:
74+
%cleanuppad = catchpad within %cs []
75+
br label %funclet_end
76+
77+
78+
funclet_end:
79+
catchret from %cleanuppad to label %unreachable
80+
81+
unreachable:
82+
unreachable
83+
}
84+
85+
define void @unreachable_catchpad_multiple_predecessors(i64 %shapes.1) personality ptr null {
86+
; CHECK-LABEL: define void @unreachable_catchpad_multiple_predecessors(
87+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
88+
; CHECK-NEXT: [[START:.*:]]
89+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
90+
; CHECK-NEXT: ret void
91+
;
92+
start:
93+
%_7 = icmp ult i64 0, %shapes.1
94+
ret void
95+
96+
dispatch:
97+
%cs = catchswitch within none [label %funclet] unwind to caller
98+
99+
funclet:
100+
%cleanuppad = catchpad within %cs []
101+
switch i64 %shapes.1, label %otherwise [ i64 0, label %one
102+
i64 1, label %two
103+
i64 42, label %three ]
104+
one:
105+
br label %funclet_end
106+
107+
two:
108+
br label %funclet_end
109+
110+
three:
111+
br label %funclet_end
112+
113+
otherwise:
114+
br label %funclet_end
115+
116+
funclet_end:
117+
catchret from %cleanuppad to label %unreachable
118+
119+
unreachable:
120+
unreachable
121+
}
122+
123+
; Issue reproducer
124+
125+
define void @gh148052(i64 %shapes.1) personality ptr null {
126+
; CHECK-LABEL: define void @gh148052(
127+
; CHECK-SAME: i64 [[SHAPES_1:%.*]]) personality ptr null {
128+
; CHECK-NEXT: [[START:.*:]]
129+
; CHECK-NEXT: [[_7:%.*]] = icmp ult i64 0, [[SHAPES_1]]
130+
; CHECK-NEXT: call void @llvm.assume(i1 [[_7]])
131+
; CHECK-NEXT: ret void
132+
;
133+
start:
134+
%_7 = icmp ult i64 0, %shapes.1
135+
br i1 %_7, label %bb1, label %panic
136+
137+
bb1:
138+
%_11 = icmp ult i64 0, %shapes.1
139+
br i1 %_11, label %bb3, label %panic1
140+
141+
panic:
142+
unreachable
143+
144+
bb3:
145+
ret void
146+
147+
panic1:
148+
invoke void @func(i64 0, i64 0, ptr null)
149+
to label %unreachable unwind label %funclet_bb14
150+
151+
funclet_bb14:
152+
%cleanuppad = cleanuppad within none []
153+
br label %bb13
154+
155+
unreachable:
156+
unreachable
157+
158+
bb10:
159+
cleanupret from %cleanuppad5 unwind to caller
160+
161+
funclet_bb10:
162+
%cleanuppad5 = cleanuppad within none []
163+
br label %bb10
164+
165+
bb13:
166+
cleanupret from %cleanuppad unwind label %funclet_bb10
167+
}
168+
169+
%struct.foo = type { ptr, %struct.eggs, ptr }
170+
%struct.eggs = type { ptr, ptr, ptr }
171+
172+
declare x86_thiscallcc ptr @quux(ptr, ptr, i32)
173+
174+
define x86_thiscallcc ptr @baz(ptr %arg, ptr %arg1, ptr %arg2, i1 %arg3, ptr %arg4) personality ptr null {
175+
bb:
176+
%alloca = alloca [2 x %struct.foo], align 4
177+
%invoke = invoke x86_thiscallcc ptr @quux(ptr null, ptr null, i32 0)
178+
to label %bb5 unwind label %bb10
179+
180+
bb5: ; preds = %bb
181+
%getelementptr = getelementptr i8, ptr %arg, i32 20
182+
%call = call x86_thiscallcc ptr null(ptr null, ptr null, i32 0)
183+
br label %bb6
184+
185+
bb6: ; preds = %bb10, %bb5
186+
%phi = phi ptr [ null, %bb10 ], [ null, %bb5 ]
187+
ret ptr %phi
188+
189+
bb7: ; No predecessors!
190+
%cleanuppad = cleanuppad within none []
191+
%getelementptr8 = getelementptr i8, ptr %arg2, i32 -20
192+
%icmp = icmp eq ptr %arg, null
193+
br label %bb9
194+
195+
bb9: ; preds = %bb7
196+
cleanupret from %cleanuppad unwind label %bb10
197+
198+
bb10: ; preds = %bb9, %bb
199+
%phi11 = phi ptr [ %arg, %bb9 ], [ null, %bb ]
200+
%cleanuppad12 = cleanuppad within none []
201+
%getelementptr13 = getelementptr i8, ptr %phi11, i32 -20
202+
store i32 0, ptr %phi11, align 4
203+
br label %bb6
204+
}
205+
206+
declare void @func(i64, i64, ptr)

0 commit comments

Comments
 (0)