From b0e22d28f5ec891d6b256703575d08fce4228bd9 Mon Sep 17 00:00:00 2001
From: Alastair Donaldson <afdx@google.com>
Date: Wed, 25 Nov 2020 12:03:05 +0000
Subject: [PATCH] spirv-fuzz: Fix facts arising from CompositeConstruct (#4034)

Fixes #4023.
---
 .../transformation_composite_construct.cpp    | 18 +++++--
 ...ransformation_composite_construct_test.cpp | 49 +++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/source/fuzz/transformation_composite_construct.cpp b/source/fuzz/transformation_composite_construct.cpp
index f6711f5a9..f0de1aeb4 100644
--- a/source/fuzz/transformation_composite_construct.cpp
+++ b/source/fuzz/transformation_composite_construct.cpp
@@ -283,15 +283,25 @@ void TransformationCompositeConstruct::AddDataSynonymFacts(
       ir_context->get_type_mgr()->GetType(message_.composite_type_id());
   uint32_t index = 0;
   for (auto component : message_.component()) {
+    auto component_type = ir_context->get_type_mgr()->GetType(
+        ir_context->get_def_use_mgr()->GetDef(component)->type_id());
+    // Whether the component is a vector being packed into a vector determines
+    // how we should keep track of the indices associated with components.
+    const bool packing_vector_into_vector =
+        composite_type->AsVector() && component_type->AsVector();
     if (!fuzzerutil::CanMakeSynonymOf(
             ir_context, *transformation_context,
             ir_context->get_def_use_mgr()->GetDef(component))) {
-      index++;
+      // We can't make a synonym of this component, so we skip on to the next
+      // component.  In the case where we're packing a vector into a vector we
+      // have to skip as many components of the resulting vectors as there are
+      // elements of the component vector.
+      index += packing_vector_into_vector
+                   ? component_type->AsVector()->element_count()
+                   : 1;
       continue;
     }
-    auto component_type = ir_context->get_type_mgr()->GetType(
-        ir_context->get_def_use_mgr()->GetDef(component)->type_id());
-    if (composite_type->AsVector() && component_type->AsVector()) {
+    if (packing_vector_into_vector) {
       // The case where the composite being constructed is a vector and the
       // component provided for construction is also a vector is special.  It
       // requires adding a synonym fact relating each element of the sub-vector
diff --git a/test/fuzz/transformation_composite_construct_test.cpp b/test/fuzz/transformation_composite_construct_test.cpp
index d2a18b000..edbfe3b60 100644
--- a/test/fuzz/transformation_composite_construct_test.cpp
+++ b/test/fuzz/transformation_composite_construct_test.cpp
@@ -1643,6 +1643,55 @@ TEST(TransformationCompositeConstructTest, OneIrrelevantComponent) {
       MakeDataDescriptor(100, {2}), MakeDataDescriptor(10, {})));
 }
 
+TEST(TransformationCompositeConstructTest, IrrelevantVec2ThenFloat) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeFloat 32
+          %7 = OpTypeVector %6 2
+          %8 = OpTypeVector %6 3
+          %9 = OpConstant %6 0
+         %11 = OpConstant %6 1
+         %12 = OpConstant %6 2
+         %10 = OpConstantComposite %7 %11 %12
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  const auto consumer = nullptr;
+  const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+  spvtools::ValidatorOptions validator_options;
+  ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options,
+                                               kConsoleMessageConsumer));
+  TransformationContext transformation_context(
+      MakeUnique<FactManager>(context.get()), validator_options);
+
+  transformation_context.GetFactManager()->AddFactIdIsIrrelevant(10);
+
+  TransformationCompositeConstruct transformation(
+      8, {10, 9}, MakeInstructionDescriptor(5, SpvOpReturn, 0), 100);
+  ASSERT_TRUE(
+      transformation.IsApplicable(context.get(), transformation_context));
+  ApplyAndCheckFreshIds(transformation, context.get(), &transformation_context);
+  ASSERT_FALSE(transformation_context.GetFactManager()->IsSynonymous(
+      MakeDataDescriptor(100, {0}), MakeDataDescriptor(10, {0})));
+  ASSERT_FALSE(transformation_context.GetFactManager()->IsSynonymous(
+      MakeDataDescriptor(100, {1}), MakeDataDescriptor(10, {1})));
+  ASSERT_TRUE(transformation_context.GetFactManager()->IsSynonymous(
+      MakeDataDescriptor(100, {2}), MakeDataDescriptor(9, {})));
+  ASSERT_FALSE(transformation_context.GetFactManager()->IsSynonymous(
+      MakeDataDescriptor(100, {1}), MakeDataDescriptor(9, {})));
+}
+
 }  // namespace
 }  // namespace fuzz
 }  // namespace spvtools