From 8eb8385e2efc6b4cc5e1d77fa888a64d9d6b0511 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Mon, 7 Oct 2019 20:38:47 +0000 Subject: [PATCH] dawn_wire: Tag deserialize commands with volatile pointer This prevents bugs where the compiler assumes a piece of memory will be the same if read from twice. Bug: dawn:230 Change-Id: Ib3358e56b6cf8f1fbf449c5d564ef85c969d695b Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11840 Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez Commit-Queue: Austin Eng --- generator/templates/dawn_wire/WireCmd.cpp | 34 +++++++++++++------ generator/templates/dawn_wire/WireCmd.h | 8 ++++- .../dawn_wire/client/ClientHandlers.cpp | 6 ++-- .../dawn_wire/client/ClientPrototypes.inc | 2 +- .../dawn_wire/server/ServerHandlers.cpp | 6 ++-- .../dawn_wire/server/ServerPrototypes.inc | 2 +- src/dawn_wire/WireClient.cpp | 2 +- src/dawn_wire/WireServer.cpp | 2 +- src/dawn_wire/client/Client.h | 2 +- src/dawn_wire/server/Server.h | 2 +- src/include/dawn_wire/Wire.h | 2 +- src/include/dawn_wire/WireClient.h | 3 +- src/include/dawn_wire/WireServer.h | 3 +- 13 files changed, 47 insertions(+), 27 deletions(-) diff --git a/generator/templates/dawn_wire/WireCmd.cpp b/generator/templates/dawn_wire/WireCmd.cpp index f9c6a448c9..ab28047cb7 100644 --- a/generator/templates/dawn_wire/WireCmd.cpp +++ b/generator/templates/dawn_wire/WireCmd.cpp @@ -16,6 +16,7 @@ #include "common/Assert.h" +#include #include #include @@ -225,8 +226,8 @@ //* Deserializes `transfer` into `record` getting more serialized data from `buffer` and `size` //* if needed, using `allocator` to store pointed-to values and `resolver` to translate object //* Ids to actual objects. - DAWN_DECLARE_UNUSED DeserializeResult {{Return}}{{name}}Deserialize({{Return}}{{name}}{{Cmd}}* record, const {{Return}}{{name}}Transfer* transfer, - const char** buffer, size_t* size, DeserializeAllocator* allocator + DAWN_DECLARE_UNUSED DeserializeResult {{Return}}{{name}}Deserialize({{Return}}{{name}}{{Cmd}}* record, const volatile {{Return}}{{name}}Transfer* transfer, + const volatile char** buffer, size_t* size, DeserializeAllocator* allocator {%- if record.has_dawn_object -%} , const ObjectIdResolver& resolver {%- endif -%} @@ -257,18 +258,19 @@ {% for member in members if member.length == "strlen" %} {% set memberName = as_varName(member.name) %} - record->{{memberName}} = nullptr; {% if member.optional %} - if (transfer->has_{{memberName}}) + bool has_{{memberName}} = transfer->has_{{memberName}}; + record->{{memberName}} = nullptr; + if (has_{{memberName}}) {% endif %} { size_t stringLength = transfer->{{memberName}}Strlen; - const char* stringInBuffer = nullptr; + const volatile char* stringInBuffer = nullptr; DESERIALIZE_TRY(GetPtrFromBuffer(buffer, size, stringLength, &stringInBuffer)); char* copiedString = nullptr; DESERIALIZE_TRY(GetSpace(allocator, stringLength + 1, &copiedString)); - memcpy(copiedString, stringInBuffer, stringLength); + std::copy(stringInBuffer, stringInBuffer + stringLength, copiedString); copiedString[stringLength] = '\0'; record->{{memberName}} = copiedString; } @@ -285,7 +287,7 @@ {% endif %} { size_t memberLength = {{member_length(member, "record->")}}; - auto memberBuffer = reinterpret_cast(buffer); + auto memberBuffer = reinterpret_cast(buffer); DESERIALIZE_TRY(GetPtrFromBuffer(buffer, size, memberLength, &memberBuffer)); {{as_cType(member.type.name)}}* copiedMembers = nullptr; @@ -337,12 +339,12 @@ ); } - DeserializeResult {{Cmd}}::Deserialize(const char** buffer, size_t* size, DeserializeAllocator* allocator + DeserializeResult {{Cmd}}::Deserialize(const volatile char** buffer, size_t* size, DeserializeAllocator* allocator {%- if command.has_dawn_object -%} , const ObjectIdResolver& resolver {%- endif -%} ) { - const {{Name}}Transfer* transfer = nullptr; + const volatile {{Name}}Transfer* transfer = nullptr; DESERIALIZE_TRY(GetPtrFromBuffer(buffer, size, 1, &transfer)); return {{Name}}Deserialize(this, transfer, buffer, size, allocator @@ -364,12 +366,22 @@ namespace dawn_wire { } \ } + ObjectHandle::ObjectHandle() = default; + ObjectHandle::ObjectHandle(ObjectId id, ObjectSerial serial) : id(id), serial(serial) {} + ObjectHandle::ObjectHandle(const volatile ObjectHandle& rhs) : id(rhs.id), serial(rhs.serial) {} + ObjectHandle& ObjectHandle::operator=(const ObjectHandle& rhs) = default; + ObjectHandle& ObjectHandle::operator=(const volatile ObjectHandle& rhs) { + id = rhs.id; + serial = rhs.serial; + return *this; + } + namespace { // Consumes from (buffer, size) enough memory to contain T[count] and return it in data. // Returns FatalError if not enough memory was available template - DeserializeResult GetPtrFromBuffer(const char** buffer, size_t* size, size_t count, const T** data) { + DeserializeResult GetPtrFromBuffer(const volatile char** buffer, size_t* size, size_t count, const volatile T** data) { constexpr size_t kMaxCountWithoutOverflows = std::numeric_limits::max() / sizeof(T); if (count > kMaxCountWithoutOverflows) { return DeserializeResult::FatalError; @@ -380,7 +392,7 @@ namespace dawn_wire { return DeserializeResult::FatalError; } - *data = reinterpret_cast(*buffer); + *data = reinterpret_cast(*buffer); *buffer += totalSize; *size -= totalSize; diff --git a/generator/templates/dawn_wire/WireCmd.h b/generator/templates/dawn_wire/WireCmd.h index ad3a839e98..83490e6772 100644 --- a/generator/templates/dawn_wire/WireCmd.h +++ b/generator/templates/dawn_wire/WireCmd.h @@ -24,6 +24,12 @@ namespace dawn_wire { struct ObjectHandle { ObjectId id; ObjectSerial serial; + + ObjectHandle(); + ObjectHandle(ObjectId id, ObjectSerial serial); + ObjectHandle(const volatile ObjectHandle& rhs); + ObjectHandle& operator=(const ObjectHandle& rhs); + ObjectHandle& operator=(const volatile ObjectHandle& rhs); }; enum class DeserializeResult { @@ -99,7 +105,7 @@ namespace dawn_wire { //* Deserialize returns: //* - Success if everything went well (yay!) //* - FatalError is something bad happened (buffer too small for example) - DeserializeResult Deserialize(const char** buffer, size_t* size, DeserializeAllocator* allocator + DeserializeResult Deserialize(const volatile char** buffer, size_t* size, DeserializeAllocator* allocator {%- if command.has_dawn_object -%} , const ObjectIdResolver& resolver {%- endif -%} diff --git a/generator/templates/dawn_wire/client/ClientHandlers.cpp b/generator/templates/dawn_wire/client/ClientHandlers.cpp index 60eb9ee1f7..f275b33d92 100644 --- a/generator/templates/dawn_wire/client/ClientHandlers.cpp +++ b/generator/templates/dawn_wire/client/ClientHandlers.cpp @@ -19,7 +19,7 @@ namespace dawn_wire { namespace client { {% for command in cmd_records["return command"] %} - bool Client::Handle{{command.name.CamelCase()}}(const char** commands, size_t* size) { + bool Client::Handle{{command.name.CamelCase()}}(const volatile char** commands, size_t* size) { Return{{command.name.CamelCase()}}Cmd cmd; DeserializeResult deserializeResult = cmd.Deserialize(commands, size, &mAllocator); @@ -53,9 +53,9 @@ namespace dawn_wire { namespace client { } {% endfor %} - const char* Client::HandleCommands(const char* commands, size_t size) { + const volatile char* Client::HandleCommands(const volatile char* commands, size_t size) { while (size >= sizeof(ReturnWireCmd)) { - ReturnWireCmd cmdId = *reinterpret_cast(commands); + ReturnWireCmd cmdId = *reinterpret_cast(commands); bool success = false; switch (cmdId) { diff --git a/generator/templates/dawn_wire/client/ClientPrototypes.inc b/generator/templates/dawn_wire/client/ClientPrototypes.inc index b29f68bded..df18896587 100644 --- a/generator/templates/dawn_wire/client/ClientPrototypes.inc +++ b/generator/templates/dawn_wire/client/ClientPrototypes.inc @@ -14,7 +14,7 @@ //* Return command handlers {% for command in cmd_records["return command"] %} - bool Handle{{command.name.CamelCase()}}(const char** commands, size_t* size); + bool Handle{{command.name.CamelCase()}}(const volatile char** commands, size_t* size); {% endfor %} //* Return command doers diff --git a/generator/templates/dawn_wire/server/ServerHandlers.cpp b/generator/templates/dawn_wire/server/ServerHandlers.cpp index 4a99bb66a1..0594c8c4bc 100644 --- a/generator/templates/dawn_wire/server/ServerHandlers.cpp +++ b/generator/templates/dawn_wire/server/ServerHandlers.cpp @@ -25,7 +25,7 @@ namespace dawn_wire { namespace server { {% set Suffix = command.name.CamelCase() %} {% if Suffix not in client_side_commands %} //* The generic command handlers - bool Server::Handle{{Suffix}}(const char** commands, size_t* size) { + bool Server::Handle{{Suffix}}(const volatile char** commands, size_t* size) { {{Suffix}}Cmd cmd; DeserializeResult deserializeResult = cmd.Deserialize(commands, size, &mAllocator {%- if command.has_dawn_object -%} @@ -91,11 +91,11 @@ namespace dawn_wire { namespace server { {% endif %} {% endfor %} - const char* Server::HandleCommands(const char* commands, size_t size) { + const volatile char* Server::HandleCommands(const volatile char* commands, size_t size) { mProcs.deviceTick(DeviceObjects().Get(1)->handle); while (size >= sizeof(WireCmd)) { - WireCmd cmdId = *reinterpret_cast(commands); + WireCmd cmdId = *reinterpret_cast(commands); bool success = false; switch (cmdId) { diff --git a/generator/templates/dawn_wire/server/ServerPrototypes.inc b/generator/templates/dawn_wire/server/ServerPrototypes.inc index 03b7b4aa7b..caa699d5db 100644 --- a/generator/templates/dawn_wire/server/ServerPrototypes.inc +++ b/generator/templates/dawn_wire/server/ServerPrototypes.inc @@ -15,7 +15,7 @@ // Command handlers & doers {% for command in cmd_records["command"] if command.name.CamelCase() not in client_side_commands %} {% set Suffix = command.name.CamelCase() %} - bool Handle{{Suffix}}(const char** commands, size_t* size); + bool Handle{{Suffix}}(const volatile char** commands, size_t* size); bool Do{{Suffix}}( {%- for member in command.members -%} diff --git a/src/dawn_wire/WireClient.cpp b/src/dawn_wire/WireClient.cpp index f7b949122f..0cbbcd6c1e 100644 --- a/src/dawn_wire/WireClient.cpp +++ b/src/dawn_wire/WireClient.cpp @@ -33,7 +33,7 @@ namespace dawn_wire { return client::GetProcs(); } - const char* WireClient::HandleCommands(const char* commands, size_t size) { + const volatile char* WireClient::HandleCommands(const volatile char* commands, size_t size) { return mImpl->HandleCommands(commands, size); } diff --git a/src/dawn_wire/WireServer.cpp b/src/dawn_wire/WireServer.cpp index 45ef9ca4b5..1896647126 100644 --- a/src/dawn_wire/WireServer.cpp +++ b/src/dawn_wire/WireServer.cpp @@ -28,7 +28,7 @@ namespace dawn_wire { mImpl.reset(); } - const char* WireServer::HandleCommands(const char* commands, size_t size) { + const volatile char* WireServer::HandleCommands(const volatile char* commands, size_t size) { return mImpl->HandleCommands(commands, size); } diff --git a/src/dawn_wire/client/Client.h b/src/dawn_wire/client/Client.h index f7b06871f2..c1af427633 100644 --- a/src/dawn_wire/client/Client.h +++ b/src/dawn_wire/client/Client.h @@ -33,7 +33,7 @@ namespace dawn_wire { namespace client { Client(CommandSerializer* serializer, MemoryTransferService* memoryTransferService); ~Client(); - const char* HandleCommands(const char* commands, size_t size); + const volatile char* HandleCommands(const volatile char* commands, size_t size); ReservedTexture ReserveTexture(DawnDevice device); void* GetCmdSpace(size_t size) { diff --git a/src/dawn_wire/server/Server.h b/src/dawn_wire/server/Server.h index 5e9c8b608a..0f901ad2fc 100644 --- a/src/dawn_wire/server/Server.h +++ b/src/dawn_wire/server/Server.h @@ -53,7 +53,7 @@ namespace dawn_wire { namespace server { MemoryTransferService* memoryTransferService); ~Server(); - const char* HandleCommands(const char* commands, size_t size); + const volatile char* HandleCommands(const volatile char* commands, size_t size); bool InjectTexture(DawnTexture texture, uint32_t id, uint32_t generation); diff --git a/src/include/dawn_wire/Wire.h b/src/include/dawn_wire/Wire.h index b5ee54d15c..7d60c31a5c 100644 --- a/src/include/dawn_wire/Wire.h +++ b/src/include/dawn_wire/Wire.h @@ -32,7 +32,7 @@ namespace dawn_wire { class DAWN_WIRE_EXPORT CommandHandler { public: virtual ~CommandHandler() = default; - virtual const char* HandleCommands(const char* commands, size_t size) = 0; + virtual const volatile char* HandleCommands(const volatile char* commands, size_t size) = 0; }; } // namespace dawn_wire diff --git a/src/include/dawn_wire/WireClient.h b/src/include/dawn_wire/WireClient.h index 458a5930c6..15e56665b8 100644 --- a/src/include/dawn_wire/WireClient.h +++ b/src/include/dawn_wire/WireClient.h @@ -44,7 +44,8 @@ namespace dawn_wire { DawnDevice GetDevice() const; DawnProcTable GetProcs() const; - const char* HandleCommands(const char* commands, size_t size) override final; + const volatile char* HandleCommands(const volatile char* commands, + size_t size) override final; ReservedTexture ReserveTexture(DawnDevice device); diff --git a/src/include/dawn_wire/WireServer.h b/src/include/dawn_wire/WireServer.h index f5ae1dcf40..0501a44e6b 100644 --- a/src/include/dawn_wire/WireServer.h +++ b/src/include/dawn_wire/WireServer.h @@ -38,7 +38,8 @@ namespace dawn_wire { WireServer(const WireServerDescriptor& descriptor); ~WireServer(); - const char* HandleCommands(const char* commands, size_t size) override final; + const volatile char* HandleCommands(const volatile char* commands, + size_t size) override final; bool InjectTexture(DawnTexture texture, uint32_t id, uint32_t generation);