Skip to content

Commit ae235f0

Browse files
smilczekigcbot
authored andcommitted
Prevent nullptr dereference in GenerateBlockMemOpsPass
GenerateBlockMemOpsPass::checkGep() assumes that if there's a phi value, the instruction belongs to a loop. While it is a common use for phi instructions, the spirv spec doesn't disallow their use outside of loops. This commit adds a null check.
1 parent 6cb861d commit ae235f0

File tree

3 files changed

+107
-12
lines changed

3 files changed

+107
-12
lines changed

IGC/Compiler/CISACodeGen/GenerateBlockMemOpsPass.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -726,23 +726,20 @@ Value *GenerateBlockMemOpsPass::checkGep(Instruction *PtrInstr) {
726726
if (!PtrInstr)
727727
return nullptr;
728728

729-
PHINode *Phi = dyn_cast<PHINode>(PtrInstr);
730729
GetElementPtrInst *Gep = nullptr;
731-
if (Phi) {
730+
if (PHINode *Phi = dyn_cast<PHINode>(PtrInstr)) {
732731
unsigned NumIncomingValues = Phi->getNumIncomingValues();
733-
if (NumIncomingValues != 2)
732+
if (NumIncomingValues != 2) {
734733
return nullptr;
734+
}
735735

736736
BasicBlock *BB = PtrInstr->getParent();
737-
Loop *L = LI->getLoopFor(BB);
738-
BasicBlock *Preheader = L->getLoopPreheader();
739-
740-
Value *IncomingVal1 = Phi->getIncomingValueForBlock(Preheader);
741-
742-
Gep = dyn_cast<GetElementPtrInst>(IncomingVal1);
743-
744-
if (!Gep)
745-
return nullptr;
737+
// If this is not a loop, we can't be sure of the flow. Better do nothing.
738+
if (Loop *L = LI->getLoopFor(BB)) {
739+
BasicBlock *Preheader = L->getLoopPreheader();
740+
Value *IncomingVal1 = Phi->getIncomingValueForBlock(Preheader);
741+
Gep = dyn_cast<GetElementPtrInst>(IncomingVal1);
742+
}
746743
} else {
747744
Gep = dyn_cast<GetElementPtrInst>(PtrInstr);
748745
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
;=========================== begin_copyright_notice ============================
2+
;
3+
; Copyright (C) 2025 Intel Corporation
4+
;
5+
; SPDX-License-Identifier: MIT
6+
;
7+
;============================ end_copyright_notice =============================
8+
9+
; Check that we don't dereference nullptr
10+
11+
; REQUIRES: llvm-14-plus
12+
; RUN: igc_opt %s -S -o - -generate-block-mem-ops -platformpvc | FileCheck %s
13+
14+
15+
define spir_kernel void @test() {
16+
; CHECK: @test() {
17+
entry:
18+
%alloc = alloca i32
19+
br i1 true, label %BB0, label %BB1
20+
21+
BB0:
22+
%gep0 = getelementptr i32, i32* %alloc, i32 0
23+
br label %store_BB
24+
25+
BB1:
26+
%gep1 = getelementptr i32, i32* %alloc, i32 0
27+
br label %store_BB
28+
29+
store_BB:
30+
%store_addr = phi i32* [%gep0, %BB0], [%gep1, %BB1]
31+
store i32 1, i32* %store_addr
32+
ret void
33+
}
34+
35+
!igc.functions = !{!0}
36+
!IGCMetadata = !{!3}
37+
38+
!0 = !{void ()* @test, !1}
39+
!1 = !{!2, !11}
40+
!2 = !{!"function_type", i32 0}
41+
!3 = !{!"ModuleMD", !4}
42+
!4 = !{!"FuncMD", !5, !6}
43+
!5 = !{!"FuncMDMap[0]", void ()* @test}
44+
!6 = !{!"FuncMDValue[0]", !7}
45+
!7 = !{!"workGroupWalkOrder", !8, !9, !10}
46+
!8 = !{!"dim0", i32 0}
47+
!9 = !{!"dim1", i32 1}
48+
!10 = !{!"dim2", i32 2}
49+
!11 = !{!"thread_group_size", i32 32, i32 32, i32 32}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
;=========================== begin_copyright_notice ============================
2+
;
3+
; Copyright (C) 2025 Intel Corporation
4+
;
5+
; SPDX-License-Identifier: MIT
6+
;
7+
;============================ end_copyright_notice =============================
8+
9+
; Check that we don't dereference nullptr
10+
11+
; REQUIRES: llvm-14-plus
12+
; RUN: igc_opt --opaque-pointers %s -S -o - -generate-block-mem-ops -platformpvc | FileCheck %s
13+
14+
15+
define spir_kernel void @test() {
16+
; CHECK: @test() {
17+
entry:
18+
%alloc = alloca i32, align 4
19+
br i1 true, label %BB0, label %BB1
20+
21+
BB0: ; preds = %entry
22+
%gep0 = getelementptr i32, ptr %alloc, i32 0
23+
br label %store_BB
24+
25+
BB1: ; preds = %entry
26+
%gep1 = getelementptr i32, ptr %alloc, i32 0
27+
br label %store_BB
28+
29+
store_BB: ; preds = %BB1, %BB0
30+
%store_addr = phi ptr [ %gep0, %BB0 ], [ %gep1, %BB1 ]
31+
store i32 1, ptr %store_addr, align 4
32+
ret void
33+
}
34+
35+
!igc.functions = !{!0}
36+
!IGCMetadata = !{!4}
37+
38+
!0 = !{ptr @test, !1}
39+
!1 = !{!2, !3}
40+
!2 = !{!"function_type", i32 0}
41+
!3 = !{!"thread_group_size", i32 32, i32 32, i32 32}
42+
!4 = !{!"ModuleMD", !5}
43+
!5 = !{!"FuncMD", !6, !7}
44+
!6 = !{!"FuncMDMap[0]", ptr @test}
45+
!7 = !{!"FuncMDValue[0]", !8}
46+
!8 = !{!"workGroupWalkOrder", !9, !10, !11}
47+
!9 = !{!"dim0", i32 0}
48+
!10 = !{!"dim1", i32 1}
49+
!11 = !{!"dim2", i32 2}

0 commit comments

Comments
 (0)