d3d12/vulkan: Skip descriptor update/creation if resource is destroyed
Validation of resource destroy state does not happen on bind group
creation. It happens on queue submit. This means the Vulkan and D3D12
backends need to gracefully handle BindGroup creation with destroyed
resources by skipping the descriptor creation if the resource has
been destroyed.
Bug: dawn:319
Change-Id: I270afba86d1a961e1e4c39f2419d8c34ff889e46
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38440
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
diff --git a/src/dawn_native/d3d12/BindGroupD3D12.cpp b/src/dawn_native/d3d12/BindGroupD3D12.cpp
index a410807..88a54b8 100644
--- a/src/dawn_native/d3d12/BindGroupD3D12.cpp
+++ b/src/dawn_native/d3d12/BindGroupD3D12.cpp
@@ -57,6 +57,14 @@
case BindingInfoType::Buffer: {
BufferBinding binding = GetBindingAsBufferBinding(bindingIndex);
+ ID3D12Resource* resource = ToBackend(binding.buffer)->GetD3D12Resource();
+ if (resource == nullptr) {
+ // The Buffer was destroyed. Skip creating buffer views since there is no
+ // resource. This bind group won't be used as it is an error to submit a
+ // command buffer that references destroyed resources.
+ continue;
+ }
+
switch (bindingInfo.buffer.type) {
case wgpu::BufferBindingType::Uniform: {
D3D12_CONSTANT_BUFFER_VIEW_DESC desc;
@@ -89,7 +97,7 @@
desc.Buffer.Flags = D3D12_BUFFER_UAV_FLAG_RAW;
d3d12Device->CreateUnorderedAccessView(
- ToBackend(binding.buffer)->GetD3D12Resource(), nullptr, &desc,
+ resource, nullptr, &desc,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@@ -108,7 +116,7 @@
desc.Buffer.StructureByteStride = 0;
desc.Buffer.Flags = D3D12_BUFFER_SRV_FLAG_RAW;
d3d12Device->CreateShaderResourceView(
- ToBackend(binding.buffer)->GetD3D12Resource(), &desc,
+ resource, &desc,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@@ -123,8 +131,17 @@
case BindingInfoType::Texture: {
auto* view = ToBackend(GetBindingAsTextureView(bindingIndex));
auto& srv = view->GetSRVDescriptor();
+
+ ID3D12Resource* resource = ToBackend(view->GetTexture())->GetD3D12Resource();
+ if (resource == nullptr) {
+ // The Texture was destroyed. Skip creating the SRV since there is no
+ // resource. This bind group won't be used as it is an error to submit a
+ // command buffer that references destroyed resources.
+ continue;
+ }
+
d3d12Device->CreateShaderResourceView(
- ToBackend(view->GetTexture())->GetD3D12Resource(), &srv,
+ resource, &srv,
viewAllocation.OffsetFrom(viewSizeIncrement, bindingOffsets[bindingIndex]));
break;
}
@@ -132,13 +149,21 @@
case BindingInfoType::StorageTexture: {
TextureView* view = ToBackend(GetBindingAsTextureView(bindingIndex));
+ ID3D12Resource* resource = ToBackend(view->GetTexture())->GetD3D12Resource();
+ if (resource == nullptr) {
+ // The Texture was destroyed. Skip creating the SRV/UAV since there is no
+ // resource. This bind group won't be used as it is an error to submit a
+ // command buffer that references destroyed resources.
+ continue;
+ }
+
switch (bindingInfo.storageTexture.access) {
case wgpu::StorageTextureAccess::ReadOnly: {
// Readonly storage is implemented as SRV so it can be used at the same
// time as a sampled texture.
auto& srv = view->GetSRVDescriptor();
d3d12Device->CreateShaderResourceView(
- ToBackend(view->GetTexture())->GetD3D12Resource(), &srv,
+ resource, &srv,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@@ -147,7 +172,7 @@
case wgpu::StorageTextureAccess::WriteOnly: {
D3D12_UNORDERED_ACCESS_VIEW_DESC uav = view->GetUAVDescriptor();
d3d12Device->CreateUnorderedAccessView(
- ToBackend(view->GetTexture())->GetD3D12Resource(), nullptr, &uav,
+ resource, nullptr, &uav,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
diff --git a/src/dawn_native/vulkan/BindGroupVk.cpp b/src/dawn_native/vulkan/BindGroupVk.cpp
index e80f6e2..f7dd72f 100644
--- a/src/dawn_native/vulkan/BindGroupVk.cpp
+++ b/src/dawn_native/vulkan/BindGroupVk.cpp
@@ -66,7 +66,15 @@
case BindingInfoType::Buffer: {
BufferBinding binding = GetBindingAsBufferBinding(bindingIndex);
- writeBufferInfo[numWrites].buffer = ToBackend(binding.buffer)->GetHandle();
+ VkBuffer handle = ToBackend(binding.buffer)->GetHandle();
+ if (handle == VK_NULL_HANDLE) {
+ // The Buffer was destroyed. Skip this descriptor write since it would be
+ // a Vulkan Validation Layers error. This bind group won't be used as it
+ // is an error to submit a command buffer that references destroyed
+ // resources.
+ continue;
+ }
+ writeBufferInfo[numWrites].buffer = handle;
writeBufferInfo[numWrites].offset = binding.offset;
writeBufferInfo[numWrites].range = binding.size;
write.pBufferInfo = &writeBufferInfo[numWrites];
diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp
index 78f9929..6e710bf 100644
--- a/src/tests/end2end/BindGroupTests.cpp
+++ b/src/tests/end2end/BindGroupTests.cpp
@@ -1320,6 +1320,63 @@
EXPECT_BUFFER_U32_EQ(1, result, 0);
}
+// This is a regression test for crbug.com/dawn/319 where creating a bind group with a
+// destroyed resource would crash the backend.
+TEST_P(BindGroupTests, CreateWithDestroyedResource) {
+ auto doBufferTest = [&](wgpu::BufferBindingType bindingType, wgpu::BufferUsage usage) {
+ wgpu::BindGroupLayout bgl =
+ utils::MakeBindGroupLayout(device, {{0, wgpu::ShaderStage::Fragment, bindingType}});
+
+ wgpu::BufferDescriptor bufferDesc;
+ bufferDesc.size = sizeof(float);
+ bufferDesc.usage = usage;
+ wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc);
+ buffer.Destroy();
+
+ wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, sizeof(float)}});
+ };
+
+ // Test various usages and binding types since they take different backend code paths.
+ doBufferTest(wgpu::BufferBindingType::Uniform, wgpu::BufferUsage::Uniform);
+ doBufferTest(wgpu::BufferBindingType::Storage, wgpu::BufferUsage::Storage);
+ doBufferTest(wgpu::BufferBindingType::ReadOnlyStorage, wgpu::BufferUsage::Storage);
+
+ // Test a sampled texture.
+ {
+ wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+ device, {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}});
+
+ wgpu::TextureDescriptor textureDesc;
+ textureDesc.usage = wgpu::TextureUsage::Sampled;
+ textureDesc.size = {1, 1, 1};
+ textureDesc.format = wgpu::TextureFormat::BGRA8Unorm;
+
+ wgpu::Texture texture = device.CreateTexture(&textureDesc);
+ wgpu::TextureView textureView = texture.CreateView();
+
+ texture.Destroy();
+ wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, textureView}});
+ }
+
+ // Test a storage texture.
+ {
+ wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+ device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly,
+ wgpu::TextureFormat::R32Uint}});
+
+ wgpu::TextureDescriptor textureDesc;
+ textureDesc.usage = wgpu::TextureUsage::Storage;
+ textureDesc.size = {1, 1, 1};
+ textureDesc.format = wgpu::TextureFormat::R32Uint;
+
+ wgpu::Texture texture = device.CreateTexture(&textureDesc);
+ wgpu::TextureView textureView = texture.CreateView();
+
+ texture.Destroy();
+ wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, textureView}});
+ }
+}
+
DAWN_INSTANTIATE_TEST(BindGroupTests,
D3D12Backend(),
MetalBackend(),
