Validate SPIR-V code when creating ShaderModules

This integrates spirv-val in dawn_native so that regular and
WebGPU-specific validation of shaders is done.

Also adds tests to check OpUndef is correctly rejected so we know
WebGPU-specific validation is working.

Change-Id: If49d276c98bca8cd3c6c1a420903fe34923a2942

BUILD.gn

@@ -314,6 +314,7 @@ source_set("libdawn_native_sources") {
   deps = [
     ":dawn_common",
     ":libdawn_native_utils_gen",
+    "${dawn_spirv_tools_dir}:spvtools_val",
     "third_party:spirv_cross",
   ]
 
@@ -772,6 +773,7 @@ test("dawn_unittests") {
     "src/tests/unittests/validation/PushConstantsValidationTests.cpp",
     "src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp",
     "src/tests/unittests/validation/RenderPipelineValidationTests.cpp",
+    "src/tests/unittests/validation/ShaderModuleValidationTests.cpp",
     "src/tests/unittests/validation/ValidationTest.cpp",
     "src/tests/unittests/validation/ValidationTest.h",
     "src/tests/unittests/validation/VertexBufferValidationTests.cpp",

src/dawn_native/CMakeLists.txt

@@ -31,8 +31,8 @@ Generate(
 )
 
 set(DAWN_NATIVE_SOURCES)
-set(DAWN_NATIVE_DEPS dawn_common spirv_cross dawn_native_utils_autogen)
+set(DAWN_NATIVE_DEPS dawn_common spirv_cross dawn_native_utils_autogen SPIRV-Tools)
-set(DAWN_NATIVE_INCLUDE_DIRS ${SPIRV_CROSS_INCLUDE_DIR})
+set(DAWN_NATIVE_INCLUDE_DIRS ${SPIRV_CROSS_INCLUDE_DIR} ${SPIRV_TOOLS_INCLUDE_DIR})
 
 ################################################################################
 # OpenGL Backend

src/dawn_native/ShaderModule.cpp

@@ -20,13 +20,44 @@
 #include "dawn_native/PipelineLayout.h"
 
 #include <spirv-cross/spirv_cross.hpp>
+#include <spirv-tools/libspirv.hpp>
 
 namespace dawn_native {
 
     MaybeError ValidateShaderModuleDescriptor(DeviceBase*,
                                               const ShaderModuleDescriptor* descriptor) {
         DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr");
-        // TODO(cwallez@chromium.org): Use spirv-val to check the module is well-formed
+
+        spvtools::SpirvTools spirvTools(SPV_ENV_WEBGPU_0);
+
+        std::ostringstream errorStream;
+        errorStream << "SPIRV Validation failure:" << std::endl;
+
+        spirvTools.SetMessageConsumer([&errorStream](spv_message_level_t level, const char*,
+                                                     const spv_position_t& position,
+                                                     const char* message) {
+            switch (level) {
+                case SPV_MSG_FATAL:
+                case SPV_MSG_INTERNAL_ERROR:
+                case SPV_MSG_ERROR:
+                    errorStream << "error: line " << position.index << ": " << message << std::endl;
+                    break;
+                case SPV_MSG_WARNING:
+                    errorStream << "warning: line " << position.index << ": " << message
+                                << std::endl;
+                    break;
+                case SPV_MSG_INFO:
+                    errorStream << "info: line " << position.index << ": " << message << std::endl;
+                    break;
+                default:
+                    break;
+            }
+        });
+
+        if (!spirvTools.Validate(descriptor->code, descriptor->codeSize)) {
+            DAWN_RETURN_ERROR(errorStream.str().c_str());
+        }
+
         return {};
     }
 

src/tests/CMakeLists.txt

@@ -53,6 +53,7 @@ list(APPEND UNITTEST_SOURCES
     ${VALIDATION_TESTS_DIR}/PushConstantsValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/RenderPassDescriptorValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/RenderPipelineValidationTests.cpp
+    ${VALIDATION_TESTS_DIR}/ShaderModuleValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/VertexBufferValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/ValidationTest.cpp
     ${VALIDATION_TESTS_DIR}/ValidationTest.h

src/tests/unittests/validation/ShaderModuleValidationTests.cpp

@@ -0,0 +1,89 @@
+// Copyright 2018 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tests/unittests/validation/ValidationTest.h"
+
+#include "utils/DawnHelpers.h"
+
+class ShaderModuleValidationTest : public ValidationTest {
+};
+
+// Test case with a simpler shader that should successfully be created
+TEST_F(ShaderModuleValidationTest, CreationSuccess) {
+    const char* shader = R"(
+                   OpCapability Shader
+              %1 = OpExtInstImport "GLSL.std.450"
+                   OpMemoryModel Logical GLSL450
+                   OpEntryPoint Fragment %main "main" %fragColor
+                   OpExecutionMode %main OriginUpperLeft
+                   OpSource GLSL 450
+                   OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
+                   OpSourceExtension "GL_GOOGLE_include_directive"
+                   OpName %main "main"
+                   OpName %fragColor "fragColor"
+                   OpDecorate %fragColor Location 0
+           %void = OpTypeVoid
+              %3 = OpTypeFunction %void
+          %float = OpTypeFloat 32
+        %v4float = OpTypeVector %float 4
+    %_ptr_Output_v4float = OpTypePointer Output %v4float
+      %fragColor = OpVariable %_ptr_Output_v4float Output
+        %float_1 = OpConstant %float 1
+        %float_0 = OpConstant %float 0
+             %12 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+           %main = OpFunction %void None %3
+              %5 = OpLabel
+                   OpStore %fragColor %12
+                   OpReturn
+                   OpFunctionEnd)";
+
+    utils::CreateShaderModuleFromASM(device, shader);
+}
+
+// Test case with a shader with OpUndef to test WebGPU-specific validation
+TEST_F(ShaderModuleValidationTest, OpUndef) {
+    const char* shader = R"(
+                   OpCapability Shader
+              %1 = OpExtInstImport "GLSL.std.450"
+                   OpMemoryModel Logical GLSL450
+                   OpEntryPoint Fragment %main "main" %fragColor
+                   OpExecutionMode %main OriginUpperLeft
+                   OpSource GLSL 450
+                   OpSourceExtension "GL_GOOGLE_cpp_style_line_directive"
+                   OpSourceExtension "GL_GOOGLE_include_directive"
+                   OpName %main "main"
+                   OpName %fragColor "fragColor"
+                   OpDecorate %fragColor Location 0
+           %void = OpTypeVoid
+              %3 = OpTypeFunction %void
+          %float = OpTypeFloat 32
+        %v4float = OpTypeVector %float 4
+    %_ptr_Output_v4float = OpTypePointer Output %v4float
+      %fragColor = OpVariable %_ptr_Output_v4float Output
+        %float_1 = OpConstant %float 1
+        %float_0 = OpConstant %float 0
+             %12 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1
+           %main = OpFunction %void None %3
+              %5 = OpLabel
+              %6 = OpUndef %v4float
+                   OpStore %fragColor %12
+                   OpReturn
+                   OpFunctionEnd)";
+
+    // Notice "%6 = OpUndef %v4float" above
+    ASSERT_DEVICE_ERROR(utils::CreateShaderModuleFromASM(device, shader));
+
+    std::string error = GetLastDeviceErrorMessage();
+    ASSERT_NE(error.find("OpUndef"), std::string::npos);
+}

src/tests/unittests/validation/ValidationTest.cpp

@@ -69,6 +69,9 @@ bool ValidationTest::EndExpectDeviceError() {
     mExpectError = false;
     return mError;
 }
+std::string ValidationTest::GetLastDeviceErrorMessage() const {
+    return mDeviceErrorMessage;
+}
 
 dawn::RenderPassDescriptor ValidationTest::CreateSimpleRenderPass() {
         dawn::TextureDescriptor descriptor;
@@ -91,14 +94,16 @@ dawn::RenderPassDescriptor ValidationTest::CreateSimpleRenderPass() {
 }
 
 void ValidationTest::OnDeviceError(const char* message, dawnCallbackUserdata userdata) {
+    auto self = reinterpret_cast<ValidationTest*>(static_cast<uintptr_t>(userdata));
+    self->mDeviceErrorMessage = message;
+
     // Skip this one specific error that is raised when a builder is used after it got an error
     // this is important because we don't want to wrap all creation tests in ASSERT_DEVICE_ERROR.
     // Yes the error message is misleading.
-    if (std::string(message) == "Builder cannot be used after GetResult") {
+    if (self->mDeviceErrorMessage == "Builder cannot be used after GetResult") {
         return;
     }
 
-    auto self = reinterpret_cast<ValidationTest*>(static_cast<uintptr_t>(userdata));
     ASSERT_TRUE(self->mExpectError) << "Got unexpected device error: " << message;
     ASSERT_FALSE(self->mError) << "Got two errors in expect block";
     self->mError = true;

src/tests/unittests/validation/ValidationTest.h

@@ -47,6 +47,7 @@ class ValidationTest : public testing::Test {
 
         void StartExpectDeviceError();
         bool EndExpectDeviceError();
+        std::string GetLastDeviceErrorMessage() const;
 
         dawn::RenderPassDescriptor CreateSimpleRenderPass();
 
@@ -66,6 +67,7 @@ class ValidationTest : public testing::Test {
 
     private:
         static void OnDeviceError(const char* message, dawnCallbackUserdata userdata);
+        std::string mDeviceErrorMessage;
         bool mExpectError = false;
         bool mError = false;
 

src/utils/DawnHelpers.cpp

@@ -25,33 +25,24 @@
 
 namespace utils {
 
-    dawn::ShaderModule CreateShaderModule(const dawn::Device& device,
+    namespace {
-                                          dawn::ShaderStage stage,
-                                          const char* source) {
-        shaderc::Compiler compiler;
-        shaderc::CompileOptions options;
 
-        shaderc_shader_kind kind;
+        shaderc_shader_kind ShadercShaderKind(dawn::ShaderStage stage) {
             switch (stage) {
                 case dawn::ShaderStage::Vertex:
-                kind = shaderc_glsl_vertex_shader;
+                    return shaderc_glsl_vertex_shader;
-                break;
                 case dawn::ShaderStage::Fragment:
-                kind = shaderc_glsl_fragment_shader;
+                    return shaderc_glsl_fragment_shader;
-                break;
                 case dawn::ShaderStage::Compute:
-                kind = shaderc_glsl_compute_shader;
+                    return shaderc_glsl_compute_shader;
-                break;
                 default:
                     UNREACHABLE();
             }
-
-        auto result = compiler.CompileGlslToSpv(source, strlen(source), kind, "myshader?", options);
-        if (result.GetCompilationStatus() != shaderc_compilation_status_success) {
-            std::cerr << result.GetErrorMessage();
-            return {};
         }
 
+        dawn::ShaderModule CreateShaderModuleFromResult(
+            const dawn::Device& device,
+            const shaderc::SpvCompilationResult& result) {
             // result.cend and result.cbegin return pointers to uint32_t.
             const uint32_t* resultBegin = result.cbegin();
             const uint32_t* resultEnd = result.cend();
@@ -62,9 +53,25 @@ namespace utils {
             dawn::ShaderModuleDescriptor descriptor;
             descriptor.codeSize = static_cast<uint32_t>(resultSize);
             descriptor.code = result.cbegin();
+            return device.CreateShaderModule(&descriptor);
+        }
 
+    }  // anonymous namespace
+
+    dawn::ShaderModule CreateShaderModule(const dawn::Device& device,
+                                          dawn::ShaderStage stage,
+                                          const char* source) {
+        shaderc_shader_kind kind = ShadercShaderKind(stage);
+
+        shaderc::Compiler compiler;
+        auto result = compiler.CompileGlslToSpv(source, strlen(source), kind, "myshader?");
+        if (result.GetCompilationStatus() != shaderc_compilation_status_success) {
+            std::cerr << result.GetErrorMessage();
+            return {};
+        }
 #ifdef DUMP_SPIRV_ASSEMBLY
         {
+            shaderc::CompileOptions options;
             auto resultAsm = compiler.CompileGlslToSpvAssembly(source, strlen(source), kind,
                                                                "myshader?", options);
             size_t sizeAsm = (resultAsm.cend() - resultAsm.cbegin());
@@ -91,7 +98,18 @@ namespace utils {
         printf("SPIRV JS ARRAY DUMP END\n");
 #endif
 
-        return device.CreateShaderModule(&descriptor);
+        return CreateShaderModuleFromResult(device, result);
+    }
+
+    dawn::ShaderModule CreateShaderModuleFromASM(const dawn::Device& device, const char* source) {
+        shaderc::Compiler compiler;
+        shaderc::SpvCompilationResult result = compiler.AssembleToSpv(source, strlen(source));
+        if (result.GetCompilationStatus() != shaderc_compilation_status_success) {
+            std::cerr << result.GetErrorMessage();
+            return {};
+        }
+
+        return CreateShaderModuleFromResult(device, result);
     }
 
     dawn::Buffer CreateBufferFromData(const dawn::Device& device,

src/utils/DawnHelpers.h

@@ -21,6 +21,8 @@ namespace utils {
     dawn::ShaderModule CreateShaderModule(const dawn::Device& device,
                                           dawn::ShaderStage stage,
                                           const char* source);
+    dawn::ShaderModule CreateShaderModuleFromASM(const dawn::Device& device, const char* source);
+
     dawn::Buffer CreateBufferFromData(const dawn::Device& device,
                                       const void* data,
                                       uint32_t size,

third_party/CMakeLists.txt

@@ -41,6 +41,9 @@ set(GLAD_INCLUDE_DIR ${GLAD_INCLUDE_DIR} PARENT_SCOPE)
 target_include_directories(glad SYSTEM PUBLIC ${GLAD_INCLUDE_DIR})
 DawnExternalTarget("third_party" glad)
 
+# SPIRV-Tools
+set(SPIRV_TOOLS_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/spirv-tools/include PARENT_SCOPE)
+
 # ShaderC
 # Prevent SPIRV-Tools from using Werror as it has a warning on MSVC
 set(SPIRV_WERROR OFF CACHE BOOL "" FORCE)