spirv-fuzz: Skip OpFunction when replacing irrelevant ids (#3932)
Fixes #3927.
diff --git a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
index 71f3507..f35ee50 100644
--- a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
+++ b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
@@ -70,10 +70,12 @@
}
// For every type for which we have at least one irrelevant id, record all ids
- // in the module which have that type.
+ // in the module which have that type. Skip ids of OpFunction instructions as
+ // we cannot use these as replacements.
for (const auto& pair : GetIRContext()->get_def_use_mgr()->id_to_defs()) {
uint32_t type_id = pair.second->type_id();
- if (type_id && types_to_ids.count(type_id)) {
+ if (pair.second->opcode() != SpvOpFunction && type_id &&
+ types_to_ids.count(type_id)) {
types_to_ids[type_id].push_back(pair.first);
}
}
@@ -122,7 +124,7 @@
std::vector<uint32_t> available_replacement_ids;
for (auto replacement_id : types_to_ids[type_id]) {
- // We cannot replace an id with itself.
+ // It would be pointless to replace an id with itself.
if (replacement_id == irrelevant_id) {
continue;
}
diff --git a/source/fuzz/transformation_replace_irrelevant_id.cpp b/source/fuzz/transformation_replace_irrelevant_id.cpp
index 7cd4eff..5cea04d 100644
--- a/source/fuzz/transformation_replace_irrelevant_id.cpp
+++ b/source/fuzz/transformation_replace_irrelevant_id.cpp
@@ -64,6 +64,11 @@
return false;
}
+ // The replacement id must not be the result of an OpFunction instruction.
+ if (replacement_id_def->opcode() == SpvOpFunction) {
+ return false;
+ }
+
// Consistency check: an irrelevant id cannot be a pointer.
assert(
!ir_context->get_type_mgr()->GetType(type_id_of_interest)->AsPointer() &&
diff --git a/source/fuzz/transformation_replace_irrelevant_id.h b/source/fuzz/transformation_replace_irrelevant_id.h
index 0210520..35b1987 100644
--- a/source/fuzz/transformation_replace_irrelevant_id.h
+++ b/source/fuzz/transformation_replace_irrelevant_id.h
@@ -32,6 +32,7 @@
// - The id of interest in |message_.id_use_descriptor| is irrelevant
// according to the fact manager.
// - The types of the original id and of the replacement ids are the same.
+ // - The replacement must not be the result id of an OpFunction instruction.
// - |message_.replacement_id| is available to use at the enclosing
// instruction of |message_.id_use_descriptor|.
// - The original id is in principle replaceable with any other id of the same
diff --git a/test/fuzz/transformation_replace_irrelevant_id_test.cpp b/test/fuzz/transformation_replace_irrelevant_id_test.cpp
index 345ef86..828fd82 100644
--- a/test/fuzz/transformation_replace_irrelevant_id_test.cpp
+++ b/test/fuzz/transformation_replace_irrelevant_id_test.cpp
@@ -236,6 +236,55 @@
.IsApplicable(context.get(), transformation_context));
}
+TEST(TransformationReplaceIrrelevantIdTest,
+ DoNotReplaceIrrelevantIdWithOpFunction) {
+ // Checks that an OpFunction result id is not allowed to be used to replace an
+ // irrelevant id.
+ const std::string reference_shader = R"(
+ OpCapability Shader
+ %1 = OpExtInstImport "GLSL.std.450"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %4 "main"
+ OpExecutionMode %4 OriginUpperLeft
+ OpSource ESSL 320
+ %2 = OpTypeVoid
+ %3 = OpTypeFunction %2
+ %6 = OpTypeInt 32 1
+ %7 = OpTypeFunction %6
+ %13 = OpConstant %6 2
+ %4 = OpFunction %2 None %3
+ %5 = OpLabel
+ %20 = OpCopyObject %6 %13
+ %21 = OpCopyObject %6 %20
+ OpReturn
+ OpFunctionEnd
+ %10 = OpFunction %6 None %7
+ %11 = OpLabel
+ OpReturnValue %13
+ OpFunctionEnd
+ )";
+
+ const auto env = SPV_ENV_UNIVERSAL_1_5;
+ const auto consumer = nullptr;
+ const auto context =
+ BuildModule(env, consumer, reference_shader, kFuzzAssembleOption);
+ spvtools::ValidatorOptions validator_options;
+ ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options,
+ kConsoleMessageConsumer));
+ TransformationContext transformation_context(
+ MakeUnique<FactManager>(context.get()), validator_options);
+ transformation_context.GetFactManager()->AddFactIdIsIrrelevant(20);
+
+ // We cannot replace the use of %20 in by %21 with %10 because %10 is an
+ // OpFunction instruction.
+ ASSERT_FALSE(
+ TransformationReplaceIrrelevantId(
+ MakeIdUseDescriptor(
+ 20, MakeInstructionDescriptor(21, SpvOpCopyObject, 0), 0),
+ 10)
+ .IsApplicable(context.get(), transformation_context));
+}
+
} // namespace
} // namespace fuzz
} // namespace spvtools
