From 69f07da41b3bc4f563fade2b2d9aeab7f3329ae4 Mon Sep 17 00:00:00 2001 From: Alastair Donaldson Date: Thu, 22 Oct 2020 23:23:59 +0100 Subject: [PATCH] spirv-fuzz: Fix mismatch with shrinker step limit (#3985) Fixes #3984. --- source/fuzz/added_function_reducer.cpp | 17 +++- source/fuzz/added_function_reducer.h | 7 +- test/fuzz/shrinker_test.cpp | 114 ++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 8 deletions(-) diff --git a/source/fuzz/added_function_reducer.cpp b/source/fuzz/added_function_reducer.cpp index 0ea88c885..e7cb027b9 100644 --- a/source/fuzz/added_function_reducer.cpp +++ b/source/fuzz/added_function_reducer.cpp @@ -45,7 +45,7 @@ AddedFunctionReducer::AddedFunctionReducer( validator_options_(validator_options), shrinker_step_limit_(shrinker_step_limit), num_existing_shrink_attempts_(num_existing_shrink_attempts), - num_reduction_attempts_(0) {} + num_reducer_interestingness_function_invocations_(0) {} AddedFunctionReducer::~AddedFunctionReducer() = default; @@ -99,16 +99,25 @@ AddedFunctionReducer::AddedFunctionReducerResult AddedFunctionReducer::Run() { protobufs::TransformationSequence transformation_sequence_out; ReplayAdaptedTransformations(reduced_binary, &binary_out, &transformation_sequence_out); + // We subtract 1 from |num_reducer_interestingness_function_invocations_| to + // account for the fact that spirv-reduce invokes its interestingness test + // once before reduction commences in order to check that the initial module + // is interesting. + assert(num_reducer_interestingness_function_invocations_ > 0 && + "At a minimum spirv-reduce should have invoked its interestingness " + "test once."); return {AddedFunctionReducerResultStatus::kComplete, std::move(binary_out), - std::move(transformation_sequence_out), num_reduction_attempts_}; + std::move(transformation_sequence_out), + num_reducer_interestingness_function_invocations_ - 1}; } bool AddedFunctionReducer::InterestingnessFunctionForReducingAddedFunction( const std::vector& binary_under_reduction, const std::unordered_set& irrelevant_pointee_global_variables) { uint32_t counter_for_shrinker_interestingness_function = - num_existing_shrink_attempts_ + num_reduction_attempts_; - num_reduction_attempts_++; + num_existing_shrink_attempts_ + + num_reducer_interestingness_function_invocations_; + num_reducer_interestingness_function_invocations_++; // The reduced version of the added function must be limited to accessing // global variables appearing in |irrelevant_pointee_global_variables|. This diff --git a/source/fuzz/added_function_reducer.h b/source/fuzz/added_function_reducer.h index 9dcc63207..3efd2684f 100644 --- a/source/fuzz/added_function_reducer.h +++ b/source/fuzz/added_function_reducer.h @@ -181,9 +181,10 @@ class AddedFunctionReducer { // AddedFunctionReducer instance. const uint32_t num_existing_shrink_attempts_; - // Tracks the number of attempts that spirv-reduce has made in reducing the - // added function. - uint32_t num_reduction_attempts_; + // Tracks the number of attempts that spirv-reduce has invoked its + // interestingness function, which it does once at the start of reduction, + // and then once more each time it makes a reduction step. + uint32_t num_reducer_interestingness_function_invocations_; }; } // namespace fuzz diff --git a/test/fuzz/shrinker_test.cpp b/test/fuzz/shrinker_test.cpp index 1ea2c16a8..42cd182cb 100644 --- a/test/fuzz/shrinker_test.cpp +++ b/test/fuzz/shrinker_test.cpp @@ -144,7 +144,7 @@ TEST(ShrinkerTest, ReduceAddedFunctions) { // compilers are kept happy. See: // https://developercommunity.visualstudio.com/content/problem/367326/problems-with-capturing-constexpr-in-lambda.html spv_target_env env = SPV_ENV_UNIVERSAL_1_3; - const auto consumer = kConsoleMessageConsumer; + const auto consumer = fuzzerutil::kSilentMessageConsumer; SpirvTools tools(env); std::vector reference_binary; @@ -266,6 +266,118 @@ TEST(ShrinkerTest, ReduceAddedFunctions) { } } +TEST(ShrinkerTest, HitStepLimitWhenReducingAddedFunctions) { + const std::string kReferenceModule = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 1 + %7 = OpTypePointer Private %6 + %8 = OpVariable %7 Private + %9 = OpConstant %6 2 + %10 = OpTypePointer Function %6 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %11 = OpVariable %10 Function + OpStore %8 %9 + %12 = OpLoad %6 %8 + OpStore %11 %12 + OpReturn + OpFunctionEnd + )"; + + const std::string kDonorModule = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %6 = OpTypeInt 32 1 + %48 = OpConstant %6 3 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %52 = OpCopyObject %6 %48 + %53 = OpCopyObject %6 %52 + %54 = OpCopyObject %6 %53 + %55 = OpCopyObject %6 %54 + %56 = OpCopyObject %6 %55 + %57 = OpCopyObject %6 %56 + %58 = OpCopyObject %6 %48 + %59 = OpCopyObject %6 %58 + %60 = OpCopyObject %6 %59 + %61 = OpCopyObject %6 %60 + %62 = OpCopyObject %6 %61 + %63 = OpCopyObject %6 %62 + %64 = OpCopyObject %6 %48 + OpReturn + OpFunctionEnd + )"; + + spv_target_env env = SPV_ENV_UNIVERSAL_1_3; + const auto consumer = fuzzerutil::kSilentMessageConsumer; + + SpirvTools tools(env); + std::vector reference_binary; + ASSERT_TRUE( + tools.Assemble(kReferenceModule, &reference_binary, kFuzzAssembleOption)); + + spvtools::ValidatorOptions validator_options; + + const auto variant_ir_context = + BuildModule(env, consumer, kReferenceModule, kFuzzAssembleOption); + ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed( + variant_ir_context.get(), validator_options, kConsoleMessageConsumer)); + + const auto donor_ir_context = + BuildModule(env, consumer, kDonorModule, kFuzzAssembleOption); + ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed( + donor_ir_context.get(), validator_options, kConsoleMessageConsumer)); + + PseudoRandomGenerator random_generator(0); + FuzzerContext fuzzer_context(&random_generator, 100); + TransformationContext transformation_context( + MakeUnique(variant_ir_context.get()), validator_options); + + protobufs::TransformationSequence transformations; + FuzzerPassDonateModules pass(variant_ir_context.get(), + &transformation_context, &fuzzer_context, + &transformations, {}); + pass.DonateSingleModule(donor_ir_context.get(), true); + + protobufs::FactSequence no_facts; + + Shrinker::InterestingnessFunction interestingness_function = + [consumer, env](const std::vector& binary, + uint32_t /*unused*/) -> bool { + auto temp_ir_context = + BuildModule(env, consumer, binary.data(), binary.size()); + uint32_t copy_object_count = 0; + temp_ir_context->module()->ForEachInst( + [©_object_count](opt::Instruction* inst) { + if (inst->opcode() == SpvOpCopyObject) { + copy_object_count++; + } + + }); + return copy_object_count >= 8; + }; + + auto shrinker_result = + Shrinker(env, consumer, reference_binary, no_facts, transformations, + interestingness_function, 30, true, validator_options) + .Run(); + ASSERT_EQ(Shrinker::ShrinkerResultStatus::kStepLimitReached, + shrinker_result.status); +} + } // namespace } // namespace fuzz } // namespace spvtools