spirv-fuzz: Avoid using block-decorated structs in transformations (#3877)
Fixes #3875.
diff --git a/source/fuzz/fuzzer_pass.cpp b/source/fuzz/fuzzer_pass.cpp
index b1f39b7..da6b9a5 100644
--- a/source/fuzz/fuzzer_pass.cpp
+++ b/source/fuzz/fuzzer_pass.cpp
@@ -439,17 +439,21 @@
}
break;
case SpvOpTypeStruct: {
- // A struct type is basic if all of its members are basic.
- bool all_members_are_basic_types = true;
- for (uint32_t i = 0; i < inst.NumInOperands(); i++) {
- if (!basic_types.count(inst.GetSingleWordInOperand(i))) {
- all_members_are_basic_types = false;
- break;
+ // A struct type is basic if it does not have the Block/BufferBlock
+ // decoration, and if all of its members are basic.
+ if (!fuzzerutil::HasBlockOrBufferBlockDecoration(GetIRContext(),
+ inst.result_id())) {
+ bool all_members_are_basic_types = true;
+ for (uint32_t i = 0; i < inst.NumInOperands(); i++) {
+ if (!basic_types.count(inst.GetSingleWordInOperand(i))) {
+ all_members_are_basic_types = false;
+ break;
+ }
}
- }
- if (all_members_are_basic_types) {
- basic_types.insert(inst.result_id());
- basic_type_to_pointers.insert({inst.result_id(), {}});
+ if (all_members_are_basic_types) {
+ basic_types.insert(inst.result_id());
+ basic_type_to_pointers.insert({inst.result_id(), {}});
+ }
}
break;
}
@@ -515,6 +519,10 @@
scalar_or_composite_type_id, is_irrelevant);
}
case SpvOpTypeStruct: {
+ assert(!fuzzerutil::HasBlockOrBufferBlockDecoration(
+ GetIRContext(), scalar_or_composite_type_id) &&
+ "We do not construct constants of struct types decorated with "
+ "Block or BufferBlock.");
std::vector<uint32_t> field_zero_ids;
for (uint32_t index = 0; index < type_instruction->NumInOperands();
index++) {
diff --git a/source/fuzz/fuzzer_pass_construct_composites.cpp b/source/fuzz/fuzzer_pass_construct_composites.cpp
index 6443e89..584fa1a 100644
--- a/source/fuzz/fuzzer_pass_construct_composites.cpp
+++ b/source/fuzz/fuzzer_pass_construct_composites.cpp
@@ -32,11 +32,14 @@
FuzzerPassConstructComposites::~FuzzerPassConstructComposites() = default;
void FuzzerPassConstructComposites::Apply() {
- // Gather up the ids of all composite types.
+ // Gather up the ids of all composite types, but skip block-/buffer
+ // block-decorated struct types.
std::vector<uint32_t> composite_type_ids;
for (auto& inst : GetIRContext()->types_values()) {
if (fuzzerutil::IsCompositeType(
- GetIRContext()->get_type_mgr()->GetType(inst.result_id()))) {
+ GetIRContext()->get_type_mgr()->GetType(inst.result_id())) &&
+ !fuzzerutil::HasBlockOrBufferBlockDecoration(GetIRContext(),
+ inst.result_id())) {
composite_type_ids.push_back(inst.result_id());
}
}
diff --git a/source/fuzz/fuzzer_util.cpp b/source/fuzz/fuzzer_util.cpp
index 48aee58..99d64eb 100644
--- a/source/fuzz/fuzzer_util.cpp
+++ b/source/fuzz/fuzzer_util.cpp
@@ -1550,6 +1550,18 @@
return builtin_count != 0;
}
+bool HasBlockOrBufferBlockDecoration(opt::IRContext* ir_context, uint32_t id) {
+ for (auto decoration : {SpvDecorationBlock, SpvDecorationBufferBlock}) {
+ if (!ir_context->get_decoration_mgr()->WhileEachDecoration(
+ id, decoration, [](const opt::Instruction & /*unused*/) -> bool {
+ return false;
+ })) {
+ return true;
+ }
+ }
+ return false;
+}
+
bool SplittingBeforeInstructionSeparatesOpSampledImageDefinitionFromUse(
opt::BasicBlock* block_to_split, opt::Instruction* split_before) {
std::set<uint32_t> sampled_image_result_ids;
diff --git a/source/fuzz/fuzzer_util.h b/source/fuzz/fuzzer_util.h
index c7e55e2..b0141f8 100644
--- a/source/fuzz/fuzzer_util.h
+++ b/source/fuzz/fuzzer_util.h
@@ -541,6 +541,12 @@
bool MembersHaveBuiltInDecoration(opt::IRContext* ir_context,
uint32_t struct_type_id);
+// Returns true if and only if |id| is decorated with either Block or
+// BufferBlock. Even though these decorations are only allowed on struct types,
+// for convenience |id| can be any result id so that it is possible to call this
+// method on something that *might* be a struct type.
+bool HasBlockOrBufferBlockDecoration(opt::IRContext* ir_context, uint32_t id);
+
// Returns true iff splitting block |block_to_split| just before the instruction
// |split_before| would separate an OpSampledImage instruction from its usage.
bool SplittingBeforeInstructionSeparatesOpSampledImageDefinitionFromUse(
diff --git a/source/fuzz/transformation_add_constant_composite.cpp b/source/fuzz/transformation_add_constant_composite.cpp
index ac4d599..0260321 100644
--- a/source/fuzz/transformation_add_constant_composite.cpp
+++ b/source/fuzz/transformation_add_constant_composite.cpp
@@ -77,14 +77,9 @@
// BufferBlock. The SPIR-V spec does not explicitly disallow this, but it
// seems like a strange thing to do, so we disallow it to avoid triggering
// low priorty edge case issues related to it.
- for (auto decoration : {SpvDecorationBlock, SpvDecorationBufferBlock}) {
- if (!ir_context->get_decoration_mgr()->WhileEachDecoration(
- composite_type_instruction->result_id(), decoration,
- [](const opt::Instruction & /*unused*/) -> bool {
- return false;
- })) {
- return false;
- }
+ if (fuzzerutil::HasBlockOrBufferBlockDecoration(
+ ir_context, composite_type_instruction->result_id())) {
+ return false;
}
composite_type_instruction->ForEachInOperand(
[&constituent_type_ids](const uint32_t* member_type_id) {
diff --git a/source/fuzz/transformation_add_constant_composite.h b/source/fuzz/transformation_add_constant_composite.h
index 56ac52a..9e9222d 100644
--- a/source/fuzz/transformation_add_constant_composite.h
+++ b/source/fuzz/transformation_add_constant_composite.h
@@ -38,7 +38,7 @@
// - |message_.type_id| must be the id of a composite type
// - |message_.constituent_id| must refer to ids that match the constituent
// types of this composite type
- // - If |message_.type_id| is a struc type, it must not have the Block or
+ // - If |message_.type_id| is a struct type, it must not have the Block or
// BufferBlock decoration
bool IsApplicable(
opt::IRContext* ir_context,
diff --git a/source/fuzz/transformation_add_copy_memory.cpp b/source/fuzz/transformation_add_copy_memory.cpp
index e2c7eee..44bb9c5 100644
--- a/source/fuzz/transformation_add_copy_memory.cpp
+++ b/source/fuzz/transformation_add_copy_memory.cpp
@@ -162,8 +162,21 @@
const auto* type = ir_context->get_type_mgr()->GetType(inst->type_id());
assert(type && "Instruction must have a valid type");
- return type->AsPointer() &&
- CanUsePointeeWithCopyMemory(*type->AsPointer()->pointee_type());
+ if (!type->AsPointer()) {
+ return false;
+ }
+
+ // We do not support copying memory from a pointer to a block-/buffer
+ // block-decorated struct.
+ auto pointee_type_inst = ir_context->get_def_use_mgr()
+ ->GetDef(inst->type_id())
+ ->GetSingleWordInOperand(1);
+ if (fuzzerutil::HasBlockOrBufferBlockDecoration(ir_context,
+ pointee_type_inst)) {
+ return false;
+ }
+
+ return CanUsePointeeWithCopyMemory(*type->AsPointer()->pointee_type());
}
bool TransformationAddCopyMemory::CanUsePointeeWithCopyMemory(
diff --git a/source/fuzz/transformation_add_copy_memory.h b/source/fuzz/transformation_add_copy_memory.h
index b3f58c0..cc42f1e 100644
--- a/source/fuzz/transformation_add_copy_memory.h
+++ b/source/fuzz/transformation_add_copy_memory.h
@@ -41,6 +41,8 @@
// - |fresh_id| must be a fresh id to copy memory into.
// - type of |source_id| must be OpTypePointer where pointee can be used with
// OpCopyMemory.
+ // - If the pointee type of |source_id| is a struct type, it must not have the
+ // Block or BufferBlock decoration.
// - |storage_class| must be either Private or Function.
// - type ids of instructions with result ids |source_id| and |initialize_id|
// must be the same.
diff --git a/test/fuzz/transformation_add_copy_memory_test.cpp b/test/fuzz/transformation_add_copy_memory_test.cpp
index 10487e6..be4a300 100644
--- a/test/fuzz/transformation_add_copy_memory_test.cpp
+++ b/test/fuzz/transformation_add_copy_memory_test.cpp
@@ -381,6 +381,98 @@
ASSERT_TRUE(IsEqual(env, expected, context.get()));
}
+TEST(TransformationAddCopyMemoryTest, DisallowBufferBlockDecoration) {
+ std::string shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %4 "main"
+ OpExecutionMode %4 LocalSize 1 1 1
+ OpSource ESSL 320
+ OpName %4 "main"
+ OpName %7 "buf"
+ OpMemberName %7 0 "a"
+ OpMemberName %7 1 "b"
+ OpName %9 ""
+ OpMemberDecorate %7 0 Offset 0
+ OpMemberDecorate %7 1 Offset 4
+ OpDecorate %7 BufferBlock
+ OpDecorate %9 DescriptorSet 0
+ OpDecorate %9 Binding 0
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeInt 32 1
+ %10 = OpConstant %6 42
+ %7 = OpTypeStruct %6 %6
+ %8 = OpTypePointer Uniform %7
+ %9 = OpVariable %8 Uniform
+ %50 = OpUndef %7
+ %4 = OpFunction %2 None %3
+ %5 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_0;
+ const auto consumer = nullptr;
+ const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+ ASSERT_TRUE(IsValid(env, context.get()));
+
+ spvtools::ValidatorOptions validator_options;
+ TransformationContext transformation_context(
+ MakeUnique<FactManager>(context.get()), validator_options);
+ ASSERT_FALSE(
+ TransformationAddCopyMemory(MakeInstructionDescriptor(5, SpvOpReturn, 0),
+ 100, 9, SpvStorageClassPrivate, 50)
+ .IsApplicable(context.get(), transformation_context));
+}
+
+TEST(TransformationAddCopyMemoryTest, DisallowBlockDecoration) {
+ std::string shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %4 "main" %9
+ OpExecutionMode %4 LocalSize 1 1 1
+ OpSource ESSL 320
+ OpName %4 "main"
+ OpName %7 "buf"
+ OpMemberName %7 0 "a"
+ OpMemberName %7 1 "b"
+ OpName %9 ""
+ OpMemberDecorate %7 0 Offset 0
+ OpMemberDecorate %7 1 Offset 4
+ OpDecorate %7 Block
+ OpDecorate %9 DescriptorSet 0
+ OpDecorate %9 Binding 0
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeInt 32 1
+ %10 = OpConstant %6 42
+ %7 = OpTypeStruct %6 %6
+ %8 = OpTypePointer StorageBuffer %7
+ %9 = OpVariable %8 StorageBuffer
+ %50 = OpUndef %7
+ %4 = OpFunction %2 None %3
+ %5 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_5;
+ const auto consumer = nullptr;
+ const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+ ASSERT_TRUE(IsValid(env, context.get()));
+
+ spvtools::ValidatorOptions validator_options;
+ TransformationContext transformation_context(
+ MakeUnique<FactManager>(context.get()), validator_options);
+ ASSERT_FALSE(
+ TransformationAddCopyMemory(MakeInstructionDescriptor(5, SpvOpReturn, 0),
+ 100, 9, SpvStorageClassPrivate, 50)
+ .IsApplicable(context.get(), transformation_context));
+}
+
} // namespace
} // namespace fuzz
} // namespace spvtools
\ No newline at end of file
