Skip to content

Commit 2f8b486

Browse files
ro-iro-i
authored
[IR][JumpThreading] Fix infinite recursion on compare self-reference (#129501)
In unreachable code, constant PHI nodes may appear and be replaced by their single value. As a result, instructions may become self-referencing. This commit adds checks to avoid going into infinite recursion when handling self-referencing compare instructions in `evaluateOnPredecessorEdge()`. This LLVM defect was identified via the AMD Fuzzing project.
1 parent 4a5ff3e commit 2f8b486

File tree

3 files changed

+142
-4
lines changed

3 files changed

+142
-4
lines changed

‎llvm/include/llvm/Transforms/Scalar/JumpThreading.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,11 @@ class JumpThreadingPass : public PassInfoMixin<JumpThreadingPass> {
208208
/// if 'HasProfile' is true creates new instance through
209209
/// FunctionAnalysisManager, otherwise nullptr.
210210
BlockFrequencyInfo *getOrCreateBFI(bool Force = false);
211+
212+
// Internal overload of evaluateOnPredecessorEdge().
213+
Constant *evaluateOnPredecessorEdge(BasicBlock *BB, BasicBlock *PredPredBB,
214+
Value *cond, const DataLayout &DL,
215+
SmallPtrSet<Value *, 8> &Visited);
211216
};
212217

213218
} // end namespace llvm

