Fixing bug in OpAccessChain validation code.
authorEhsan Nasiri <ehsann@google.com>
Mon, 5 Dec 2016 20:11:51 +0000 (15:11 -0500)
committerDavid Neto <dneto@google.com>
Tue, 6 Dec 2016 14:55:39 +0000 (09:55 -0500)
The validation code for OpAccessChain was missing OpTypeRuntimeArray as
a possible type that can be indexed into.

This was caught by running the validator on VKCTS.

Also adding unit tests for it.

source/validate_id.cpp
test/val/val_id_test.cpp

index 94b0753..292c8e3 100644 (file)
@@ -1264,9 +1264,10 @@ bool idUsage::isValid<SpvOpAccessChain>(const spv_instruction_t* inst,
     switch (typePointedTo->opcode()) {
       case SpvOpTypeMatrix:
       case SpvOpTypeVector:
-      case SpvOpTypeArray: {
-        // In OpTypeMatrix, OpTypeArray, and OpTypeVector, word 2 is the
-        // Element Type.
+      case SpvOpTypeArray:
+      case SpvOpTypeRuntimeArray: {
+        // In OpTypeMatrix, OpTypeVector, OpTypeArray, and OpTypeRuntimeArray,
+        // word 2 is the Element Type.
         typePointedTo = module_.FindDef(typePointedTo->word(2));
         break;
       }
index 11191b6..dd7335a 100644 (file)
@@ -1814,8 +1814,10 @@ string opAccessChainSpirvSetup = R"(
 ; uniform blockName {
 ;   S s;
 ;   bool cond;
+;   RunTimeArray arr;
 ; }
 
+%f32arr = OpTypeRuntimeArray %float
 %bool = OpTypeBool
 %v4float = OpTypeVector %float 4
 %array5_mat4x3 = OpTypeArray %mat4x3 %int_5
@@ -1824,7 +1826,7 @@ string opAccessChainSpirvSetup = R"(
 %_ptr_Function_vec4 = OpTypePointer Function %v4float
 %_ptr_Uniform_vec4 = OpTypePointer Uniform %v4float
 %struct_s = OpTypeStruct %bool %array5_vec4 %int %array5_mat4x3
-%struct_blockName = OpTypeStruct %struct_s %bool
+%struct_blockName = OpTypeStruct %struct_s %bool %f32arr
 %_ptr_Uniform_blockName = OpTypePointer Uniform %struct_blockName
 %_ptr_Uniform_struct_s = OpTypePointer Uniform %struct_s
 %_ptr_Uniform_array5_mat4x3 = OpTypePointer Uniform %array5_mat4x3
@@ -1857,7 +1859,7 @@ OpFunctionEnd
   CompileSuccessfully(spirv);
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("The Result Type of OpAccessChain <id> '35' must be "
+              HasSubstr("The Result Type of OpAccessChain <id> '36' must be "
                         "OpTypePointer. Found OpTypeFloat."));
 }
 
@@ -2010,10 +2012,10 @@ OpFunctionEnd
                         "indexes still remain to be traversed."));
 }
 
-// Invalid: Trying to find index 2 of the struct that has only 2 members.
+// Invalid: Trying to find index 3 of the struct that has only 3 members.
 TEST_F(ValidateIdWithMessage, OpAccessChainStructIndexOutOfBoundBad) {
   string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"(
-%entry = OpAccessChain %_ptr_Uniform_float %blockName_var %int_2 %int_2 %int_2
+%entry = OpAccessChain %_ptr_Uniform_float %blockName_var %int_3 %int_2 %int_2
 OpReturn
 OpFunctionEnd
   )";
@@ -2021,8 +2023,8 @@ OpFunctionEnd
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
               HasSubstr("Index is out of bound: OpAccessChain can not find "
-                        "index 2 into the structure <id> '25'. This structure "
-                        "has 2 members. Largest valid index is 1."));
+                        "index 3 into the structure <id> '26'. This structure "
+                        "has 3 members. Largest valid index is 2."));
 }
 
 // Valid: Tests that we can index into Struct, Array, Matrix, and Vector!
@@ -2046,6 +2048,31 @@ OpFunctionEnd
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
+// Valid: Access an element of OpTypeRuntimeArray.
+TEST_F(ValidateIdWithMessage, OpAccessChainIndexIntoRuntimeArrayGood) {
+  string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"(
+%runtime_arr_entry = OpAccessChain %_ptr_Uniform_float %blockName_var %int_2 %int_0
+OpReturn
+OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+// Invalid: Unused index when accessing OpTypeRuntimeArray.
+TEST_F(ValidateIdWithMessage, OpAccessChainIndexIntoRuntimeArrayBad) {
+  string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"(
+%runtime_arr_entry = OpAccessChain %_ptr_Uniform_float %blockName_var %int_2 %int_0 %int_1
+OpReturn
+OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpAccessChain reached non-composite type while "
+                        "indexes still remain to be traversed."));
+}
+
 // Invalid: Reached scalar type before arguments to OpAccessChain finished.
 TEST_F(ValidateIdWithMessage, OpAccessChainMatrixMoreArgsThanNeededBad) {
   string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"(