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 877a38c67f..a046a33155 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 @@ -52,9 +52,13 @@ bool DataSynonymAndIdEquationFacts::OperationEquals::operator()( } void DataSynonymAndIdEquationFacts::AddFact( - const protobufs::FactDataSynonym& fact, opt::IRContext* context) { - // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550) - // Assert that ids are not irrelevant. + const protobufs::FactDataSynonym& fact, + const IrrelevantValueFacts& irrelevant_value_facts, + opt::IRContext* context) { + (void)irrelevant_value_facts; // Keep release compilers happy. + assert(!irrelevant_value_facts.IdIsIrrelevant(fact.data1().object()) && + !irrelevant_value_facts.IdIsIrrelevant(fact.data2().object()) && + "Irrelevant ids cannot be synonymous with other ids."); // Add the fact, including all facts relating sub-components of the data // descriptors that are involved. @@ -62,9 +66,12 @@ void DataSynonymAndIdEquationFacts::AddFact( } void DataSynonymAndIdEquationFacts::AddFact( - const protobufs::FactIdEquation& fact, opt::IRContext* context) { - // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550) - // Assert that ids are not irrelevant. + const protobufs::FactIdEquation& fact, + const IrrelevantValueFacts& irrelevant_value_facts, + opt::IRContext* context) { + (void)irrelevant_value_facts; // Keep release compilers happy. + assert(!irrelevant_value_facts.IdIsIrrelevant(fact.lhs_id()) && + "Irrelevant ids are not allowed."); protobufs::DataDescriptor lhs_dd = MakeDataDescriptor(fact.lhs_id(), {}); @@ -75,8 +82,8 @@ void DataSynonymAndIdEquationFacts::AddFact( // equation. std::vector rhs_dds; for (auto rhs_id : fact.rhs_id()) { - // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550) - // Assert that ids are not irrelevant. + assert(!irrelevant_value_facts.IdIsIrrelevant(rhs_id) && + "Irrelevant ids are not allowed."); // Register a data descriptor based on this id in the equivalence relation // if needed, and then record the equivalence class representative. @@ -828,6 +835,11 @@ bool DataSynonymAndIdEquationFacts::DataDescriptorsAreWellFormedAndComparable( get_bit_count_for_numeric_type(*type_b)); } +std::vector +DataSynonymAndIdEquationFacts::GetSynonymsForId(uint32_t id) const { + return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {})); +} + std::vector DataSynonymAndIdEquationFacts::GetSynonymsForDataDescriptor( const protobufs::DataDescriptor& data_descriptor) const { 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 fb15e2ce84..df35ad64ef 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,15 +27,28 @@ namespace spvtools { namespace fuzz { namespace fact_manager { +// Forward reference to the IrrelevantValueFacts class. +class IrrelevantValueFacts; + // The purpose of this class is to group the fields and data used to represent // facts about data synonyms and id equations. class DataSynonymAndIdEquationFacts { public: // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactDataSynonym& fact, opt::IRContext* context); + // |irrelevant_value_facts| is passed for consistency checks. + void AddFact(const protobufs::FactDataSynonym& fact, + 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. + void AddFact(const protobufs::FactIdEquation& fact, + const IrrelevantValueFacts& irrelevant_value_facts, + opt::IRContext* context); // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactIdEquation& fact, opt::IRContext* context); + std::vector GetSynonymsForId( + uint32_t id) const; // See method in FactManager which delegates to this method. std::vector GetSynonymsForDataDescriptor( diff --git a/source/fuzz/fact_manager/fact_manager.cpp b/source/fuzz/fact_manager/fact_manager.cpp index b2886bcef5..79c203701b 100644 --- a/source/fuzz/fact_manager/fact_manager.cpp +++ b/source/fuzz/fact_manager/fact_manager.cpp @@ -105,8 +105,8 @@ bool FactManager::AddFact(const fuzz::protobufs::Fact& fact, return constant_uniform_facts_.AddFact(fact.constant_uniform_fact(), context); case protobufs::Fact::kDataSynonymFact: - data_synonym_and_id_equation_facts_.AddFact(fact.data_synonym_fact(), - context); + data_synonym_and_id_equation_facts_.AddFact( + fact.data_synonym_fact(), irrelevant_value_facts_, context); return true; case protobufs::Fact::kBlockIsDeadFact: dead_block_facts_.AddFact(fact.block_is_dead_fact()); @@ -126,7 +126,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, context); + data_synonym_and_id_equation_facts_.AddFact(fact, irrelevant_value_facts_, + context); } std::vector FactManager::GetConstantsAvailableFromUniformsForType( @@ -172,7 +173,7 @@ FactManager::GetSynonymsForDataDescriptor( std::vector FactManager::GetSynonymsForId( uint32_t id) const { - return GetSynonymsForDataDescriptor(MakeDataDescriptor(id, {})); + return data_synonym_and_id_equation_facts_.GetSynonymsForId(id); } bool FactManager::IsSynonymous( @@ -214,16 +215,20 @@ const std::unordered_set& FactManager::GetIrrelevantIds() const { return irrelevant_value_facts_.GetIrrelevantIds(); } -void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id) { +void FactManager::AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id, + opt::IRContext* context) { protobufs::FactPointeeValueIsIrrelevant fact; fact.set_pointer_id(pointer_id); - irrelevant_value_facts_.AddFact(fact); + irrelevant_value_facts_.AddFact(fact, data_synonym_and_id_equation_facts_, + context); } -void FactManager::AddFactIdIsIrrelevant(uint32_t result_id) { +void FactManager::AddFactIdIsIrrelevant(uint32_t result_id, + opt::IRContext* context) { protobufs::FactIdIsIrrelevant fact; fact.set_result_id(result_id); - irrelevant_value_facts_.AddFact(fact); + irrelevant_value_facts_.AddFact(fact, data_synonym_and_id_equation_facts_, + context); } void FactManager::AddFactIdEquation(uint32_t lhs_id, SpvOp opcode, @@ -235,7 +240,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, context); + data_synonym_and_id_equation_facts_.AddFact(fact, 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 a8268d929f..b1f4f35229 100644 --- a/source/fuzz/fact_manager/fact_manager.h +++ b/source/fuzz/fact_manager/fact_manager.h @@ -66,11 +66,14 @@ class FactManager { // Records the fact that the value of the pointee associated with |pointer_id| // is irrelevant: it does not affect the observable behaviour of the module. - void AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id); + // |pointer_id| must exist in the module and actually be a pointer. + void AddFactValueOfPointeeIsIrrelevant(uint32_t pointer_id, + opt::IRContext* context); // Records a fact that the |result_id| is irrelevant (i.e. it doesn't affect - // the semantics of the module) - void AddFactIdIsIrrelevant(uint32_t result_id); + // the semantics of the module). + // |result_id| must exist in the module and actually be a pointer. + void AddFactIdIsIrrelevant(uint32_t result_id, opt::IRContext* context); // Records the fact that |lhs_id| is defined by the equation: // diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.cpp b/source/fuzz/fact_manager/irrelevant_value_facts.cpp index 16146f7803..dda17b3d63 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.cpp +++ b/source/fuzz/fact_manager/irrelevant_value_facts.cpp @@ -15,25 +15,43 @@ #include "source/fuzz/fact_manager/irrelevant_value_facts.h" #include "source/fuzz/data_descriptor.h" -#include "source/fuzz/fuzzer_util.h" +#include "source/fuzz/fact_manager/data_synonym_and_id_equation_facts.h" +#include "source/opt/ir_context.h" namespace spvtools { namespace fuzz { namespace fact_manager { void IrrelevantValueFacts::AddFact( - const protobufs::FactPointeeValueIsIrrelevant& fact) { - // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550) - // Assert that the id does not participate in DataSynonym facts and is a - // pointer. + const protobufs::FactPointeeValueIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts, + opt::IRContext* context) { + (void)data_synonym_and_id_equation_facts; // Keep release compilers happy. + assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.pointer_id()) + .empty() && + "The id cannot participate in DataSynonym facts."); + auto pointer_def = context->get_def_use_mgr()->GetDef(fact.pointer_id()); + assert(pointer_def && "The id must exist in the module."); + auto type = context->get_type_mgr()->GetType(pointer_def->type_id()); + (void)type; // Keep release compilers happy. + assert(type && type->AsPointer() && "The id must be a pointer."); pointers_to_irrelevant_pointees_ids_.insert(fact.pointer_id()); } -void IrrelevantValueFacts::AddFact(const protobufs::FactIdIsIrrelevant& fact) { - // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3550) - // Assert that the id does not participate in DataSynonym facts and is not a - // pointer. +void IrrelevantValueFacts::AddFact( + const protobufs::FactIdIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts, + opt::IRContext* context) { + (void)data_synonym_and_id_equation_facts; // Keep release compilers happy. + assert(data_synonym_and_id_equation_facts.GetSynonymsForId(fact.result_id()) + .empty() && + "The id cannot participate in DataSynonym facts."); + auto pointer_def = context->get_def_use_mgr()->GetDef(fact.result_id()); + assert(pointer_def && "The id must exist in the module."); + auto type = context->get_type_mgr()->GetType(pointer_def->type_id()); + (void)type; // Keep release compilers happy. + assert(type && !type->AsPointer() && "The id must not be a pointer."); irrelevant_ids_.insert(fact.result_id()); } diff --git a/source/fuzz/fact_manager/irrelevant_value_facts.h b/source/fuzz/fact_manager/irrelevant_value_facts.h index 4e04faf89a..76ff55d50a 100644 --- a/source/fuzz/fact_manager/irrelevant_value_facts.h +++ b/source/fuzz/fact_manager/irrelevant_value_facts.h @@ -24,15 +24,28 @@ namespace spvtools { namespace fuzz { namespace fact_manager { +// Forward reference to the DataSynonymAndIdEquationFacts class. +class DataSynonymAndIdEquationFacts; + // The purpose of this class is to group the fields and data used to represent // facts about various irrelevant values in the module. class IrrelevantValueFacts { public: // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactPointeeValueIsIrrelevant& fact); + // |data_synonym_and_id_equation_facts| and |context| are passed for + // consistency checks. + void AddFact( + const protobufs::FactPointeeValueIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts, + opt::IRContext* context); // See method in FactManager which delegates to this method. - void AddFact(const protobufs::FactIdIsIrrelevant& fact); + // |data_synonym_and_id_equation_facts| and |context| are passed for + // consistency checks. + void AddFact( + const protobufs::FactIdIsIrrelevant& fact, + const DataSynonymAndIdEquationFacts& data_synonym_and_id_equation_facts, + opt::IRContext* context); // See method in FactManager which delegates to this method. bool PointeeValueIsIrrelevant(uint32_t pointer_id) const; diff --git a/source/fuzz/transformation_access_chain.cpp b/source/fuzz/transformation_access_chain.cpp index 33668694f9..a12b7a2fc8 100644 --- a/source/fuzz/transformation_access_chain.cpp +++ b/source/fuzz/transformation_access_chain.cpp @@ -339,7 +339,7 @@ void TransformationAccessChain::Apply( if (transformation_context->GetFactManager()->PointeeValueIsIrrelevant( message_.pointer_id())) { transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.fresh_id()); + message_.fresh_id(), ir_context); } } diff --git a/source/fuzz/transformation_add_constant_boolean.cpp b/source/fuzz/transformation_add_constant_boolean.cpp index 904ad61042..4e94b20c41 100644 --- a/source/fuzz/transformation_add_constant_boolean.cpp +++ b/source/fuzz/transformation_add_constant_boolean.cpp @@ -53,7 +53,7 @@ void TransformationAddConstantBoolean::Apply( if (message_.is_irrelevant()) { transformation_context->GetFactManager()->AddFactIdIsIrrelevant( - message_.fresh_id()); + message_.fresh_id(), ir_context); } } diff --git a/source/fuzz/transformation_add_constant_composite.cpp b/source/fuzz/transformation_add_constant_composite.cpp index 99d88b40b1..36626f557e 100644 --- a/source/fuzz/transformation_add_constant_composite.cpp +++ b/source/fuzz/transformation_add_constant_composite.cpp @@ -122,7 +122,7 @@ void TransformationAddConstantComposite::Apply( if (message_.is_irrelevant()) { transformation_context->GetFactManager()->AddFactIdIsIrrelevant( - message_.fresh_id()); + message_.fresh_id(), ir_context); } } diff --git a/source/fuzz/transformation_add_constant_scalar.cpp b/source/fuzz/transformation_add_constant_scalar.cpp index e0b4dfbad8..3823a60958 100644 --- a/source/fuzz/transformation_add_constant_scalar.cpp +++ b/source/fuzz/transformation_add_constant_scalar.cpp @@ -80,7 +80,7 @@ void TransformationAddConstantScalar::Apply( if (message_.is_irrelevant()) { transformation_context->GetFactManager()->AddFactIdIsIrrelevant( - message_.fresh_id()); + message_.fresh_id(), ir_context); } } diff --git a/source/fuzz/transformation_add_copy_memory.cpp b/source/fuzz/transformation_add_copy_memory.cpp index e9c401d737..9a679cf9d8 100644 --- a/source/fuzz/transformation_add_copy_memory.cpp +++ b/source/fuzz/transformation_add_copy_memory.cpp @@ -132,6 +132,10 @@ void TransformationAddCopyMemory::Apply( fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id()); + // Make sure our changes are analyzed + ir_context->InvalidateAnalysesExceptFor( + opt::IRContext::Analysis::kAnalysisNone); + // Even though the copy memory instruction will - at least temporarily - lead // to the destination and source pointers referring to identical values, this // fact is not guaranteed to hold throughout execution of the SPIR-V code @@ -139,11 +143,7 @@ void TransformationAddCopyMemory::Apply( // about the destination pointer, and record this fact so that the destination // pointer can be used freely by other fuzzer passes. transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.fresh_id()); - - // Make sure our changes are analyzed - ir_context->InvalidateAnalysesExceptFor( - opt::IRContext::Analysis::kAnalysisNone); + message_.fresh_id(), ir_context); } protobufs::Transformation TransformationAddCopyMemory::ToMessage() const { diff --git a/source/fuzz/transformation_add_function.cpp b/source/fuzz/transformation_add_function.cpp index 44c7d05393..ff9a9b53b9 100644 --- a/source/fuzz/transformation_add_function.cpp +++ b/source/fuzz/transformation_add_function.cpp @@ -168,30 +168,6 @@ void TransformationAddFunction::Apply( (void)(success); // Keep release builds happy (otherwise they may complain // that |success| is not used). - // Record the fact that all pointer parameters and variables declared in the - // function should be regarded as having irrelevant values. This allows other - // passes to store arbitrarily to such variables, and to pass them freely as - // parameters to other functions knowing that it is OK if they get - // over-written. - for (auto& instruction : message_.instruction()) { - switch (instruction.opcode()) { - case SpvOpFunctionParameter: - if (ir_context->get_def_use_mgr() - ->GetDef(instruction.result_type_id()) - ->opcode() == SpvOpTypePointer) { - transformation_context->GetFactManager() - ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id()); - } - break; - case SpvOpVariable: - transformation_context->GetFactManager() - ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id()); - break; - default: - break; - } - } - if (message_.is_livesafe()) { // Make the function livesafe, which also should succeed. success = TryToMakeFunctionLivesafe(ir_context, *transformation_context); @@ -214,6 +190,32 @@ void TransformationAddFunction::Apply( } } ir_context->InvalidateAnalysesExceptFor(opt::IRContext::kAnalysisNone); + + // Record the fact that all pointer parameters and variables declared in the + // function should be regarded as having irrelevant values. This allows other + // passes to store arbitrarily to such variables, and to pass them freely as + // parameters to other functions knowing that it is OK if they get + // over-written. + for (auto& instruction : message_.instruction()) { + switch (instruction.opcode()) { + case SpvOpFunctionParameter: + if (ir_context->get_def_use_mgr() + ->GetDef(instruction.result_type_id()) + ->opcode() == SpvOpTypePointer) { + transformation_context->GetFactManager() + ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id(), + ir_context); + } + break; + case SpvOpVariable: + transformation_context->GetFactManager() + ->AddFactValueOfPointeeIsIrrelevant(instruction.result_id(), + ir_context); + break; + default: + break; + } + } } protobufs::Transformation TransformationAddFunction::ToMessage() const { diff --git a/source/fuzz/transformation_add_global_variable.cpp b/source/fuzz/transformation_add_global_variable.cpp index 303c4d9778..4ecdb63a26 100644 --- a/source/fuzz/transformation_add_global_variable.cpp +++ b/source/fuzz/transformation_add_global_variable.cpp @@ -98,15 +98,15 @@ void TransformationAddGlobalVariable::Apply( static_cast(message_.storage_class()), message_.initializer_id()); - if (message_.value_is_irrelevant()) { - transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.fresh_id()); - } - // We have added an instruction to the module, so need to be careful about the // validity of existing analyses. ir_context->InvalidateAnalysesExceptFor( opt::IRContext::Analysis::kAnalysisNone); + + if (message_.value_is_irrelevant()) { + transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( + message_.fresh_id(), ir_context); + } } protobufs::Transformation TransformationAddGlobalVariable::ToMessage() const { diff --git a/source/fuzz/transformation_add_local_variable.cpp b/source/fuzz/transformation_add_local_variable.cpp index a6b31b4913..25ea0725ae 100644 --- a/source/fuzz/transformation_add_local_variable.cpp +++ b/source/fuzz/transformation_add_local_variable.cpp @@ -74,11 +74,12 @@ void TransformationAddLocalVariable::Apply( message_.type_id(), message_.function_id(), message_.initializer_id()); + ir_context->InvalidateAnalysesExceptFor(opt::IRContext::kAnalysisNone); + if (message_.value_is_irrelevant()) { transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.fresh_id()); + message_.fresh_id(), ir_context); } - ir_context->InvalidateAnalysesExceptFor(opt::IRContext::kAnalysisNone); } protobufs::Transformation TransformationAddLocalVariable::ToMessage() const { diff --git a/source/fuzz/transformation_add_parameter.cpp b/source/fuzz/transformation_add_parameter.cpp index 456e5ff6df..d0b80eb2a1 100644 --- a/source/fuzz/transformation_add_parameter.cpp +++ b/source/fuzz/transformation_add_parameter.cpp @@ -125,20 +125,6 @@ void TransformationAddParameter::Apply( fuzzerutil::UpdateModuleIdBound(ir_context, message_.parameter_fresh_id()); - // If the |new_parameter_type_id| is not a pointer type, mark id as - // irrelevant so that we can replace its use with some other id. If the - // |new_parameter_type_id| is a pointer type, we cannot mark it with - // IdIsIrrelevant, because this pointer might be replaced by a pointer from - // original shader. This would change the semantics of the module. In the case - // of a pointer type we mark it with PointeeValueIsIrrelevant. - if (new_parameter_type->kind() != opt::analysis::Type::kPointer) { - transformation_context->GetFactManager()->AddFactIdIsIrrelevant( - message_.parameter_fresh_id()); - } else { - transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.parameter_fresh_id()); - } - // Fix all OpFunctionCall instructions. for (auto* inst : fuzzerutil::GetCallers(ir_context, function->result_id())) { inst->AddOperand( @@ -167,9 +153,25 @@ void TransformationAddParameter::Apply( old_function_type->GetSingleWordInOperand(0), parameter_type_ids); } + auto new_parameter_kind = new_parameter_type->kind(); + // Make sure our changes are analyzed. ir_context->InvalidateAnalysesExceptFor( opt::IRContext::Analysis::kAnalysisNone); + + // If the |new_parameter_type_id| is not a pointer type, mark id as + // irrelevant so that we can replace its use with some other id. If the + // |new_parameter_type_id| is a pointer type, we cannot mark it with + // IdIsIrrelevant, because this pointer might be replaced by a pointer from + // original shader. This would change the semantics of the module. In the case + // of a pointer type we mark it with PointeeValueIsIrrelevant. + if (new_parameter_kind != opt::analysis::Type::kPointer) { + transformation_context->GetFactManager()->AddFactIdIsIrrelevant( + message_.parameter_fresh_id(), ir_context); + } else { + transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( + message_.parameter_fresh_id(), ir_context); + } } protobufs::Transformation TransformationAddParameter::ToMessage() const { diff --git a/source/fuzz/transformation_add_synonym.cpp b/source/fuzz/transformation_add_synonym.cpp index 6a93e61b97..1deaeef80e 100644 --- a/source/fuzz/transformation_add_synonym.cpp +++ b/source/fuzz/transformation_add_synonym.cpp @@ -109,7 +109,7 @@ void TransformationAddSynonym::Apply( message_.result_id()) && new_synonym_type->AsPointer()) { transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.synonym_fresh_id()); + message_.synonym_fresh_id(), ir_context); } // Mark two ids as synonymous. diff --git a/source/fuzz/transformation_outline_function.cpp b/source/fuzz/transformation_outline_function.cpp index 8534507a00..bf66b41fd4 100644 --- a/source/fuzz/transformation_outline_function.cpp +++ b/source/fuzz/transformation_outline_function.cpp @@ -632,16 +632,27 @@ TransformationOutlineFunction::PrepareFunctionPrototype( // Add one parameter to the function for each input id, using the fresh ids // provided in |input_id_to_fresh_id_map|, or overflow ids if needed. for (auto id : region_input_ids) { + uint32_t fresh_id = input_id_to_fresh_id_map.at(id); outlined_function->AddParameter(MakeUnique( ir_context, SpvOpFunctionParameter, - ir_context->get_def_use_mgr()->GetDef(id)->type_id(), - input_id_to_fresh_id_map.at(id), opt::Instruction::OperandList())); + ir_context->get_def_use_mgr()->GetDef(id)->type_id(), fresh_id, + opt::Instruction::OperandList())); + + // Analyse the use of the new parameter instruction. + outlined_function->ForEachParam( + [fresh_id, ir_context](opt::Instruction* inst) { + if (inst->result_id() == fresh_id) { + ir_context->AnalyzeDefUse(inst); + } + }); + // If the input id is an irrelevant-valued variable, the same should be true // of the corresponding parameter. if (transformation_context->GetFactManager()->PointeeValueIsIrrelevant( id)) { transformation_context->GetFactManager() - ->AddFactValueOfPointeeIsIrrelevant(input_id_to_fresh_id_map.at(id)); + ->AddFactValueOfPointeeIsIrrelevant(input_id_to_fresh_id_map.at(id), + ir_context); } } diff --git a/source/fuzz/transformation_replace_parameter_with_global.cpp b/source/fuzz/transformation_replace_parameter_with_global.cpp index 3f54d5774f..489d339404 100644 --- a/source/fuzz/transformation_replace_parameter_with_global.cpp +++ b/source/fuzz/transformation_replace_parameter_with_global.cpp @@ -99,14 +99,6 @@ void TransformationReplaceParameterWithGlobal::Apply( fuzzerutil::MaybeGetZeroConstant(ir_context, *transformation_context, param_inst->type_id(), false)); - // Mark the global variable's pointee as irrelevant if replaced parameter is - // irrelevant. - if (transformation_context->GetFactManager()->IdIsIrrelevant( - message_.parameter_id())) { - transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - message_.global_variable_fresh_id()); - } - auto* function = fuzzerutil::GetFunctionFromParameterId( ir_context, message_.parameter_id()); assert(function && "Function must exist"); @@ -188,6 +180,14 @@ void TransformationReplaceParameterWithGlobal::Apply( // Make sure our changes are analyzed ir_context->InvalidateAnalysesExceptFor( opt::IRContext::Analysis::kAnalysisNone); + + // 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())) { + transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant( + message_.global_variable_fresh_id(), ir_context); + } } protobufs::Transformation TransformationReplaceParameterWithGlobal::ToMessage() diff --git a/test/fuzz/fact_manager_test.cpp b/test/fuzz/fact_manager_test.cpp index 6f08ccc059..26b9eccddb 100644 --- a/test/fuzz/fact_manager_test.cpp +++ b/test/fuzz/fact_manager_test.cpp @@ -1370,7 +1370,7 @@ TEST(FactManagerTest, IdIsIrrelevant) { ASSERT_FALSE(fact_manager.IdIsIrrelevant(12)); ASSERT_FALSE(fact_manager.IdIsIrrelevant(13)); - fact_manager.AddFactIdIsIrrelevant(12); + fact_manager.AddFactIdIsIrrelevant(12, context.get()); ASSERT_TRUE(fact_manager.IdIsIrrelevant(12)); ASSERT_FALSE(fact_manager.IdIsIrrelevant(13)); @@ -1406,12 +1406,12 @@ TEST(FactManagerTest, GetIrrelevantIds) { ASSERT_TRUE(fact_manager.GetIrrelevantIds() == std::unordered_set({})); - fact_manager.AddFactIdIsIrrelevant(12); + fact_manager.AddFactIdIsIrrelevant(12, context.get()); ASSERT_TRUE(fact_manager.GetIrrelevantIds() == std::unordered_set({12})); - fact_manager.AddFactIdIsIrrelevant(13); + fact_manager.AddFactIdIsIrrelevant(13, context.get()); ASSERT_TRUE(fact_manager.GetIrrelevantIds() == std::unordered_set({12, 13})); diff --git a/test/fuzz/transformation_access_chain_test.cpp b/test/fuzz/transformation_access_chain_test.cpp index adb14e3c4e..bd14c113e7 100644 --- a/test/fuzz/transformation_access_chain_test.cpp +++ b/test/fuzz/transformation_access_chain_test.cpp @@ -123,7 +123,7 @@ TEST(TransformationAccessChainTest, BasicTest) { validator_options); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 54); + 54, context.get()); // Bad: id is not fresh ASSERT_FALSE(TransformationAccessChain( diff --git a/test/fuzz/transformation_add_synonym_test.cpp b/test/fuzz/transformation_add_synonym_test.cpp index 93253a109a..d8e3af7902 100644 --- a/test/fuzz/transformation_add_synonym_test.cpp +++ b/test/fuzz/transformation_add_synonym_test.cpp @@ -75,7 +75,7 @@ TEST(TransformationAddSynonymTest, NotApplicable) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(24); + fact_manager.AddFactIdIsIrrelevant(24, context.get()); auto insert_before = MakeInstructionDescriptor(22, SpvOpReturn, 0); @@ -1300,7 +1300,8 @@ TEST(TransformationAddSynonymTest, PropagateIrrelevantPointeeFact) { TransformationContext transformation_context(&fact_manager, validator_options); - transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant(8); + transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( + 8, context.get()); TransformationAddSynonym transformation1( 8, protobufs::TransformationAddSynonym::COPY_OBJECT, 100, diff --git a/test/fuzz/transformation_composite_construct_test.cpp b/test/fuzz/transformation_composite_construct_test.cpp index df6f3825bc..1e390d4c44 100644 --- a/test/fuzz/transformation_composite_construct_test.cpp +++ b/test/fuzz/transformation_composite_construct_test.cpp @@ -1515,7 +1515,7 @@ TEST(TransformationCompositeConstructTest, DontAddSynonymsForIrrelevantIds) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(25); + fact_manager.AddFactIdIsIrrelevant(25, context.get()); TransformationCompositeConstruct transformation( 32, {25, 28, 31}, MakeInstructionDescriptor(31, SpvOpReturn, 0), 200); diff --git a/test/fuzz/transformation_composite_extract_test.cpp b/test/fuzz/transformation_composite_extract_test.cpp index 5b1a0f126a..6af3feaca7 100644 --- a/test/fuzz/transformation_composite_extract_test.cpp +++ b/test/fuzz/transformation_composite_extract_test.cpp @@ -569,7 +569,7 @@ TEST(TransformationCompositeExtractTest, DontAddSynonymsForIrrelevantIds) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(100); + fact_manager.AddFactIdIsIrrelevant(100, context.get()); TransformationCompositeExtract transformation( MakeInstructionDescriptor(36, SpvOpConvertFToS, 0), 201, 100, {2}); ASSERT_TRUE( diff --git a/test/fuzz/transformation_composite_insert_test.cpp b/test/fuzz/transformation_composite_insert_test.cpp index de0cbe3554..b6658cdbb4 100644 --- a/test/fuzz/transformation_composite_insert_test.cpp +++ b/test/fuzz/transformation_composite_insert_test.cpp @@ -365,7 +365,7 @@ TEST(TransformationCompositeInsertTest, IrrelevantCompositeNoSynonyms) { validator_options); // Add fact that the composite is irrelevant. - fact_manager.AddFactIdIsIrrelevant(30); + fact_manager.AddFactIdIsIrrelevant(30, context.get()); auto transformation_good_1 = TransformationCompositeInsert( MakeInstructionDescriptor(30, SpvOpStore, 0), 50, 30, 11, {1, 0, 0}); @@ -470,7 +470,7 @@ TEST(TransformationCompositeInsertTest, IrrelevantObjectSomeSynonyms) { validator_options); // Add fact that the object is irrelevant. - fact_manager.AddFactIdIsIrrelevant(11); + fact_manager.AddFactIdIsIrrelevant(11, context.get()); auto transformation_good_1 = TransformationCompositeInsert( MakeInstructionDescriptor(30, SpvOpStore, 0), 50, 30, 11, {1, 0, 0}); diff --git a/test/fuzz/transformation_equation_instruction_test.cpp b/test/fuzz/transformation_equation_instruction_test.cpp index 132e446911..8dd896dc73 100644 --- a/test/fuzz/transformation_equation_instruction_test.cpp +++ b/test/fuzz/transformation_equation_instruction_test.cpp @@ -1578,10 +1578,10 @@ TEST(TransformationEquationInstructionTest, HandlesIrrelevantIds) { transformation.IsApplicable(context.get(), transformation_context)); // Handles irrelevant ids. - fact_manager.AddFactIdIsIrrelevant(16); + fact_manager.AddFactIdIsIrrelevant(16, context.get()); ASSERT_FALSE( transformation.IsApplicable(context.get(), transformation_context)); - fact_manager.AddFactIdIsIrrelevant(15); + fact_manager.AddFactIdIsIrrelevant(15, context.get()); ASSERT_FALSE( transformation.IsApplicable(context.get(), transformation_context)); } diff --git a/test/fuzz/transformation_function_call_test.cpp b/test/fuzz/transformation_function_call_test.cpp index d7305f8705..37cdec6481 100644 --- a/test/fuzz/transformation_function_call_test.cpp +++ b/test/fuzz/transformation_function_call_test.cpp @@ -147,23 +147,23 @@ TEST(TransformationFunctionCallTest, BasicTest) { transformation_context.GetFactManager()->AddFactFunctionIsLivesafe(21); transformation_context.GetFactManager()->AddFactFunctionIsLivesafe(200); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 71); + 71, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 72); + 72, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 19); + 19, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 20); + 20, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 23); + 23, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 44); + 44, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 46); + 46, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 51); + 51, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 52); + 52, context.get()); // Livesafe functions with argument types: 21(7, 13), 200(7, 13) // Non-livesafe functions with argument types: 4(), 10(7), 17(7, 13), 24(7) diff --git a/test/fuzz/transformation_load_test.cpp b/test/fuzz/transformation_load_test.cpp index 18ca195e73..9c3fb526ab 100644 --- a/test/fuzz/transformation_load_test.cpp +++ b/test/fuzz/transformation_load_test.cpp @@ -90,15 +90,15 @@ TEST(TransformationLoadTest, BasicTest) { validator_options); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 27); + 27, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 11); + 11, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 46); + 46, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 16); + 16, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 52); + 52, context.get()); transformation_context.GetFactManager()->AddFactBlockIsDead(36); diff --git a/test/fuzz/transformation_mutate_pointer_test.cpp b/test/fuzz/transformation_mutate_pointer_test.cpp index e724c5e207..3d935d9c14 100644 --- a/test/fuzz/transformation_mutate_pointer_test.cpp +++ b/test/fuzz/transformation_mutate_pointer_test.cpp @@ -84,8 +84,8 @@ TEST(TransformationMutatePointerTest, BasicTest) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(35); - fact_manager.AddFactIdIsIrrelevant(39); + fact_manager.AddFactIdIsIrrelevant(35, context.get()); + fact_manager.AddFactIdIsIrrelevant(39, context.get()); const auto insert_before = MakeInstructionDescriptor(26, SpvOpReturn, 0); @@ -140,7 +140,7 @@ TEST(TransformationMutatePointerTest, BasicTest) { 26, 70, MakeInstructionDescriptor(26, SpvOpAccessChain, 0)) .IsApplicable(context.get(), transformation_context)); - fact_manager.AddFactIdIsIrrelevant(40); + fact_manager.AddFactIdIsIrrelevant(40, context.get()); uint32_t fresh_id = 70; uint32_t pointer_ids[] = { @@ -274,7 +274,7 @@ TEST(TransformationMutatePointerTest, HandlesUnreachableBlocks) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(7); + fact_manager.AddFactIdIsIrrelevant(7, context.get()); ASSERT_FALSE( context->GetDominatorAnalysis(context->GetFunction(4))->IsReachable(10)); diff --git a/test/fuzz/transformation_outline_function_test.cpp b/test/fuzz/transformation_outline_function_test.cpp index bbce7e3683..dd727c3225 100644 --- a/test/fuzz/transformation_outline_function_test.cpp +++ b/test/fuzz/transformation_outline_function_test.cpp @@ -2033,9 +2033,9 @@ TEST(TransformationOutlineFunctionTest, OutlineLivesafe) { transformation_context.GetFactManager()->AddFactFunctionIsLivesafe(30); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 200); + 200, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 201); + 201, context.get()); TransformationOutlineFunction transformation( /*entry_block*/ 198, @@ -2429,9 +2429,10 @@ TEST(TransformationOutlineFunctionTest, TransformationContext transformation_context(&fact_manager, validator_options); - transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant(9); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 14); + 9, context.get()); + transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( + 14, context.get()); TransformationOutlineFunction transformation( /*entry_block*/ 50, diff --git a/test/fuzz/transformation_push_id_through_variable_test.cpp b/test/fuzz/transformation_push_id_through_variable_test.cpp index eac782067c..ccba46a6a8 100644 --- a/test/fuzz/transformation_push_id_through_variable_test.cpp +++ b/test/fuzz/transformation_push_id_through_variable_test.cpp @@ -682,7 +682,7 @@ TEST(TransformationPushIdThroughVariableTest, DontAddSynonymsForIrrelevantIds) { // Tests the reference shader validity. ASSERT_TRUE(IsValid(env, context.get())); - fact_manager.AddFactIdIsIrrelevant(21); + fact_manager.AddFactIdIsIrrelevant(21, context.get()); uint32_t value_id = 21; uint32_t value_synonym_id = 62; diff --git a/test/fuzz/transformation_record_synonymous_constants_test.cpp b/test/fuzz/transformation_record_synonymous_constants_test.cpp index 2c309fd382..7cded19e22 100644 --- a/test/fuzz/transformation_record_synonymous_constants_test.cpp +++ b/test/fuzz/transformation_record_synonymous_constants_test.cpp @@ -814,7 +814,7 @@ TEST(TransformationRecordSynonymousConstantsTest, FirstIrrelevantConstant) { ASSERT_TRUE(TransformationRecordSynonymousConstants(7, 8).IsApplicable( context.get(), transformation_context)); - fact_manager.AddFactIdIsIrrelevant(7); + fact_manager.AddFactIdIsIrrelevant(7, context.get()); ASSERT_FALSE(TransformationRecordSynonymousConstants(7, 8).IsApplicable( context.get(), transformation_context)); } @@ -851,7 +851,7 @@ TEST(TransformationRecordSynonymousConstantsTest, SecondIrrelevantConstant) { ASSERT_TRUE(TransformationRecordSynonymousConstants(7, 8).IsApplicable( context.get(), transformation_context)); - fact_manager.AddFactIdIsIrrelevant(8); + fact_manager.AddFactIdIsIrrelevant(8, context.get()); ASSERT_FALSE(TransformationRecordSynonymousConstants(7, 8).IsApplicable( context.get(), transformation_context)); } diff --git a/test/fuzz/transformation_replace_irrelevant_id_test.cpp b/test/fuzz/transformation_replace_irrelevant_id_test.cpp index ac04cd39bd..ccad1082b8 100644 --- a/test/fuzz/transformation_replace_irrelevant_id_test.cpp +++ b/test/fuzz/transformation_replace_irrelevant_id_test.cpp @@ -61,11 +61,12 @@ const std::string shader = R"( OpFunctionEnd )"; -void SetUpIrrelevantIdFacts(FactManager* fact_manager) { - fact_manager->AddFactIdIsIrrelevant(17); - fact_manager->AddFactIdIsIrrelevant(23); - fact_manager->AddFactIdIsIrrelevant(24); - fact_manager->AddFactIdIsIrrelevant(25); +void SetUpIrrelevantIdFacts(FactManager* fact_manager, + opt::IRContext* context) { + fact_manager->AddFactIdIsIrrelevant(17, context); + fact_manager->AddFactIdIsIrrelevant(23, context); + fact_manager->AddFactIdIsIrrelevant(24, context); + fact_manager->AddFactIdIsIrrelevant(25, context); } TEST(TransformationReplaceIrrelevantIdTest, Inapplicable) { @@ -79,7 +80,8 @@ TEST(TransformationReplaceIrrelevantIdTest, Inapplicable) { TransformationContext transformation_context(&fact_manager, validator_options); - SetUpIrrelevantIdFacts(transformation_context.GetFactManager()); + SetUpIrrelevantIdFacts(transformation_context.GetFactManager(), + context.get()); auto instruction_21_descriptor = MakeInstructionDescriptor(21, SpvOpAccessChain, 0); @@ -132,7 +134,8 @@ TEST(TransformationReplaceIrrelevantIdTest, Apply) { TransformationContext transformation_context(&fact_manager, validator_options); - SetUpIrrelevantIdFacts(transformation_context.GetFactManager()); + SetUpIrrelevantIdFacts(transformation_context.GetFactManager(), + context.get()); auto instruction_24_descriptor = MakeInstructionDescriptor(24, SpvOpIAdd, 0); diff --git a/test/fuzz/transformation_replace_parameter_with_global_test.cpp b/test/fuzz/transformation_replace_parameter_with_global_test.cpp index 25e25db9d2..3540021822 100644 --- a/test/fuzz/transformation_replace_parameter_with_global_test.cpp +++ b/test/fuzz/transformation_replace_parameter_with_global_test.cpp @@ -330,7 +330,7 @@ TEST(TransformationReplaceParameterWithGlobalTest, TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(10); + fact_manager.AddFactIdIsIrrelevant(10, context.get()); { TransformationReplaceParameterWithGlobal transformation(20, 10, 21); diff --git a/test/fuzz/transformation_store_test.cpp b/test/fuzz/transformation_store_test.cpp index 07d222f095..661ead4937 100644 --- a/test/fuzz/transformation_store_test.cpp +++ b/test/fuzz/transformation_store_test.cpp @@ -99,19 +99,19 @@ TEST(TransformationStoreTest, BasicTest) { validator_options); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 27); + 27, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 11); + 11, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 46); + 46, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 16); + 16, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 52); + 52, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 81); + 81, context.get()); transformation_context.GetFactManager()->AddFactValueOfPointeeIsIrrelevant( - 82); + 82, context.get()); transformation_context.GetFactManager()->AddFactBlockIsDead(36); diff --git a/test/fuzz/transformation_vector_shuffle_test.cpp b/test/fuzz/transformation_vector_shuffle_test.cpp index f72fc95c08..dbed48a8ab 100644 --- a/test/fuzz/transformation_vector_shuffle_test.cpp +++ b/test/fuzz/transformation_vector_shuffle_test.cpp @@ -693,7 +693,7 @@ TEST(TransformationVectorShuffle, HandlesIrrelevantIds2) { TransformationContext transformation_context(&fact_manager, validator_options); - fact_manager.AddFactIdIsIrrelevant(112); + fact_manager.AddFactIdIsIrrelevant(112, context.get()); TransformationVectorShuffle transformation( MakeInstructionDescriptor(100, SpvOpReturn, 0), 200, 12, 112, {2, 0}); ASSERT_TRUE(