Skip to content

Commit 845d2d3

Browse files
nikicllvmbot
authored andcommitted
[GVN] Fix use-after-free in load PRE with select available value (#69314)
replaceValuesPerBlockEntry() only handled simple and coerced load values, however the load may also be referenced by a select value. Additionally, I suspect that the previous code might have been incorrect if a load had an offset, as it always constructed the AvailableValue from scratch. Fixes llvm/llvm-project#69301. (cherry picked from commit 7f1733a252cbbad74445bd54dc95aeec52bb3199)
1 parent 888437e commit 845d2d3

File tree

2 files changed

+67
-3
lines changed

2 files changed

+67
-3
lines changed

llvm/lib/Transforms/Scalar/GVN.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -946,9 +946,14 @@ static void replaceValuesPerBlockEntry(
946946
SmallVectorImpl<AvailableValueInBlock> &ValuesPerBlock, Value *OldValue,
947947
Value *NewValue) {
948948
for (AvailableValueInBlock &V : ValuesPerBlock) {
949-
if ((V.AV.isSimpleValue() && V.AV.getSimpleValue() == OldValue) ||
950-
(V.AV.isCoercedLoadValue() && V.AV.getCoercedLoadValue() == OldValue))
951-
V = AvailableValueInBlock::get(V.BB, NewValue);
949+
if (V.AV.Val == OldValue)
950+
V.AV.Val = NewValue;
951+
if (V.AV.isSelectValue()) {
952+
if (V.AV.V1 == OldValue)
953+
V.AV.V1 = NewValue;
954+
if (V.AV.V2 == OldValue)
955+
V.AV.V2 = NewValue;
956+
}
952957
}
953958
}
954959

llvm/test/Transforms/GVN/pr69301.ll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
2+
; RUN: opt -S -passes=gvn < %s | FileCheck %s
3+
4+
; Make sure we don't have use-after-free due to dangling values in
5+
; select available value.
6+
7+
define i64 @test(i1 %c, ptr %p) {
8+
; CHECK-LABEL: define i64 @test(
9+
; CHECK-SAME: i1 [[C:%.*]], ptr [[P:%.*]]) {
10+
; CHECK-NEXT: entry:
11+
; CHECK-NEXT: br label [[LOOP:%.*]]
12+
; CHECK: loop:
13+
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[LOOP_LATCH:%.*]] ]
14+
; CHECK-NEXT: [[PTR_IV:%.*]] = phi ptr [ [[P]], [[ENTRY]] ], [ [[SELECT:%.*]], [[LOOP_LATCH]] ]
15+
; CHECK-NEXT: [[ICMP:%.*]] = icmp eq i64 [[IV]], 0
16+
; CHECK-NEXT: br i1 [[ICMP]], label [[LOOP_EXIT_CRIT_EDGE:%.*]], label [[LOOP_CONT:%.*]]
17+
; CHECK: loop.exit_crit_edge:
18+
; CHECK-NEXT: [[RES_PRE:%.*]] = load i64, ptr [[PTR_IV]], align 8
19+
; CHECK-NEXT: br label [[EXIT:%.*]]
20+
; CHECK: loop.cont:
21+
; CHECK-NEXT: [[ADD]] = add i64 [[IV]], -1
22+
; CHECK-NEXT: [[RES_PRE1:%.*]] = load i64, ptr [[PTR_IV]], align 8
23+
; CHECK-NEXT: br i1 [[C]], label [[EXITSPLIT:%.*]], label [[LOOP_LATCH]]
24+
; CHECK: loop.latch:
25+
; CHECK-NEXT: [[LOAD6:%.*]] = load i64, ptr [[P]], align 8
26+
; CHECK-NEXT: [[ICMP7:%.*]] = icmp ugt i64 [[RES_PRE1]], [[LOAD6]]
27+
; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[ICMP7]], i64 [[RES_PRE1]], i64 [[LOAD6]]
28+
; CHECK-NEXT: [[SELECT]] = select i1 [[ICMP7]], ptr [[PTR_IV]], ptr [[P]]
29+
; CHECK-NEXT: br label [[LOOP]]
30+
; CHECK: exitsplit:
31+
; CHECK-NEXT: br label [[EXIT]]
32+
; CHECK: exit:
33+
; CHECK-NEXT: [[RES:%.*]] = phi i64 [ [[RES_PRE1]], [[EXITSPLIT]] ], [ [[RES_PRE]], [[LOOP_EXIT_CRIT_EDGE]] ]
34+
; CHECK-NEXT: ret i64 [[RES]]
35+
;
36+
entry:
37+
br label %loop
38+
39+
loop:
40+
%iv = phi i64 [ 0, %entry ], [ %add, %loop.latch ]
41+
%ptr.iv = phi ptr [ %p, %entry ], [ %select, %loop.latch ]
42+
%icmp = icmp eq i64 %iv, 0
43+
br i1 %icmp, label %exit, label %loop.cont
44+
45+
loop.cont:
46+
%add = add i64 %iv, -1
47+
br i1 %c, label %exit, label %loop.latch
48+
49+
loop.latch:
50+
%load = load i64, ptr %ptr.iv, align 8
51+
%load6 = load i64, ptr %p, align 8
52+
%icmp7 = icmp ugt i64 %load, %load6
53+
%select = select i1 %icmp7, ptr %ptr.iv, ptr %p
54+
br label %loop
55+
56+
exit:
57+
%res = load i64, ptr %ptr.iv, align 8
58+
ret i64 %res
59+
}

0 commit comments

Comments
 (0)