From ed1afa81084ab96e6d7ec1af9111467f7fc26207 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 21 Apr 2021 15:17:02 +0000 Subject: [PATCH] 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 Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez --- src/dawn_wire/client/Buffer.cpp | 32 ++++++++++++++++++++++---------- src/dawn_wire/client/Buffer.h | 2 ++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/dawn_wire/client/Buffer.cpp b/src/dawn_wire/client/Buffer.cpp index 3c7519c6ed..6c9c70e7f6 100644 --- a/src/dawn_wire/client/Buffer.cpp +++ b/src/dawn_wire/client/Buffer.cpp @@ -119,6 +119,8 @@ namespace dawn_wire { namespace client { } } mRequests.clear(); + + FreeMappedData(true); } void Buffer::CancelCallbacksForDisconnect() { @@ -360,15 +362,9 @@ namespace dawn_wire { namespace client { 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 @@ namespace dawn_wire { namespace client { 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 @@ namespace dawn_wire { namespace client { 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 diff --git a/src/dawn_wire/client/Buffer.h b/src/dawn_wire/client/Buffer.h index 5e0d5ec2a3..50b9639f3f 100644 --- a/src/dawn_wire/client/Buffer.h +++ b/src/dawn_wire/client/Buffer.h @@ -57,6 +57,8 @@ namespace dawn_wire { namespace client { 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