Update default value for map range size to WGPU_WHOLE_MAP_SIZE

Bug exposed by emscripten webgpu binding issue.

It seems the spec updated after the last code updates. Update the
validation part a bit (introduce rangeSize).

Bug: dawn:1400
Change-Id: I0ddefd5c1a0976cc34102a44514bccd70f7a1ac0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/88080
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Shrek Shao <shrekshao@google.com>
This commit is contained in:
Shrek Shao 2022-04-29 18:12:17 +00:00 committed by Dawn LUCI CQ
parent 3daebe896f
commit 286061d810
5 changed files with 49 additions and 15 deletions

View File

@ -413,7 +413,7 @@
"returns": "void *", "returns": "void *",
"args": [ "args": [
{"name": "offset", "type": "size_t", "default": 0}, {"name": "offset", "type": "size_t", "default": 0},
{"name": "size", "type": "size_t", "default": 0} {"name": "size", "type": "size_t", "default": "WGPU_WHOLE_MAP_SIZE"}
] ]
}, },
{ {
@ -421,7 +421,7 @@
"returns": "void const *", "returns": "void const *",
"args": [ "args": [
{"name": "offset", "type": "size_t", "default": 0}, {"name": "offset", "type": "size_t", "default": 0},
{"name": "size", "type": "size_t", "default": 0} {"name": "size", "type": "size_t", "default": "WGPU_WHOLE_MAP_SIZE"}
] ]
}, },
{ {

View File

@ -494,16 +494,18 @@ 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) { if (offset % 8 != 0 || offset < mMapOffset || offset > mSize) {
return false; return false;
} }
if (size > mMapSize || offset < mMapOffset) { size_t rangeSize = size == WGPU_WHOLE_MAP_SIZE ? mSize - offset : size;
if (rangeSize % 4 != 0 || rangeSize > mMapSize) {
return false; return false;
} }
size_t offsetInMappedRange = offset - mMapOffset; size_t offsetInMappedRange = offset - mMapOffset;
if (offsetInMappedRange > mMapSize - size) { if (offsetInMappedRange > mMapSize - rangeSize) {
return false; return false;
} }

View File

@ -280,13 +280,13 @@ TEST_P(BufferMappingTests, MapWrite_TwicePreserve) {
uint32_t data1 = 0x08090a0b; uint32_t data1 = 0x08090a0b;
size_t offset1 = 8; size_t offset1 = 8;
MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset1, sizeof(data1)); MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset1, wgpu::kWholeMapSize);
memcpy(buffer.GetMappedRange(offset1), &data1, sizeof(data1)); memcpy(buffer.GetMappedRange(offset1), &data1, sizeof(data1));
buffer.Unmap(); buffer.Unmap();
uint32_t data2 = 0x00010203; uint32_t data2 = 0x00010203;
size_t offset2 = 0; size_t offset2 = 0;
MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset2, sizeof(data2)); MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset2, wgpu::kWholeMapSize);
memcpy(buffer.GetMappedRange(offset2), &data2, sizeof(data2)); memcpy(buffer.GetMappedRange(offset2), &data2, sizeof(data2));
buffer.Unmap(); buffer.Unmap();
@ -301,7 +301,7 @@ TEST_P(BufferMappingTests, MapWrite_TwiceRangeOverlap) {
uint32_t data1[] = {0x01234567, 0x89abcdef}; uint32_t data1[] = {0x01234567, 0x89abcdef};
size_t offset1 = 8; size_t offset1 = 8;
MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset1, 8); MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset1, 8);
memcpy(buffer.GetMappedRange(offset1), data1, 8); memcpy(buffer.GetMappedRange(offset1, 8), data1, 8);
buffer.Unmap(); buffer.Unmap();
EXPECT_BUFFER_U32_EQ(0x00000000, buffer, 0); EXPECT_BUFFER_U32_EQ(0x00000000, buffer, 0);
@ -312,7 +312,7 @@ TEST_P(BufferMappingTests, MapWrite_TwiceRangeOverlap) {
uint32_t data2[] = {0x01234567, 0x89abcdef, 0x55555555}; uint32_t data2[] = {0x01234567, 0x89abcdef, 0x55555555};
size_t offset2 = 0; size_t offset2 = 0;
MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset2, 12); MapAsyncAndWait(buffer, wgpu::MapMode::Write, offset2, 12);
memcpy(buffer.GetMappedRange(offset2), data2, 12); memcpy(buffer.GetMappedRange(offset2, 12), data2, 12);
buffer.Unmap(); buffer.Unmap();
EXPECT_BUFFER_U32_EQ(0x01234567, buffer, 0); EXPECT_BUFFER_U32_EQ(0x01234567, buffer, 0);
@ -354,7 +354,7 @@ TEST_P(BufferMappingTests, MapWrite_Large) {
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));
memcpy(buffer.GetMappedRange(16), myData.data(), kByteSize - 20); memcpy(buffer.GetMappedRange(16, kByteSize - 20), myData.data(), kByteSize - 20);
buffer.Unmap(); buffer.Unmap();
EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 16, kDataSize - 5); EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 16, kDataSize - 5);
} }

