Browse Source

Error for in/out structs containing booleans (#3973)

The GLSL spec says:
It is a compile-time error to declare a fragment shader input with, or
that contains, any of the following types:
 -a boolean type
 -an opaque type
and similar rules for other shader stages and 'out' variables.

Previously glslang would error for simple 'bool' IO variables, but not
structs/blocks containing them.

This could lead to generating invalid spir-v, violating this validator
rule:

"If OpTypeBool is stored in conjunction with OpVariable using Input or
Output Storage Classes it requires a BuiltIn decoration"
- VUID 7290
rj123-nv 2 weeks ago
parent
commit
2e24222cd9

+ 1 - 1
Test/330.frag

@@ -77,7 +77,7 @@ struct S2 {
 };
 
 layout(location = 28) in inblock2 {
-    bool b1;
+    bool b1;                              // ERROR
     float f1;
     layout(location = 25) float f2;
     vec4 f3;

+ 1 - 1
Test/430.vert

@@ -115,7 +115,7 @@ const int start2 = 5;
 layout(location = start2 * start2 - 2 - 4) in vec4 v6;
 
 layout(location = 28) in inblock2 {  // ERROR, input block in vertex shader, other errors are valid checks still...
-    bool b1;
+    bool b1;                         // ERROR
     float f1;
     layout(location = 25) float f2;
 } ininst2;

+ 1 - 1
Test/440.vert

@@ -146,7 +146,7 @@ struct T {
 };  // total size = 36
 
 out layout(xfb_buffer=0, xfb_offset=0, xfb_stride=92) bblck8 {  // ERROR, stride not multiple of 8
-    bool b;    // offset 0
+    bool b;    // offset 0                    // ERROR, out block containing bool
     T t;       // offset 8, size 40
     int i;     // offset 40 + 4 = 48
     mat3x3 m3; // offset 52

+ 2 - 1
Test/baseResults/330.frag.out

@@ -20,6 +20,7 @@ ERROR: 0:60: 'location' : cannot apply to uniform or buffer block
 ERROR: 0:68: 'layout-id value' : cannot be negative 
 ERROR: 0:69: 'layout-id value' : cannot be negative 
 ERROR: 0:76: 'f2' : cannot use layout qualifiers on structure members 
+ERROR: 0:79: 'in' : cannot contain bool 
 ERROR: 0:91: 'location on block member' : can only use in an in/out block 
 ERROR: 0:91: 'location qualifier on uniform or buffer' : not supported for this version or the enabled extensions 
 ERROR: 0:91: 'location' : overlapping use of location 3
@@ -38,7 +39,7 @@ ERROR: 0:128: 'output block' : not supported in this stage: fragment
 ERROR: 0:138: 'index' : value must be 0 or 1 
 ERROR: 0:140: 'location qualifier on uniform or buffer' : not supported for this version or the enabled extensions 
 ERROR: 0:146: 'location' : cannot apply to uniform or buffer block 
-ERROR: 39 compilation errors.  No code generated.
+ERROR: 40 compilation errors.  No code generated.
 
 
 Shader version: 330

+ 2 - 1
Test/baseResults/430.vert.out

@@ -50,6 +50,7 @@ ERROR: 0:93: 'transform feedback qualifier' : not supported for this version or
 ERROR: 0:93: 'transform feedback qualifier' : not supported for this version or the enabled extensions 
 ERROR: 0:93: 'transform feedback qualifier' : not supported for this version or the enabled extensions 
 ERROR: 0:117: 'input block' : not supported in this stage: vertex
+ERROR: 0:117: 'in' : cannot contain bool 
 ERROR: 0:123: 'input block' : not supported in this stage: vertex
 ERROR: 0:146: 'shared' : not supported in this stage: vertex
 ERROR: 0:150: 'barrier' : no matching overloaded function found 
@@ -64,7 +65,7 @@ ERROR: 0:221: 'textureQueryLevels' : no matching overloaded function found
 ERROR: 0:221: 'assign' :  cannot convert from ' const float' to ' temp int'
 ERROR: 0:222: 'textureQueryLevels' : no matching overloaded function found 
 ERROR: 0:222: 'assign' :  cannot convert from ' const float' to ' temp int'
-ERROR: 65 compilation errors.  No code generated.
+ERROR: 66 compilation errors.  No code generated.
 
 
 Shader version: 430

+ 3 - 1
Test/baseResults/440.vert.out

@@ -42,11 +42,13 @@ ERROR: 0:129: 'xfb_stride' : all stride settings must match for xfb buffer 3
 ERROR: 0:133: 'xfb_stride' : all stride settings must match for xfb buffer 3
 ERROR: 0:131: 'xfb_stride' : all stride settings must match for xfb buffer 3
 ERROR: 0:152: 'xfb_offset' : overlapping offsets at offset 64 in buffer 0
+ERROR: 0:148: 'out' : cannot contain bool 
 ERROR: 0:157: 'xfb_buffer' : buffer is too large: gl_MaxTransformFeedbackBuffers is 4
 ERROR: 0:158: 'xfb_offset' : must be a multiple of size of first component 
 ERROR: 0:159: 'xfb_offset' : type contains double or 64-bit integer; xfb_offset must be a multiple of 8 
 ERROR: 0:161: 'xfb_offset' : must be a multiple of size of first component 
 ERROR: 0:162: 'xfb_offset' : type contains double or 64-bit integer; xfb_offset must be a multiple of 8 
+ERROR: 0:157: 'out' : cannot contain bool 
 ERROR: 0:166: 'xfb_buffer' : buffer is too large: gl_MaxTransformFeedbackBuffers is 4
 ERROR: 0:169: 'xfb_buffer' : buffer is too large: gl_MaxTransformFeedbackBuffers is 4
 ERROR: 0:169: 'xfb_stride' : 1/4 stride is too large: gl_MaxTransformFeedbackInterleavedComponents is 64
@@ -61,7 +63,7 @@ ERROR: 0:193: 'assign' :  l-value required "gl_BaseVertexARB" (can't modify shad
 ERROR: 0:194: 'assign' :  l-value required "gl_BaseInstanceARB" (can't modify shader input)
 ERROR: 0:195: 'assign' :  l-value required "gl_DrawIDARB" (can't modify shader input)
 ERROR: 0:196: 'glBaseInstanceARB' : undeclared identifier 
-ERROR: 62 compilation errors.  No code generated.
+ERROR: 64 compilation errors.  No code generated.
 
 
 Shader version: 440

+ 29 - 0
Test/baseResults/boolinput.error.frag.out

@@ -0,0 +1,29 @@
+boolinput.error.frag
+ERROR: 0:3: 'in' : cannot be bool 
+ERROR: 0:9: 'in' : cannot contain bool 
+ERROR: 0:11: 'in' : cannot contain bool 
+ERROR: 3 compilation errors.  No code generated.
+
+
+Shader version: 460
+ERROR: node is still EOpNull!
+0:15  Function Definition: main( ( global void)
+0:15    Function Parameters: 
+0:?   Linker Objects
+0:?     'a' ( smooth in bool)
+0:?     's' ( smooth in structure{ global bool b})
+0:?     'anon@0' ( in block{ in bool c})
+
+
+Linked fragment stage:
+
+
+Shader version: 460
+ERROR: node is still EOpNull!
+0:15  Function Definition: main( ( global void)
+0:15    Function Parameters: 
+0:?   Linker Objects
+0:?     'a' ( smooth in bool)
+0:?     's' ( smooth in structure{ global bool b})
+0:?     'anon@0' ( in block{ in bool c})
+

+ 33 - 0
Test/baseResults/booloutput.error.vert.out

@@ -0,0 +1,33 @@
+booloutput.error.vert
+ERROR: 0:3: 'out' : cannot be bool 
+ERROR: 0:9: 'out' : cannot contain bool 
+ERROR: 0:11: 'out' : cannot contain bool 
+ERROR: 3 compilation errors.  No code generated.
+
+
+Shader version: 460
+ERROR: node is still EOpNull!
+0:15  Function Definition: main( ( global void)
+0:15    Function Parameters: 
+0:?   Linker Objects
+0:?     'a' ( smooth out bool)
+0:?     's' ( smooth out structure{ global bool b})
+0:?     'anon@0' ( out block{ out bool c})
+0:?     'gl_VertexID' ( gl_VertexId int VertexId)
+0:?     'gl_InstanceID' ( gl_InstanceId int InstanceId)
+
+
+Linked vertex stage:
+
+
+Shader version: 460
+ERROR: node is still EOpNull!
+0:15  Function Definition: main( ( global void)
+0:15    Function Parameters: 
+0:?   Linker Objects
+0:?     'a' ( smooth out bool)
+0:?     's' ( smooth out structure{ global bool b})
+0:?     'anon@0' ( out block{ out bool c})
+0:?     'gl_VertexID' ( gl_VertexId int VertexId)
+0:?     'gl_InstanceID' ( gl_InstanceId int InstanceId)
+

+ 16 - 0
Test/boolinput.error.frag

@@ -0,0 +1,16 @@
+#version 460
+
+in bool a;
+
+struct S {
+    bool b;
+};
+
+in S s;
+
+in Block {
+    bool c;
+};
+
+void main() {
+}

+ 16 - 0
Test/booloutput.error.vert

@@ -0,0 +1,16 @@
+#version 460
+
+out bool a;
+
+struct S {
+    bool b;
+};
+
+out S s;
+
+out Block {
+    bool c;
+};
+
+void main() {
+}

+ 16 - 5
glslang/MachineIndependent/ParseHelper.cpp

@@ -4575,11 +4575,6 @@ void TParseContext::globalQualifierTypeCheck(const TSourceLoc& loc, const TQuali
 
     // now, knowing it is a shader in/out, do all the in/out semantic checks
 
-    if (publicType.basicType == EbtBool && !parsingBuiltins) {
-        error(loc, "cannot be bool", GetStorageQualifierString(qualifier.storage), "");
-        return;
-    }
-
     if (isTypeInt(publicType.basicType) || publicType.basicType == EbtDouble) {
         profileRequires(loc, EEsProfile, 300, nullptr, "non-float shader input/output");
         profileRequires(loc, ~EEsProfile, 130, nullptr, "non-float shader input/output");
@@ -7068,6 +7063,22 @@ void TParseContext::layoutObjectCheck(const TSourceLoc& loc, const TSymbol& symb
             break;
         }
     }
+
+    // Check that an in/out variable or block doesn't contain a boolean member
+    // Don't enforce if redeclaring a builtin, which are allowed to contain bool
+    if (!parsingBuiltins && type.containsBasicType(EbtBool) && !builtInName(symbol.getName())) {
+        switch(qualifier.storage) {
+        case EvqVaryingIn:
+        case EvqVaryingOut:
+        {
+            const char *reason = type.getBasicType() == EbtBool ? "cannot be bool" : "cannot contain bool";
+            error(loc, reason, GetStorageQualifierString(qualifier.storage), "");
+            break;
+        }
+        default:
+            break;
+        }
+    }
 }
 
 // "For some blocks declared as arrays, the location can only be applied at the block level:

+ 2 - 0
gtests/AST.FromFile.cpp

@@ -184,6 +184,8 @@ INSTANTIATE_TEST_SUITE_P(
         "aggOps.frag",
         "always-discard.frag",
         "always-discard2.frag",
+        "boolinput.error.frag",
+        "booloutput.error.vert",
         "conditionalDiscard.frag",
         "conversion.frag",
         "dataOut.frag",