spirv-val: Disallow stores according to VUID 06924

Ensure that the validator rejects stores to objects of types `OpTypeSampledImage` and `OpTypeAccelerationStructureKHR`, according to `VUID-StandaloneSpirv-OpTypeImage-06924`. Stores to objects of types `OpTypeImage` and `OpTypeSampler` should be rejected too, but HLSL-to-SPIR-V translation by glslang and shaderc seems to result in some violations; so leave validation of those as future work. Contributes to #4796 Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>
KhronosGroup · Sep 20, 2024 · b81dfd2 · b81dfd2
1 parent 6dcc7e3
commit b81dfd2
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 0 deletions.
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
@@ -1163,6 +1163,22 @@ spv_result_t ValidateStore(ValidationState_t& _, const Instruction* inst) {
     }
   }
 
+  if (spvIsVulkanEnv(_.context()->target_env)) {
+    const auto isForbiddenType = [](const Instruction* type_inst) {
+      auto opcode = type_inst->opcode();
+      // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/4796): add
+      // OpTypeImage and OpTypeSampler.
+      return opcode == spv::Op::OpTypeSampledImage ||
+             opcode == spv::Op::OpTypeAccelerationStructureKHR;
+    };
+    if (_.ContainsType(object_type->id(), isForbiddenType)) {
+      return _.diag(SPV_ERROR_INVALID_ID, inst)
+             << _.VkErrorID(6924)
+             << "Cannot store to OpTypeImage, OpTypeSampler, "
+                "OpTypeSampledImage, or OpTypeAccelerationStructureKHR objects";
+    }
+  }
+
   return SPV_SUCCESS;
 }
 

diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp
@@ -2368,6 +2368,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-StandaloneSpirv-Uniform-06807);
     case 6808:
       return VUID_WRAP(VUID-StandaloneSpirv-PushConstant-06808);
+    case 6924:
+      return VUID_WRAP(VUID-StandaloneSpirv-OpTypeImage-06924);
     case 6925:
       return VUID_WRAP(VUID-StandaloneSpirv-Uniform-06925);
     case 7041:

diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp
@@ -3934,6 +3934,68 @@ OpMemoryModel Logical GLSL450
               HasSubstr("Initializer type must match the data type"));
 }
 
+TEST_F(ValidateMemory, StoreToSampledImage) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%img = OpTypeImage %int 2D 2 0 0 1 R32i
+%samp_img = OpTypeSampledImage %img
+%ptr_samp_img = OpTypePointer Function %samp_img
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+%var = OpVariable %ptr_samp_img Function
+%value = OpLoad %samp_img %var
+OpStore %var %value
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_1);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-OpTypeImage-06924"));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Cannot store to OpTypeImage, OpTypeSampler, "
+                "OpTypeSampledImage, or OpTypeAccelerationStructureKHR"));
+}
+
+TEST_F(ValidateMemory, StoreToAccelarationStructureKHR) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability RayQueryKHR
+OpExtension "SPV_KHR_ray_query"
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+%void = OpTypeVoid
+%as = OpTypeAccelerationStructureKHR
+%ptr_as = OpTypePointer Function %as
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+%var = OpVariable %ptr_as Function
+%value = OpLoad %as %var
+OpStore %var %value
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_1);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-OpTypeImage-06924"));
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Cannot store to OpTypeImage, OpTypeSampler, "
+                "OpTypeSampledImage, or OpTypeAccelerationStructureKHR"));
+}
+
 TEST_F(ValidateMemory, StoreToUniformBlock) {
   const std::string spirv = R"(
 OpCapability Shader