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>

generator/templates/dawn/wire/server/ServerHandlers.cpp

@@ -47,7 +47,7 @@ namespace dawn::wire::server {
                 {% 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;
                 }

src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp

@@ -72,6 +72,47 @@ TEST_F(WireInjectTextureTests, InjectExistingID) {
                                                 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);

src/dawn/wire/server/ObjectStorage.h

@@ -126,8 +126,8 @@ class KnownObjectsBase {
     // 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) {
+    Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) {
-        if (id == 0 || id > mKnown.size()) {
+        if (handle.id == 0 || handle.id > mKnown.size()) {
             return nullptr;
         }
 
@@ -135,17 +135,24 @@ class KnownObjectsBase {
         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);
+        // The generation should be strictly increasing.
-        return &mKnown[id];
+        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 @@ class KnownObjects<WGPUDevice> : public KnownObjectsBase<WGPUDevice> {
   public:
     KnownObjects() = default;
 
-    Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) {
+    Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) {
-        Data* data = KnownObjectsBase<WGPUDevice>::Allocate(id, state);
+        Data* data = KnownObjectsBase<WGPUDevice>::Allocate(handle, state);
         AddToKnownSet(data);
         return data;
     }

src/dawn/wire/server/Server.cpp

@@ -54,7 +54,7 @@ bool Server::InjectTexture(WGPUTexture texture,
         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 @@ bool Server::InjectSwapChain(WGPUSwapChain swapchain,
         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::InjectSwapChain(WGPUSwapChain swapchain,
 
 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::InjectDevice(WGPUDevice device, uint32_t id, uint32_t generation) {
 
 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;
     }

src/dawn/wire/server/ServerAdapter.cpp

@@ -28,7 +28,7 @@ bool Server::DoAdapterRequestDevice(ObjectId adapterId,
         return false;
     }
 
-    auto* resultData = DeviceObjects().Allocate(deviceHandle.id, AllocationState::Reserved);
+    auto* resultData = DeviceObjects().Allocate(deviceHandle, AllocationState::Reserved);
     if (resultData == nullptr) {
         return false;
     }

src/dawn/wire/server/ServerBuffer.cpp

@@ -110,7 +110,7 @@ bool Server::DoDeviceCreateBuffer(ObjectId deviceId,
     }
 
     // Create and register the buffer object.
-    auto* resultData = BufferObjects().Allocate(bufferResult.id);
+    auto* resultData = BufferObjects().Allocate(bufferResult);
     if (resultData == nullptr) {
         return false;
     }

src/dawn/wire/server/ServerDevice.cpp

@@ -99,7 +99,7 @@ bool Server::DoDeviceCreateComputePipelineAsync(ObjectId deviceId,
     }
 
     auto* resultData =
-        ComputePipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved);
+        ComputePipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved);
     if (resultData == nullptr) {
         return false;
     }
@@ -143,7 +143,7 @@ bool Server::DoDeviceCreateRenderPipelineAsync(ObjectId deviceId,
     }
 
     auto* resultData =
-        RenderPipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved);
+        RenderPipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved);
     if (resultData == nullptr) {
         return false;
     }

src/dawn/wire/server/ServerInstance.cpp

@@ -29,7 +29,7 @@ bool Server::DoInstanceRequestAdapter(ObjectId instanceId,
         return false;
     }
 
-    auto* resultData = AdapterObjects().Allocate(adapterHandle.id, AllocationState::Reserved);
+    auto* resultData = AdapterObjects().Allocate(adapterHandle, AllocationState::Reserved);
     if (resultData == nullptr) {
         return false;
     }