spirv-reader: reject undef and null pointers

This is defensive.  Without variable pointers capabilities, this is
definitely invalid, but not yet checked by the SPIRV-Tools validator.

Bug: tint:807
Change-Id: If9b0b19573b1ca14a1c55aa20c9d42784ec12568
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56700
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/reader/spirv/function.cc

@@ -4560,6 +4560,8 @@ bool FunctionEmitter::RegisterLocallyDefinedValues() {
           }
           switch (inst.opcode()) {
             case SpvOpUndef:
+              return Fail()
+                     << "undef pointer is not valid: " << inst.PrettyPrint();
             case SpvOpVariable:
               // Keep the default decision based on the result type.
               break;

src/reader/spirv/function_memory_test.cc

@@ -1280,12 +1280,11 @@ Return{}
 }
 
 TEST_F(SpvParserMemoryTest, DISABLED_RemapStorageBuffer_ThroughFunctionCall) {
-  // TODO(dneto): Blocked on OpFunctionCall support.
+  // WGSL does not support pointer-to-storage-buffer as function parameter
-  // We might need this for passing pointers into atomic builtins.
 }
 TEST_F(SpvParserMemoryTest,
        DISABLED_RemapStorageBuffer_ThroughFunctionParameter) {
-  // TODO(dneto): Blocked on OpFunctionCall support.
+  // WGSL does not support pointer-to-storage-buffer as function parameter
 }
 
 std::string RuntimeArrayPreamble() {
@@ -1359,6 +1358,79 @@ TEST_F(SpvParserMemoryTest, ArrayLength) {
 )")) << body_str;
 }
 
+std::string InvalidPointerPreamble() {
+  return R"(
+OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+
+%uint = OpTypeInt 32 0
+%ptr_ty = OpTypePointer Function %uint
+
+  %void = OpTypeVoid
+%voidfn = OpTypeFunction %void
+)";
+}
+
+TEST_F(SpvParserMemoryTest, InvalidPointer_Undef_ModuleScope_IsError) {
+  const std::string assembly = InvalidPointerPreamble() + R"(
+ %ptr = OpUndef %ptr_ty
+
+  %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+     %1 = OpCopyObject %ptr_ty %ptr
+     %2 = OpAccessChain %ptr_ty %ptr
+     %3 = OpInBoundsAccessChain %ptr_ty %ptr
+; now show the invalid pointer propagates
+     %10 = OpCopyObject %ptr_ty %1
+     %20 = OpAccessChain %ptr_ty %2
+     %30 = OpInBoundsAccessChain %ptr_ty %3
+          OpReturn
+          OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+  EXPECT_FALSE(p->BuildAndParseInternalModule()) << assembly;
+  EXPECT_EQ(p->error(), "undef pointer is not valid: %9 = OpUndef %6");
+}
+
+TEST_F(SpvParserMemoryTest, InvalidPointer_Undef_FunctionScope_IsError) {
+  const std::string assembly = InvalidPointerPreamble() + R"(
+
+  %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+   %ptr = OpUndef %ptr_ty
+          OpReturn
+          OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+  EXPECT_FALSE(p->BuildAndParseInternalModule()) << assembly;
+  EXPECT_EQ(p->error(), "undef pointer is not valid: %7 = OpUndef %3");
+}
+
+TEST_F(SpvParserMemoryTest, InvalidPointer_ConstantNull_IsError) {
+  // OpConstantNull on logical pointer requires variable-pointers, which
+  // is not (yet) supported by WGSL features.
+  const std::string assembly = InvalidPointerPreamble() + R"(
+ %ptr = OpConstantNull %ptr_ty
+
+  %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+     %1 = OpCopyObject %ptr_ty %ptr
+     %2 = OpAccessChain %ptr_ty %ptr
+     %3 = OpInBoundsAccessChain %ptr_ty %ptr
+; now show the invalid pointer propagates
+     %10 = OpCopyObject %ptr_ty %1
+     %20 = OpAccessChain %ptr_ty %2
+     %30 = OpInBoundsAccessChain %ptr_ty %3
+          OpReturn
+          OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+  EXPECT_FALSE(p->BuildAndParseInternalModule());
+  EXPECT_EQ(p->error(), "null pointer is not valid: %9 = OpConstantNull %6");
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace reader

src/reader/spirv/parser_impl.cc

@@ -610,6 +610,9 @@ bool ParserImpl::ParseInternalModuleExceptFunctions() {
   if (!RegisterTypes()) {
     return false;
   }
+  if (!RejectInvalidPointerRoots()) {
+    return false;
+  }
   if (!EmitScalarSpecConstants()) {
     return false;
   }
@@ -1288,6 +1291,33 @@ bool ParserImpl::RegisterTypes() {
   return success_;
 }
 
+bool ParserImpl::RejectInvalidPointerRoots() {
+  if (!success_) {
+    return false;
+  }
+  for (auto& inst : module_->types_values()) {
+    if (const auto* result_type = type_mgr_->GetType(inst.type_id())) {
+      if (result_type->AsPointer()) {
+        switch (inst.opcode()) {
+          case SpvOpVariable:
+            // This is the only valid case.
+            break;
+          case SpvOpUndef:
+            return Fail() << "undef pointer is not valid: "
+                          << inst.PrettyPrint();
+          case SpvOpConstantNull:
+            return Fail() << "null pointer is not valid: "
+                          << inst.PrettyPrint();
+          default:
+            return Fail() << "module-scope pointer is not valid: "
+                          << inst.PrettyPrint();
+        }
+      }
+    }
+  }
+  return success();
+}
+
 bool ParserImpl::EmitScalarSpecConstants() {
   if (!success_) {
     return false;

src/reader/spirv/parser_impl.h

@@ -349,6 +349,11 @@ class ParserImpl : Reader {
   /// @returns true if parser is still successful.
   bool RegisterTypes();
 
+  /// Fail if there are any module-scope pointer values other than those
+  /// declared by OpVariable.
+  /// @returns true if parser is still successful.
+  bool RejectInvalidPointerRoots();
+
   /// Register sampler and texture usage for memory object declarations.
   /// This must be called after we've registered line numbers for all
   /// instructions. This is a no-op if the parser has already failed.

src/reader/spirv/parser_impl_module_var_test.cc

@@ -344,7 +344,7 @@ TEST_F(SpvModuleScopeVarParserTest,
   const std::string assembly = PerVertexPreamble() + R"(
   %main = OpFunction %void None %voidfn
   %entry = OpLabel
-  %1000 = OpUndef %11
+  %1000 = OpCopyObject %11 %1
   OpReturn
   OpFunctionEnd
   )";
@@ -352,7 +352,7 @@ TEST_F(SpvModuleScopeVarParserTest,
   EXPECT_FALSE(p->BuildAndParseInternalModule());
   EXPECT_THAT(p->error(),
               Eq("operations producing a pointer to a per-vertex structure are "
-                 "not supported: %1000 = OpUndef %11"))
+                 "not supported: %1000 = OpCopyObject %11 %1"))
       << p->error();
 }