spirv-fuzz: Do not expose synonym facts for non-existent ids (#3891)

Fixes #3888.
This commit is contained in:
Alastair Donaldson 2020-10-09 07:18:46 +01:00 committed by GitHub
parent 3602287858
commit c2553a315f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 3 deletions

View File

@ -867,17 +867,30 @@ DataSynonymAndIdEquationFacts::GetSynonymsForId(uint32_t id) const {
std::vector<const protobufs::DataDescriptor*>
DataSynonymAndIdEquationFacts::GetSynonymsForDataDescriptor(
const protobufs::DataDescriptor& data_descriptor) const {
std::vector<const protobufs::DataDescriptor*> result;
if (synonymous_.Exists(data_descriptor)) {
return synonymous_.GetEquivalenceClass(data_descriptor);
for (auto dd : synonymous_.GetEquivalenceClass(data_descriptor)) {
// There may be data descriptors in the equivalence class whose base
// objects have been removed from the module. We do not expose these
// data descriptors to clients of the fact manager.
if (ir_context_->get_def_use_mgr()->GetDef(dd->object()) != nullptr) {
result.push_back(dd);
}
}
}
return {};
return result;
}
std::vector<uint32_t>
DataSynonymAndIdEquationFacts::GetIdsForWhichSynonymsAreKnown() const {
std::vector<uint32_t> result;
for (auto& data_descriptor : synonymous_.GetAllKnownValues()) {
if (data_descriptor->index().empty()) {
// We skip any data descriptors whose base objects no longer exist in the
// module, and we restrict attention to data descriptors for plain ids,
// which have no indices.
if (ir_context_->get_def_use_mgr()->GetDef(data_descriptor->object()) !=
nullptr &&
data_descriptor->index().empty()) {
result.push_back(data_descriptor->object());
}
}

View File

@ -20,6 +20,23 @@ namespace spvtools {
namespace fuzz {
namespace {
void CheckConsistencyOfSynonymFacts(
opt::IRContext* ir_context,
const TransformationContext& transformation_context) {
for (uint32_t id : transformation_context.GetFactManager()
->GetIdsForWhichSynonymsAreKnown()) {
// Every id reported by the fact manager should exist in the module.
ASSERT_NE(ir_context->get_def_use_mgr()->GetDef(id), nullptr);
auto synonyms =
transformation_context.GetFactManager()->GetSynonymsForId(id);
for (auto& dd : synonyms) {
// Every reported synonym should have a base object that exists in the
// module.
ASSERT_NE(ir_context->get_def_use_mgr()->GetDef(dd->object()), nullptr);
}
}
}
TEST(DataSynonymAndIdEquationFactsTest, RecursiveAdditionOfFacts) {
std::string shader = R"(
OpCapability Shader
@ -192,6 +209,8 @@ TEST(DataSynonymAndIdEquationFactsTest, HandlesCorollariesWithInvalidIds) {
transformation_context.GetFactManager()->AddFactDataSynonym(
MakeDataDescriptor(14, {}), MakeDataDescriptor(17, {}));
CheckConsistencyOfSynonymFacts(context.get(), transformation_context);
// Apply TransformationMergeBlocks which will remove %17 from the module.
TransformationMergeBlocks transformation(16);
ASSERT_TRUE(
@ -199,6 +218,8 @@ TEST(DataSynonymAndIdEquationFactsTest, HandlesCorollariesWithInvalidIds) {
transformation.Apply(context.get(), &transformation_context);
ASSERT_TRUE(IsValid(env, context.get()));
CheckConsistencyOfSynonymFacts(context.get(), transformation_context);
ASSERT_EQ(context->get_def_use_mgr()->GetDef(17), nullptr);
// Add another equation.
@ -212,6 +233,8 @@ TEST(DataSynonymAndIdEquationFactsTest, HandlesCorollariesWithInvalidIds) {
ASSERT_TRUE(transformation_context.GetFactManager()->IsSynonymous(
MakeDataDescriptor(15, {}), MakeDataDescriptor(14, {})));
CheckConsistencyOfSynonymFacts(context.get(), transformation_context);
// Remove some instructions from the module. At this point, the equivalence
// class of %14 has no valid members.
ASSERT_TRUE(context->KillDef(14));
@ -220,6 +243,8 @@ TEST(DataSynonymAndIdEquationFactsTest, HandlesCorollariesWithInvalidIds) {
transformation_context.GetFactManager()->AddFactIdEquation(
18, SpvOpConvertSToF, {9});
CheckConsistencyOfSynonymFacts(context.get(), transformation_context);
// We don't create synonyms if at least one of the equivalence classes has no
// valid members.
ASSERT_FALSE(transformation_context.GetFactManager()->IsSynonymous(