Add validation for structs decorated as Block or BufferBlock.

Fixes #937

Stop std140/430 validation when runtime array is encountered.

Check for standard uniform/storage buffer layout instead of std140/430.

Added validator command line switch to skip block layout checking.

Validate structs decorated as Block/BufferBlock only when they
are used as variable with storage class of uniform or push
constant.

Expose --relax-block-layout to command line.

dneto0 modification:
- Use integer arithmetic instead of floor.

include/spirv-tools/libspirv.h

@@ -473,6 +473,12 @@ SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetRelaxStoreStruct(
 SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetRelaxLogicalPointer(
     spv_validator_options options, bool val);
 
+// Records whether or not the validator should relax the rules on block layout.
+//
+// When relaxed, it will skip checking standard uniform/storage buffer layout.
+SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetRelaxBlockLayout(
+    spv_validator_options options, bool val);
+
 // Encodes the given SPIR-V assembly text to its binary representation. The
 // length parameter specifies the number of bytes for text. Encoded binary will
 // be stored into *binary. Any error will be written into *diagnostic if

include/spirv-tools/libspirv.hpp

@@ -81,6 +81,10 @@ class ValidatorOptions {
     spvValidatorOptionsSetRelaxStoreStruct(options_, val);
   }
 
+  void SetRelaxBlockLayout(bool val) {
+    spvValidatorOptionsSetRelaxBlockLayout(options_, val);
+  }
+
   // Records whether or not the validator should relax the rules on pointer
   // usage in logical addressing mode.
   //

source/spirv_validator_options.cpp

@@ -84,5 +84,10 @@ void spvValidatorOptionsSetRelaxStoreStruct(spv_validator_options options,
 
 void spvValidatorOptionsSetRelaxLogicalPointer(spv_validator_options options,
                                                bool val) {
-  options->relax_logcial_pointer = val;
+  options->relax_logical_pointer = val;
+}
+
+void spvValidatorOptionsSetRelaxBlockLayout(spv_validator_options options,
+                                            bool val) {
+  options->relax_block_layout = val;
 }

source/spirv_validator_options.h

@@ -40,11 +40,13 @@ struct spv_validator_options_t {
   spv_validator_options_t()
       : universal_limits_(),
         relax_struct_store(false),
-        relax_logcial_pointer(false) {}
+        relax_logical_pointer(false),
+        relax_block_layout(false) {}
 
   validator_universal_limits_t universal_limits_;
   bool relax_struct_store;
-  bool relax_logcial_pointer;
+  bool relax_logical_pointer;
+  bool relax_block_layout;
 };
 
 #endif  // LIBSPIRV_SPIRV_VALIDATOR_OPTIONS_H_

source/validate_decorations.cpp

@@ -20,6 +20,7 @@
 #include "diagnostic.h"
 #include "opcode.h"
 #include "spirv_target_env.h"
+#include "spirv_validator_options.h"
 #include "val/validation_state.h"
 
 using libspirv::Decoration;
@@ -59,6 +60,296 @@ bool hasImportLinkageAttribute(uint32_t id, ValidationState_t& vstate) {
                      });
 }
 
