From e5d94c31a052985075d0987dc852fa543007929e Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Fri, 19 Jun 2020 22:39:53 +0000 Subject: [PATCH] Fix a bug in ComputeTextureCopyBufferSize with empty copySize This patch fixes a bug in buffer-to-texture and texture-to-buffer copies when copySize.height == 0 or copySize.depth == 0. Previously we miss the checks if either copySize.height or copySize.depth is 0 before doing (copySize.height / blockHeight - 1) and (copySize.depth - 1) thus we will get incorrect results because of arithmetic overflows. This patch fixes this bug by adding the missed check and adds the related regression tests in dawn_unittests BUG=dawn:453 TEST=dawn_unittests Change-Id: I970e420c0fa7f0b61c656365bef079c123a59e6a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23520 Commit-Queue: Kai Ninomiya Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya --- src/dawn_native/CommandEncoder.cpp | 9 +++++++++ .../unittests/validation/CopyCommandsValidationTests.cpp | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index b79e3501a2..5b8bc640f8 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -198,14 +198,23 @@ namespace dawn_native { uint32_t rowsPerImage, uint32_t* bufferSize) { ASSERT(rowsPerImage >= copySize.height); + if (copySize.width == 0 || copySize.height == 0 || copySize.depth == 0) { + *bufferSize = 0; + return {}; + } + uint32_t blockByteSize = textureFormat.blockByteSize; uint32_t blockWidth = textureFormat.blockWidth; uint32_t blockHeight = textureFormat.blockHeight; // TODO(cwallez@chromium.org): check for overflows uint32_t slicePitch = bytesPerRow * rowsPerImage / blockWidth; + + ASSERT(copySize.height >= 1); uint32_t sliceSize = bytesPerRow * (copySize.height / blockHeight - 1) + (copySize.width / blockWidth) * blockByteSize; + + ASSERT(copySize.depth >= 1); *bufferSize = (slicePitch * (copySize.depth - 1)) + sliceSize; return {}; diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp index 634dfb6406..53e4bd405f 100644 --- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp +++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp @@ -364,6 +364,12 @@ TEST_F(CopyCommandTest_B2T, Success) { // An empty copy touching the side of the texture TestB2TCopy(utils::Expectation::Success, source, 0, 0, 0, destination, 0, {16, 16, 0}, {0, 0, 1}); + // An empty copy with depth = 1 and bytesPerRow > 0 + TestB2TCopy(utils::Expectation::Success, source, 0, kTextureBytesPerRowAlignment, 0, + destination, 0, {0, 0, 0}, {0, 0, 1}); + // An empty copy with height > 0, depth = 0, bytesPerRow > 0 and rowsPerImage > 0 + TestB2TCopy(utils::Expectation::Success, source, 0, kTextureBytesPerRowAlignment, 16, + destination, 0, {0, 0, 0}, {0, 1, 0}); } }