From ce32a94b3711d6a94f59deb2876f5507a4b674fc Mon Sep 17 00:00:00 2001 From: Rafael Cintron Date: Tue, 2 Jul 2019 07:15:17 +0000 Subject: [PATCH] Remove mResourcePtr member from d3d12::Texture mResourcePtr is not necessary because mResource already contains a pointer to the resource mResourcePtr is initialized in the constructor that takes an ID3D12Resource but mResource is not. This causes memory corruption when the caller releases the resource out from under the object. Texture::DestroyImpl was releasing mResource but leaving mResourcePtr pointing to freed memory Bug: dawn:62 Change-Id: Ib68786fc041e57dbf184c09c9afe4b9d8fa3241f Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/8500 Commit-Queue: Corentin Wallez Reviewed-by: Corentin Wallez --- src/dawn_native/d3d12/TextureD3D12.cpp | 18 +++++++++--------- src/dawn_native/d3d12/TextureD3D12.h | 3 +-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index 215f26fa1b..c428238754 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -143,7 +143,6 @@ namespace dawn_native { namespace d3d12 { ->GetResourceAllocator() ->Allocate(D3D12_HEAP_TYPE_DEFAULT, resourceDescriptor, D3D12_RESOURCE_STATE_COMMON); - mResourcePtr = mResource.Get(); if (device->IsToggleEnabled(Toggle::NonzeroClearResourcesOnCreationForTesting)) { DescriptorHeapAllocator* descriptorHeapAllocator = device->GetDescriptorHeapAllocator(); @@ -154,7 +153,8 @@ namespace dawn_native { namespace d3d12 { descriptorHeapAllocator->AllocateCPUHeap(D3D12_DESCRIPTOR_HEAP_TYPE_DSV, 1); D3D12_CPU_DESCRIPTOR_HANDLE dsvHandle = dsvHeap.GetCPUHandle(0); D3D12_DEPTH_STENCIL_VIEW_DESC dsvDesc = GetDSVDescriptor(0); - device->GetD3D12Device()->CreateDepthStencilView(mResourcePtr, &dsvDesc, dsvHandle); + device->GetD3D12Device()->CreateDepthStencilView(mResource.Get(), &dsvDesc, + dsvHandle); D3D12_CLEAR_FLAGS clearFlags = {}; if (GetFormat().HasDepth()) { @@ -178,7 +178,7 @@ namespace dawn_native { namespace d3d12 { for (int i = 0; i < resourceDescriptor.MipLevels; i++) { D3D12_RENDER_TARGET_VIEW_DESC rtvDesc = GetRTVDescriptor(i, 0, GetArrayLayers()); - device->GetD3D12Device()->CreateRenderTargetView(mResourcePtr, &rtvDesc, + device->GetD3D12Device()->CreateRenderTargetView(mResource.Get(), &rtvDesc, rtvHandle); device->GetPendingCommandList()->ClearRenderTargetView(rtvHandle, clearColor, 0, nullptr); @@ -191,8 +191,7 @@ namespace dawn_native { namespace d3d12 { Texture::Texture(Device* device, const TextureDescriptor* descriptor, ID3D12Resource* nativeTexture) - : TextureBase(device, descriptor, TextureState::OwnedExternal), - mResourcePtr(nativeTexture) { + : TextureBase(device, descriptor, TextureState::OwnedExternal), mResource(nativeTexture) { } Texture::~Texture() { @@ -237,7 +236,7 @@ namespace dawn_native { namespace d3d12 { barrier->Type = D3D12_RESOURCE_BARRIER_TYPE_TRANSITION; barrier->Flags = D3D12_RESOURCE_BARRIER_FLAG_NONE; - barrier->Transition.pResource = mResourcePtr; + barrier->Transition.pResource = mResource.Get(); barrier->Transition.StateBefore = mLastState; barrier->Transition.StateAfter = newState; barrier->Transition.Subresource = D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES; @@ -256,7 +255,7 @@ namespace dawn_native { namespace d3d12 { } ID3D12Resource* Texture::GetD3D12Resource() const { - return mResourcePtr; + return mResource.Get(); } UINT16 Texture::GetDepthOrArraySize() { @@ -345,7 +344,7 @@ namespace dawn_native { namespace d3d12 { descriptorHeapAllocator->AllocateCPUHeap(D3D12_DESCRIPTOR_HEAP_TYPE_DSV, 1); D3D12_CPU_DESCRIPTOR_HANDLE dsvHandle = dsvHeap.GetCPUHandle(0); D3D12_DEPTH_STENCIL_VIEW_DESC dsvDesc = GetDSVDescriptor(baseMipLevel); - device->GetD3D12Device()->CreateDepthStencilView(mResourcePtr, &dsvDesc, dsvHandle); + device->GetD3D12Device()->CreateDepthStencilView(mResource.Get(), &dsvDesc, dsvHandle); D3D12_CLEAR_FLAGS clearFlags = {}; if (GetFormat().HasDepth()) { @@ -367,7 +366,8 @@ namespace dawn_native { namespace d3d12 { for (uint32_t i = baseMipLevel; i < baseMipLevel + levelCount; i++) { D3D12_RENDER_TARGET_VIEW_DESC rtvDesc = GetRTVDescriptor(i, baseArrayLayer, layerCount); - device->GetD3D12Device()->CreateRenderTargetView(mResourcePtr, &rtvDesc, rtvHandle); + device->GetD3D12Device()->CreateRenderTargetView(mResource.Get(), &rtvDesc, + rtvHandle); commandList->ClearRenderTargetView(rtvHandle, clearColor, 0, nullptr); } } diff --git a/src/dawn_native/d3d12/TextureD3D12.h b/src/dawn_native/d3d12/TextureD3D12.h index 7b5eb53e2d..c74a746933 100644 --- a/src/dawn_native/d3d12/TextureD3D12.h +++ b/src/dawn_native/d3d12/TextureD3D12.h @@ -64,8 +64,7 @@ namespace dawn_native { namespace d3d12 { UINT16 GetDepthOrArraySize(); - ComPtr mResource = {}; - ID3D12Resource* mResourcePtr = nullptr; + ComPtr mResource; D3D12_RESOURCE_STATES mLastState = D3D12_RESOURCE_STATES::D3D12_RESOURCE_STATE_COMMON; };