Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate decoration of structs with RuntimeArray #5094

Merged
merged 2 commits into from
Feb 3, 2023
Merged

Validate decoration of structs with RuntimeArray #5094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d75f96
Validate decoration of structs with RuntimeArray
FoggyMist Feb 1, 2023
a544fe2
Formatting fixes
FoggyMist Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions source/val/validate_type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,15 @@ spv_result_t ValidateTypeStruct(ValidationState_t& _, const Instruction* inst) {
<< ", OpTypeRuntimeArray must only be used for the last member "
"of an OpTypeStruct";
}

if (!_.HasDecoration(inst->id(), spv::Decoration::Block) &&
!_.HasDecoration(inst->id(), spv::Decoration::BufferBlock)) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< _.VkErrorID(4680)
<< spvLogStringForEnv(_.context()->target_env)
<< ", OpTypeStruct containing an OpTypeRuntimeArray "
<< "must be decorated with Block or BufferBlock.";
}
}
}

Expand Down
31 changes: 31 additions & 0 deletions test/val/val_decoration_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5281,6 +5281,37 @@ OpFunctionEnd
"rules: member 1 at offset 1 is not aligned to 4"));
}

TEST_F(ValidateDecorations, VulkanStructWithoutDecorationWithRuntimeArray) {
std::string str = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
OpDecorate %array_t ArrayStride 4
OpMemberDecorate %struct_t 0 Offset 0
OpMemberDecorate %struct_t 1 Offset 4
%uint_t = OpTypeInt 32 0
%array_t = OpTypeRuntimeArray %uint_t
%struct_t = OpTypeStruct %uint_t %array_t
%struct_ptr = OpTypePointer StorageBuffer %struct_t
%2 = OpVariable %struct_ptr StorageBuffer
%void = OpTypeVoid
%func_t = OpTypeFunction %void
%func = OpFunction %void None %func_t
%1 = OpLabel
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_1);
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-OpTypeRuntimeArray-04680"));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Vulkan, OpTypeStruct containing an OpTypeRuntimeArray "
"must be decorated with Block or BufferBlock."));
}

TEST_F(ValidateDecorations, EmptyStructAtNonZeroOffsetGood) {
const std::string spirv = R"(
OpCapability Shader
Expand Down
9 changes: 6 additions & 3 deletions test/val/val_memory_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2475,6 +2475,7 @@ OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
OpDecorate %struct_t Block
%uint_t = OpTypeInt 32 0
%array_t = OpTypeRuntimeArray %uint_t
%struct_t = OpTypeStruct %array_t
Expand All @@ -2498,7 +2499,7 @@ OpFunctionEnd
"For Vulkan, OpTypeStruct variables containing OpTypeRuntimeArray "
"must have storage class of StorageBuffer, PhysicalStorageBuffer, or "
"Uniform.\n %6 = "
"OpVariable %_ptr_Workgroup__struct_4 Workgroup\n"));
"OpVariable %_ptr_Workgroup__struct_2 Workgroup\n"));
}

TEST_F(ValidateMemory, VulkanRTAInsideStorageBufferStructWithoutBlockBad) {
Expand All @@ -2507,6 +2508,7 @@ OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
OpDecorate %struct_t BufferBlock
%uint_t = OpTypeInt 32 0
%array_t = OpTypeRuntimeArray %uint_t
%struct_t = OpTypeStruct %array_t
Expand All @@ -2529,7 +2531,7 @@ OpFunctionEnd
"OpTypeRuntimeArray must be decorated with Block if it "
"has storage class StorageBuffer or "
"PhysicalStorageBuffer.\n %6 = OpVariable "
"%_ptr_StorageBuffer__struct_4 StorageBuffer\n"));
"%_ptr_StorageBuffer__struct_2 StorageBuffer\n"));
}

TEST_F(ValidateMemory, VulkanRTAInsideUniformStructGood) {
Expand Down Expand Up @@ -2564,6 +2566,7 @@ OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %func "func"
OpExecutionMode %func OriginUpperLeft
OpDecorate %struct_t Block
%uint_t = OpTypeInt 32 0
%array_t = OpTypeRuntimeArray %uint_t
%struct_t = OpTypeStruct %array_t
Expand All @@ -2585,7 +2588,7 @@ OpFunctionEnd
HasSubstr("For Vulkan, an OpTypeStruct variable containing an "
"OpTypeRuntimeArray must be decorated with BufferBlock "
"if it has storage class Uniform.\n %6 = OpVariable "
"%_ptr_Uniform__struct_4 Uniform\n"));
"%_ptr_Uniform__struct_2 Uniform\n"));
}

TEST_F(ValidateMemory, VulkanRTAInsideRTABad) {
Expand Down