spirv-fuzz: Handle invalid ids in fact manager (#3742)

Fixes #3741.

source/fuzz/fact_manager/data_synonym_and_id_equation_facts.cpp

@@ -281,14 +281,9 @@ void DataSynonymAndIdEquationFacts::ComputeConversionDataSynonymFacts(
   assert(synonymous_.Exists(dd) &&
          "|dd| should've been registered in the equivalence relation");
 
-  const auto* representative = synonymous_.Find(&dd);
-  assert(representative &&
-         "Representative can't be null for a registered descriptor");
-
   const auto* type =
       context->get_type_mgr()->GetType(fuzzerutil::WalkCompositeTypeIndices(
-          context, fuzzerutil::GetTypeId(context, representative->object()),
+          context, fuzzerutil::GetTypeId(context, dd.object()), dd.index()));
-          representative->index()));
   assert(type && "Data descriptor has invalid type");
 
   if ((type->AsVector() && type->AsVector()->element_type()->AsInteger()) ||
@@ -300,24 +295,36 @@ void DataSynonymAndIdEquationFacts::ComputeConversionDataSynonymFacts(
     std::vector<const protobufs::DataDescriptor*> convert_u_to_f_lhs;
 
     for (const auto& fact : id_equations_) {
+      auto equivalence_class = synonymous_.GetEquivalenceClass(*fact.first);
+      auto dd_it = std::find_if(
+          equivalence_class.begin(), equivalence_class.end(),
+          [context](const protobufs::DataDescriptor* a) {
+            return context->get_def_use_mgr()->GetDef(a->object()) != nullptr;
+          });
+      if (dd_it == equivalence_class.end()) {
+        // Skip |equivalence_class| if it has no valid ids.
+        continue;
+      }
+
       for (const auto& equation : fact.second) {
-        if (synonymous_.IsEquivalent(*equation.operands[0], *representative)) {
+        if (synonymous_.IsEquivalent(*equation.operands[0], dd)) {
           if (equation.opcode == SpvOpConvertSToF) {
-            convert_s_to_f_lhs.push_back(fact.first);
+            convert_s_to_f_lhs.push_back(*dd_it);
           } else if (equation.opcode == SpvOpConvertUToF) {
-            convert_u_to_f_lhs.push_back(fact.first);
+            convert_u_to_f_lhs.push_back(*dd_it);
           }
         }
       }
     }
 
-    for (const auto& synonyms :
+    // We use pointers in the initializer list here since otherwise we would
-         {std::move(convert_s_to_f_lhs), std::move(convert_u_to_f_lhs)}) {
+    // copy memory from these vectors.
-      for (const auto* synonym_a : synonyms) {
+    for (const auto* synonyms : {&convert_s_to_f_lhs, &convert_u_to_f_lhs}) {
-        for (const auto* synonym_b : synonyms) {
+      for (const auto* synonym_a : *synonyms) {
-          if (!synonymous_.IsEquivalent(*synonym_a, *synonym_b) &&
+        for (const auto* synonym_b : *synonyms) {
-              DataDescriptorsAreWellFormedAndComparable(context, *synonym_a,
+          // DataDescriptorsAreWellFormedAndComparable will be called in the
-                                                        *synonym_b)) {
+          // AddDataSynonymFactRecursive method.
+          if (!synonymous_.IsEquivalent(*synonym_a, *synonym_b)) {
             // |synonym_a| and |synonym_b| have compatible types - they are
             // synonymous.
             AddDataSynonymFactRecursive(*synonym_a, *synonym_b, context);
@@ -765,12 +772,14 @@ DataSynonymAndIdEquationFacts::RegisterDataDescriptor(
 bool DataSynonymAndIdEquationFacts::DataDescriptorsAreWellFormedAndComparable(
     opt::IRContext* context, const protobufs::DataDescriptor& dd1,
     const protobufs::DataDescriptor& dd2) {
+  assert(context->get_def_use_mgr()->GetDef(dd1.object()) &&
+         context->get_def_use_mgr()->GetDef(dd2.object()) &&
+         "Both descriptors must exist in the module");
+
   auto end_type_id_1 = fuzzerutil::WalkCompositeTypeIndices(
-      context, context->get_def_use_mgr()->GetDef(dd1.object())->type_id(),
+      context, fuzzerutil::GetTypeId(context, dd1.object()), dd1.index());
-      dd1.index());
   auto end_type_id_2 = fuzzerutil::WalkCompositeTypeIndices(
-      context, context->get_def_use_mgr()->GetDef(dd2.object())->type_id(),
+      context, fuzzerutil::GetTypeId(context, dd2.object()), dd2.index());
-      dd2.index());
   // The end types of the data descriptors must exist.
   if (end_type_id_1 == 0 || end_type_id_2 == 0) {
     return false;

source/fuzz/fuzzer_util.cpp

@@ -572,7 +572,9 @@ bool InstructionIsFunctionParameter(opt::Instruction* instruction,
 }
 
 uint32_t GetTypeId(opt::IRContext* context, uint32_t result_id) {
-  return context->get_def_use_mgr()->GetDef(result_id)->type_id();
+  const auto* inst = context->get_def_use_mgr()->GetDef(result_id);
+  assert(inst && "|result_id| is invalid");
+  return inst->type_id();
 }
 
 uint32_t GetPointeeTypeIdFromPointerType(opt::Instruction* pointer_type_inst) {

test/fuzz/fact_manager_test.cpp

@@ -16,6 +16,7 @@
 
 #include <limits>
 
+#include "source/fuzz/transformation_merge_blocks.h"
 #include "source/fuzz/uniform_buffer_element_descriptor.h"
 #include "test/fuzz/fuzz_test_util.h"
 
@@ -871,6 +872,78 @@ TEST(FactManagerTest, CorollaryConversionFacts) {
                                         MakeDataDescriptor(29, {})));
 }
 
+TEST(FactManagerTest, HandlesCorollariesWithInvalidIds) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %12 "main"
+               OpExecutionMode %12 OriginUpperLeft
+               OpSource ESSL 310
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeFloat 32
+          %8 = OpTypeInt 32 1
+          %9 = OpConstant %8 3
+         %12 = OpFunction %2 None %3
+         %13 = OpLabel
+         %14 = OpConvertSToF %6 %9
+               OpBranch %16
+         %16 = OpLabel
+         %17 = OpPhi %6 %14 %13
+         %15 = OpConvertSToF %6 %9
+         %18 = OpConvertSToF %6 %9
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  const auto consumer = nullptr;
+  const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+  ASSERT_TRUE(IsValid(env, context.get()));
+
+  FactManager fact_manager;
+
+  // Add required facts.
+  fact_manager.AddFactIdEquation(14, SpvOpConvertSToF, {9}, context.get());
+  fact_manager.AddFactDataSynonym(MakeDataDescriptor(14, {}),
+                                  MakeDataDescriptor(17, {}), context.get());
+
+  // Apply TransformationMergeBlocks which will remove %17 from the module.
+  spvtools::ValidatorOptions validator_options;
+  TransformationContext transformation_context(&fact_manager,
+                                               validator_options);
+  TransformationMergeBlocks transformation(16);
+  ASSERT_TRUE(
+      transformation.IsApplicable(context.get(), transformation_context));
+  transformation.Apply(context.get(), &transformation_context);
+  ASSERT_TRUE(IsValid(env, context.get()));
+
+  ASSERT_EQ(context->get_def_use_mgr()->GetDef(17), nullptr);
+
+  // Add another equation.
+  fact_manager.AddFactIdEquation(15, SpvOpConvertSToF, {9}, context.get());
+
+  // Check that two ids are synonymous even though one of them doesn't exist in
+  // the module (%17).
+  ASSERT_TRUE(fact_manager.IsSynonymous(MakeDataDescriptor(15, {}),
+                                        MakeDataDescriptor(17, {})));
+  ASSERT_TRUE(fact_manager.IsSynonymous(MakeDataDescriptor(15, {}),
+                                        MakeDataDescriptor(14, {})));
+
+  // Remove some instructions from the module. At this point, the equivalence
+  // class of %14 has no valid members.
+  ASSERT_TRUE(context->KillDef(14));
+  ASSERT_TRUE(context->KillDef(15));
+
+  fact_manager.AddFactIdEquation(18, SpvOpConvertSToF, {9}, context.get());
+
+  // We don't create synonyms if at least one of the equivalence classes has no
+  // valid members.
+  ASSERT_FALSE(fact_manager.IsSynonymous(MakeDataDescriptor(14, {}),
+                                         MakeDataDescriptor(18, {})));
+}
+
 TEST(FactManagerTest, LogicalNotEquationFacts) {
   std::string shader = R"(
                OpCapability Shader