From 268fe254ec50a72b28b03017c63044a8ecbe92ba Mon Sep 17 00:00:00 2001 From: Tomek Ponitka Date: Tue, 30 Jun 2020 18:49:20 +0000 Subject: [PATCH] Added constraints for too large uniform buffer bindings Added a constraint in ValidateBufferBinding function checking for too large uniform buffer bindings. Added MaxUniformBufferBindingSize test in BindGroupValidationTest. Bug: dawn:436 Change-Id: I31c6e2236ce928d5e81c43455eb18cf4eacdc0f1 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24081 Commit-Queue: Tomek Ponitka Reviewed-by: Austin Eng --- src/common/Constants.h | 2 + src/dawn_native/BindGroup.cpp | 16 ++++++-- .../validation/BindGroupValidationTests.cpp | 38 +++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/common/Constants.h b/src/common/Constants.h index df792cd5d6..ab3730308d 100644 --- a/src/common/Constants.h +++ b/src/common/Constants.h @@ -42,6 +42,8 @@ static constexpr uint32_t kMaxDynamicStorageBufferCount = 4u; // Max numbers of dynamic buffers static constexpr uint32_t kMaxDynamicBufferCount = kMaxDynamicUniformBufferCount + kMaxDynamicStorageBufferCount; +// Max size of uniform buffer binding +static constexpr uint64_t kMaxUniformBufferBindingSize = 16384u; // Indirect command sizes static constexpr uint64_t kDispatchIndirectSize = 3 * sizeof(uint32_t); static constexpr uint64_t kDrawIndirectSize = 4 * sizeof(uint32_t); diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 4e4e67a447..2da980dab1 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -32,7 +32,8 @@ namespace dawn_native { MaybeError ValidateBufferBinding(const DeviceBase* device, const BindGroupEntry& entry, wgpu::BufferUsage requiredUsage, - const BindingInfo& bindingInfo) { + const BindingInfo& bindingInfo, + const uint64_t maxBindingSize) { if (entry.buffer == nullptr || entry.sampler != nullptr || entry.textureView != nullptr) { return DAWN_VALIDATION_ERROR("expected buffer binding"); @@ -79,6 +80,14 @@ namespace dawn_native { " bytes"); } + if (bindingSize > maxBindingSize) { + return DAWN_VALIDATION_ERROR( + "Binding size bigger than maximum uniform buffer binding size: binding " + + std::to_string(entry.binding) + " given " + std::to_string(bindingSize) + + " bytes, maximum is " + std::to_string(kMaxUniformBufferBindingSize) + + " bytes"); + } + return {}; } @@ -192,12 +201,13 @@ namespace dawn_native { switch (bindingInfo.type) { case wgpu::BindingType::UniformBuffer: DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Uniform, - bindingInfo)); + bindingInfo, kMaxUniformBufferBindingSize)); break; case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::ReadonlyStorageBuffer: DAWN_TRY(ValidateBufferBinding(device, entry, wgpu::BufferUsage::Storage, - bindingInfo)); + bindingInfo, + std::numeric_limits::max())); break; case wgpu::BindingType::SampledTexture: DAWN_TRY(ValidateTextureBinding(device, entry, wgpu::TextureUsage::Sampled, diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index a3bd47c4ae..5e6f16ff96 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -441,6 +441,44 @@ TEST_F(BindGroupValidationTest, BufferBindingOOB) { ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, buffer, 256, uint32_t(0) - uint32_t(256)}})); } +// Tests constraints to be sure the uniform buffer binding isn't too large +TEST_F(BindGroupValidationTest, MaxUniformBufferBindingSize) { + wgpu::BufferDescriptor descriptor; + descriptor.size = 2 * kMaxUniformBufferBindingSize; + descriptor.usage = wgpu::BufferUsage::Uniform | wgpu::BufferUsage::Storage; + wgpu::Buffer buffer = device.CreateBuffer(&descriptor); + + wgpu::BindGroupLayout uniformLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer}}); + + // Success case, this is exactly the limit + utils::MakeBindGroup(device, uniformLayout, {{0, buffer, 0, kMaxUniformBufferBindingSize}}); + + wgpu::BindGroupLayout doubleUniformLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer}, + {1, wgpu::ShaderStage::Vertex, wgpu::BindingType::UniformBuffer}}); + + // Success case, individual bindings don't exceed the limit + utils::MakeBindGroup(device, doubleUniformLayout, + {{0, buffer, 0, kMaxUniformBufferBindingSize}, + {1, buffer, kMaxUniformBufferBindingSize, kMaxUniformBufferBindingSize}}); + + // Error case, this is above the limit + ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, uniformLayout, + {{0, buffer, 0, kMaxUniformBufferBindingSize + 1}})); + + // Making sure the constraint doesn't apply to storage buffers + wgpu::BindGroupLayout readonlyStorageLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}}); + wgpu::BindGroupLayout storageLayout = utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}}); + + // Success case, storage buffer can still be created. + utils::MakeBindGroup(device, readonlyStorageLayout, + {{0, buffer, 0, 2 * kMaxUniformBufferBindingSize}}); + utils::MakeBindGroup(device, storageLayout, {{0, buffer, 0, 2 * kMaxUniformBufferBindingSize}}); +} + // Test what happens when the layout is an error. TEST_F(BindGroupValidationTest, ErrorLayout) { wgpu::BindGroupLayout goodLayout = utils::MakeBindGroupLayout(