Validator: check OpTypeBool inside Blocks (#1405)

OpTypeBool can only be used with non-externally visible shader
Storage Classes: Workgroup, CrossWorkgroup, Private, and Function.

source/val/validate_memory.cpp

@@ -139,6 +139,47 @@ bool HasConflictingMemberOffsets(
   return false;
 }
 
+// If |skip_builtin| is true, returns true if |storage| contains bool within
+// it and no storage that contains the bool is builtin.
+// If |skip_builtin| is false, returns true if |storage| contains bool within
+// it.
+bool ContainsInvalidBool(ValidationState_t& _, const Instruction* storage,
+                         bool skip_builtin) {
+  if (skip_builtin) {
+    for (const Decoration& decoration : _.id_decorations(storage->id())) {
+      if (decoration.dec_type() == SpvDecorationBuiltIn) return false;
+    }
+  }
+
+  const size_t elem_type_index = 1;
+  uint32_t elem_type_id;
+  Instruction* elem_type;
+
+  switch (storage->opcode()) {
+    case SpvOpTypeBool:
+      return true;
+    case SpvOpTypeVector:
+    case SpvOpTypeMatrix:
+    case SpvOpTypeArray:
+    case SpvOpTypeRuntimeArray:
+      elem_type_id = storage->GetOperandAs<uint32_t>(elem_type_index);
+      elem_type = _.FindDef(elem_type_id);
+      return ContainsInvalidBool(_, elem_type, skip_builtin);
+    case SpvOpTypeStruct:
+      for (size_t member_type_index = 1;
+           member_type_index < storage->operands().size();
+           ++member_type_index) {
+        auto member_type_id =
+            storage->GetOperandAs<uint32_t>(member_type_index);
+        auto member_type = _.FindDef(member_type_id);
+        if (ContainsInvalidBool(_, member_type, skip_builtin)) return true;
+      }
+    default:
+      break;
+  }
+  return false;
+}
+
 spv_result_t ValidateVariable(ValidationState_t& _, const Instruction& inst) {
   auto result_type = _.FindDef(inst.type_id());
   if (!result_type || result_type->opcode() != SpvOpTypePointer) {
@@ -148,10 +189,10 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction& inst) {
   }
 
   const auto initializer_index = 3;
+  const auto storage_class_index = 2;
   if (initializer_index < inst.operands().size()) {
     const auto initializer_id = inst.GetOperandAs<uint32_t>(initializer_index);
     const auto initializer = _.FindDef(initializer_id);
-    const auto storage_class_index = 2;
     const auto is_module_scope_var =
         initializer && (initializer->opcode() == SpvOpVariable) &&
         (initializer->GetOperandAs<uint32_t>(storage_class_index) !=
@@ -165,6 +206,33 @@ spv_result_t ValidateVariable(ValidationState_t& _, const Instruction& inst) {
     }
   }
 
+  const auto storage_class = inst.GetOperandAs<uint32_t>(storage_class_index);
+  if (storage_class != SpvStorageClassWorkgroup &&
+      storage_class != SpvStorageClassCrossWorkgroup &&
+      storage_class != SpvStorageClassPrivate &&
+      storage_class != SpvStorageClassFunction) {
+    const auto storage_index = 2;
+    const auto storage_id = result_type->GetOperandAs<uint32_t>(storage_index);
+    const auto storage = _.FindDef(storage_id);
+    bool storage_input_or_output = storage_class == SpvStorageClassInput ||
+                                   storage_class == SpvStorageClassOutput;
+    bool builtin = false;
+    if (storage_input_or_output) {
+      for (const Decoration& decoration : _.id_decorations(inst.id())) {
+        if (decoration.dec_type() == SpvDecorationBuiltIn) {
+          builtin = true;
+          break;
+        }
+      }
+    }
+    if (!(storage_input_or_output && builtin) &&
+        ContainsInvalidBool(_, storage, storage_input_or_output)) {
+      return _.diag(SPV_ERROR_INVALID_ID, &inst)
+             << "If OpTypeBool is stored in conjunction with OpVariable, it "
+             << "can only be used with non-externally visible shader Storage "
+             << "Classes: Workgroup, CrossWorkgroup, Private, and Function";
+    }
+  }
   return SPV_SUCCESS;
 }
 

test/val/val_composites_test.cpp

@@ -149,15 +149,14 @@ OpMemoryModel Logical GLSL450
 ; }
 
 %f32arr = OpTypeRuntimeArray %float
-%bool = OpTypeBool
 %v4float = OpTypeVector %float 4
 %array5_mat4x3 = OpTypeArray %mat4x3 %int_5
 %array5_vec4 = OpTypeArray %v4float %int_5
 %_ptr_Uniform_float = OpTypePointer Uniform %float
 %_ptr_Function_vec4 = OpTypePointer Function %v4float
 %_ptr_Uniform_vec4 = OpTypePointer Uniform %v4float
-%struct_s = OpTypeStruct %bool %array5_vec4 %int %array5_mat4x3
-%struct_blockName = OpTypeStruct %struct_s %bool %f32arr
+%struct_s = OpTypeStruct %int %array5_vec4 %int %array5_mat4x3
+%struct_blockName = OpTypeStruct %struct_s %int %f32arr
 %_ptr_Uniform_blockName = OpTypePointer Uniform %struct_blockName
 %_ptr_Uniform_struct_s = OpTypePointer Uniform %struct_s
 %_ptr_Uniform_array5_mat4x3 = OpTypePointer Uniform %array5_mat4x3
@@ -1364,7 +1363,7 @@ TEST_F(ValidateComposites, CompositeExtractStructIndexOutOfBoundBad) {
   EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
               HasSubstr("Index is out of bounds, can not find index 3 in the "
-                        "structure <id> '26'. This structure has 3 members. "
+                        "structure <id> '25'. This structure has 3 members. "
                         "Largest valid index is 2."));
 }
 
