spirv-val: Validate PhysicalStorageBuffer Stage Interface (#5539)

Disallow PhysicalStorageBuffer pointers in Input and Output storage classes.

source/val/validate_interfaces.cpp

@@ -46,6 +46,29 @@ bool is_interface_variable(const Instruction* inst, bool is_spv_1_4) {
   }
 }
 
+// Special validation for varibles that are between shader stages
+spv_result_t ValidateInputOutputInterfaceVariables(ValidationState_t& _,
+                                                   const Instruction* var) {
+  auto var_pointer = _.FindDef(var->GetOperandAs<uint32_t>(0));
+  uint32_t pointer_id = var_pointer->GetOperandAs<uint32_t>(2);
+
+  const auto isPhysicalStorageBuffer = [](const Instruction* insn) {
+    return insn->opcode() == spv::Op::OpTypePointer &&
+           insn->GetOperandAs<spv::StorageClass>(1) ==
+               spv::StorageClass::PhysicalStorageBuffer;
+  };
+
+  if (_.ContainsType(pointer_id, isPhysicalStorageBuffer)) {
+    return _.diag(SPV_ERROR_INVALID_ID, var)
+           << _.VkErrorID(9557) << "Input/Output interface variable id <"
+           << var->id()
+           << "> contains a PhysicalStorageBuffer pointer, which is not "
+              "allowed. If you want to interface shader stages with a "
+              "PhysicalStorageBuffer, cast to a uint64 or uvec2 instead.";
+  }
+  return SPV_SUCCESS;
+}
+
 // Checks that \c var is listed as an interface in all the entry points that use
 // it.
 spv_result_t check_interface_variable(ValidationState_t& _,
@@ -105,6 +128,12 @@ spv_result_t check_interface_variable(ValidationState_t& _,
     }
   }
 
+  if (var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Input ||
+      var->GetOperandAs<spv::StorageClass>(2) == spv::StorageClass::Output) {
+    if (auto error = ValidateInputOutputInterfaceVariables(_, var))
+      return error;
+  }
+
   return SPV_SUCCESS;
 }
 

source/val/validation_state.cpp

@@ -2299,6 +2299,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
       return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08722);
     case 8973:
       return VUID_WRAP(VUID-StandaloneSpirv-Pointer-08973);
+    case 9557:
+      return VUID_WRAP(VUID-StandaloneSpirv-Input-09557);
     default:
       return "";  // unknown id
   }

test/val/val_interfaces_test.cpp

@@ -1599,7 +1599,7 @@ TEST_F(ValidateInterfacesTest, InvalidLocationTypePointer) {
               HasSubstr("Invalid type to assign a location"));
 }
 
-TEST_F(ValidateInterfacesTest, ValidLocationTypePhysicalStorageBufferPointer) {
+TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointer) {
   const std::string text = R"(
 OpCapability Shader
 OpCapability PhysicalStorageBufferAddresses
@@ -1608,10 +1608,10 @@ OpEntryPoint Vertex %main "main" %var
 OpDecorate %var Location 0
 OpDecorate %var RestrictPointer
 %void = OpTypeVoid
-%int = OpTypeInt 32 0
-%ptr = OpTypePointer PhysicalStorageBuffer %int
-%ptr2 = OpTypePointer Input %ptr
-%var = OpVariable %ptr2 Input
+%uint = OpTypeInt 32 0
+%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
+%in_ptr = OpTypePointer Input %psb_ptr
+%var = OpVariable %in_ptr Input
 %void_fn = OpTypeFunction %void
 %main = OpFunction %void None %void_fn
 %entry = OpLabel
@@ -1619,7 +1619,140 @@ OpReturn
 OpFunctionEnd
 )";
   CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
-  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-Input-09557"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Input/Output interface variable id <2> contains a "
+                        "PhysicalStorageBuffer pointer, which is not allowed"));
+}
+
+TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArray) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability PhysicalStorageBufferAddresses
+OpMemoryModel PhysicalStorageBuffer64 GLSL450
+OpEntryPoint Vertex %main "main" %var
+OpDecorate %var Location 0
+OpDecorate %var RestrictPointer
+%void = OpTypeVoid
+%uint = OpTypeInt 32 0
+%uint_3 = OpConstant %uint 3
+%psb_ptr = OpTypePointer PhysicalStorageBuffer %uint
+%array = OpTypeArray %psb_ptr %uint_3
+%in_ptr = OpTypePointer Input %array
+%var = OpVariable %in_ptr Input
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-Input-09557"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Input/Output interface variable id <2> contains a "
+                        "PhysicalStorageBuffer pointer, which is not allowed"));
+}
+
+TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerStruct) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability PhysicalStorageBufferAddresses
+OpMemoryModel PhysicalStorageBuffer64 GLSL450
+OpEntryPoint Vertex %main "main" %var
+OpDecorate %var Location 0
+OpDecorate %var RestrictPointer
+%void = OpTypeVoid
+%int = OpTypeInt 32 1
+OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
+%struct_0 = OpTypeStruct %int %psb_ptr
+%struct_1 = OpTypeStruct %int %int
+%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
+%in_ptr = OpTypePointer Input %struct_0
+%var = OpVariable %in_ptr Input
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-Input-09557"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Input/Output interface variable id <2> contains a "
+                        "PhysicalStorageBuffer pointer, which is not allowed"));
+}
+
+TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerArrayOfStruct) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability PhysicalStorageBufferAddresses
+OpMemoryModel PhysicalStorageBuffer64 GLSL450
+OpEntryPoint Vertex %main "main" %var
+OpDecorate %var Location 0
+OpDecorate %var RestrictPointer
+%void = OpTypeVoid
+%int = OpTypeInt 32 1
+%uint = OpTypeInt 32 0
+%uint_3 = OpConstant %uint 3
+OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
+%array_1 = OpTypeArray %psb_ptr %uint_3
+%struct_0 = OpTypeStruct %int %array_1
+ %struct_1 = OpTypeStruct %int %int
+%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
+%array_0 = OpTypeArray %struct_0 %uint_3
+%in_ptr = OpTypePointer Input %array_0
+%var = OpVariable %in_ptr Input
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-Input-09557"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Input/Output interface variable id <2> contains a "
+                        "PhysicalStorageBuffer pointer, which is not allowed"));
+}
+
+TEST_F(ValidateInterfacesTest, PhysicalStorageBufferPointerNestedStruct) {
+  const std::string text = R"(
+OpCapability Shader
+OpCapability PhysicalStorageBufferAddresses
+OpMemoryModel PhysicalStorageBuffer64 GLSL450
+OpEntryPoint Vertex %main "main" %var
+OpDecorate %var Location 0
+OpDecorate %var RestrictPointer
+%void = OpTypeVoid
+%int = OpTypeInt 32 1
+OpTypeForwardPointer %psb_ptr PhysicalStorageBuffer
+%struct_0 = OpTypeStruct %int %psb_ptr
+%struct_1 = OpTypeStruct %int %int
+%psb_ptr = OpTypePointer PhysicalStorageBuffer %struct_1
+%struct_2 = OpTypeStruct %int %struct_0
+%in_ptr = OpTypePointer Input %struct_2
+%var = OpVariable %in_ptr Input
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+  EXPECT_THAT(getDiagnosticString(),
+              AnyVUID("VUID-StandaloneSpirv-Input-09557"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Input/Output interface variable id <2> contains a "
+                        "PhysicalStorageBuffer pointer, which is not allowed"));
 }
 
 }  // namespace