Add validation of the max binding number.

Fixed: dawn:1283
Change-Id: I5efd0d5c92bd6c1a4cdfe91079a12a9f98ddfd61
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/79260
Reviewed-by: Brandon Jones <bajones@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn/common/Constants.h

@@ -62,4 +62,7 @@ static constexpr uint8_t kSampledTexturesPerExternalTexture = 3u;
 static constexpr uint8_t kSamplersPerExternalTexture = 1u;
 static constexpr uint8_t kUniformsPerExternalTexture = 1u;
 
+// A spec defined constant but that doesn't have a name.
+static constexpr uint32_t kMaxBindingNumber = 65535;
+
 #endif  // COMMON_CONSTANTS_H_

src/dawn/native/BindGroupLayout.cpp

@@ -167,6 +167,9 @@ namespace dawn::native {
             const BindGroupLayoutEntry& entry = descriptor->entries[i];
             BindingNumber bindingNumber = BindingNumber(entry.binding);
 
+            DAWN_INVALID_IF(bindingNumber > kMaxBindingNumberTyped,
+                            "Binding number (%u) exceeds the maximum binding number (%u).",
+                            uint32_t(bindingNumber), uint32_t(kMaxBindingNumberTyped));
             DAWN_INVALID_IF(bindingsSet.count(bindingNumber) != 0,
                             "On entries[%u]: binding index (%u) was specified by a previous entry.",
                             i, entry.binding);

src/dawn/native/IntegerTypes.h

@@ -23,13 +23,14 @@
 namespace dawn::native {
     // Binding numbers in the shader and BindGroup/BindGroupLayoutDescriptors
     using BindingNumber = TypedInteger<struct BindingNumberT, uint32_t>;
+    constexpr BindingNumber kMaxBindingNumberTyped = BindingNumber(kMaxBindingNumber);
 
     // Binding numbers get mapped to a packed range of indices
     using BindingIndex = TypedInteger<struct BindingIndexT, uint32_t>;
 
     using BindGroupIndex = TypedInteger<struct BindGroupIndexT, uint32_t>;
 
-    static constexpr BindGroupIndex kMaxBindGroupsTyped = BindGroupIndex(kMaxBindGroups);
+    constexpr BindGroupIndex kMaxBindGroupsTyped = BindGroupIndex(kMaxBindGroups);
 
     using ColorAttachmentIndex = TypedInteger<struct ColorAttachmentIndexT, uint8_t>;
 

src/dawn/native/ShaderModule.cpp

@@ -832,6 +832,10 @@ namespace dawn::native {
                     BindingNumber bindingNumber(resource.binding);
                     BindGroupIndex bindGroupIndex(resource.bind_group);
 
+                    DAWN_INVALID_IF(bindingNumber > kMaxBindingNumberTyped,
+                                    "Binding number (%u) exceeds the maximum binding number (%u).",
+                                    uint32_t(bindingNumber), uint32_t(kMaxBindingNumberTyped));
+
                     const auto& [binding, inserted] = metadata->bindings[bindGroupIndex].emplace(
                         bindingNumber, ShaderBindingInfo{});
                     DAWN_INVALID_IF(

src/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -925,11 +925,15 @@ TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutStorageBindingsInVertexShad
 }
 
 // Tests setting that bind group layout bindings numbers may be very large.
-TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutEntryNumberLarge) {
+TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutEntryMax) {
-    // Checks that uint32_t max is valid.
+    // Check that up to kMaxBindingNumber is valid.
-    utils::MakeBindGroupLayout(device,
+    utils::MakeBindGroupLayout(
-                               {{std::numeric_limits<uint32_t>::max(), wgpu::ShaderStage::Vertex,
+        device, {{kMaxBindingNumber, wgpu::ShaderStage::Vertex, wgpu::BufferBindingType::Uniform}});
-                                 wgpu::BufferBindingType::Uniform}});
+
+    // But after is an error.
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device,
+        {{kMaxBindingNumber + 1, wgpu::ShaderStage::Vertex, wgpu::BufferBindingType::Uniform}}));
 }
 
 // This test verifies that the BindGroupLayout bindings are correctly validated, even if the

src/tests/unittests/validation/ShaderModuleValidationTests.cpp

@@ -512,3 +512,26 @@ struct Buf {
     buf.data[1] = c1;
 })"));
 }
+
+// Test that @binding must be less then kMaxBindingNumber
+TEST_F(ShaderModuleValidationTest, MaxBindingNumber) {
+    static_assert(kMaxBindingNumber == 65535);
+
+    // kMaxBindingNumber is valid.
+    utils::CreateShaderModule(device, R"(
+        @group(0) @binding(65535) var s : sampler;
+        @stage(fragment) fn main() -> @location(0) u32 {
+            _ = s;
+            return 0u;
+        }
+    )");
+
+    // kMaxBindingNumber + 1 is an error
+    ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
+        @group(0) @binding(65536) var s : sampler;
+        @stage(fragment) fn main() -> @location(0) u32 {
+            _ = s;
+            return 0u;
+        }
+    )"));
+}