More layout check fixes (#2315)
* check array strides for multidimensional arrays
* check layouts of structs in arrays for multiple indices
* new tests
diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
index 6dd775d..164a5dd 100644
--- a/source/val/validate_decorations.cpp
+++ b/source/val/validate_decorations.cpp
@@ -509,23 +509,53 @@
<< " not satisfying alignment to " << alignment;
}
}
- // Check arrays and runtime arrays.
- if (SpvOpTypeArray == opcode || SpvOpTypeRuntimeArray == opcode) {
- const auto typeId = inst->word(2);
- const auto arrayInst = vstate.FindDef(typeId);
- if (SpvOpTypeStruct == arrayInst->opcode() &&
- SPV_SUCCESS != (recursive_status = checkLayout(
- typeId, storage_class_str, decoration_str,
- blockRules, offset, constraints, vstate)))
- return recursive_status;
+
+ // Check arrays and runtime arrays recursively.
+ auto array_inst = inst;
+ auto array_alignment = alignment;
+ while (array_inst->opcode() == SpvOpTypeArray ||
+ array_inst->opcode() == SpvOpTypeRuntimeArray) {
+ const auto typeId = array_inst->word(2);
+ const auto element_inst = vstate.FindDef(typeId);
// Check array stride.
- for (auto& decoration : vstate.id_decorations(id)) {
- if (SpvDecorationArrayStride == decoration.dec_type() &&
- !IsAlignedTo(decoration.params()[0], alignment))
- return fail(memberIdx)
- << "is an array with stride " << decoration.params()[0]
- << " not satisfying alignment to " << alignment;
+ auto array_stride = 0;
+ for (auto& decoration : vstate.id_decorations(array_inst->id())) {
+ if (SpvDecorationArrayStride == decoration.dec_type()) {
+ array_stride = decoration.params()[0];
+ if (!IsAlignedTo(array_stride, array_alignment))
+ return fail(memberIdx)
+ << "contains an array with stride " << decoration.params()[0]
+ << " not satisfying alignment to " << alignment;
+ }
}
+
+ bool is_int32 = false;
+ bool is_const = false;
+ uint32_t num_elements = 0;
+ if (array_inst->opcode() == SpvOpTypeArray) {
+ std::tie(is_int32, is_const, num_elements) =
+ vstate.EvalInt32IfConst(array_inst->word(3));
+ }
+ num_elements = std::max(1u, num_elements);
+ // Check each element recursively if it is a struct. There is a
+ // limitation to this check if the array size is a spec constant or is a
+ // runtime array then we will only check a single element. This means
+ // some improper straddles might be missed.
+ for (uint32_t i = 0; i < num_elements; ++i) {
+ uint32_t next_offset = i * array_stride + offset;
+ if (SpvOpTypeStruct == element_inst->opcode() &&
+ SPV_SUCCESS != (recursive_status = checkLayout(
+ typeId, storage_class_str, decoration_str,
+ blockRules, next_offset, constraints, vstate)))
+ return recursive_status;
+ }
+
+ // Proceed to the element in case it is an array.
+ array_inst = element_inst;
+ array_alignment = scalar_block_layout
+ ? getScalarAlignment(array_inst->id(), vstate)
+ : getBaseAlignment(array_inst->id(), blockRules,
+ constraint, constraints, vstate);
}
nextValidOffset = offset + size;
if (!scalar_block_layout && blockRules &&
diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index 4c7a80e..1506a29 100644
--- a/test/val/val_decoration_test.cpp
+++ b/test/val/val_decoration_test.cpp
@@ -3418,7 +3418,8 @@
getDiagnosticString(),
HasSubstr(
"Structure id 6 decorated as Block for variable in Uniform storage "
- "class must follow standard uniform buffer layout rules: member 4 is "
+ "class must follow standard uniform buffer layout rules: member 4 "
+ "contains "
"an array with stride 49 not satisfying alignment to 16"));
}
@@ -5413,6 +5414,83 @@
"not aligned to 8"));
}
+TEST_F(ValidateDecorations, MultiDimensionalArray) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %struct Block
+OpMemberDecorate %struct 0 Offset 0
+OpDecorate %array_4 ArrayStride 4
+OpDecorate %array_3 ArrayStride 48
+OpDecorate %var DescriptorSet 0
+OpDecorate %var Binding 0
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_3 = OpConstant %int 3
+%int_4 = OpConstant %int 4
+%array_4 = OpTypeArray %int %int_4
+%array_3 = OpTypeArray %array_4 %int_3
+%struct = OpTypeStruct %array_3
+%ptr_struct = OpTypePointer Uniform %struct
+%var = OpVariable %ptr_struct Uniform
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_0);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Structure id 2 decorated as Block for variable in "
+ "Uniform storage class must follow standard uniform "
+ "buffer layout rules: member 0 contains an array with "
+ "stride 4 not satisfying alignment to 16"));
+}
+
+TEST_F(ValidateDecorations, ImproperStraddleInArray) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %struct Block
+OpMemberDecorate %struct 0 Offset 0
+OpDecorate %array ArrayStride 24
+OpMemberDecorate %inner 0 Offset 0
+OpMemberDecorate %inner 1 Offset 4
+OpMemberDecorate %inner 2 Offset 12
+OpMemberDecorate %inner 3 Offset 16
+OpDecorate %var DescriptorSet 0
+OpDecorate %var Binding 0
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_2 = OpConstant %int 2
+%int2 = OpTypeVector %int 2
+%inner = OpTypeStruct %int %int2 %int %int
+%array = OpTypeArray %inner %int_2
+%struct = OpTypeStruct %array
+%ptr_struct = OpTypePointer StorageBuffer %struct
+%var = OpVariable %ptr_struct StorageBuffer
+%void_fn = OpTypeFunction %void
+%main = OpFunction %void None %void_fn
+%entry = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_1);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("Structure id 4 decorated as Block for variable in "
+ "StorageBuffer storage class must follow relaxed "
+ "storage buffer layout rules: member 1 is an "
+ "improperly straddling vector at offset 28"));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
