Issue 32: Guard against constructors consuming unsized arrays.

KhronosGroup · Jul 22, 2015 · 12a3833 · 12a3833
1 parent f498400
commit 12a3833
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 13 deletions.
diff --git a/Test/array.frag b/Test/array.frag
@@ -101,4 +101,6 @@ void foo3()
     resize2[5] = 4.0;  // ERROR
 }
 
-int[] i = int[](); // ERROR, need constructor arguments
+int[] i = int[]();    // ERROR, need constructor arguments
+float emptyA[];
+float b = vec4(emptyA);    // ERROR, array can't be a constructor argument
diff --git a/Test/baseResults/array.frag.out b/Test/baseResults/array.frag.out
@@ -24,7 +24,8 @@ ERROR: 0:93: 'length' :  array must be declared with a size before using this me
 ERROR: 0:101: '[' :  array index out of range '5'
 ERROR: 0:104: 'constructor' : array constructor must have at least one argument 
 ERROR: 0:104: '=' :  cannot convert from 'const float' to 'global implicitly-sized array of int'
-ERROR: 25 compilation errors.  No code generated.
+ERROR: 0:106: 'constructor' : array argument must be sized 
+ERROR: 26 compilation errors.  No code generated.
 
 
 Shader version: 130
@@ -258,6 +259,11 @@ ERROR: node is still EOpNull!
 0:101            5 (const int)
 0:101        Constant:
 0:101          4.000000
+0:106  Sequence
+0:106    move second child to first child (temp float)
+0:106      'b' (global float)
+0:106      Constant:
+0:106        0.000000
 0:?   Linker Objects
 0:?     'gu' (global implicitly-sized array of float)
 0:?     'g4' (global 4-element array of float)
@@ -267,6 +273,8 @@ ERROR: node is still EOpNull!
 0:?     'f' (global float)
 0:?     'gUnusedUnsized' (global implicitly-sized array of float)
 0:?     'i' (global implicitly-sized array of int)
+0:?     'emptyA' (global implicitly-sized array of float)
+0:?     'b' (global float)
 
 
 Linked fragment stage:
@@ -503,6 +511,11 @@ ERROR: node is still EOpNull!
 0:101            5 (const int)
 0:101        Constant:
 0:101          4.000000
+0:106  Sequence
+0:106    move second child to first child (temp float)
+0:106      'b' (global float)
+0:106      Constant:
+0:106        0.000000
 0:?   Linker Objects
 0:?     'gu' (global 4-element array of float)
 0:?     'g4' (global 4-element array of float)
@@ -512,4 +525,6 @@ ERROR: node is still EOpNull!
 0:?     'f' (global float)
 0:?     'gUnusedUnsized' (global 1-element array of float)
 0:?     'i' (global 1-element array of int)
+0:?     'emptyA' (global 1-element array of float)
+0:?     'b' (global float)
 
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
@@ -1947,9 +1947,7 @@ bool TParseContext::constructorError(TSourceLoc loc, TIntermNode* node, TFunctio
     }
 
     //
-    // Note: It's okay to have too many components available, but not okay to have unused
-    // arguments.  'full' will go to true when enough args have been seen.  If we loop
-    // again, there is an extra argument, so 'overfull' will become true.
+    // Walk the arguments for first-pass checks and collection of information.
     //
 
     int size = 0;
@@ -1958,19 +1956,32 @@ bool TParseContext::constructorError(TSourceLoc loc, TIntermNode* node, TFunctio
     bool overFull = false;
     bool matrixInMatrix = false;
     bool arrayArg = false;
-    for (int i = 0; i < function.getParamCount(); ++i) {
-        size += function[i].type->computeNumComponents();
-
-        if (constructingMatrix && function[i].type->isMatrix())
+    for (int arg = 0; arg < function.getParamCount(); ++arg) {
+        if (function[arg].type->isArray()) {
+            if (! function[arg].type->isExplicitlySizedArray()) {
+                // Can't construct from an unsized array.
+                error(loc, "array argument must be sized", "constructor", "");
+                return true;
+            }
+            arrayArg = true;
+        }
+        if (constructingMatrix && function[arg].type->isMatrix())
             matrixInMatrix = true;
-        if (full)
+
+        // 'full' will go to true when enough args have been seen.  If we loop
+        // again, there is an extra argument.
+        if (full) {
+            // For vectors and matrices, it's okay to have too many components
+            // available, but not okay to have unused arguments.
             overFull = true;
+        }
+
+        size += function[arg].type->computeNumComponents();
         if (op != EOpConstructStruct && ! type.isArray() && size >= type.computeNumComponents())
             full = true;
-        if (function[i].type->getQualifier().storage != EvqConst)
+
+        if (function[arg].type->getQualifier().storage != EvqConst)
             constType = false;
-        if (function[i].type->isArray())
-            arrayArg = true;
     }
 
     if (constType)