dawn::wire validate that the generation must be strictly increasing
Bug: chromium:1379634
Change-Id: Iaa067a30ac5992adfb5aac168104163a62f8bf9c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/108549
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Jie A Chen <jie.a.chen@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/generator/templates/dawn/wire/server/ServerHandlers.cpp b/generator/templates/dawn/wire/server/ServerHandlers.cpp
index 9a9f05a..fcba3d1 100644
--- a/generator/templates/dawn/wire/server/ServerHandlers.cpp
+++ b/generator/templates/dawn/wire/server/ServerHandlers.cpp
@@ -47,7 +47,7 @@
{% set Type = member.handle_type.name.CamelCase() %}
{% set name = as_varName(member.name) %}
- auto* {{name}}Data = {{Type}}Objects().Allocate(cmd.{{name}}.id);
+ auto* {{name}}Data = {{Type}}Objects().Allocate(cmd.{{name}});
if ({{name}}Data == nullptr) {
return false;
}
diff --git a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
index 3438d9b..005522f 100644
--- a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp
@@ -72,6 +72,47 @@
reservation.deviceGeneration));
}
+// Test that injecting the same id without a destroy first fails.
+TEST_F(WireInjectTextureTests, ReuseIDAndGeneration) {
+ // Do this loop multiple times since the first time, we can't test `generation - 1` since
+ // generation == 0.
+ ReservedTexture reservation;
+ WGPUTexture apiTexture = nullptr;
+ for (int i = 0; i < 2; ++i) {
+ reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
+
+ apiTexture = api.GetNewTexture();
+ EXPECT_CALL(api, TextureReference(apiTexture));
+ ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id,
+ reservation.generation, reservation.deviceId,
+ reservation.deviceGeneration));
+
+ // Release the texture. It should be possible to reuse the ID now, but not the generation
+ wgpuTextureRelease(reservation.texture);
+ EXPECT_CALL(api, TextureRelease(apiTexture));
+ FlushClient();
+
+ // Invalid to inject with the same ID and generation.
+ ASSERT_FALSE(GetWireServer()->InjectTexture(apiTexture, reservation.id,
+ reservation.generation, reservation.deviceId,
+ reservation.deviceGeneration));
+ if (i > 0) {
+ EXPECT_GE(reservation.generation, 1u);
+
+ // Invalid to inject with the same ID and lesser generation.
+ ASSERT_FALSE(GetWireServer()->InjectTexture(
+ apiTexture, reservation.id, reservation.generation - 1, reservation.deviceId,
+ reservation.deviceGeneration));
+ }
+ }
+
+ // Valid to inject with the same ID and greater generation.
+ EXPECT_CALL(api, TextureReference(apiTexture));
+ ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id,
+ reservation.generation + 1, reservation.deviceId,
+ reservation.deviceGeneration));
+}
+
// Test that the server only borrows the texture and does a single reference-release
TEST_F(WireInjectTextureTests, InjectedTextureLifetime) {
ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc);
diff --git a/src/dawn/wire/server/ObjectStorage.h b/src/dawn/wire/server/ObjectStorage.h
index 1bef0ad..4c72c65 100644
--- a/src/dawn/wire/server/ObjectStorage.h
+++ b/src/dawn/wire/server/ObjectStorage.h
@@ -126,8 +126,8 @@
// Allocates the data for a given ID and returns it.
// Returns nullptr 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*
- Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) {
- if (id == 0 || id > mKnown.size()) {
+ Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) {
+ if (handle.id == 0 || handle.id > mKnown.size()) {
return nullptr;
}
@@ -135,17 +135,24 @@
data.state = state;
data.handle = nullptr;
- if (id >= mKnown.size()) {
+ if (handle.id >= mKnown.size()) {
mKnown.push_back(std::move(data));
return &mKnown.back();
}
- if (mKnown[id].state != AllocationState::Free) {
+ if (mKnown[handle.id].state != AllocationState::Free) {
return nullptr;
}
- mKnown[id] = std::move(data);
- return &mKnown[id];
+ // The generation should be strictly increasing.
+ if (handle.generation <= mKnown[handle.id].generation) {
+ return nullptr;
+ }
+ // update the generation in the slot
+ data.generation = handle.generation;
+
+ mKnown[handle.id] = std::move(data);
+ return &mKnown[handle.id];
}
// Marks an ID as deallocated
@@ -193,8 +200,8 @@
public:
KnownObjects() = default;
- Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) {
- Data* data = KnownObjectsBase<WGPUDevice>::Allocate(id, state);
+ Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) {
+ Data* data = KnownObjectsBase<WGPUDevice>::Allocate(handle, state);
AddToKnownSet(data);
return data;
}
diff --git a/src/dawn/wire/server/Server.cpp b/src/dawn/wire/server/Server.cpp
index 2796966..7de375f 100644
--- a/src/dawn/wire/server/Server.cpp
+++ b/src/dawn/wire/server/Server.cpp
@@ -54,7 +54,7 @@
return false;
}
- ObjectData<WGPUTexture>* data = TextureObjects().Allocate(id);
+ ObjectData<WGPUTexture>* data = TextureObjects().Allocate(ObjectHandle{id, generation});
if (data == nullptr) {
return false;
}
@@ -81,7 +81,7 @@
return false;
}
- ObjectData<WGPUSwapChain>* data = SwapChainObjects().Allocate(id);
+ ObjectData<WGPUSwapChain>* data = SwapChainObjects().Allocate(ObjectHandle{id, generation});
if (data == nullptr) {
return false;
}
@@ -99,7 +99,7 @@
bool Server::InjectDevice(WGPUDevice device, uint32_t id, uint32_t generation) {
ASSERT(device != nullptr);
- ObjectData<WGPUDevice>* data = DeviceObjects().Allocate(id);
+ ObjectData<WGPUDevice>* data = DeviceObjects().Allocate(ObjectHandle{id, generation});
if (data == nullptr) {
return false;
}
@@ -121,7 +121,7 @@
bool Server::InjectInstance(WGPUInstance instance, uint32_t id, uint32_t generation) {
ASSERT(instance != nullptr);
- ObjectData<WGPUInstance>* data = InstanceObjects().Allocate(id);
+ ObjectData<WGPUInstance>* data = InstanceObjects().Allocate(ObjectHandle{id, generation});
if (data == nullptr) {
return false;
}
diff --git a/src/dawn/wire/server/ServerAdapter.cpp b/src/dawn/wire/server/ServerAdapter.cpp
index 9735d26..8fa9e06 100644
--- a/src/dawn/wire/server/ServerAdapter.cpp
+++ b/src/dawn/wire/server/ServerAdapter.cpp
@@ -28,7 +28,7 @@
return false;
}
- auto* resultData = DeviceObjects().Allocate(deviceHandle.id, AllocationState::Reserved);
+ auto* resultData = DeviceObjects().Allocate(deviceHandle, AllocationState::Reserved);
if (resultData == nullptr) {
return false;
}
diff --git a/src/dawn/wire/server/ServerBuffer.cpp b/src/dawn/wire/server/ServerBuffer.cpp
index 61d5e15..f07bdfa 100644
--- a/src/dawn/wire/server/ServerBuffer.cpp
+++ b/src/dawn/wire/server/ServerBuffer.cpp
@@ -110,7 +110,7 @@
}
// Create and register the buffer object.
- auto* resultData = BufferObjects().Allocate(bufferResult.id);
+ auto* resultData = BufferObjects().Allocate(bufferResult);
if (resultData == nullptr) {
return false;
}
diff --git a/src/dawn/wire/server/ServerDevice.cpp b/src/dawn/wire/server/ServerDevice.cpp
index 2797815..0281c33 100644
--- a/src/dawn/wire/server/ServerDevice.cpp
+++ b/src/dawn/wire/server/ServerDevice.cpp
@@ -99,7 +99,7 @@
}
auto* resultData =
- ComputePipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved);
+ ComputePipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved);
if (resultData == nullptr) {
return false;
}
@@ -143,7 +143,7 @@
}
auto* resultData =
- RenderPipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved);
+ RenderPipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved);
if (resultData == nullptr) {
return false;
}
diff --git a/src/dawn/wire/server/ServerInstance.cpp b/src/dawn/wire/server/ServerInstance.cpp
index dcdf62c..4357604 100644
--- a/src/dawn/wire/server/ServerInstance.cpp
+++ b/src/dawn/wire/server/ServerInstance.cpp
@@ -29,7 +29,7 @@
return false;
}
- auto* resultData = AdapterObjects().Allocate(adapterHandle.id, AllocationState::Reserved);
+ auto* resultData = AdapterObjects().Allocate(adapterHandle, AllocationState::Reserved);
if (resultData == nullptr) {
return false;
}
