Move zero-size copy skips from the frontend to the backend

Bug: chromium:1217741
Change-Id: Ib4884b5aa80124ed9da48262fcae11cf6d605034
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/53883
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
Austin Eng 2021-06-10 02:40:18 +00:00 committed by Dawn LUCI CQ
parent 5173a0236d
commit f8a0f82fad
6 changed files with 134 additions and 60 deletions

View File

@ -633,16 +633,13 @@ namespace dawn_native {
mTopLevelBuffers.insert(destination); mTopLevelBuffers.insert(destination);
} }
// Skip noop copies. Some backends validation rules disallow them. CopyBufferToBufferCmd* copy =
if (size != 0) { allocator->Allocate<CopyBufferToBufferCmd>(Command::CopyBufferToBuffer);
CopyBufferToBufferCmd* copy = copy->source = source;
allocator->Allocate<CopyBufferToBufferCmd>(Command::CopyBufferToBuffer); copy->sourceOffset = sourceOffset;
copy->source = source; copy->destination = destination;
copy->sourceOffset = sourceOffset; copy->destinationOffset = destinationOffset;
copy->destination = destination; copy->size = size;
copy->destinationOffset = destinationOffset;
copy->size = size;
}
return {}; return {};
}); });
@ -682,23 +679,18 @@ namespace dawn_native {
ApplyDefaultTextureDataLayoutOptions(&srcLayout, blockInfo, *copySize); ApplyDefaultTextureDataLayoutOptions(&srcLayout, blockInfo, *copySize);
// Skip noop copies. CopyBufferToTextureCmd* copy =
if (copySize->width != 0 && copySize->height != 0 && allocator->Allocate<CopyBufferToTextureCmd>(Command::CopyBufferToTexture);
copySize->depthOrArrayLayers != 0) { copy->source.buffer = source->buffer;
// Record the copy command. copy->source.offset = srcLayout.offset;
CopyBufferToTextureCmd* copy = copy->source.bytesPerRow = srcLayout.bytesPerRow;
allocator->Allocate<CopyBufferToTextureCmd>(Command::CopyBufferToTexture); copy->source.rowsPerImage = srcLayout.rowsPerImage;
copy->source.buffer = source->buffer; copy->destination.texture = destination->texture;
copy->source.offset = srcLayout.offset; copy->destination.origin = destination->origin;
copy->source.bytesPerRow = srcLayout.bytesPerRow; copy->destination.mipLevel = destination->mipLevel;
copy->source.rowsPerImage = srcLayout.rowsPerImage; copy->destination.aspect =
copy->destination.texture = destination->texture; ConvertAspect(destination->texture->GetFormat(), destination->aspect);
copy->destination.origin = destination->origin; copy->copySize = *copySize;
copy->destination.mipLevel = destination->mipLevel;
copy->destination.aspect =
ConvertAspect(destination->texture->GetFormat(), destination->aspect);
copy->copySize = *copySize;
}
return {}; return {};
}); });
@ -738,22 +730,17 @@ namespace dawn_native {
ApplyDefaultTextureDataLayoutOptions(&dstLayout, blockInfo, *copySize); ApplyDefaultTextureDataLayoutOptions(&dstLayout, blockInfo, *copySize);
// Skip noop copies. CopyTextureToBufferCmd* copy =
if (copySize->width != 0 && copySize->height != 0 && allocator->Allocate<CopyTextureToBufferCmd>(Command::CopyTextureToBuffer);
copySize->depthOrArrayLayers != 0) { copy->source.texture = source->texture;
// Record the copy command. copy->source.origin = source->origin;
CopyTextureToBufferCmd* copy = copy->source.mipLevel = source->mipLevel;
allocator->Allocate<CopyTextureToBufferCmd>(Command::CopyTextureToBuffer); copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect);
copy->source.texture = source->texture; copy->destination.buffer = destination->buffer;
copy->source.origin = source->origin; copy->destination.offset = dstLayout.offset;
copy->source.mipLevel = source->mipLevel; copy->destination.bytesPerRow = dstLayout.bytesPerRow;
copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect); copy->destination.rowsPerImage = dstLayout.rowsPerImage;
copy->destination.buffer = destination->buffer; copy->copySize = *copySize;
copy->destination.offset = dstLayout.offset;
copy->destination.bytesPerRow = dstLayout.bytesPerRow;
copy->destination.rowsPerImage = dstLayout.rowsPerImage;
copy->copySize = *copySize;
}
return {}; return {};
}); });
@ -783,22 +770,18 @@ namespace dawn_native {
mTopLevelTextures.insert(destination->texture); mTopLevelTextures.insert(destination->texture);
} }
// Skip noop copies. CopyTextureToTextureCmd* copy =
if (copySize->width != 0 && copySize->height != 0 && allocator->Allocate<CopyTextureToTextureCmd>(Command::CopyTextureToTexture);
copySize->depthOrArrayLayers != 0) { copy->source.texture = source->texture;
CopyTextureToTextureCmd* copy = copy->source.origin = source->origin;
allocator->Allocate<CopyTextureToTextureCmd>(Command::CopyTextureToTexture); copy->source.mipLevel = source->mipLevel;
copy->source.texture = source->texture; copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect);
copy->source.origin = source->origin; copy->destination.texture = destination->texture;
copy->source.mipLevel = source->mipLevel; copy->destination.origin = destination->origin;
copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect); copy->destination.mipLevel = destination->mipLevel;
copy->destination.texture = destination->texture; copy->destination.aspect =
copy->destination.origin = destination->origin; ConvertAspect(destination->texture->GetFormat(), destination->aspect);
copy->destination.mipLevel = destination->mipLevel; copy->copySize = *copySize;
copy->destination.aspect =
ConvertAspect(destination->texture->GetFormat(), destination->aspect);
copy->copySize = *copySize;
}
return {}; return {};
}); });

