commit ed1afa81084ab96e6d7ec1af9111467f7fc26207
author Corentin Wallez <cwallez@chromium.org> Wed Apr 21 15:17:02 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed Apr 21 15:17:02 2021 +0000
tree 2600b80ff6724d850eee777b90a4f59d340c98a3
parent d0e8dc0e922aeb2df7b69e4c299426b79a00f337

client::Buffer: In debug mode, clobber mMappedData when it is freed

This will help detect cases where the mapped data is used after it is
freed, in particular in WebGPU tests around the interaction of mapping
and GC.

Bug: chromium:971949
Change-Id: I820d9885d39379fbc95c6504b9a4151053768d93
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/48382
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_wire/client/Buffer.cpp

diff --git a/src/dawn_wire/client/Buffer.cpp b/src/dawn_wire/client/Buffer.cpp
index 3c7519c..6c9c70e 100644
--- a/src/dawn_wire/client/Buffer.cpp
+++ b/src/dawn_wire/client/Buffer.cpp
@@ -119,6 +119,8 @@
             }
         }
         mRequests.clear();
+
+        FreeMappedData(true);
     }
 
     void Buffer::CancelCallbacksForDisconnect() {
@@ -360,15 +362,9 @@
                     mWriteHandle->SerializeFlush(writeHandleBuffer);
                     return WireResult::Success;
                 });
-            mWriteHandle = nullptr;
-
-        } else if (mReadHandle) {
-            mReadHandle = nullptr;
         }
 
-        mMappedData = nullptr;
-        mMapOffset = 0;
-        mMapSize = 0;
+        FreeMappedData(false);
 
         // Tag all mapping requests still in flight as unmapped before callback.
         for (auto& it : mRequests) {
@@ -384,9 +380,7 @@
 
     void Buffer::Destroy() {
         // Remove the current mapping.
-        mWriteHandle = nullptr;
-        mReadHandle = nullptr;
-        mMappedData = nullptr;
+        FreeMappedData(true);
 
         // Tag all mapping requests still in flight as destroyed before callback.
         for (auto& it : mRequests) {
@@ -420,4 +414,22 @@
         size_t offsetInMappedRange = offset - mMapOffset;
         return offsetInMappedRange <= mMapSize - size;
     }
+
+    void Buffer::FreeMappedData(bool destruction) {
+#if defined(DAWN_ENABLE_ASSERTS)
+        // When in "debug" mode, 0xCA-out the mapped data when we free it so that in we can detect
+        // use-after-free of the mapped data. This is particularly useful for WebGPU test about the
+        // interaction of mapping and GC.
+        if (mMappedData && destruction) {
+            memset(mMappedData, 0xCA, mMapSize);
+        }
+#endif  // defined(DAWN_ENABLE_ASSERTS)
+
+        mMapOffset = 0;
+        mMapSize = 0;
+        mWriteHandle = nullptr;
+        mReadHandle = nullptr;
+        mMappedData = nullptr;
+    }
+
 }}  // namespace dawn_wire::client

src/dawn_wire/client/Buffer.h

diff --git a/src/dawn_wire/client/Buffer.h b/src/dawn_wire/client/Buffer.h
index 5e0d5ec..50b9639 100644
--- a/src/dawn_wire/client/Buffer.h
+++ b/src/dawn_wire/client/Buffer.h
@@ -57,6 +57,8 @@
         bool IsMappedForWriting() const;
         bool CheckGetMappedRangeOffsetSize(size_t offset, size_t size) const;
 
+        void FreeMappedData(bool destruction);
+
         Device* mDevice;
 
         // We want to defer all the validation to the server, which means we could have multiple