‎llvm/lib/Transforms/Scalar/JumpThreading.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "llvm/ADT/DenseMap.h"
1515
#include "llvm/ADT/MapVector.h"
1616
#include "llvm/ADT/STLExtras.h"
17+
#include "llvm/ADT/ScopeExit.h"
1718
#include "llvm/ADT/SmallPtrSet.h"
1819
#include "llvm/ADT/SmallVector.h"
1920
#include "llvm/ADT/Statistic.h"
@@ -1494,6 +1495,17 @@ Constant *JumpThreadingPass::evaluateOnPredecessorEdge(BasicBlock *BB,
14941495
BasicBlock *PredPredBB,
14951496
Value *V,
14961497
const DataLayout &DL) {
1498+
SmallPtrSet<Value *, 8> Visited;
1499+
return evaluateOnPredecessorEdge(BB, PredPredBB, V, DL, Visited);
1500+
}
1501+
1502+
Constant *JumpThreadingPass::evaluateOnPredecessorEdge(
1503+
BasicBlock *BB, BasicBlock *PredPredBB, Value *V, const DataLayout &DL,
1504+
SmallPtrSet<Value *, 8> &Visited) {
1505+
if (!Visited.insert(V).second)
1506+
return nullptr;
1507+
auto _ = make_scope_exit([&Visited, V]() { Visited.erase(V); });
1508+
14971509
BasicBlock *PredBB = BB->getSinglePredecessor();
14981510
assert(PredBB && "Expected a single predecessor");
14991511

@@ -1515,12 +1527,16 @@ Constant *JumpThreadingPass::evaluateOnPredecessorEdge(BasicBlock *BB,
15151527
}
15161528

15171529
// If we have a CmpInst, try to fold it for each incoming edge into PredBB.
1530+
// Note that during the execution of the pass, phi nodes may become constant
1531+
// and may be removed, which can lead to self-referencing instructions in
1532+
// code that becomes unreachable. Consequently, we need to handle those
1533+
// instructions in unreachable code and check before going into recursion.
15181534
if (CmpInst *CondCmp = dyn_cast<CmpInst>(V)) {
15191535
if (CondCmp->getParent() == BB) {
1520-
Constant *Op0 =
1521-
evaluateOnPredecessorEdge(BB, PredPredBB, CondCmp->getOperand(0), DL);
1522-
Constant *Op1 =
1523-
evaluateOnPredecessorEdge(BB, PredPredBB, CondCmp->getOperand(1), DL);
1536+
Constant *Op0 = evaluateOnPredecessorEdge(
1537+
BB, PredPredBB, CondCmp->getOperand(0), DL, Visited);
1538+
Constant *Op1 = evaluateOnPredecessorEdge(
1539+
BB, PredPredBB, CondCmp->getOperand(1), DL, Visited);
15241540
if (Op0 && Op1) {
15251541
return ConstantFoldCompareInstOperands(CondCmp->getPredicate(), Op0,
15261542
Op1, DL);

‎llvm/test/Transforms/JumpThreading/unreachable-loops.ll

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,4 +180,121 @@ cleanup2343.loopexit4: ; preds = %cleanup1491
180180
unreachable
181181
}
182182

183+
; This segfaults due to recursion in %C4. Reason: %L6 is identified to be a
184+
; "partially redundant load" and is replaced by a PHI node. The PHI node is then
185+
; simplified to be constant and is removed. This leads to %L6 being replaced by
186+
; %C4, which makes %C4 invalid since it uses %L6.
187+
; The test case has been generated by the AMD Fuzzing project and simplified
188+
; manually and by llvm-reduce.
189+
190+
define i32 @constant_phi_leads_to_self_reference(ptr %ptr) {
191+
; CHECK-LABEL: @constant_phi_leads_to_self_reference(
192+
; CHECK-NEXT: [[A9:%.*]] = alloca i1, align 1
193+
; CHECK-NEXT: br label [[F6:%.*]]
194+
; CHECK: T3:
195+
; CHECK-NEXT: br label [[BB5:%.*]]
196+
; CHECK: BB5:
197+
; CHECK-NEXT: [[L10:%.*]] = load i1, ptr [[A9]], align 1
198+
; CHECK-NEXT: br i1 [[L10]], label [[BB6:%.*]], label [[F6]]
199+
; CHECK: BB6:
200+
; CHECK-NEXT: [[LGV3:%.*]] = load i1, ptr [[PTR:%.*]], align 1
201+
; CHECK-NEXT: [[C4:%.*]] = icmp sle i1 [[C4]], true
202+
; CHECK-NEXT: store i1 [[C4]], ptr [[PTR]], align 1
203+
; CHECK-NEXT: br i1 [[C4]], label [[F6]], label [[T3:%.*]]
204+
; CHECK: F6:
205+
; CHECK-NEXT: ret i32 0
206+
; CHECK: F7:
207+
; CHECK-NEXT: br label [[BB5]]
208+
;
209+
%A9 = alloca i1, align 1
210+
br i1 false, label %BB4, label %F6
211+
212+
BB4: ; preds = %0
213+
br i1 false, label %F6, label %F1
214+
215+
F1: ; preds = %BB4
216+
br i1 false, label %T4, label %T3
217+
218+
T3: ; preds = %T4, %BB6, %F1
219+
%L6 = load i1, ptr %ptr, align 1
220+
br label %BB5
221+
222+
BB5: ; preds = %F7, %T3
223+
%L10 = load i1, ptr %A9, align 1
224+
br i1 %L10, label %BB6, label %F6
225+
226+
BB6: ; preds = %BB5
227+
%LGV3 = load i1, ptr %ptr, align 1
228+
%C4 = icmp sle i1 %L6, true
229+
store i1 %C4, ptr %ptr, align 1
230+
br i1 %L6, label %F6, label %T3
231+
232+
T4: ; preds = %F1
233+
br label %T3
234+
235+
F6: ; preds = %BB6, %BB5, %BB4, %0
236+
ret i32 0
237+
238+
F7: ; No predecessors!
239+
br label %BB5
240+
}
241+
242+
; Same as above, but with multiple icmps referencing the same PHI node.
243+
244+
define i32 @recursive_icmp_mult(ptr %ptr) {
245+
; CHECK-LABEL: @recursive_icmp_mult(
246+
; CHECK-NEXT: [[A9:%.*]] = alloca i1, align 1
247+
; CHECK-NEXT: br label [[F6:%.*]]
248+
; CHECK: T3:
249+
; CHECK-NEXT: br label [[BB5:%.*]]
250+
; CHECK: BB5:
251+
; CHECK-NEXT: [[L10:%.*]] = load i1, ptr [[A9]], align 1
252+
; CHECK-NEXT: br i1 [[L10]], label [[BB6:%.*]], label [[F6]]
253+
; CHECK: BB6:
254+
; CHECK-NEXT: [[LGV3:%.*]] = load i1, ptr [[PTR:%.*]], align 1
255+
; CHECK-NEXT: [[C4:%.*]] = icmp sle i1 [[C6:%.*]], true
256+
; CHECK-NEXT: [[C5:%.*]] = icmp sle i1 [[C6]], false
257+
; CHECK-NEXT: [[C6]] = icmp sle i1 [[C4]], [[C5]]
258+
; CHECK-NEXT: store i1 [[C6]], ptr [[PTR]], align 1
259+
; CHECK-NEXT: br i1 [[C6]], label [[F6]], label [[T3:%.*]]
260+
; CHECK: F6:
261+
; CHECK-NEXT: ret i32 0
262+
; CHECK: F7:
263+
; CHECK-NEXT: br label [[BB5]]
264+
;
265+
%A9 = alloca i1, align 1
266+
br i1 false, label %BB4, label %F6
267+
268+
BB4: ; preds = %0
269+
br i1 false, label %F6, label %F1
270+
271+
F1: ; preds = %BB4
272+
br i1 false, label %T4, label %T3
273+
274+
T3: ; preds = %T4, %BB6, %F1
275+
%L6 = load i1, ptr %ptr, align 1
276+
br label %BB5
277+
278+
BB5: ; preds = %F7, %T3
279+
%L10 = load i1, ptr %A9, align 1
280+
br i1 %L10, label %BB6, label %F6
281+
282+
BB6: ; preds = %BB5
283+
%LGV3 = load i1, ptr %ptr, align 1
284+
%C4 = icmp sle i1 %L6, true
285+
%C5 = icmp sle i1 %L6, false
286+
%C6 = icmp sle i1 %C4, %C5
287+
store i1 %C6, ptr %ptr, align 1
288+
br i1 %L6, label %F6, label %T3
289+
290+
T4: ; preds = %F1
291+
br label %T3
292+
293+
F6: ; preds = %BB6, %BB5, %BB4, %0
294+
ret i32 0
295+
296+
F7: ; No predecessors!
297+
br label %BB5
298+
}
299+
183300
!0 = !{!"branch_weights", i32 2146410443, i32 1073205}

0 commit comments

Comments
 (0)