Skip to content

Commit 281b55e

Browse files
spencer-lunargspencer-lunarg
authored
Fix crash in non-struct physical storage pointers (#289)
1 parent 7082a63 commit 281b55e

File tree

4 files changed

+242
-17
lines changed

4 files changed

+242
-17
lines changed

‎spirv_reflect.c‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,6 +1986,13 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
19861986
p_type->type_flags |= SPV_REFLECT_TYPE_FLAG_REF;
19871987
IF_READU32_CAST(result, p_parser, p_node->word_offset + 2, SpvStorageClass, p_type->storage_class);
19881988

1989+
SpvReflectPrvNode* p_next_node = FindNode(p_parser, p_node->type_id);
1990+
if (IsNull(p_next_node)) {
1991+
result = SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
1992+
SPV_REFLECT_ASSERT(false);
1993+
break;
1994+
}
1995+
19891996
bool found_recursion = false;
19901997
if (p_type->storage_class == SpvStorageClassPhysicalStorageBuffer) {
19911998
// Need to make sure we haven't started an infinite recursive loop
@@ -1997,7 +2004,7 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
19972004
return SPV_REFLECT_RESULT_SUCCESS;
19982005
}
19992006
}
2000-
if (!found_recursion) {
2007+
if (!found_recursion && p_next_node->op == SpvOpTypeStruct) {
20012008
p_parser->physical_pointer_struct_count++;
20022009
p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_type;
20032010
p_parser->physical_pointer_count++;
@@ -2007,12 +2014,7 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
20072014
}
20082015
}
20092016

2010-
// Parse type
2011-
SpvReflectPrvNode* p_next_node = FindNode(p_parser, p_node->type_id);
2012-
if (IsNull(p_next_node)) {
2013-
result = SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
2014-
SPV_REFLECT_ASSERT(false);
2015-
} else if (!found_recursion) {
2017+
if (!found_recursion) {
20162018
if (p_next_node->op == SpvOpTypeStruct) {
20172019
p_type->struct_type_description = FindType(p_module, p_next_node->result_id);
20182020
}
@@ -2545,15 +2547,19 @@ static SpvReflectResult ParseDescriptorBlockVariable(SpvReflectPrvParser* p_pars
25452547
}
25462548
}
25472549
if (!found_recursion) {
2548-
uint32_t struct_id = FindType(p_module, p_member_type->id)->struct_type_description->id;
2549-
p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].struct_id = struct_id;
2550-
p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].p_var = p_member_var;
2551-
p_parser->physical_pointer_struct_count++;
2552-
2553-
p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_member_type;
2554-
p_parser->physical_pointer_count++;
2555-
if (p_parser->physical_pointer_count >= MAX_RECURSIVE_PHYSICAL_POINTER_CHECK) {
2556-
return SPV_REFLECT_RESULT_ERROR_SPIRV_MAX_RECURSIVE_EXCEEDED;
2550+
SpvReflectTypeDescription* struct_type = FindType(p_module, p_member_type->id);
2551+
// could be pointer directly to non-struct type here
2552+
if (struct_type->struct_type_description) {
2553+
uint32_t struct_id = struct_type->struct_type_description->id;
2554+
p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].struct_id = struct_id;
2555+
p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].p_var = p_member_var;
2556+
p_parser->physical_pointer_struct_count++;
2557+
2558+
p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_member_type;
2559+
p_parser->physical_pointer_count++;
2560+
if (p_parser->physical_pointer_count >= MAX_RECURSIVE_PHYSICAL_POINTER_CHECK) {
2561+
return SPV_REFLECT_RESULT_ERROR_SPIRV_MAX_RECURSIVE_EXCEEDED;
2562+
}
25572563
}
25582564
}
25592565

