From f0df9e88b07d3fa49048202e12943bf75fffd945 Mon Sep 17 00:00:00 2001 From: Stefano Milizia Date: Fri, 18 Sep 2020 12:45:02 +0200 Subject: [PATCH] spirv-fuzz: Consider all ids from dead blocks irrelevant (#3795) This PR modifies the FactManager methods IdIsIrrelevant and GetIrrelevantIds so that an id is always considered irrelevant if it comes from a dead block. Fixes #3733. --- .../data_synonym_and_id_equation_facts.cpp | 16 ++- .../data_synonym_and_id_equation_facts.h | 10 +- source/fuzz/fact_manager/dead_block_facts.cpp | 4 + source/fuzz/fact_manager/dead_block_facts.h | 3 + source/fuzz/fact_manager/fact_manager.cpp | 22 +-- source/fuzz/fact_manager/fact_manager.h | 11 +- .../fact_manager/irrelevant_value_facts.cpp | 53 ++++++- .../fact_manager/irrelevant_value_facts.h | 13 +- .../fuzzer_pass_add_equation_instructions.cpp | 6 +- .../fuzz/fuzzer_pass_add_opphi_synonyms.cpp | 4 +- ...r_pass_add_vector_shuffle_instructions.cpp | 3 +- source/fuzz/fuzzer_pass_apply_id_synonyms.cpp | 2 +- .../fuzz/fuzzer_pass_construct_composites.cpp | 2 +- ...rchange_signedness_of_integer_operands.cpp | 4 +- ...r_pass_interchange_zero_like_constants.cpp | 4 +- ...fuzzer_pass_push_ids_through_variables.cpp | 3 +- .../fuzzer_pass_replace_irrelevant_ids.cpp | 4 +- source/fuzz/fuzzer_util.cpp | 10 +- .../transformation_composite_construct.cpp | 7 +- .../fuzz/transformation_composite_extract.cpp | 4 +- .../fuzz/transformation_composite_insert.cpp | 4 +- .../transformation_equation_instruction.cpp | 3 +- ...ransformation_push_id_through_variable.cpp | 4 +- ...sformation_record_synonymous_constants.cpp | 4 +- .../transformation_replace_irrelevant_id.cpp | 4 +- ...ormation_replace_parameter_with_global.cpp | 2 +- source/fuzz/transformation_vector_shuffle.cpp | 15 +- test/fuzz/fact_manager_test.cpp | 136 ++++++++++++++++-- ...ansformation_add_constant_boolean_test.cpp | 8 +- ...sformation_add_constant_composite_test.cpp | 4 +- ...ransformation_add_constant_scalar_test.cpp | 4 +- .../transformation_add_parameter_test.cpp | 8 +- ...mation_flatten_conditional_branch_test.cpp | 3 +- 33 files changed, 296 insertions(+), 88 deletions(-) diff --git a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp index d5d9f3fc..32c9c758 100644 --- a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp +++ b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp @@ -53,11 +53,15 @@ bool DataSynonymAndIdEquationFacts::OperationEquals::operator()( void DataSynonymAndIdEquationFacts::AddFact( const protobufs::FactDataSynonym& fact, + const DeadBlockFacts& dead_block_facts, const IrrelevantValueFacts& irrelevant_value_facts, opt::IRContext* context) { + (void)dead_block_facts; // Keep release compilers happy. (void)irrelevant_value_facts; // Keep release compilers happy. - assert(!irrelevant_value_facts.IdIsIrrelevant(fact.data1().object()) && - !irrelevant_value_facts.IdIsIrrelevant(fact.data2().object()) && + assert(!irrelevant_value_facts.IdIsIrrelevant(fact.data1().object(), + dead_block_facts, context) && + !irrelevant_value_facts.IdIsIrrelevant(fact.data2().object(), + dead_block_facts, context) && "Irrelevant ids cannot be synonymous with other ids."); // Add the fact, including all facts relating sub-components of the data @@ -67,10 +71,13 @@ void DataSynonymAndIdEquationFacts::AddFact( void DataSynonymAndIdEquationFacts::AddFact( const protobufs::FactIdEquation& fact, + const DeadBlockFacts& dead_block_facts, const IrrelevantValueFacts& irrelevant_value_facts, opt::IRContext* context) { + (void)dead_block_facts; // Keep release compilers happy. (void)irrelevant_value_facts; // Keep release compilers happy. - assert(!irrelevant_value_facts.IdIsIrrelevant(fact.lhs_id()) && + assert(!irrelevant_value_facts.IdIsIrrelevant(fact.lhs_id(), dead_block_facts, + context) && "Irrelevant ids are not allowed."); protobufs::DataDescriptor lhs_dd = MakeDataDescriptor(fact.lhs_id(), {}); @@ -82,7 +89,8 @@ void DataSynonymAndIdEquationFacts::AddFact( // equation. std::vector rhs_dds; for (auto rhs_id : fact.rhs_id()) { - assert(!irrelevant_value_facts.IdIsIrrelevant(rhs_id) && + assert(!irrelevant_value_facts.IdIsIrrelevant(rhs_id, dead_block_facts, + context) && "Irrelevant ids are not allowed."); // Register a data descriptor based on this id in the equivalence relation diff --git a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h index df35ad64..7b3a21f7 100644 --- a/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h +++ b/source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h @@ -27,6 +27,8 @@ namespace spvtools { namespace fuzz { namespace fact_manager { +// Forward reference to the DeadBlockFacts class. +class DeadBlockFacts; // Forward reference to the IrrelevantValueFacts class. class IrrelevantValueFacts; @@ -35,14 +37,18 @@ class IrrelevantValueFacts; class DataSynonymAndIdEquationFacts { public: // See method in FactManager which delegates to this method. - // |irrelevant_value_facts| is passed for consistency checks. + // |dead_block_facts| and |irrelevant_value_facts| are passed for consistency + // checks. void AddFact(const protobufs::FactDataSynonym& fact, + const DeadBlockFacts& dead_block_facts, const IrrelevantValueFacts& irrelevant_value_facts, opt::IRContext* context); // See method in FactManager which delegates to this method. - // |irrelevant_value_facts| is passed for consistency checks. + // |dead_block_facts| and |irrelevant_value_facts| are passed for consistency + // checks. void AddFact(const protobufs::FactIdEquation& fact, + const DeadBlockFacts& dead_block_facts, const IrrelevantValueFacts& irrelevant_value_facts, opt::IRContext* context); diff --git a/source/fuzz/fact_manager/dead_block_facts.cpp b/source/fuzz/fact_manager/dead_block_facts.cpp index 86d51af5..5f4f8bc2 100644 --- a/source/fuzz/fact_manager/dead_block_facts.cpp +++ b/source/fuzz/fact_manager/dead_block_facts.cpp @@ -26,6 +26,10 @@ bool DeadBlockFacts::BlockIsDead(uint32_t block_id) const { return dead_block_ids_.count(block_id) != 0; } +const std::unordered_set& DeadBlockFacts::GetDeadBlocks() const { + return dead_block_ids_; +} + } // namespace fact_manager } // namespace fuzz } // namespace spvtools diff --git a/source/fuzz/fact_manager/dead_block_facts.h b/source/fuzz/fact_manager/dead_block_facts.h index ca3f71ac..b2da90bf 100644 --- a/source/fuzz/fact_manager/dead_block_facts.h +++ b/source/fuzz/fact_manager/dead_block_facts.h @@ -33,6 +33,9 @@ class DeadBlockFacts { // See method in FactManager which delegates to this method. bool BlockIsDead(uint32_t block_id) const; + // Returns a set of all the block ids that have been declared dead. + const std::unordered_set& GetDeadBlocks() const; + private: std::unordered_set dead_block_ids_; }; diff --git a/source/fuzz/fact_manager/fact_manager.cpp b/source/fuzz/fact_manager/fact_manager.cpp index 79c20370..3a65ba81 100644 --- a/source/fuzz/fact_manager/fact_manager.cpp +++ b/source/fuzz/fact_manager/fact_manager.cpp @@ -106,7 +106,8 @@ bool FactManager::AddFact(const fuzz::protobufs::Fact& fact, context); case protobufs::Fact::kDataSynonymFact: data_synonym_and_id_equation_facts_.AddFact( - fact.data_synonym_fact(), irrelevant_value_facts_, context); + fact.data_synonym_fact(), dead_block_facts_, irrelevant_value_facts_, + context); return true; case protobufs::Fact::kBlockIsDeadFact: dead_block_facts_.AddFact(fact.block_is_dead_fact()); @@ -126,8 +127,8 @@ void FactManager::AddFactDataSynonym(const protobufs::DataDescriptor& data1, protobufs::FactDataSynonym fact; *fact.mutable_data1() = data1; *fact.mutable_data2() = data2; - data_synonym_and_id_equation_facts_.AddFact(fact, irrelevant_value_facts_, - context); + data_synonym_and_id_equation_facts_.AddFact(fact, dead_block_facts_, + irrelevant_value_facts_, context); } std::vector FactManager::GetConstantsAvailableFromUniformsForType( @@ -207,12 +208,15 @@ bool FactManager::PointeeValueIsIrrelevant(uint32_t pointer_id) const { return irrelevant_value_facts_.PointeeValueIsIrrelevant(pointer_id); } -bool FactManager::IdIsIrrelevant(uint32_t result_id) const { - return irrelevant_value_facts_.IdIsIrrelevant(result_id); +bool FactManager::IdIsIrrelevant(uint32_t result_id, + opt::IRContext* context) const { + return irrelevant_value_facts_.IdIsIrrelevant(result_id, dead_block_facts_, + context); } -const std::unordered_set& FactManager::GetIrrelevantIds() const { - return irrelevant_value_facts_.GetIrrelevantIds(); +std::unordered_set FactManager::GetIrrelevantIds( + opt::IRContext* context) const { + return irrelevant_value_facts_.GetIrrelevantIds(dead_block_facts_, context); } void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id, @@ -240,8 +244,8 @@ void FactManager::AddFactIdEquation(uint32_t lhs_id, SpvOp opcode, for (auto an_rhs_id : rhs_id) { fact.add_rhs_id(an_rhs_id); } - data_synonym_and_id_equation_facts_.AddFact(fact, irrelevant_value_facts_, - context); + data_synonym_and_id_equation_facts_.AddFact(fact, dead_block_facts_, + irrelevant_value_facts_, context); } void FactManager::ComputeClosureOfFacts( diff --git a/source/fuzz/fact_manager/fact_manager.h b/source/fuzz/fact_manager/fact_manager.h index b1f4f352..b899d9f8 100644 --- a/source/fuzz/fact_manager/fact_manager.h +++ b/source/fuzz/fact_manager/fact_manager.h @@ -194,12 +194,13 @@ class FactManager { // |pointer_id| is irrelevant. bool PointeeValueIsIrrelevant(uint32_t pointer_id) const; - // Returns true iff there exists a fact that the |result_id| is irrelevant. - bool IdIsIrrelevant(uint32_t result_id) const; + // Returns true if there exists a fact that the |result_id| is irrelevant or + // if |result_id| is declared in a block that has been declared dead. + bool IdIsIrrelevant(uint32_t result_id, opt::IRContext* context) const; - // Returns an unordered set of all the ids which have been declared - // irrelevant. - const std::unordered_set& GetIrrelevantIds() const; + // Returns a set of all the ids which have been declared irrelevant, or which + // have been declared inside a dead block. + std::unordered_set GetIrrelevantIds(opt::IRContext* context) const; // End of irrelevant value facts //============================== diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.cpp b/source/fuzz/fact_manager/irrelevant_value_facts.cpp index dda17b3d..cc4d20d5 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.cpp +++ b/source/fuzz/fact_manager/irrelevant_value_facts.cpp @@ -16,6 +16,8 @@ #include "source/fuzz/data_descriptor.h" #include "source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h" +#include "source/fuzz/fact_manager/dead_block_facts.h" +#include "source/fuzz/fuzzer_util.h" #include "source/opt/ir_context.h" namespace spvtools { @@ -60,13 +62,54 @@ bool IrrelevantValueFacts::PointeeValueIsIrrelevant(uint32_t pointer_id) const { return pointers_to_irrelevant_pointees_ids_.count(pointer_id) != 0; } -bool IrrelevantValueFacts::IdIsIrrelevant(uint32_t pointer_id) const { - return irrelevant_ids_.count(pointer_id) != 0; +bool IrrelevantValueFacts::IdIsIrrelevant( + uint32_t result_id, const DeadBlockFacts& dead_block_facts, + opt::IRContext* context) const { + // The id is irrelevant if it has been declared irrelevant. + if (irrelevant_ids_.count(result_id)) { + return true; + } + + // The id must have a non-pointer type to be irrelevant. + auto def = context->get_def_use_mgr()->GetDef(result_id); + if (!def) { + return false; + } + auto type = context->get_type_mgr()->GetType(def->type_id()); + if (!type || type->AsPointer()) { + return false; + } + + // The id is irrelevant if it is in a dead block. + return context->get_instr_block(result_id) && + dead_block_facts.BlockIsDead( + context->get_instr_block(result_id)->id()); } -const std::unordered_set& IrrelevantValueFacts::GetIrrelevantIds() - const { - return irrelevant_ids_; +std::unordered_set IrrelevantValueFacts::GetIrrelevantIds( + const DeadBlockFacts& dead_block_facts, opt::IRContext* context) const { + // Get all the ids that have been declared irrelevant. + auto irrelevant_ids = irrelevant_ids_; + + // Get all the non-pointer ids declared in dead blocks that have a type. + for (uint32_t block_id : dead_block_facts.GetDeadBlocks()) { + auto block = fuzzerutil::MaybeFindBlock(context, block_id); + // It is possible and allowed for the block not to exist, e.g. it could have + // been merged with another block. + if (!block) { + continue; + } + block->ForEachInst([context, &irrelevant_ids](opt::Instruction* inst) { + // The instruction must have a result id and a type, and it must not be a + // pointer. + if (inst->HasResultId() && inst->type_id() && + !context->get_type_mgr()->GetType(inst->type_id())->AsPointer()) { + irrelevant_ids.emplace(inst->result_id()); + } + }); + } + + return irrelevant_ids; } } // namespace fact_manager diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.h b/source/fuzz/fact_manager/irrelevant_value_facts.h index 76ff55d5..71393c55 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.h +++ b/source/fuzz/fact_manager/irrelevant_value_facts.h @@ -26,6 +26,8 @@ namespace fact_manager { // Forward reference to the DataSynonymAndIdEquationFacts class. class DataSynonymAndIdEquationFacts; +// Forward reference to the DeadBlockFacts class. +class DeadBlockFacts; // The purpose of this class is to group the fields and data used to represent // facts about various irrelevant values in the module. @@ -51,10 +53,17 @@ class IrrelevantValueFacts { bool PointeeValueIsIrrelevant(uint32_t pointer_id) const; // See method in FactManager which delegates to this method. - bool IdIsIrrelevant(uint32_t pointer_id) const; + // |dead_block_facts| and |context| are passed to check whether |result_id| is + // declared inside a dead block, in which case it is irrelevant. + bool IdIsIrrelevant(uint32_t result_id, + const DeadBlockFacts& dead_block_facts, + opt::IRContext* context) const; // See method in FactManager which delegates to this method. - const std::unordered_set& GetIrrelevantIds() const; + // |dead_block_facts| and |context| are passed to also add all the ids + // declared in dead blocks to the set of irrelevant ids. + std::unordered_set GetIrrelevantIds( + const DeadBlockFacts& dead_block_facts, opt::IRContext* context) const; private: std::unordered_set pointers_to_irrelevant_pointees_ids_; diff --git a/source/fuzz/fuzzer_pass_add_equation_instructions.cpp b/source/fuzz/fuzzer_pass_add_equation_instructions.cpp index 62fcfea7..d54e17a8 100644 --- a/source/fuzz/fuzzer_pass_add_equation_instructions.cpp +++ b/source/fuzz/fuzzer_pass_add_equation_instructions.cpp @@ -77,12 +77,14 @@ void FuzzerPassAddEquationInstructions::Apply() { std::vector available_instructions = FindAvailableInstructions( function, block, inst_it, - [this](opt::IRContext*, opt::Instruction* instruction) -> bool { + [this](opt::IRContext* ir_context, + opt::Instruction* instruction) -> bool { return instruction->result_id() && instruction->type_id() && instruction->opcode() != SpvOpUndef && !GetTransformationContext() ->GetFactManager() - ->IdIsIrrelevant(instruction->result_id()); + ->IdIsIrrelevant(instruction->result_id(), + ir_context); }); // Try the opcodes for which we know how to make ids at random until diff --git a/source/fuzz/fuzzer_pass_add_opphi_synonyms.cpp b/source/fuzz/fuzzer_pass_add_opphi_synonyms.cpp index 88cc830f..d1cd4775 100644 --- a/source/fuzz/fuzzer_pass_add_opphi_synonyms.cpp +++ b/source/fuzz/fuzzer_pass_add_opphi_synonyms.cpp @@ -157,7 +157,7 @@ FuzzerPassAddOpPhiSynonyms::GetIdEquivalenceClasses() { // Exclude irrelevant ids. if (GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - pair.first)) { + pair.first, GetIRContext())) { continue; } @@ -195,7 +195,7 @@ FuzzerPassAddOpPhiSynonyms::GetIdEquivalenceClasses() { // The synonym must not be irrelevant. if (GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - synonym->object())) { + synonym->object(), GetIRContext())) { continue; } diff --git a/source/fuzz/fuzzer_pass_add_vector_shuffle_instructions.cpp b/source/fuzz/fuzzer_pass_add_vector_shuffle_instructions.cpp index 453448ba..945bc1fa 100644 --- a/source/fuzz/fuzzer_pass_add_vector_shuffle_instructions.cpp +++ b/source/fuzz/fuzzer_pass_add_vector_shuffle_instructions.cpp @@ -75,7 +75,8 @@ void FuzzerPassAddVectorShuffleInstructions::Apply() { if (!GetTransformationContext() ->GetFactManager() - ->IdIsIrrelevant(instruction->result_id()) && + ->IdIsIrrelevant(instruction->result_id(), + GetIRContext()) && !fuzzerutil::CanMakeSynonymOf(ir_context, *GetTransformationContext(), instruction)) { diff --git a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp index 9cbf590c..2691b2a5 100644 --- a/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp +++ b/source/fuzz/fuzzer_pass_apply_id_synonyms.cpp @@ -148,7 +148,7 @@ void FuzzerPassApplyIdSynonyms::Apply() { } assert(!GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - synonym_to_try->object()) && + synonym_to_try->object(), GetIRContext()) && "Irrelevant ids can't participate in DataSynonym facts"); ApplyTransformation(TransformationCompositeExtract( MakeInstructionDescriptor(GetIRContext(), diff --git a/source/fuzz/fuzzer_pass_construct_composites.cpp b/source/fuzz/fuzzer_pass_construct_composites.cpp index 6443e896..9dde70d1 100644 --- a/source/fuzz/fuzzer_pass_construct_composites.cpp +++ b/source/fuzz/fuzzer_pass_construct_composites.cpp @@ -77,7 +77,7 @@ void FuzzerPassConstructComposites::Apply() { // to produce a synonym out of the id. return GetTransformationContext() ->GetFactManager() - ->IdIsIrrelevant(inst->result_id()) || + ->IdIsIrrelevant(inst->result_id(), GetIRContext()) || fuzzerutil::CanMakeSynonymOf( ir_context, *GetTransformationContext(), inst); }); diff --git a/source/fuzz/fuzzer_pass_interchange_signedness_of_integer_operands.cpp b/source/fuzz/fuzzer_pass_interchange_signedness_of_integer_operands.cpp index 0e40b496..5446cf80 100644 --- a/source/fuzz/fuzzer_pass_interchange_signedness_of_integer_operands.cpp +++ b/source/fuzz/fuzzer_pass_interchange_signedness_of_integer_operands.cpp @@ -48,7 +48,7 @@ void FuzzerPassInterchangeSignednessOfIntegerOperands::Apply() { // constant with opposite signedness, and this can only be done if they are // not irrelevant. if (GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - constant_id)) { + constant_id, GetIRContext())) { continue; } @@ -60,7 +60,7 @@ void FuzzerPassInterchangeSignednessOfIntegerOperands::Apply() { } assert(!GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - toggled_id) && + toggled_id, GetIRContext()) && "FindOrCreateToggledConstant can't produce an irrelevant id"); // Record synonymous constants diff --git a/source/fuzz/fuzzer_pass_interchange_zero_like_constants.cpp b/source/fuzz/fuzzer_pass_interchange_zero_like_constants.cpp index 20575e11..d939efdf 100644 --- a/source/fuzz/fuzzer_pass_interchange_zero_like_constants.cpp +++ b/source/fuzz/fuzzer_pass_interchange_zero_like_constants.cpp @@ -73,7 +73,7 @@ void FuzzerPassInterchangeZeroLikeConstants::Apply() { for (auto constant : GetIRContext()->GetConstants()) { uint32_t constant_id = constant->result_id(); if (GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - constant_id)) { + constant_id, GetIRContext())) { continue; } @@ -84,7 +84,7 @@ void FuzzerPassInterchangeZeroLikeConstants::Apply() { } assert(!GetTransformationContext()->GetFactManager()->IdIsIrrelevant( - toggled_id) && + toggled_id, GetIRContext()) && "FindOrCreateToggledConstant can't produce an irrelevant id"); // Record synonymous constants diff --git a/source/fuzz/fuzzer_pass_push_ids_through_variables.cpp b/source/fuzz/fuzzer_pass_push_ids_through_variables.cpp index 8d9acaa0..07fe645a 100644 --- a/source/fuzz/fuzzer_pass_push_ids_through_variables.cpp +++ b/source/fuzz/fuzzer_pass_push_ids_through_variables.cpp @@ -96,7 +96,8 @@ void FuzzerPassPushIdsThroughVariables::Apply() { // able to produce a synonym out of the id. if (!GetTransformationContext() ->GetFactManager() - ->IdIsIrrelevant(instruction->result_id()) && + ->IdIsIrrelevant(instruction->result_id(), + GetIRContext()) && !fuzzerutil::CanMakeSynonymOf(ir_context, *GetTransformationContext(), instruction)) { diff --git a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp index cc92e283..c6bdd166 100644 --- a/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp +++ b/source/fuzz/fuzzer_pass_replace_irrelevant_ids.cpp @@ -46,8 +46,8 @@ void FuzzerPassReplaceIrrelevantIds::Apply() { // Find all the irrelevant ids that still exist in the module and all the // types for which irrelevant ids exist. - for (auto id : - GetTransformationContext()->GetFactManager()->GetIrrelevantIds()) { + for (auto id : GetTransformationContext()->GetFactManager()->GetIrrelevantIds( + GetIRContext())) { // Check that the id still exists in the module. auto declaration = GetIRContext()->get_def_use_mgr()->GetDef(id); if (!declaration) { diff --git a/source/fuzz/fuzzer_util.cpp b/source/fuzz/fuzzer_util.cpp index 2502da71..b550a741 100644 --- a/source/fuzz/fuzzer_util.cpp +++ b/source/fuzz/fuzzer_util.cpp @@ -33,7 +33,7 @@ uint32_t MaybeGetOpConstant(opt::IRContext* ir_context, if (inst.opcode() == SpvOpConstant && inst.type_id() == type_id && inst.GetInOperand(0).words == words && transformation_context.GetFactManager()->IdIsIrrelevant( - inst.result_id()) == is_irrelevant) { + inst.result_id(), ir_context) == is_irrelevant) { return inst.result_id(); } } @@ -261,8 +261,8 @@ bool CanMakeSynonymOf(opt::IRContext* ir_context, // We can only make a synonym of an instruction that generates an id. return false; } - if (transformation_context.GetFactManager()->IdIsIrrelevant( - inst->result_id())) { + if (transformation_context.GetFactManager()->IdIsIrrelevant(inst->result_id(), + ir_context)) { // An irrelevant id can't be a synonym of anything. return false; } @@ -1149,7 +1149,7 @@ uint32_t MaybeGetCompositeConstant( if (inst.opcode() == SpvOpConstantComposite && inst.type_id() == composite_type_id && transformation_context.GetFactManager()->IdIsIrrelevant( - inst.result_id()) == is_irrelevant && + inst.result_id(), ir_context) == is_irrelevant && inst.NumInOperands() == component_ids.size()) { bool is_match = true; @@ -1229,7 +1229,7 @@ uint32_t MaybeGetBoolConstant( if (inst.opcode() == (value ? SpvOpConstantTrue : SpvOpConstantFalse) && inst.type_id() == type_id && transformation_context.GetFactManager()->IdIsIrrelevant( - inst.result_id()) == is_irrelevant) { + inst.result_id(), ir_context) == is_irrelevant) { return inst.result_id(); } } diff --git a/source/fuzz/transformation_composite_construct.cpp b/source/fuzz/transformation_composite_construct.cpp index 15af53ed..c92349d7 100644 --- a/source/fuzz/transformation_composite_construct.cpp +++ b/source/fuzz/transformation_composite_construct.cpp @@ -96,7 +96,8 @@ bool TransformationCompositeConstruct::IsApplicable( // We should be able to create a synonym of |component| if it's not // irrelevant. - if (!transformation_context.GetFactManager()->IdIsIrrelevant(component) && + if (!transformation_context.GetFactManager()->IdIsIrrelevant(component, + ir_context) && !fuzzerutil::CanMakeSynonymOf(ir_context, transformation_context, inst)) { return false; @@ -159,7 +160,7 @@ void TransformationCompositeConstruct::Apply( subvector_index < component_type->AsVector()->element_count(); subvector_index++) { if (!transformation_context->GetFactManager()->IdIsIrrelevant( - component)) { + component, ir_context)) { transformation_context->GetFactManager()->AddFactDataSynonym( MakeDataDescriptor(component, {subvector_index}), MakeDataDescriptor(message_.fresh_id(), {index}), ir_context); @@ -170,7 +171,7 @@ void TransformationCompositeConstruct::Apply( // The other cases are simple: the component is made directly synonymous // with the element of the composite being constructed. if (!transformation_context->GetFactManager()->IdIsIrrelevant( - component)) { + component, ir_context)) { transformation_context->GetFactManager()->AddFactDataSynonym( MakeDataDescriptor(component, {}), MakeDataDescriptor(message_.fresh_id(), {index}), ir_context); diff --git a/source/fuzz/transformation_composite_extract.cpp b/source/fuzz/transformation_composite_extract.cpp index 9f4d5546..809454db 100644 --- a/source/fuzz/transformation_composite_extract.cpp +++ b/source/fuzz/transformation_composite_extract.cpp @@ -56,7 +56,7 @@ bool TransformationCompositeExtract::IsApplicable( return false; } if (!transformation_context.GetFactManager()->IdIsIrrelevant( - message_.composite_id()) && + message_.composite_id(), ir_context) && !fuzzerutil::CanMakeSynonymOf(ir_context, transformation_context, composite_instruction)) { // |composite_id| will participate in DataSynonym facts. Thus, it can't be @@ -115,7 +115,7 @@ void TransformationCompositeExtract::Apply( // Add the fact that the id storing the extracted element is synonymous with // the index into the structure. if (!transformation_context->GetFactManager()->IdIsIrrelevant( - message_.composite_id())) { + message_.composite_id(), ir_context)) { std::vector indices; for (auto an_index : message_.index()) { indices.push_back(an_index); diff --git a/source/fuzz/transformation_composite_insert.cpp b/source/fuzz/transformation_composite_insert.cpp index 75eebd43..e57ffe8c 100644 --- a/source/fuzz/transformation_composite_insert.cpp +++ b/source/fuzz/transformation_composite_insert.cpp @@ -143,7 +143,7 @@ void TransformationCompositeInsert::Apply( // If |composite_id| is irrelevant then don't add any synonyms. if (transformation_context->GetFactManager()->IdIsIrrelevant( - message_.composite_id())) { + message_.composite_id(), ir_context)) { return; } uint32_t current_node_type_id = composite_type_id; @@ -184,7 +184,7 @@ void TransformationCompositeInsert::Apply( // The element which has been changed is synonymous to the found object // itself. Add this fact only if |object_id| is not irrelevant. if (!transformation_context->GetFactManager()->IdIsIrrelevant( - message_.object_id())) { + message_.object_id(), ir_context)) { transformation_context->GetFactManager()->AddFactDataSynonym( MakeDataDescriptor(message_.object_id(), {}), MakeDataDescriptor(message_.fresh_id(), std::vector(index)), diff --git a/source/fuzz/transformation_equation_instruction.cpp b/source/fuzz/transformation_equation_instruction.cpp index e27cd297..ae6d2360 100644 --- a/source/fuzz/transformation_equation_instruction.cpp +++ b/source/fuzz/transformation_equation_instruction.cpp @@ -60,7 +60,8 @@ bool TransformationEquationInstruction::IsApplicable( if (inst->opcode() == SpvOpUndef) { return false; } - if (transformation_context.GetFactManager()->IdIsIrrelevant(id)) { + if (transformation_context.GetFactManager()->IdIsIrrelevant(id, + ir_context)) { return false; } if (!fuzzerutil::IdIsAvailableBeforeInstruction(ir_context, insert_before, diff --git a/source/fuzz/transformation_push_id_through_variable.cpp b/source/fuzz/transformation_push_id_through_variable.cpp index 647bfa06..167e83b2 100644 --- a/source/fuzz/transformation_push_id_through_variable.cpp +++ b/source/fuzz/transformation_push_id_through_variable.cpp @@ -76,7 +76,7 @@ bool TransformationPushIdThroughVariable::IsApplicable( // We should be able to create a synonym of |value_id| if it's not irrelevant. if (!transformation_context.GetFactManager()->IdIsIrrelevant( - message_.value_id()) && + message_.value_id(), ir_context) && !fuzzerutil::CanMakeSynonymOf(ir_context, transformation_context, value_instruction)) { return false; @@ -154,7 +154,7 @@ void TransformationPushIdThroughVariable::Apply( ir_context->InvalidateAnalysesExceptFor(opt::IRContext::kAnalysisNone); if (!transformation_context->GetFactManager()->IdIsIrrelevant( - message_.value_id())) { + message_.value_id(), ir_context)) { // Adds the fact that |message_.value_synonym_id| // and |message_.value_id| are synonymous. transformation_context->GetFactManager()->AddFactDataSynonym( diff --git a/source/fuzz/transformation_record_synonymous_constants.cpp b/source/fuzz/transformation_record_synonymous_constants.cpp index a93e1d42..87a9c51e 100644 --- a/source/fuzz/transformation_record_synonymous_constants.cpp +++ b/source/fuzz/transformation_record_synonymous_constants.cpp @@ -41,9 +41,9 @@ bool TransformationRecordSynonymousConstants::IsApplicable( } if (transformation_context.GetFactManager()->IdIsIrrelevant( - message_.constant1_id()) || + message_.constant1_id(), ir_context) || transformation_context.GetFactManager()->IdIsIrrelevant( - message_.constant2_id())) { + message_.constant2_id(), ir_context)) { return false; } diff --git a/source/fuzz/transformation_replace_irrelevant_id.cpp b/source/fuzz/transformation_replace_irrelevant_id.cpp index 5ac182a7..84cd8498 100644 --- a/source/fuzz/transformation_replace_irrelevant_id.cpp +++ b/source/fuzz/transformation_replace_irrelevant_id.cpp @@ -37,8 +37,8 @@ bool TransformationReplaceIrrelevantId::IsApplicable( auto id_of_interest = message_.id_use_descriptor().id_of_interest(); // The id must be irrelevant. - if (!transformation_context.GetFactManager()->IdIsIrrelevant( - id_of_interest)) { + if (!transformation_context.GetFactManager()->IdIsIrrelevant(id_of_interest, + ir_context)) { return false; } diff --git a/source/fuzz/transformation_replace_parameter_with_global.cpp b/source/fuzz/transformation_replace_parameter_with_global.cpp index 489d3394..7b0abea1 100644 --- a/source/fuzz/transformation_replace_parameter_with_global.cpp +++ b/source/fuzz/transformation_replace_parameter_with_global.cpp @@ -184,7 +184,7 @@ void TransformationReplaceParameterWithGlobal::Apply( // Mark the pointee of the global variable storing the parameter's value as // irrelevant if replaced parameter is irrelevant. if (transformation_context->GetFactManager()->IdIsIrrelevant( - message_.parameter_id())) { + message_.parameter_id(), ir_context)) { transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( message_.global_variable_fresh_id(), ir_context); } diff --git a/source/fuzz/transformation_vector_shuffle.cpp b/source/fuzz/transformation_vector_shuffle.cpp index b3bd5935..45641072 100644 --- a/source/fuzz/transformation_vector_shuffle.cpp +++ b/source/fuzz/transformation_vector_shuffle.cpp @@ -59,7 +59,7 @@ bool TransformationVectorShuffle::IsApplicable( } // We should be able to create a synonym of |vector1| if it's not irrelevant. if (!transformation_context.GetFactManager()->IdIsIrrelevant( - message_.vector1()) && + message_.vector1(), ir_context) && !fuzzerutil::CanMakeSynonymOf(ir_context, transformation_context, vector1_instruction)) { return false; @@ -72,7 +72,7 @@ bool TransformationVectorShuffle::IsApplicable( } // We should be able to create a synonym of |vector2| if it's not irrelevant. if (!transformation_context.GetFactManager()->IdIsIrrelevant( - message_.vector2()) && + message_.vector2(), ir_context) && !fuzzerutil::CanMakeSynonymOf(ir_context, transformation_context, vector2_instruction)) { return false; @@ -153,6 +153,13 @@ void TransformationVectorShuffle::Apply( ir_context->InvalidateAnalysesExceptFor( opt::IRContext::Analysis::kAnalysisNone); + // If the new instruction is irrelevant (because it is in a dead block), it + // cannot participate in any DataSynonym fact. + if (transformation_context->GetFactManager()->IdIsIrrelevant( + message_.fresh_id(), ir_context)) { + return; + } + // Add synonym facts relating the defined elements of the shuffle result to // the vector components that they come from. for (uint32_t component_index = 0; @@ -178,7 +185,7 @@ void TransformationVectorShuffle::Apply( GetVectorType(ir_context, message_.vector1())->element_count()) { // Irrelevant id cannot participate in DataSynonym facts. if (transformation_context->GetFactManager()->IdIsIrrelevant( - message_.vector1())) { + message_.vector1(), ir_context)) { continue; } @@ -187,7 +194,7 @@ void TransformationVectorShuffle::Apply( } else { // Irrelevant id cannot participate in DataSynonym facts. if (transformation_context->GetFactManager()->IdIsIrrelevant( - message_.vector2())) { + message_.vector2(), ir_context)) { continue; } diff --git a/test/fuzz/fact_manager_test.cpp b/test/fuzz/fact_manager_test.cpp index 64104df2..ad7e52a8 100644 --- a/test/fuzz/fact_manager_test.cpp +++ b/test/fuzz/fact_manager_test.cpp @@ -1440,13 +1440,13 @@ TEST(FactManagerTest, IdIsIrrelevant) { FactManager fact_manager; - ASSERT_FALSE(fact_manager.IdIsIrrelevant(12)); - ASSERT_FALSE(fact_manager.IdIsIrrelevant(13)); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(12, context.get())); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(13, context.get())); fact_manager.AddFactIdIsIrrelevant(12, context.get()); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(12)); - ASSERT_FALSE(fact_manager.IdIsIrrelevant(13)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(12, context.get())); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(13, context.get())); } TEST(FactManagerTest, GetIrrelevantIds) { @@ -1476,20 +1476,136 @@ TEST(FactManagerTest, GetIrrelevantIds) { FactManager fact_manager; - ASSERT_TRUE(fact_manager.GetIrrelevantIds() == - std::unordered_set({})); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({})); fact_manager.AddFactIdIsIrrelevant(12, context.get()); - ASSERT_TRUE(fact_manager.GetIrrelevantIds() == - std::unordered_set({12})); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({12})); fact_manager.AddFactIdIsIrrelevant(13, context.get()); - ASSERT_TRUE(fact_manager.GetIrrelevantIds() == - std::unordered_set({12, 13})); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({12, 13})); } +TEST(FactManagerTest, BlockIsDead) { + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "main" + OpExecutionMode %2 OriginUpperLeft + OpSource ESSL 310 + %3 = OpTypeVoid + %4 = OpTypeFunction %3 + %5 = OpTypeBool + %6 = OpConstantTrue %5 + %7 = OpTypeInt 32 1 + %8 = OpTypePointer Function %7 + %2 = OpFunction %3 None %4 + %9 = OpLabel + OpSelectionMerge %10 None + OpBranchConditional %6 %11 %12 + %11 = OpLabel + OpBranch %10 + %12 = OpLabel + OpBranch %10 + %10 = 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())); + + FactManager fact_manager; + + ASSERT_FALSE(fact_manager.BlockIsDead(9)); + ASSERT_FALSE(fact_manager.BlockIsDead(11)); + ASSERT_FALSE(fact_manager.BlockIsDead(12)); + + fact_manager.AddFactBlockIsDead(12); + + ASSERT_FALSE(fact_manager.BlockIsDead(9)); + ASSERT_FALSE(fact_manager.BlockIsDead(11)); + ASSERT_TRUE(fact_manager.BlockIsDead(12)); +} + +TEST(FactManagerTest, IdsFromDeadBlocksAreIrrelevant) { + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "main" + OpExecutionMode %2 OriginUpperLeft + OpSource ESSL 310 + %3 = OpTypeVoid + %4 = OpTypeFunction %3 + %5 = OpTypeBool + %6 = OpConstantTrue %5 + %7 = OpTypeInt 32 1 + %8 = OpTypePointer Function %7 + %9 = OpConstant %7 1 + %2 = OpFunction %3 None %4 + %10 = OpLabel + %11 = OpVariable %8 Function + OpSelectionMerge %12 None + OpBranchConditional %6 %13 %14 + %13 = OpLabel + OpBranch %12 + %14 = OpLabel + %15 = OpCopyObject %8 %11 + %16 = OpCopyObject %7 %9 + %17 = OpFunctionCall %3 %18 + OpBranch %12 + %12 = OpLabel + OpReturn + OpFunctionEnd + %18 = OpFunction %3 None %4 + %19 = OpLabel + %20 = OpVariable %8 Function + %21 = OpCopyObject %7 %9 + 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())); + + FactManager fact_manager; + + ASSERT_FALSE(fact_manager.BlockIsDead(14)); + ASSERT_FALSE(fact_manager.BlockIsDead(19)); + + // Initially no id is irrelevant. + ASSERT_FALSE(fact_manager.IdIsIrrelevant(16, context.get())); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(17, context.get())); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({})); + + fact_manager.AddFactBlockIsDead(14); + + // %16 and %17 should now be considered irrelevant. + ASSERT_TRUE(fact_manager.IdIsIrrelevant(16, context.get())); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(17, context.get())); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({16, 17})); + + // Similarly for %21. + ASSERT_FALSE(fact_manager.IdIsIrrelevant(21, context.get())); + + fact_manager.AddFactBlockIsDead(19); + + ASSERT_TRUE(fact_manager.IdIsIrrelevant(21, context.get())); + ASSERT_EQ(fact_manager.GetIrrelevantIds(context.get()), + std::unordered_set({16, 17, 21})); +} } // namespace } // namespace fuzz } // namespace spvtools diff --git a/test/fuzz/transformation_add_constant_boolean_test.cpp b/test/fuzz/transformation_add_constant_boolean_test.cpp index 1a403295..958232c8 100644 --- a/test/fuzz/transformation_add_constant_boolean_test.cpp +++ b/test/fuzz/transformation_add_constant_boolean_test.cpp @@ -107,10 +107,10 @@ TEST(TransformationAddConstantBooleanTest, NeitherPresentInitiallyAddBoth) { irrelevant_false.Apply(context.get(), &transformation_context); ASSERT_TRUE(IsValid(env, context.get())); - ASSERT_FALSE(fact_manager.IdIsIrrelevant(100)); - ASSERT_FALSE(fact_manager.IdIsIrrelevant(101)); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(102)); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(103)); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(100, context.get())); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(101, context.get())); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(102, context.get())); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(103, context.get())); std::string after_transformation = R"( OpCapability Shader diff --git a/test/fuzz/transformation_add_constant_composite_test.cpp b/test/fuzz/transformation_add_constant_composite_test.cpp index 75e23ada..fdd68ca4 100644 --- a/test/fuzz/transformation_add_constant_composite_test.cpp +++ b/test/fuzz/transformation_add_constant_composite_test.cpp @@ -135,11 +135,11 @@ TEST(TransformationAddConstantCompositeTest, BasicTest) { ASSERT_TRUE(IsValid(env, context.get())); for (uint32_t id = 100; id <= 106; ++id) { - ASSERT_FALSE(fact_manager.IdIsIrrelevant(id)); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(id, context.get())); } for (uint32_t id = 107; id <= 113; ++id) { - ASSERT_TRUE(fact_manager.IdIsIrrelevant(id)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(id, context.get())); } std::string after_transformation = R"( diff --git a/test/fuzz/transformation_add_constant_scalar_test.cpp b/test/fuzz/transformation_add_constant_scalar_test.cpp index d75ac7a1..3aacf419 100644 --- a/test/fuzz/transformation_add_constant_scalar_test.cpp +++ b/test/fuzz/transformation_add_constant_scalar_test.cpp @@ -272,11 +272,11 @@ TEST(TransformationAddConstantScalarTest, Apply) { ASSERT_TRUE(IsValid(env, context.get())); for (uint32_t result_id = 19; result_id <= 24; ++result_id) { - ASSERT_FALSE(fact_manager.IdIsIrrelevant(result_id)); + ASSERT_FALSE(fact_manager.IdIsIrrelevant(result_id, context.get())); } for (uint32_t result_id = 25; result_id <= 30; ++result_id) { - ASSERT_TRUE(fact_manager.IdIsIrrelevant(result_id)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(result_id, context.get())); } std::string variant_shader = R"( diff --git a/test/fuzz/transformation_add_parameter_test.cpp b/test/fuzz/transformation_add_parameter_test.cpp index 4826b950..ce738884 100644 --- a/test/fuzz/transformation_add_parameter_test.cpp +++ b/test/fuzz/transformation_add_parameter_test.cpp @@ -136,28 +136,28 @@ TEST(TransformationAddParameterTest, NonPointerBasicTest) { ASSERT_TRUE(correct.IsApplicable(context.get(), transformation_context)); correct.Apply(context.get(), &transformation_context); ASSERT_TRUE(IsValid(env, context.get())); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(60)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(60, context.get())); } { TransformationAddParameter correct(17, 62, 7, {{}}, 63); ASSERT_TRUE(correct.IsApplicable(context.get(), transformation_context)); correct.Apply(context.get(), &transformation_context); ASSERT_TRUE(IsValid(env, context.get())); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(62)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(62, context.get())); } { TransformationAddParameter correct(29, 64, 31, {{}}, 65); ASSERT_TRUE(correct.IsApplicable(context.get(), transformation_context)); correct.Apply(context.get(), &transformation_context); ASSERT_TRUE(IsValid(env, context.get())); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(64)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(64, context.get())); } { TransformationAddParameter correct(34, 66, 7, {{}}, 67); ASSERT_TRUE(correct.IsApplicable(context.get(), transformation_context)); correct.Apply(context.get(), &transformation_context); ASSERT_TRUE(IsValid(env, context.get())); - ASSERT_TRUE(fact_manager.IdIsIrrelevant(66)); + ASSERT_TRUE(fact_manager.IdIsIrrelevant(66, context.get())); } std::string expected_shader = R"( diff --git a/test/fuzz/transformation_flatten_conditional_branch_test.cpp b/test/fuzz/transformation_flatten_conditional_branch_test.cpp index 095acbc3..91802aeb 100644 --- a/test/fuzz/transformation_flatten_conditional_branch_test.cpp +++ b/test/fuzz/transformation_flatten_conditional_branch_test.cpp @@ -490,7 +490,8 @@ TEST(TransformationFlattenConditionalBranchTest, LoadStoreFunctionCall) { transformation1.Apply(context.get(), &transformation_context); // Check that the placeholder id was marked as irrelevant. - ASSERT_TRUE(transformation_context.GetFactManager()->IdIsIrrelevant(103)); + ASSERT_TRUE(transformation_context.GetFactManager()->IdIsIrrelevant( + 103, context.get())); // Make a new transformation context with a source of overflow ids. TransformationContext new_transformation_context(