commit 393d555a0ea9a809c6bd486e19e177118102c61a
author Corentin Wallez <cwallez@chromium.org> Mon Feb 11 23:35:33 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Feb 11 23:35:33 2019 +0000
tree d7ab3da2d5c5a71f7d74ae7d4d5c801a2bce6dd8
parent 6be3b97ff55d419be666fc172a3f01b3d0814cd4

PipelineLayout: Only allow accessing defined BGLs.

Undefined BGLs are stored as nullptr in the pipeline layout, so to be
defensive we assert clients of PipelineLayout can't request undefined
BGLs (which forces iteration on the BGLMask).

Also fix two cases where the BGLs were iterated on incorrectly.

BUG=dawn:8

Change-Id: I3c084125b93d6684b4c79a56745632874079ebec
Reviewed-on: https://dawn-review.googlesource.com/c/4480
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/PipelineLayout.cpp

diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp
index f067ec5..96772bd 100644
--- a/src/dawn_native/PipelineLayout.cpp
+++ b/src/dawn_native/PipelineLayout.cpp
@@ -52,6 +52,7 @@
 
     const BindGroupLayoutBase* PipelineLayoutBase::GetBindGroupLayout(size_t group) const {
         ASSERT(group < kMaxBindGroups);
+        ASSERT(mMask[group]);
         return mBindGroupLayouts[group].Get();
     }
 

src/dawn_native/ShaderModule.cpp

diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index d7867a3..7a0e49e 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -227,11 +227,20 @@
     }
 
     bool ShaderModuleBase::IsCompatibleWithPipelineLayout(const PipelineLayoutBase* layout) {
-        for (size_t group = 0; group < kMaxBindGroups; ++group) {
+        for (uint32_t group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
             if (!IsCompatibleWithBindGroupLayout(group, layout->GetBindGroupLayout(group))) {
                 return false;
             }
         }
+
+        for (uint32_t group : IterateBitSet(~layout->GetBindGroupLayoutsMask())) {
+            for (size_t i = 0; i < kMaxBindingsPerGroup; ++i) {
+                if (mBindingInfo[group][i].used) {
+                    return false;
+                }
+            }
+        }
+
         return true;
     }
 

src/dawn_native/d3d12/ShaderModuleD3D12.cpp

diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
index 9911d51..b9ec960 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
@@ -15,6 +15,7 @@
 #include "dawn_native/d3d12/ShaderModuleD3D12.h"
 
 #include "common/Assert.h"
+#include "common/BitSetIterator.h"
 #include "dawn_native/d3d12/BindGroupLayoutD3D12.h"
 #include "dawn_native/d3d12/DeviceD3D12.h"
 #include "dawn_native/d3d12/PipelineLayoutD3D12.h"
@@ -45,7 +46,7 @@
         compiler.set_hlsl_options(options_hlsl);
 
         const ModuleBindingInfo& moduleBindingInfo = GetBindingInfo();
-        for (uint32_t group = 0; group < moduleBindingInfo.size(); ++group) {
+        for (uint32_t group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
             const auto& bindingOffsets =
                 ToBackend(layout->GetBindGroupLayout(group))->GetBindingOffsets();
             const auto& groupBindingInfo = moduleBindingInfo[group];