commit f5b3f644188d256f38bae87aebf6574e7da8f945
author Jiawei Shao <jiawei.shao@intel.com> Wed Mar 20 05:09:26 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 20 05:09:26 2024 +0000
tree 0a12d3681d17588a1e4f0c228947cb982e458dd1
parent 70dfde5147a935c5e077ac008e32c0f23d237b3c

Fix dangling pointer in IndirectDrawMetadata

In `IndirectDrawMetadata` (used in `RenderEncoderBase`) all the
`IndirectDraw` structs must be cleared before the `EncodingContext`
object is destroyed (in `RenderPassEncoder` and `RenderBundleEncoder`).
Otherwise any `IndirectDraw.cmd` will become a dangling pointer.

Bug: dawn:2349
Change-Id: I020e516e0d93f055f406f6ca10105269534c88d7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/179842
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn/native/IndirectDrawMetadata.cpp

diff --git a/src/dawn/native/IndirectDrawMetadata.cpp b/src/dawn/native/IndirectDrawMetadata.cpp
index 36ff8dc..1ef6980 100644
--- a/src/dawn/native/IndirectDrawMetadata.cpp
+++ b/src/dawn/native/IndirectDrawMetadata.cpp
@@ -233,6 +233,10 @@
     it->second.AddIndirectDraw(mMaxDrawCallsPerBatch, mMaxBatchOffsetRange, draw);
 }
 
+void IndirectDrawMetadata::ClearIndexedIndirectBufferValidationInfo() {
+    mIndexedIndirectBufferValidationInfo.clear();
+}
+
 bool IndirectDrawMetadata::IndexedIndirectConfig::operator<(
     const IndexedIndirectConfig& other) const {
     return std::tie(inputIndirectBufferPtr, duplicateBaseVertexInstance, drawType) <

src/dawn/native/IndirectDrawMetadata.h

diff --git a/src/dawn/native/IndirectDrawMetadata.h b/src/dawn/native/IndirectDrawMetadata.h
index 6046c25..d86cb9b 100644
--- a/src/dawn/native/IndirectDrawMetadata.h
+++ b/src/dawn/native/IndirectDrawMetadata.h
@@ -63,8 +63,7 @@
         // This is a pointer to the command that should be populated with the validated
         // indirect scratch buffer. It is only valid up until the encoded command buffer
         // is submitted.
-        // TODO(https://crbug.com/dawn/2349): Investigate DanglingUntriaged in dawn/native.
-        raw_ptr<DrawIndirectCmd, DanglingUntriaged> cmd;
+        raw_ptr<DrawIndirectCmd> cmd;
     };
 
     struct IndirectValidationBatch {
@@ -150,6 +149,8 @@
                          bool duplicateBaseVertexInstance,
                          DrawIndirectCmd* cmd);
 
+    void ClearIndexedIndirectBufferValidationInfo();
+
   private:
     IndexedIndirectBufferValidationInfoMap mIndexedIndirectBufferValidationInfo;
     absl::flat_hash_set<RenderBundleBase*> mAddedBundles;

src/dawn/native/RenderBundle.cpp

diff --git a/src/dawn/native/RenderBundle.cpp b/src/dawn/native/RenderBundle.cpp
index 7b70502..5e59e97 100644
--- a/src/dawn/native/RenderBundle.cpp
+++ b/src/dawn/native/RenderBundle.cpp
@@ -58,6 +58,7 @@
 }
 
 void RenderBundleBase::DestroyImpl() {
+    mIndirectDrawMetadata.ClearIndexedIndirectBufferValidationInfo();
     FreeCommands(&mCommands);
 
     // Remove reference to the attachment state so that we don't have lingering references to

src/dawn/native/RenderBundleEncoder.cpp

diff --git a/src/dawn/native/RenderBundleEncoder.cpp b/src/dawn/native/RenderBundleEncoder.cpp
index cd203e0..ddc9026 100644
--- a/src/dawn/native/RenderBundleEncoder.cpp
+++ b/src/dawn/native/RenderBundleEncoder.cpp
@@ -123,6 +123,7 @@
 }
 
 void RenderBundleEncoder::DestroyImpl() {
+    mIndirectDrawMetadata.ClearIndexedIndirectBufferValidationInfo();
     mCommandBufferState.End();
     RenderEncoderBase::DestroyImpl();
     mBundleEncodingContext.Destroy();

src/dawn/native/RenderPassEncoder.cpp

diff --git a/src/dawn/native/RenderPassEncoder.cpp b/src/dawn/native/RenderPassEncoder.cpp
index 2571a04..5740c9a 100644
--- a/src/dawn/native/RenderPassEncoder.cpp
+++ b/src/dawn/native/RenderPassEncoder.cpp
@@ -133,6 +133,7 @@
 }
 
 void RenderPassEncoder::DestroyImpl() {
+    mIndirectDrawMetadata.ClearIndexedIndirectBufferValidationInfo();
     mCommandBufferState.End();
 
     RenderEncoderBase::DestroyImpl();