commit 7b57c5bb770fb233cea6a4b6c70b54c4d98e71df
author Corentin Wallez <cwallez@chromium.org> Tue Jul 30 06:49:09 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Jul 30 06:49:09 2019 +0000
tree f44a5ba19db115287290306c3d5efbc6c1579583
parent 34f8bd8c4850fd007057de3a7ae64cde91304c42

Check bindgroup validity first in SetBindGroup

bindgroup->GetLayout() was called before checking the bindgroup is an
error, causing an assert to fire in Debug. Adds a regression unittest.

BUG=dawn:196

Change-Id: I58e95879a7f2a7cf5c47c7b228f7e3b5bb72d8ea
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/9560
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/ProgrammablePassEncoder.cpp

diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp
index 4210a13..e59eab8 100644
--- a/src/dawn_native/ProgrammablePassEncoder.cpp
+++ b/src/dawn_native/ProgrammablePassEncoder.cpp
@@ -75,8 +75,6 @@
                                                uint32_t dynamicOffsetCount,
                                                const uint64_t* dynamicOffsets) {
         mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
-            const BindGroupLayoutBase* layout = group->GetLayout();
-
             DAWN_TRY(GetDevice()->ValidateObject(group));
 
             if (groupIndex >= kMaxBindGroups) {
@@ -84,6 +82,7 @@
             }
 
             // Dynamic offsets count must match the number required by the layout perfectly.
+            const BindGroupLayoutBase* layout = group->GetLayout();
             if (layout->GetDynamicBufferCount() != dynamicOffsetCount) {
                 return DAWN_VALIDATION_ERROR("dynamicOffset count mismatch");
             }

src/tests/unittests/validation/BindGroupValidationTests.cpp

diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index 7bf8b0a..3f71629 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -729,3 +729,15 @@
 
     TestComputePassBindGroup(bindGroup, offsets.data(), 2, false);
 }
+
+// Test that an error is produced (and no ASSERTs fired) when using an error bindgroup in
+// SetBindGroup
+TEST_F(SetBindGroupValidationTest, ErrorBindGroup) {
+    // Bindgroup creation fails because not all bindings are specified.
+    dawn::BindGroup bindGroup;
+    ASSERT_DEVICE_ERROR(bindGroup = utils::MakeBindGroup(device, mBindGroupLayout, {}));
+
+    TestRenderPassBindGroup(bindGroup, nullptr, 0, false);
+
+    TestComputePassBindGroup(bindGroup, nullptr, 0, false);
+}