Vulkan BuiltIn variables can't have Location/Component decorations

As per Vulkan spec, BuiltIn variables can't have Location or Component
decorations. On some drivers, these can lead to driver crashing when
compiling the shader pipeline; for example, NVidia/AMD desktop drivers:
https://github.com/KhronosGroup/glslang/issues/1182.

This change adds validation and tests to catch this.

diff --git a/source/validate_decorations.cpp b/source/validate_decorations.cpp
index 6fd3eeb..11d3ebc 100644 (file)
--- a/source/validate_decorations.cpp
+++ b/source/validate_decorations.cpp
@@ -19,6 +19,7 @@
 
 #include "diagnostic.h"
 #include "opcode.h"
+#include "spirv_target_env.h"
 #include "val/validation_state.h"
 
 using libspirv::Decoration;
@@ -28,6 +29,14 @@ using libspirv::ValidationState_t;
 
 namespace {
 
+// Returns whether the given variable has a BuiltIn decoration.
+bool isBuiltInVar(uint32_t var_id, ValidationState_t& vstate) {
+  const auto& decorations = vstate.id_decorations(var_id);
+  return std::any_of(
+      decorations.begin(), decorations.end(),
+      [](const Decoration& d) { return SpvDecorationBuiltIn == d.dec_type(); });
+}
+
 // Returns whether the given structure type has any members with BuiltIn
 // decoration.
 bool isBuiltInStruct(uint32_t struct_id, ValidationState_t& vstate) {
@@ -91,6 +100,22 @@ spv_result_t CheckImportedVariableInitialization(ValidationState_t& vstate) {
   return SPV_SUCCESS;
 }
 
+// Checks whether a builtin variable is valid.
+spv_result_t CheckBuiltInVariable(uint32_t var_id, ValidationState_t& vstate) {
+  const auto& decorations = vstate.id_decorations(var_id);
+  for (const auto& d : decorations) {
+    if (spvIsVulkanEnv(vstate.context()->target_env)) {
+      if (d.dec_type() == SpvDecorationLocation ||
+          d.dec_type() == SpvDecorationComponent) {
+        return vstate.diag(SPV_ERROR_INVALID_ID)
+               << "A BuiltIn variable (id " << var_id
+               << ") cannot have any Location or Component decorations";
+      }
+    }
+  }
+  return SPV_SUCCESS;
+}
+
 // Checks whether proper decorations have been appied to the entry points.
 spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
   for (uint32_t entry_point : vstate.entry_points()) {
@@ -126,6 +151,9 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
         if (storage_class == SpvStorageClassInput) ++num_builtin_inputs;
         if (storage_class == SpvStorageClassOutput) ++num_builtin_outputs;
         if (num_builtin_inputs > 1 || num_builtin_outputs > 1) break;
+        if (auto error = CheckBuiltInVariable(interface, vstate)) return error;
+      } else if (isBuiltInVar(interface, vstate)) {
+        if (auto error = CheckBuiltInVariable(interface, vstate)) return error;
       }
     }
     if (num_builtin_inputs > 1 || num_builtin_outputs > 1) {

diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index 2feba52..2c0960c 100644 (file)
--- 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<bool>;
 
@@ -448,4 +448,104 @@ TEST_F(ValidateDecorations, FunctionDefinitionWithoutImportLinkageGood) {
   EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState());
 }
 
+TEST_F(ValidateDecorations, BuiltinVariablesGoodVulkan) {
+  const spv_target_env env = SPV_ENV_VULKAN_1_0;
+  std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %gl_FragCoord %_entryPointOutput
+OpExecutionMode %main OriginUpperLeft
+OpSource HLSL 500
+OpDecorate %gl_FragCoord BuiltIn FragCoord
+OpDecorate %_entryPointOutput Location 0
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%float_0 = OpConstant %float 0
+%14 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%_entryPointOutput = OpVariable %_ptr_Output_v4float Output
+%main = OpFunction %void None %3
+%5 = OpLabel
+OpStore %_entryPointOutput %14
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, env);
+  EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState(env));
+}
+
+TEST_F(ValidateDecorations, BuiltinVariablesWithLocationDecorationVulkan) {
+  const spv_target_env env = SPV_ENV_VULKAN_1_0;
+  std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %gl_FragCoord %_entryPointOutput
+OpExecutionMode %main OriginUpperLeft
+OpSource HLSL 500
+OpDecorate %gl_FragCoord BuiltIn FragCoord
+OpDecorate %gl_FragCoord Location 0
+OpDecorate %_entryPointOutput Location 0
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%float_0 = OpConstant %float 0
+%14 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%_entryPointOutput = OpVariable %_ptr_Output_v4float Output
+%main = OpFunction %void None %3
+%5 = OpLabel
+OpStore %_entryPointOutput %14
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, env);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("A BuiltIn variable (id 2) cannot have any Location or "
+                        "Component decorations"));
+}
+TEST_F(ValidateDecorations, BuiltinVariablesWithComponentDecorationVulkan) {
+  const spv_target_env env = SPV_ENV_VULKAN_1_0;
+  std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main" %gl_FragCoord %_entryPointOutput
+OpExecutionMode %main OriginUpperLeft
+OpSource HLSL 500
+OpDecorate %gl_FragCoord BuiltIn FragCoord
+OpDecorate %gl_FragCoord Component 0
+OpDecorate %_entryPointOutput Location 0
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%float = OpTypeFloat 32
+%v4float = OpTypeVector %float 4
+%float_0 = OpConstant %float 0
+%14 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+%gl_FragCoord = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+%_entryPointOutput = OpVariable %_ptr_Output_v4float Output
+%main = OpFunction %void None %3
+%5 = OpLabel
+OpStore %_entryPointOutput %14
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, env);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateAndRetrieveValidationState(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("A BuiltIn variable (id 2) cannot have any Location or "
+                        "Component decorations"));
+}
+
 }  // anonymous namespace