From 82a057184019722ef95ae33b7a8339167b61f5bc Mon Sep 17 00:00:00 2001 From: Alastair Donaldson Date: Wed, 29 Jan 2020 17:58:08 +0000 Subject: [PATCH 1/2] Checkpoint. --- source/fuzz/fact_manager.cpp | 45 ++- source/fuzz/fact_manager.h | 20 +- source/fuzz/fuzzer_pass_donate_modules.cpp | 8 +- source/fuzz/protobufs/spvtoolsfuzz.proto | 21 ++ source/fuzz/transformation_add_function.cpp | 24 ++ .../transformation_add_global_variable.cpp | 10 +- .../fuzz/transformation_add_global_variable.h | 3 +- .../fuzz/transformation_outline_function.cpp | 23 +- source/fuzz/transformation_outline_function.h | 8 +- .../fuzz/transformation_add_function_test.cpp | 163 +++++++-- ...ransformation_add_global_variable_test.cpp | 56 +-- .../transformation_outline_function_test.cpp | 342 ++++++++++++++++++ 12 files changed, 658 insertions(+), 65 deletions(-) diff --git a/source/fuzz/fact_manager.cpp b/source/fuzz/fact_manager.cpp index 9672653a10..a7b431120a 100644 --- a/source/fuzz/fact_manager.cpp +++ b/source/fuzz/fact_manager.cpp @@ -859,11 +859,43 @@ bool FactManager::LivesafeFunctionFacts::FunctionIsLivesafe( // End of livesafe function facts //============================== +//============================== +// Arbitrarily-valued variable facts + +// The purpose of this class is to group the fields and data used to represent +// facts about livesafe functions. +class FactManager::ArbitrarilyValuedVaribleFacts { + public: + // See method in FactManager which delegates to this method. + void AddFact(const protobufs::FactValueOfVariableIsArbitrary& fact); + + // See method in FactManager which delegates to this method. + bool VariableValueIsArbitrary(uint32_t variable_id) const; + + private: + std::set arbitrary_valued_varible_ids_; +}; + +void FactManager::ArbitrarilyValuedVaribleFacts::AddFact( + const protobufs::FactValueOfVariableIsArbitrary& fact) { + arbitrary_valued_varible_ids_.insert(fact.variable_id()); +} + +bool FactManager::ArbitrarilyValuedVaribleFacts::VariableValueIsArbitrary( + uint32_t variable_id) const { + return arbitrary_valued_varible_ids_.count(variable_id) != 0; +} + +// End of arbitrarily-valued variable facts +//============================== + FactManager::FactManager() : uniform_constant_facts_(MakeUnique()), data_synonym_facts_(MakeUnique()), dead_block_facts_(MakeUnique()), - livesafe_function_facts_(MakeUnique()) {} + livesafe_function_facts_(MakeUnique()), + arbitrarily_valued_variable_facts_( + MakeUnique()) {} FactManager::~FactManager() = default; @@ -985,5 +1017,16 @@ void FactManager::AddFactFunctionIsLivesafe(uint32_t function_id) { livesafe_function_facts_->AddFact(fact); } +bool FactManager::VariableValueIsArbitrary(uint32_t variable_id) const { + return arbitrarily_valued_variable_facts_->VariableValueIsArbitrary( + variable_id); +} + +void FactManager::AddFactValueOfVariableIsArbitrary(uint32_t variable_id) { + protobufs::FactValueOfVariableIsArbitrary fact; + fact.set_variable_id(variable_id); + arbitrarily_valued_variable_facts_->AddFact(fact); +} + } // namespace fuzz } // namespace spvtools diff --git a/source/fuzz/fact_manager.h b/source/fuzz/fact_manager.h index 20f270154f..117ed1c617 100644 --- a/source/fuzz/fact_manager.h +++ b/source/fuzz/fact_manager.h @@ -64,6 +64,10 @@ class FactManager { // Records the fact that |function_id| is livesafe. void AddFactFunctionIsLivesafe(uint32_t function_id); + // Records the fact that |variable_id| has an arbitrary value and can thus be + // stored to without affecting the module's behaviour. + void AddFactValueOfVariableIsArbitrary(uint32_t variable_id); + // The fact manager is responsible for managing a few distinct categories of // facts. In principle there could be different fact managers for each kind // of fact, but in practice providing one 'go to' place for facts is @@ -153,7 +157,16 @@ class FactManager { // to be livesafe. bool FunctionIsLivesafe(uint32_t function_id) const; - // End of dead block facts + // End of dead livesafe function facts + //============================== + + //============================== + // Querying facts about arbitrarily-valued variables + + // Returns true if and ony if |variable_id| is arbitrarily-valued. + bool VariableValueIsArbitrary(uint32_t variable_id) const; + + // End of arbitrarily-valued variable facts //============================== private: @@ -177,6 +190,11 @@ class FactManager { // function facts. std::unique_ptr livesafe_function_facts_; // Unique pointer to internal data. + + class ArbitrarilyValuedVaribleFacts; // Opaque class for management of + // facts about variables whose values should be expected to be arbitrary. + std::unique_ptr + arbitrarily_valued_variable_facts_; // Unique pointer to internal data. }; } // namespace fuzz diff --git a/source/fuzz/fuzzer_pass_donate_modules.cpp b/source/fuzz/fuzzer_pass_donate_modules.cpp index b0b9d27ff8..75530b10e3 100644 --- a/source/fuzz/fuzzer_pass_donate_modules.cpp +++ b/source/fuzz/fuzzer_pass_donate_modules.cpp @@ -396,6 +396,11 @@ void FuzzerPassDonateModules::HandleTypesAndValues( // have storage class private. As a result, we simply add a Private // storage class global variable, using remapped versions of the result // type and initializer ids for the global variable in the donor. + // + // We regard the added variable as having an arbitrary value. This + // means that future passes can add stores to the variable in any + // way they wish, and pass them as pointer parameters to functions + // without worrying about whether their data might get modified. new_result_id = GetFuzzerContext()->GetFreshId(); ApplyTransformation(TransformationAddGlobalVariable( new_result_id, @@ -403,7 +408,8 @@ void FuzzerPassDonateModules::HandleTypesAndValues( type_or_value.NumInOperands() == 1 ? 0 : original_id_to_donated_id->at( - type_or_value.GetSingleWordInOperand(1)))); + type_or_value.GetSingleWordInOperand(1)), + true)); } break; case SpvOpUndef: { // It is fine to have multiple Undef instructions of the same type, so diff --git a/source/fuzz/protobufs/spvtoolsfuzz.proto b/source/fuzz/protobufs/spvtoolsfuzz.proto index 2f37a7d967..52a3a788ca 100644 --- a/source/fuzz/protobufs/spvtoolsfuzz.proto +++ b/source/fuzz/protobufs/spvtoolsfuzz.proto @@ -168,6 +168,7 @@ message Fact { FactDataSynonym data_synonym_fact = 2; FactBlockIsDead block_is_dead_fact = 3; FactFunctionIsLivesafe function_is_livesafe_fact = 4; + FactValueOfVariableIsArbitrary value_of_variable_is_arbitrary = 5; } } @@ -222,6 +223,19 @@ message FactFunctionIsLivesafe { uint32 function_id = 1; } +message FactValueOfVariableIsArbitrary { + + // Records the fact that the value stored in the variable or function + // parameter with the given id can be arbitrary: the module's observable + // behaviour does not depend on it. This means that arbitrary stores can be + // made to the variable, and that nothing can be guaranteed about values + // loaded from the variable. + + // The result id of an OpVariable instruction, or an OpFunctionParameter + // instruction with pointer type + uint32 variable_id = 1; +} + message AccessChainClampingInfo { // When making a function livesafe it is necessary to clamp the indices that @@ -493,6 +507,13 @@ message TransformationAddGlobalVariable { // Optional initializer; 0 if there is no initializer uint32 initializer_id = 3; + // True if and only if the value of the variable should be regarded, in + // general, as totally unknown and subject to change (even if, due to an + // initializer, the original value is known). This is the case for variables + // added when a module is donated, for example, and means that stores to such + // variables can be performed in an arbitrary fashion. + bool value_is_arbitrary = 4; + } message TransformationAddNoContractionDecoration { diff --git a/source/fuzz/transformation_add_function.cpp b/source/fuzz/transformation_add_function.cpp index 8b8b2ddd50..b013d9441c 100644 --- a/source/fuzz/transformation_add_function.cpp +++ b/source/fuzz/transformation_add_function.cpp @@ -154,6 +154,30 @@ 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 arbitrary 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 (context->get_def_use_mgr() + ->GetDef(instruction.result_type_id()) + ->opcode() == SpvOpTypePointer) { + fact_manager->AddFactValueOfVariableIsArbitrary( + instruction.result_id()); + } + break; + case SpvOpVariable: + fact_manager->AddFactValueOfVariableIsArbitrary( + instruction.result_id()); + break; + default: + break; + } + } + if (message_.is_livesafe()) { // Make the function livesafe, which also should succeed. success = TryToMakeFunctionLivesafe(context, *fact_manager); diff --git a/source/fuzz/transformation_add_global_variable.cpp b/source/fuzz/transformation_add_global_variable.cpp index cea268c638..c08517f9a5 100644 --- a/source/fuzz/transformation_add_global_variable.cpp +++ b/source/fuzz/transformation_add_global_variable.cpp @@ -24,10 +24,12 @@ TransformationAddGlobalVariable::TransformationAddGlobalVariable( : message_(message) {} TransformationAddGlobalVariable::TransformationAddGlobalVariable( - uint32_t fresh_id, uint32_t type_id, uint32_t initializer_id) { + uint32_t fresh_id, uint32_t type_id, uint32_t initializer_id, + bool value_is_arbitrary) { message_.set_fresh_id(fresh_id); message_.set_type_id(type_id); message_.set_initializer_id(initializer_id); + message_.set_value_is_arbitrary(value_is_arbitrary); } bool TransformationAddGlobalVariable::IsApplicable( @@ -71,7 +73,7 @@ bool TransformationAddGlobalVariable::IsApplicable( } void TransformationAddGlobalVariable::Apply( - opt::IRContext* context, spvtools::fuzz::FactManager* /*unused*/) const { + opt::IRContext* context, spvtools::fuzz::FactManager* fact_manager) const { opt::Instruction::OperandList input_operands; input_operands.push_back( {SPV_OPERAND_TYPE_STORAGE_CLASS, {SpvStorageClassPrivate}}); @@ -99,6 +101,10 @@ void TransformationAddGlobalVariable::Apply( } } + if (message_.value_is_arbitrary()) { + fact_manager->AddFactValueOfVariableIsArbitrary(message_.fresh_id()); + } + // We have added an instruction to the module, so need to be careful about the // validity of existing analyses. context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisNone); diff --git a/source/fuzz/transformation_add_global_variable.h b/source/fuzz/transformation_add_global_variable.h index ca63e689a2..406c915715 100644 --- a/source/fuzz/transformation_add_global_variable.h +++ b/source/fuzz/transformation_add_global_variable.h @@ -29,7 +29,8 @@ class TransformationAddGlobalVariable : public Transformation { const protobufs::TransformationAddGlobalVariable& message); TransformationAddGlobalVariable(uint32_t fresh_id, uint32_t type_id, - uint32_t initializer_id); + uint32_t initializer_id, + bool value_is_arbitrary); // - |message_.fresh_id| must be fresh // - |message_.type_id| must be the id of a pointer type with Private storage diff --git a/source/fuzz/transformation_outline_function.cpp b/source/fuzz/transformation_outline_function.cpp index c097e6cfe6..7bbac54839 100644 --- a/source/fuzz/transformation_outline_function.cpp +++ b/source/fuzz/transformation_outline_function.cpp @@ -340,8 +340,16 @@ void TransformationOutlineFunction::Apply( // Make a function prototype for the outlined function, which involves // figuring out its required type. - std::unique_ptr outlined_function = PrepareFunctionPrototype( - context, region_input_ids, region_output_ids, input_id_to_fresh_id_map); + std::unique_ptr outlined_function = + PrepareFunctionPrototype(region_input_ids, region_output_ids, + input_id_to_fresh_id_map, context, fact_manager); + + // If the original function was livesafe, the new function should also be + // livesafe. + if (fact_manager->FunctionIsLivesafe( + original_region_entry_block->GetParent()->result_id())) { + fact_manager->AddFactFunctionIsLivesafe(message_.new_function_id()); + } // Adapt the region to be outlined so that its input ids are replaced with the // ids of the outlined function's input parameters, and so that output ids @@ -524,9 +532,10 @@ std::set TransformationOutlineFunction::GetRegionBlocks( std::unique_ptr TransformationOutlineFunction::PrepareFunctionPrototype( - opt::IRContext* context, const std::vector& region_input_ids, + const std::vector& region_input_ids, const std::vector& region_output_ids, - const std::map& input_id_to_fresh_id_map) const { + const std::map& input_id_to_fresh_id_map, + opt::IRContext* context, FactManager* fact_manager) const { uint32_t return_type_id = 0; uint32_t function_type_id = 0; @@ -608,6 +617,12 @@ TransformationOutlineFunction::PrepareFunctionPrototype( context, SpvOpFunctionParameter, context->get_def_use_mgr()->GetDef(id)->type_id(), input_id_to_fresh_id_map.at(id), opt::Instruction::OperandList())); + // If the input id is an arbitrary-valued variable, the same should be true + // of the corresponding parameter. + if (fact_manager->VariableValueIsArbitrary(id)) { + fact_manager->AddFactValueOfVariableIsArbitrary( + input_id_to_fresh_id_map.at(id)); + } } return outlined_function; diff --git a/source/fuzz/transformation_outline_function.h b/source/fuzz/transformation_outline_function.h index 43bdf3b85c..5711790620 100644 --- a/source/fuzz/transformation_outline_function.h +++ b/source/fuzz/transformation_outline_function.h @@ -158,10 +158,14 @@ class TransformationOutlineFunction : public Transformation { // A new struct type to represent the function return type, and a new function // type for the function, will be added to the module (unless suitable types // are already present). + // + // Facts about the function containing the outlined region that are relevant + // to the new function are propagated via |fact_manager|. std::unique_ptr PrepareFunctionPrototype( - opt::IRContext* context, const std::vector& region_input_ids, + const std::vector& region_input_ids, const std::vector& region_output_ids, - const std::map& input_id_to_fresh_id_map) const; + const std::map& input_id_to_fresh_id_map, + opt::IRContext* context, FactManager* fact_manager) const; // Creates the body of the outlined function by cloning blocks from the // original region, given by |region_blocks|, adapting the cloned version diff --git a/test/fuzz/transformation_add_function_test.cpp b/test/fuzz/transformation_add_function_test.cpp index 3bc7620510..040d27ca5e 100644 --- a/test/fuzz/transformation_add_function_test.cpp +++ b/test/fuzz/transformation_add_function_test.cpp @@ -58,6 +58,56 @@ std::vector GetInstructionsForFunction( return result; } +// Returns true if and only if every pointer parameter and variable associated +// with |function_id| in |context| is known by |fact_manager| to be arbitrary, +// with the exception of |loop_limiter_id|, which must not be arbitrary. (It +// can be 0 if no loop limiter is expected, and 0 should not be deemed +// arbitrary). +bool AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + opt::IRContext* context, const FactManager& fact_manager, + uint32_t function_id, uint32_t loop_limiter_id) { + // Look at all the functions until the function of interest is found. + for (auto& function : *context->module()) { + if (function.result_id() != function_id) { + continue; + } + // Check that the parameters are all arbitrary. + bool found_non_arbitrary_parameter = false; + function.ForEachParam( + [context, &fact_manager, + &found_non_arbitrary_parameter](opt::Instruction* inst) { + if (context->get_def_use_mgr()->GetDef(inst->type_id())->opcode() == + SpvOpTypePointer && + !fact_manager.VariableValueIsArbitrary(inst->result_id())) { + found_non_arbitrary_parameter = true; + } + }); + if (found_non_arbitrary_parameter) { + // A non-arbitrary parameter was found. + return false; + } + // Look through the instructions in the function's first block. + for (auto& inst : *function.begin()) { + if (inst.opcode() != SpvOpVariable) { + // We have found a non-variable instruction; this means we have gotten + // past all variables, so we are done. + return true; + } + // The variable should be arbitrary if and only if it is not the loop + // limiter. + if ((inst.result_id() == loop_limiter_id) == + fact_manager.VariableValueIsArbitrary(inst.result_id())) { + return false; + } + } + assert(false && + "We should have processed all variables and returned by " + "this point."); + } + assert(false && "We should have found the function of interest."); + return true; +} + TEST(TransformationAddFunctionTest, BasicTest) { std::string shader = R"( OpCapability Shader @@ -570,18 +620,22 @@ TEST(TransformationAddFunctionTest, LoopLimiters) { instructions.push_back(MakeInstructionMessage(SpvOpReturn, 0, 0, {})); instructions.push_back(MakeInstructionMessage(SpvOpFunctionEnd, 0, 0, {})); - FactManager fact_manager; + FactManager fact_manager1; + FactManager fact_manager2; const auto context1 = BuildModule(env, consumer, shader, kFuzzAssembleOption); const auto context2 = BuildModule(env, consumer, shader, kFuzzAssembleOption); ASSERT_TRUE(IsValid(env, context1.get())); TransformationAddFunction add_dead_function(instructions); - ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager)); - add_dead_function.Apply(context1.get(), &fact_manager); + ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); + add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); // The added function should not be deemed livesafe. - ASSERT_FALSE(fact_manager.FunctionIsLivesafe(30)); + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(30)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 30, 0)); std::string added_as_dead_code = R"( OpCapability Shader @@ -657,11 +711,16 @@ TEST(TransformationAddFunctionTest, LoopLimiters) { TransformationAddFunction add_livesafe_function(instructions, 100, 10, loop_limiters, 0, {}); - ASSERT_TRUE(add_livesafe_function.IsApplicable(context2.get(), fact_manager)); - add_livesafe_function.Apply(context2.get(), &fact_manager); + ASSERT_TRUE( + add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); + add_livesafe_function.Apply(context2.get(), &fact_manager2); ASSERT_TRUE(IsValid(env, context2.get())); // The added function should indeed be deemed livesafe. - ASSERT_TRUE(fact_manager.FunctionIsLivesafe(30)); + ASSERT_TRUE(fact_manager2.FunctionIsLivesafe(30)); + // All variables/parameters in the function should be deemed arbitrary, + // except the loop limiter. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context2.get(), fact_manager2, 30, 100)); std::string added_as_livesafe_code = R"( OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" @@ -776,18 +835,22 @@ TEST(TransformationAddFunctionTest, KillAndUnreachableInVoidFunction) { instructions.push_back(MakeInstructionMessage(SpvOpKill, 0, 0, {})); instructions.push_back(MakeInstructionMessage(SpvOpFunctionEnd, 0, 0, {})); - FactManager fact_manager; + FactManager fact_manager1; + FactManager fact_manager2; const auto context1 = BuildModule(env, consumer, shader, kFuzzAssembleOption); const auto context2 = BuildModule(env, consumer, shader, kFuzzAssembleOption); ASSERT_TRUE(IsValid(env, context1.get())); TransformationAddFunction add_dead_function(instructions); - ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager)); - add_dead_function.Apply(context1.get(), &fact_manager); + ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); + add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); // The added function should not be deemed livesafe. - ASSERT_FALSE(fact_manager.FunctionIsLivesafe(10)); + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(10)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 10, 0)); std::string added_as_dead_code = R"( OpCapability Shader @@ -824,11 +887,15 @@ TEST(TransformationAddFunctionTest, KillAndUnreachableInVoidFunction) { TransformationAddFunction add_livesafe_function(instructions, 0, 0, {}, 0, {}); - ASSERT_TRUE(add_livesafe_function.IsApplicable(context2.get(), fact_manager)); - add_livesafe_function.Apply(context2.get(), &fact_manager); + ASSERT_TRUE( + add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); + add_livesafe_function.Apply(context2.get(), &fact_manager2); ASSERT_TRUE(IsValid(env, context2.get())); // The added function should indeed be deemed livesafe. - ASSERT_TRUE(fact_manager.FunctionIsLivesafe(10)); + ASSERT_TRUE(fact_manager2.FunctionIsLivesafe(10)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context2.get(), fact_manager2, 10, 0)); std::string added_as_livesafe_code = R"( OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" @@ -916,18 +983,22 @@ TEST(TransformationAddFunctionTest, KillAndUnreachableInNonVoidFunction) { instructions.push_back(MakeInstructionMessage(SpvOpKill, 0, 0, {})); instructions.push_back(MakeInstructionMessage(SpvOpFunctionEnd, 0, 0, {})); - FactManager fact_manager; + FactManager fact_manager1; + FactManager fact_manager2; const auto context1 = BuildModule(env, consumer, shader, kFuzzAssembleOption); const auto context2 = BuildModule(env, consumer, shader, kFuzzAssembleOption); ASSERT_TRUE(IsValid(env, context1.get())); TransformationAddFunction add_dead_function(instructions); - ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager)); - add_dead_function.Apply(context1.get(), &fact_manager); + ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); + add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); // The added function should not be deemed livesafe. - ASSERT_FALSE(fact_manager.FunctionIsLivesafe(10)); + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(10)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 10, 0)); std::string added_as_dead_code = R"( OpCapability Shader @@ -965,11 +1036,15 @@ TEST(TransformationAddFunctionTest, KillAndUnreachableInNonVoidFunction) { TransformationAddFunction add_livesafe_function(instructions, 0, 0, {}, 13, {}); - ASSERT_TRUE(add_livesafe_function.IsApplicable(context2.get(), fact_manager)); - add_livesafe_function.Apply(context2.get(), &fact_manager); + ASSERT_TRUE( + add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); + add_livesafe_function.Apply(context2.get(), &fact_manager2); ASSERT_TRUE(IsValid(env, context2.get())); // The added function should indeed be deemed livesafe. - ASSERT_TRUE(fact_manager.FunctionIsLivesafe(10)); + ASSERT_TRUE(fact_manager2.FunctionIsLivesafe(10)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context2.get(), fact_manager2, 10, 0)); std::string added_as_livesafe_code = R"( OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" @@ -1188,16 +1263,22 @@ TEST(TransformationAddFunctionTest, ClampedAccessChains) { instructions.push_back(MakeInstructionMessage(SpvOpReturn, 0, 0, {})); instructions.push_back(MakeInstructionMessage(SpvOpFunctionEnd, 0, 0, {})); - FactManager fact_manager; + FactManager fact_manager1; + FactManager fact_manager2; const auto context1 = BuildModule(env, consumer, shader, kFuzzAssembleOption); const auto context2 = BuildModule(env, consumer, shader, kFuzzAssembleOption); ASSERT_TRUE(IsValid(env, context1.get())); TransformationAddFunction add_dead_function(instructions); - ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager)); - add_dead_function.Apply(context1.get(), &fact_manager); + ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); + add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); + // The function should not be deemed livesafe + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(12)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 12, 0)); std::string added_as_dead_code = R"( OpCapability Shader @@ -1328,9 +1409,15 @@ TEST(TransformationAddFunctionTest, ClampedAccessChains) { TransformationAddFunction add_livesafe_function(instructions, 0, 0, {}, 13, access_chain_clamping_info); - ASSERT_TRUE(add_livesafe_function.IsApplicable(context2.get(), fact_manager)); - add_livesafe_function.Apply(context2.get(), &fact_manager); + ASSERT_TRUE( + add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); + add_livesafe_function.Apply(context2.get(), &fact_manager2); ASSERT_TRUE(IsValid(env, context2.get())); + // The function should be deemed livesafe + ASSERT_TRUE(fact_manager2.FunctionIsLivesafe(12)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context2.get(), fact_manager2, 12, 0)); std::string added_as_livesafe_code = R"( OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" @@ -1510,6 +1597,11 @@ TEST(TransformationAddFunctionTest, LivesafeCanCallLivesafe) { ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); + // The function should not be deemed livesafe + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(8)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 8, 0)); std::string added_as_live_or_dead_code = R"( OpCapability Shader @@ -1542,6 +1634,11 @@ TEST(TransformationAddFunctionTest, LivesafeCanCallLivesafe) { add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); add_livesafe_function.Apply(context2.get(), &fact_manager2); ASSERT_TRUE(IsValid(env, context2.get())); + // The function should be deemed livesafe + ASSERT_TRUE(fact_manager2.FunctionIsLivesafe(8)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context2.get(), fact_manager2, 8, 0)); ASSERT_TRUE(IsEqual(env, added_as_live_or_dead_code, context2.get())); } @@ -1580,16 +1677,22 @@ TEST(TransformationAddFunctionTest, LivesafeOnlyCallsLivesafe) { instructions.push_back(MakeInstructionMessage(SpvOpReturn, 0, 0, {})); instructions.push_back(MakeInstructionMessage(SpvOpFunctionEnd, 0, 0, {})); - FactManager fact_manager; + FactManager fact_manager1; + FactManager fact_manager2; const auto context1 = BuildModule(env, consumer, shader, kFuzzAssembleOption); const auto context2 = BuildModule(env, consumer, shader, kFuzzAssembleOption); ASSERT_TRUE(IsValid(env, context1.get())); TransformationAddFunction add_dead_function(instructions); - ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager)); - add_dead_function.Apply(context1.get(), &fact_manager); + ASSERT_TRUE(add_dead_function.IsApplicable(context1.get(), fact_manager1)); + add_dead_function.Apply(context1.get(), &fact_manager1); ASSERT_TRUE(IsValid(env, context1.get())); + // The function should not be deemed livesafe + ASSERT_FALSE(fact_manager1.FunctionIsLivesafe(8)); + // All variables/parameters in the function should be deemed arbitrary. + ASSERT_TRUE(AllVariablesAndParametersExceptLoopLimiterAreArbitrary( + context1.get(), fact_manager1, 8, 0)); std::string added_as_dead_code = R"( OpCapability Shader @@ -1619,7 +1722,7 @@ TEST(TransformationAddFunctionTest, LivesafeOnlyCallsLivesafe) { TransformationAddFunction add_livesafe_function(instructions, 0, 0, {}, 0, {}); ASSERT_FALSE( - add_livesafe_function.IsApplicable(context2.get(), fact_manager)); + add_livesafe_function.IsApplicable(context2.get(), fact_manager2)); } TEST(TransformationAddFunctionTest, diff --git a/test/fuzz/transformation_add_global_variable_test.cpp b/test/fuzz/transformation_add_global_variable_test.cpp index eda6828f2b..7fb4fa0860 100644 --- a/test/fuzz/transformation_add_global_variable_test.cpp +++ b/test/fuzz/transformation_add_global_variable_test.cpp @@ -60,71 +60,78 @@ TEST(TransformationAddGlobalVariableTest, BasicTest) { FactManager fact_manager; // Id already in use - ASSERT_FALSE(TransformationAddGlobalVariable(4, 10, 0).IsApplicable( - context.get(), fact_manager)); + ASSERT_FALSE(TransformationAddGlobalVariable(4, 10, 0, true) + .IsApplicable(context.get(), fact_manager)); // %1 is not a type - ASSERT_FALSE(TransformationAddGlobalVariable(100, 1, 0).IsApplicable( - context.get(), fact_manager)); + ASSERT_FALSE(TransformationAddGlobalVariable(100, 1, 0, false) + .IsApplicable(context.get(), fact_manager)); // %7 is not a pointer type - ASSERT_FALSE(TransformationAddGlobalVariable(100, 7, 0).IsApplicable( - context.get(), fact_manager)); + ASSERT_FALSE(TransformationAddGlobalVariable(100, 7, 0, true) + .IsApplicable(context.get(), fact_manager)); // %9 does not have Private storage class - ASSERT_FALSE(TransformationAddGlobalVariable(100, 9, 0).IsApplicable( - context.get(), fact_manager)); + ASSERT_FALSE(TransformationAddGlobalVariable(100, 9, 0, false) + .IsApplicable(context.get(), fact_manager)); // %15 does not have Private storage class - ASSERT_FALSE(TransformationAddGlobalVariable(100, 15, 0) + ASSERT_FALSE(TransformationAddGlobalVariable(100, 15, 0, true) .IsApplicable(context.get(), fact_manager)); // %10 is a pointer to float, while %16 is an int constant - ASSERT_FALSE(TransformationAddGlobalVariable(100, 10, 16) + ASSERT_FALSE(TransformationAddGlobalVariable(100, 10, 16, false) .IsApplicable(context.get(), fact_manager)); // %10 is a Private pointer to float, while %15 is a variable with type // Uniform float pointer - ASSERT_FALSE(TransformationAddGlobalVariable(100, 10, 15) + ASSERT_FALSE(TransformationAddGlobalVariable(100, 10, 15, true) .IsApplicable(context.get(), fact_manager)); // %12 is a Private pointer to int, while %10 is a variable with type // Private float pointer - ASSERT_FALSE(TransformationAddGlobalVariable(100, 12, 10) + ASSERT_FALSE(TransformationAddGlobalVariable(100, 12, 10, false) .IsApplicable(context.get(), fact_manager)); // %10 is pointer-to-float, and %14 has type pointer-to-float; that's not OK // since the initializer's type should be the *pointee* type. - ASSERT_FALSE(TransformationAddGlobalVariable(104, 10, 14) + ASSERT_FALSE(TransformationAddGlobalVariable(104, 10, 14, true) .IsApplicable(context.get(), fact_manager)); // This would work in principle, but logical addressing does not allow // a pointer to a pointer. - ASSERT_FALSE(TransformationAddGlobalVariable(104, 17, 14) + ASSERT_FALSE(TransformationAddGlobalVariable(104, 17, 14, false) .IsApplicable(context.get(), fact_manager)); TransformationAddGlobalVariable transformations[] = { // %100 = OpVariable %12 Private - TransformationAddGlobalVariable(100, 12, 0), + TransformationAddGlobalVariable(100, 12, 0, true), // %101 = OpVariable %10 Private - TransformationAddGlobalVariable(101, 10, 0), + TransformationAddGlobalVariable(101, 10, 0, false), // %102 = OpVariable %13 Private - TransformationAddGlobalVariable(102, 13, 0), + TransformationAddGlobalVariable(102, 13, 0, true), // %103 = OpVariable %12 Private %16 - TransformationAddGlobalVariable(103, 12, 16), + TransformationAddGlobalVariable(103, 12, 16, false), // %104 = OpVariable %19 Private %21 - TransformationAddGlobalVariable(104, 19, 21), + TransformationAddGlobalVariable(104, 19, 21, true), // %105 = OpVariable %19 Private %22 - TransformationAddGlobalVariable(105, 19, 22)}; + TransformationAddGlobalVariable(105, 19, 22, false)}; for (auto& transformation : transformations) { ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); transformation.Apply(context.get(), &fact_manager); } + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(100)); + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(102)); + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(104)); + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(101)); + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(103)); + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(105)); + ASSERT_TRUE(IsValid(env, context.get())); std::string after_transformation = R"( @@ -215,18 +222,21 @@ TEST(TransformationAddGlobalVariableTest, TestEntryPointInterfaceEnlargement) { TransformationAddGlobalVariable transformations[] = { // %100 = OpVariable %12 Private - TransformationAddGlobalVariable(100, 12, 0), + TransformationAddGlobalVariable(100, 12, 0, true), // %101 = OpVariable %12 Private %16 - TransformationAddGlobalVariable(101, 12, 16), + TransformationAddGlobalVariable(101, 12, 16, false), // %102 = OpVariable %19 Private %21 - TransformationAddGlobalVariable(102, 19, 21)}; + TransformationAddGlobalVariable(102, 19, 21, true)}; for (auto& transformation : transformations) { ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); transformation.Apply(context.get(), &fact_manager); } + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(100)); + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(102)); + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(101)); ASSERT_TRUE(IsValid(env, context.get())); std::string after_transformation = R"( diff --git a/test/fuzz/transformation_outline_function_test.cpp b/test/fuzz/transformation_outline_function_test.cpp index ff3d91314d..d6bc3f8b05 100644 --- a/test/fuzz/transformation_outline_function_test.cpp +++ b/test/fuzz/transformation_outline_function_test.cpp @@ -1826,6 +1826,348 @@ TEST(TransformationOutlineFunctionTest, ASSERT_TRUE(IsEqual(env, after_transformation, context.get())); } +TEST(TransformationOutlineFunctionTest, OutlineLivesafe) { + // In the following, %30 is a livesafe function, with arbitrary parameter + // %200 and arbitrary local variable %201. Variable %100 is a loop limiter, + // which is not arbitrary. The test checks that the outlined function is + // livesafe, and that the parameters corresponding to %200 and %201 have the + // arbitrary fact associated with them. + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 0 + %7 = OpTypePointer Function %6 + %199 = OpTypeFunction %2 %7 + %8 = OpConstant %6 0 + %9 = OpConstant %6 1 + %10 = OpConstant %6 5 + %11 = OpTypeBool + %12 = OpConstantTrue %11 + %4 = OpFunction %2 None %3 + %5 = OpLabel + OpReturn + OpFunctionEnd + %30 = OpFunction %2 None %199 + %200 = OpFunctionParameter %7 + %31 = OpLabel + %100 = OpVariable %7 Function %8 + %201 = OpVariable %7 Function %8 + OpBranch %198 + %198 = OpLabel + OpBranch %20 + %20 = OpLabel + %101 = OpLoad %6 %100 + %102 = OpIAdd %6 %101 %9 + %202 = OpLoad %6 %200 + OpStore %201 %202 + OpStore %100 %102 + %103 = OpUGreaterThanEqual %11 %101 %10 + OpLoopMerge %21 %22 None + OpBranchConditional %103 %21 %104 + %104 = OpLabel + OpBranchConditional %12 %23 %21 + %23 = OpLabel + %105 = OpLoad %6 %100 + %106 = OpIAdd %6 %105 %9 + OpStore %100 %106 + %107 = OpUGreaterThanEqual %11 %105 %10 + OpLoopMerge %25 %26 None + OpBranchConditional %107 %25 %108 + %108 = OpLabel + OpBranch %28 + %28 = OpLabel + OpBranchConditional %12 %26 %25 + %26 = OpLabel + OpBranch %23 + %25 = OpLabel + %109 = OpLoad %6 %100 + %110 = OpIAdd %6 %109 %9 + OpStore %100 %110 + %111 = OpUGreaterThanEqual %11 %109 %10 + OpLoopMerge %24 %27 None + OpBranchConditional %111 %24 %112 + %112 = OpLabel + OpBranchConditional %12 %24 %27 + %27 = OpLabel + OpBranch %25 + %24 = OpLabel + OpBranch %22 + %22 = OpLabel + OpBranch %20 + %21 = OpLabel + OpBranch %197 + %197 = 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; + fact_manager.AddFactFunctionIsLivesafe(30); + fact_manager.AddFactValueOfVariableIsArbitrary(200); + fact_manager.AddFactValueOfVariableIsArbitrary(201); + + TransformationOutlineFunction transformation( + /*entry_block*/ 198, + /*exit_block*/ 197, + /*new_function_struct_return_type_id*/ 400, + /*new_function_type_id*/ 401, + /*new_function_id*/ 402, + /*new_function_region_entry_block*/ 404, + /*new_caller_result_id*/ 405, + /*new_callee_result_id*/ 406, + /*input_id_to_fresh_id*/ {{100, 407}, {200, 408}, {201, 409}}, + /*output_id_to_fresh_id*/ {}); + + ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); + transformation.Apply(context.get(), &fact_manager); + ASSERT_TRUE(IsValid(env, context.get())); + + // The original function should still be livesafe. + ASSERT_TRUE(fact_manager.FunctionIsLivesafe(30)); + // The outlined function should be livesafe. + ASSERT_TRUE(fact_manager.FunctionIsLivesafe(402)); + // The variable and parameter that were originally arbitrary should still be. + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(200)); + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(201)); + // The loop limiter should still be non-arbitrary. + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(100)); + // The parameters for the original arbitrary variables should be arbitrary. + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(408)); + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(409)); + // The parameter for the loop limiter should not be arbitrary. + ASSERT_FALSE(fact_manager.VariableValueIsArbitrary(407)); + + std::string after_transformation = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 0 + %7 = OpTypePointer Function %6 + %199 = OpTypeFunction %2 %7 + %8 = OpConstant %6 0 + %9 = OpConstant %6 1 + %10 = OpConstant %6 5 + %11 = OpTypeBool + %12 = OpConstantTrue %11 + %401 = OpTypeFunction %2 %7 %7 %7 + %4 = OpFunction %2 None %3 + %5 = OpLabel + OpReturn + OpFunctionEnd + %30 = OpFunction %2 None %199 + %200 = OpFunctionParameter %7 + %31 = OpLabel + %100 = OpVariable %7 Function %8 + %201 = OpVariable %7 Function %8 + OpBranch %198 + %198 = OpLabel + %405 = OpFunctionCall %2 %402 %200 %100 %201 + OpReturn + OpFunctionEnd + %402 = OpFunction %2 None %401 + %408 = OpFunctionParameter %7 + %407 = OpFunctionParameter %7 + %409 = OpFunctionParameter %7 + %404 = OpLabel + OpBranch %20 + %20 = OpLabel + %101 = OpLoad %6 %407 + %102 = OpIAdd %6 %101 %9 + %202 = OpLoad %6 %408 + OpStore %409 %202 + OpStore %407 %102 + %103 = OpUGreaterThanEqual %11 %101 %10 + OpLoopMerge %21 %22 None + OpBranchConditional %103 %21 %104 + %104 = OpLabel + OpBranchConditional %12 %23 %21 + %23 = OpLabel + %105 = OpLoad %6 %407 + %106 = OpIAdd %6 %105 %9 + OpStore %407 %106 + %107 = OpUGreaterThanEqual %11 %105 %10 + OpLoopMerge %25 %26 None + OpBranchConditional %107 %25 %108 + %108 = OpLabel + OpBranch %28 + %28 = OpLabel + OpBranchConditional %12 %26 %25 + %26 = OpLabel + OpBranch %23 + %25 = OpLabel + %109 = OpLoad %6 %407 + %110 = OpIAdd %6 %109 %9 + OpStore %407 %110 + %111 = OpUGreaterThanEqual %11 %109 %10 + OpLoopMerge %24 %27 None + OpBranchConditional %111 %24 %112 + %112 = OpLabel + OpBranchConditional %12 %24 %27 + %27 = OpLabel + OpBranch %25 + %24 = OpLabel + OpBranch %22 + %22 = OpLabel + OpBranch %20 + %21 = OpLabel + OpBranch %197 + %197 = OpLabel + OpReturn + OpFunctionEnd + )"; + ASSERT_TRUE(IsEqual(env, after_transformation, context.get())); +} + +TEST(TransformationOutlineFunctionTest, OutlineWithDeadBlocks1) { + // This checks that if all blocks in the region being outlined were dead, all + // blocks in the outlined function will be dead. + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + OpName %4 "main" + OpName %10 "foo(i1;" + OpName %9 "x" + OpName %12 "y" + OpName %21 "i" + OpName %46 "param" + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 1 + %7 = OpTypePointer Function %6 + %8 = OpTypeFunction %2 %7 + %13 = OpConstant %6 2 + %14 = OpTypeBool + %15 = OpConstantFalse %14 + %22 = OpConstant %6 0 + %29 = OpConstant %6 10 + %41 = OpConstant %6 1 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %46 = OpVariable %7 Function + OpStore %46 %13 + %47 = OpFunctionCall %2 %10 %46 + OpReturn + OpFunctionEnd + %10 = OpFunction %2 None %8 + %9 = OpFunctionParameter %7 + %11 = OpLabel + %12 = OpVariable %7 Function + %21 = OpVariable %7 Function + OpStore %12 %13 + OpSelectionMerge %17 None + OpBranchConditional %15 %16 %17 + %16 = OpLabel + %18 = OpLoad %6 %9 + OpStore %12 %18 + %19 = OpLoad %6 %9 + %20 = OpIAdd %6 %19 %13 + OpStore %9 %20 + OpStore %21 %22 + OpBranch %23 + %23 = OpLabel + OpLoopMerge %25 %26 None + OpBranch %27 + %27 = OpLabel + %28 = OpLoad %6 %21 + %30 = OpSLessThan %14 %28 %29 + OpBranchConditional %30 %24 %25 + %24 = OpLabel + %31 = OpLoad %6 %9 + %32 = OpLoad %6 %21 + %33 = OpSGreaterThan %14 %31 %32 + OpSelectionMerge %35 None + OpBranchConditional %33 %34 %35 + %34 = OpLabel + OpBranch %26 + %35 = OpLabel + %37 = OpLoad %6 %9 + %38 = OpLoad %6 %12 + %39 = OpIAdd %6 %38 %37 + OpStore %12 %39 + OpBranch %26 + %26 = OpLabel + %40 = OpLoad %6 %21 + %42 = OpIAdd %6 %40 %41 + OpStore %21 %42 + OpBranch %23 + %25 = OpLabel + OpBranch %50 + %50 = OpLabel + OpBranch %17 + %17 = OpLabel + %43 = OpLoad %6 %9 + %44 = OpLoad %6 %12 + %45 = OpIAdd %6 %44 %43 + OpStore %12 %45 + 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; + for (uint32_t block_id : {16u, 23u, 24u, 26u, 27u, 34u, 35u, 50u}) { + fact_manager.AddFactBlockIsDead(block_id); + } + + TransformationOutlineFunction transformation( + /*entry_block*/ 16, + /*exit_block*/ 50, + /*new_function_struct_return_type_id*/ 200, + /*new_function_type_id*/ 201, + /*new_function_id*/ 202, + /*new_function_region_entry_block*/ 203, + /*new_caller_result_id*/ 204, + /*new_callee_result_id*/ 205, + /*input_id_to_fresh_id*/ {{9, 206}, {12, 207}, {21, 208}}, + /*output_id_to_fresh_id*/ {}); + + ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); + transformation.Apply(context.get(), &fact_manager); + ASSERT_TRUE(IsValid(env, context.get())); + // All the original blocks, plus the new function entry block, should be dead. + for (uint32_t block_id : {16u, 23u, 24u, 26u, 27u, 34u, 35u, 50u, 203u}) { + ASSERT_TRUE(fact_manager.BlockIsDead(block_id)); + } +} + +TEST(TransformationOutlineFunctionTest, OutlineWithDeadBlocks2) { + // This checks that if some, but not all, blocks in the outlined region are + // dead, those (but not others) will be dead in the outlined function. + FAIL(); +} + +TEST(TransformationOutlineFunctionTest, + OutlineWithArbitraryVariablesAndParameters) { + // This checks that if the outlined region uses a mixture of arbitrary and + // non-arbitrary variables and parameters, these properties are preserved + // during outlining. + FAIL(); +} + TEST(TransformationOutlineFunctionTest, Miscellaneous1) { // This tests outlining of some non-trivial code. From 74f3693b24d5cf9d4206c220d10eeedfa10b6086 Mon Sep 17 00:00:00 2001 From: Alastair Donaldson Date: Wed, 29 Jan 2020 18:57:27 +0000 Subject: [PATCH 2/2] Added tests. --- .../transformation_outline_function_test.cpp | 151 +++++++++++++++++- 1 file changed, 149 insertions(+), 2 deletions(-) diff --git a/test/fuzz/transformation_outline_function_test.cpp b/test/fuzz/transformation_outline_function_test.cpp index d6bc3f8b05..7313538b09 100644 --- a/test/fuzz/transformation_outline_function_test.cpp +++ b/test/fuzz/transformation_outline_function_test.cpp @@ -2157,7 +2157,83 @@ TEST(TransformationOutlineFunctionTest, OutlineWithDeadBlocks1) { TEST(TransformationOutlineFunctionTest, OutlineWithDeadBlocks2) { // This checks that if some, but not all, blocks in the outlined region are // dead, those (but not others) will be dead in the outlined function. - FAIL(); + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" %8 + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeBool + %7 = OpTypePointer Private %6 + %8 = OpVariable %7 Private + %9 = OpConstantFalse %6 + %10 = OpTypePointer Function %6 + %12 = OpConstantTrue %6 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %11 = OpVariable %10 Function + OpBranch %30 + %30 = OpLabel + OpStore %8 %9 + OpBranch %31 + %31 = OpLabel + OpStore %11 %12 + OpSelectionMerge %36 None + OpBranchConditional %9 %32 %33 + %32 = OpLabel + OpBranch %34 + %33 = OpLabel + OpBranch %36 + %34 = OpLabel + OpBranch %35 + %35 = OpLabel + OpBranch %36 + %36 = OpLabel + OpBranch %37 + %37 = OpLabel + %13 = OpLoad %6 %8 + OpStore %11 %13 + %14 = OpLoad %6 %11 + OpStore %8 %14 + 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; + for (uint32_t block_id : {32u, 34u, 35u}) { + fact_manager.AddFactBlockIsDead(block_id); + } + + TransformationOutlineFunction transformation( + /*entry_block*/ 30, + /*exit_block*/ 37, + /*new_function_struct_return_type_id*/ 200, + /*new_function_type_id*/ 201, + /*new_function_id*/ 202, + /*new_function_region_entry_block*/ 203, + /*new_caller_result_id*/ 204, + /*new_callee_result_id*/ 205, + /*input_id_to_fresh_id*/ {{11, 206}}, + /*output_id_to_fresh_id*/ {}); + + ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); + transformation.Apply(context.get(), &fact_manager); + ASSERT_TRUE(IsValid(env, context.get())); + // The blocks that were originally dead, but not others, should be dead. + for (uint32_t block_id : {32u, 34u, 35u}) { + ASSERT_TRUE(fact_manager.BlockIsDead(block_id)); + } + for (uint32_t block_id : {5u, 30u, 31u, 33u, 36u, 37u, 203u}) { + ASSERT_FALSE(fact_manager.BlockIsDead(block_id)); + } } TEST(TransformationOutlineFunctionTest, @@ -2165,7 +2241,78 @@ TEST(TransformationOutlineFunctionTest, // This checks that if the outlined region uses a mixture of arbitrary and // non-arbitrary variables and parameters, these properties are preserved // during outlining. - FAIL(); + std::string shader = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 310 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 1 + %7 = OpTypePointer Function %6 + %8 = OpTypeFunction %2 %7 %7 + %13 = OpConstant %6 2 + %15 = OpConstant %6 3 + %4 = OpFunction %2 None %3 + %5 = OpLabel + OpReturn + OpFunctionEnd + %11 = OpFunction %2 None %8 + %9 = OpFunctionParameter %7 + %10 = OpFunctionParameter %7 + %12 = OpLabel + %14 = OpVariable %7 Function + %20 = OpVariable %7 Function + OpBranch %50 + %50 = OpLabel + OpStore %9 %13 + OpStore %14 %15 + %16 = OpLoad %6 %14 + OpStore %10 %16 + %17 = OpLoad %6 %9 + %18 = OpLoad %6 %10 + %19 = OpIAdd %6 %17 %18 + OpStore %14 %19 + %21 = OpLoad %6 %9 + OpStore %20 %21 + 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; + fact_manager.AddFactValueOfVariableIsArbitrary(9); + fact_manager.AddFactValueOfVariableIsArbitrary(14); + + TransformationOutlineFunction transformation( + /*entry_block*/ 50, + /*exit_block*/ 50, + /*new_function_struct_return_type_id*/ 200, + /*new_function_type_id*/ 201, + /*new_function_id*/ 202, + /*new_function_region_entry_block*/ 203, + /*new_caller_result_id*/ 204, + /*new_callee_result_id*/ 205, + /*input_id_to_fresh_id*/ {{9, 206}, {10, 207}, {14, 208}, {20, 209}}, + /*output_id_to_fresh_id*/ {}); + + ASSERT_TRUE(transformation.IsApplicable(context.get(), fact_manager)); + transformation.Apply(context.get(), &fact_manager); + ASSERT_TRUE(IsValid(env, context.get())); + // The variables that were originally abitrary, plus input parameters + // corresponding to them, should be arbitrary. The rest should not be. + for (uint32_t variable_id : {9u, 14u, 206u, 208u}) { + ASSERT_TRUE(fact_manager.VariableValueIsArbitrary(variable_id)); + } + for (uint32_t variable_id : {10u, 20u, 207u, 209u}) { + ASSERT_FALSE(fact_manager.BlockIsDead(variable_id)); + } } TEST(TransformationOutlineFunctionTest, Miscellaneous1) {