spirv-fuzz: Remove opaque pointer design pattern (#3755)
There's no real need for Fuzzer, Replayer and Shrinker to use the
opaque pointer design pattern. This change removes it, paving the way
for making some upcoming changes to Fuzzer easier.
diff --git a/source/fuzz/fuzzer.cpp b/source/fuzz/fuzzer.cpp
index e35fc8b..4cf3ba3 100644
--- a/source/fuzz/fuzzer.cpp
+++ b/source/fuzz/fuzzer.cpp
@@ -16,7 +16,6 @@
#include <cassert>
#include <memory>
-#include <sstream>
#include "source/fuzz/fact_manager/fact_manager.h"
#include "source/fuzz/fuzzer_context.h"
@@ -112,51 +111,32 @@
} // namespace
-struct Fuzzer::Impl {
- Impl(spv_target_env env, uint32_t random_seed, bool validate_after_each_pass,
- spv_validator_options options)
- : target_env(env),
- seed(random_seed),
- validate_after_each_fuzzer_pass(validate_after_each_pass),
- validator_options(options) {}
-
- bool ApplyPassAndCheckValidity(FuzzerPass* pass,
- const opt::IRContext& ir_context,
- const spvtools::SpirvTools& tools) const;
-
- const spv_target_env target_env; // Target environment.
- MessageConsumer consumer; // Message consumer.
- const uint32_t seed; // Seed for random number generator.
- bool validate_after_each_fuzzer_pass; // Determines whether the validator
- // should be invoked after every fuzzer
- // pass.
- spv_validator_options validator_options; // Options to control validation.
-};
-
-Fuzzer::Fuzzer(spv_target_env env, uint32_t seed,
+Fuzzer::Fuzzer(spv_target_env target_env, uint32_t seed,
bool validate_after_each_fuzzer_pass,
spv_validator_options validator_options)
- : impl_(MakeUnique<Impl>(env, seed, validate_after_each_fuzzer_pass,
- validator_options)) {}
+ : target_env_(target_env),
+ seed_(seed),
+ validate_after_each_fuzzer_pass_(validate_after_each_fuzzer_pass),
+ validator_options_(validator_options) {}
Fuzzer::~Fuzzer() = default;
-void Fuzzer::SetMessageConsumer(MessageConsumer c) {
- impl_->consumer = std::move(c);
+void Fuzzer::SetMessageConsumer(MessageConsumer consumer) {
+ consumer_ = std::move(consumer);
}
-bool Fuzzer::Impl::ApplyPassAndCheckValidity(
+bool Fuzzer::ApplyPassAndCheckValidity(
FuzzerPass* pass, const opt::IRContext& ir_context,
const spvtools::SpirvTools& tools) const {
pass->Apply();
- if (validate_after_each_fuzzer_pass) {
+ if (validate_after_each_fuzzer_pass_) {
std::vector<uint32_t> binary_to_validate;
ir_context.module()->ToBinary(&binary_to_validate, false);
if (!tools.Validate(&binary_to_validate[0], binary_to_validate.size(),
- validator_options)) {
- consumer(SPV_MSG_INFO, nullptr, {},
- "Binary became invalid during fuzzing (set a breakpoint to "
- "inspect); stopping.");
+ validator_options_)) {
+ consumer_(SPV_MSG_INFO, nullptr, {},
+ "Binary became invalid during fuzzing (set a breakpoint to "
+ "inspect); stopping.");
return false;
}
}
@@ -173,29 +153,28 @@
// header files being used.
GOOGLE_PROTOBUF_VERIFY_VERSION;
- spvtools::SpirvTools tools(impl_->target_env);
- tools.SetMessageConsumer(impl_->consumer);
+ spvtools::SpirvTools tools(target_env_);
+ tools.SetMessageConsumer(consumer_);
if (!tools.IsValid()) {
- impl_->consumer(SPV_MSG_ERROR, nullptr, {},
- "Failed to create SPIRV-Tools interface; stopping.");
+ consumer_(SPV_MSG_ERROR, nullptr, {},
+ "Failed to create SPIRV-Tools interface; stopping.");
return Fuzzer::FuzzerResultStatus::kFailedToCreateSpirvToolsInterface;
}
// Initial binary should be valid.
- if (!tools.Validate(&binary_in[0], binary_in.size(),
- impl_->validator_options)) {
- impl_->consumer(SPV_MSG_ERROR, nullptr, {},
- "Initial binary is invalid; stopping.");
+ if (!tools.Validate(&binary_in[0], binary_in.size(), validator_options_)) {
+ consumer_(SPV_MSG_ERROR, nullptr, {},
+ "Initial binary is invalid; stopping.");
return Fuzzer::FuzzerResultStatus::kInitialBinaryInvalid;
}
// Build the module from the input binary.
- std::unique_ptr<opt::IRContext> ir_context = BuildModule(
- impl_->target_env, impl_->consumer, binary_in.data(), binary_in.size());
+ std::unique_ptr<opt::IRContext> ir_context =
+ BuildModule(target_env_, consumer_, binary_in.data(), binary_in.size());
assert(ir_context);
// Make a PRNG from the seed passed to the fuzzer on creation.
- PseudoRandomGenerator random_generator(impl_->seed);
+ PseudoRandomGenerator random_generator(seed_);
// The fuzzer will introduce new ids into the module. The module's id bound
// gives the smallest id that can be used for this purpose. We add an offset
@@ -208,9 +187,9 @@
FuzzerContext fuzzer_context(&random_generator, minimum_fresh_id);
FactManager fact_manager;
- fact_manager.AddFacts(impl_->consumer, initial_facts, ir_context.get());
+ fact_manager.AddFacts(consumer_, initial_facts, ir_context.get());
TransformationContext transformation_context(&fact_manager,
- impl_->validator_options);
+ validator_options_);
// Apply some semantics-preserving passes.
std::vector<std::unique_ptr<FuzzerPass>> passes;
@@ -356,7 +335,7 @@
(is_first ||
fuzzer_context.ChoosePercentage(kChanceOfApplyingAnotherPass))) {
is_first = false;
- if (!impl_->ApplyPassAndCheckValidity(
+ if (!ApplyPassAndCheckValidity(
passes[fuzzer_context.RandomIndex(passes)].get(), *ir_context,
tools)) {
return Fuzzer::FuzzerResultStatus::kFuzzerPassLedToInvalidModule;
@@ -400,7 +379,7 @@
&final_passes, ir_context.get(), &transformation_context, &fuzzer_context,
transformation_sequence_out);
for (auto& pass : final_passes) {
- if (!impl_->ApplyPassAndCheckValidity(pass.get(), *ir_context, tools)) {
+ if (!ApplyPassAndCheckValidity(pass.get(), *ir_context, tools)) {
return Fuzzer::FuzzerResultStatus::kFuzzerPassLedToInvalidModule;
}
}
diff --git a/source/fuzz/fuzzer.h b/source/fuzz/fuzzer.h
index 6c3ef71..a437d58 100644
--- a/source/fuzz/fuzzer.h
+++ b/source/fuzz/fuzzer.h
@@ -18,6 +18,7 @@
#include <memory>
#include <vector>
+#include "source/fuzz/fuzzer_pass.h"
#include "source/fuzz/fuzzer_util.h"
#include "source/fuzz/protobufs/spirvfuzz_protobufs.h"
#include "spirv-tools/libspirv.hpp"
@@ -37,11 +38,11 @@
kInitialBinaryInvalid,
};
- // Constructs a fuzzer from the given target environment |env|. |seed| is a
- // seed for pseudo-random number generation.
+ // Constructs a fuzzer from the given target environment |target_env|. |seed|
+ // is a seed for pseudo-random number generation.
// |validate_after_each_fuzzer_pass| controls whether the validator will be
// invoked after every fuzzer pass is applied.
- Fuzzer(spv_target_env env, uint32_t seed,
+ Fuzzer(spv_target_env target_env, uint32_t seed,
bool validate_after_each_fuzzer_pass,
spv_validator_options validator_options);
@@ -71,8 +72,27 @@
protobufs::TransformationSequence* transformation_sequence_out) const;
private:
- struct Impl; // Opaque struct for holding internal data.
- std::unique_ptr<Impl> impl_; // Unique pointer to internal data.
+ // Applies |pass|, which must be a pass constructed with |ir_context|, and
+ // then returns true if and only if |ir_context| is valid. |tools| is used to
+ // check validity.
+ bool ApplyPassAndCheckValidity(FuzzerPass* pass,
+ const opt::IRContext& ir_context,
+ const spvtools::SpirvTools& tools) const;
+
+ // Target environment.
+ const spv_target_env target_env_;
+
+ // Message consumer.
+ MessageConsumer consumer_;
+
+ // Seed for random number generator.
+ const uint32_t seed_;
+
+ // Determines whether the validator should be invoked after every fuzzer pass.
+ bool validate_after_each_fuzzer_pass_;
+
+ // Options to control validation.
+ spv_validator_options validator_options_;
};
} // namespace fuzz
diff --git a/source/fuzz/replayer.cpp b/source/fuzz/replayer.cpp
index 3a49f55..d439b9d 100644
--- a/source/fuzz/replayer.cpp
+++ b/source/fuzz/replayer.cpp
@@ -28,28 +28,16 @@
namespace spvtools {
namespace fuzz {
-struct Replayer::Impl {
- Impl(spv_target_env env, bool validate, spv_validator_options options)
- : target_env(env),
- validate_during_replay(validate),
- validator_options(options) {}
-
- const spv_target_env target_env; // Target environment.
- MessageConsumer consumer; // Message consumer.
- const bool validate_during_replay; // Controls whether the validator should
- // be run after every replay step.
- spv_validator_options validator_options; // Options to control
- // validation
-};
-
-Replayer::Replayer(spv_target_env env, bool validate_during_replay,
+Replayer::Replayer(spv_target_env target_env, bool validate_during_replay,
spv_validator_options validator_options)
- : impl_(MakeUnique<Impl>(env, validate_during_replay, validator_options)) {}
+ : target_env_(target_env),
+ validate_during_replay_(validate_during_replay),
+ validator_options_(validator_options) {}
Replayer::~Replayer() = default;
-void Replayer::SetMessageConsumer(MessageConsumer c) {
- impl_->consumer = std::move(c);
+void Replayer::SetMessageConsumer(MessageConsumer consumer) {
+ consumer_ = std::move(consumer);
}
Replayer::ReplayerResultStatus Replayer::Run(
@@ -65,47 +53,45 @@
if (num_transformations_to_apply >
static_cast<uint32_t>(transformation_sequence_in.transformation_size())) {
- impl_->consumer(SPV_MSG_ERROR, nullptr, {},
- "The number of transformations to be replayed must not "
- "exceed the size of the transformation sequence.");
+ consumer_(SPV_MSG_ERROR, nullptr, {},
+ "The number of transformations to be replayed must not "
+ "exceed the size of the transformation sequence.");
return Replayer::ReplayerResultStatus::kTooManyTransformationsRequested;
}
- spvtools::SpirvTools tools(impl_->target_env);
+ spvtools::SpirvTools tools(target_env_);
if (!tools.IsValid()) {
- impl_->consumer(SPV_MSG_ERROR, nullptr, {},
- "Failed to create SPIRV-Tools interface; stopping.");
+ consumer_(SPV_MSG_ERROR, nullptr, {},
+ "Failed to create SPIRV-Tools interface; stopping.");
return Replayer::ReplayerResultStatus::kFailedToCreateSpirvToolsInterface;
}
// Initial binary should be valid.
- if (!tools.Validate(&binary_in[0], binary_in.size(),
- impl_->validator_options)) {
- impl_->consumer(SPV_MSG_INFO, nullptr, {},
- "Initial binary is invalid; stopping.");
+ if (!tools.Validate(&binary_in[0], binary_in.size(), validator_options_)) {
+ consumer_(SPV_MSG_INFO, nullptr, {},
+ "Initial binary is invalid; stopping.");
return Replayer::ReplayerResultStatus::kInitialBinaryInvalid;
}
// Build the module from the input binary.
- std::unique_ptr<opt::IRContext> ir_context = BuildModule(
- impl_->target_env, impl_->consumer, binary_in.data(), binary_in.size());
+ std::unique_ptr<opt::IRContext> ir_context =
+ BuildModule(target_env_, consumer_, binary_in.data(), binary_in.size());
assert(ir_context);
// For replay validation, we track the last valid SPIR-V binary that was
// observed. Initially this is the input binary.
std::vector<uint32_t> last_valid_binary;
- if (impl_->validate_during_replay) {
+ if (validate_during_replay_) {
last_valid_binary = binary_in;
}
FactManager fact_manager;
- fact_manager.AddFacts(impl_->consumer, initial_facts, ir_context.get());
+ fact_manager.AddFacts(consumer_, initial_facts, ir_context.get());
std::unique_ptr<TransformationContext> transformation_context =
first_overflow_id == 0
- ? MakeUnique<TransformationContext>(&fact_manager,
- impl_->validator_options)
+ ? MakeUnique<TransformationContext>(&fact_manager, validator_options_)
: MakeUnique<TransformationContext>(
- &fact_manager, impl_->validator_options,
+ &fact_manager, validator_options_,
MakeUnique<CounterOverflowIdSource>(first_overflow_id));
// We track the largest id bound observed, to ensure that it only increases
@@ -136,16 +122,16 @@
"transformations.");
max_observed_id_bound = ir_context->module()->id_bound();
- if (impl_->validate_during_replay) {
+ if (validate_during_replay_) {
std::vector<uint32_t> binary_to_validate;
ir_context->module()->ToBinary(&binary_to_validate, false);
// Check whether the latest transformation led to a valid binary.
if (!tools.Validate(&binary_to_validate[0], binary_to_validate.size(),
- impl_->validator_options)) {
- impl_->consumer(SPV_MSG_INFO, nullptr, {},
- "Binary became invalid during replay (set a "
- "breakpoint to inspect); stopping.");
+ validator_options_)) {
+ consumer_(SPV_MSG_INFO, nullptr, {},
+ "Binary became invalid during replay (set a "
+ "breakpoint to inspect); stopping.");
return Replayer::ReplayerResultStatus::kReplayValidationFailure;
}
diff --git a/source/fuzz/replayer.h b/source/fuzz/replayer.h
index 23e39d0..a10e536 100644
--- a/source/fuzz/replayer.h
+++ b/source/fuzz/replayer.h
@@ -38,7 +38,7 @@
};
// Constructs a replayer from the given target environment.
- Replayer(spv_target_env env, bool validate_during_replay,
+ Replayer(spv_target_env target_env, bool validate_during_replay,
spv_validator_options validator_options);
// Disables copy/move constructor/assignment operations.
@@ -76,8 +76,17 @@
protobufs::TransformationSequence* transformation_sequence_out) const;
private:
- struct Impl; // Opaque struct for holding internal data.
- std::unique_ptr<Impl> impl_; // Unique pointer to internal data.
+ // Target environment.
+ const spv_target_env target_env_;
+
+ // Message consumer.
+ MessageConsumer consumer_;
+
+ // Controls whether the validator should be run after every replay step.
+ const bool validate_during_replay_;
+
+ // Options to control validation
+ spv_validator_options validator_options_;
};
} // namespace fuzz
diff --git a/source/fuzz/shrinker.cpp b/source/fuzz/shrinker.cpp
index dcdf54a..7b88405 100644
--- a/source/fuzz/shrinker.cpp
+++ b/source/fuzz/shrinker.cpp
@@ -61,45 +61,26 @@
} // namespace
-struct Shrinker::Impl {
- Impl(spv_target_env env, uint32_t limit, bool validate,
- spv_validator_options options)
- : target_env(env),
- step_limit(limit),
- validate_during_replay(validate),
- validator_options(options) {}
-
- // Returns the id bound for the given SPIR-V binary, which is assumed to be
- // valid.
- uint32_t GetIdBound(const std::vector<uint32_t>& binary);
-
- const spv_target_env target_env; // Target environment.
- MessageConsumer consumer; // Message consumer.
- const uint32_t step_limit; // Step limit for reductions.
- const bool validate_during_replay; // Determines whether to check for
- // validity during the replaying of
- // transformations.
- spv_validator_options validator_options; // Options to control validation.
-};
-
-uint32_t Shrinker::Impl::GetIdBound(const std::vector<uint32_t>& binary) {
+uint32_t Shrinker::GetIdBound(const std::vector<uint32_t>& binary) const {
// Build the module from the input binary.
std::unique_ptr<opt::IRContext> ir_context =
- BuildModule(target_env, consumer, binary.data(), binary.size());
+ BuildModule(target_env_, consumer_, binary.data(), binary.size());
assert(ir_context && "Error building module.");
return ir_context->module()->id_bound();
}
-Shrinker::Shrinker(spv_target_env env, uint32_t step_limit,
+Shrinker::Shrinker(spv_target_env target_env, uint32_t step_limit,
bool validate_during_replay,
spv_validator_options validator_options)
- : impl_(MakeUnique<Impl>(env, step_limit, validate_during_replay,
- validator_options)) {}
+ : target_env_(target_env),
+ step_limit_(step_limit),
+ validate_during_replay_(validate_during_replay),
+ validator_options_(validator_options) {}
Shrinker::~Shrinker() = default;
-void Shrinker::SetMessageConsumer(MessageConsumer c) {
- impl_->consumer = std::move(c);
+void Shrinker::SetMessageConsumer(MessageConsumer consumer) {
+ consumer_ = std::move(consumer);
}
Shrinker::ShrinkerResultStatus Shrinker::Run(
@@ -113,18 +94,17 @@
// header files being used.
GOOGLE_PROTOBUF_VERIFY_VERSION;
- SpirvTools tools(impl_->target_env);
+ SpirvTools tools(target_env_);
if (!tools.IsValid()) {
- impl_->consumer(SPV_MSG_ERROR, nullptr, {},
- "Failed to create SPIRV-Tools interface; stopping.");
+ consumer_(SPV_MSG_ERROR, nullptr, {},
+ "Failed to create SPIRV-Tools interface; stopping.");
return Shrinker::ShrinkerResultStatus::kFailedToCreateSpirvToolsInterface;
}
// Initial binary should be valid.
- if (!tools.Validate(&binary_in[0], binary_in.size(),
- impl_->validator_options)) {
- impl_->consumer(SPV_MSG_INFO, nullptr, {},
- "Initial binary is invalid; stopping.");
+ if (!tools.Validate(&binary_in[0], binary_in.size(), validator_options_)) {
+ consumer_(SPV_MSG_INFO, nullptr, {},
+ "Initial binary is invalid; stopping.");
return Shrinker::ShrinkerResultStatus::kInitialBinaryInvalid;
}
@@ -135,9 +115,8 @@
// succeeds, (b) get the binary that results from running these
// transformations, and (c) get the subsequence of the initial transformations
// that actually apply (in principle this could be a strict subsequence).
- Replayer replayer(impl_->target_env, impl_->validate_during_replay,
- impl_->validator_options);
- replayer.SetMessageConsumer(impl_->consumer);
+ Replayer replayer(target_env_, validate_during_replay_, validator_options_);
+ replayer.SetMessageConsumer(consumer_);
if (replayer.Run(binary_in, initial_facts, transformation_sequence_in,
static_cast<uint32_t>(
transformation_sequence_in.transformation_size()),
@@ -150,15 +129,15 @@
// Check that the binary produced by applying the initial transformations is
// indeed interesting.
if (!interestingness_function(current_best_binary, 0)) {
- impl_->consumer(SPV_MSG_INFO, nullptr, {},
- "Initial binary is not interesting; stopping.");
+ consumer_(SPV_MSG_INFO, nullptr, {},
+ "Initial binary is not interesting; stopping.");
return ShrinkerResultStatus::kInitialBinaryNotInteresting;
}
// The largest id used by the module before any shrinking has been applied
// serves as the first id that can be used for overflow purposes.
- const uint32_t first_overflow_id = impl_->GetIdBound(current_best_binary);
- assert(first_overflow_id >= impl_->GetIdBound(binary_in) &&
+ const uint32_t first_overflow_id = GetIdBound(current_best_binary);
+ assert(first_overflow_id >= GetIdBound(binary_in) &&
"Applying transformations should only increase a module's id bound.");
uint32_t attempt = 0; // Keeps track of the number of shrink attempts that
@@ -174,7 +153,7 @@
// - reach the step limit,
// - run out of transformations to remove, or
// - cannot make the chunk size any smaller.
- while (attempt < impl_->step_limit &&
+ while (attempt < step_limit_ &&
!current_best_transformations.transformation().empty() &&
chunk_size > 0) {
bool progress_this_round =
@@ -204,7 +183,7 @@
// |chunk_size|, using |chunk_index| to track which chunk to try removing
// next. The loop exits early if we reach the shrinking step limit.
for (int chunk_index = num_chunks - 1;
- attempt < impl_->step_limit && chunk_index >= 0; chunk_index--) {
+ attempt < step_limit_ && chunk_index >= 0; chunk_index--) {
// Remove a chunk of transformations according to the current index and
// chunk size.
auto transformations_with_chunk_removed =
@@ -265,12 +244,12 @@
// Indicate whether shrinking completed or was truncated due to reaching the
// step limit.
- assert(attempt <= impl_->step_limit);
- if (attempt == impl_->step_limit) {
+ assert(attempt <= step_limit_);
+ if (attempt == step_limit_) {
std::stringstream strstream;
- strstream << "Shrinking did not complete; step limit " << impl_->step_limit
+ strstream << "Shrinking did not complete; step limit " << step_limit_
<< " was reached.";
- impl_->consumer(SPV_MSG_WARNING, nullptr, {}, strstream.str().c_str());
+ consumer_(SPV_MSG_WARNING, nullptr, {}, strstream.str().c_str());
return Shrinker::ShrinkerResultStatus::kStepLimitReached;
}
return Shrinker::ShrinkerResultStatus::kComplete;
diff --git a/source/fuzz/shrinker.h b/source/fuzz/shrinker.h
index 17b15bf..0fe8929 100644
--- a/source/fuzz/shrinker.h
+++ b/source/fuzz/shrinker.h
@@ -49,8 +49,8 @@
using InterestingnessFunction = std::function<bool(
const std::vector<uint32_t>& binary, uint32_t counter)>;
- // Constructs a shrinker from the given target environment.
- Shrinker(spv_target_env env, uint32_t step_limit, bool validate_during_replay,
+ Shrinker(spv_target_env target_env, uint32_t step_limit,
+ bool validate_during_replay,
spv_validator_options validator_options);
// Disables copy/move constructor/assignment operations.
@@ -82,8 +82,25 @@
protobufs::TransformationSequence* transformation_sequence_out) const;
private:
- struct Impl; // Opaque struct for holding internal data.
- std::unique_ptr<Impl> impl_; // Unique pointer to internal data.
+ // Returns the id bound for the given SPIR-V binary, which is assumed to be
+ // valid.
+ uint32_t GetIdBound(const std::vector<uint32_t>& binary) const;
+
+ // Target environment.
+ const spv_target_env target_env_;
+
+ // Message consumer.
+ MessageConsumer consumer_;
+
+ // Step limit to decide when to terminate shrinking early.
+ const uint32_t step_limit_;
+
+ // Determines whether to check for validity during the replaying of
+ // transformations.
+ const bool validate_during_replay_;
+
+ // Options to control validation.
+ spv_validator_options validator_options_;
};
} // namespace fuzz