View File

@ -859,6 +859,14 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
EXPECT_NE(buffer.GetMappedRange(0, 8), nullptr); EXPECT_NE(buffer.GetMappedRange(0, 8), nullptr);
} }
// Valid case: full range is ok with defaulted MapAsync size and defaulted GetMappedRangeSize
{
wgpu::Buffer buffer = CreateMapWriteBuffer(8);
buffer.MapAsync(wgpu::MapMode::Write, 0, wgpu::kWholeMapSize, nullptr, nullptr);
WaitForAllOperations(device);
EXPECT_NE(buffer.GetMappedRange(0, wgpu::kWholeMapSize), nullptr);
}
// Valid case: empty range at the end is ok // Valid case: empty range at the end is ok
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(8); wgpu::Buffer buffer = CreateMapWriteBuffer(8);
@ -882,6 +890,17 @@ TEST_F(BufferValidationTest, GetMappedRange_OffsetSizeOOB) {
WaitForAllOperations(device); WaitForAllOperations(device);
EXPECT_EQ(buffer.GetMappedRange(9, 0), nullptr); EXPECT_EQ(buffer.GetMappedRange(9, 0), nullptr);
EXPECT_EQ(buffer.GetMappedRange(16, 0), nullptr); EXPECT_EQ(buffer.GetMappedRange(16, 0), nullptr);
EXPECT_EQ(buffer.GetMappedRange(std::numeric_limits<size_t>::max(), 0), nullptr);
}
// Error case: offset is larger than the buffer size (even with size = 0)
{
wgpu::Buffer buffer = CreateMapWriteBuffer(16);
buffer.MapAsync(wgpu::MapMode::Write, 8, 8, nullptr, nullptr);
WaitForAllOperations(device);
EXPECT_EQ(buffer.GetMappedRange(16, 4), nullptr);
EXPECT_EQ(buffer.GetMappedRange(24, 0), nullptr);
EXPECT_EQ(buffer.GetMappedRange(std::numeric_limits<size_t>::max(), 0), nullptr);
} }
// Error case: offset + size is larger than the mapped range // Error case: offset + size is larger than the mapped range
@ -898,15 +917,26 @@ 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);
WaitForAllOperations(device); WaitForAllOperations(device);
EXPECT_EQ(buffer.GetMappedRange(8, std::numeric_limits<size_t>::max()), nullptr); // set size to (max - 1) to avoid being equal to kWholeMapSize
EXPECT_EQ(buffer.GetMappedRange(8, std::numeric_limits<size_t>::max() - 1), nullptr);
} }
// Error case: offset is before the start of the range // Error case: size is larger than the mapped range when using default kWholeMapSize
{
wgpu::Buffer buffer = CreateMapWriteBuffer(12);
buffer.MapAsync(wgpu::MapMode::Write, 0, 8, nullptr, nullptr);
WaitForAllOperations(device);
EXPECT_EQ(buffer.GetMappedRange(0), nullptr);
}
// Error case: offset is before the start of the range (even with size = 0)
{ {
wgpu::Buffer buffer = CreateMapWriteBuffer(12); wgpu::Buffer buffer = CreateMapWriteBuffer(12);
buffer.MapAsync(wgpu::MapMode::Write, 8, 4, nullptr, nullptr); buffer.MapAsync(wgpu::MapMode::Write, 8, 4, nullptr, nullptr);
WaitForAllOperations(device); WaitForAllOperations(device);
EXPECT_EQ(buffer.GetMappedRange(7, 4), nullptr); EXPECT_EQ(buffer.GetMappedRange(7, 4), nullptr);
EXPECT_EQ(buffer.GetMappedRange(0, 4), nullptr); EXPECT_EQ(buffer.GetMappedRange(0, 4), nullptr);
EXPECT_EQ(buffer.GetMappedRange(0, 12), nullptr);
EXPECT_EQ(buffer.GetMappedRange(0, 0), nullptr);
} }
} }

View File

@ -377,16 +377,18 @@ namespace dawn::wire::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) { if (offset % 8 != 0 || offset < mMapOffset || offset > mSize) {
return false; return false;
} }
if (size > mMapSize || offset < mMapOffset) { size_t rangeSize = size == WGPU_WHOLE_MAP_SIZE ? mSize - offset : size;
if (rangeSize % 4 != 0 || rangeSize > mMapSize) {
return false; return false;
} }
size_t offsetInMappedRange = offset - mMapOffset; size_t offsetInMappedRange = offset - mMapOffset;
return offsetInMappedRange <= mMapSize - size; return offsetInMappedRange <= mMapSize - rangeSize;
} }
void Buffer::FreeMappedData() { void Buffer::FreeMappedData() {