View File

@ -626,6 +626,10 @@ namespace dawn_native { namespace d3d12 {
case Command::CopyBufferToBuffer: { case Command::CopyBufferToBuffer: {
CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>(); CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>();
if (copy->size == 0) {
// Skip no-op copies.
break;
}
Buffer* srcBuffer = ToBackend(copy->source.Get()); Buffer* srcBuffer = ToBackend(copy->source.Get());
Buffer* dstBuffer = ToBackend(copy->destination.Get()); Buffer* dstBuffer = ToBackend(copy->destination.Get());
@ -646,6 +650,11 @@ namespace dawn_native { namespace d3d12 {
case Command::CopyBufferToTexture: { case Command::CopyBufferToTexture: {
CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>(); CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
Buffer* buffer = ToBackend(copy->source.buffer.Get()); Buffer* buffer = ToBackend(copy->source.buffer.Get());
Texture* texture = ToBackend(copy->destination.texture.Get()); Texture* texture = ToBackend(copy->destination.texture.Get());
@ -676,6 +685,11 @@ namespace dawn_native { namespace d3d12 {
case Command::CopyTextureToBuffer: { case Command::CopyTextureToBuffer: {
CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>(); CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
Texture* texture = ToBackend(copy->source.texture.Get()); Texture* texture = ToBackend(copy->source.texture.Get());
Buffer* buffer = ToBackend(copy->destination.buffer.Get()); Buffer* buffer = ToBackend(copy->destination.buffer.Get());
@ -700,7 +714,11 @@ namespace dawn_native { namespace d3d12 {
case Command::CopyTextureToTexture: { case Command::CopyTextureToTexture: {
CopyTextureToTextureCmd* copy = CopyTextureToTextureCmd* copy =
mCommands.NextCommand<CopyTextureToTextureCmd>(); mCommands.NextCommand<CopyTextureToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
Texture* source = ToBackend(copy->source.texture.Get()); Texture* source = ToBackend(copy->source.texture.Get());
Texture* destination = ToBackend(copy->destination.texture.Get()); Texture* destination = ToBackend(copy->destination.texture.Get());

View File

@ -704,6 +704,10 @@ namespace dawn_native { namespace metal {
case Command::CopyBufferToBuffer: { case Command::CopyBufferToBuffer: {
CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>(); CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>();
if (copy->size == 0) {
// Skip no-op copies.
break;
}
ToBackend(copy->source)->EnsureDataInitialized(commandContext); ToBackend(copy->source)->EnsureDataInitialized(commandContext);
ToBackend(copy->destination) ToBackend(copy->destination)
@ -721,6 +725,11 @@ namespace dawn_native { namespace metal {
case Command::CopyBufferToTexture: { case Command::CopyBufferToTexture: {
CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>(); CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
auto& copySize = copy->copySize; auto& copySize = copy->copySize;
@ -739,6 +748,11 @@ namespace dawn_native { namespace metal {
case Command::CopyTextureToBuffer: { case Command::CopyTextureToBuffer: {
CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>(); CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
auto& copySize = copy->copySize; auto& copySize = copy->copySize;
@ -814,6 +828,11 @@ namespace dawn_native { namespace metal {
case Command::CopyTextureToTexture: { case Command::CopyTextureToTexture: {
CopyTextureToTextureCmd* copy = CopyTextureToTextureCmd* copy =
mCommands.NextCommand<CopyTextureToTextureCmd>(); mCommands.NextCommand<CopyTextureToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
Texture* srcTexture = ToBackend(copy->source.texture.Get()); Texture* srcTexture = ToBackend(copy->source.texture.Get());
Texture* dstTexture = ToBackend(copy->destination.texture.Get()); Texture* dstTexture = ToBackend(copy->destination.texture.Get());

View File

@ -612,6 +612,10 @@ namespace dawn_native { namespace opengl {
case Command::CopyBufferToBuffer: { case Command::CopyBufferToBuffer: {
CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>(); CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>();
if (copy->size == 0) {
// Skip no-op copies.
break;
}
ToBackend(copy->source)->EnsureDataInitialized(); ToBackend(copy->source)->EnsureDataInitialized();
ToBackend(copy->destination) ToBackend(copy->destination)
@ -630,6 +634,11 @@ namespace dawn_native { namespace opengl {
case Command::CopyBufferToTexture: { case Command::CopyBufferToTexture: {
CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>(); CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
Buffer* buffer = ToBackend(src.buffer.Get()); Buffer* buffer = ToBackend(src.buffer.Get());
@ -664,6 +673,11 @@ namespace dawn_native { namespace opengl {
case Command::CopyTextureToBuffer: { case Command::CopyTextureToBuffer: {
CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>(); CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
auto& copySize = copy->copySize; auto& copySize = copy->copySize;
@ -771,6 +785,11 @@ namespace dawn_native { namespace opengl {
case Command::CopyTextureToTexture: { case Command::CopyTextureToTexture: {
CopyTextureToTextureCmd* copy = CopyTextureToTextureCmd* copy =
mCommands.NextCommand<CopyTextureToTextureCmd>(); mCommands.NextCommand<CopyTextureToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;

View File

@ -516,6 +516,10 @@ namespace dawn_native { namespace vulkan {
switch (type) { switch (type) {
case Command::CopyBufferToBuffer: { case Command::CopyBufferToBuffer: {
CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>(); CopyBufferToBufferCmd* copy = mCommands.NextCommand<CopyBufferToBufferCmd>();
if (copy->size == 0) {
// Skip no-op copies.
break;
}
Buffer* srcBuffer = ToBackend(copy->source.Get()); Buffer* srcBuffer = ToBackend(copy->source.Get());
Buffer* dstBuffer = ToBackend(copy->destination.Get()); Buffer* dstBuffer = ToBackend(copy->destination.Get());
@ -540,6 +544,11 @@ namespace dawn_native { namespace vulkan {
case Command::CopyBufferToTexture: { case Command::CopyBufferToTexture: {
CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>(); CopyBufferToTextureCmd* copy = mCommands.NextCommand<CopyBufferToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
@ -578,6 +587,11 @@ namespace dawn_native { namespace vulkan {
case Command::CopyTextureToBuffer: { case Command::CopyTextureToBuffer: {
CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>(); CopyTextureToBufferCmd* copy = mCommands.NextCommand<CopyTextureToBufferCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
auto& src = copy->source; auto& src = copy->source;
auto& dst = copy->destination; auto& dst = copy->destination;
@ -610,6 +624,11 @@ namespace dawn_native { namespace vulkan {
case Command::CopyTextureToTexture: { case Command::CopyTextureToTexture: {
CopyTextureToTextureCmd* copy = CopyTextureToTextureCmd* copy =
mCommands.NextCommand<CopyTextureToTextureCmd>(); mCommands.NextCommand<CopyTextureToTextureCmd>();
if (copy->copySize.width == 0 || copy->copySize.height == 0 ||
copy->copySize.depthOrArrayLayers == 0) {
// Skip no-op copies.
continue;
}
TextureCopy& src = copy->source; TextureCopy& src = copy->source;
TextureCopy& dst = copy->destination; TextureCopy& dst = copy->destination;
SubresourceRange srcRange = GetSubresourcesAffectedByCopy(src, copy->copySize); SubresourceRange srcRange = GetSubresourcesAffectedByCopy(src, copy->copySize);

View File

@ -226,6 +226,22 @@ TEST_F(CopyCommandTest_B2B, Success) {
} }
} }
// Test a successful B2B copy where the last external reference is dropped.
// This is a regression test for crbug.com/1217741 where submitting a command
// buffer with dropped resources when the copy size is 0 was a use-after-free.
TEST_F(CopyCommandTest_B2B, DroppedBuffer) {
wgpu::Buffer source = CreateBuffer(16, wgpu::BufferUsage::CopySrc);
wgpu::Buffer destination = CreateBuffer(16, wgpu::BufferUsage::CopyDst);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.CopyBufferToBuffer(source, 0, destination, 0, 0);
wgpu::CommandBuffer commandBuffer = encoder.Finish();
source = nullptr;
destination = nullptr;
device.GetQueue().Submit(1, &commandBuffer);
}
// Test B2B copies with OOB // Test B2B copies with OOB
TEST_F(CopyCommandTest_B2B, OutOfBounds) { TEST_F(CopyCommandTest_B2B, OutOfBounds) {
wgpu::Buffer source = CreateBuffer(16, wgpu::BufferUsage::CopySrc); wgpu::Buffer source = CreateBuffer(16, wgpu::BufferUsage::CopySrc);