Validate that SpecId decoration target is a OpSpecConstant instruction

on a scalar

Fixes: https://github.com/KhronosGroup/SPIRV-Tools/issues/275

diff --git a/source/opcode.cpp b/source/opcode.cpp
index d0f1ef1..9f51b95 100644 (file)
--- 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 (file)
--- 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 (file)
--- a/source/validate_id.cpp
+++ b/source/validate_id.cpp
@@ -135,6 +135,25 @@ bool idUsage::isValid<SpvOpLine>(const spv_instruction_t* inst,
 }
 
 template <>
+bool idUsage::isValid<SpvOpDecorate>(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 <id> '"
+                        << inst->words[decorationIndex]
+                        << "' is not a scalar specialization constant.";
+      return false;
+    }
+  }
+  // TODO: Add validations for all decorations.
+  return true;
+}
+
+template <>
 bool idUsage::isValid<SpvOpMemberDecorate>(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 (file)
--- 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 (file)
--- 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 <id> '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 <id> '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 <id> '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