From a9f2a145e6546e4c4f35a74bd636b0853fcb819c Mon Sep 17 00:00:00 2001
From: Alastair Donaldson <afdx@google.com>
Date: Wed, 13 May 2020 22:04:52 +0100
Subject: [PATCH] spirv-fuzz: Fix to fact manager (#3339)

Reworks the way uniforms with known constants are queried to avoid
unintended side effects to the constant manager.

Fixes #3338.
---
 source/fuzz/fact_manager.cpp    | 23 ++++++++++++--
 test/fuzz/fact_manager_test.cpp | 56 +++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/source/fuzz/fact_manager.cpp b/source/fuzz/fact_manager.cpp
index bc6f3b1e1..0b41eeb59 100644
--- a/source/fuzz/fact_manager.cpp
+++ b/source/fuzz/fact_manager.cpp
@@ -159,9 +159,26 @@ uint32_t FactManager::ConstantUniformFacts::GetConstantId(
     uint32_t type_id) const {
   auto type = context->get_type_mgr()->GetType(type_id);
   assert(type != nullptr && "Unknown type id.");
-  auto constant = context->get_constant_mgr()->GetConstant(
-      type, GetConstantWords(constant_uniform_fact));
-  return context->get_constant_mgr()->FindDeclaredConstant(constant, type_id);
+  const opt::analysis::Constant* known_constant;
+  if (type->AsInteger()) {
+    opt::analysis::IntConstant candidate_constant(
+        type->AsInteger(), GetConstantWords(constant_uniform_fact));
+    known_constant =
+        context->get_constant_mgr()->FindConstant(&candidate_constant);
+  } else {
+    assert(
+        type->AsFloat() &&
+        "Uniform constant facts are only supported for int and float types.");
+    opt::analysis::FloatConstant candidate_constant(
+        type->AsFloat(), GetConstantWords(constant_uniform_fact));
+    known_constant =
+        context->get_constant_mgr()->FindConstant(&candidate_constant);
+  }
+  if (!known_constant) {
+    return 0;
+  }
+  return context->get_constant_mgr()->FindDeclaredConstant(known_constant,
+                                                           type_id);
 }
 
 std::vector<uint32_t> FactManager::ConstantUniformFacts::GetConstantWords(
diff --git a/test/fuzz/fact_manager_test.cpp b/test/fuzz/fact_manager_test.cpp
index 04f0372d5..8b1e0c4b3 100644
--- a/test/fuzz/fact_manager_test.cpp
+++ b/test/fuzz/fact_manager_test.cpp
@@ -1054,6 +1054,62 @@ TEST(FactManagerTest, EquationAndEquivalenceFacts) {
                                         MakeDataDescriptor(16, {})));
 }
 
+TEST(FactManagerTest, CheckingFactsDoesNotAddConstants) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+               OpMemberDecorate %9 0 Offset 0
+               OpDecorate %9 Block
+               OpDecorate %11 DescriptorSet 0
+               OpDecorate %11 Binding 0
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeInt 32 1
+          %7 = OpTypePointer Function %6
+          %9 = OpTypeStruct %6
+         %10 = OpTypePointer Uniform %9
+         %11 = OpVariable %10 Uniform
+         %12 = OpConstant %6 0
+         %13 = OpTypePointer Uniform %6
+          %4 = OpFunction %2 None %3
+          %5 = OpLabel
+          %8 = OpVariable %7 Function
+         %14 = OpAccessChain %13 %11 %12
+         %15 = OpLoad %6 %14
+               OpStore %8 %15
+               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;
+
+  // 8[0] == int(1)
+  ASSERT_TRUE(AddFactHelper(&fact_manager, context.get(), {1},
+                            MakeUniformBufferElementDescriptor(0, 0, {0})));
+
+  // Although 8[0] has the value 1, we do not have the constant 1 in the module.
+  // We thus should not find any constants available from uniforms for int type.
+  // Furthermore, the act of looking for appropriate constants should not change
+  // which constants are known to the constant manager.
+  auto int_type = context->get_type_mgr()->GetType(6)->AsInteger();
+  opt::analysis::IntConstant constant_one(int_type, {1});
+  ASSERT_FALSE(context->get_constant_mgr()->FindConstant(&constant_one));
+  auto available_constants =
+      fact_manager.GetConstantsAvailableFromUniformsForType(context.get(), 6);
+  ASSERT_EQ(0, available_constants.size());
+  ASSERT_TRUE(IsEqual(env, shader, context.get()));
+  ASSERT_FALSE(context->get_constant_mgr()->FindConstant(&constant_one));
+}
+
 }  // namespace
 }  // namespace fuzz
 }  // namespace spvtools