dawn_wire: Directly use the data in deserialized buffer when possible
This patch tries to use the data directly in deserialized buffer
when the data is "data-only" and doesn't affect the control flow instead
of allocating and copying into a temporary buffer. Due to the protection
of TOCTOU attacks, currently we only set "wire_is_data_only" on the
parameter "data" in Queue.WriteBuffer() and Queue.WriteTexture().
With this patch, the performance of dawn_perf_tests
BufferUploadPerf.Run/*_WriteBuffer_BufferSize_* with "-w" will
be greatly improved (~20%) when the upload buffer size is greater than
1MB.
BUG=chromium:1266727
Change-Id: I7a9d54c9b505975235ee37aa72ee97f082ad3aa3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/72063
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/dawn_wire.json b/dawn_wire.json
index 5742573..41abdd8 100644
--- a/dawn_wire.json
+++ b/dawn_wire.json
@@ -70,13 +70,13 @@
{"name": "queue id", "type": "ObjectId" },
{"name": "buffer id", "type": "ObjectId" },
{"name": "buffer offset", "type": "uint64_t"},
- {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "size"},
+ {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "size", "wire_is_data_only": true},
{"name": "size", "type": "uint64_t"}
],
"queue write texture": [
{"name": "queue id", "type": "ObjectId" },
{"name": "destination", "type": "image copy texture", "annotation": "const*"},
- {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "data size"},
+ {"name": "data", "type": "uint8_t", "annotation": "const*", "length": "data size", "wire_is_data_only": true},
{"name": "data size", "type": "uint64_t"},
{"name": "data layout", "type": "texture data layout", "annotation": "const*"},
{"name": "writeSize", "type": "extent 3D", "annotation": "const*"}
diff --git a/docs/codegen.md b/docs/codegen.md
index 75a208b..e0cfd55 100644
--- a/docs/codegen.md
+++ b/docs/codegen.md
@@ -42,6 +42,7 @@
- `"length"` (default to 1 if not set), a string. Defines length of the array pointed to for pointer arguments. If not set the length is implicitly 1 (so not an array), but otherwise it can be set to the name of another member in the same record that will contain the length of the array (this is heavily used in the `fooCount` `foos` pattern in the API). As a special case `"strlen"` can be used for `const char*` record members to denote that the length should be determined with `strlen`.
- `"optional"` (default to false) a boolean that says whether this member is optional. Member records can be optional if they are pointers (otherwise dawn_wire will always try to dereference them), objects (otherwise dawn_wire will always try to encode their ID and crash), or if they have a `"default"` key. Optional pointers and objects will always default to `nullptr`.
- `"default"` (optional) a number or string. If set the record member will use that value as default value. Depending on the member's category it can be a number, a string containing a number, or the name of an enum/bitmask value.
+ - `"wire_is_data_only"` (default to false) a boolean that says whether it is safe to directly return a pointer of this member that is pointing to a piece of memory in the transfer buffer into dawn_wire. To prevent TOCTOU attacks, by default in dawn_wire we must ensure every single value returned to dawn_native a copy of what's in the wire, so `"wire_is_data_only"` is set to true only when the member is data-only and don't impact control flow.
**`"native"`**, doesn't have any other key. This is used to define native types that can be referenced by name in other things.
diff --git a/generator/templates/dawn_wire/WireCmd.cpp b/generator/templates/dawn_wire/WireCmd.cpp
index e6d9504..0eb1bd1 100644
--- a/generator/templates/dawn_wire/WireCmd.cpp
+++ b/generator/templates/dawn_wire/WireCmd.cpp
@@ -373,26 +373,39 @@
const volatile {{member_transfer_type(member)}}* memberBuffer;
WIRE_TRY(deserializeBuffer->ReadN(memberLength, &memberBuffer));
- {{as_cType(member.type.name)}}* copiedMembers;
- WIRE_TRY(GetSpace(allocator, memberLength, &copiedMembers));
- record->{{memberName}} = copiedMembers;
+ //* For data-only members (e.g. "data" in WriteBuffer and WriteTexture), they are
+ //* not security sensitive so we can directly refer the data inside the transfer
+ //* buffer in dawn_native. For other members, as prevention of TOCTOU attacks is an
+ //* important feature of the wire, we must make sure every single value returned to
+ //* dawn_native must be a copy of what's in the wire.
+ {% if member.json_data["wire_is_data_only"] %}
+ record->{{memberName}} =
+ const_cast<const {{member_transfer_type(member)}}*>(memberBuffer);
- {% if member.type.is_wire_transparent %}
- //* memcpy is not allowed to copy from volatile objects. However, these arrays
- //* are just used as plain data, and don't impact control flow. So if the
- //* underlying data were changed while the copy was still executing, we would
- //* get different data - but it wouldn't cause unexpected downstream effects.
- memcpy(
- copiedMembers,
- const_cast<const {{member_transfer_type(member)}}*>(memberBuffer),
- {{member_transfer_sizeof(member)}} * memberLength);
{% else %}
- //* This loop cannot overflow because it iterates up to |memberLength|. Even if
- //* memberLength were the maximum integer value, |i| would become equal to it
- //* just before exiting the loop, but not increment past or wrap around.
- for (decltype(memberLength) i = 0; i < memberLength; ++i) {
- {{deserialize_member(member, "memberBuffer[i]", "copiedMembers[i]")}}
- }
+ {{as_cType(member.type.name)}}* copiedMembers;
+ WIRE_TRY(GetSpace(allocator, memberLength, &copiedMembers));
+ record->{{memberName}} = copiedMembers;
+
+ {% if member.type.is_wire_transparent %}
+ //* memcpy is not allowed to copy from volatile objects. However, these
+ //* arrays are just used as plain data, and don't impact control flow. So if
+ //* the underlying data were changed while the copy was still executing, we
+ //* would get different data - but it wouldn't cause unexpected downstream
+ //* effects.
+ memcpy(
+ copiedMembers,
+ const_cast<const {{member_transfer_type(member)}}*>(memberBuffer),
+ {{member_transfer_sizeof(member)}} * memberLength);
+ {% else %}
+ //* This loop cannot overflow because it iterates up to |memberLength|. Even
+ //* if memberLength were the maximum integer value, |i| would become equal
+ //* to it just before exiting the loop, but not increment past or wrap
+ //* around.
+ for (decltype(memberLength) i = 0; i < memberLength; ++i) {
+ {{deserialize_member(member, "memberBuffer[i]", "copiedMembers[i]")}}
+ }
+ {% endif %}
{% endif %}
}
{% endfor %}
