spirv-fuzz: Handle isomorphic types property in composite construction (#3262)

The fuzzer pass that constructs composites had an issue where it would
regard isomorphic but distinct structs (similarly arrays) as being
interchangeable when constructing composites.  This change fixes the
problem by relying less on the type manager.
diff --git a/source/fuzz/fuzzer_pass_construct_composites.cpp b/source/fuzz/fuzzer_pass_construct_composites.cpp
index b1b0823..e78f8ec 100644
--- a/source/fuzz/fuzzer_pass_construct_composites.cpp
+++ b/source/fuzz/fuzzer_pass_construct_composites.cpp
@@ -88,7 +88,7 @@
         // for constructing a composite of that type.  Otherwise these variables
         // will remain 0 and null respectively.
         uint32_t chosen_composite_type = 0;
-        std::unique_ptr<std::vector<uint32_t>> constructor_arguments = nullptr;
+        std::vector<uint32_t> constructor_arguments;
 
         // Initially, all composite type ids are available for us to try.  Keep
         // trying until we run out of options.
@@ -96,35 +96,38 @@
         while (!composites_to_try_constructing.empty()) {
           // Remove a composite type from the composite types left for us to
           // try.
-          auto index =
-              GetFuzzerContext()->RandomIndex(composites_to_try_constructing);
           auto next_composite_to_try_constructing =
-              composites_to_try_constructing[index];
-          composites_to_try_constructing.erase(
-              composites_to_try_constructing.begin() + index);
+              GetFuzzerContext()->RemoveAtRandomIndex(
+                  &composites_to_try_constructing);
 
           // Now try to construct a composite of this type, using an appropriate
           // helper method depending on the kind of composite type.
-          auto composite_type = GetIRContext()->get_type_mgr()->GetType(
+          auto composite_type_inst = GetIRContext()->get_def_use_mgr()->GetDef(
               next_composite_to_try_constructing);
-          if (auto array_type = composite_type->AsArray()) {
-            constructor_arguments = TryConstructingArrayComposite(
-                *array_type, type_id_to_available_instructions);
-          } else if (auto matrix_type = composite_type->AsMatrix()) {
-            constructor_arguments = TryConstructingMatrixComposite(
-                *matrix_type, type_id_to_available_instructions);
-          } else if (auto struct_type = composite_type->AsStruct()) {
-            constructor_arguments = TryConstructingStructComposite(
-                *struct_type, type_id_to_available_instructions);
-          } else {
-            auto vector_type = composite_type->AsVector();
-            assert(vector_type &&
-                   "The space of possible composite types should be covered by "
-                   "the above cases.");
-            constructor_arguments = TryConstructingVectorComposite(
-                *vector_type, type_id_to_available_instructions);
+          switch (composite_type_inst->opcode()) {
+            case SpvOpTypeArray:
+              constructor_arguments = FindComponentsToConstructArray(
+                  *composite_type_inst, type_id_to_available_instructions);
+              break;
+            case SpvOpTypeMatrix:
+              constructor_arguments = FindComponentsToConstructMatrix(
+                  *composite_type_inst, type_id_to_available_instructions);
+              break;
+            case SpvOpTypeStruct:
+              constructor_arguments = FindComponentsToConstructStruct(
+                  *composite_type_inst, type_id_to_available_instructions);
+              break;
+            case SpvOpTypeVector:
+              constructor_arguments = FindComponentsToConstructVector(
+                  *composite_type_inst, type_id_to_available_instructions);
+              break;
+            default:
+              assert(false &&
+                     "The space of possible composite types should be covered "
+                     "by the above cases.");
+              break;
           }
-          if (constructor_arguments != nullptr) {
+          if (!constructor_arguments.empty()) {
             // We succeeded!  Note the composite type we finally settled on, and
             // exit from the loop.
             chosen_composite_type = next_composite_to_try_constructing;
@@ -135,21 +138,15 @@
         if (!chosen_composite_type) {
           // We did not manage to make a composite; return 0 to indicate that no
           // instructions were added.
-          assert(constructor_arguments == nullptr);
+          assert(constructor_arguments.empty());
           return;
         }
-        assert(constructor_arguments != nullptr);
+        assert(!constructor_arguments.empty());
 
         // Make and apply a transformation.
-        TransformationCompositeConstruct transformation(
-            chosen_composite_type, *constructor_arguments,
-            instruction_descriptor, GetFuzzerContext()->GetFreshId());
-        assert(transformation.IsApplicable(GetIRContext(),
-                                           *GetTransformationContext()) &&
-               "This transformation should be applicable by construction.");
-        transformation.Apply(GetIRContext(), GetTransformationContext());
-        *GetTransformations()->add_transformation() =
-            transformation.ToMessage();
+        ApplyTransformation(TransformationCompositeConstruct(
+            chosen_composite_type, constructor_arguments,
+            instruction_descriptor, GetFuzzerContext()->GetFreshId()));
       });
 }
 
@@ -162,20 +159,15 @@
   type_id_to_available_instructions->at(inst->type_id()).push_back(inst);
 }
 
-std::unique_ptr<std::vector<uint32_t>>
-FuzzerPassConstructComposites::TryConstructingArrayComposite(
-    const opt::analysis::Array& array_type,
+std::vector<uint32_t>
+FuzzerPassConstructComposites::FindComponentsToConstructArray(
+    const opt::Instruction& array_type_instruction,
     const TypeIdToInstructions& type_id_to_available_instructions) {
-  // At present we assume arrays have a constant size.
-  assert(array_type.length_info().words.size() == 2);
-  assert(array_type.length_info().words[0] ==
-         opt::analysis::Array::LengthInfo::kConstant);
-
-  auto result = MakeUnique<std::vector<uint32_t>>();
+  assert(array_type_instruction.opcode() == SpvOpTypeArray &&
+         "Precondition: instruction must be an array type.");
 
   // Get the element type for the array.
-  auto element_type_id =
-      GetIRContext()->get_type_mgr()->GetId(array_type.element_type());
+  auto element_type_id = array_type_instruction.GetSingleWordInOperand(0);
 
   // Get all instructions at our disposal that compute something of this element
   // type.
@@ -186,26 +178,34 @@
     // If there are not any instructions available that compute the element type
     // of the array then we are not in a position to construct a composite with
     // this array type.
-    return nullptr;
+    return {};
   }
-  for (uint32_t index = 0; index < array_type.length_info().words[1]; index++) {
-    result->push_back(available_instructions
-                          ->second[GetFuzzerContext()->RandomIndex(
-                              available_instructions->second)]
-                          ->result_id());
+
+  uint32_t array_length =
+      GetIRContext()
+          ->get_def_use_mgr()
+          ->GetDef(array_type_instruction.GetSingleWordInOperand(1))
+          ->GetSingleWordInOperand(0);
+
+  std::vector<uint32_t> result;
+  for (uint32_t index = 0; index < array_length; index++) {
+    result.push_back(available_instructions
+                         ->second[GetFuzzerContext()->RandomIndex(
+                             available_instructions->second)]
+                         ->result_id());
   }
   return result;
 }
 
-std::unique_ptr<std::vector<uint32_t>>
-FuzzerPassConstructComposites::TryConstructingMatrixComposite(
-    const opt::analysis::Matrix& matrix_type,
+std::vector<uint32_t>
+FuzzerPassConstructComposites::FindComponentsToConstructMatrix(
+    const opt::Instruction& matrix_type_instruction,
     const TypeIdToInstructions& type_id_to_available_instructions) {
-  auto result = MakeUnique<std::vector<uint32_t>>();
+  assert(matrix_type_instruction.opcode() == SpvOpTypeMatrix &&
+         "Precondition: instruction must be a matrix type.");
 
   // Get the element type for the matrix.
-  auto element_type_id =
-      GetIRContext()->get_type_mgr()->GetId(matrix_type.element_type());
+  auto element_type_id = matrix_type_instruction.GetSingleWordInOperand(0);
 
   // Get all instructions at our disposal that compute something of this element
   // type.
@@ -216,25 +216,32 @@
     // If there are not any instructions available that compute the element type
     // of the matrix then we are not in a position to construct a composite with
     // this matrix type.
-    return nullptr;
+    return {};
   }
-  for (uint32_t index = 0; index < matrix_type.element_count(); index++) {
-    result->push_back(available_instructions
-                          ->second[GetFuzzerContext()->RandomIndex(
-                              available_instructions->second)]
-                          ->result_id());
+  std::vector<uint32_t> result;
+  for (uint32_t index = 0;
+       index < matrix_type_instruction.GetSingleWordInOperand(1); index++) {
+    result.push_back(available_instructions
+                         ->second[GetFuzzerContext()->RandomIndex(
+                             available_instructions->second)]
+                         ->result_id());
   }
   return result;
 }
 
-std::unique_ptr<std::vector<uint32_t>>
-FuzzerPassConstructComposites::TryConstructingStructComposite(
-    const opt::analysis::Struct& struct_type,
+std::vector<uint32_t>
+FuzzerPassConstructComposites::FindComponentsToConstructStruct(
+    const opt::Instruction& struct_type_instruction,
     const TypeIdToInstructions& type_id_to_available_instructions) {
-  auto result = MakeUnique<std::vector<uint32_t>>();
+  assert(struct_type_instruction.opcode() == SpvOpTypeStruct &&
+         "Precondition: instruction must be a struct type.");
+  std::vector<uint32_t> result;
   // Consider the type of each field of the struct.
-  for (auto element_type : struct_type.element_types()) {
-    auto element_type_id = GetIRContext()->get_type_mgr()->GetId(element_type);
+  for (uint32_t in_operand_index = 0;
+       in_operand_index < struct_type_instruction.NumInOperands();
+       in_operand_index++) {
+    auto element_type_id =
+        struct_type_instruction.GetSingleWordInOperand(in_operand_index);
     // Find the instructions at our disposal that compute something of the field
     // type.
     auto available_instructions =
@@ -242,24 +249,28 @@
     if (available_instructions == type_id_to_available_instructions.cend()) {
       // If there are no such instructions, we cannot construct a composite of
       // this struct type.
-      return nullptr;
+      return {};
     }
-    result->push_back(available_instructions
-                          ->second[GetFuzzerContext()->RandomIndex(
-                              available_instructions->second)]
-                          ->result_id());
+    result.push_back(available_instructions
+                         ->second[GetFuzzerContext()->RandomIndex(
+                             available_instructions->second)]
+                         ->result_id());
   }
   return result;
 }
 
-std::unique_ptr<std::vector<uint32_t>>
-FuzzerPassConstructComposites::TryConstructingVectorComposite(
-    const opt::analysis::Vector& vector_type,
+std::vector<uint32_t>
+FuzzerPassConstructComposites::FindComponentsToConstructVector(
+    const opt::Instruction& vector_type_instruction,
     const TypeIdToInstructions& type_id_to_available_instructions) {
+  assert(vector_type_instruction.opcode() == SpvOpTypeVector &&
+         "Precondition: instruction must be a vector type.");
+
   // Get details of the type underlying the vector, and the width of the vector,
   // for convenience.
-  auto element_type = vector_type.element_type();
-  auto element_count = vector_type.element_count();
+  auto element_type_id = vector_type_instruction.GetSingleWordInOperand(0);
+  auto element_type = GetIRContext()->get_type_mgr()->GetType(element_type_id);
+  auto element_count = vector_type_instruction.GetSingleWordInOperand(1);
 
   // Collect a mapping, from type id to width, for scalar/vector types that are
   // smaller in width than |vector_type|, but that have the same underlying
@@ -270,14 +281,12 @@
   std::map<uint32_t, uint32_t> smaller_vector_type_id_to_width;
   // Add the underlying type.  This id must exist, in order for |vector_type| to
   // exist.
-  auto scalar_type_id = GetIRContext()->get_type_mgr()->GetId(element_type);
-  smaller_vector_type_id_to_width[scalar_type_id] = 1;
+  smaller_vector_type_id_to_width[element_type_id] = 1;
 
   // Now add every vector type with width at least 2, and less than the width of
   // |vector_type|.
   for (uint32_t width = 2; width < element_count; width++) {
-    opt::analysis::Vector smaller_vector_type(vector_type.element_type(),
-                                              width);
+    opt::analysis::Vector smaller_vector_type(element_type, width);
     auto smaller_vector_type_id =
         GetIRContext()->get_type_mgr()->GetId(&smaller_vector_type);
     // We might find that there is no declared type of this smaller width.
@@ -304,12 +313,11 @@
   // order at this stage.
   std::vector<opt::Instruction*> instructions_to_use;
 
-  while (vector_slots_used < vector_type.element_count()) {
+  while (vector_slots_used < element_count) {
     std::vector<opt::Instruction*> instructions_to_choose_from;
     for (auto& entry : smaller_vector_type_id_to_width) {
       if (entry.second >
-          std::min(vector_type.element_count() - 1,
-                   vector_type.element_count() - vector_slots_used)) {
+          std::min(element_count - 1, element_count - vector_slots_used)) {
         continue;
       }
       auto available_instructions =
@@ -328,7 +336,7 @@
       // another manner, so we could opt to retry a few times here, but it is
       // simpler to just give up on the basis that this will not happen
       // frequently.
-      return nullptr;
+      return {};
     }
     auto instruction_to_use =
         instructions_to_choose_from[GetFuzzerContext()->RandomIndex(
@@ -347,16 +355,16 @@
       vector_slots_used += 1;
     }
   }
-  assert(vector_slots_used == vector_type.element_count());
+  assert(vector_slots_used == element_count);
 
-  auto result = MakeUnique<std::vector<uint32_t>>();
+  std::vector<uint32_t> result;
   std::vector<uint32_t> operands;
   while (!instructions_to_use.empty()) {
     auto index = GetFuzzerContext()->RandomIndex(instructions_to_use);
-    result->push_back(instructions_to_use[index]->result_id());
+    result.push_back(instructions_to_use[index]->result_id());
     instructions_to_use.erase(instructions_to_use.begin() + index);
   }
-  assert(result->size() > 1);
+  assert(result.size() > 1);
   return result;
 }
 
diff --git a/source/fuzz/fuzzer_pass_construct_composites.h b/source/fuzz/fuzzer_pass_construct_composites.h
index d293514..9853fad 100644
--- a/source/fuzz/fuzzer_pass_construct_composites.h
+++ b/source/fuzz/fuzzer_pass_construct_composites.h
@@ -49,27 +49,28 @@
       opt::Instruction* inst,
       TypeIdToInstructions* type_id_to_available_instructions);
 
+  // Requires that |array_type_instruction| has opcode OpTypeArray.
   // Attempts to find suitable instruction result ids from the values of
   // |type_id_to_available_instructions| that would allow a composite of type
-  // |array_type| to be constructed.  Returns said ids if they can be found.
-  // Returns |nullptr| otherwise.
-  std::unique_ptr<std::vector<uint32_t>> TryConstructingArrayComposite(
-      const opt::analysis::Array& array_type,
+  // |array_type_instruction| to be constructed.  Returns said ids if they can
+  // be found and an empty vector otherwise.
+  std::vector<uint32_t> FindComponentsToConstructArray(
+      const opt::Instruction& array_type_instruction,
       const TypeIdToInstructions& type_id_to_available_instructions);
 
-  // Similar to TryConstructingArrayComposite, but for matrices.
-  std::unique_ptr<std::vector<uint32_t>> TryConstructingMatrixComposite(
-      const opt::analysis::Matrix& matrix_type,
+  // Similar to FindComponentsToConstructArray, but for matrices.
+  std::vector<uint32_t> FindComponentsToConstructMatrix(
+      const opt::Instruction& matrix_type_instruction,
       const TypeIdToInstructions& type_id_to_available_instructions);
 
-  // Similar to TryConstructingArrayComposite, but for structs.
-  std::unique_ptr<std::vector<uint32_t>> TryConstructingStructComposite(
-      const opt::analysis::Struct& struct_type,
+  // Similar to FindComponentsToConstructArray, but for structs.
+  std::vector<uint32_t> FindComponentsToConstructStruct(
+      const opt::Instruction& struct_type_instruction,
       const TypeIdToInstructions& type_id_to_available_instructions);
 
-  // Similar to TryConstructingArrayComposite, but for vectors.
-  std::unique_ptr<std::vector<uint32_t>> TryConstructingVectorComposite(
-      const opt::analysis::Vector& vector_type,
+  // Similar to FindComponentsToConstructArray, but for vectors.
+  std::vector<uint32_t> FindComponentsToConstructVector(
+      const opt::Instruction& vector_type_instruction,
       const TypeIdToInstructions& type_id_to_available_instructions);
 };
 
diff --git a/test/fuzz/CMakeLists.txt b/test/fuzz/CMakeLists.txt
index 99a78fd..a8b09b9 100644
--- a/test/fuzz/CMakeLists.txt
+++ b/test/fuzz/CMakeLists.txt
@@ -22,6 +22,7 @@
           fact_manager_test.cpp
           fuzz_test_util.cpp
           fuzzer_pass_add_useful_constructs_test.cpp
+          fuzzer_pass_construct_composites_test.cpp
           fuzzer_pass_donate_modules_test.cpp
           instruction_descriptor_test.cpp
           transformation_access_chain_test.cpp
diff --git a/test/fuzz/fuzzer_pass_construct_composites_test.cpp b/test/fuzz/fuzzer_pass_construct_composites_test.cpp
new file mode 100644
index 0000000..cc21f74
--- /dev/null
+++ b/test/fuzz/fuzzer_pass_construct_composites_test.cpp
@@ -0,0 +1,187 @@
+// Copyright (c) 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "source/fuzz/fuzzer_pass_construct_composites.h"
+#include "source/fuzz/pseudo_random_generator.h"
+#include "test/fuzz/fuzz_test_util.h"
+
+namespace spvtools {
+namespace fuzz {
+namespace {
+
+TEST(FuzzerPassConstructCompositesTest, IsomorphicStructs) {
+  // This test declares various isomorphic structs, and a struct that is made up
+  // of these isomorphic structs.  The pass to construct composites is then
+  // applied several times to check that no issues arise related to using a
+  // value of one struct type when a value of an isomorphic struct type is
+  // required.
+
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 310
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeFloat 32
+          %7 = OpConstant %6 0
+          %8 = OpTypeStruct %6 %6 %6
+          %9 = OpTypeStruct %6 %6 %6
+         %10 = OpTypeStruct %6 %6 %6
+         %11 = OpTypeStruct %6 %6 %6
+         %12 = OpTypeStruct %6 %6 %6
+         %13 = OpTypeStruct %8 %9 %10 %11 %12
+         %14 = OpConstantComposite %8 %7 %7 %7
+         %15 = OpConstantComposite %9 %7 %7 %7
+         %16 = OpConstantComposite %10 %7 %7 %7
+         %17 = OpConstantComposite %11 %7 %7 %7
+         %18 = OpConstantComposite %12 %7 %7 %7
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  const auto consumer = nullptr;
+
+  auto prng = MakeUnique<PseudoRandomGenerator>(0);
+
+  for (uint32_t i = 0; i < 10; i++) {
+    const auto context =
+        BuildModule(env, consumer, shader, kFuzzAssembleOption);
+    ASSERT_TRUE(IsValid(env, context.get()));
+
+    FactManager fact_manager;
+    spvtools::ValidatorOptions validator_options;
+    TransformationContext transformation_context(&fact_manager,
+                                                 validator_options);
+
+    FuzzerContext fuzzer_context(prng.get(), 100);
+    protobufs::TransformationSequence transformation_sequence;
+
+    FuzzerPassConstructComposites fuzzer_pass(
+        context.get(), &transformation_context, &fuzzer_context,
+        &transformation_sequence);
+
+    fuzzer_pass.Apply();
+
+    // We just check that the result is valid.
+    ASSERT_TRUE(IsValid(env, context.get()));
+  }
+}
+
+TEST(FuzzerPassConstructCompositesTest, IsomorphicArrays) {
+  // This test declares various isomorphic arrays, and a struct that is made up
+  // of these isomorphic arrays.  The pass to construct composites is then
+  // applied several times to check that no issues arise related to using a
+  // value of one array type when a value of an isomorphic array type is
+  // required.
+
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 310
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeFloat 32
+         %50 = OpTypeInt 32 0
+         %51 = OpConstant %50 3
+          %7 = OpConstant %6 0
+          %8 = OpTypeArray %6 %51
+          %9 = OpTypeArray %6 %51
+         %10 = OpTypeArray %6 %51
+         %11 = OpTypeArray %6 %51
+         %12 = OpTypeArray %6 %51
+         %13 = OpTypeStruct %8 %9 %10 %11 %12
+         %14 = OpConstantComposite %8 %7 %7 %7
+         %15 = OpConstantComposite %9 %7 %7 %7
+         %16 = OpConstantComposite %10 %7 %7 %7
+         %17 = OpConstantComposite %11 %7 %7 %7
+         %18 = OpConstantComposite %12 %7 %7 %7
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpNop
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  const auto consumer = nullptr;
+
+  auto prng = MakeUnique<PseudoRandomGenerator>(0);
+
+  for (uint32_t i = 0; i < 10; i++) {
+    const auto context =
+        BuildModule(env, consumer, shader, kFuzzAssembleOption);
+    ASSERT_TRUE(IsValid(env, context.get()));
+
+    FactManager fact_manager;
+    spvtools::ValidatorOptions validator_options;
+    TransformationContext transformation_context(&fact_manager,
+                                                 validator_options);
+
+    FuzzerContext fuzzer_context(prng.get(), 100);
+    protobufs::TransformationSequence transformation_sequence;
+
+    FuzzerPassConstructComposites fuzzer_pass(
+        context.get(), &transformation_context, &fuzzer_context,
+        &transformation_sequence);
+
+    fuzzer_pass.Apply();
+
+    // We just check that the result is valid.
+    ASSERT_TRUE(IsValid(env, context.get()));
+  }
+}
+
+}  // namespace
+}  // namespace fuzz
+}  // namespace spvtools