spirv-fuzz: Don't replace irrelevant indices in OpAccessChain (#3988)
Part of #3980.
diff --git a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp
index ae9a754..38553d2 100644
--- a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp
+++ b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp
@@ -76,7 +76,8 @@
// the index of the operand restricted to input operands only.
uint32_t use_in_operand_index =
fuzzerutil::InOperandIndexFromOperandIndex(*use_inst, use_index);
- if (!fuzzerutil::IdUseCanBeReplaced(GetIRContext(), use_inst,
+ if (!fuzzerutil::IdUseCanBeReplaced(GetIRContext(),
+ *GetTransformationContext(), use_inst,
use_in_operand_index)) {
continue;
}
diff --git a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
index f35ee50..432addb 100644
--- a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
+++ b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp
@@ -115,8 +115,9 @@
fuzzerutil::InOperandIndexFromOperandIndex(*use_inst, use_index);
// Only go ahead if this id use can be replaced in principle.
- if (!fuzzerutil::IdUseCanBeReplaced(GetIRContext(), use_inst,
- in_index)) {
+ if (!fuzzerutil::IdUseCanBeReplaced(GetIRContext(),
+ *GetTransformationContext(),
+ use_inst, in_index)) {
return;
}
diff --git a/source/fuzz/fuzzer_util.cpp b/source/fuzz/fuzzer_util.cpp
index aa186c2..8c14db4 100644
--- a/source/fuzz/fuzzer_util.cpp
+++ b/source/fuzz/fuzzer_util.cpp
@@ -1530,10 +1530,18 @@
}
bool IdUseCanBeReplaced(opt::IRContext* ir_context,
+ const TransformationContext& transformation_context,
opt::Instruction* use_instruction,
uint32_t use_in_operand_index) {
if (spvOpcodeIsAccessChain(use_instruction->opcode()) &&
use_in_operand_index > 0) {
+ // A replacement for an irrelevant index in OpAccessChain must be clamped
+ // first.
+ if (transformation_context.GetFactManager()->IdIsIrrelevant(
+ use_instruction->GetSingleWordInOperand(use_in_operand_index))) {
+ return false;
+ }
+
// This is an access chain index. If the (sub-)object being accessed by the
// given index has struct type then we cannot replace the use, as it needs
// to be an OpConstant.
diff --git a/source/fuzz/fuzzer_util.h b/source/fuzz/fuzzer_util.h
index f23826a..4e6ec36 100644
--- a/source/fuzz/fuzzer_util.h
+++ b/source/fuzz/fuzzer_util.h
@@ -545,13 +545,16 @@
// replacing the id use at |use_in_operand_index| of |use_instruction| with a
// synonym or another id of appropriate type if the original id is irrelevant.
// In particular, this checks that:
-// - the id use is not an index into a struct field in an OpAccessChain - such
+// - If id use is an index of an irrelevant id (|use_in_operand_index > 0|)
+// in OpAccessChain - it can't be replaced.
+// - The id use is not an index into a struct field in an OpAccessChain - such
// indices must be constants, so it is dangerous to replace them.
-// - the id use is not a pointer function call argument, on which there are
+// - The id use is not a pointer function call argument, on which there are
// restrictions that make replacement problematic.
-// - the id use is not the Sample parameter of an OpImageTexelPointer
+// - The id use is not the Sample parameter of an OpImageTexelPointer
// instruction, as this must satisfy particular requirements.
bool IdUseCanBeReplaced(opt::IRContext* ir_context,
+ const TransformationContext& transformation_context,
opt::Instruction* use_instruction,
uint32_t use_in_operand_index);
diff --git a/source/fuzz/transformation_replace_id_with_synonym.cpp b/source/fuzz/transformation_replace_id_with_synonym.cpp
index 0b5cf8e..24e079f 100644
--- a/source/fuzz/transformation_replace_id_with_synonym.cpp
+++ b/source/fuzz/transformation_replace_id_with_synonym.cpp
@@ -74,7 +74,7 @@
// Is the use suitable for being replaced in principle?
if (!fuzzerutil::IdUseCanBeReplaced(
- ir_context, use_instruction,
+ ir_context, transformation_context, use_instruction,
message_.id_use_descriptor().in_operand_index())) {
return false;
}
diff --git a/source/fuzz/transformation_replace_irrelevant_id.cpp b/source/fuzz/transformation_replace_irrelevant_id.cpp
index 5cea04d..27f56eb 100644
--- a/source/fuzz/transformation_replace_irrelevant_id.cpp
+++ b/source/fuzz/transformation_replace_irrelevant_id.cpp
@@ -78,8 +78,8 @@
message_.id_use_descriptor().in_operand_index();
// The id use must be replaceable with any other id of the same type.
- if (!fuzzerutil::IdUseCanBeReplaced(ir_context, use_instruction,
- use_in_operand_index)) {
+ if (!fuzzerutil::IdUseCanBeReplaced(ir_context, transformation_context,
+ use_instruction, use_in_operand_index)) {
return false;
}
diff --git a/test/fuzz/transformation_replace_irrelevant_id_test.cpp b/test/fuzz/transformation_replace_irrelevant_id_test.cpp
index 828fd82..c04a091 100644
--- a/test/fuzz/transformation_replace_irrelevant_id_test.cpp
+++ b/test/fuzz/transformation_replace_irrelevant_id_test.cpp
@@ -285,6 +285,52 @@
.IsApplicable(context.get(), transformation_context));
}
+TEST(TransformationReplaceIrrelevantIdTest, OpAccessChainIrrelevantIndex) {
+ // Checks that we can't replace irrelevant index operands in OpAccessChain.
+ 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 = OpTypeVector %6 2
+ %8 = OpTypePointer Function %7
+ %10 = OpConstant %6 0
+ %11 = OpConstant %6 2
+ %13 = OpTypePointer Function %6
+ %4 = OpFunction %2 None %3
+ %5 = OpLabel
+ %9 = OpVariable %8 Function
+ %12 = OpAccessChain %13 %9 %10
+ OpReturn
+ 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(10);
+
+ // We cannot replace the use of %10 in %12 with %11 because %10 is an
+ // irrelevant id.
+ ASSERT_FALSE(
+ TransformationReplaceIrrelevantId(
+ MakeIdUseDescriptor(
+ 10, MakeInstructionDescriptor(12, SpvOpAccessChain, 0), 1),
+ 11)
+ .IsApplicable(context.get(), transformation_context));
+}
+
} // namespace
} // namespace fuzz
} // namespace spvtools
