From edb52647bdab1484e8b5d13c393c2fbd77c28a03 Mon Sep 17 00:00:00 2001 From: Aliya Pazylbekova Date: Fri, 24 Feb 2017 20:43:28 -0500 Subject: [PATCH] Validate that SpecId decoration target is a OpSpecConstant instruction on a scalar Fixes: https://github.com/KhronosGroup/SPIRV-Tools/issues/275 --- source/opcode.cpp | 11 ++++++ source/opcode.h | 3 ++ source/validate_id.cpp | 20 ++++++++++ test/val/val_capability_test.cpp | 18 +++++---- test/val/val_id_test.cpp | 83 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 7 deletions(-) diff --git a/source/opcode.cpp b/source/opcode.cpp index d0f1ef1..9f51b95 100644 --- a/source/opcode.cpp +++ b/source/opcode.cpp @@ -207,6 +207,17 @@ bool spvOpcodeIsConstantOrUndef(const SpvOp opcode) { return opcode == SpvOpUndef || spvOpcodeIsConstant(opcode); } +bool spvOpcodeIsScalarSpecConstant(const SpvOp opcode) { + switch (opcode) { + case SpvOpSpecConstantTrue: + case SpvOpSpecConstantFalse: + case SpvOpSpecConstant: + return true; + default: + return false; + } +} + int32_t spvOpcodeIsComposite(const SpvOp opcode) { switch (opcode) { case SpvOpTypeVector: diff --git a/source/opcode.h b/source/opcode.h index 60d8e15..9742cbc 100644 --- a/source/opcode.h +++ b/source/opcode.h @@ -68,6 +68,9 @@ int32_t spvOpcodeIsConstant(const SpvOp opcode); // Returns true if the given opcode is a constant or undef. bool spvOpcodeIsConstantOrUndef(const SpvOp opcode); +// Returns true if the given opcode is a scalar specialization constant. +bool spvOpcodeIsScalarSpecConstant(const SpvOp opcode); + // Determines if the given opcode is a composite type. Returns zero if false, // non-zero otherwise. int32_t spvOpcodeIsComposite(const SpvOp opcode); diff --git a/source/validate_id.cpp b/source/validate_id.cpp index a5a73ea..dd799bc 100644 --- a/source/validate_id.cpp +++ b/source/validate_id.cpp @@ -135,6 +135,25 @@ bool idUsage::isValid(const spv_instruction_t* inst, } template <> +bool idUsage::isValid(const spv_instruction_t* inst, + const spv_opcode_desc) { + auto decorationIndex = 2; + auto decoration = inst->words[decorationIndex]; + if (decoration == SpvDecorationSpecId) { + auto targetIndex = 1; + auto target = module_.FindDef(inst->words[targetIndex]); + if (!target || !spvOpcodeIsScalarSpecConstant(target->opcode())) { + DIAG(targetIndex) << "OpDecorate SpectId decoration target '" + << inst->words[decorationIndex] + << "' is not a scalar specialization constant."; + return false; + } + } + // TODO: Add validations for all decorations. + return true; +} + +template <> bool idUsage::isValid(const spv_instruction_t* inst, const spv_opcode_desc) { auto structTypeIndex = 1; @@ -2808,6 +2827,7 @@ bool idUsage::isValid(const spv_instruction_t* inst) { TODO(OpUndef) CASE(OpMemberName) CASE(OpLine) + CASE(OpDecorate) CASE(OpMemberDecorate) CASE(OpGroupDecorate) CASE(OpGroupMemberDecorate) diff --git a/test/val/val_capability_test.cpp b/test/val/val_capability_test.cpp index eb64b46..4c8658d 100644 --- a/test/val/val_capability_test.cpp +++ b/test/val/val_capability_test.cpp @@ -1029,9 +1029,11 @@ INSTANTIATE_TEST_CASE_P( DecorationSpecId, ValidateCapability, Combine(ValuesIn(AllV10Capabilities()), Values(make_pair(string(kOpenCLMemoryModel) + - "OpEntryPoint Vertex %func \"shader\" \n" + - "OpDecorate %intt SpecId 1\n" - "%intt = OpTypeInt 32 0\n" + string(kVoidFVoid), + "OpEntryPoint Vertex %func \"shader\" \n" + + "OpDecorate %1 SpecId 1\n" + "%intt = OpTypeInt 32 0\n" + "%1 = OpSpecConstant %intt 0\n" + + string(kVoidFVoid), ShaderDependencies()))), ); INSTANTIATE_TEST_CASE_P( @@ -1050,14 +1052,16 @@ INSTANTIATE_TEST_CASE_P( // fixed. make_pair(string("OpMemoryModel Logical OpenCL " "OpEntryPoint Kernel %func \"compute\" \n" - "OpDecorate %intt SpecId 1 " - "%intt = OpTypeInt 32 0 ") + + "OpDecorate %1 SpecId 1 " + "%intt = OpTypeInt 32 0 " + "%1 = OpSpecConstant %intt 0") + string(kVoidFVoid), KernelDependencies()), make_pair(string("OpMemoryModel Logical Simple " "OpEntryPoint Vertex %func \"shader\" \n" - "OpDecorate %intt SpecId 1 " - "%intt = OpTypeInt 32 0 ") + + "OpDecorate %1 SpecId 1 " + "%intt = OpTypeInt 32 0 " + "%1 = OpSpecConstant %intt 0") + string(kVoidFVoid), ShaderDependencies()))), ); // clang-format off diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index f8ede50..7861f4c 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -3798,6 +3798,89 @@ OpFunctionEnd "ID 7 defined in block 6 does not dominate its use in block 9")); } +TEST_F(ValidateIdWithMessage, SpecIdTargetNotSpecializationConstant) { + string spirv = kGLSL450MemoryModel + R"( +OpDecorate %3 SpecId 200 +%1 = OpTypeVoid +%2 = OpTypeFunction %1 +%int = OpTypeInt 32 0 +%3 = OpConstant %int 3 +%main = OpFunction %1 None %2 +%4 = OpLabel +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpDecorate SpectId decoration target '1' is not a " + "scalar specialization constant.")); +} + +TEST_F(ValidateIdWithMessage, SpecIdTargetOpSpecConstantOpBad) { + string spirv = kGLSL450MemoryModel + R"( +OpDecorate %5 SpecId 200 +%1 = OpTypeVoid +%2 = OpTypeFunction %1 +%int = OpTypeInt 32 0 +%3 = OpConstant %int 1 +%4 = OpConstant %int 2 +%5 = OpSpecConstantOp %int IAdd %3 %4 +%main = OpFunction %1 None %2 +%6 = OpLabel +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpDecorate SpectId decoration target '1' is not a " + "scalar specialization constant.")); +} + +TEST_F(ValidateIdWithMessage, SpecIdTargetOpSpecConstantCompositeBad) { + string spirv = kGLSL450MemoryModel + R"( +OpDecorate %3 SpecId 200 +%1 = OpTypeVoid +%2 = OpTypeFunction %1 +%int = OpTypeInt 32 0 +%3 = OpSpecConstantComposite %int +%main = OpFunction %1 None %2 +%4 = OpLabel +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OpDecorate SpectId decoration target '1' is not a " + "scalar specialization constant.")); +} + +TEST_F(ValidateIdWithMessage, SpecIdTargetGood) { + string spirv = kGLSL450MemoryModel + R"( +OpDecorate %3 SpecId 200 +OpDecorate %4 SpecId 201 +OpDecorate %5 SpecId 202 +%1 = OpTypeVoid +%2 = OpTypeFunction %1 +%int = OpTypeInt 32 0 +%bool = OpTypeBool +%3 = OpSpecConstant %int 3 +%4 = OpSpecConstantTrue %bool +%5 = OpSpecConstantFalse %bool +%main = OpFunction %1 None %2 +%6 = OpLabel +OpReturn +OpFunctionEnd + )"; + CompileSuccessfully(spirv.c_str()); + EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState()); +} + // TODO: OpLifetimeStart // TODO: OpLifetimeStop // TODO: OpAtomicInit -- 2.7.4