Wire: Make validation error prior to OOM if mappedAtCreation == false
This patch updates the validations about CreateBuffer() with dawn_wire
to match the latest WebGPU SPEC.
According to the SPEC, the validations in CreateBuffer() should be
executed in the below order:
1. If mappedAtCreation == true, return nullptr and a RangeError will be
generated in Chromium.
2. Validate BufferDescriptor and check if there is OOM at device timeline
3. Check if there is OOM at content timeline
Bug: dawn:1586
Change-Id: I97ff5f82a42208442ddf6e46e66381c3b3680450
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/109040
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/dawn.json b/dawn.json
index 817991f..972d68d 100644
--- a/dawn.json
+++ b/dawn.json
@@ -994,7 +994,10 @@
{
"name": "create error buffer",
"returns": "buffer",
- "tags": ["dawn"]
+ "tags": ["dawn"],
+ "args": [
+ {"name": "descriptor", "type": "buffer descriptor", "annotation": "const*"}
+ ]
},
{
"name": "create command encoder",
@@ -2567,7 +2570,8 @@
{"value": 1003, "name": "dawn encoder internal usage descriptor", "tags": ["dawn"]},
{"value": 1004, "name": "dawn instance descriptor", "tags": ["dawn", "native"]},
{"value": 1005, "name": "dawn cache device descriptor", "tags": ["dawn", "native"]},
- {"value": 1006, "name": "dawn adapter properties power preference", "tags": ["dawn", "native"]}
+ {"value": 1006, "name": "dawn adapter properties power preference", "tags": ["dawn", "native"]},
+ {"value": 1007, "name": "dawn buffer descriptor error info from wire client", "tags": ["dawn"]}
]
},
"texture": {
@@ -2979,5 +2983,14 @@
"members": [
{"name": "power preference", "type": "power preference", "default": "undefined"}
]
+ },
+ "dawn buffer descriptor error info from wire client": {
+ "category": "structure",
+ "chained": "in",
+ "chain roots": ["buffer descriptor"],
+ "tags": ["dawn"],
+ "members": [
+ {"name": "out of memory", "type": "bool", "default": "false"}
+ ]
}
}
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index fd13a3c..9256829 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -17,6 +17,7 @@
#include <cstdio>
#include <cstring>
#include <limits>
+#include <string>
#include <utility>
#include "dawn/common/Alloc.h"
@@ -96,7 +97,7 @@
} // anonymous namespace
-MaybeError ValidateBufferDescriptor(DeviceBase*, const BufferDescriptor* descriptor) {
+MaybeError ValidateBufferDescriptor(DeviceBase* device, const BufferDescriptor* descriptor) {
DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr");
DAWN_TRY(ValidateBufferUsage(descriptor->usage));
@@ -124,6 +125,14 @@
"Buffer is mapped at creation but its size (%u) is not a multiple of 4.",
descriptor->size);
+ // TODO(dawn:1525): Change to validation error after the deprecation period.
+ if (descriptor->size > device->GetLimits().v1.maxBufferSize) {
+ std::string warning =
+ absl::StrFormat("Buffer size (%u) exceeds the max buffer size limit (%u).",
+ descriptor->size, device->GetLimits().v1.maxBufferSize);
+ device->EmitDeprecationWarning(warning.c_str());
+ }
+
return {};
}
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index ff08f66..19c5df1 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -1193,9 +1193,27 @@
// For Dawn Wire
-BufferBase* DeviceBase::APICreateErrorBuffer() {
- BufferDescriptor desc = {};
- return BufferBase::MakeError(this, &desc);
+BufferBase* DeviceBase::APICreateErrorBuffer(const BufferDescriptor* desc) {
+ BufferDescriptor fakeDescriptor = *desc;
+ fakeDescriptor.nextInChain = nullptr;
+
+ // The validation errors on BufferDescriptor should be prior to any OOM errors when
+ // MapppedAtCreation == false.
+ MaybeError maybeError = ValidateBufferDescriptor(this, &fakeDescriptor);
+ if (maybeError.IsError()) {
+ ConsumedError(maybeError.AcquireError(), "calling %s.CreateBuffer(%s).", this, desc);
+ } else {
+ const DawnBufferDescriptorErrorInfoFromWireClient* clientErrorInfo = nullptr;
+ FindInChain(desc->nextInChain, &clientErrorInfo);
+ if (clientErrorInfo != nullptr && clientErrorInfo->outOfMemory) {
+ ConsumedError(DAWN_OUT_OF_MEMORY_ERROR("Failed to allocate memory for buffer mapping"));
+ }
+ }
+
+ // Set the size of the error buffer to 0 as this function is called only when an OOM happens at
+ // the client side.
+ fakeDescriptor.size = 0;
+ return BufferBase::MakeError(this, &fakeDescriptor);
}
ExternalTextureBase* DeviceBase::APICreateErrorExternalTexture() {
@@ -1408,15 +1426,7 @@
ResultOrError<Ref<BufferBase>> DeviceBase::CreateBuffer(const BufferDescriptor* descriptor) {
DAWN_TRY(ValidateIsAlive());
if (IsValidationEnabled()) {
- DAWN_TRY_CONTEXT(ValidateBufferDescriptor(this, descriptor), "validating %s", descriptor);
-
- // TODO(dawn:1525): Change to validation error after the deprecation period.
- if (descriptor->size > mLimits.v1.maxBufferSize) {
- std::string warning =
- absl::StrFormat("Buffer size (%u) exceeds the max buffer size limit (%u).",
- descriptor->size, mLimits.v1.maxBufferSize);
- EmitDeprecationWarning(warning.c_str());
- }
+ DAWN_TRY(ValidateBufferDescriptor(this, descriptor));
}
Ref<BufferBase> buffer;
diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h
index 6fd0d5a..a7f96d1 100644
--- a/src/dawn/native/Device.h
+++ b/src/dawn/native/Device.h
@@ -272,7 +272,7 @@
InternalPipelineStore* GetInternalPipelineStore();
// For Dawn Wire
- BufferBase* APICreateErrorBuffer();
+ BufferBase* APICreateErrorBuffer(const BufferDescriptor* desc);
ExternalTextureBase* APICreateErrorExternalTexture();
TextureBase* APICreateErrorTexture(const TextureDescriptor* desc);
diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp
index 3ba9261..db9f84c 100644
--- a/src/dawn/tests/end2end/BufferTests.cpp
+++ b/src/dawn/tests/end2end/BufferTests.cpp
@@ -819,6 +819,10 @@
descriptor.size = 1ull << 50;
// TODO(dawn:1525): remove warning expectation after the deprecation period.
ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+
+ // Validation errors should always be prior to OOM.
+ descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::Uniform;
+ ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
}
// Test that a very large buffer mappedAtCreation fails gracefully.
@@ -844,12 +848,22 @@
// Test an enormous buffer fails
descriptor.size = std::numeric_limits<uint64_t>::max();
- ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+ if (UsesWire()) {
+ wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+ ASSERT_EQ(nullptr, buffer.Get());
+ } else {
+ ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+ }
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
descriptor.size = 1ull << 50;
- // TODO(dawn:1525): remove warning expectation after the deprecation period.
- ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+ if (UsesWire()) {
+ wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+ ASSERT_EQ(nullptr, buffer.Get());
+ } else {
+ // TODO(dawn:1525): remove warning expectation after the deprecation period.
+ ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+ }
}
// Test mappable buffer
@@ -864,12 +878,22 @@
// Test an enormous buffer fails
descriptor.size = std::numeric_limits<uint64_t>::max();
- ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+ if (UsesWire()) {
+ wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+ ASSERT_EQ(nullptr, buffer.Get());
+ } else {
+ ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
+ }
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
descriptor.size = 1ull << 50;
- // TODO(dawn:1525): remove warning expectation after the deprecation period.
- ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+ if (UsesWire()) {
+ wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+ ASSERT_EQ(nullptr, buffer.Get());
+ } else {
+ // TODO(dawn:1525): remove warning expectation after the deprecation period.
+ ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)));
+ }
}
}
diff --git a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
index 3b2f3ae..3a0b4e5 100644
--- a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
@@ -849,13 +849,20 @@
uint64_t kStupidLarge = uint64_t(1) << uint64_t(63);
- wgpu::Buffer buffer;
- ASSERT_DEVICE_ERROR(buffer = BufferMappedAtCreation(
- kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
+ if (UsesWire()) {
+ wgpu::Buffer buffer = BufferMappedAtCreation(
+ kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead);
+ ASSERT_EQ(nullptr, buffer.Get());
+ } else {
+ wgpu::Buffer buffer;
+ ASSERT_DEVICE_ERROR(
+ buffer = BufferMappedAtCreation(
+ kStupidLarge, wgpu::BufferUsage::Storage | wgpu::BufferUsage::MapRead));
- // GetMappedRange after mappedAtCreation OOM case returns nullptr.
- ASSERT_EQ(buffer.GetConstMappedRange(), nullptr);
- ASSERT_EQ(buffer.GetConstMappedRange(), buffer.GetMappedRange());
+ // GetMappedRange after mappedAtCreation OOM case returns nullptr.
+ ASSERT_EQ(buffer.GetConstMappedRange(), nullptr);
+ ASSERT_EQ(buffer.GetConstMappedRange(), buffer.GetMappedRange());
+ }
}
// Test validation of the GetMappedRange parameters
diff --git a/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp b/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
index e49ee7d..5a7bc23 100644
--- a/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireBufferMappingTests.cpp
@@ -680,8 +680,7 @@
FlushClient();
}
-// Check that trying to create a buffer of size MAX_SIZE_T is an error handling in the client
-// and never gets to the server-side.
+// Check that trying to create a buffer of size MAX_SIZE_T won't get OOM error at the client side.
TEST_F(WireBufferMappingTests, MaxSizeMappableBufferOOMDirectly) {
size_t kOOMSize = std::numeric_limits<size_t>::max();
WGPUBuffer apiBuffer = api.GetNewBuffer();
@@ -694,8 +693,6 @@
descriptor.mappedAtCreation = true;
wgpuDeviceCreateBuffer(device, &descriptor);
- EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
- EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
FlushClient();
}
@@ -706,8 +703,7 @@
descriptor.size = kOOMSize;
wgpuDeviceCreateBuffer(device, &descriptor);
- EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
- EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
+ EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice, _)).WillOnce(Return(apiBuffer));
FlushClient();
}
@@ -718,8 +714,7 @@
descriptor.size = kOOMSize;
wgpuDeviceCreateBuffer(device, &descriptor);
- EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_OutOfMemory, _));
- EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice)).WillOnce(Return(apiBuffer));
+ EXPECT_CALL(api, DeviceCreateErrorBuffer(apiDevice, _)).WillOnce(Return(apiBuffer));
FlushClient();
}
}
diff --git a/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp b/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
index b2ce580..913d6a7 100644
--- a/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
@@ -891,7 +891,7 @@
descriptor.mappedAtCreation = true;
WGPUBuffer buffer = wgpuDeviceCreateBuffer(device, &descriptor);
- EXPECT_EQ(nullptr, wgpuBufferGetMappedRange(buffer, 0, sizeof(mBufferContent)));
+ EXPECT_EQ(nullptr, buffer);
}
// Test buffer creation with mappedAtCreation DeserializeWriteHandle failure.
diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp
index 4315452..43335ef 100644
--- a/src/dawn/wire/client/Buffer.cpp
+++ b/src/dawn/wire/client/Buffer.cpp
@@ -24,6 +24,20 @@
namespace dawn::wire::client {
+namespace {
+WGPUBuffer CreateErrorBufferOOMAtClient(Device* device, const WGPUBufferDescriptor* descriptor) {
+ if (descriptor->mappedAtCreation) {
+ return nullptr;
+ }
+ WGPUBufferDescriptor errorBufferDescriptor = *descriptor;
+ WGPUDawnBufferDescriptorErrorInfoFromWireClient errorInfo = {};
+ errorInfo.chain.sType = WGPUSType_DawnBufferDescriptorErrorInfoFromWireClient;
+ errorInfo.outOfMemory = true;
+ errorBufferDescriptor.nextInChain = &errorInfo.chain;
+ return device->CreateErrorBuffer(&errorBufferDescriptor);
+}
+} // anonymous namespace
+
// static
WGPUBuffer Buffer::Create(Device* device, const WGPUBufferDescriptor* descriptor) {
Client* wireClient = device->GetClient();
@@ -32,8 +46,7 @@
(descriptor->usage & (WGPUBufferUsage_MapRead | WGPUBufferUsage_MapWrite)) != 0 ||
descriptor->mappedAtCreation;
if (mappable && descriptor->size >= std::numeric_limits<size_t>::max()) {
- device->InjectError(WGPUErrorType_OutOfMemory, "Buffer is too large for map usage");
- return device->CreateErrorBuffer();
+ return CreateErrorBufferOOMAtClient(device, descriptor);
}
std::unique_ptr<MemoryTransferService::ReadHandle> readHandle = nullptr;
@@ -55,8 +68,7 @@
readHandle.reset(
wireClient->GetMemoryTransferService()->CreateReadHandle(descriptor->size));
if (readHandle == nullptr) {
- device->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
- return CreateError(device, descriptor);
+ return CreateErrorBufferOOMAtClient(device, descriptor);
}
readHandleCreateInfoLength = readHandle->SerializeCreateSize();
cmd.readHandleCreateInfoLength = readHandleCreateInfoLength;
@@ -67,8 +79,7 @@
writeHandle.reset(
wireClient->GetMemoryTransferService()->CreateWriteHandle(descriptor->size));
if (writeHandle == nullptr) {
- device->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
- return CreateError(device, descriptor);
+ return CreateErrorBufferOOMAtClient(device, descriptor);
}
writeHandleCreateInfoLength = writeHandle->SerializeCreateSize();
cmd.writeHandleCreateInfoLength = writeHandleCreateInfoLength;
@@ -131,6 +142,8 @@
DeviceCreateErrorBufferCmd cmd;
cmd.self = ToAPI(device);
+ cmd.selfId = device->GetWireId();
+ cmd.descriptor = descriptor;
cmd.result = buffer->GetWireHandle();
client->SerializeCommand(cmd);
diff --git a/src/dawn/wire/client/Device.cpp b/src/dawn/wire/client/Device.cpp
index f2bffe2..f63875f 100644
--- a/src/dawn/wire/client/Device.cpp
+++ b/src/dawn/wire/client/Device.cpp
@@ -199,9 +199,8 @@
return Buffer::Create(this, descriptor);
}
-WGPUBuffer Device::CreateErrorBuffer() {
- WGPUBufferDescriptor fakeDescriptor = {};
- return Buffer::CreateError(this, &fakeDescriptor);
+WGPUBuffer Device::CreateErrorBuffer(const WGPUBufferDescriptor* descriptor) {
+ return Buffer::CreateError(this, descriptor);
}
WGPUQuerySet Device::CreateQuerySet(const WGPUQuerySetDescriptor* descriptor) {
diff --git a/src/dawn/wire/client/Device.h b/src/dawn/wire/client/Device.h
index 90f3eda..9ca7b31 100644
--- a/src/dawn/wire/client/Device.h
+++ b/src/dawn/wire/client/Device.h
@@ -41,7 +41,7 @@
void InjectError(WGPUErrorType type, const char* message);
bool PopErrorScope(WGPUErrorCallback callback, void* userdata);
WGPUBuffer CreateBuffer(const WGPUBufferDescriptor* descriptor);
- WGPUBuffer CreateErrorBuffer();
+ WGPUBuffer CreateErrorBuffer(const WGPUBufferDescriptor* descriptor);
void CreateComputePipelineAsync(WGPUComputePipelineDescriptor const* descriptor,
WGPUCreateComputePipelineAsyncCallback callback,
void* userdata);
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index 3bd78e0..29ebf86 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -206,11 +206,6 @@
crbug.com/dawn/1357 webgpu:api,operation,shader_module,compilation_info:offset_and_length:valid=false;name="unicode" [ Failure ]
################################################################################
-# createBuffer_invalid_and_oom failures
-################################################################################
-crbug.com/dawn/0000 webgpu:api,validation,buffer,create:createBuffer_invalid_and_oom: [ Failure ]
-
-################################################################################
# atan2 shader execution failures
# Very slow, many failing. Skip for now.
# KEEP