@@ -1385,7 +1384,7 @@ TEST_F(ValidateComposites, CompositeInsertStructIndexOutOfBoundBad) {
   EXPECT_THAT(
       getDiagnosticString(),
       HasSubstr("Index is out of bounds, can not find index 3 in the structure "
-                "<id> '26'. This structure has 3 members. Largest valid index "
+                "<id> '25'. This structure has 3 members. Largest valid index "
                 "is 2."));
 }
 

test/val/val_id_test.cpp

@@ -1848,6 +1848,69 @@ OpFunctionEnd
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
+TEST_F(ValidateIdWithMessage, OpVariableContainsBoolBad) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+%bool = OpTypeBool
+%int = OpTypeInt 32 0
+%block = OpTypeStruct %bool %int
+%_ptr_Uniform_block = OpTypePointer Uniform %block
+%var = OpVariable %_ptr_Uniform_block Uniform
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%load = OpLoad %block %var
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("If OpTypeBool is stored in conjunction with OpVariable"
+                        ", it can only be used with non-externally visible "
+                        "shader Storage Classes: Workgroup, CrossWorkgroup, "
+                        "Private, and Function"));
+}
+
+TEST_F(ValidateIdWithMessage, OpVariableContainsBoolPointerGood) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+%bool = OpTypeBool
+%boolptr = OpTypePointer Uniform %bool
+%int = OpTypeInt 32 0
+%block = OpTypeStruct %boolptr %int
+%_ptr_Uniform_block = OpTypePointer Uniform %block
+%var = OpVariable %_ptr_Uniform_block Uniform
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%load = OpLoad %block %var
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateIdWithMessage, OpVariableContainsBuiltinBoolGood) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+OpMemberDecorate %input 0 BuiltIn FrontFacing
+%bool = OpTypeBool
+%input = OpTypeStruct %bool
+%_ptr_input = OpTypePointer Input %input
+%var = OpVariable %_ptr_input Input
+%void = OpTypeVoid
+%fnty = OpTypeFunction %void
+%main = OpFunction %void None %fnty
+%entry = OpLabel
+%load = OpLoad %input %var
+OpReturn
+OpFunctionEnd
+)";
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateIdWithMessage, OpLoadGood) {
   std::string spirv = kGLSL450MemoryModel + R"(
  %1 = OpTypeVoid
@@ -2991,15 +3054,14 @@ const char kDeeplyNestedStructureSetup[] = R"(
 ; }
 
 %f32arr = OpTypeRuntimeArray %float
-%bool = OpTypeBool
 %v4float = OpTypeVector %float 4
 %array5_mat4x3 = OpTypeArray %mat4x3 %int_5
 %array5_vec4 = OpTypeArray %v4float %int_5
 %_ptr_Uniform_float = OpTypePointer Uniform %float
 %_ptr_Function_vec4 = OpTypePointer Function %v4float
 %_ptr_Uniform_vec4 = OpTypePointer Uniform %v4float
-%struct_s = OpTypeStruct %bool %array5_vec4 %int %array5_mat4x3
-%struct_blockName = OpTypeStruct %struct_s %bool %f32arr
+%struct_s = OpTypeStruct %int %array5_vec4 %int %array5_mat4x3
+%struct_blockName = OpTypeStruct %struct_s %int %f32arr
 %_ptr_Uniform_blockName = OpTypePointer Uniform %struct_blockName
 %_ptr_Uniform_struct_s = OpTypePointer Uniform %struct_s
 %_ptr_Uniform_array5_mat4x3 = OpTypePointer Uniform %array5_mat4x3
@@ -3051,7 +3113,7 @@ OpFunctionEnd
   )";
 
   const std::string expected_err = "The Result Type of " + instr +
-                                   " <id> '36' must be "
+                                   " <id> '35' must be "
                                    "OpTypePointer. Found OpTypeFloat.";
   CompileSuccessfully(spirv);
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
@@ -3397,7 +3459,7 @@ OpFunctionEnd
   )";
   const std::string expected_err = "Index is out of bounds: " + instr +
                                    " can not find index 3 into the structure "
-                                   "<id> '26'. This structure has 3 members. "
+                                   "<id> '25'. This structure has 3 members. "
                                    "Largest valid index is 2.";
   CompileSuccessfully(spirv);
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());