[dawn][wire] Introduce Reserved objects for allocation and release.
- This differentiates Known and Reserved objects. This is important
so that we properly release objects, i.e. Devices, that may be
reserved but never fulfilled on the Server side.
- Fixes the fuzzer bug by ensuring that trying to create children
object with a bad/non-existent backend-backed Device does not
cause a bad memory read when trying to access a nullptr.
Bug: b:333756600 b:333641451
Change-Id: Idd0f754c46d265d45e4d89a96f4756b982bcd746
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/183422
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/generator/templates/dawn/wire/server/ServerDoers.cpp b/generator/templates/dawn/wire/server/ServerDoers.cpp
index 4431f14..8f05ea4 100644
--- a/generator/templates/dawn/wire/server/ServerDoers.cpp
+++ b/generator/templates/dawn/wire/server/ServerDoers.cpp
@@ -81,21 +81,17 @@
switch(objectType) {
{% for type in by_category["object"] %}
case ObjectType::{{type.name.CamelCase()}}: {
- Known<WGPU{{type.name.CamelCase()}}> obj;
+ Reserved<WGPU{{type.name.CamelCase()}}> obj;
WIRE_TRY({{type.name.CamelCase()}}Objects().Get(objectId, &obj));
if (obj->state == AllocationState::Allocated) {
+ DAWN_ASSERT(obj->handle != nullptr);
{% if type.name.get() == "device" %}
- if (obj->handle != nullptr) {
- //* Deregisters uncaptured error and device lost callbacks since
- //* they should not be forwarded if the device no longer exists on the wire.
- ClearDeviceCallbacks(obj->handle);
- mProcs.{{as_varName(type.name, Name("release"))}}(obj->handle);
- }
- {% else %}
- DAWN_ASSERT(obj->handle != nullptr);
- mProcs.{{as_varName(type.name, Name("release"))}}(obj->handle);
+ //* Deregisters uncaptured error and device lost callbacks since
+ //* they should not be forwarded if the device no longer exists on the wire.
+ ClearDeviceCallbacks(obj->handle);
{% endif %}
+ mProcs.{{as_varName(type.name, Name("release"))}}(obj->handle);
}
{{type.name.CamelCase()}}Objects().Free(objectId);
return WireResult::Success;
diff --git a/generator/templates/dawn/wire/server/ServerHandlers.cpp b/generator/templates/dawn/wire/server/ServerHandlers.cpp
index b6624b6..7b8110b 100644
--- a/generator/templates/dawn/wire/server/ServerHandlers.cpp
+++ b/generator/templates/dawn/wire/server/ServerHandlers.cpp
@@ -52,7 +52,7 @@
{{ assert(member.handle_type) }}
{% set Type = member.handle_type.name.CamelCase() %}
{% set name = as_varName(member.name) %}
- Known<WGPU{{Type}}> {{name}}Data;
+ Reserved<WGPU{{Type}}> {{name}}Data;
WIRE_TRY({{Type}}Objects().Allocate(&{{name}}Data, cmd.{{name}}));
{{name}}Data->generation = cmd.{{name}}.generation;
{%- endfor %}
diff --git a/src/dawn/wire/server/ObjectStorage.h b/src/dawn/wire/server/ObjectStorage.h
index 797499a..5ad842a 100644
--- a/src/dawn/wire/server/ObjectStorage.h
+++ b/src/dawn/wire/server/ObjectStorage.h
@@ -52,7 +52,7 @@
template <typename T>
struct ObjectDataBase {
// The backend-provided handle and generation to this object.
- T handle;
+ T handle = nullptr;
ObjectGeneration generation = 0;
AllocationState state;
@@ -87,9 +87,10 @@
std::unique_ptr<DeviceInfo> info = std::make_unique<DeviceInfo>();
};
-// Information of both an ID and an object data for use as a shorthand in doers.
+// Information of both an ID and an object data for use as a shorthand in doers. Reserved objects
+// are guaranteed to have been reserved, but not guaranteed to be backed by a valid backend handle.
template <typename T>
-struct Known {
+struct Reserved {
ObjectId id;
raw_ptr<ObjectData<T>> data;
@@ -108,6 +109,31 @@
}
};
+// Information of both an ID and an object data for use as a shorthand in doers. Known objects are
+// guaranteed to be backed by a valid backend handle.
+template <typename T>
+struct Known {
+ ObjectId id;
+ raw_ptr<ObjectData<T>> data;
+
+ const ObjectData<T>* operator->() const {
+ DAWN_ASSERT(data != nullptr);
+ DAWN_ASSERT(data->state == AllocationState::Allocated);
+ return data;
+ }
+ ObjectData<T>* operator->() {
+ DAWN_ASSERT(data != nullptr);
+ DAWN_ASSERT(data->state == AllocationState::Allocated);
+ return data;
+ }
+
+ ObjectHandle AsHandle() const {
+ DAWN_ASSERT(data != nullptr);
+ DAWN_ASSERT(data->state == AllocationState::Allocated);
+ return {id, data->generation};
+ }
+};
+
// Keeps track of the mapping between client IDs and backend objects.
template <typename T>
class KnownObjectsBase {
@@ -140,6 +166,20 @@
return WireResult::Success;
}
+ WireResult Get(ObjectId id, Reserved<T>* result) {
+ if (id >= mKnown.size()) {
+ return WireResult::FatalError;
+ }
+
+ Data* data = &mKnown[id];
+ if (data->state == AllocationState::Free) {
+ return WireResult::FatalError;
+ }
+
+ *result = Reserved<T>{id, data};
+ return WireResult::Success;
+ }
+
WireResult Get(ObjectId id, Known<T>* result) {
if (id >= mKnown.size()) {
return WireResult::FatalError;
@@ -166,7 +206,7 @@
// Allocates the data for a given ID and returns it in result.
// Returns false if the ID is already allocated, or too far ahead, or if ID is 0 (ID 0 is
// reserved for nullptr). Invalidates all the Data*
- WireResult Allocate(Known<T>* result,
+ WireResult Allocate(Reserved<T>* result,
ObjectHandle handle,
AllocationState state = AllocationState::Allocated) {
if (handle.id == 0 || handle.id > mKnown.size()) {
@@ -245,17 +285,16 @@
public:
KnownObjects() = default;
- WireResult Allocate(Known<WGPUDevice>* result,
+ WireResult Allocate(Reserved<WGPUDevice>* result,
ObjectHandle handle,
AllocationState state = AllocationState::Allocated) {
WIRE_TRY(KnownObjectsBase<WGPUDevice>::Allocate(result, handle, state));
- AddToKnownSet(*result);
return WireResult::Success;
}
Known<WGPUDevice> FillReservation(ObjectId id, WGPUDevice handle) {
Known<WGPUDevice> result = KnownObjectsBase<WGPUDevice>::FillReservation(id, handle);
- AddToKnownSet(result);
+ mKnownSet.insert(result->handle);
return result;
}
@@ -267,11 +306,6 @@
bool IsKnown(WGPUDevice device) const { return mKnownSet.contains(device); }
private:
- void AddToKnownSet(Known<WGPUDevice> device) {
- if (device->state == AllocationState::Allocated && device->handle != nullptr) {
- mKnownSet.insert(device->handle);
- }
- }
absl::flat_hash_set<WGPUDevice> mKnownSet;
};
diff --git a/src/dawn/wire/server/Server.cpp b/src/dawn/wire/server/Server.cpp
index 7ed991e..bac95a1 100644
--- a/src/dawn/wire/server/Server.cpp
+++ b/src/dawn/wire/server/Server.cpp
@@ -71,7 +71,7 @@
return WireResult::FatalError;
}
- Known<WGPUTexture> data;
+ Reserved<WGPUTexture> data;
WIRE_TRY(TextureObjects().Allocate(&data, handle));
data->handle = texture;
@@ -95,7 +95,7 @@
return WireResult::FatalError;
}
- Known<WGPUSwapChain> data;
+ Reserved<WGPUSwapChain> data;
WIRE_TRY(SwapChainObjects().Allocate(&data, handle));
data->handle = swapchain;
@@ -111,7 +111,7 @@
WireResult Server::InjectInstance(WGPUInstance instance, const Handle& handle) {
DAWN_ASSERT(instance != nullptr);
- Known<WGPUInstance> data;
+ Reserved<WGPUInstance> data;
WIRE_TRY(InstanceObjects().Allocate(&data, handle));
data->handle = instance;
diff --git a/src/dawn/wire/server/ServerAdapter.cpp b/src/dawn/wire/server/ServerAdapter.cpp
index c5bbba1..4eb4c93 100644
--- a/src/dawn/wire/server/ServerAdapter.cpp
+++ b/src/dawn/wire/server/ServerAdapter.cpp
@@ -38,7 +38,7 @@
ObjectHandle deviceHandle,
WGPUFuture deviceLostFuture,
const WGPUDeviceDescriptor* descriptor) {
- Known<WGPUDevice> device;
+ Reserved<WGPUDevice> device;
WIRE_TRY(DeviceObjects().Allocate(&device, deviceHandle, AllocationState::Reserved));
auto userdata = MakeUserdata<RequestDeviceUserdata>();
@@ -73,19 +73,9 @@
cmd.status = status;
cmd.message = message;
- // We always fill the reservation once we complete so that the client is the one to release it.
- auto FillReservation = [&]() {
- Known<WGPUDevice> reservation =
- DeviceObjects().FillReservation(data->deviceObjectId, device);
- reservation->info->server = this;
- reservation->info->self = reservation.AsHandle();
- SerializeCommand(cmd);
- return reservation;
- };
-
if (status != WGPURequestDeviceStatus_Success) {
DAWN_ASSERT(device == nullptr);
- FillReservation();
+ SerializeCommand(cmd);
return;
}
@@ -107,7 +97,7 @@
cmd.status = WGPURequestDeviceStatus_Error;
cmd.message = "Requested feature not supported.";
- FillReservation();
+ SerializeCommand(cmd);
return;
}
}
@@ -124,9 +114,12 @@
cmd.limits = &limits;
// Assign the handle and allocated status if the device is created successfully.
- Known<WGPUDevice> reservation = FillReservation();
+ Known<WGPUDevice> reservation = DeviceObjects().FillReservation(data->deviceObjectId, device);
DAWN_ASSERT(reservation.data != nullptr);
+ reservation->info->server = this;
+ reservation->info->self = reservation.AsHandle();
SetForwardingDeviceCallbacks(reservation);
+ SerializeCommand(cmd);
}
} // namespace dawn::wire::server
diff --git a/src/dawn/wire/server/ServerBuffer.cpp b/src/dawn/wire/server/ServerBuffer.cpp
index 22e2769..56be933 100644
--- a/src/dawn/wire/server/ServerBuffer.cpp
+++ b/src/dawn/wire/server/ServerBuffer.cpp
@@ -113,7 +113,7 @@
uint64_t writeHandleCreateInfoLength,
const uint8_t* writeHandleCreateInfo) {
// Create and register the buffer object.
- Known<WGPUBuffer> buffer;
+ Reserved<WGPUBuffer> buffer;
WIRE_TRY(BufferObjects().Allocate(&buffer, bufferHandle));
buffer->handle = mProcs.deviceCreateBuffer(device->handle, descriptor);
buffer->usage = descriptor->usage;
diff --git a/src/dawn/wire/server/ServerDevice.cpp b/src/dawn/wire/server/ServerDevice.cpp
index f9fa66b..8c20356 100644
--- a/src/dawn/wire/server/ServerDevice.cpp
+++ b/src/dawn/wire/server/ServerDevice.cpp
@@ -109,7 +109,7 @@
WGPUFuture future,
ObjectHandle pipelineObjectHandle,
const WGPUComputePipelineDescriptor* descriptor) {
- Known<WGPUComputePipeline> pipeline;
+ Reserved<WGPUComputePipeline> pipeline;
WIRE_TRY(ComputePipelineObjects().Allocate(&pipeline, pipelineObjectHandle,
AllocationState::Reserved));
@@ -147,7 +147,7 @@
WGPUFuture future,
ObjectHandle pipelineObjectHandle,
const WGPURenderPipelineDescriptor* descriptor) {
- Known<WGPURenderPipeline> pipeline;
+ Reserved<WGPURenderPipeline> pipeline;
WIRE_TRY(RenderPipelineObjects().Allocate(&pipeline, pipelineObjectHandle,
AllocationState::Reserved));
diff --git a/src/dawn/wire/server/ServerInstance.cpp b/src/dawn/wire/server/ServerInstance.cpp
index c25688e..255ab3f 100644
--- a/src/dawn/wire/server/ServerInstance.cpp
+++ b/src/dawn/wire/server/ServerInstance.cpp
@@ -38,7 +38,7 @@
WGPUFuture future,
ObjectHandle adapterHandle,
const WGPURequestAdapterOptions* options) {
- Known<WGPUAdapter> adapter;
+ Reserved<WGPUAdapter> adapter;
WIRE_TRY(AdapterObjects().Allocate(&adapter, adapterHandle, AllocationState::Reserved));
auto userdata = MakeUserdata<RequestAdapterUserdata>();
