From 27a294c2e831412f3063b289378e341ee0a0814f Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Thu, 28 Mar 2019 15:28:01 +0000 Subject: [PATCH] Fix null pointer dereference in CommandBufferStateTracker The CommandBufferStateTracker dereferences a null pointer if there is no bind group set for an index of the layout Change-Id: Id247f9876340d189088e8bbe8cb4d68a4d517c6e Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6100 Commit-Queue: Austin Eng Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez --- BUILD.gn | 1 + src/dawn_native/CommandBufferStateTracker.cpp | 3 +- .../validation/RenderPassValidationTests.cpp | 117 ++++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 src/tests/unittests/validation/RenderPassValidationTests.cpp diff --git a/BUILD.gn b/BUILD.gn index b4b1890783..b69343a884 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -581,6 +581,7 @@ test("dawn_unittests") { "src/tests/unittests/validation/PushConstantsValidationTests.cpp", "src/tests/unittests/validation/QueueSubmitValidationTests.cpp", "src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp", + "src/tests/unittests/validation/RenderPassValidationTests.cpp", "src/tests/unittests/validation/RenderPipelineValidationTests.cpp", "src/tests/unittests/validation/ShaderModuleValidationTests.cpp", "src/tests/unittests/validation/TextureValidationTests.cpp", diff --git a/src/dawn_native/CommandBufferStateTracker.cpp b/src/dawn_native/CommandBufferStateTracker.cpp index 77c14d2440..2697041093 100644 --- a/src/dawn_native/CommandBufferStateTracker.cpp +++ b/src/dawn_native/CommandBufferStateTracker.cpp @@ -91,7 +91,8 @@ namespace dawn_native { bool matches = true; for (uint32_t i : IterateBitSet(mLastPipelineLayout->GetBindGroupLayoutsMask())) { - if (mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout()) { + if (mBindgroups[i] == nullptr || + mLastPipelineLayout->GetBindGroupLayout(i) != mBindgroups[i]->GetLayout()) { matches = false; break; } diff --git a/src/tests/unittests/validation/RenderPassValidationTests.cpp b/src/tests/unittests/validation/RenderPassValidationTests.cpp new file mode 100644 index 0000000000..43cb58b454 --- /dev/null +++ b/src/tests/unittests/validation/RenderPassValidationTests.cpp @@ -0,0 +1,117 @@ +// Copyright 2019 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tests/unittests/validation/ValidationTest.h" + +#include "common/Constants.h" + +#include "utils/ComboRenderPipelineDescriptor.h" +#include "utils/DawnHelpers.h" + +namespace { + +class RenderPassValidationTest : public ValidationTest {}; + +// Test that it is invalid to draw in a render pass with missing bind groups +TEST_F(RenderPassValidationTest, MissingBindGroup) { + dawn::ShaderModule vsModule = + utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, R"( +#version 450 +layout (set = 0, binding = 0) uniform vertexUniformBuffer { + mat2 transform; +}; +void main() { + const vec2 pos[3] = vec2[3](vec2(-1.f, -1.f), vec2(1.f, -1.f), vec2(-1.f, 1.f)); + gl_Position = vec4(transform * pos[gl_VertexIndex], 0.f, 1.f); +})"); + + dawn::ShaderModule fsModule = + utils::CreateShaderModule(device, dawn::ShaderStage::Fragment, R"( +#version 450 +layout (set = 1, binding = 0) uniform fragmentUniformBuffer { + vec4 color; +}; +layout(location = 0) out vec4 fragColor; +void main() { + fragColor = color; +})"); + + dawn::BindGroupLayout bgls[] = { + utils::MakeBindGroupLayout( + device, {{0, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer}}), + utils::MakeBindGroupLayout( + device, {{0, dawn::ShaderStageBit::Fragment, dawn::BindingType::UniformBuffer}})}; + + dawn::PipelineLayoutDescriptor pipelineLayoutDesc; + pipelineLayoutDesc.bindGroupLayoutCount = 2; + pipelineLayoutDesc.bindGroupLayouts = bgls; + + dawn::PipelineLayout pipelineLayout = device.CreatePipelineLayout(&pipelineLayoutDesc); + + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.layout = pipelineLayout; + descriptor.cVertexStage.module = vsModule; + descriptor.cFragmentStage.module = fsModule; + + dawn::RenderPipeline pipeline = device.CreateRenderPipeline(&descriptor); + + float data[4]; + dawn::Buffer buffer = utils::CreateBufferFromData(device, data, 4 * sizeof(float), + dawn::BufferUsageBit::Uniform); + + dawn::BindGroup bg1 = + utils::MakeBindGroup(device, bgls[0], {{0, buffer, 0, 4 * sizeof(float)}}); + dawn::BindGroup bg2 = + utils::MakeBindGroup(device, bgls[1], {{0, buffer, 0, 4 * sizeof(float)}}); + + DummyRenderPass renderPass(device); + { + dawn::CommandEncoder commandEncoder = device.CreateCommandEncoder(); + dawn::RenderPassEncoder pass = commandEncoder.BeginRenderPass(&renderPass); + pass.SetPipeline(pipeline); + pass.SetBindGroup(0, bg1, 0, nullptr); + pass.SetBindGroup(1, bg2, 0, nullptr); + pass.Draw(3, 0, 0, 0); + pass.EndPass(); + commandEncoder.Finish(); + } + { + dawn::CommandEncoder commandEncoder = device.CreateCommandEncoder(); + dawn::RenderPassEncoder pass = commandEncoder.BeginRenderPass(&renderPass); + pass.SetPipeline(pipeline); + pass.Draw(3, 0, 0, 0); + pass.EndPass(); + ASSERT_DEVICE_ERROR(commandEncoder.Finish()); + } + { + dawn::CommandEncoder commandEncoder = device.CreateCommandEncoder(); + dawn::RenderPassEncoder pass = commandEncoder.BeginRenderPass(&renderPass); + pass.SetPipeline(pipeline); + pass.SetBindGroup(1, bg2, 0, nullptr); + pass.Draw(3, 0, 0, 0); + pass.EndPass(); + ASSERT_DEVICE_ERROR(commandEncoder.Finish()); + } + { + dawn::CommandEncoder commandEncoder = device.CreateCommandEncoder(); + dawn::RenderPassEncoder pass = commandEncoder.BeginRenderPass(&renderPass); + pass.SetPipeline(pipeline); + pass.SetBindGroup(0, bg1, 0, nullptr); + pass.Draw(3, 0, 0, 0); + pass.EndPass(); + ASSERT_DEVICE_ERROR(commandEncoder.Finish()); + } +} + +} // anonymous namespace