dawn_wire: Move BufferConsumer to it's own file. Unify WIRE_TRY

Bug: dawn:680
Change-Id: I6d57280ab11381649deef51ee7babf5ca73f359b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42340
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>

src/dawn_wire/BUILD.gn

@@ -59,6 +59,8 @@ dawn_component("dawn_wire") {
   configs = [ "${dawn_root}/src/common:dawn_internal" ]
   sources = get_target_outputs(":dawn_wire_gen")
   sources += [
+    "BufferConsumer.h",
+    "BufferConsumer_impl.h",
     "ChunkedCommandHandler.cpp",
     "ChunkedCommandHandler.h",
     "ChunkedCommandSerializer.cpp",
@@ -67,6 +69,7 @@ dawn_component("dawn_wire") {
     "WireClient.cpp",
     "WireDeserializeAllocator.cpp",
     "WireDeserializeAllocator.h",
+    "WireResult.h",
     "WireServer.cpp",
     "client/ApiObjects.h",
     "client/Buffer.cpp",

src/dawn_wire/BufferConsumer.h

@@ -0,0 +1,85 @@
+// Copyright 2021 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNWIRE_BUFFERCONSUMER_H_
+#define DAWNWIRE_BUFFERCONSUMER_H_
+
+#include "dawn_wire/WireResult.h"
+
+#include <cstddef>
+
+namespace dawn_wire {
+
+    // BufferConsumer is a utility class that allows reading bytes from a buffer
+    // while simultaneously decrementing the amount of remaining space by exactly
+    // the amount read. It helps prevent bugs where incrementing a pointer and
+    // decrementing a size value are not kept in sync.
+    // BufferConsumer also contains bounds checks to prevent reading out-of-bounds.
+    template <typename BufferT>
+    class BufferConsumer {
+        static_assert(sizeof(BufferT) == 1,
+                      "BufferT must be 1-byte, but may have const/volatile qualifiers.");
+
+      public:
+        BufferConsumer(BufferT* buffer, size_t size) : mBuffer(buffer), mSize(size) {
+        }
+
+        BufferT* Buffer() const {
+            return mBuffer;
+        }
+        size_t AvailableSize() const {
+            return mSize;
+        }
+
+      protected:
+        template <typename T, typename N>
+        WireResult NextN(N count, T** data);
+
+        template <typename T>
+        WireResult Next(T** data);
+
+        template <typename T>
+        WireResult Peek(T** data);
+
+      private:
+        BufferT* mBuffer;
+        size_t mSize;
+    };
+
+    class SerializeBuffer : public BufferConsumer<char> {
+      public:
+        using BufferConsumer::BufferConsumer;
+        using BufferConsumer::Next;
+        using BufferConsumer::NextN;
+    };
+
+    class DeserializeBuffer : public BufferConsumer<const volatile char> {
+      public:
+        using BufferConsumer::BufferConsumer;
+        using BufferConsumer::Peek;
+
+        template <typename T, typename N>
+        WireResult ReadN(N count, const volatile T** data) {
+            return NextN(count, data);
+        }
+
+        template <typename T>
+        WireResult Read(const volatile T** data) {
+            return Next(data);
+        }
+    };
+
+}  // namespace dawn_wire
+
+#endif  // DAWNWIRE_BUFFERCONSUMER_H_

src/dawn_wire/BufferConsumer_impl.h

@@ -0,0 +1,72 @@
+// Copyright 2021 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNWIRE_BUFFERCONSUMER_IMPL_H_
+#define DAWNWIRE_BUFFERCONSUMER_IMPL_H_
+
+#include "dawn_wire/BufferConsumer.h"
+
+#include <limits>
+
+namespace dawn_wire {
+
+    template <typename BufferT>
+    template <typename T>
+    WireResult BufferConsumer<BufferT>::Peek(T** data) {
+        if (sizeof(T) > mSize) {
+            return WireResult::FatalError;
+        }
+
+        *data = reinterpret_cast<T*>(mBuffer);
+        return WireResult::Success;
+    }
+
+    template <typename BufferT>
+    template <typename T>
+    WireResult BufferConsumer<BufferT>::Next(T** data) {
+        if (sizeof(T) > mSize) {
+            return WireResult::FatalError;
+        }
+
+        *data = reinterpret_cast<T*>(mBuffer);
+        mBuffer += sizeof(T);
+        mSize -= sizeof(T);
+        return WireResult::Success;
+    }
+
+    template <typename BufferT>
+    template <typename T, typename N>
+    WireResult BufferConsumer<BufferT>::NextN(N count, T** data) {
+        static_assert(std::is_unsigned<N>::value, "|count| argument of NextN must be unsigned.");
+
+        constexpr size_t kMaxCountWithoutOverflows = std::numeric_limits<size_t>::max() / sizeof(T);
+        if (count > kMaxCountWithoutOverflows) {
+            return WireResult::FatalError;
+        }
+
+        // Cannot overflow because |count| is not greater than |kMaxCountWithoutOverflows|.
+        size_t totalSize = sizeof(T) * count;
+        if (totalSize > mSize) {
+            return WireResult::FatalError;
+        }
+
+        *data = reinterpret_cast<T*>(mBuffer);
+        mBuffer += totalSize;
+        mSize -= totalSize;
+        return WireResult::Success;
+    }
+
+}  // namespace dawn_wire
+
+#endif  // DAWNWIRE_BUFFERCONSUMER_IMPL_H_

src/dawn_wire/CMakeLists.txt

@@ -31,6 +31,8 @@ target_sources(dawn_wire PRIVATE
     "${DAWN_INCLUDE_DIR}/dawn_wire/WireServer.h"
     "${DAWN_INCLUDE_DIR}/dawn_wire/dawn_wire_export.h"
     ${DAWN_WIRE_GEN_SOURCES}
+    "BufferConsumer.h"
+    "BufferConsumer_impl.h"
     "ChunkedCommandHandler.cpp"
     "ChunkedCommandHandler.h"
     "ChunkedCommandSerializer.cpp"
@@ -39,6 +41,7 @@ target_sources(dawn_wire PRIVATE
     "WireClient.cpp"
     "WireDeserializeAllocator.cpp"
     "WireDeserializeAllocator.h"
+    "WireResult.h"
     "WireServer.cpp"
     "client/ApiObjects.h"
     "client/Buffer.cpp"

src/dawn_wire/ChunkedCommandSerializer.h

@@ -32,7 +32,7 @@ namespace dawn_wire {
 
         template <typename Cmd>
         void SerializeCommand(const Cmd& cmd) {
-            SerializeCommand(cmd, 0, [](SerializeBuffer*) { return true; });
+            SerializeCommand(cmd, 0, [](SerializeBuffer*) { return WireResult::Success; });
         }
 
         template <typename Cmd, typename ExtraSizeSerializeFn>
@@ -49,7 +49,8 @@ namespace dawn_wire {
 
         template <typename Cmd>
         void SerializeCommand(const Cmd& cmd, const ObjectIdProvider& objectIdProvider) {
-            SerializeCommand(cmd, objectIdProvider, 0, [](SerializeBuffer*) { return true; });
+            SerializeCommand(cmd, objectIdProvider, 0,
+                             [](SerializeBuffer*) { return WireResult::Success; });
         }
 
         template <typename Cmd, typename ExtraSizeSerializeFn>
@@ -79,10 +80,9 @@ namespace dawn_wire {
                 char* allocatedBuffer = static_cast<char*>(mSerializer->GetCmdSpace(requiredSize));
                 if (allocatedBuffer != nullptr) {
                     SerializeBuffer serializeBuffer(allocatedBuffer, requiredSize);
-                    bool success = true;
-                    success &= SerializeCmd(cmd, requiredSize, &serializeBuffer);
-                    success &= SerializeExtraSize(&serializeBuffer);
-                    if (DAWN_UNLIKELY(!success)) {
+                    WireResult r1 = SerializeCmd(cmd, requiredSize, &serializeBuffer);
+                    WireResult r2 = SerializeExtraSize(&serializeBuffer);
+                    if (DAWN_UNLIKELY(r1 != WireResult::Success || r2 != WireResult::Success)) {
                         mSerializer->OnSerializeError();
                     }
                 }
@@ -94,10 +94,9 @@ namespace dawn_wire {
                 return;
             }
             SerializeBuffer serializeBuffer(cmdSpace.get(), requiredSize);
-            bool success = true;
-            success &= SerializeCmd(cmd, requiredSize, &serializeBuffer);
-            success &= SerializeExtraSize(&serializeBuffer);
-            if (DAWN_UNLIKELY(!success)) {
+            WireResult r1 = SerializeCmd(cmd, requiredSize, &serializeBuffer);
+            WireResult r2 = SerializeExtraSize(&serializeBuffer);
+            if (DAWN_UNLIKELY(r1 != WireResult::Success || r2 != WireResult::Success)) {
                 mSerializer->OnSerializeError();
                 return;
             }

src/dawn_wire/WireResult.h

@@ -0,0 +1,38 @@
+// Copyright 2021 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNWIRE_WIRERESULT_H_
+#define DAWNWIRE_WIRERESULT_H_
+
+#include "common/Compiler.h"
+
+namespace dawn_wire {
+
+    enum DAWN_NO_DISCARD class WireResult {
+        Success,
+        FatalError,
+    };
+
+// Macro to simplify error handling, similar to DAWN_TRY but for WireResult.
+#define WIRE_TRY(EXPR)                                          \
+    do {                                                        \
+        WireResult exprResult = EXPR;                           \
+        if (DAWN_UNLIKELY(exprResult != WireResult::Success)) { \
+            return exprResult;                                  \
+        }                                                       \
+    } while (0)
+
+}  // namespace dawn_wire
+
+#endif  // DAWNWIRE_WIRERESULT_H_

src/dawn_wire/client/Buffer.cpp

@@ -14,6 +14,7 @@
 
 #include "dawn_wire/client/Buffer.h"
 
+#include "dawn_wire/BufferConsumer_impl.h"
 #include "dawn_wire/WireCmd_autogen.h"
 #include "dawn_wire/client/Client.h"
 #include "dawn_wire/client/Device.h"
@@ -77,11 +78,11 @@ namespace dawn_wire { namespace client {
         wireClient->SerializeCommand(
             cmd, writeHandleCreateInfoLength, [&](SerializeBuffer* serializeBuffer) {
                 if (descriptor->mappedAtCreation) {
-                    if (serializeBuffer->AvailableSize() != writeHandleCreateInfoLength) {
-                        return false;
-                    }
+                    char* writeHandleBuffer;
+                    WIRE_TRY(
+                        serializeBuffer->NextN(writeHandleCreateInfoLength, &writeHandleBuffer));
                     // Serialize the WriteHandle into the space after the command.
-                    writeHandle->SerializeCreate(serializeBuffer->Buffer());
+                    writeHandle->SerializeCreate(writeHandleBuffer);
 
                     // Set the buffer state for the mapping at creation. The buffer now owns the
                     // write handle..
@@ -90,7 +91,7 @@ namespace dawn_wire { namespace client {
                     buffer->mMapOffset = 0;
                     buffer->mMapSize = buffer->mSize;
                 }
-                return true;
+                return WireResult::Success;
             });
         return ToAPI(buffer);
     }
@@ -207,22 +208,21 @@ namespace dawn_wire { namespace client {
             cmd.handleCreateInfoLength = request.readHandle->SerializeCreateSize();
             client->SerializeCommand(
                 cmd, cmd.handleCreateInfoLength, [&](SerializeBuffer* serializeBuffer) {
-                    bool success = serializeBuffer->AvailableSize() == cmd.handleCreateInfoLength;
-                    if (success) {
-                        request.readHandle->SerializeCreate(serializeBuffer->Buffer());
-                    }
-                    return success;
+                    char* readHandleBuffer;
+                    WIRE_TRY(serializeBuffer->NextN(cmd.handleCreateInfoLength, &readHandleBuffer));
+                    request.readHandle->SerializeCreate(readHandleBuffer);
+                    return WireResult::Success;
                 });
         } else {
             ASSERT(isWriteMode);
             cmd.handleCreateInfoLength = request.writeHandle->SerializeCreateSize();
             client->SerializeCommand(
                 cmd, cmd.handleCreateInfoLength, [&](SerializeBuffer* serializeBuffer) {
-                    bool success = serializeBuffer->AvailableSize() == cmd.handleCreateInfoLength;
-                    if (success) {
-                        request.writeHandle->SerializeCreate(serializeBuffer->Buffer());
-                    }
-                    return success;
+                    char* writeHandleBuffer;
+                    WIRE_TRY(
+                        serializeBuffer->NextN(cmd.handleCreateInfoLength, &writeHandleBuffer));
+                    request.writeHandle->SerializeCreate(writeHandleBuffer);
+                    return WireResult::Success;
                 });
         }
 
@@ -352,13 +352,13 @@ namespace dawn_wire { namespace client {
 
             client->SerializeCommand(
                 cmd, writeFlushInfoLength, [&](SerializeBuffer* serializeBuffer) {
-                    bool success = serializeBuffer->AvailableSize() == writeFlushInfoLength;
-                    if (success) {
-                        // Serialize flush metadata into the space after the command.
-                        // This closes the handle for writing.
-                        mWriteHandle->SerializeFlush(serializeBuffer->Buffer());
-                    }
-                    return success;
+                    char* writeHandleBuffer;
+                    WIRE_TRY(serializeBuffer->NextN(writeFlushInfoLength, &writeHandleBuffer));
+
+                    // Serialize flush metadata into the space after the command.
+                    // This closes the handle for writing.
+                    mWriteHandle->SerializeFlush(writeHandleBuffer);
+                    return WireResult::Success;
                 });
             mWriteHandle = nullptr;
 

src/dawn_wire/server/ServerBuffer.cpp

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "common/Assert.h"
+#include "dawn_wire/BufferConsumer_impl.h"
 #include "dawn_wire/WireCmd_autogen.h"
 #include "dawn_wire/server/Server.h"
 
@@ -251,12 +252,12 @@ namespace dawn_wire { namespace server {
         SerializeCommand(cmd, cmd.readInitialDataInfoLength, [&](SerializeBuffer* serializeBuffer) {
             if (isSuccess) {
                 if (isRead) {
-                    if (serializeBuffer->AvailableSize() != cmd.readInitialDataInfoLength) {
-                        return false;
-                    }
+                    char* readHandleBuffer;
+                    WIRE_TRY(
+                        serializeBuffer->NextN(cmd.readInitialDataInfoLength, &readHandleBuffer));
+
                     // Serialize the initialization message into the space after the command.
-                    data->readHandle->SerializeInitialData(readData, data->size,
-                                                           serializeBuffer->Buffer());
+                    data->readHandle->SerializeInitialData(readData, data->size, readHandleBuffer);
                     // The in-flight map request returned successfully.
                     // Move the ReadHandle so it is owned by the buffer.
                     bufferData->readHandle = std::move(data->readHandle);
@@ -271,7 +272,7 @@ namespace dawn_wire { namespace server {
                         data->size);
                 }
             }
-            return true;
+            return WireResult::Success;
         });
     }