From 62c8f07015eacb7cbcf6ffed042abfea54e2a1aa Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Wed, 18 Nov 2020 02:14:09 +0000
Subject: [PATCH] spirv-reader: GetMemoryObjectDeclarationForHandl can return
 null

It's valid to look for but not find an underlying memory object
declaration.

Bug: tint:109
Change-Id: I7296d79550a50050d2438996dc3e0c8d09a6babd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33140
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
---
 src/reader/spirv/parser_impl.cc             |  8 ++++++--
 src/reader/spirv/parser_impl.h              | 10 +++++-----
 src/reader/spirv/parser_impl_handle_test.cc | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index bcc95a1e12..1bfe0e4b24 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -1579,8 +1579,12 @@ ParserImpl::GetMemoryObjectDeclarationForHandle(uint32_t id,
         id = inst->GetSingleWordInOperand(0);
         break;
       default:
-        // This is not valid.
-        return local_fail();
+        // Can't trace further.
+        // Remember it as the answer for the whole path.
+        for (auto iter : visited) {
+          memo_table[iter] = nullptr;
+        }
+        return nullptr;
     }
   }
 }
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h
index 5f0d350aca..ee6e1b2f21 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/src/reader/spirv/parser_impl.h
@@ -379,14 +379,14 @@ class ParserImpl : Reader {
   /// return the SPIR-V instruction that represents the memory object
   /// declaration for the object.  If we encounter an OpSampledImage along the
   /// way, follow the image operand when follow_image is true; otherwise follow
-  /// the sampler operand. Returns null and emits an error if it can't trace
-  /// back to a memory object declaration.
-  /// This method can be used any time after BuildInternalModule has been
-  /// invoked.
+  /// the sampler operand. Returns nullptr if we can't trace back to a memory
+  /// object declaration.  Emits an error and returns nullptr when the scan
+  /// fails due to a malformed module. This method can be used any time after
+  /// BuildInternalModule has been invoked.
   /// @param id the SPIR-V ID of the sampler, image, or sampled image
   /// @param follow_image indicates whether to follow the image operand of
   /// OpSampledImage
-  /// @returns the memory object declaration for the handle, or nullptr on error
+  /// @returns the memory object declaration for the handle, or nullptr
   const spvtools::opt::Instruction* GetMemoryObjectDeclarationForHandle(
       uint32_t id,
       bool follow_image);
diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc
index 70d6afb982..95911f0794 100644
--- a/src/reader/spirv/parser_impl_handle_test.cc
+++ b/src/reader/spirv/parser_impl_handle_test.cc
@@ -179,6 +179,22 @@ std::string CommonTypes() {
   )";
 }
 
+TEST_F(SpvParserTest,
+       GetMemoryObjectDeclarationForHandle_WellFormedButNotAHandle) {
+  const auto assembly = Preamble() + CommonTypes() + R"(
+     %10 = OpConstantNull %ptr_sampler
+     %20 = OpConstantNull %ptr_f_texture_1d
+  )";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildInternalModule());
+  const auto* sampler = p->GetMemoryObjectDeclarationForHandle(10, false);
+  const auto* image = p->GetMemoryObjectDeclarationForHandle(20, true);
+
+  EXPECT_EQ(sampler, nullptr);
+  EXPECT_EQ(image, nullptr);
+  EXPECT_TRUE(p->error().empty());
+}
+
 TEST_F(SpvParserTest, GetMemoryObjectDeclarationForHandle_Variable_Direct) {
   const auto assembly = Preamble() + CommonTypes() + R"(
      %10 = OpVariable %ptr_sampler UniformConstant