Warn when input facts are invalid. (#2699)
Fixes #2621.
Instead of aborting when an invalid input fact is provided, the tool
now warns about the invalid fact and then ignores it. This is
convenient for example if facts are specified about uniforms with
descriptor sets and bindings that happen to not be present in the
input binary.
diff --git a/source/fuzz/fact_manager.cpp b/source/fuzz/fact_manager.cpp
index 9cb6896..442ff16 100644
--- a/source/fuzz/fact_manager.cpp
+++ b/source/fuzz/fact_manager.cpp
@@ -14,12 +14,60 @@
#include "source/fuzz/fact_manager.h"
+#include <sstream>
+
#include "source/fuzz/uniform_buffer_element_descriptor.h"
#include "source/opt/ir_context.h"
namespace spvtools {
namespace fuzz {
+namespace {
+
+std::string ToString(const protobufs::Fact& fact) {
+ assert(fact.fact_case() == protobufs::Fact::kConstantUniformFact &&
+ "Right now this is the only fact.");
+ std::stringstream stream;
+ stream << "("
+ << fact.constant_uniform_fact()
+ .uniform_buffer_element_descriptor()
+ .descriptor_set()
+ << ", "
+ << fact.constant_uniform_fact()
+ .uniform_buffer_element_descriptor()
+ .binding()
+ << ")[";
+
+ bool first = true;
+ for (auto index : fact.constant_uniform_fact()
+ .uniform_buffer_element_descriptor()
+ .index()) {
+ if (first) {
+ first = false;
+ } else {
+ stream << ", ";
+ }
+ stream << index;
+ }
+
+ stream << "] == [";
+
+ first = true;
+ for (auto constant_word : fact.constant_uniform_fact().constant_word()) {
+ if (first) {
+ first = false;
+ } else {
+ stream << ", ";
+ }
+ stream << constant_word;
+ }
+
+ stream << "]";
+ return stream.str();
+}
+
+} // namespace
+
// The purpose of this struct is to group the fields and data used to represent
// facts about uniform constants.
struct FactManager::ConstantUniformFacts {
@@ -288,16 +336,16 @@
FactManager::~FactManager() = default;
-bool FactManager::AddFacts(const protobufs::FactSequence& initial_facts,
+void FactManager::AddFacts(const MessageConsumer& message_consumer,
+ const protobufs::FactSequence& initial_facts,
opt::IRContext* context) {
for (auto& fact : initial_facts.fact()) {
if (!AddFact(fact, context)) {
- // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/2621) Provide
- // information about the fact that could not be added.
- return false;
+ message_consumer(
+ SPV_MSG_WARNING, nullptr, {},
+ ("Invalid fact " + ToString(fact) + " ignored.").c_str());
}
}
- return true;
}
bool FactManager::AddFact(const spvtools::fuzz::protobufs::Fact& fact,
diff --git a/source/fuzz/fact_manager.h b/source/fuzz/fact_manager.h
index 2eee9ab..cb4ac58 100644
--- a/source/fuzz/fact_manager.h
+++ b/source/fuzz/fact_manager.h
@@ -41,11 +41,14 @@
~FactManager();
// Adds all the facts from |facts|, checking them for validity with respect to
- // |context|. Returns true if and only if all facts are valid.
- bool AddFacts(const protobufs::FactSequence& facts, opt::IRContext* context);
+ // |context|. Warnings about invalid facts are communicated via
+ // |message_consumer|; such facts are otherwise ignored.
+ void AddFacts(const MessageConsumer& message_consumer,
+ const protobufs::FactSequence& facts, opt::IRContext* context);
- // Adds |fact| to the fact manager, checking it for validity with respect to
- // |context|. Returns true if and only if the fact is valid.
+ // Checks the fact for validity with respect to |context|. Returns false,
+ // with no side effects, if the fact is invalid. Otherwise adds |fact| to the
+ // fact manager.
bool AddFact(const protobufs::Fact& fact, opt::IRContext* context);
// The fact manager will ultimately be responsible for managing a few distinct
diff --git a/source/fuzz/fuzzer.cpp b/source/fuzz/fuzzer.cpp
index 42033e2..606ccd6 100644
--- a/source/fuzz/fuzzer.cpp
+++ b/source/fuzz/fuzzer.cpp
@@ -97,9 +97,7 @@
FuzzerContext fuzzer_context(&random_generator, minimum_fresh_id);
FactManager fact_manager;
- if (!fact_manager.AddFacts(initial_facts, ir_context.get())) {
- return Fuzzer::FuzzerResultStatus::kInitialFactsInvalid;
- }
+ fact_manager.AddFacts(impl_->consumer, initial_facts, ir_context.get());
// Add some essential ingredients to the module if they are not already
// present, such as boolean constants.
diff --git a/source/fuzz/fuzzer.h b/source/fuzz/fuzzer.h
index f81f297..af45d9b 100644
--- a/source/fuzz/fuzzer.h
+++ b/source/fuzz/fuzzer.h
@@ -33,7 +33,6 @@
kComplete,
kFailedToCreateSpirvToolsInterface,
kInitialBinaryInvalid,
- kInitialFactsInvalid,
};
// Constructs a fuzzer from the given target environment.
diff --git a/source/fuzz/replayer.cpp b/source/fuzz/replayer.cpp
index 951b2a5..b0d4ee2 100644
--- a/source/fuzz/replayer.cpp
+++ b/source/fuzz/replayer.cpp
@@ -81,9 +81,7 @@
assert(ir_context);
FactManager fact_manager;
- if (!fact_manager.AddFacts(initial_facts, ir_context.get())) {
- return Replayer::ReplayerResultStatus::kInitialFactsInvalid;
- }
+ fact_manager.AddFacts(impl_->consumer, initial_facts, ir_context.get());
// Consider the transformation proto messages in turn.
for (auto& message : transformation_sequence_in.transformation()) {
diff --git a/source/fuzz/replayer.h b/source/fuzz/replayer.h
index a45cbb4..13391d0 100644
--- a/source/fuzz/replayer.h
+++ b/source/fuzz/replayer.h
@@ -33,7 +33,6 @@
kComplete,
kFailedToCreateSpirvToolsInterface,
kInitialBinaryInvalid,
- kInitialFactsInvalid,
};
// Constructs a replayer from the given target environment.
diff --git a/tools/fuzz/fuzz.cpp b/tools/fuzz/fuzz.cpp
index 9943dff..8967b4a 100644
--- a/tools/fuzz/fuzz.cpp
+++ b/tools/fuzz/fuzz.cpp
@@ -257,7 +257,7 @@
const std::string dot_spv(".spv");
std::string in_facts_file =
in_binary_file.substr(0, in_binary_file.length() - dot_spv.length()) +
- ".json";
+ ".facts";
std::ifstream facts_input(in_facts_file);
if (facts_input) {
std::string facts_json_string((std::istreambuf_iterator<char>(facts_input)),
