Validate decoration of structs with RuntimeArray (#5094)

Contributes to KhronosGroup/glslang#2439 * When OpTypeStruct is used in Vulkan env and its last member is a RuntimeArray, check if the struct is decorated with Block or BufferBlock, as required by VUID-...-04680.
KhronosGroup · Feb 3, 2023 · fd1e650 · fd1e650
1 parent 5890763
commit fd1e650
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 3 deletions.
diff --git a/source/val/validate_type.cpp b/source/val/validate_type.cpp
@@ -349,6 +349,15 @@ spv_result_t ValidateTypeStruct(ValidationState_t& _, const Instruction* inst) {
                << ", OpTypeRuntimeArray must only be used for the last member "
                   "of an OpTypeStruct";
       }
+
+      if (!_.HasDecoration(inst->id(), spv::Decoration::Block) &&
+          !_.HasDecoration(inst->id(), spv::Decoration::BufferBlock)) {
+        return _.diag(SPV_ERROR_INVALID_ID, inst)
+               << _.VkErrorID(4680)
+               << spvLogStringForEnv(_.context()->target_env)
+               << ", OpTypeStruct containing an OpTypeRuntimeArray "
+               << "must be decorated with Block or BufferBlock.";
+      }
     }
   }
 

diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
@@ -5281,6 +5281,37 @@ OpFunctionEnd
           "rules: member 1 at offset 1 is not aligned to 4"));
 }
 
+TEST_F(ValidateDecorations, VulkanStructWithoutDecorationWithRuntimeArray) {
+  std::string str = R"(
+              OpCapability Shader
+              OpMemoryModel Logical GLSL450
+              OpEntryPoint Fragment %func "func"
+              OpExecutionMode %func OriginUpperLeft
+              OpDecorate %array_t ArrayStride 4
+              OpMemberDecorate %struct_t 0 Offset 0
+              OpMemberDecorate %struct_t 1 Offset 4
+     %uint_t = OpTypeInt 32 0
+   %array_t = OpTypeRuntimeArray %uint_t
+  %struct_t = OpTypeStruct %uint_t %array_t
+%struct_ptr = OpTypePointer StorageBuffer %struct_t
+         %2 = OpVariable %struct_ptr StorageBuffer
+      %void = OpTypeVoid
+    %func_t = OpTypeFunction %void
+      %func = OpFunction %void None %func_t
+         %1 = OpLabel
+              OpReturn
+              OpFunctionEnd
+)";
+
+  CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_1);
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-OpTypeRuntimeArray-04680"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Vulkan, OpTypeStruct containing an OpTypeRuntimeArray "
+                        "must be decorated with Block or BufferBlock."));
+}
+
 TEST_F(ValidateDecorations, EmptyStructAtNonZeroOffsetGood) {
   const std::string spirv = R"(
 OpCapability Shader

diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp
@@ -2475,6 +2475,7 @@ OpCapability Shader
 OpMemoryModel Logical GLSL450
 OpEntryPoint Fragment %func "func"
 OpExecutionMode %func OriginUpperLeft
+OpDecorate %struct_t Block
 %uint_t = OpTypeInt 32 0
 %array_t = OpTypeRuntimeArray %uint_t
 %struct_t = OpTypeStruct %array_t
@@ -2498,7 +2499,7 @@ OpFunctionEnd
           "For Vulkan, OpTypeStruct variables containing OpTypeRuntimeArray "
           "must have storage class of StorageBuffer, PhysicalStorageBuffer, or "
           "Uniform.\n  %6 = "
-          "OpVariable %_ptr_Workgroup__struct_4 Workgroup\n"));
+          "OpVariable %_ptr_Workgroup__struct_2 Workgroup\n"));
 }
 
 TEST_F(ValidateMemory, VulkanRTAInsideStorageBufferStructWithoutBlockBad) {
@@ -2507,6 +2508,7 @@ OpCapability Shader
 OpMemoryModel Logical GLSL450
 OpEntryPoint Fragment %func "func"
 OpExecutionMode %func OriginUpperLeft
+OpDecorate %struct_t BufferBlock
 %uint_t = OpTypeInt 32 0
 %array_t = OpTypeRuntimeArray %uint_t
 %struct_t = OpTypeStruct %array_t
@@ -2529,7 +2531,7 @@ OpFunctionEnd
                         "OpTypeRuntimeArray must be decorated with Block if it "
                         "has storage class StorageBuffer or "
                         "PhysicalStorageBuffer.\n  %6 = OpVariable "
-                        "%_ptr_StorageBuffer__struct_4 StorageBuffer\n"));
+                        "%_ptr_StorageBuffer__struct_2 StorageBuffer\n"));
 }
 
 TEST_F(ValidateMemory, VulkanRTAInsideUniformStructGood) {
@@ -2564,6 +2566,7 @@ OpCapability Shader
 OpMemoryModel Logical GLSL450
 OpEntryPoint Fragment %func "func"
 OpExecutionMode %func OriginUpperLeft
+OpDecorate %struct_t Block
 %uint_t = OpTypeInt 32 0
 %array_t = OpTypeRuntimeArray %uint_t
 %struct_t = OpTypeStruct %array_t
@@ -2585,7 +2588,7 @@ OpFunctionEnd
               HasSubstr("For Vulkan, an OpTypeStruct variable containing an "
                         "OpTypeRuntimeArray must be decorated with BufferBlock "
                         "if it has storage class Uniform.\n  %6 = OpVariable "
-                        "%_ptr_Uniform__struct_4 Uniform\n"));
+                        "%_ptr_Uniform__struct_2 Uniform\n"));
 }
 
 TEST_F(ValidateMemory, VulkanRTAInsideRTABad) {