diff --git a/source/validate_id.cpp b/source/validate_id.cpp index 1ebfa826..c1523e03 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -1193,11 +1193,14 @@ bool idUsage::isValid(const spv_instruction_t* inst, template <> bool idUsage::isValid(const spv_instruction_t* inst, const spv_opcode_desc) { + std::string instr_name = + "Op" + std::string(spvOpcodeString(static_cast(inst->opcode))); + // The result type must be OpTypePointer. Result Type is at word 1. auto resultTypeIndex = 1; auto resultTypeInstr = module_.FindDef(inst->words[resultTypeIndex]); if (SpvOpTypePointer != resultTypeInstr->opcode()) { - DIAG(resultTypeIndex) << "The Result Type of OpAccessChain '" + DIAG(resultTypeIndex) << "The Result Type of " << instr_name << " '" << inst->words[2] << "' must be OpTypePointer. Found Op" << spvOpcodeString( @@ -1217,7 +1220,8 @@ bool idUsage::isValid(const spv_instruction_t* inst, auto baseTypeInstr = module_.FindDef(baseInstr->type_id()); if (!baseTypeInstr || SpvOpTypePointer != baseTypeInstr->opcode()) { DIAG(baseIdIndex) << "The Base '" << inst->words[baseIdIndex] - << "' in OpAccessChain instruction must be a pointer."; + << "' in " << instr_name + << " instruction must be a pointer."; return false; } @@ -1227,8 +1231,8 @@ bool idUsage::isValid(const spv_instruction_t* inst, auto baseTypeStorageClass = baseTypeInstr->word(2); if (resultTypeStorageClass != baseTypeStorageClass) { DIAG(resultTypeIndex) << "The result pointer storage class and base " - "pointer storage class in OpAccessChain do not " - "match."; + "pointer storage class in " + << instr_name << " do not match."; return false; } @@ -1241,13 +1245,13 @@ bool idUsage::isValid(const spv_instruction_t* inst, const size_t num_indexes = inst->words.size() - 4; const size_t num_indexes_limit = 255; if (num_indexes > num_indexes_limit) { - DIAG(resultTypeIndex) - << "The number of indexes in OpAccessChain may not exceed " - << num_indexes_limit << ". Found " << num_indexes << " indexes."; + DIAG(resultTypeIndex) << "The number of indexes in " << instr_name + << " may not exceed " << num_indexes_limit + << ". Found " << num_indexes << " indexes."; return false; } if (num_indexes <= 0) { - DIAG(resultTypeIndex) << "No Indexes were passes to OpAccessChain."; + DIAG(resultTypeIndex) << "No Indexes were passes to " << instr_name << "."; return false; } // Indexes walk the type hierarchy to the desired depth, potentially down to @@ -1264,7 +1268,8 @@ bool idUsage::isValid(const spv_instruction_t* inst, // The index must be a scalar integer type (See OpAccessChain in the Spec.) auto indexTypeInstr = module_.FindDef(cur_word_instr->type_id()); if (!indexTypeInstr || SpvOpTypeInt != indexTypeInstr->opcode()) { - DIAG(i) << "Indexes passed to OpAccessChain must be of type integer."; + DIAG(i) << "Indexes passed to " << instr_name + << " must be of type integer."; return false; } switch (typePointedTo->opcode()) { @@ -1281,7 +1286,8 @@ bool idUsage::isValid(const spv_instruction_t* inst, // In case of structures, there is an additional constraint on the // index: the index must be an OpConstant. if (SpvOpConstant != cur_word_instr->opcode()) { - DIAG(i) << "The passed to OpAccessChain to index into a " + DIAG(i) << "The passed to " << instr_name + << " to index into a " "structure must be an OpConstant."; return false; } @@ -1296,10 +1302,11 @@ bool idUsage::isValid(const spv_instruction_t* inst, const uint32_t num_struct_members = static_cast(typePointedTo->words().size() - 2); if (cur_index >= num_struct_members) { - DIAG(i) << "Index is out of bound: OpAccessChain can not find index " - << cur_index << " into the structure '" - << typePointedTo->id() << "'. This structure has " - << num_struct_members << " members. Largest valid index is " + DIAG(i) << "Index is out of bound: " << instr_name + << " can not find index " << cur_index + << " into the structure '" << typePointedTo->id() + << "'. This structure has " << num_struct_members + << " members. Largest valid index is " << num_struct_members - 1 << "."; return false; } @@ -1310,8 +1317,8 @@ bool idUsage::isValid(const spv_instruction_t* inst, } default: { // Give an error. reached non-composite type while indexes still remain. - DIAG(i) << "OpAccessChain reached non-composite type while indexes " - "still remain to be traversed."; + DIAG(i) << instr_name << " reached non-composite type while indexes " + "still remain to be traversed."; return false; } } @@ -1320,7 +1327,7 @@ bool idUsage::isValid(const spv_instruction_t* inst, // The type being pointed to should be the same as the result type. if (typePointedTo->id() != resultTypePointedTo->id()) { DIAG(resultTypeIndex) - << "OpAccessChain result type (Op" + << instr_name << " result type (Op" << spvOpcodeString(static_cast(resultTypePointedTo->opcode())) << ") does not match the type that results from indexing into the base " " (Op" @@ -1331,11 +1338,11 @@ bool idUsage::isValid(const spv_instruction_t* inst, return true; } -#if 0 template <> bool idUsage::isValid( - const spv_instruction_t *inst, const spv_opcode_desc opcodeEntry) {} -#endif + const spv_instruction_t* inst, const spv_opcode_desc opcodeEntry) { + return isValid(inst, opcodeEntry); +} #if 0 template <> @@ -2564,7 +2571,7 @@ bool idUsage::isValid(const spv_instruction_t* inst) { CASE(OpCopyMemory) CASE(OpCopyMemorySized) CASE(OpAccessChain) - TODO(OpInBoundsAccessChain) + CASE(OpInBoundsAccessChain) TODO(OpArrayLength) TODO(OpGenericPtrMemSemantics) CASE(OpFunction) diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index 0e4964e3..21ba26b8 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -1985,8 +1985,48 @@ OpFunctionEnd HasSubstr("No Indexes were passes to OpAccessChain.")); } +// Valid: 255 indexes passed to OpAccessChain. Limit is 255. +TEST_F(ValidateIdWithMessage, OpAccessChainTooManyIndexesGood) { + int depth = 255; + std::string header = kGLSL450MemoryModel + opAccessChainSpirvSetup; + header.erase(header.find("%func")); + std::ostringstream spirv; + spirv << header << "\n"; + + // Build nested structures. Struct 'i' contains struct 'i-1' + spirv << "%s_depth_1 = OpTypeStruct %float\n"; + for (int i = 2; i <= depth; ++i) { + spirv << "%s_depth_" << i << " = OpTypeStruct %s_depth_" << i - 1 << "\n"; + } + + // Define Pointer and Variable to use for OpAccessChain. + spirv << "%_ptr_Uniform_deep_struct = OpTypePointer Uniform %s_depth_" + << depth << "\n"; + spirv << "%deep_var = OpVariable %_ptr_Uniform_deep_struct Uniform\n"; + + // Function Start + spirv << R"( + %func = OpFunction %void None %void_f + %my_label = OpLabel + )"; + + // OpAccessChain with 'n' indexes (n = depth) + spirv << "%entry = OpAccessChain %_ptr_Uniform_float %deep_var"; + for (int i = 0; i < depth; ++i) { + spirv << " %int_0"; + } + + // Function end + spirv << R"( + OpReturn + OpFunctionEnd + )"; + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + // Invalid: 256 indexes passed to OpAccessChain. Limit is 255. -TEST_F(ValidateIdWithMessage, OpAccessChainTooManyIndecesBad) { +TEST_F(ValidateIdWithMessage, OpAccessChainTooManyIndexesBad) { std::ostringstream spirv; spirv << kGLSL450MemoryModel << opAccessChainSpirvSetup; spirv << "%entry = OpAccessChain %_ptr_Private_float %my_matrix"; @@ -2155,7 +2195,327 @@ OpFunctionEnd "base (OpTypeFloat).")); } -// TODO: OpInBoundsAccessChain +// Valid: Access a float in a matrix using OpInBoundsAccessChain +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainGood) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%float_entry = OpInBoundsAccessChain %_ptr_Private_float %my_matrix %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Invalid. The result type of OpInBoundsAccessChain must be a pointer. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainResultTypeBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%float_entry = OpInBoundsAccessChain %float %my_matrix %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The Result Type of OpInBoundsAccessChain '36' must be " + "OpTypePointer. Found OpTypeFloat.")); +} + +// Invalid. The base type of OpInBoundsAccessChain must be a pointer. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainBaseTypeVoidBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%float_entry = OpInBoundsAccessChain %_ptr_Private_float %void %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The Base '1' in OpInBoundsAccessChain instruction must " + "be a pointer.")); +} + +// Invalid. The base type of OpInBoundsAccessChain must be a pointer. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainBaseTypeNonPtrVariableBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_float %_ptr_Private_float %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The Base '8' in OpInBoundsAccessChain instruction must " + "be a pointer.")); +} + +// Invalid: The storage class of Base and Result do not match. +TEST_F(ValidateIdWithMessage, + OpInBoundsAccessChainResultAndBaseStorageClassDoesntMatchBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Function_float %my_matrix %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The result pointer storage class and base pointer " + "storage class in OpInBoundsAccessChain do not match.")); +} + +// Invalid. The base type of OpInBoundsAccessChain must point to a composite +// object. +TEST_F(ValidateIdWithMessage, + OpInBoundsAccessChainBasePtrNotPointingToCompositeBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_float %my_float_var %int_0 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpInBoundsAccessChain reached non-composite type while " + "indexes still remain to be traversed.")); +} + +// Invalid. No Indexes passed to OpInBoundsAccessChain +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainMissingIndexesBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_float %my_float_var +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("No Indexes were passes to OpInBoundsAccessChain.")); +} + +// Valid: 255 indexes passed to OpInBoundsAccessChain. Limit is 255. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainTooManyIndexesGood) { + int depth = 255; + std::string header = kGLSL450MemoryModel + opAccessChainSpirvSetup; + header.erase(header.find("%func")); + std::ostringstream spirv; + spirv << header << "\n"; + + // Build nested structures. Struct 'i' contains struct 'i-1' + spirv << "%s_depth_1 = OpTypeStruct %float\n"; + for (int i = 2; i <= depth; ++i) { + spirv << "%s_depth_" << i << " = OpTypeStruct %s_depth_" << i - 1 << "\n"; + } + + // Define Pointer and Variable to use for OpInBoundsAccessChain. + spirv << "%_ptr_Uniform_deep_struct = OpTypePointer Uniform %s_depth_" + << depth << "\n"; + spirv << "%deep_var = OpVariable %_ptr_Uniform_deep_struct Uniform\n"; + + // Function Start + spirv << R"( + %func = OpFunction %void None %void_f + %my_label = OpLabel + )"; + + // OpAccessChain with 'n' indexes (n = depth) + spirv << "%entry = OpInBoundsAccessChain %_ptr_Uniform_float %deep_var"; + for (int i = 0; i < depth; ++i) { + spirv << " %int_0"; + } + + // Function end + spirv << R"( + OpReturn + OpFunctionEnd + )"; + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Invalid: 256 indexes passed to OpInBoundsAccessChain. Limit is 255. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainTooManyIndexesBad) { + std::ostringstream spirv; + spirv << kGLSL450MemoryModel << opAccessChainSpirvSetup; + spirv << "%entry = OpInBoundsAccessChain %_ptr_Private_float %my_matrix"; + for (int i = 0; i < 256; ++i) { + spirv << " %int_0"; + } + spirv << R"( + OpReturn + OpFunctionEnd + )"; + CompileSuccessfully(spirv.str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The number of indexes in OpInBoundsAccessChain may not exceed " + "255. Found 256 indexes.")); +} + +// Invalid: Index passed to OpInBoundsAccessChain is float (must be integer). +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainUndefinedIndexBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_float %my_matrix %float %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "Indexes passed to OpInBoundsAccessChain must be of type integer.")); +} + +// Invalid: The OpInBoundsAccessChain index argument that indexes into a struct +// must be of type OpConstant. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainStructIndexNotConstantBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%f = OpInBoundsAccessChain %_ptr_Uniform_float %blockName_var %int_0 %spec_int %int_2 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("The passed to OpInBoundsAccessChain to index into a " + "structure must be an OpConstant.")); +} + +// Invalid: Indexing up to a vec4 granularity, but result type expected float. +TEST_F(ValidateIdWithMessage, + OpInBoundsAccessChainStructResultTypeDoesntMatchIndexedTypeBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Uniform_float %blockName_var %int_0 %int_1 %int_2 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("OpInBoundsAccessChain result type (OpTypeFloat) does " + "not match the type that results from indexing into " + "the base (OpTypeVector).")); +} + +// Invalid: Reach non-composite type (bool) when unused indexes remain. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainStructTooManyIndexesBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Uniform_float %blockName_var %int_0 %int_2 %int_2 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpInBoundsAccessChain reached non-composite type while " + "indexes still remain to be traversed.")); +} + +// Invalid: Trying to find index 3 of the struct that has only 3 members. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainStructIndexOutOfBoundBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Uniform_float %blockName_var %int_3 %int_2 %int_2 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("Index is out of bound: OpInBoundsAccessChain can not find " + "index 3 into the structure '26'. This structure " + "has 3 members. Largest valid index is 2.")); +} + +// Valid: Tests that we can index into Struct, Array, Matrix, and Vector! +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainIndexIntoAllTypesGood) { + // indexes that we are passing are: 0, 3, 1, 2, 0 + // 0 will select the struct_s within the base struct (blockName) + // 3 will select the Array that contains 5 matrices + // 1 will select the Matrix that is at index 1 of the array + // 2 will select the column (which is a vector) within the matrix at index 2 + // 0 will select the element at the index 0 of the vector. (which is a float). + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%ss = OpInBoundsAccessChain %_ptr_Uniform_struct_s %blockName_var %int_0 +%sa = OpInBoundsAccessChain %_ptr_Uniform_array5_mat4x3 %blockName_var %int_0 %int_3 +%sm = OpInBoundsAccessChain %_ptr_Uniform_mat4x3 %blockName_var %int_0 %int_3 %int_1 +%sc = OpInBoundsAccessChain %_ptr_Uniform_v3float %blockName_var %int_0 %int_3 %int_1 %int_2 +%entry = OpInBoundsAccessChain %_ptr_Uniform_float %blockName_var %int_0 %int_3 %int_1 %int_2 %int_0 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +// Valid: Access an element of OpTypeRuntimeArray. +TEST_F(ValidateIdWithMessage, OpInBoundsAccessChainIndexIntoRuntimeArrayGood) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%runtime_arr_entry = OpInBoundsAccessChain %_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, OpInBoundsAccessChainIndexIntoRuntimeArrayBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%runtime_arr_entry = OpInBoundsAccessChain %_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("OpInBoundsAccessChain reached non-composite type while " + "indexes still remain to be traversed.")); +} + +// Invalid: Reached scalar type before arguments to OpInBoundsAccessChain +// finished. +TEST_F(ValidateIdWithMessage, + OpInBoundsAccessChainMatrixMoreArgsThanNeededBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_float %my_matrix %int_0 %int_1 %int_0 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpInBoundsAccessChain reached non-composite type while " + "indexes still remain to be traversed.")); +} + +// Invalid: The result type and the type indexed into do not match. +TEST_F(ValidateIdWithMessage, + OpInBoundsAccessChainResultTypeDoesntMatchIndexedTypeBad) { + string spirv = kGLSL450MemoryModel + opAccessChainSpirvSetup + R"( +%entry = OpInBoundsAccessChain %_ptr_Private_mat4x3 %my_matrix %int_0 %int_1 +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpInBoundsAccessChain result type (OpTypeMatrix) does not " + "match the type that results from indexing into the " + "base (OpTypeFloat).")); +} + // TODO: OpArrayLength // TODO: OpImagePointer // TODO: OpGenericPtrMemSemantics