+// Returns a vector of all members of a structure.
+std::vector<uint32_t> getStructMembers(uint32_t struct_id,
+                                       ValidationState_t& vstate) {
+  const auto inst = vstate.FindDef(struct_id);
+  return std::vector<uint32_t>(inst->words().begin() + 2, inst->words().end());
+}
+
+// Returns a vector of all members of a structure that have specific type.
+std::vector<uint32_t> getStructMembers(uint32_t struct_id, SpvOp type,
+                                       ValidationState_t& vstate) {
+  std::vector<uint32_t> members;
+  for (auto id : getStructMembers(struct_id, vstate)) {
+    if (type == vstate.FindDef(id)->opcode()) {
+      members.push_back(id);
+    }
+  }
+  return members;
+}
+
+// Returns whether the given structure is missing Offset decoration for any
+// member. Handles also nested structures.
+bool isMissingOffsetInStruct(uint32_t struct_id, ValidationState_t& vstate) {
+  std::vector<bool> hasOffset(getStructMembers(struct_id, vstate).size(),
+                              false);
+  // Check offsets of member decorations
+  for (auto& decoration : vstate.id_decorations(struct_id)) {
+    if (SpvDecorationOffset == decoration.dec_type() &&
+        Decoration::kInvalidMember != decoration.struct_member_index()) {
+      hasOffset[decoration.struct_member_index()] = true;
+    }
+  }
+  // Check also nested structures
+  bool nestedStructsMissingOffset = false;
+  for (auto id : getStructMembers(struct_id, SpvOpTypeStruct, vstate)) {
+    if (isMissingOffsetInStruct(id, vstate)) {
+      nestedStructsMissingOffset = true;
+      break;
+    }
+  }
+  return nestedStructsMissingOffset ||
+         !std::all_of(hasOffset.begin(), hasOffset.end(),
+                      [](const bool b) { return b; });
+}
+
+// Rounds x up to the next alignment. Assumes alignment is a power of two.
+uint32_t align(uint32_t x, uint32_t alignment) {
+  return (x + alignment - 1) & ~(alignment - 1);
+}
+
+// Returns base alignment of struct member.
+uint32_t getBaseAlignment(uint32_t member_id, bool roundUp,
+                          ValidationState_t& vstate) {
+  const auto inst = vstate.FindDef(member_id);
+  const auto words = inst->words();
+  uint32_t baseAlignment = 0;
+  switch (inst->opcode()) {
+    case SpvOpTypeInt:
+    case SpvOpTypeFloat:
+      baseAlignment = words[2] / 8;
+      break;
+    case SpvOpTypeVector: {
+      const auto componentId = words[2];
+      const auto numComponents = words[3];
+      const auto componentAlignment =
+          getBaseAlignment(componentId, roundUp, vstate);
+      baseAlignment =
+          componentAlignment * (numComponents == 3 ? 4 : numComponents);
+      break;
+    }
+    case SpvOpTypeMatrix:
+    case SpvOpTypeArray:
+    case SpvOpTypeRuntimeArray:
+      baseAlignment = getBaseAlignment(words[2], roundUp, vstate);
+      if (roundUp) baseAlignment = align(baseAlignment, 16u);
+      break;
+    case SpvOpTypeStruct:
+      for (auto id : getStructMembers(member_id, vstate)) {
+        baseAlignment =
+            std::max(baseAlignment, getBaseAlignment(id, roundUp, vstate));
+      }
+      if (roundUp) baseAlignment = align(baseAlignment, 16u);
+      break;
+    default:
+      assert(0);
+      break;
+  }
+
+  return baseAlignment;
+}
+
+// Returns size of a struct member. Doesn't include padding at the end of struct
+// or array.
+uint32_t getSize(uint32_t member_id, bool roundUp, ValidationState_t& vstate) {
+  const auto inst = vstate.FindDef(member_id);
+  const auto words = inst->words();
+  const auto baseAlignment = getBaseAlignment(member_id, roundUp, vstate);
+  switch (inst->opcode()) {
+    case SpvOpTypeInt:
+    case SpvOpTypeFloat:
+      return baseAlignment;
+    case SpvOpTypeVector: {
+      const auto componentId = words[2];
+      const auto numComponents = words[3];
+      const auto componentSize = getSize(componentId, roundUp, vstate);
+      return componentSize * numComponents;
+    }
+    case SpvOpTypeArray: {
+      const auto sizeInst = vstate.FindDef(words[3]);
+      if (spvOpcodeIsSpecConstant(sizeInst->opcode())) return 0;
+      assert(SpvOpConstant == sizeInst->opcode());
+      return (sizeInst->words()[3] - 1) * baseAlignment +
+             getSize(vstate.FindDef(member_id)->words()[2], roundUp, vstate);
+    }
+    case SpvOpTypeRuntimeArray:
+      return 0;
+    case SpvOpTypeMatrix:
+      return words[3] * baseAlignment;
+    case SpvOpTypeStruct: {
+      const auto members = getStructMembers(member_id, vstate);
+      if (members.empty()) return 0;
+      const auto lastIdx = members.size() - 1;
+      const auto& lastMember = members.back();
+      uint32_t offset = 0xffffffff;
+      // Find the offset of the last element and add the size.
+      for (auto& decoration : vstate.id_decorations(member_id)) {
+        if (SpvDecorationOffset == decoration.dec_type() &&
+            decoration.struct_member_index() == (int)lastIdx) {
+          offset = decoration.params()[0];
+        }
+      }
+      assert(offset != 0xffffffff);
+      return offset + getSize(lastMember, roundUp, vstate);
+    }
+    default:
+      assert(0);
+      return 0;
+  }
+}
+
+// A member is defined to improperly straddle if either of the following are
+// true:
+// - It is a vector with total size less than or equal to 16 bytes, and has
+// Offset decorations placing its first byte at F and its last byte at L, where
+// floor(F / 16) != floor(L / 16).
+// - It is a vector with total size greater than 16 bytes and has its Offset
+// decorations placing its first byte at a non-integer multiple of 16.
+bool hasImproperStraddle(uint32_t id, uint32_t offset,
+                         ValidationState_t& vstate) {
+  const auto inst = vstate.FindDef(id);
+  const auto words = inst->words();
+  const auto size = getSize(id, false, vstate);
+  const auto F = offset;
+  const auto L = offset + size - 1;
+  if (size <= 16) {
+    if ((F >> 4) != (L >> 4)) return true;
+  } else {
+    if (F % 16 != 0) return true;
+  }
+  return false;
+}
+
+// Check alignment of x. For storage buffers the alignment can be either rounded
+// up or not.
+bool checkAlignment(uint32_t x, uint32_t alignment, uint32_t alignmentRoundedUp,
+                    bool isBlock) {
+  if (isBlock) {
+    if (alignmentRoundedUp == 0) {
+      if (x != 0) return false;
+    } else if (x % alignmentRoundedUp != 0)
+      return false;
+  } else {
+    if (alignment == 0 || alignmentRoundedUp == 0) {
+      if (x != 0) return false;
+    } else if (x % alignment != 0 && x % alignmentRoundedUp != 0)
+      return false;
+  }
+  return true;
+}
+
+// Checks for standard layout rules.
+bool checkLayout(uint32_t struct_id, bool isBlock, ValidationState_t& vstate) {
+  if (vstate.options()->relax_block_layout) return true;
+  const auto members = getStructMembers(struct_id, vstate);
+  uint32_t padStart = 0, padEnd = 0;
+  for (size_t memberIdx = 0; memberIdx < members.size(); memberIdx++) {
+    auto id = members[memberIdx];
+    const auto baseAlignment = getBaseAlignment(id, false, vstate);
+    const auto baseAlignmentRoundedUp = getBaseAlignment(id, true, vstate);
+    const auto inst = vstate.FindDef(id);
+    const auto opcode = inst->opcode();
+    uint32_t offset = 0xffffffff;
+    for (auto& decoration : vstate.id_decorations(struct_id)) {
+      if (SpvDecorationOffset == decoration.dec_type() &&
+          decoration.struct_member_index() == (int)memberIdx) {
+        offset = decoration.params()[0];
+      }
+    }
+    const auto size = getSize(id, isBlock, vstate);
+    const auto lastByte = size - 1;
+    // Check offset.
+    if (offset == 0xffffffff) return false;
+    if (!checkAlignment(offset, baseAlignment, baseAlignmentRoundedUp, isBlock))
+      return false;
+    if (offset >= padStart && offset + lastByte <= padEnd) return false;
+    // Check improper straddle of vectors.
+    if (SpvOpTypeVector == opcode && hasImproperStraddle(id, offset, vstate))
+      return false;
+    // Check struct members recursively.
+    if (SpvOpTypeStruct == opcode && !checkLayout(id, isBlock, vstate))
+      return false;
+    // Check matrix stride.
+    if (SpvOpTypeMatrix == opcode) {
+      for (auto& decoration : vstate.id_decorations(id)) {
+        if (SpvDecorationMatrixStride == decoration.dec_type() &&
+            !checkAlignment(decoration.params()[0], baseAlignment,
+                            baseAlignmentRoundedUp, isBlock))
+          return false;
+      }
+    }
+    // Check arrays.
+    if (SpvOpTypeArray == opcode) {
+      const auto typeId = inst->words()[2];
+      const auto arrayInst = vstate.FindDef(typeId);
+      if (SpvOpTypeStruct == arrayInst->opcode() &&
+          !checkLayout(typeId, isBlock, vstate))
+        return false;
+      // Check array stride.
+      for (auto& decoration : vstate.id_decorations(id)) {
+        if (SpvDecorationArrayStride == decoration.dec_type() &&
+            !checkAlignment(decoration.params()[0], baseAlignment,
+                            baseAlignmentRoundedUp, isBlock))
+          return false;
+      }
+      if ((SpvOpTypeArray == opcode || SpvOpTypeStruct == opcode) &&
+          size != 0) {
+        const auto alignment = isBlock ? baseAlignmentRoundedUp : baseAlignment;
+        padStart = lastByte + 1;
+        padEnd = align(lastByte, alignment);
+      } else {
+        padStart = padEnd = 0;
+      }
+    }
+  }
+  return true;
+}
+
+// Returns true if structure id has given decoration. Handles also nested
+// structures.
+bool hasDecoration(uint32_t struct_id, SpvDecoration decoration,
+                   ValidationState_t& vstate) {
+  for (auto& dec : vstate.id_decorations(struct_id)) {
+    if (decoration == dec.dec_type()) return true;
+  }
+  for (auto id : getStructMembers(struct_id, SpvOpTypeStruct, vstate)) {
+    if (hasDecoration(id, decoration, vstate)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+// Returns true if all ids of given type have a specified decoration.
+bool checkForRequiredDecoration(uint32_t struct_id, SpvDecoration decoration,
+                                SpvOp type, ValidationState_t& vstate) {
+  const auto members = getStructMembers(struct_id, vstate);
+  for (size_t memberIdx = 0; memberIdx < members.size(); memberIdx++) {
+    const auto id = members[memberIdx];
+    if (type != vstate.FindDef(id)->opcode()) continue;
+    bool found = false;
+    for (auto& dec : vstate.id_decorations(id)) {
+      if (decoration == dec.dec_type()) found = true;
+    }
+    for (auto& dec : vstate.id_decorations(struct_id)) {
+      if (decoration == dec.dec_type() &&
+          (int)memberIdx == dec.struct_member_index()) {
+        found = true;
+      }
+    }
+    if (!found) {
+      return false;
+    }
+  }
+  for (auto id : getStructMembers(struct_id, SpvOpTypeStruct, vstate)) {
+    if (!checkForRequiredDecoration(id, decoration, type, vstate)) {
+      return false;
+    }
+  }
+  return true;
+}
+
 spv_result_t CheckLinkageAttrOfFunctions(ValidationState_t& vstate) {
   for (const auto& function : vstate.functions()) {
     if (function.block_count() == 0u) {
@@ -225,6 +516,62 @@ spv_result_t CheckDescriptorSetArrayOfArrays(ValidationState_t& vstate) {
   return SPV_SUCCESS;
 }
 
+spv_result_t CheckDecorationsOfBuffers(ValidationState_t& vstate) {
+  for (const auto& def : vstate.all_definitions()) {
+    const auto inst = def.second;
+    const auto words = inst->words();
+    if (SpvOpVariable == inst->opcode() &&
+        (SpvStorageClassUniform == words[3] ||
+         SpvStorageClassPushConstant == words[3])) {
+      const auto ptrInst = vstate.FindDef(words[1]);
+      assert(SpvOpTypePointer == ptrInst->opcode());
+      const auto id = ptrInst->words()[3];
+      if (SpvOpTypeStruct != vstate.FindDef(id)->opcode()) continue;
+      for (const auto& dec : vstate.id_decorations(id)) {
+        const bool isBlock = SpvDecorationBlock == dec.dec_type();
+        const bool isBufferBlock = SpvDecorationBufferBlock == dec.dec_type();
+        if (isBlock || isBufferBlock) {
+          std::string dec_str = isBlock ? "Block" : "BufferBlock";
+          if (isMissingOffsetInStruct(id, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as " << dec_str
+                   << " must be explicitly laid out with Offset decorations.";
+          } else if (hasDecoration(id, SpvDecorationGLSLShared, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as " << dec_str
+                   << " must not use GLSLShared decoration.";
+          } else if (hasDecoration(id, SpvDecorationGLSLPacked, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as " << dec_str
+                   << " must not use GLSLPacked decoration.";
+          } else if (!checkForRequiredDecoration(id, SpvDecorationArrayStride,
+                                                 SpvOpTypeArray, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as " << dec_str
+                   << " must be explicitly laid out with ArrayStride "
+                      "decorations.";
+          } else if (!checkForRequiredDecoration(id, SpvDecorationMatrixStride,
+                                                 SpvOpTypeMatrix, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as " << dec_str
+                   << " must be explicitly laid out with MatrixStride "
+                      "decorations.";
+          } else if (isBlock && !checkLayout(id, true, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as Block"
+                   << " must follow standard uniform buffer layout rules.";
+          } else if (isBufferBlock && !checkLayout(id, false, vstate)) {
+            return vstate.diag(SPV_ERROR_INVALID_ID)
+                   << "Structure id " << id << " decorated as BufferBlock"
+                   << " must follow standard storage buffer layout rules.";
+          }
+        }
+      }
+    }
+  }
+  return SPV_SUCCESS;
+}
+
 }  // anonymous namespace
 
 namespace libspirv {
@@ -233,6 +580,7 @@ namespace libspirv {
 spv_result_t ValidateDecorations(ValidationState_t& vstate) {
   if (auto error = CheckImportedVariableInitialization(vstate)) return error;
   if (auto error = CheckDecorationsOfEntryPoints(vstate)) return error;
+  if (auto error = CheckDecorationsOfBuffers(vstate)) return error;
   if (auto error = CheckLinkageAttrOfFunctions(vstate)) return error;
   if (auto error = CheckDescriptorSetArrayOfArrays(vstate)) return error;
   return SPV_SUCCESS;

source/validate_id.cpp

@@ -1996,7 +1996,7 @@ bool idUsage::isValid<SpvOpReturnValue>(const spv_instruction_t* inst,
 
   if (addressingModel == SpvAddressingModelLogical &&
       SpvOpTypePointer == valueType->opcode() && !uses_variable_pointer &&
-      !module_.options()->relax_logcial_pointer) {
+      !module_.options()->relax_logical_pointer) {
     DIAG(value)
         << "OpReturnValue value's type <id> '"
         << module_.getIdName(value->type_id())

tools/val/val.cpp

@@ -44,8 +44,9 @@ Options:
   --max-function-args              <maximum number arguments allowed per function>
   --max-control-flow-nesting-depth <maximum Control Flow nesting depth allowed>
   --max-access-chain-indexes       <maximum number of indexes allowed to use for Access Chain instructions>
-  --relax-logcial-pointer          Allow allocating an object of a pointer type and returning
+  --relax-logical-pointer          Allow allocating an object of a pointer type and returning
                                    a pointer value from a function in logical addressing mode
+  --relax-block-layout             Skips checking of standard uniform/storage buffer layout
   --relax-struct-store             Allow store from one struct type to a
                                    different type with compatible layout and
                                    members.
@@ -121,6 +122,8 @@ int main(int argc, char** argv) {
         }
       } else if (0 == strcmp(cur_arg, "--relax-logical-pointer")) {
         options.SetRelaxLogicalPointer(true);
+      } else if (0 == strcmp(cur_arg, "--relax-block-layout")) {
+        options.SetRelaxBlockLayout(true);
       } else if (0 == strcmp(cur_arg, "--relax-struct-store")) {
         options.SetRelaxStructStore(true);
       } else if (0 == cur_arg[1]) {