commit 6e2461096a248805f5e6e8f2c2f19584c3e6cd47
author David Neto <dneto@google.com> Wed May 05 16:03:02 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed May 05 16:03:02 2021 +0000
tree 71be3c822ed0a6b55842240a4adaa2256b670396
parent 01c3ce7c03f139239c2275348492ff547f1b9e80

spirv-reader: set failure bit when SPIR-V validation fails

The error message was already being logged, but the status
flag was not being set.

Change-Id: I7f6f2f45c15a64dd089e221c62218afe9e774edd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49643
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/reader/spirv/parser_impl.cc

diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index 9977113..ce972fa 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -291,6 +291,7 @@
   // Only consider modules valid for Vulkan 1.0.  On failure, the message
   // consumer will set the error status.
   if (!spv_tools.Validate(spv_binary_)) {
+    success_ = false;
     return false;
   }
   if (!BuildInternalModule()) {

src/reader/spirv/parser_impl_module_var_test.cc

diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc
index 5d4ff8e..adc8cc5 100644
--- a/src/reader/spirv/parser_impl_module_var_test.cc
+++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -2184,13 +2184,13 @@
     OpFunctionEnd
  )";
   auto p = parser(test::Assemble(assembly));
-  // TODO(dneto): We can handle this if we make a shadow variable and mutate
-  // the parameter type.
-  ASSERT_FALSE(p->BuildAndParseInternalModule());
-  EXPECT_THAT(
-      p->error(),
-      HasSubstr(
-          "unhandled use of a pointer to the SampleId builtin, with ID: 1"));
+
+  // This example is invalid because you can't pass pointer-to-Input
+  // as a function parameter.
+  EXPECT_FALSE(p->Parse());
+  EXPECT_FALSE(p->success());
+  EXPECT_THAT(p->error(),
+              HasSubstr("Invalid storage class for pointer operand 1"));
 }
 
 TEST_F(SpvModuleScopeVarParserTest, SampleId_U32_Load_Direct) {

src/reader/spirv/parser_impl_test_helper.h

diff --git a/src/reader/spirv/parser_impl_test_helper.h b/src/reader/spirv/parser_impl_test_helper.h
index 35cb635..240880a 100644
--- a/src/reader/spirv/parser_impl_test_helper.h
+++ b/src/reader/spirv/parser_impl_test_helper.h
@@ -64,6 +64,7 @@
   Namer& namer() { return impl_.namer(); }
   ProgramBuilder& builder() { return impl_.builder(); }
   const std::string error() { return impl_.error(); }
+  bool success() { return impl_.success(); }
   FailStream& Fail() { return impl_.Fail(); }
   spvtools::opt::IRContext* ir_context() { return impl_.ir_context(); }