From b639e68495ae732faaa81d5ed85584f39ff6e580 Mon Sep 17 00:00:00 2001
From: Yan <shaobo.yan@intel.com>
Date: Fri, 15 Nov 2019 09:32:56 +0000
Subject: [PATCH] Add large buffer to handle super large data block commands

TerribleCommandBuffer has space for 10,000,000 bytes worth of commands. If
commands contain super large data block (e.g. setsubdata), it will
return nullptr and crash dawn wire layer.

This patch adds a large buffer to handle super large data block.

BUG=dawn:251

Change-Id: Ib007e92b5282afbb93aef63cfffe5a3965f6d5c1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/13040
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
---
 src/tests/end2end/BufferTests.cpp   | 19 ++++++++++
 src/utils/TerribleCommandBuffer.cpp | 57 ++++++++++++++++++++++++++---
 src/utils/TerribleCommandBuffer.h   |  6 ++-
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp
index d90528adac..c29e34fefd 100644
--- a/src/tests/end2end/BufferTests.cpp
+++ b/src/tests/end2end/BufferTests.cpp
@@ -256,6 +256,25 @@ TEST_P(BufferSetSubDataTests, LargeSetSubData) {
     EXPECT_BUFFER_U32_RANGE_EQ(expectedData.data(), buffer, 0, kElements);
 }
 
+// Test using SetSubData for super large data block
+TEST_P(BufferSetSubDataTests, SuperLargeSetSubData) {
+    constexpr uint64_t kSize = 12000 * 1000;
+    constexpr uint64_t kElements = 3000 * 1000;
+    wgpu::BufferDescriptor descriptor;
+    descriptor.size = kSize;
+    descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
+    wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+
+    std::vector<uint32_t> expectedData;
+    for (uint32_t i = 0; i < kElements; ++i) {
+        expectedData.push_back(i);
+    }
+
+    buffer.SetSubData(0, kElements * sizeof(uint32_t), expectedData.data());
+
+    EXPECT_BUFFER_U32_RANGE_EQ(expectedData.data(), buffer, 0, kElements);
+}
+
 DAWN_INSTANTIATE_TEST(BufferSetSubDataTests,
                      D3D12Backend,
                      MetalBackend,
diff --git a/src/utils/TerribleCommandBuffer.cpp b/src/utils/TerribleCommandBuffer.cpp
index 77f86ec4b2..aa0bc8ca28 100644
--- a/src/utils/TerribleCommandBuffer.cpp
+++ b/src/utils/TerribleCommandBuffer.cpp
@@ -34,26 +34,71 @@ namespace utils {
         //   (Here and/or in the caller?) It might be good to make the wire receiver get a nullptr
         //   instead of pointer to zero-sized allocation in mBuffer.
 
+        // Cannot have commands in mBuffer and mLargeBuffer at same time.
+        ASSERT(mOffset == 0 || mLargeBufferCmdSize == 0);
+
         if (size > sizeof(mBuffer)) {
-            return nullptr;
+            // Flush current cmds in mBuffer to keep order.
+            if (mOffset > 0) {
+                if (!Flush()) {
+                    return nullptr;
+                }
+                return GetCmdSpace(size);
+            }
+
+            // Resize large buffer to the size that can
+            // contain incoming command if needed.
+            if (mLargeBuffer.size() < size) {
+                mLargeBuffer.resize(size);
+            }
+
+            // Record whole cmd space.
+            mLargeBufferCmdSize = size;
+
+            return mLargeBuffer.data();
         }
 
-        char* result = &mBuffer[mOffset];
-        mOffset += size;
-
-        if (mOffset > sizeof(mBuffer)) {
+        // Trigger flush if large buffer contain cmds.
+        if (mLargeBufferCmdSize > 0) {
             if (!Flush()) {
                 return nullptr;
             }
             return GetCmdSpace(size);
         }
 
+        // Need to flush large buffer first.
+        ASSERT(mLargeBufferCmdSize == 0);
+
+        char* result = &mBuffer[mOffset];
+
+        if (sizeof(mBuffer) - size < mOffset) {
+            if (!Flush()) {
+                return nullptr;
+            }
+            return GetCmdSpace(size);
+        }
+
+        mOffset += size;
+
         return result;
     }
 
     bool TerribleCommandBuffer::Flush() {
-        bool success = mHandler->HandleCommands(mBuffer, mOffset) != nullptr;
+        // Cannot have commands in mBuffer and mLargeBuffer at same time.
+        ASSERT(mOffset == 0 || mLargeBufferCmdSize == 0);
+
+        bool success = false;
+        // Big buffer not empty, flush it!
+        if (mLargeBufferCmdSize > 0) {
+            success = mHandler->HandleCommands(mLargeBuffer.data(), mLargeBufferCmdSize) != nullptr;
+            // Clear big command buffers.
+            mLargeBufferCmdSize = 0;
+            return success;
+        }
+
+        success = mHandler->HandleCommands(mBuffer, mOffset) != nullptr;
         mOffset = 0;
+
         return success;
     }
 
diff --git a/src/utils/TerribleCommandBuffer.h b/src/utils/TerribleCommandBuffer.h
index b5affc8553..9a41bb3611 100644
--- a/src/utils/TerribleCommandBuffer.h
+++ b/src/utils/TerribleCommandBuffer.h
@@ -34,7 +34,11 @@ namespace utils {
       private:
         dawn_wire::CommandHandler* mHandler = nullptr;
         size_t mOffset = 0;
-        char mBuffer[10000000];
+        // Cannot have commands in mBuffer and mLargeBuffer
+        // at the same time to ensure commands order.
+        char mBuffer[1000000];
+        std::vector<char> mLargeBuffer;
+        size_t mLargeBufferCmdSize = 0;
     };
 
 }  // namespace utils