Skip to content

Commit 40c76f7

Browse files
sudonataliesudonatalie
authored
Fix another UAF in SimplifyCFG (microsoft#6680)
In certain cases of unreachable code, SimplifyCFG could try to replace a phi node with a select where the phi node itself was the select's condition. This resulted in an ASAN use-after-free during SimplifyCFG. The test case added isn't quite ideal because by the end of the SimplifyCFG pass, the phi node is restored to its original state both before and after this fix. However, an ASAN build of `dxopt` or `check-clang-dxc` will identify a heap-use-after-free failure in the intermediary steps of this test without this patch and succeeds with it. This was also fixed in upstream LLVM: llvm/llvm-project@602ab24
1 parent 0b9acdb commit 40c76f7

File tree

2 files changed

+271
-0
lines changed

2 files changed

+271
-0
lines changed

‎lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1830,6 +1830,14 @@ static bool FoldTwoEntryPHINode(PHINode *PN, const TargetTransformInfo &TTI,
18301830
isa<ConstantInt>(IfCond))
18311831
return false;
18321832

1833+
// HLSL Change Begins: Patching in llvm/llvm-project@602ab24
1834+
// Don't try to fold an unreachable block. For example, the phi node itself
1835+
// can't be the candidate if-condition for a select that we want to form.
1836+
if (auto *IfCondPhiInst = dyn_cast<PHINode>(IfCond))
1837+
if (IfCondPhiInst->getParent() == BB)
1838+
return false;
1839+
// HLSL Change Ends.
1840+
18331841
// Okay, we found that we can merge this two-entry phi node into a select.
18341842
// Doing so would require us to fold *all* two entry phi nodes in this block.
18351843
// At some point this becomes non-profitable (particularly if the target

‎tools/clang/test/DXC/Passes/SimplifyCFG/simplifycfg-uaf-phi-condition.ll

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
; RUN: %dxopt %s -hlsl-passes-resume -simplifycfg -S | FileCheck %s
2+
3+
; The IR below comes from the following HLSL.
4+
; Compiling this HLSL with dxc was resulting in an ASAN use-after-free in
5+
; SimplifyCFG during FoldTwoEntryPHINode because it was deleting a PHI node
6+
; which was itself used as the condition for the select that replaced it.
7+
8+
; struct a {
9+
; int b[2];
10+
; };
11+
;
12+
; cbuffer cbuffer_c : register(b0) {
13+
; uint4 c[1];
14+
; };
15+
;
16+
; void d(inout a e, inout int f) {
17+
; int n = f;
18+
; int g = asint(c[0].x);
19+
; int s = f;
20+
; bool i = (s >= 0);
21+
; int j = (s * n);
22+
; bool k = (6 > g);
23+
; int l = 0;
24+
; bool q = (s > j);
25+
; while (true) {
26+
; while (true) {
27+
; while (true) {
28+
; if (k) {
29+
; {
30+
; int t[2] = e.b;
31+
; t[g] = n;
32+
; e.b = t;
33+
; }
34+
; }
35+
; e.b[1] = g;
36+
; e.b[0] = s;
37+
; if (q) {
38+
; break;
39+
; }
40+
; }
41+
; switch(j) {
42+
; case 0: {
43+
; break;
44+
; }
45+
; case 9: {
46+
; break;
47+
; }
48+
; default: {
49+
; {
50+
; int u[2] = e.b;
51+
; u[g] = l;
52+
; e.b = u;
53+
; }
54+
; break;
55+
; }
56+
; }
57+
; {
58+
; if (q) { break; }
59+
; }
60+
; }
61+
; {
62+
; int v[2] = e.b;
63+
; v[g] = j;
64+
; e.b = v;
65+
; }
66+
; if (!(i)) {
67+
; break;
68+
; }
69+
; }
70+
; }
71+
;
72+
; [numthreads(1, 1, 1)]
73+
; void main() {
74+
; int o = 0;
75+
; a p = (a)0;
76+
; while (true) {
77+
; bool i = (o < asint(c[0].x));
78+
; if (i) {
79+
; bool r = !(i);
80+
; if (!(r)) {
81+
; return;
82+
; }
83+
; d(p, o);
84+
; }
85+
; o = (o + 1);
86+
; }
87+
; return;
88+
; }
89+
90+
; Make sure the phi node did not get deleted by simplifycfg
91+
; CHECK: while.body:
92+
; CHECK-NEXT: %o.0 = phi i32 [ 0, %entry ], [ %add, %if.end.6 ]
93+
94+
;
95+
; Buffer Definitions:
96+
;
97+
; cbuffer cbuffer_c
98+
; {
99+
;
100+
; struct cbuffer_c
101+
; {
102+
;
103+
; uint4 c[1]; ; Offset: 0
104+
;
105+
; } cbuffer_c; ; Offset: 0 Size: 16
106+
;
107+
; }
108+
;
109+
;
110+
; Resource Bindings:
111+
;
112+
; Name Type Format Dim ID HLSL Bind Count
113+
; ------------------------------ ---------- ------- ----------- ------- -------------- ------
114+
; cbuffer_c cbuffer NA NA CB0 cb0 1
115+
;
116+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
117+
target triple = "dxil-ms-dx"
118+
119+
%cbuffer_c = type { [1 x <4 x i32>] }
120+
%dx.types.Handle = type { i8* }
121+
%dx.types.ResourceProperties = type { i32, i32 }
122+
%dx.types.CBufRet.i32 = type { i32, i32, i32, i32 }
123+
%struct.a = type { [2 x i32] }
124+
125+
@cbuffer_c = external constant %cbuffer_c
126+
@llvm.used = appending global [1 x i8*] [i8* bitcast (%cbuffer_c* @cbuffer_c to i8*)], section "llvm.metadata"
127+
128+
; Function Attrs: nounwind
129+
define void @main() #0 {
130+
entry:
131+
%0 = load %cbuffer_c, %cbuffer_c* @cbuffer_c, align 4
132+
%cbuffer_c8 = call %dx.types.Handle @dx.op.createHandleForLib.cbuffer_c(i32 160, %cbuffer_c %0) ; CreateHandleForLib(Resource)
133+
%1 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %cbuffer_c8, %dx.types.ResourceProperties { i32 13, i32 16 }) ; AnnotateHandle(res,props) resource: CBuffer
134+
%cbuffer_c = call %dx.types.Handle @dx.op.createHandleForLib.cbuffer_c(i32 160, %cbuffer_c %0) ; CreateHandleForLib(Resource)
135+
%2 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %cbuffer_c, %dx.types.ResourceProperties { i32 13, i32 16 }) ; AnnotateHandle(res,props) resource: CBuffer
136+
br label %while.body, !dbg !21 ; line:69 col:3
137+
138+
while.body: ; preds = %if.end.6, %entry
139+
%o.0 = phi i32 [ 0, %entry ], [ %add, %if.end.6 ]
140+
%3 = call %dx.types.CBufRet.i32 @dx.op.cbufferLoadLegacy.i32(i32 59, %dx.types.Handle %1, i32 0), !dbg !25 ; line:70 col:25 ; CBufferLoadLegacy(handle,regIndex)
141+
%4 = extractvalue %dx.types.CBufRet.i32 %3, 0, !dbg !25 ; line:70 col:25
142+
%cmp = icmp slt i32 %o.0, %4, !dbg !26 ; line:70 col:17
143+
br i1 %cmp, label %if.then, label %if.end.6, !dbg !27 ; line:71 col:9
144+
145+
if.then: ; preds = %while.body
146+
br i1 %cmp, label %if.then.5, label %if.end, !dbg !28 ; line:73 col:11
147+
148+
if.then.5: ; preds = %if.then
149+
ret void, !dbg !29 ; line:74 col:9
150+
151+
if.end: ; preds = %if.then
152+
%5 = call %dx.types.CBufRet.i32 @dx.op.cbufferLoadLegacy.i32(i32 59, %dx.types.Handle %2, i32 0), !dbg !30 ; line:11 col:17 ; CBufferLoadLegacy(handle,regIndex)
153+
%6 = extractvalue %dx.types.CBufRet.i32 %5, 0, !dbg !30 ; line:11 col:17
154+
%cmp.i = icmp sgt i32 %o.0, -1, !dbg !33 ; line:13 col:15
155+
%mul.i = mul nsw i32 %o.0, %o.0, !dbg !34 ; line:14 col:14
156+
%cmp1.i = icmp slt i32 %6, 6, !dbg !35 ; line:15 col:15
157+
%cmp4.i = icmp sgt i32 %o.0, %mul.i, !dbg !36 ; line:17 col:15
158+
br label %while.body.10.i, !dbg !37 ; line:18 col:3
159+
160+
while.body.10.i: ; preds = %while.end.27.i, %sw.epilog.i, %if.end.i, %if.end
161+
br i1 %cmp1.i, label %if.then.i, label %if.end.i, !dbg !38 ; line:21 col:13
162+
163+
if.then.i: ; preds = %while.body.10.i
164+
br label %if.end.i, !dbg !39 ; line:27 col:9
165+
166+
if.end.i: ; preds = %if.then.i, %while.body.10.i
167+
br i1 %cmp4.i, label %while.end.i, label %while.body.10.i, !dbg !40 ; line:30 col:13
168+
169+
while.end.i: ; preds = %if.end.i
170+
switch i32 %mul.i, label %sw.default.i [
171+
i32 0, label %sw.epilog.i
172+
i32 9, label %sw.epilog.i
173+
], !dbg !41 ; line:34 col:7
174+
175+
sw.default.i: ; preds = %while.end.i
176+
br label %sw.epilog.i, !dbg !42 ; line:47 col:11
177+
178+
sw.epilog.i: ; preds = %sw.default.i, %while.end.i, %while.end.i
179+
br i1 %cmp4.i, label %while.end.27.i, label %while.body.10.i, !dbg !43 ; line:51 col:13
180+
181+
while.end.27.i: ; preds = %sw.epilog.i
182+
br i1 %cmp.i, label %while.body.10.i, label %if.end.6, !dbg !44 ; line:59 col:9
183+
184+
if.end.6: ; preds = %while.end.27.i, %while.body
185+
%add = add nsw i32 %o.0, 1, !dbg !45 ; line:78 col:12
186+
br label %while.body, !dbg !21 ; line:69 col:3
187+
}
188+
189+
; Function Attrs: nounwind readnone
190+
declare %dx.types.Handle @"dx.hl.createhandle..%dx.types.Handle (i32, %cbuffer_c*, i32)"(i32, %cbuffer_c*, i32) #1
191+
192+
; Function Attrs: nounwind readnone
193+
declare %dx.types.Handle @"dx.hl.annotatehandle..%dx.types.Handle (i32, %dx.types.Handle, %dx.types.ResourceProperties, %cbuffer_c)"(i32, %dx.types.Handle, %dx.types.ResourceProperties, %cbuffer_c) #1
194+
195+
; Function Attrs: nounwind readonly
196+
declare %dx.types.CBufRet.i32 @dx.op.cbufferLoadLegacy.i32(i32, %dx.types.Handle, i32) #2
197+
198+
; Function Attrs: nounwind readonly
199+
declare %dx.types.Handle @dx.op.createHandleForLib.cbuffer_c(i32, %cbuffer_c) #2
200+
201+
; Function Attrs: nounwind readnone
202+
declare %dx.types.Handle @dx.op.annotateHandle(i32, %dx.types.Handle, %dx.types.ResourceProperties) #1
203+
204+
attributes #0 = { nounwind }
205+
attributes #1 = { nounwind readnone }
206+
attributes #2 = { nounwind readonly }
207+
208+
!llvm.module.flags = !{!0}
209+
!pauseresume = !{!1}
210+
!llvm.ident = !{!2}
211+
!dx.version = !{!3}
212+
!dx.valver = !{!4}
213+
!dx.shaderModel = !{!5}
214+
!dx.resources = !{!6}
215+
!dx.typeAnnotations = !{!9, !14}
216+
!dx.entryPoints = !{!18}
217+
218+
!0 = !{i32 2, !"Debug Info Version", i32 3}
219+
!1 = !{!"hlsl-dxilemit", !"hlsl-dxilload"}
220+
!2 = !{!"dxc(private) 1.8.0.14620 (main, 8408ae882)"}
221+
!3 = !{i32 1, i32 2}
222+
!4 = !{i32 1, i32 8}
223+
!5 = !{!"cs", i32 6, i32 2}
224+
!6 = !{null, null, !7, null}
225+
!7 = !{!8}
226+
!8 = !{i32 0, %cbuffer_c* @cbuffer_c, !"cbuffer_c", i32 0, i32 0, i32 1, i32 16, null}
227+
!9 = !{i32 0, %struct.a undef, !10, %cbuffer_c undef, !12}
228+
!10 = !{i32 20, !11}
229+
!11 = !{i32 6, !"b", i32 3, i32 0, i32 7, i32 4}
230+
!12 = !{i32 16, !13}
231+
!13 = !{i32 6, !"c", i32 3, i32 0, i32 7, i32 5}
232+
!14 = !{i32 1, void ()* @main, !15}
233+
!15 = !{!16}
234+
!16 = !{i32 1, !17, !17}
235+
!17 = !{}
236+
!18 = !{void ()* @main, !"main", null, !6, !19}
237+
!19 = !{i32 4, !20}
238+
!20 = !{i32 1, i32 1, i32 1}
239+
!21 = !DILocation(line: 69, column: 3, scope: !22)
240+
!22 = !DISubprogram(name: "main", scope: !23, file: !23, line: 66, type: !24, isLocal: false, isDefinition: true, scopeLine: 66, flags: DIFlagPrototyped, isOptimized: false, function: void ()* @main)
241+
!23 = !DIFile(filename: "/usr/local/google/home/chouinard/Downloads/standalone.hlsl", directory: "")
242+
!24 = !DISubroutineType(types: !17)
243+
!25 = !DILocation(line: 70, column: 25, scope: !22)
244+
!26 = !DILocation(line: 70, column: 17, scope: !22)
245+
!27 = !DILocation(line: 71, column: 9, scope: !22)
246+
!28 = !DILocation(line: 73, column: 11, scope: !22)
247+
!29 = !DILocation(line: 74, column: 9, scope: !22)
248+
!30 = !DILocation(line: 11, column: 17, scope: !31, inlinedAt: !32)
249+
!31 = !DISubprogram(name: "d", scope: !23, file: !23, line: 9, type: !24, isLocal: false, isDefinition: true, scopeLine: 9, flags: DIFlagPrototyped, isOptimized: false)
250+
!32 = distinct !DILocation(line: 76, column: 7, scope: !22)
251+
!33 = !DILocation(line: 13, column: 15, scope: !31, inlinedAt: !32)
252+
!34 = !DILocation(line: 14, column: 14, scope: !31, inlinedAt: !32)
253+
!35 = !DILocation(line: 15, column: 15, scope: !31, inlinedAt: !32)
254+
!36 = !DILocation(line: 17, column: 15, scope: !31, inlinedAt: !32)
255+
!37 = !DILocation(line: 18, column: 3, scope: !31, inlinedAt: !32)
256+
!38 = !DILocation(line: 21, column: 13, scope: !31, inlinedAt: !32)
257+
!39 = !DILocation(line: 27, column: 9, scope: !31, inlinedAt: !32)
258+
!40 = !DILocation(line: 30, column: 13, scope: !31, inlinedAt: !32)
259+
!41 = !DILocation(line: 34, column: 7, scope: !31, inlinedAt: !32)
260+
!42 = !DILocation(line: 47, column: 11, scope: !31, inlinedAt: !32)
261+
!43 = !DILocation(line: 51, column: 13, scope: !31, inlinedAt: !32)
262+
!44 = !DILocation(line: 59, column: 9, scope: !31, inlinedAt: !32)
263+
!45 = !DILocation(line: 78, column: 12, scope: !22)

0 commit comments

Comments
 (0)