Check that certain decorations cannot be used more than once and/or are mutually exclusive (#2171)
Fixes #1636
* Add a hash functor for decoration types for c++11 compliance
* Change non-POD static variables and add test for Block+BufferBlock
diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
index f12c30f..582d11c 100644
--- a/source/val/validate_decorations.cpp
+++ b/source/val/validate_decorations.cpp
@@ -17,7 +17,9 @@
#include <algorithm>
#include <cassert>
#include <string>
+#include <tuple>
#include <unordered_map>
+#include <unordered_set>
#include <utility>
#include <vector>
@@ -44,6 +46,13 @@
}
};
+// A functor for hashing decoration types.
+struct SpvDecorationHash {
+ std::size_t operator()(SpvDecoration dec) const {
+ return static_cast<std::size_t>(dec);
+ }
+};
+
// Struct member layout attributes that are inherited through arrays.
struct LayoutConstraints {
explicit LayoutConstraints(
@@ -941,6 +950,109 @@
return SPV_SUCCESS;
}
+spv_result_t CheckDecorationsCompatibility(ValidationState_t& vstate) {
+ using AtMostOnceSet = std::unordered_set<SpvDecoration, SpvDecorationHash>;
+ using MutuallyExclusiveSets =
+ std::vector<std::unordered_set<SpvDecoration, SpvDecorationHash>>;
+ using PerIDKey = std::tuple<SpvDecoration, uint32_t>;
+ using PerMemberKey = std::tuple<SpvDecoration, uint32_t, uint32_t>;
+ using DecorationNameTable =
+ std::unordered_map<SpvDecoration, std::string, SpvDecorationHash>;
+
+ static const auto* const at_most_once_per_id = new AtMostOnceSet{
+ SpvDecorationArrayStride,
+ };
+ static const auto* const at_most_once_per_member = new AtMostOnceSet{
+ SpvDecorationOffset,
+ SpvDecorationMatrixStride,
+ SpvDecorationRowMajor,
+ SpvDecorationColMajor,
+ };
+ static const auto* const mutually_exclusive_per_id =
+ new MutuallyExclusiveSets{
+ {SpvDecorationBlock, SpvDecorationBufferBlock},
+ };
+ static const auto* const mutually_exclusive_per_member =
+ new MutuallyExclusiveSets{
+ {SpvDecorationRowMajor, SpvDecorationColMajor},
+ };
+ // For printing the decoration name.
+ static const auto* const decoration_name = new DecorationNameTable{
+ {SpvDecorationArrayStride, "ArrayStride"},
+ {SpvDecorationOffset, "Offset"},
+ {SpvDecorationMatrixStride, "MatrixStride"},
+ {SpvDecorationRowMajor, "RowMajor"},
+ {SpvDecorationColMajor, "ColMajor"},
+ {SpvDecorationBlock, "Block"},
+ {SpvDecorationBufferBlock, "BufferBlock"},
+ };
+
+ std::set<PerIDKey> seen_per_id;
+ std::set<PerMemberKey> seen_per_member;
+
+ for (const auto& inst : vstate.ordered_instructions()) {
+ const auto& words = inst.words();
+ if (SpvOpDecorate == inst.opcode()) {
+ const auto id = words[1];
+ const auto dec_type = static_cast<SpvDecoration>(words[2]);
+ const auto k = PerIDKey(dec_type, id);
+ const auto already_used = !seen_per_id.insert(k).second;
+ if (already_used &&
+ at_most_once_per_id->find(dec_type) != at_most_once_per_id->end()) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, vstate.FindDef(id))
+ << "ID '" << id << "' decorated with "
+ << decoration_name->at(dec_type)
+ << " multiple times is not allowed.";
+ }
+ // Verify certain mutually exclusive decorations are not both applied on
+ // an ID.
+ for (const auto& s : *mutually_exclusive_per_id) {
+ if (s.find(dec_type) == s.end()) continue;
+ for (auto excl_dec_type : s) {
+ if (excl_dec_type == dec_type) continue;
+ const auto excl_k = PerIDKey(excl_dec_type, id);
+ if (seen_per_id.find(excl_k) != seen_per_id.end()) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, vstate.FindDef(id))
+ << "ID '" << id << "' decorated with both "
+ << decoration_name->at(dec_type) << " and "
+ << decoration_name->at(excl_dec_type) << " is not allowed.";
+ }
+ }
+ }
+ } else if (SpvOpMemberDecorate == inst.opcode()) {
+ const auto id = words[1];
+ const auto member_id = words[2];
+ const auto dec_type = static_cast<SpvDecoration>(words[3]);
+ const auto k = PerMemberKey(dec_type, id, member_id);
+ const auto already_used = !seen_per_member.insert(k).second;
+ if (already_used && at_most_once_per_member->find(dec_type) !=
+ at_most_once_per_member->end()) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, vstate.FindDef(id))
+ << "ID '" << id << "', member '" << member_id
+ << "' decorated with " << decoration_name->at(dec_type)
+ << " multiple times is not allowed.";
+ }
+ // Verify certain mutually exclusive decorations are not both applied on
+ // a (ID, member) tuple.
+ for (const auto& s : *mutually_exclusive_per_member) {
+ if (s.find(dec_type) == s.end()) continue;
+ for (auto excl_dec_type : s) {
+ if (excl_dec_type == dec_type) continue;
+ const auto excl_k = PerMemberKey(excl_dec_type, id, member_id);
+ if (seen_per_member.find(excl_k) != seen_per_member.end()) {
+ return vstate.diag(SPV_ERROR_INVALID_ID, vstate.FindDef(id))
+ << "ID '" << id << "', member '" << member_id
+ << "' decorated with both " << decoration_name->at(dec_type)
+ << " and " << decoration_name->at(excl_dec_type)
+ << " is not allowed.";
+ }
+ }
+ }
+ }
+ }
+ return SPV_SUCCESS;
+}
+
spv_result_t CheckVulkanMemoryModelDeprecatedDecorations(
ValidationState_t& vstate) {
if (vstate.memory_model() != SpvMemoryModelVulkanKHR) return SPV_SUCCESS;
@@ -1108,6 +1220,7 @@
if (auto error = CheckImportedVariableInitialization(vstate)) return error;
if (auto error = CheckDecorationsOfEntryPoints(vstate)) return error;
if (auto error = CheckDecorationsOfBuffers(vstate)) return error;
+ if (auto error = CheckDecorationsCompatibility(vstate)) return error;
if (auto error = CheckLinkageAttrOfFunctions(vstate)) return error;
if (auto error = CheckVulkanMemoryModelDeprecatedDecorations(vstate))
return error;
diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index 6db3d12..3a4320d 100644
--- a/test/val/val_decoration_test.cpp
+++ b/test/val/val_decoration_test.cpp
@@ -4590,6 +4590,226 @@
" %call = OpFunctionCall %void %myfunc"));
}
+TEST_F(ValidateDecorations, MultipleOffsetDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 Offset 0
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %struct = OpTypeStruct %float
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2', member '0' decorated with Offset multiple "
+ "times is not allowed."));
+}
+
+TEST_F(ValidateDecorations, MultipleArrayStrideDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpDecorate %array ArrayStride 4
+ OpDecorate %array ArrayStride 4
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %uint = OpTypeInt 32 0
+ %uint_4 = OpConstant %uint 4
+ %array = OpTypeArray %float %uint_4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2' decorated with ArrayStride multiple "
+ "times is not allowed."));
+}
+
+TEST_F(ValidateDecorations, MultipleMatrixStrideDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 ColMajor
+ OpMemberDecorate %struct 0 MatrixStride 16
+ OpMemberDecorate %struct 0 MatrixStride 16
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %fvec4 = OpTypeVector %float 4
+ %fmat4 = OpTypeMatrix %fvec4 4
+ %struct = OpTypeStruct %fmat4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2', member '0' decorated with MatrixStride "
+ "multiple times is not allowed."));
+}
+
+TEST_F(ValidateDecorations, MultipleRowMajorDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 MatrixStride 16
+ OpMemberDecorate %struct 0 RowMajor
+ OpMemberDecorate %struct 0 RowMajor
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %fvec4 = OpTypeVector %float 4
+ %fmat4 = OpTypeMatrix %fvec4 4
+ %struct = OpTypeStruct %fmat4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2', member '0' decorated with RowMajor multiple "
+ "times is not allowed."));
+}
+
+TEST_F(ValidateDecorations, MultipleColMajorDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 MatrixStride 16
+ OpMemberDecorate %struct 0 ColMajor
+ OpMemberDecorate %struct 0 ColMajor
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %fvec4 = OpTypeVector %float 4
+ %fmat4 = OpTypeMatrix %fvec4 4
+ %struct = OpTypeStruct %fmat4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2', member '0' decorated with ColMajor multiple "
+ "times is not allowed."));
+}
+
+TEST_F(ValidateDecorations, RowMajorAndColMajorDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 MatrixStride 16
+ OpMemberDecorate %struct 0 ColMajor
+ OpMemberDecorate %struct 0 RowMajor
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %fvec4 = OpTypeVector %float 4
+ %fmat4 = OpTypeMatrix %fvec4 4
+ %struct = OpTypeStruct %fmat4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID '2', member '0' decorated with both RowMajor and "
+ "ColMajor is not allowed."));
+}
+
+TEST_F(ValidateDecorations, BlockAndBufferBlockDecorationsOnSameID) {
+ std::string spirv = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %1 "main"
+ OpExecutionMode %1 OriginUpperLeft
+
+ OpDecorate %struct Block
+ OpDecorate %struct BufferBlock
+ OpMemberDecorate %struct 0 Offset 0
+ OpMemberDecorate %struct 0 MatrixStride 16
+ OpMemberDecorate %struct 0 RowMajor
+
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %fvec4 = OpTypeVector %float 4
+ %fmat4 = OpTypeMatrix %fvec4 4
+ %struct = OpTypeStruct %fmat4
+
+ %1 = OpFunction %void None %voidfn
+ %label = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState());
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr(
+ "ID '2' decorated with both BufferBlock and Block is not allowed."));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
