diff --git a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp index ae9a7543..38553d25 100644 --- a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp +++ b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp @@ -76,7 +76,8 @@ void FuzzerPassApplyIdSynonyms::Apply() { // 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 f35ee50f..432addbc 100644 --- a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp +++ b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp @@ -115,8 +115,9 @@ void FuzzerPassReplaceIrrelevantIds::Apply() { 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 aa186c29..8c14db4b 100644 --- a/source/fuzz/fuzzer_util.cpp +++ b/source/fuzz/fuzzer_util.cpp @@ -1530,10 +1530,18 @@ opt::Instruction* GetLastInsertBeforeInstruction(opt::IRContext* ir_context, } 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 f23826ae..4e6ec36e 100644 --- a/source/fuzz/fuzzer_util.h +++ b/source/fuzz/fuzzer_util.h @@ -545,13 +545,16 @@ opt::Instruction* GetLastInsertBeforeInstruction(opt::IRContext* ir_context, // 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 0b5cf8ed..24e079ff 100644 --- a/source/fuzz/transformation_replace_id_with_synonym.cpp +++ b/source/fuzz/transformation_replace_id_with_synonym.cpp @@ -74,7 +74,7 @@ bool TransformationReplaceIdWithSynonym::IsApplicable( // 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 5cea04d5..27f56eba 100644 --- a/source/fuzz/transformation_replace_irrelevant_id.cpp +++ b/source/fuzz/transformation_replace_irrelevant_id.cpp @@ -78,8 +78,8 @@ bool TransformationReplaceIrrelevantId::IsApplicable( 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 828fd829..c04a091d 100644 --- a/test/fuzz/transformation_replace_irrelevant_id_test.cpp +++ b/test/fuzz/transformation_replace_irrelevant_id_test.cpp @@ -285,6 +285,52 @@ TEST(TransformationReplaceIrrelevantIdTest, .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(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