Validate array stride does not cause overlap (#3028)
Fixes #3027
* Disallow array stride 0
* Check array stride against element size
* Fix up tests
* Add new tests
diff --git a/source/val/validate_decorations.cpp b/source/val/validate_decorations.cpp
index 0a6da46..d513a25 100644
--- a/source/val/validate_decorations.cpp
+++ b/source/val/validate_decorations.cpp
@@ -522,10 +522,13 @@
const auto typeId = array_inst->word(2);
const auto element_inst = vstate.FindDef(typeId);
// Check array stride.
- auto array_stride = 0;
+ uint32_t array_stride = 0;
for (auto& decoration : vstate.id_decorations(array_inst->id())) {
if (SpvDecorationArrayStride == decoration.dec_type()) {
array_stride = decoration.params()[0];
+ if (array_stride == 0) {
+ return fail(memberIdx) << "contains an array with stride 0";
+ }
if (!IsAlignedTo(array_stride, array_alignment))
return fail(memberIdx)
<< "contains an array with stride " << decoration.params()[0]
@@ -563,6 +566,14 @@
? getScalarAlignment(array_inst->id(), vstate)
: getBaseAlignment(array_inst->id(), blockRules,
constraint, constraints, vstate);
+
+ const auto element_size =
+ getSize(element_inst->id(), constraint, constraints, vstate);
+ if (element_size > array_stride) {
+ return fail(memberIdx)
+ << "contains an array with stride " << array_stride
+ << ", but with an element size of " << element_size;
+ }
}
nextValidOffset = offset + size;
if (!scalar_block_layout && blockRules &&
diff --git a/test/opt/eliminate_dead_member_test.cpp b/test/opt/eliminate_dead_member_test.cpp
index 7d5db7d..b6925d7 100644
--- a/test/opt/eliminate_dead_member_test.cpp
+++ b/test/opt/eliminate_dead_member_test.cpp
@@ -561,6 +561,7 @@
OpName %main "main"
OpDecorate %_Globals DescriptorSet 0
OpDecorate %_Globals Binding 0
+ OpDecorate %_runtimearr_float ArrayStride 16
OpMemberDecorate %type__Globals 0 Offset 0
OpMemberDecorate %type__Globals 1 Offset 4
OpMemberDecorate %type__Globals 2 Offset 16
diff --git a/test/opt/graphics_robust_access_test.cpp b/test/opt/graphics_robust_access_test.cpp
index 4ae4121..58bd404 100644
--- a/test/opt/graphics_robust_access_test.cpp
+++ b/test/opt/graphics_robust_access_test.cpp
@@ -898,7 +898,7 @@
TEST_F(GraphicsRobustAccessTest, ACRTArrayLeastInboundClamped) {
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
- shaders << ShaderPreambleAC() << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 "
+ shaders << ShaderPreambleAC() << "OpDecorate %rtarr ArrayStride 4 "
<< DecoSSBO() << TypesVoid() << TypesInt() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %uint %uint %rtarr
@@ -924,9 +924,8 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << "OpCapability Int16\n"
- << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesShort() << TypesFloat() << R"(
+ << ShaderPreambleAC({"i"}) << "OpDecorate %rtarr ArrayStride 4 "
+ << DecoSSBO() << TypesVoid() << TypesShort() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %short %short %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -954,9 +953,8 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << "OpCapability Int16\n"
- << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesShort() << TypesFloat() << R"(
+ << ShaderPreambleAC({"i"}) << "OpDecorate %rtarr ArrayStride 4 "
+ << DecoSSBO() << TypesVoid() << TypesShort() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %short %short %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -983,9 +981,8 @@
TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralIntIndexClamped) {
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
- shaders << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesInt() << TypesFloat() << R"(
+ shaders << ShaderPreambleAC({"i"}) << "OpDecorate %rtarr ArrayStride 4 "
+ << DecoSSBO() << TypesVoid() << TypesInt() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %int %int %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -1000,8 +997,9 @@
; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
; CHECK: %[[max:\w+]] = OpISub %int %[[arrlen]] %int_1
; CHECK: %[[clamp:\w+]] = OpExtInst %int %[[GLSLSTD450]] UClamp %i %int_0 %[[max]]
- )" << MainPrefix()
- << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
+ )"
+ << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
+ << MainSuffix();
SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
}
}
@@ -1009,9 +1007,8 @@
TEST_F(GraphicsRobustAccessTest, ACRTArrayGeneralUIntIndexClamped) {
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
- shaders << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesInt() << TypesFloat() << R"(
+ shaders << ShaderPreambleAC({"i"}) << "OpDecorate %rtarr ArrayStride 4 "
+ << DecoSSBO() << TypesVoid() << TypesInt() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %int %int %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -1026,8 +1023,9 @@
; CHECK: %[[arrlen:\w+]] = OpArrayLength %uint %var 2
; CHECK: %[[max:\w+]] = OpISub %uint %[[arrlen]] %uint_1
; CHECK: %[[clamp:\w+]] = OpExtInst %uint %[[GLSLSTD450]] UClamp %i %uint_0 %[[max]]
- )" << MainPrefix()
- << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
+ )"
+ << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
+ << MainSuffix();
SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
}
}
@@ -1036,8 +1034,8 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << "OpCapability Int64" << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesInt() << TypesLong() << TypesFloat() << R"(
+ << "OpDecorate %rtarr ArrayStride 4 " << DecoSSBO() << TypesVoid()
+ << TypesInt() << TypesLong() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %int %int %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -1053,9 +1051,8 @@
; CHECK: %[[arrlen_ext:\w+]] = OpUConvert %ulong %[[arrlen]]
; CHECK: %[[max:\w+]] = OpISub %long %[[arrlen_ext]] %long_1
; CHECK: %[[clamp:\w+]] = OpExtInst %long %[[GLSLSTD450]] UClamp %i %long_0 %[[max]]
- )"
- << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
- << MainSuffix();
+ )" << MainPrefix()
+ << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
}
}
@@ -1064,8 +1061,8 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << "OpCapability Int64" << ShaderPreambleAC({"i"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 4 " << DecoSSBO()
- << TypesVoid() << TypesInt() << TypesLong() << TypesFloat() << R"(
+ << "OpDecorate %rtarr ArrayStride 4 " << DecoSSBO() << TypesVoid()
+ << TypesInt() << TypesLong() << TypesFloat() << R"(
%rtarr = OpTypeRuntimeArray %float
%ssbo_s = OpTypeStruct %int %int %rtarr
%var_ty = OpTypePointer Uniform %ssbo_s
@@ -1081,9 +1078,8 @@
; CHECK: %[[arrlen_ext:\w+]] = OpUConvert %ulong %[[arrlen]]
; CHECK: %[[max:\w+]] = OpISub %ulong %[[arrlen_ext]] %ulong_1
; CHECK: %[[clamp:\w+]] = OpExtInst %ulong %[[GLSLSTD450]] UClamp %i %ulong_0 %[[max]]
- )"
- << MainPrefix() << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]")
- << MainSuffix();
+ )" << MainPrefix()
+ << ACCheck(ac, "%int_2 %i", "%int_2 %[[clamp]]") << MainSuffix();
SinglePassRunAndMatch<GraphicsRobustAccessPass>(shaders.str(), true);
}
}
@@ -1095,7 +1091,7 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << ShaderPreambleAC({"i", "j"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 32\n"
+ << "OpDecorate %rtarr ArrayStride 32\n"
<< DecoSSBO() << "OpMemberDecorate %rtelem 0 Offset 0\n"
<< "OpMemberDecorate %rtelem 1 Offset 16\n"
<< TypesVoid() << TypesInt() << TypesFloat() << R"(
@@ -1131,7 +1127,7 @@
for (auto* ac : AccessChains()) {
std::ostringstream shaders;
shaders << ShaderPreambleAC({"i", "ssbo_s"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 32\n"
+ << "OpDecorate %rtarr ArrayStride 32\n"
<< DecoSSBO() << "OpMemberDecorate %rtelem 0 Offset 0\n"
<< "OpMemberDecorate %rtelem 1 Offset 16\n"
<< TypesVoid() << TypesInt() << TypesFloat() << R"(
@@ -1175,7 +1171,7 @@
std::ostringstream shaders;
shaders << ShaderPreambleAC({"i", "j", "k", "ssbo_s", "ssbo_pty",
"rtarr_pty", "ac_ssbo", "ac_rtarr"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 32\n"
+ << "OpDecorate %rtarr ArrayStride 32\n"
<< DecoSSBO() << "OpMemberDecorate %rtelem 0 Offset 0\n"
<< "OpMemberDecorate %rtelem 1 Offset 16\n"
<< TypesVoid() << TypesInt() << TypesFloat() << R"(
@@ -1238,7 +1234,7 @@
shaders << ShaderPreambleAC({"i", "j", "k", "bb1", "bb2", "ssbo_s",
"ssbo_pty", "rtarr_pty", "ac_ssbo",
"ac_rtarr"})
- << "OpMemberDecorate %ssbo_s 0 ArrayStride 32\n"
+ << "OpDecorate %rtarr ArrayStride 32\n"
<< DecoSSBO() << "OpMemberDecorate %rtelem 0 Offset 0\n"
<< "OpMemberDecorate %rtelem 1 Offset 16\n"
<< TypesVoid() << TypesInt() << TypesFloat() << R"(
diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index 3840d7d..256e115 100644
--- a/test/val/val_decoration_test.cpp
+++ b/test/val/val_decoration_test.cpp
@@ -6929,6 +6929,72 @@
"identified with a Block or BufferBlock decoration"));
}
+TEST_F(ValidateDecorations, VulkanArrayStrideZero) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %var DescriptorSet 0
+OpDecorate %var Binding 0
+OpDecorate %struct Block
+OpMemberDecorate %struct 0 Offset 0
+OpDecorate %array ArrayStride 0
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_4 = OpConstant %int 4
+%array = OpTypeArray %int %int_4
+%struct = OpTypeStruct %array
+%ptr_ssbo_struct = OpTypePointer StorageBuffer %struct
+%var = OpVariable %ptr_ssbo_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("contains an array with stride 0"));
+}
+
+TEST_F(ValidateDecorations, VulkanArrayStrideTooSmall) {
+ const std::string spirv = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %main "main"
+OpExecutionMode %main LocalSize 1 1 1
+OpDecorate %var DescriptorSet 0
+OpDecorate %var Binding 0
+OpDecorate %struct Block
+OpMemberDecorate %struct 0 Offset 0
+OpDecorate %inner ArrayStride 4
+OpDecorate %outer ArrayStride 4
+%void = OpTypeVoid
+%int = OpTypeInt 32 0
+%int_4 = OpConstant %int 4
+%inner = OpTypeArray %int %int_4
+%outer = OpTypeArray %inner %int_4
+%struct = OpTypeStruct %outer
+%ptr_ssbo_struct = OpTypePointer StorageBuffer %struct
+%var = OpVariable %ptr_ssbo_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(
+ "contains an array with stride 4, but with an element size of 16"));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp
index 17f26ee..22761cc 100644
--- a/test/val/val_memory_test.cpp
+++ b/test/val/val_memory_test.cpp
@@ -2996,6 +2996,7 @@
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
+OpDecorate %inner_array_t ArrayStride 4
OpDecorate %array_t ArrayStride 4
OpMemberDecorate %struct_t 0 Offset 0
OpDecorate %struct_t Block
@@ -3025,6 +3026,7 @@
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
+OpDecorate %inner_array_t ArrayStride 4
OpDecorate %array_t ArrayStride 4
OpMemberDecorate %struct_t 0 Offset 0
OpDecorate %struct_t Block
