spirv-val: Add Concrete pointer check

Author: spencer-lunarg

source/val/validate_misc.cpp

@@ -88,6 +88,13 @@ spv_result_t ValidateSizeOf(ValidationState_t& _, const Instruction* inst) {
     return _.diag(SPV_ERROR_INVALID_DATA, inst)
            << "Expected OpSizeOf Result Type to be a 32-bit int scalar.";
   }
+
+  uint32_t pointer_id = inst->GetOperandAs<uint32_t>(2);
+  if (!_.IsConcreteType(pointer_id)) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "OpSizeOf Pointer operand is not concrete.";
+  }
+
   return SPV_SUCCESS;
 }
 

source/val/validation_state.cpp

@@ -30,6 +30,7 @@
 #include "source/val/construct.h"
 #include "source/val/function.h"
 #include "spirv-tools/libspirv.h"
+#include "spirv/unified1/spirv.hpp11"
 
 namespace spvtools {
 namespace val {
@@ -926,22 +927,16 @@ uint32_t ValidationState_t::GetComponentType(uint32_t id) const {
 
     case spv::Op::OpTypeArray:
     case spv::Op::OpTypeRuntimeArray:
-      return inst->word(2);
-
     case spv::Op::OpTypeVector:
-      return inst->word(2);
-
-    case spv::Op::OpTypeMatrix:
-      return GetComponentType(inst->word(2));
-
+    case spv::Op::OpTypeVectorIdEXT:
     case spv::Op::OpTypeCooperativeMatrixNV:
     case spv::Op::OpTypeCooperativeMatrixKHR:
-    case spv::Op::OpTypeVectorIdEXT:
-      return inst->word(2);
-
     case spv::Op::OpTypeTensorARM:
       return inst->word(2);
 
+    case spv::Op::OpTypeMatrix:
+      return GetComponentType(inst->word(2));
+
     default:
       break;
   }
@@ -1547,6 +1542,57 @@ bool ValidationState_t::IsTensorType(uint32_t id) const {
   return inst && inst->opcode() == spv::Op::OpTypeTensorARM;
 }
 
+// From the spec (SPIRV.html#PhysicalPointerType)
+bool ValidationState_t::IsPhysicalPointerType(uint32_t id) const {
+  const Instruction* inst = FindDef(id);
+  const spv::Op opcode = inst->opcode();
+  if (opcode != spv::Op::OpTypePointer &&
+      opcode != spv::Op::OpTypeUntypedPointerKHR) {
+    return false;
+  }
+
+  const spv::AddressingModel am = addressing_model();
+  if (am == spv::AddressingModel::Logical) {
+    return false;
+  } else if (am == spv::AddressingModel::Physical32 ||
+             am == spv::AddressingModel::Physical64) {
+    return true;
+  } else if (am == spv::AddressingModel::PhysicalStorageBuffer64) {
+    const spv::StorageClass storage_class = spv::StorageClass(inst->word(2));
+    return storage_class == spv::StorageClass::PhysicalStorageBuffer;
+  }
+
+  assert(0);
+  return false;
+}
+
+// From the spec (SPIRV.html#Numerical)
+bool ValidationState_t::IsNumericalType(uint32_t id) const {
+  const Instruction* inst = FindDef(id);
+  const spv::Op opcode = inst->opcode();
+  return opcode == spv::Op::OpTypeInt || opcode == spv::Op::OpTypeFloat;
+}
+
+// From the spec (SPIRV.html#Concrete)
+bool ValidationState_t::IsConcreteType(uint32_t id) const {
+  const Instruction* inst = FindDef(id);
+  const spv::Op opcode = inst->opcode();
+
+  if (opcode == spv::Op::OpTypeStruct) {
+    // all elements must be concrete
+    for (uint32_t i = 1; i < inst->operands().size(); ++i) {
+      if (!IsConcreteType(inst->GetOperandAs<uint32_t>(i))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  const uint32_t component_type = GetComponentType(id);
+  return IsNumericalType(component_type) ||
+         IsPhysicalPointerType(component_type);
+}
+
 spv_result_t ValidationState_t::CooperativeMatrixShapesMatch(
     const Instruction* inst, uint32_t result_type_id, uint32_t m2,
     bool is_conversion, bool swap_row_col) {

source/val/validation_state.h

@@ -708,6 +708,9 @@ class ValidationState_t {
   bool IsIntCooperativeVectorNVType(uint32_t id) const;
   bool IsUnsignedIntCooperativeVectorNVType(uint32_t id) const;
   bool IsTensorType(uint32_t id) const;
+  bool IsPhysicalPointerType(uint32_t id) const;
+  bool IsNumericalType(uint32_t id) const;
+  bool IsConcreteType(uint32_t id) const;
   // When |length| is not 0, return true only if the array length is equal to
   // |length| and the array length is not defined by a specialization constant.
   bool IsArrayType(uint32_t id, uint64_t length = 0) const;

test/val/val_misc_test.cpp

@@ -105,6 +105,56 @@ TEST_F(ValidateMisc, SizeOfValid) {
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
 }
 
+TEST_F(ValidateMisc, SizeOfStructValid) {
+  const std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %f "f"
+       %void = OpTypeVoid
+        %i32 = OpTypeInt 32 0
+        %ptr = OpTypePointer CrossWorkgroup %i32
+   %struct_a = OpTypeStruct %ptr %i32
+   %struct_b = OpTypeStruct %struct_a %ptr
+       %fnTy = OpTypeFunction %void
+          %f = OpFunction %void None %fnTy
+      %entry = OpLabel
+          %s = OpSizeOf %i32 %struct_b
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
+}
+
+TEST_F(ValidateMisc, SizeOfStructWithAbstract) {
+  const std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %f "f"
+       %void = OpTypeVoid
+        %i32 = OpTypeInt 32 0
+       %bool = OpTypeBool
+        %ptr = OpTypePointer CrossWorkgroup %i32
+   %struct_a = OpTypeStruct %ptr %bool
+   %struct_b = OpTypeStruct %struct_a %ptr
+       %fnTy = OpTypeFunction %void
+          %f = OpFunction %void None %fnTy
+      %entry = OpLabel
+          %s = OpSizeOf %i32 %struct_b
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_1);
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA,
+            ValidateInstructions(SPV_ENV_UNIVERSAL_1_1));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpSizeOf Pointer operand is not concrete"));
+}
+
 TEST_F(ValidateMisc, SizeOfFloat) {
   const std::string spirv = R"(
                OpCapability Addresses