commit e1d7581ee5e4a2a1de567bd97d85eed0c7270d92
author Jiawei Shao <jiawei.shao@intel.com> Mon Apr 15 02:37:11 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Apr 15 02:37:11 2024 +0000
tree 0689ae59f15c2f2b610e703b29a9cb3c353ebaaa
parent fb97d04c0c2e2307dc11a9d9de4eab607af111f9

Fix dangling pointer in TerribleCommandBuffer

Fixed: dawn:2343, dawn:2346
Change-Id: Ic6d584f3769e61c1225e389694517ff63ae55e98
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/183365
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn/fuzzers/lpmfuzz/DawnLPMFuzzer.cpp

diff --git a/src/dawn/fuzzers/lpmfuzz/DawnLPMFuzzer.cpp b/src/dawn/fuzzers/lpmfuzz/DawnLPMFuzzer.cpp
index 39014b1..d8d3880 100644
--- a/src/dawn/fuzzers/lpmfuzz/DawnLPMFuzzer.cpp
+++ b/src/dawn/fuzzers/lpmfuzz/DawnLPMFuzzer.cpp
@@ -125,6 +125,7 @@
 
     // Note: Deleting the server will release all created objects.
     // Deleted devices will wait for idle on destruction.
+    mCommandBuffer->SetHandler(nullptr);
     wireServer = nullptr;
     return result == dawn::wire::WireResult::FatalError;
 }

src/dawn/tests/unittests/GetProcAddressTests.cpp

diff --git a/src/dawn/tests/unittests/GetProcAddressTests.cpp b/src/dawn/tests/unittests/GetProcAddressTests.cpp
index b6c3b85..73ebcc2 100644
--- a/src/dawn/tests/unittests/GetProcAddressTests.cpp
+++ b/src/dawn/tests/unittests/GetProcAddressTests.cpp
@@ -97,6 +97,7 @@
                 // accordingly.
                 mDevice = nullptr;
                 mProcs = wire::client::GetProcs();
+                mC2sBuf->SetHandler(mWireClient.get());
                 break;
             }
 
@@ -111,6 +112,9 @@
     void TearDown() override {
         // Destroy the device before freeing the instance or the wire client in the destructor
         mDevice = wgpu::Device();
+        if (mC2sBuf != nullptr) {
+            mC2sBuf->SetHandler(nullptr);
+        }
     }
 
   protected:

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

diff --git a/src/dawn/tests/unittests/wire/WireCreatePipelineAsyncTests.cpp b/src/dawn/tests/unittests/wire/WireCreatePipelineAsyncTests.cpp
index ff17f30..c1c20cc 100644
--- a/src/dawn/tests/unittests/wire/WireCreatePipelineAsyncTests.cpp
+++ b/src/dawn/tests/unittests/wire/WireCreatePipelineAsyncTests.cpp
@@ -348,6 +348,7 @@
     ASSERT_TRUE(c2sBuf->Flush());
 
     // Delete the server. It should force async work to complete.
+    c2sBuf->SetHandler(nullptr);
     wireServer.reset();
 
     ASSERT_TRUE(s2cBuf->Flush());
@@ -358,6 +359,8 @@
     wgpuDeviceRelease(device);
     wgpuAdapterRelease(adapter);
     wgpuInstanceRelease(instance);
+
+    s2cBuf->SetHandler(nullptr);
 }
 
 }  // anonymous namespace

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

diff --git a/src/dawn/tests/unittests/wire/WireTest.cpp b/src/dawn/tests/unittests/wire/WireTest.cpp
index d96a4cb..0dc22de 100644
--- a/src/dawn/tests/unittests/wire/WireTest.cpp
+++ b/src/dawn/tests/unittests/wire/WireTest.cpp
@@ -194,6 +194,7 @@
     // Incomplete client callbacks will be called on deletion, so the mocks
     // cannot be null.
     api.IgnoreAllReleaseCalls();
+    mS2cBuf->SetHandler(nullptr);
     mWireClient = nullptr;
 
     if (mWireServer && apiDevice) {
@@ -203,6 +204,7 @@
             .Times(Exactly(1));
         EXPECT_CALL(api, OnDeviceSetLoggingCallback(apiDevice, nullptr, nullptr)).Times(Exactly(1));
     }
+    mC2sBuf->SetHandler(nullptr);
     mWireServer = nullptr;
 }
 
@@ -250,10 +252,12 @@
             .Times(Exactly(1));
         EXPECT_CALL(api, OnDeviceSetLoggingCallback(apiDevice, nullptr, nullptr)).Times(Exactly(1));
     }
+    mC2sBuf->SetHandler(nullptr);
     mWireServer = nullptr;
 }
 
 void WireTest::DeleteClient() {
+    mS2cBuf->SetHandler(nullptr);
     mWireClient = nullptr;
 }
 

src/dawn/utils/TerribleCommandBuffer.h

diff --git a/src/dawn/utils/TerribleCommandBuffer.h b/src/dawn/utils/TerribleCommandBuffer.h
index ad17418..493d941 100644
--- a/src/dawn/utils/TerribleCommandBuffer.h
+++ b/src/dawn/utils/TerribleCommandBuffer.h
@@ -46,8 +46,7 @@
     bool Flush() override;
 
   private:
-    // TODO(https://crbug/dawn/2343): Remove DanglingUntriaged.
-    raw_ptr<dawn::wire::CommandHandler, DanglingUntriaged> mHandler = nullptr;
+    raw_ptr<dawn::wire::CommandHandler> mHandler = nullptr;
     size_t mOffset = 0;
     char mBuffer[1000000];
 };

src/dawn/utils/WireHelper.cpp

diff --git a/src/dawn/utils/WireHelper.cpp b/src/dawn/utils/WireHelper.cpp
index 9e26a34..3214c5a 100644
--- a/src/dawn/utils/WireHelper.cpp
+++ b/src/dawn/utils/WireHelper.cpp
@@ -149,6 +149,11 @@
         dawnProcSetProcs(&dawn::wire::client::GetProcs());
     }
 
+    ~WireHelperProxy() override {
+        mC2sBuf->SetHandler(nullptr);
+        mS2cBuf->SetHandler(nullptr);
+    }
+
     wgpu::Instance RegisterInstance(WGPUInstance backendInstance,
                                     const WGPUInstanceDescriptor* wireDesc) override {
         DAWN_ASSERT(backendInstance != nullptr);