From e3f1f3bda51387626094b054a19973c3d25b62dc Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 5 Jun 2018 09:11:35 -0400 Subject: [PATCH] Disallow array-of-arrays with DescriptorSets when validating. (#1586) * Disallow array-of-arrays with DescriptorSets when validating. This CL adds validation to disallow using an array-of-arrays when attached to a DescriptorSet. Fixes #1522 --- source/validate_decorations.cpp | 39 +++++++ test/val/val_decoration_test.cpp | 181 ++++++++++++++++++++++++++++++- 2 files changed, 218 insertions(+), 2 deletions(-) diff --git a/source/validate_decorations.cpp b/source/validate_decorations.cpp index 11d3ebca9..a8b8ee060 100644 --- a/source/validate_decorations.cpp +++ b/source/validate_decorations.cpp @@ -180,6 +180,44 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) { return SPV_SUCCESS; } +spv_result_t CheckDescriptorSetArrayOfArrays(ValidationState_t& vstate) { + for (const auto& def : vstate.all_definitions()) { + const auto inst = def.second; + if (SpvOpVariable != inst->opcode()) continue; + + // Verify this variable is a DescriptorSet + bool has_descriptor_set = false; + for (const auto& decoration : vstate.id_decorations(def.first)) { + if (SpvDecorationDescriptorSet == decoration.dec_type()) { + has_descriptor_set = true; + break; + } + } + if (!has_descriptor_set) continue; + + const auto& words = inst->words(); + const auto* ptrInst = vstate.FindDef(words[1]); + assert(SpvOpTypePointer == ptrInst->opcode()); + + // Check for a first level array + const auto typePtr = vstate.FindDef(ptrInst->words()[3]); + if (SpvOpTypeRuntimeArray != typePtr->opcode() && + SpvOpTypeArray != typePtr->opcode()) { + continue; + } + + // Check for a second level array + const auto secondaryTypePtr = vstate.FindDef(typePtr->words()[2]); + if (SpvOpTypeRuntimeArray == secondaryTypePtr->opcode() || + SpvOpTypeArray == secondaryTypePtr->opcode()) { + return vstate.diag(SPV_ERROR_INVALID_ID) + << "Only a single level of array is allowed for descriptor " + "set variables"; + } + } + return SPV_SUCCESS; +} + } // anonymous namespace namespace libspirv { @@ -189,6 +227,7 @@ spv_result_t ValidateDecorations(ValidationState_t& vstate) { if (auto error = CheckImportedVariableInitialization(vstate)) return error; if (auto error = CheckDecorationsOfEntryPoints(vstate)) return error; if (auto error = CheckLinkageAttrOfFunctions(vstate)) return error; + if (auto error = CheckDescriptorSetArrayOfArrays(vstate)) return error; return SPV_SUCCESS; } diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp index 2c0960c0b..f1e912051 100644 --- a/test/val/val_decoration_test.cpp +++ b/test/val/val_decoration_test.cpp @@ -21,11 +21,11 @@ namespace { -using ::testing::Eq; -using ::testing::HasSubstr; using libspirv::Decoration; using std::string; using std::vector; +using ::testing::Eq; +using ::testing::HasSubstr; using ValidateDecorations = spvtest::ValidateBase; @@ -548,4 +548,181 @@ OpFunctionEnd "Component decorations")); } +// #version 440 +// #extension GL_EXT_nonuniform_qualifier : enable +// layout(binding = 1) uniform sampler2D s2d[]; +// layout(location = 0) in nonuniformEXT int i; +// void main() +// { +// vec4 v = texture(s2d[i], vec2(0.3)); +// } +TEST_F(ValidateDecorations, RuntimeArrayOfDescriptorSetsIsAllowed) { + const spv_target_env env = SPV_ENV_VULKAN_1_0; + std::string spirv = R"( + OpCapability Shader + OpCapability ShaderNonUniformEXT + OpCapability RuntimeDescriptorArrayEXT + OpCapability SampledImageArrayNonUniformIndexingEXT + OpExtension "SPV_EXT_descriptor_indexing" + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Vertex %main "main" %i + OpSource GLSL 440 + OpSourceExtension "GL_EXT_nonuniform_qualifier" + OpName %main "main" + OpName %v "v" + OpName %s2d "s2d" + OpName %i "i" + OpDecorate %s2d DescriptorSet 0 + OpDecorate %s2d Binding 1 + OpDecorate %i Location 0 + OpDecorate %i NonUniformEXT + OpDecorate %18 NonUniformEXT + OpDecorate %21 NonUniformEXT + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float + %10 = OpTypeImage %float 2D 0 0 0 1 Unknown + %11 = OpTypeSampledImage %10 +%_runtimearr_11 = OpTypeRuntimeArray %11 +%_ptr_UniformConstant__runtimearr_11 = OpTypePointer UniformConstant %_runtimearr_11 + %s2d = OpVariable %_ptr_UniformConstant__runtimearr_11 UniformConstant + %int = OpTypeInt 32 1 +%_ptr_Input_int = OpTypePointer Input %int + %i = OpVariable %_ptr_Input_int Input +%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 + %v2float = OpTypeVector %float 2 +%float_0_300000012 = OpConstant %float 0.300000012 + %24 = OpConstantComposite %v2float %float_0_300000012 %float_0_300000012 + %float_0 = OpConstant %float 0 + %main = OpFunction %void None %3 + %5 = OpLabel + %v = OpVariable %_ptr_Function_v4float Function + %18 = OpLoad %int %i + %20 = OpAccessChain %_ptr_UniformConstant_11 %s2d %18 + %21 = OpLoad %11 %20 + %26 = OpImageSampleExplicitLod %v4float %21 %24 Lod %float_0 + OpStore %v %26 + OpReturn + OpFunctionEnd +)"; + CompileSuccessfully(spirv, env); + EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState()); +} + +// #version 440 +// #extension GL_EXT_nonuniform_qualifier : enable +// layout(binding = 1) uniform sampler2D s2d[][2]; +// layout(location = 0) in nonuniformEXT int i; +// void main() +// { +// vec4 v = texture(s2d[i][i], vec2(0.3)); +// } +TEST_F(ValidateDecorations, RuntimeArrayOfArraysOfDescriptorSetsIsDisallowed) { + const spv_target_env env = SPV_ENV_VULKAN_1_0; + std::string spirv = R"( + OpCapability Shader + OpCapability ShaderNonUniformEXT + OpCapability RuntimeDescriptorArrayEXT + OpCapability SampledImageArrayNonUniformIndexingEXT + OpExtension "SPV_EXT_descriptor_indexing" + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Vertex %main "main" %i + OpSource GLSL 440 + OpSourceExtension "GL_EXT_nonuniform_qualifier" + OpName %main "main" + OpName %v "v" + OpName %s2d "s2d" + OpName %i "i" + OpDecorate %s2d DescriptorSet 0 + OpDecorate %s2d Binding 1 + OpDecorate %i Location 0 + OpDecorate %i NonUniformEXT + OpDecorate %21 NonUniformEXT + OpDecorate %22 NonUniformEXT + OpDecorate %25 NonUniformEXT + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Function_v4float = OpTypePointer Function %v4float + %10 = OpTypeImage %float 2D 0 0 0 1 Unknown + %11 = OpTypeSampledImage %10 + %uint = OpTypeInt 32 0 + %uint_2 = OpConstant %uint 2 +%_arr_11_uint_2 = OpTypeArray %11 %uint_2 +%_runtimearr__arr_11_uint_2 = OpTypeRuntimeArray %_arr_11_uint_2 +%_ptr_UniformConstant__runtimearr__arr_11_uint_2 = OpTypePointer UniformConstant %_runtimearr__arr_11_uint_2 + %s2d = OpVariable %_ptr_UniformConstant__runtimearr__arr_11_uint_2 UniformConstant + %int = OpTypeInt 32 1 +%_ptr_Input_int = OpTypePointer Input %int + %i = OpVariable %_ptr_Input_int Input +%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 + %v2float = OpTypeVector %float 2 +%float_0_300000012 = OpConstant %float 0.300000012 + %28 = OpConstantComposite %v2float %float_0_300000012 %float_0_300000012 + %float_0 = OpConstant %float 0 + %main = OpFunction %void None %3 + %5 = OpLabel + %v = OpVariable %_ptr_Function_v4float Function + %21 = OpLoad %int %i + %22 = OpLoad %int %i + %24 = OpAccessChain %_ptr_UniformConstant_11 %s2d %21 %22 + %25 = OpLoad %11 %24 + %30 = OpImageSampleExplicitLod %v4float %25 %28 Lod %float_0 + OpStore %v %30 + OpReturn + OpFunctionEnd +)"; + CompileSuccessfully(spirv, env); + + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState(env)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Only a single level of array is allowed for " + "descriptor set variables")); +} + +// #version 440 +// layout (set=1, binding=1) uniform sampler2D variableName[2][2]; +// void main() { +// } +TEST_F(ValidateDecorations, ArrayOfArraysOfDescriptorSetsIsDisallowed) { + const spv_target_env env = SPV_ENV_VULKAN_1_0; + std::string spirv = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Vertex %main "main" + OpSource GLSL 440 + OpName %main "main" + OpName %variableName "variableName" + OpDecorate %variableName DescriptorSet 1 + OpDecorate %variableName Binding 1 + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %7 = OpTypeImage %float 2D 0 0 0 1 Unknown + %8 = OpTypeSampledImage %7 + %uint = OpTypeInt 32 0 + %uint_2 = OpConstant %uint 2 +%_arr_8_uint_2 = OpTypeArray %8 %uint_2 +%_arr__arr_8_uint_2_uint_2 = OpTypeArray %_arr_8_uint_2 %uint_2 +%_ptr_UniformConstant__arr__arr_8_uint_2_uint_2 = OpTypePointer UniformConstant %_arr__arr_8_uint_2_uint_2 +%variableName = OpVariable %_ptr_UniformConstant__arr__arr_8_uint_2_uint_2 UniformConstant + %main = OpFunction %void None %3 + %5 = OpLabel + OpReturn + OpFunctionEnd +)"; + CompileSuccessfully(spirv, env); + + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState(env)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Only a single level of array is allowed for " + "descriptor set variables")); +} + } // anonymous namespace