Buffer: Validate the offset is aligned to 8

This is to match the upstream WebGPU spec.

Bug: dawn:445

Change-Id: I1a511ed9a2a04c7b95368ce724d69c128158f097
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/29360
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2020-10-07 17:19:53 +00:00 committed by Commit Bot service account
parent bf7fc62175
commit 4171134daf
5 changed files with 63 additions and 49 deletions

View File

@ -403,8 +403,8 @@ namespace dawn_native {
*status = WGPUBufferMapAsyncStatus_Error; *status = WGPUBufferMapAsyncStatus_Error;
DAWN_TRY(GetDevice()->ValidateObject(this)); DAWN_TRY(GetDevice()->ValidateObject(this));
if (offset % 4 != 0) { if (offset % 8 != 0) {
return DAWN_VALIDATION_ERROR("offset must be a multiple of 4"); return DAWN_VALIDATION_ERROR("offset must be a multiple of 8");
} }
if (size % 4 != 0) { if (size % 4 != 0) {
@ -448,6 +448,10 @@ namespace dawn_native {
} }
bool BufferBase::CanGetMappedRange(bool writable, size_t offset, size_t size) const { bool BufferBase::CanGetMappedRange(bool writable, size_t offset, size_t size) const {
if (offset % 8 != 0 || size % 4 != 0) {
return false;
}
if (size > mMapSize || offset < mMapOffset) { if (size > mMapSize || offset < mMapOffset) {
return false; return false;
} }

View File

@ -366,6 +366,10 @@ namespace dawn_wire { namespace client {
} }
bool Buffer::CheckGetMappedRangeOffsetSize(size_t offset, size_t size) const { bool Buffer::CheckGetMappedRangeOffsetSize(size_t offset, size_t size) const {
if (offset % 8 != 0 || size % 4 != 0) {
return false;
}
if (size > mMapSize || offset < mMapOffset) { if (size > mMapSize || offset < mMapOffset) {
return false; return false;
} }

View File

@ -84,13 +84,13 @@ TEST_P(BufferMappingTests, MapRead_ZeroSized) {
// Test map-reading with a non-zero offset // Test map-reading with a non-zero offset
TEST_P(BufferMappingTests, MapRead_NonZeroOffset) { TEST_P(BufferMappingTests, MapRead_NonZeroOffset) {
wgpu::Buffer buffer = CreateMapReadBuffer(8); wgpu::Buffer buffer = CreateMapReadBuffer(12);
uint32_t myData[2] = {0x01020304, 0x05060708}; uint32_t myData[3] = {0x01020304, 0x05060708, 0x090A0B0C};
queue.WriteBuffer(buffer, 0, &myData, sizeof(myData)); queue.WriteBuffer(buffer, 0, &myData, sizeof(myData));
MapAsyncAndWait(buffer, wgpu::MapMode::Read, 4, 4); MapAsyncAndWait(buffer, wgpu::MapMode::Read, 8, 4);
ASSERT_EQ(myData[1], *static_cast<const uint32_t*>(buffer.GetConstMappedRange(4))); ASSERT_EQ(myData[2], *static_cast<const uint32_t*>(buffer.GetConstMappedRange(8)));
buffer.Unmap(); buffer.Unmap();
} }
@ -133,23 +133,27 @@ TEST_P(BufferMappingTests, MapRead_Large) {
0, memcmp(buffer.GetConstMappedRange(8, kByteSize - 8), myData.data() + 2, kByteSize - 8)); 0, memcmp(buffer.GetConstMappedRange(8, kByteSize - 8), myData.data() + 2, kByteSize - 8));
buffer.Unmap(); buffer.Unmap();
MapAsyncAndWait(buffer, wgpu::MapMode::Read, 12, kByteSize - 12); MapAsyncAndWait(buffer, wgpu::MapMode::Read, 16, kByteSize - 16);
// Size is too big.
EXPECT_EQ(nullptr, buffer.GetConstMappedRange(16, kByteSize - 12)); EXPECT_EQ(nullptr, buffer.GetConstMappedRange(16, kByteSize - 12));
// Offset defaults to 0 which is less than 16
EXPECT_EQ(nullptr, buffer.GetConstMappedRange()); EXPECT_EQ(nullptr, buffer.GetConstMappedRange());
// Offset less than 8 is less than 16
EXPECT_EQ(nullptr, buffer.GetConstMappedRange(8)); EXPECT_EQ(nullptr, buffer.GetConstMappedRange(8));
EXPECT_EQ(0, memcmp(buffer.GetConstMappedRange(12), myData.data() + 3, kByteSize - 12));
EXPECT_EQ(0, memcmp(buffer.GetConstMappedRange(20), myData.data() + 5, kByteSize - 20)); // Test a couple values.
EXPECT_EQ(0, memcmp(buffer.GetConstMappedRange(20, kByteSize - 24), myData.data() + 5, EXPECT_EQ(0, memcmp(buffer.GetConstMappedRange(16), myData.data() + 4, kByteSize - 16));
kByteSize - 44)); EXPECT_EQ(0, memcmp(buffer.GetConstMappedRange(24), myData.data() + 6, kByteSize - 24));
buffer.Unmap(); buffer.Unmap();
} }
// Test that GetConstMappedRange works inside map-read callback // Test that GetConstMappedRange works inside map-read callback
TEST_P(BufferMappingTests, MapRead_InCallback) { TEST_P(BufferMappingTests, MapRead_InCallback) {
constexpr size_t kBufferSize = 8; constexpr size_t kBufferSize = 12;
wgpu::Buffer buffer = CreateMapReadBuffer(kBufferSize); wgpu::Buffer buffer = CreateMapReadBuffer(kBufferSize);
uint32_t myData[2] = {0x01020304, 0x05060708}; uint32_t myData[3] = {0x01020304, 0x05060708, 0x090A0B0C};
constexpr size_t kSize = sizeof(myData); constexpr size_t kSize = sizeof(myData);
queue.WriteBuffer(buffer, 0, &myData, kSize); queue.WriteBuffer(buffer, 0, &myData, kSize);
@ -170,8 +174,8 @@ TEST_P(BufferMappingTests, MapRead_InCallback) {
CheckMapping(user->buffer.GetConstMappedRange(), user->expected, kSize); CheckMapping(user->buffer.GetConstMappedRange(), user->expected, kSize);
CheckMapping(user->buffer.GetConstMappedRange(0, kSize), user->expected, kSize); CheckMapping(user->buffer.GetConstMappedRange(0, kSize), user->expected, kSize);
CheckMapping(user->buffer.GetConstMappedRange(4, 4), CheckMapping(user->buffer.GetConstMappedRange(8, 4),
static_cast<const uint32_t*>(user->expected) + 1, sizeof(uint32_t)); static_cast<const uint32_t*>(user->expected) + 2, sizeof(uint32_t));
user->buffer.Unmap(); user->buffer.Unmap();
} }
@ -224,14 +228,14 @@ TEST_P(BufferMappingTests, MapWrite_ZeroSized) {
// Test map-writing with a non-zero offset. // Test map-writing with a non-zero offset.
TEST_P(BufferMappingTests, MapWrite_NonZeroOffset) { TEST_P(BufferMappingTests, MapWrite_NonZeroOffset) {
wgpu::Buffer buffer = CreateMapWriteBuffer(8); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
uint32_t myData = 2934875; uint32_t myData = 2934875;
MapAsyncAndWait(buffer, wgpu::MapMode::Write, 4, 4); MapAsyncAndWait(buffer, wgpu::MapMode::Write, 8, 4);
memcpy(buffer.GetMappedRange(4), &myData, sizeof(myData)); memcpy(buffer.GetMappedRange(8), &myData, sizeof(myData));
buffer.Unmap(); buffer.Unmap();
EXPECT_BUFFER_U32_EQ(myData, buffer, 4); EXPECT_BUFFER_U32_EQ(myData, buffer, 8);
} }
// Map, write and unmap twice. Test that both of these two iterations work. // Map, write and unmap twice. Test that both of these two iterations work.
@ -264,15 +268,14 @@ TEST_P(BufferMappingTests, MapWrite_Large) {
myData.push_back(i); myData.push_back(i);
} }
MapAsyncAndWait(buffer, wgpu::MapMode::Write, 12, kByteSize - 20); MapAsyncAndWait(buffer, wgpu::MapMode::Write, 16, kByteSize - 20);
EXPECT_EQ(nullptr, buffer.GetMappedRange()); EXPECT_EQ(nullptr, buffer.GetMappedRange());
EXPECT_EQ(nullptr, buffer.GetMappedRange(0)); EXPECT_EQ(nullptr, buffer.GetMappedRange(0));
EXPECT_EQ(nullptr, buffer.GetMappedRange(8)); EXPECT_EQ(nullptr, buffer.GetMappedRange(8));
EXPECT_EQ(nullptr, buffer.GetMappedRange(16, kByteSize - 8)); EXPECT_EQ(nullptr, buffer.GetMappedRange(16, kByteSize - 8));
EXPECT_EQ(nullptr, buffer.GetMappedRange(16, kByteSize - 20)); memcpy(buffer.GetMappedRange(16), myData.data(), kByteSize - 20);
memcpy(buffer.GetMappedRange(12), myData.data(), kByteSize - 20);
buffer.Unmap(); buffer.Unmap();
EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 12, kDataSize - 5); EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 16, kDataSize - 5);
} }
// Stress test mapping many buffers. // Stress test mapping many buffers.
@ -329,7 +332,7 @@ TEST_P(BufferMappingTests, OffsetNotUpdatedOnError) {
// Map the buffer but do not wait on the result yet. // Map the buffer but do not wait on the result yet.
bool done = false; bool done = false;
buffer.MapAsync( buffer.MapAsync(
wgpu::MapMode::Read, 4, 4, wgpu::MapMode::Read, 8, 4,
[](WGPUBufferMapAsyncStatus status, void* userdata) { [](WGPUBufferMapAsyncStatus status, void* userdata) {
ASSERT_EQ(WGPUBufferMapAsyncStatus_Success, status); ASSERT_EQ(WGPUBufferMapAsyncStatus_Success, status);
*static_cast<bool*>(userdata) = true; *static_cast<bool*>(userdata) = true;
@ -338,14 +341,14 @@ TEST_P(BufferMappingTests, OffsetNotUpdatedOnError) {
// Call MapAsync another time, it is an error because the buffer is already being mapped so // Call MapAsync another time, it is an error because the buffer is already being mapped so
// mMapOffset is not updated. // mMapOffset is not updated.
ASSERT_DEVICE_ERROR(buffer.MapAsync(wgpu::MapMode::Read, 8, 4, nullptr, nullptr)); ASSERT_DEVICE_ERROR(buffer.MapAsync(wgpu::MapMode::Read, 0, 4, nullptr, nullptr));
while (!done) { while (!done) {
WaitABit(); WaitABit();
} }
// mMapOffset has not been updated so it should still be 4, which is data[1] // mMapOffset has not been updated so it should still be 4, which is data[1]
ASSERT_EQ(0, memcmp(buffer.GetConstMappedRange(4), &data[1], sizeof(uint32_t))); ASSERT_EQ(0, memcmp(buffer.GetConstMappedRange(8), &data[2], sizeof(uint32_t)));
} }
// Test that Get(Const)MappedRange work inside map-write callback. // Test that Get(Const)MappedRange work inside map-write callback.

View File

@ -707,7 +707,7 @@ TEST_P(BufferZeroInitTest, MapAsync_Read) {
{ {
wgpu::Buffer buffer = CreateBuffer(kBufferSize, kBufferUsage); wgpu::Buffer buffer = CreateBuffer(kBufferSize, kBufferUsage);
constexpr uint64_t kOffset = 4u; constexpr uint64_t kOffset = 8u;
constexpr uint64_t kSize = 8u; constexpr uint64_t kSize = 8u;
EXPECT_LAZY_CLEAR(1u, MapAsyncAndWait(buffer, kMapMode, kOffset, kSize)); EXPECT_LAZY_CLEAR(1u, MapAsyncAndWait(buffer, kMapMode, kOffset, kSize));
@ -753,7 +753,7 @@ TEST_P(BufferZeroInitTest, MapAsync_Write) {
{ {
wgpu::Buffer buffer = CreateBuffer(kBufferSize, kBufferUsage); wgpu::Buffer buffer = CreateBuffer(kBufferSize, kBufferUsage);
constexpr uint64_t kOffset = 4u; constexpr uint64_t kOffset = 8u;
constexpr uint64_t kSize = 8u; constexpr uint64_t kSize = 8u;
EXPECT_LAZY_CLEAR(1u, MapAsyncAndWait(buffer, kMapMode, kOffset, kSize)); EXPECT_LAZY_CLEAR(1u, MapAsyncAndWait(buffer, kMapMode, kOffset, kSize));
buffer.Unmap(); buffer.Unmap();

View File

@ -171,24 +171,24 @@ TEST_F(BufferValidationTest, MapAsync_ErrorBuffer) {
// Test map async with an invalid offset and size alignment. // Test map async with an invalid offset and size alignment.
TEST_F(BufferValidationTest, MapAsync_OffsetSizeAlignment) { TEST_F(BufferValidationTest, MapAsync_OffsetSizeAlignment) {
// Control case, both aligned to 4 is ok. // Control case, offset aligned to 8 and size to 4 is valid
{ {
wgpu::Buffer buffer = CreateMapReadBuffer(8); wgpu::Buffer buffer = CreateMapReadBuffer(12);
buffer.MapAsync(wgpu::MapMode::Read, 4, 4, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Read, 8, 4, nullptr, nullptr);
} }
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(8); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
buffer.MapAsync(wgpu::MapMode::Write, 4, 4, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 8, 4, nullptr, nullptr);
} }
// Error case, offset aligned to 2 is an error. // Error case, offset aligned to 4 is an error.
{ {
wgpu::Buffer buffer = CreateMapReadBuffer(8); wgpu::Buffer buffer = CreateMapReadBuffer(12);
AssertMapAsyncError(buffer, wgpu::MapMode::Read, 2, 4); AssertMapAsyncError(buffer, wgpu::MapMode::Read, 4, 4);
} }
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(8); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
AssertMapAsyncError(buffer, wgpu::MapMode::Write, 2, 4); AssertMapAsyncError(buffer, wgpu::MapMode::Write, 4, 4);
} }
// Error case, size aligned to 2 is an error. // Error case, size aligned to 2 is an error.
@ -212,8 +212,8 @@ TEST_F(BufferValidationTest, MapAsync_OffsetSizeOOB) {
// Valid case: range in the middle of the buffer is ok. // Valid case: range in the middle of the buffer is ok.
{ {
wgpu::Buffer buffer = CreateMapReadBuffer(12); wgpu::Buffer buffer = CreateMapReadBuffer(16);
buffer.MapAsync(wgpu::MapMode::Read, 4, 4, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Read, 8, 4, nullptr, nullptr);
} }
// Valid case: empty range at the end of the buffer is ok. // Valid case: empty range at the end of the buffer is ok.
@ -224,8 +224,8 @@ TEST_F(BufferValidationTest, MapAsync_OffsetSizeOOB) {
// Error case, offset is larger than the buffer size (even if size is 0). // Error case, offset is larger than the buffer size (even if size is 0).
{ {
wgpu::Buffer buffer = CreateMapReadBuffer(8); wgpu::Buffer buffer = CreateMapReadBuffer(12);
AssertMapAsyncError(buffer, wgpu::MapMode::Read, 12, 0); AssertMapAsyncError(buffer, wgpu::MapMode::Read, 16, 0);
} }
// Error case, offset + size is larger than the buffer // Error case, offset + size is larger than the buffer
@ -237,8 +237,8 @@ TEST_F(BufferValidationTest, MapAsync_OffsetSizeOOB) {
// Error case, offset + size is larger than the buffer, overflow case. // Error case, offset + size is larger than the buffer, overflow case.
{ {
wgpu::Buffer buffer = CreateMapReadBuffer(12); wgpu::Buffer buffer = CreateMapReadBuffer(12);
AssertMapAsyncError(buffer, wgpu::MapMode::Read, 4, AssertMapAsyncError(buffer, wgpu::MapMode::Read, 8,
std::numeric_limits<size_t>::max() & ~size_t(3)); std::numeric_limits<size_t>::max() & ~size_t(7));
} }
} }
@ -831,9 +831,9 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
// Valid case: range in the middle is ok. // Valid case: range in the middle is ok.
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(12); wgpu::Buffer buffer = CreateMapWriteBuffer(16);
buffer.MapAsync(wgpu::MapMode::Write, 0, 12, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 0, 16, nullptr, nullptr);
EXPECT_NE(buffer.GetMappedRange(4, 4), nullptr); EXPECT_NE(buffer.GetMappedRange(8, 4), nullptr);
} }
// Error case: offset is larger than the mapped range (even with size = 0) // Error case: offset is larger than the mapped range (even with size = 0)
@ -841,6 +841,7 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
wgpu::Buffer buffer = CreateMapWriteBuffer(8); wgpu::Buffer buffer = CreateMapWriteBuffer(8);
buffer.MapAsync(wgpu::MapMode::Write, 0, 8, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 0, 8, nullptr, nullptr);
EXPECT_EQ(buffer.GetMappedRange(9, 0), nullptr); EXPECT_EQ(buffer.GetMappedRange(9, 0), nullptr);
EXPECT_EQ(buffer.GetMappedRange(16, 0), nullptr);
} }
// Error case: offset + size is larger than the mapped range // Error case: offset + size is larger than the mapped range
@ -848,6 +849,7 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
wgpu::Buffer buffer = CreateMapWriteBuffer(12); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
buffer.MapAsync(wgpu::MapMode::Write, 0, 12, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 0, 12, nullptr, nullptr);
EXPECT_EQ(buffer.GetMappedRange(8, 5), nullptr); EXPECT_EQ(buffer.GetMappedRange(8, 5), nullptr);
EXPECT_EQ(buffer.GetMappedRange(8, 8), nullptr);
} }
// Error case: offset + size is larger than the mapped range, overflow case // Error case: offset + size is larger than the mapped range, overflow case
@ -860,7 +862,8 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
// Error case: offset is before the start of the range // Error case: offset is before the start of the range
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(12); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
buffer.MapAsync(wgpu::MapMode::Write, 4, 4, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 8, 4, nullptr, nullptr);
EXPECT_EQ(buffer.GetMappedRange(3, 4), nullptr); EXPECT_EQ(buffer.GetMappedRange(7, 4), nullptr);
EXPECT_EQ(buffer.GetMappedRange(0, 4), nullptr);
} }
} }