From f4c8a6ac9ba5f0032cec430b82080608c90d3567 Mon Sep 17 00:00:00 2001
From: Shrek Shao <shrekshao@google.com>
Date: Tue, 26 Oct 2021 02:06:36 +0000
Subject: [PATCH] Mark pipeline overridable constants as unsafe

Until all backend implementations are done.

Bug: dawn:1169
Change-Id: I16d184bec03bbc9a255ad33cf11aa615b1986389
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67422
Commit-Queue: Shrek Shao <shrekshao@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
---
 src/dawn_native/Pipeline.cpp                  |  6 +++
 src/dawn_native/ShaderModule.cpp              |  4 ++
 .../validation/UnsafeAPIValidationTests.cpp   | 40 +++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index 4fafe8b1e9..96258bc63c 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/src/dawn_native/Pipeline.cpp
@@ -45,6 +45,12 @@ namespace dawn_native {
             DAWN_TRY(ValidateCompatibilityWithPipelineLayout(device, metadata, layout));
         }
 
+        if (constantCount > 0u && device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+            return DAWN_VALIDATION_ERROR(
+                "Pipeline overridable constants are disallowed because they are partially "
+                "implemented.");
+        }
+
         // Validate if overridable constants exist in shader module
         // pipelineBase is not yet constructed at this moment so iterate constants from descriptor
         for (uint32_t i = 0; i < constantCount; i++) {
diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index cfb45a1927..7a7bbd7133 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -631,6 +631,10 @@ namespace dawn_native {
                 auto metadata = std::make_unique<EntryPointMetadata>();
 
                 if (!entryPoint.overridable_constants.empty()) {
+                    DAWN_INVALID_IF(device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs),
+                                    "Pipeline overridable constants are disallowed because they "
+                                    "are partially implemented.");
+
                     const auto& name2Id = inspector.GetConstantNameToIdMap();
 
                     for (auto& c : entryPoint.overridable_constants) {
diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
index 7bb9e19acc..a7e915a3bd 100644
--- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
+++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
@@ -106,6 +106,46 @@ TEST_F(UnsafeAPIValidationTest, DynamicStorageBuffer) {
     }
 }
 
+// Check that pipeline overridable constants are disallowed as part of unsafe APIs.
+// TODO(dawn:1041) Remove when implementation for all backend is added
+TEST_F(UnsafeAPIValidationTest, PipelineOverridableConstants) {
+    // Create the dummy compute pipeline.
+    wgpu::ComputePipelineDescriptor pipelineDescBase;
+    pipelineDescBase.compute.entryPoint = "main";
+
+    // Control case: shader without overridable constant is allowed.
+    {
+        wgpu::ComputePipelineDescriptor pipelineDesc = pipelineDescBase;
+        pipelineDesc.compute.module =
+            utils::CreateShaderModule(device, "[[stage(compute), workgroup_size(1)]] fn main() {}");
+
+        device.CreateComputePipeline(&pipelineDesc);
+    }
+
+    // Error case: shader with overridable constant with default value
+    {
+        ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
+[[override(1000)]] let c0: u32 = 1u;
+[[override(1000)]] let c1: u32;
+
+[[stage(compute), workgroup_size(1)]] fn main() {
+    ignore(c0);
+    ignore(c1);
+})"));
+    }
+
+    // Error case: pipeline stage with constant entry is disallowed
+    {
+        wgpu::ComputePipelineDescriptor pipelineDesc = pipelineDescBase;
+        pipelineDesc.compute.module =
+            utils::CreateShaderModule(device, "[[stage(compute), workgroup_size(1)]] fn main() {}");
+        std::vector<wgpu::ConstantEntry> constants{{nullptr, "c", 1u}};
+        pipelineDesc.compute.constants = constants.data();
+        pipelineDesc.compute.constantCount = constants.size();
+        ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&pipelineDesc));
+    }
+}
+
 class UnsafeQueryAPIValidationTest : public ValidationTest {
   protected:
     WGPUDevice CreateTestDevice() override {