Skip to content

Commit 3289b1d

Browse files
sbourassesbourasse
authored
Fix crash on unnamed function parameters under relaxed rules (KhronosGroup#3962)
* Prevent null fieldName access. * Add test for unnamed parameter fix. * Update validation_fails.txt
1 parent 7a5308f commit 3289b1d

File tree

7 files changed

+146
-9
lines changed

7 files changed

+146
-9
lines changed

‎Test/baseResults/validation_fails.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,4 @@ Test/baseResults/spv.separate.frag.out
7979
Test/baseResults/spv.sparseTextureClamp.frag.out
8080
Test/baseResults/spv.sparseTexture.frag.out
8181
Test/baseResults/vk.relaxed.errorcheck.vert.out
82+
Test/baseResults/vk.relaxed.syntaxerror.vert.out

‎Test/baseResults/vk.relaxed.syntaxerror.vert.out‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
vk.relaxed.syntaxerror.vert
2+
Shader version: 460
3+
0:? Sequence
4+
0:5 Function Definition: main( ( global void)
5+
0:5 Function Parameters:
6+
0:6 Sequence
7+
0:6 move second child to first child ( temp highp 4-component vector of float)
8+
0:6 'io' (layout( location=0) smooth out highp 4-component vector of float)
9+
0:6 Constant:
10+
0:6 0.000000
11+
0:6 0.000000
12+
0:6 0.000000
13+
0:6 0.000000
14+
0:? Linker Objects
15+
0:? 'io' (layout( location=0) smooth out highp 4-component vector of float)
16+
0:? 'gl_VertexIndex' ( in int VertexIndex)
17+
0:? 'gl_InstanceIndex' ( in int InstanceIndex)
18+
19+
vk.relaxed.syntaxerror.frag
20+
ERROR: 0:20: '' : syntax error, unexpected SAMPLER, expecting RIGHT_PAREN
21+
ERROR: 1 compilation errors. No code generated.
22+
23+
24+
Shader version: 460
25+
gl_FragCoord origin is upper left
26+
ERROR: node is still EOpNull!
27+
0:16 Function Definition: main( ( global void)
28+
0:16 Function Parameters:
29+
0:17 Sequence
30+
0:17 move second child to first child ( temp highp 4-component vector of float)
31+
0:17 'o' ( out highp 4-component vector of float)
32+
0:17 add ( temp highp 4-component vector of float)
33+
0:17 'io' (layout( location=0) smooth in highp 4-component vector of float)
34+
0:17 Function Call: foo(struct-OpaqueInStruct-s211;s21; ( global highp 4-component vector of float)
35+
0:17 uniformStruct: direct index for structure ( uniform structure{ global highp int /*smp*/})
36+
0:17 'anon@0' (layout( column_major std140) uniform block{ uniform structure{ global highp int /*smp*/} uniformStruct})
37+
0:17 Constant:
38+
0:17 0 (const uint)
39+
0:17 'uniformStruct.smp' ( uniform highp sampler2D)
40+
0:? Linker Objects
41+
0:? 'io' (layout( location=0) smooth in highp 4-component vector of float)
42+
0:? 'o' ( out highp 4-component vector of float)
43+
0:? 'uniformStruct.smp' ( uniform highp sampler2D)
44+
0:? 'anon@0' (layout( column_major std140) uniform block{ uniform structure{ global highp int /*smp*/} uniformStruct})
45+
46+
47+
Linked vertex stage:
48+
49+
50+
Linked fragment stage:
51+
52+
ERROR: Linking fragment stage: No function definition (body) found:
53+
foo(struct-OpaqueInStruct-s211;s21;
54+
55+
Shader version: 460
56+
0:? Sequence
57+
0:5 Function Definition: main( ( global void)
58+
0:5 Function Parameters:
59+
0:6 Sequence
60+
0:6 move second child to first child ( temp highp 4-component vector of float)
61+
0:6 'io' (layout( location=0) smooth out highp 4-component vector of float)
62+
0:6 Constant:
63+
0:6 0.000000
64+
0:6 0.000000
65+
0:6 0.000000
66+
0:6 0.000000
67+
0:? Linker Objects
68+
0:? 'io' (layout( location=0) smooth out highp 4-component vector of float)
69+
0:? 'gl_VertexIndex' ( in int VertexIndex)
70+
0:? 'gl_InstanceIndex' ( in int InstanceIndex)
71+
Shader version: 460
72+
gl_FragCoord origin is upper left
73+
ERROR: node is still EOpNull!
74+
0:16 Function Definition: main( ( global void)
75+
0:16 Function Parameters:
76+
0:17 Sequence
77+
0:17 move second child to first child ( temp highp 4-component vector of float)
78+
0:17 'o' ( out highp 4-component vector of float)
79+
0:17 add ( temp highp 4-component vector of float)
80+
0:17 'io' (layout( location=0) smooth in highp 4-component vector of float)
81+
0:17 Function Call: foo(struct-OpaqueInStruct-s211;s21; ( global highp 4-component vector of float)
82+
0:17 uniformStruct: direct index for structure ( uniform structure{ global highp int /*smp*/})
83+
0:17 'anon@0' (layout( column_major std140) uniform block{ uniform structure{ global highp int /*smp*/} uniformStruct})
84+
0:17 Constant:
85+
0:17 0 (const uint)
86+
0:17 'uniformStruct.smp' ( uniform highp sampler2D)
87+
0:? Linker Objects
88+
0:? 'io' (layout( location=0) smooth in highp 4-component vector of float)
89+
0:? 'o' ( out highp 4-component vector of float)
90+
0:? 'uniformStruct.smp' ( uniform highp sampler2D)
91+
0:? 'anon@0' (layout( column_major std140) uniform block{ uniform structure{ global highp int /*smp*/} uniformStruct})
92+
93+
Validation failed
94+
SPIR-V is not generated for failed compile or link

‎Test/vk.relaxed.syntaxerror.frag‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#version 460
2+
3+
layout (location = 0) in vec4 io;
4+
5+
out vec4 o;
6+
7+
struct OpaqueInStruct
8+
{
9+
sampler2D smp;
10+
};
11+
12+
uniform OpaqueInStruct uniformStruct;
13+
14+
vec4 foo(OpaqueInStruct);
15+
16+
void main() {
17+
o = io + foo(uniformStruct);
18+
}
19+
20+
vec4 foo(OpaqueInStruct sampler) { // unnamed variable and invalid SAMPLER token
21+
return a + vec4(test);
22+
}
23+

‎Test/vk.relaxed.syntaxerror.vert‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#version 460
2+
3+
layout (location = 0) out vec4 io;
4+
5+
void main() {
6+
io = vec4(0.0);
7+
}

‎glslang/Include/Types.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,7 @@ class TType {
18491849
return *typeName;
18501850
}
18511851

1852+
virtual bool hasFieldName() const { return (fieldName != nullptr); }
18521853
virtual const TString& getFieldName() const
18531854
{
18541855
assert(fieldName);

‎glslang/MachineIndependent/ParseHelper.cpp‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8626,12 +8626,15 @@ static void ForEachOpaque(const TType& type, const TString& path, Function callb
86268626
++flatIndex)
86278627
{
86288628
TString subscriptPath = path;
8629-
for (size_t dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
8629+
if (path != "")
86308630
{
8631-
int index = indices[dimIndex];
8632-
subscriptPath.append("[");
8633-
subscriptPath.append(String(index));
8634-
subscriptPath.append("]");
8631+
for (size_t dimIndex = 0; dimIndex < indices.size(); ++dimIndex)
8632+
{
8633+
int index = indices[dimIndex];
8634+
subscriptPath.append("[");
8635+
subscriptPath.append(String(index));
8636+
subscriptPath.append("]");
8637+
}
86358638
}
86368639

86378640
recursion(type, subscriptPath, true, recursion);
@@ -8653,8 +8656,11 @@ static void ForEachOpaque(const TType& type, const TString& path, Function callb
86538656
for (const TTypeLoc& typeLoc : types)
86548657
{
86558658
TString nextPath = path;
8656-
nextPath.append(".");
8657-
nextPath.append(typeLoc.type->getFieldName());
8659+
if (path != "")
8660+
{
8661+
nextPath.append(".");
8662+
nextPath.append(typeLoc.type->getFieldName());
8663+
}
86588664

86598665
recursion(*(typeLoc.type), nextPath, false, recursion);
86608666
}
@@ -8712,9 +8718,13 @@ void TParseContext::vkRelaxedRemapFunctionParameter(TFunction* function, TParame
87128718
if (!param.type->isStruct() || !param.type->containsOpaque())
87138719
return;
87148720

8715-
ForEachOpaque(*param.type, (param.name ? *param.name : param.type->getFieldName()),
8721+
TString fieldName = param.name
8722+
? *param.name
8723+
: param.type->hasFieldName() ? param.type->getFieldName() : "";
8724+
8725+
ForEachOpaque(*param.type, fieldName,
87168726
[function, param, newParams](const TType& type, const TString& path) {
8717-
TString* memberName = NewPoolTString(path.c_str());
8727+
TString* memberName = path != "" ? NewPoolTString(path.c_str()) : nullptr;
87188728

87198729
TType* memberType = new TType();
87208730
memberType->shallowCopy(type);

‎gtests/VkRelaxed.FromFile.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ INSTANTIATE_TEST_SUITE_P(
297297
{{"vk.relaxed.stagelink.vert", "vk.relaxed.stagelink.frag"}, {}},
298298
{{"vk.relaxed.errorcheck.vert", "vk.relaxed.errorcheck.frag"}, {}},
299299
{{"vk.relaxed.changeSet.vert", "vk.relaxed.changeSet.frag" }, { {"0"}, {"1"} } },
300+
{{"vk.relaxed.syntaxerror.vert", "vk.relaxed.syntaxerror.frag"}, {}},
300301
}))
301302
);
302303
// clang-format on

0 commit comments

Comments
 (0)