@@ -2724,7 +2730,13 @@ static SpvReflectResult ParseDescriptorBlockVariableSizes(SpvReflectPrvParser* p
27242730
if (IsNull(p_member_type)) {
27252731
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
27262732
}
2727-
assert(p_member_type->op == SpvOpTypeStruct);
2733+
2734+
// If we found a struct, we need to fall through and get the size of it or else we grab the size here
2735+
if (p_member_type->op != SpvOpTypeStruct) {
2736+
// TODO - we need to rework this loop as a function to get size for each type
2737+
// (or maybe determine this size doesn't matter if not a struct in the pointer)
2738+
break;
2739+
}
27282740
FALLTHROUGH;
27292741
}
27302742

‎tests/access_chains/pointer_access_chain_phy_storage_buffer.slang‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
uniform uint* data_ptr; // creates a ptr chain access
2+
3+
struct Data{
4+
int x;
5+
}
6+
uniform Data* data_struct;
7+
8+
[numthreads(1,1,1)]
9+
void computeMain()
10+
{
11+
data_ptr[0] = 1;
12+
data_struct->x = 1;
13+
}

‎tests/access_chains/pointer_access_chain_phy_storage_buffer.spv‎

952 Bytes
Binary file not shown.

‎tests/access_chains/pointer_access_chain_phy_storage_buffer.spv.yaml‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
%YAML 1.1
2+
---
3+
all_type_descriptions:
4+
- &td0
5+
id: 4
6+
op: 32
7+
type_name:
8+
struct_member_name:
9+
storage_class: 5349 # PhysicalStorageBuffer
10+
type_flags: 0x40000004 # REF INT
11+
decoration_flags: 0x00000000 # NONE
12+
traits:
13+
numeric:
14+
scalar: { width: 32, signedness: 0 }
15+
vector: { component_count: 0 }
16+
matrix: { column_count: 0, row_count: 0, stride: 0 }
17+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
18+
array: { dims_count: 0, dims: [], stride: 0 }
19+
member_count: 0
20+
members:
21+
- &td1
22+
id: 19
23+
op: 21
24+
type_name:
25+
struct_member_name:
26+
storage_class: 0 # UniformConstant
27+
type_flags: 0x00000004 # INT
28+
decoration_flags: 0x00000000 # NONE
29+
traits:
30+
numeric:
31+
scalar: { width: 32, signedness: 1 }
32+
vector: { component_count: 0 }
33+
matrix: { column_count: 0, row_count: 0, stride: 0 }
34+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
35+
array: { dims_count: 0, dims: [], stride: 0 }
36+
member_count: 0
37+
members:
38+
- &td2
39+
id: 5
40+
op: 32
41+
type_name:
42+
struct_member_name:
43+
storage_class: 5349 # PhysicalStorageBuffer
44+
type_flags: 0x50080000 # STRUCT REF EXTERNAL_BLOCK
45+
decoration_flags: 0x00000000 # NONE
46+
traits:
47+
numeric:
48+
scalar: { width: 0, signedness: 0 }
49+
vector: { component_count: 0 }
50+
matrix: { column_count: 0, row_count: 0, stride: 0 }
51+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
52+
array: { dims_count: 0, dims: [], stride: 0 }
53+
member_count: 1
54+
members:
55+
- *td1
56+
- &td3
57+
id: 6
58+
op: 30
59+
type_name:
60+
struct_member_name:
61+
storage_class: -1 # NOT APPLICABLE
62+
type_flags: 0x10080000 # STRUCT EXTERNAL_BLOCK
63+
decoration_flags: 0x00000001 # BLOCK
64+
traits:
65+
numeric:
66+
scalar: { width: 0, signedness: 0 }
67+
vector: { component_count: 0 }
68+
matrix: { column_count: 0, row_count: 0, stride: 0 }
69+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
70+
array: { dims_count: 0, dims: [], stride: 0 }
71+
member_count: 2
72+
members:
73+
- *td0
74+
- *td2
75+
- &td4
76+
id: 19
77+
op: 21
78+
type_name:
79+
struct_member_name:
80+
storage_class: 0 # UniformConstant
81+
type_flags: 0x00000004 # INT
82+
decoration_flags: 0x00000000 # NONE
83+
traits:
84+
numeric:
85+
scalar: { width: 32, signedness: 1 }
86+
vector: { component_count: 0 }
87+
matrix: { column_count: 0, row_count: 0, stride: 0 }
88+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
89+
array: { dims_count: 0, dims: [], stride: 0 }
90+
member_count: 0
91+
members:
92+
all_block_variables:
93+
- &bv0
94+
name:
95+
offset: 0
96+
absolute_offset: 0
97+
size: 0
98+
padded_size: 8
99+
decorations: 0x00000000 # NONE
100+
numeric:
101+
scalar: { width: 32, signedness: 0 }
102+
vector: { component_count: 0 }
103+
matrix: { column_count: 0, row_count: 0, stride: 0 }
104+
array: { dims_count: 0, dims: [], stride: 0 }
105+
member_count: 0
106+
members:
107+
type_description: *td0
108+
- &bv1
109+
name:
110+
offset: 0
111+
absolute_offset: 0
112+
size: 4
113+
padded_size: 16
114+
decorations: 0x00000000 # NONE
115+
numeric:
116+
scalar: { width: 32, signedness: 1 }
117+
vector: { component_count: 0 }
118+
matrix: { column_count: 0, row_count: 0, stride: 0 }
119+
array: { dims_count: 0, dims: [], stride: 0 }
120+
member_count: 0
121+
members:
122+
type_description: *td4
123+
- &bv2
124+
name:
125+
offset: 8
126+
absolute_offset: 8
127+
size: 8
128+
padded_size: 8
129+
decorations: 0x00000000 # NONE
130+
numeric:
131+
scalar: { width: 0, signedness: 0 }
132+
vector: { component_count: 0 }
133+
matrix: { column_count: 0, row_count: 0, stride: 0 }
134+
array: { dims_count: 0, dims: [], stride: 0 }
135+
member_count: 1
136+
members:
137+
- *bv1
138+
type_description: *td2
139+
- &bv3
140+
name:
141+
offset: 0
142+
absolute_offset: 0
143+
size: 16
144+
padded_size: 16
145+
decorations: 0x00000000 # NONE
146+
numeric:
147+
scalar: { width: 0, signedness: 0 }
148+
vector: { component_count: 0 }
149+
matrix: { column_count: 0, row_count: 0, stride: 0 }
150+
array: { dims_count: 0, dims: [], stride: 0 }
151+
member_count: 2
152+
members:
153+
- *bv0
154+
- *bv2
155+
type_description: *td3
156+
all_descriptor_bindings:
157+
- &db0
158+
spirv_id: 3
159+
name:
160+
binding: 0
161+
input_attachment_index: 0
162+
set: 0
163+
decoration_flags: 0x00000000 # NONE
164+
descriptor_type: 6 # VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER
165+
resource_type: 2 # CBV
166+
image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
167+
block: *bv3 #
168+
array: { dims_count: 0, dims: [] }
169+
accessed: 1
170+
uav_counter_id: 4294967295
171+
uav_counter_binding:
172+
type_description: *td3
173+
word_offset: { binding: 86, set: 90 }
174+
all_interface_variables:
175+
module:
176+
generator: 7 # Khronos SPIR-V Tools Assembler
177+
entry_point_name: "main"
178+
entry_point_id: 2
179+
source_language: 0 # Unknown
180+
source_language_version: 0
181+
spirv_execution_model: 5 # GLCompute
182+
shader_stage: 0x00000020 # CS
183+
descriptor_binding_count: 1
184+
descriptor_bindings:
185+
- *db0 #
186+
descriptor_set_count: 1
187+
descriptor_sets:
188+
- set: 0
189+
binding_count: 1
190+
bindings:
191+
- *db0 #
192+
input_variable_count: 0,
193+
input_variables:
194+
output_variable_count: 0,
195+
output_variables:
196+
push_constant_count: 0,
197+
push_constants:
198+
specialization_constant_count: 0,
199+
specialization_constants:
200+
...

0 commit comments

Comments
 (0)