diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 0ab3f1c3dd..d16330aeab 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -23,6 +23,18 @@ namespace dawn_native { + MaybeError ValidateBindingTypeWithShaderStageVisibility( + wgpu::BindingType bindingType, + wgpu::ShaderStage shaderStageVisibility) { + if (bindingType == wgpu::BindingType::StorageBuffer && + (shaderStageVisibility & wgpu::ShaderStage::Vertex) != 0) { + return DAWN_VALIDATION_ERROR( + "storage buffer binding is not supported in vertex shader"); + } + + return {}; + } + MaybeError ValidateBindGroupLayoutDescriptor(DeviceBase*, const BindGroupLayoutDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { @@ -49,11 +61,8 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("some binding index was specified more than once"); } - if (binding.type == wgpu::BindingType::StorageBuffer && - (binding.visibility & wgpu::ShaderStage::Vertex) != 0) { - return DAWN_VALIDATION_ERROR( - "storage buffer binding is not supported in vertex shader"); - } + DAWN_TRY( + ValidateBindingTypeWithShaderStageVisibility(binding.type, binding.visibility)); switch (binding.type) { case wgpu::BindingType::UniformBuffer: diff --git a/src/dawn_native/BindGroupLayout.h b/src/dawn_native/BindGroupLayout.h index 4c0dd7aae8..02d8453ac7 100644 --- a/src/dawn_native/BindGroupLayout.h +++ b/src/dawn_native/BindGroupLayout.h @@ -30,6 +30,10 @@ namespace dawn_native { MaybeError ValidateBindGroupLayoutDescriptor(DeviceBase*, const BindGroupLayoutDescriptor* descriptor); + MaybeError ValidateBindingTypeWithShaderStageVisibility( + wgpu::BindingType bindingType, + wgpu::ShaderStage shaderStageVisibility); + class BindGroupLayoutBase : public CachedObject { public: BindGroupLayoutBase(DeviceBase* device, const BindGroupLayoutDescriptor* descriptor); diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index a810f37435..5cdf781a35 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -132,6 +132,9 @@ namespace dawn_native { BindGroupLayoutBinding bindingSlot; bindingSlot.binding = binding; + + DAWN_TRY(ValidateBindingTypeWithShaderStageVisibility( + bindingInfo.type, StageBit(module->GetExecutionModel()))); if (bindingInfo.type == wgpu::BindingType::StorageBuffer) { bindingSlot.visibility = wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute; diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp index f8b3d42415..9ffbbc91ea 100644 --- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -456,3 +456,24 @@ TEST_F(RenderPipelineValidationTest, TextureViewDimensionCompatibility) { } } } + +// Test that declaring a storage buffer in the vertex shader without setting pipeline layout won't +// cause crash. +TEST_F(RenderPipelineValidationTest, StorageBufferInVertexShaderNoLayout) { + wgpu::ShaderModule vsModuleWithStorageBuffer = + utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"( + #version 450 + #define kNumValues 100 + layout(std430, set = 0, binding = 0) buffer Dst { uint dst[kNumValues]; }; + void main() { + uint index = gl_VertexIndex; + dst[index] = 0x1234; + gl_Position = vec4(1.f, 0.f, 0.f, 1.f); + })"); + + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.layout = nullptr; + descriptor.vertexStage.module = vsModuleWithStorageBuffer; + descriptor.cFragmentStage.module = fsModule; + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); +}