Fix crashes when using error textures in copy commands
This patch fixes a Dawn crash issue when using textures in error state
in the copy commands of CommandEncoder.
In Dawn's copy commands (copyBufferToTexture and CopyTextureToBuffer),
we should check if the texture is in error state or not, or the assert
ASSERT(!IsError()) in texture->GetFormat() will fail and a crash will
occur.
In current Dawn code the validations on the buffer and texture objects
in the copy commands are executed in CommandEncoder::Finish(), which
is too late for textures according to the previous investigation. This
patch moves all these validations to the call of copy commands. The
checks on buffers are also moved away to keep the consistency of the
ones on textures.
BUG=chromium:937628
TEST=dawn_unittests
Change-Id: I0bc44e76262fba5927df20c6a7551b107bad5ca1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/5240
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index 22f7fb8..ef186a1 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -555,13 +555,11 @@
return;
}
- if (source == nullptr) {
- HandleError("Source cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(source))) {
return;
}
- if (destination == nullptr) {
- HandleError("Destination cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(destination))) {
return;
}
@@ -582,13 +580,11 @@
return;
}
- if (source->buffer == nullptr) {
- HandleError("Buffer cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(source->buffer))) {
return;
}
- if (destination->texture == nullptr) {
- HandleError("Texture cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(destination->texture))) {
return;
}
@@ -621,13 +617,11 @@
return;
}
- if (source->texture == nullptr) {
- HandleError("Texture cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(source->texture))) {
return;
}
- if (destination->buffer == nullptr) {
- HandleError("Buffer cannot be null");
+ if (ConsumedError(GetDevice()->ValidateObject(destination->buffer))) {
return;
}
@@ -721,8 +715,6 @@
case Command::CopyBufferToBuffer: {
CopyBufferToBufferCmd* copy = mIterator.NextCommand<CopyBufferToBufferCmd>();
- DAWN_TRY(GetDevice()->ValidateObject(copy->source.buffer.Get()));
- DAWN_TRY(GetDevice()->ValidateObject(copy->destination.buffer.Get()));
DAWN_TRY(ValidateCopySizeFitsInBuffer(copy->source, copy->size));
DAWN_TRY(ValidateCopySizeFitsInBuffer(copy->destination, copy->size));
DAWN_TRY(ValidateB2BCopySizeAlignment(copy->size, copy->source.offset,
@@ -740,9 +732,6 @@
case Command::CopyBufferToTexture: {
CopyBufferToTextureCmd* copy = mIterator.NextCommand<CopyBufferToTextureCmd>();
- DAWN_TRY(GetDevice()->ValidateObject(copy->source.buffer.Get()));
- DAWN_TRY(GetDevice()->ValidateObject(copy->destination.texture.Get()));
-
DAWN_TRY(
ValidateTextureSampleCountInCopyCommands(copy->destination.texture.Get()));
@@ -771,9 +760,6 @@
case Command::CopyTextureToBuffer: {
CopyTextureToBufferCmd* copy = mIterator.NextCommand<CopyTextureToBufferCmd>();
- DAWN_TRY(GetDevice()->ValidateObject(copy->source.texture.Get()));
- DAWN_TRY(GetDevice()->ValidateObject(copy->destination.buffer.Get()));
-
DAWN_TRY(ValidateTextureSampleCountInCopyCommands(copy->source.texture.Get()));
uint32_t bufferCopySize = 0;
diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
index a11731b..dbb6836 100644
--- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
+++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
@@ -200,6 +200,29 @@
}
}
+// Test B2B copies with buffers in error state cause errors.
+TEST_F(CopyCommandTest_B2B, BuffersInErrorState) {
+ dawn::BufferDescriptor errorBufferDescriptor;
+ errorBufferDescriptor.size = 4;
+ errorBufferDescriptor.usage = dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::TransferSrc;
+ ASSERT_DEVICE_ERROR(dawn::Buffer errorBuffer = device.CreateBuffer(&errorBufferDescriptor));
+
+ constexpr uint32_t bufferSize = 4;
+ dawn::Buffer validBuffer = CreateBuffer(bufferSize, dawn::BufferUsageBit::TransferSrc);
+
+ {
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToBuffer(errorBuffer, 0, validBuffer, 0, 4);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ {
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToBuffer(validBuffer, 0, errorBuffer, 0, 4);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+}
+
class CopyCommandTest_B2T : public CopyCommandTest {
};
@@ -420,6 +443,47 @@
{2, 2, 1});
}
+// Test B2T copies with buffer or texture in error state causes errors.
+TEST_F(CopyCommandTest_B2T, BufferOrTextureInErrorState) {
+ dawn::BufferDescriptor errorBufferDescriptor;
+ errorBufferDescriptor.size = 4;
+ errorBufferDescriptor.usage = dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::TransferSrc;
+ ASSERT_DEVICE_ERROR(dawn::Buffer errorBuffer = device.CreateBuffer(&errorBufferDescriptor));
+
+ dawn::TextureDescriptor errorTextureDescriptor;
+ errorTextureDescriptor.arrayLayerCount = 0;
+ ASSERT_DEVICE_ERROR(dawn::Texture errorTexture = device.CreateTexture(&errorTextureDescriptor));
+
+ dawn::BufferCopyView errorBufferCopyView = utils::CreateBufferCopyView(errorBuffer, 0, 0, 0);
+ dawn::TextureCopyView errorTextureCopyView =
+ utils::CreateTextureCopyView(errorTexture, 0, 0, {1, 1, 1});
+
+ dawn::Extent3D extent3D = {1, 1, 1};
+
+ {
+ dawn::Texture destination =
+ Create2DTexture(16, 16, 1, 1, dawn::TextureFormat::R8G8B8A8Unorm,
+ dawn::TextureUsageBit::TransferDst);
+ dawn::TextureCopyView textureCopyView =
+ utils::CreateTextureCopyView(destination, 0, 0, {1, 1, 1});
+
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToTexture(&errorBufferCopyView, &textureCopyView, &extent3D);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ {
+ uint32_t bufferSize = BufferSizeForTextureCopy(4, 4, 1);
+ dawn::Buffer source = CreateBuffer(bufferSize, dawn::BufferUsageBit::TransferSrc);
+
+ dawn::BufferCopyView bufferCopyView = utils::CreateBufferCopyView(source, 0, 0, 0);
+
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToTexture(&bufferCopyView, &errorTextureCopyView, &extent3D);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+}
+
class CopyCommandTest_T2B : public CopyCommandTest {
};
@@ -633,3 +697,44 @@
TestT2BCopy(utils::Expectation::Failure, source, 0, 0, {0, 0, 0}, destination, 0, 256, 0,
{2, 2, 1});
}
+
+// Test T2B copies with buffer or texture in error state cause errors.
+TEST_F(CopyCommandTest_T2B, BufferOrTextureInErrorState) {
+ dawn::BufferDescriptor errorBufferDescriptor;
+ errorBufferDescriptor.size = 4;
+ errorBufferDescriptor.usage = dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::TransferSrc;
+ ASSERT_DEVICE_ERROR(dawn::Buffer errorBuffer = device.CreateBuffer(&errorBufferDescriptor));
+
+ dawn::TextureDescriptor errorTextureDescriptor;
+ errorTextureDescriptor.arrayLayerCount = 0;
+ ASSERT_DEVICE_ERROR(dawn::Texture errorTexture = device.CreateTexture(&errorTextureDescriptor));
+
+ dawn::BufferCopyView errorBufferCopyView = utils::CreateBufferCopyView(errorBuffer, 0, 0, 0);
+ dawn::TextureCopyView errorTextureCopyView =
+ utils::CreateTextureCopyView(errorTexture, 0, 0, {1, 1, 1});
+
+ dawn::Extent3D extent3D = {1, 1, 1};
+
+ {
+ uint32_t bufferSize = BufferSizeForTextureCopy(4, 4, 1);
+ dawn::Buffer source = CreateBuffer(bufferSize, dawn::BufferUsageBit::TransferSrc);
+
+ dawn::BufferCopyView bufferCopyView = utils::CreateBufferCopyView(source, 0, 0, 0);
+
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyTextureToBuffer(&errorTextureCopyView, &bufferCopyView, &extent3D);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ {
+ dawn::Texture destination =
+ Create2DTexture(16, 16, 1, 1, dawn::TextureFormat::R8G8B8A8Unorm,
+ dawn::TextureUsageBit::TransferDst);
+ dawn::TextureCopyView textureCopyView =
+ utils::CreateTextureCopyView(destination, 0, 0, {1, 1, 1});
+
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyTextureToBuffer(&textureCopyView, &errorBufferCopyView, &extent3D);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+}
