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 <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/generator/templates/dawn_wire/WireCmd.cpp b/generator/templates/dawn_wire/WireCmd.cpp
index f9c6a44..ab28047 100644
--- a/generator/templates/dawn_wire/WireCmd.cpp
+++ b/generator/templates/dawn_wire/WireCmd.cpp
@@ -16,6 +16,7 @@
#include "common/Assert.h"
+#include <algorithm>
#include <cstring>
#include <limits>
@@ -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<const {{member_transfer_type(member)}}*>(buffer);
+ auto memberBuffer = reinterpret_cast<const volatile {{member_transfer_type(member)}}*>(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 @@
} \
}
+ 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 <typename T>
- 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<size_t>::max() / sizeof(T);
if (count > kMaxCountWithoutOverflows) {
return DeserializeResult::FatalError;
@@ -380,7 +392,7 @@
return DeserializeResult::FatalError;
}
- *data = reinterpret_cast<const T*>(*buffer);
+ *data = reinterpret_cast<const volatile T*>(*buffer);
*buffer += totalSize;
*size -= totalSize;
diff --git a/generator/templates/dawn_wire/WireCmd.h b/generator/templates/dawn_wire/WireCmd.h
index ad3a839..83490e6 100644
--- a/generator/templates/dawn_wire/WireCmd.h
+++ b/generator/templates/dawn_wire/WireCmd.h
@@ -24,6 +24,12 @@
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 @@
//* 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 60eb9ee..f275b33 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 @@
}
{% 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<const ReturnWireCmd*>(commands);
+ ReturnWireCmd cmdId = *reinterpret_cast<const volatile ReturnWireCmd*>(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 b29f68b..df18896 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 4a99bb6..0594c8c 100644
--- a/generator/templates/dawn_wire/server/ServerHandlers.cpp
+++ b/generator/templates/dawn_wire/server/ServerHandlers.cpp
@@ -25,7 +25,7 @@
{% 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 @@
{% 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<const WireCmd*>(commands);
+ WireCmd cmdId = *reinterpret_cast<const volatile WireCmd*>(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 03b7b4a..caa699d 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 f7b9491..0cbbcd6 100644
--- a/src/dawn_wire/WireClient.cpp
+++ b/src/dawn_wire/WireClient.cpp
@@ -33,7 +33,7 @@
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 45ef9ca..1896647 100644
--- a/src/dawn_wire/WireServer.cpp
+++ b/src/dawn_wire/WireServer.cpp
@@ -28,7 +28,7 @@
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 f7b0687..c1af427 100644
--- a/src/dawn_wire/client/Client.h
+++ b/src/dawn_wire/client/Client.h
@@ -33,7 +33,7 @@
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 5e9c8b6..0f901ad 100644
--- a/src/dawn_wire/server/Server.h
+++ b/src/dawn_wire/server/Server.h
@@ -53,7 +53,7 @@
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 b5ee54d..7d60c31 100644
--- a/src/include/dawn_wire/Wire.h
+++ b/src/include/dawn_wire/Wire.h
@@ -32,7 +32,7 @@
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 458a593..15e5666 100644
--- a/src/include/dawn_wire/WireClient.h
+++ b/src/include/dawn_wire/WireClient.h
@@ -44,7 +44,8 @@
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 f5ae1dc..0501a44 100644
--- a/src/include/dawn_wire/WireServer.h
+++ b/src/include/dawn_wire/WireServer.h
@@ -38,7 +38,8 @@
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);
