From e629aee91a87e9ad7d0b5d4cee88206eda15565e Mon Sep 17 00:00:00 2001
From: Corentin Wallez <cwallez@chromium.org>
Date: Mon, 11 Jul 2022 17:21:17 +0000
Subject: [PATCH] dawn::native: Check that ExternalTexture bindings are
 ExternalTextures

The code was checking the equivalence in one direction but not the
other, leading to a case where passing a TextureView instead of an
ExternalTexture passed validation and lead to crashes in the backends.

Bug: chromium:1343099
Change-Id: I428252796df375e7cf3a6df1a03192d65364e370
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/95944
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Shrek Shao <shrekshao@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
---
 src/dawn/native/BindGroup.cpp                    |  6 ++++++
 .../validation/ExternalTextureTests.cpp          | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/src/dawn/native/BindGroup.cpp b/src/dawn/native/BindGroup.cpp
index 802213c65f..0d3af775b3 100644
--- a/src/dawn/native/BindGroup.cpp
+++ b/src/dawn/native/BindGroup.cpp
@@ -301,6 +301,12 @@ MaybeError ValidateBindGroupDescriptor(DeviceBase* device, const BindGroupDescri
                 device, entry, externalTextureBindingEntry,
                 descriptor->layout->GetExternalTextureBindingExpansionMap()));
             continue;
+        } else {
+            DAWN_INVALID_IF(descriptor->layout->GetExternalTextureBindingExpansionMap().count(
+                                BindingNumber(entry.binding)),
+                            "entries[%u] is not an ExternalTexture when the layout contains an "
+                            "ExternalTexture entry.",
+                            i);
         }
 
         const BindingInfo& bindingInfo = descriptor->layout->GetBindingInfo(bindingIndex);
diff --git a/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp b/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
index 246ce5b625..6f6aebcf74 100644
--- a/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
+++ b/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
@@ -541,6 +541,22 @@ TEST_F(ExternalTextureTest, BindGroupDoesNotMatchLayout) {
     }
 }
 
+// Regression test for crbug.com/1343099 where BindGroup validation let other binding types be used
+// for external texture bindings.
+TEST_F(ExternalTextureTest, TextureViewBindingDoesntMatch) {
+    wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+        device, {{0, wgpu::ShaderStage::Fragment, &utils::kExternalTextureBindingLayout}});
+
+    wgpu::TextureDescriptor textureDescriptor = CreateTextureDescriptor();
+    wgpu::Texture texture = device.CreateTexture(&textureDescriptor);
+
+    // The bug was that this passed validation and crashed inside the backends with a null
+    // dereference. It passed validation because the number of bindings matched (1 == 1) and that
+    // the validation didn't check that an external texture binding was required, fell back to
+    // checking for the binding type of entry 0 that had been decayed to be a sampled texture view.
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, bgl, {{0, texture.CreateView()}}));
+}
+
 // Ensure that bind group validation catches error external textures.
 TEST_F(ExternalTextureTest, UseErrorExternalTextureInBindGroup) {
     // Control case should succeed.