1 files changed, 241 insertions, 21 deletions

diff --git a/source/val/validate_annotation.cpp b/source/val/validate_annotation.cpp
index 85d2b751..3a77552e 100644
--- a/source/val/validate_annotation.cpp
+++ b/source/val/validate_annotation.cpp
@@ -138,14 +138,14 @@ std::string LogStringForDecoration(uint32_t decoration) {
       return "PerTaskNV";
     case SpvDecorationPerVertexNV:
       return "PerVertexNV";
-    case SpvDecorationNonUniformEXT:
-      return "NonUniformEXT";
-    case SpvDecorationRestrictPointerEXT:
-      return "RestrictPointerEXT";
-    case SpvDecorationAliasedPointerEXT:
-      return "AliasedPointerEXT";
-    case SpvDecorationHlslCounterBufferGOOGLE:
-      return "HlslCounterBufferGOOGLE";
+    case SpvDecorationNonUniform:
+      return "NonUniform";
+    case SpvDecorationRestrictPointer:
+      return "RestrictPointer";
+    case SpvDecorationAliasedPointer:
+      return "AliasedPointer";
+    case SpvDecorationCounterBuffer:
+      return "CounterBuffer";
     case SpvDecorationHlslSemanticGOOGLE:
       return "HlslSemanticGOOGLE";
     default:
@@ -156,8 +156,8 @@ std::string LogStringForDecoration(uint32_t decoration) {
 
 // Returns true if the decoration takes ID parameters.
 // TODO(dneto): This can be generated from the grammar.
-bool DecorationTakesIdParameters(uint32_t type) {
-  switch (static_cast<SpvDecoration>(type)) {
+bool DecorationTakesIdParameters(SpvDecoration type) {
+  switch (type) {
     case SpvDecorationUniformId:
     case SpvDecorationAlignmentId:
     case SpvDecorationMaxByteOffsetId:
@@ -169,17 +169,213 @@ bool DecorationTakesIdParameters(uint32_t type) {
   return false;
 }
 
-spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) {
-  const auto decoration = inst->GetOperandAs<uint32_t>(1);
-  if (decoration == SpvDecorationSpecId) {
-    const auto target_id = inst->GetOperandAs<uint32_t>(0);
-    const auto target = _.FindDef(target_id);
-    if (!target || !spvOpcodeIsScalarSpecConstant(target->opcode())) {
-      return _.diag(SPV_ERROR_INVALID_ID, inst)
-             << "OpDecorate SpecId decoration target <id> '"
-             << _.getIdName(target_id)
-             << "' is not a scalar specialization constant.";
+bool IsMemberDecorationOnly(SpvDecoration dec) {
+  switch (dec) {
+    case SpvDecorationRowMajor:
+    case SpvDecorationColMajor:
+    case SpvDecorationMatrixStride:
+      // SPIR-V spec bug? Offset is generated on variables when dealing with
+      // transform feedback.
+      // case SpvDecorationOffset:
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
+bool IsNotMemberDecoration(SpvDecoration dec) {
+  switch (dec) {
+    case SpvDecorationSpecId:
+    case SpvDecorationBlock:
+    case SpvDecorationBufferBlock:
+    case SpvDecorationArrayStride:
+    case SpvDecorationGLSLShared:
+    case SpvDecorationGLSLPacked:
+    case SpvDecorationCPacked:
+    // TODO: https://github.com/KhronosGroup/glslang/issues/703:
+    // glslang applies Restrict to structure members.
+    // case SpvDecorationRestrict:
+    case SpvDecorationAliased:
+    case SpvDecorationConstant:
+    case SpvDecorationUniform:
+    case SpvDecorationUniformId:
+    case SpvDecorationSaturatedConversion:
+    case SpvDecorationIndex:
+    case SpvDecorationBinding:
+    case SpvDecorationDescriptorSet:
+    case SpvDecorationFuncParamAttr:
+    case SpvDecorationFPRoundingMode:
+    case SpvDecorationFPFastMathMode:
+    case SpvDecorationLinkageAttributes:
+    case SpvDecorationNoContraction:
+    case SpvDecorationInputAttachmentIndex:
+    case SpvDecorationAlignment:
+    case SpvDecorationMaxByteOffset:
+    case SpvDecorationAlignmentId:
+    case SpvDecorationMaxByteOffsetId:
+    case SpvDecorationNoSignedWrap:
+    case SpvDecorationNoUnsignedWrap:
+    case SpvDecorationNonUniform:
+    case SpvDecorationRestrictPointer:
+    case SpvDecorationAliasedPointer:
+    case SpvDecorationCounterBuffer:
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
+spv_result_t ValidateDecorationTarget(ValidationState_t& _, SpvDecoration dec,
+                                      const Instruction* inst,
+                                      const Instruction* target) {
+  auto fail = [&_, dec, inst, target](uint32_t vuid = 0) -> DiagnosticStream {
+    DiagnosticStream ds = std::move(
+        _.diag(SPV_ERROR_INVALID_ID, inst)
+        << _.VkErrorID(vuid) << LogStringForDecoration(dec)
+        << " decoration on target <id> '" << _.getIdName(target->id()) << "' ");
+    return ds;
+  };
+  switch (dec) {
+    case SpvDecorationSpecId:
+      if (!spvOpcodeIsScalarSpecConstant(target->opcode())) {
+        return fail() << "must be a scalar specialization constant";
+      }
+      break;
+    case SpvDecorationBlock:
+    case SpvDecorationBufferBlock:
+    case SpvDecorationGLSLShared:
+    case SpvDecorationGLSLPacked:
+    case SpvDecorationCPacked:
+      if (target->opcode() != SpvOpTypeStruct) {
+        return fail() << "must be a structure type";
+      }
+      break;
+    case SpvDecorationArrayStride:
+      if (target->opcode() != SpvOpTypeArray &&
+          target->opcode() != SpvOpTypeRuntimeArray &&
+          target->opcode() != SpvOpTypePointer) {
+        return fail() << "must be an array or pointer type";
+      }
+      break;
+    case SpvDecorationBuiltIn:
+      if (target->opcode() != SpvOpVariable &&
+          !spvOpcodeIsConstant(target->opcode())) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "BuiltIns can only target variables, structure members or "
+                  "constants";
+      }
+      if (_.HasCapability(SpvCapabilityShader) &&
+          inst->GetOperandAs<SpvBuiltIn>(2) == SpvBuiltInWorkgroupSize) {
+        if (!spvOpcodeIsConstant(target->opcode())) {
+          return fail() << "must be a constant for WorkgroupSize";
+        }
+      } else if (target->opcode() != SpvOpVariable) {
+        return fail() << "must be a variable";
+      }
+      break;
+    case SpvDecorationNoPerspective:
+    case SpvDecorationFlat:
+    case SpvDecorationPatch:
+    case SpvDecorationCentroid:
+    case SpvDecorationSample:
+    case SpvDecorationRestrict:
+    case SpvDecorationAliased:
+    case SpvDecorationVolatile:
+    case SpvDecorationCoherent:
+    case SpvDecorationNonWritable:
+    case SpvDecorationNonReadable:
+    case SpvDecorationXfbBuffer:
+    case SpvDecorationXfbStride:
+    case SpvDecorationComponent:
+    case SpvDecorationStream:
+    case SpvDecorationRestrictPointer:
+    case SpvDecorationAliasedPointer:
+      if (target->opcode() != SpvOpVariable &&
+          target->opcode() != SpvOpFunctionParameter) {
+        return fail() << "must be a memory object declaration";
+      }
+      if (_.GetIdOpcode(target->type_id()) != SpvOpTypePointer) {
+        return fail() << "must be a pointer type";
+      }
+      break;
+    case SpvDecorationInvariant:
+    case SpvDecorationConstant:
+    case SpvDecorationLocation:
+    case SpvDecorationIndex:
+    case SpvDecorationBinding:
+    case SpvDecorationDescriptorSet:
+    case SpvDecorationInputAttachmentIndex:
+      if (target->opcode() != SpvOpVariable) {
+        return fail() << "must be a variable";
+      }
+      break;
+    default:
+      break;
+  }
+
+  if (spvIsVulkanEnv(_.context()->target_env)) {
+    // The following were all checked as pointer types above.
+    SpvStorageClass sc = SpvStorageClassUniform;
+    const auto type = _.FindDef(target->type_id());
+    if (type && type->operands().size() > 2) {
+      sc = type->GetOperandAs<SpvStorageClass>(1);
     }
+    switch (dec) {
+      case SpvDecorationLocation:
+      case SpvDecorationComponent:
+        // Location is used for input, output and ray tracing stages.
+        if (sc == SpvStorageClassStorageBuffer ||
+            sc == SpvStorageClassUniform ||
+            sc == SpvStorageClassUniformConstant ||
+            sc == SpvStorageClassWorkgroup || sc == SpvStorageClassPrivate ||
+            sc == SpvStorageClassFunction) {
+          return _.diag(SPV_ERROR_INVALID_ID, target)
+                 << LogStringForDecoration(dec)
+                 << " decoration must not be applied to this storage class";
+        }
+        break;
+      case SpvDecorationIndex:
+        if (sc != SpvStorageClassOutput) {
+          return fail() << "must be in the Output storage class";
+        }
+        break;
+      case SpvDecorationBinding:
+      case SpvDecorationDescriptorSet:
+        if (sc != SpvStorageClassStorageBuffer &&
+            sc != SpvStorageClassUniform &&
+            sc != SpvStorageClassUniformConstant) {
+          return fail() << "must be in the StorageBuffer, Uniform, or "
+                           "UniformConstant storage class";
+        }
+        break;
+      case SpvDecorationInputAttachmentIndex:
+        if (sc != SpvStorageClassUniformConstant) {
+          return fail() << "must be in the UniformConstant storage class";
+        }
+        break;
+      case SpvDecorationFlat:
+      case SpvDecorationNoPerspective:
+      case SpvDecorationCentroid:
+      case SpvDecorationSample:
+        if (sc != SpvStorageClassInput && sc != SpvStorageClassOutput) {
+          return fail(4670) << "storage class must be Input or Output";
+        }
+        break;
+      default:
+        break;
+    }
+  }
+  return SPV_SUCCESS;
+}
+
+spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) {
+  const auto decoration = inst->GetOperandAs<SpvDecoration>(1);
+  const auto target_id = inst->GetOperandAs<uint32_t>(0);
+  const auto target = _.FindDef(target_id);
+  if (!target) {
+    return _.diag(SPV_ERROR_INVALID_ID, inst) << "target is not defined";
   }
 
   if (spvIsVulkanEnv(_.context()->target_env)) {
@@ -197,17 +393,34 @@ spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) {
            << "Decorations taking ID parameters may not be used with "
               "OpDecorateId";
   }
+
+  if (target->opcode() != SpvOpDecorationGroup) {
+    if (IsMemberDecorationOnly(decoration)) {
+      return _.diag(SPV_ERROR_INVALID_ID, inst)
+             << LogStringForDecoration(decoration)
+             << " can only be applied to structure members";
+    }
+
+    if (auto error = ValidateDecorationTarget(_, decoration, inst, target)) {
+      return error;
+    }
+  }
+
   // TODO: Add validations for all decorations.
   return SPV_SUCCESS;
 }
 
 spv_result_t ValidateDecorateId(ValidationState_t& _, const Instruction* inst) {
-  const auto decoration = inst->GetOperandAs<uint32_t>(1);
+  const auto decoration = inst->GetOperandAs<SpvDecoration>(1);
   if (!DecorationTakesIdParameters(decoration)) {
     return _.diag(SPV_ERROR_INVALID_ID, inst)
            << "Decorations that don't take ID parameters may not be used with "
               "OpDecorateId";
   }
+
+  // No member decorations take id parameters, so we don't bother checking if
+  // we are using a member only decoration here.
+
   // TODO: Add validations for these decorations.
   // UniformId is covered elsewhere.
   return SPV_SUCCESS;
@@ -234,6 +447,13 @@ spv_result_t ValidateMemberDecorate(ValidationState_t& _,
            << " members. Largest valid index is " << member_count - 1 << ".";
   }
 
+  const auto decoration = inst->GetOperandAs<SpvDecoration>(2);
+  if (IsNotMemberDecoration(decoration)) {
+    return _.diag(SPV_ERROR_INVALID_ID, inst)
+           << LogStringForDecoration(decoration)
+           << " cannot be applied to structure members";
+  }
+
   return SPV_SUCCESS;
 }