Remove size_t from wire transfer structs

This makes the primitives in the serialized wire protocol
the same across platforms and architectures which is better
for both fuzzing and remoting Dawn.

Commands that used size_t are updated to use uint64_t, and
the server-side implementation checks if conversion to
size_t would narrow.

Bug: dawn:680
Change-Id: Icef9dc11a72699685ed7191c34d6a922b652c887
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41582
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Austin Eng 2021-02-18 22:36:19 +00:00 committed by Commit Bot service account
parent f0f6d2f8e1
commit f104fea367
4 changed files with 54 additions and 26 deletions

View File

@ -14,16 +14,13 @@
"See the License for the specific language governing permissions and",
"limitations under the License."
],
"_todos": [
"Remove usage of size_t because it is not network transparent"
],
"commands": {
"buffer map async": [
{ "name": "buffer id", "type": "ObjectId" },
{ "name": "request serial", "type": "uint32_t" },
{ "name": "mode", "type": "map mode" },
{ "name": "offset", "type": "size_t"},
{ "name": "size", "type": "size_t"},
{ "name": "offset", "type": "uint64_t"},
{ "name": "size", "type": "uint64_t"},
{ "name": "handle create info length", "type": "uint64_t" },
{ "name": "handle create info", "type": "uint8_t", "annotation": "const*", "length": "handle create info length", "skip_serialize": true}
],
@ -69,13 +66,13 @@
{"name": "buffer id", "type": "ObjectId" },
{"name": "buffer offset", "type": "uint64_t"},
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "size"},
{"name": "size", "type": "size_t"}
{"name": "size", "type": "uint64_t"}
],
"queue write texture internal": [
{"name": "queue id", "type": "ObjectId" },
{"name": "destination", "type": "texture copy view", "annotation": "const*"},
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "data size"},
{"name": "data size", "type": "size_t"},
{"name": "data size", "type": "uint64_t"},
{"name": "data layout", "type": "texture data layout", "annotation": "const*"},
{"name": "writeSize", "type": "extent 3D", "annotation": "const*"}
]

View File

@ -42,6 +42,7 @@
{%- elif member.type.category == "bitmask" -%}
{{as_cType(member.type.name)}}Flags
{%- else -%}
{{ assert(as_cType(member.type.name) != "size_t") }}
{{as_cType(member.type.name)}}
{%- endif -%}
{%- endmacro %}
@ -80,6 +81,7 @@
{%- endif -%}
));
{%- else -%}
static_assert(sizeof({{out}}) >= sizeof({{in}}), "Deserialize assignment may not narrow.");
{{out}} = {{in}};
{%- endif -%}
{% endmacro %}
@ -124,7 +126,7 @@ namespace {
//* const char* have their length embedded directly in the command.
{% for member in members if member.length == "strlen" %}
size_t {{as_varName(member.name)}}Strlen;
uint64_t {{as_varName(member.name)}}Strlen;
{% endfor %}
{% for member in members if member.optional and member.annotation != "value" and member.type.category != "object" %}
@ -327,19 +329,21 @@ namespace {
if (has_{{memberName}})
{% endif %}
{
size_t stringLength = transfer->{{memberName}}Strlen;
if (stringLength == std::numeric_limits<size_t>::max()) {
//* Cannot allocate enough space for the null terminator.
uint64_t stringLength64 = transfer->{{memberName}}Strlen;
if (stringLength64 >= std::numeric_limits<size_t>::max()) {
//* Cannot allocate space for the string. It can be at most
//* size_t::max() - 1. We need 1 byte for the null-terminator.
return DeserializeResult::FatalError;
}
size_t stringLength = static_cast<size_t>(stringLength64);
const volatile char* stringInBuffer;
DESERIALIZE_TRY(deserializeBuffer->ReadN(stringLength, &stringInBuffer));
char* copiedString;
DESERIALIZE_TRY(GetSpace(allocator, stringLength + 1, &copiedString));
//* We can cast away the volatile qualifier because GetPtrFromBuffer already validated
//* that the range [stringInBuffer, stringInBuffer + stringLength) is valid.
//* We can cast away the volatile qualifier because DeserializeBuffer::ReadN already
//* validated that the range [stringInBuffer, stringInBuffer + stringLength) is valid.
//* memcpy may have an unknown access pattern, but this is fine since the string is only
//* data and won't affect control flow of this function.
memcpy(copiedString, const_cast<const char*>(stringInBuffer), stringLength);

View File

@ -48,8 +48,8 @@ namespace dawn_wire { namespace server {
bool Server::DoBufferMapAsync(ObjectId bufferId,
uint32_t requestSerial,
WGPUMapModeFlags mode,
size_t offset,
size_t size,
uint64_t offset64,
uint64_t size64,
uint64_t handleCreateInfoLength,
const uint8_t* handleCreateInfo) {
// These requests are just forwarded to the buffer, with userdata containing what the
@ -72,19 +72,24 @@ namespace dawn_wire { namespace server {
return false;
}
if (handleCreateInfoLength > std::numeric_limits<size_t>::max()) {
// This is the size of data deserialized from the command stream, which must be
// CPU-addressable.
return false;
}
std::unique_ptr<MapUserdata> userdata = MakeUserdata<MapUserdata>();
userdata->buffer = ObjectHandle{bufferId, buffer->generation};
userdata->bufferObj = buffer->handle;
userdata->requestSerial = requestSerial;
userdata->mode = mode;
if (offset64 > std::numeric_limits<size_t>::max() ||
size64 > std::numeric_limits<size_t>::max() ||
handleCreateInfoLength > std::numeric_limits<size_t>::max()) {
OnBufferMapAsyncCallback(WGPUBufferMapAsyncStatus_Error, userdata.get());
return true;
}
size_t offset = static_cast<size_t>(offset64);
size_t size = static_cast<size_t>(size64);
userdata->offset = offset;
userdata->size = size;
userdata->mode = mode;
// The handle will point to the mapped memory or staging memory for the mapping.
// Store it on the map request.

View File

@ -44,7 +44,7 @@ namespace dawn_wire { namespace server {
ObjectId bufferId,
uint64_t bufferOffset,
const uint8_t* data,
size_t size) {
uint64_t size) {
// The null object isn't valid as `self` or `buffer` so we can combine the check with the
// check that the ID is valid.
auto* queue = QueueObjects().Get(queueId);
@ -53,14 +53,25 @@ namespace dawn_wire { namespace server {
return false;
}
mProcs.queueWriteBuffer(queue->handle, buffer->handle, bufferOffset, data, size);
if (size > std::numeric_limits<size_t>::max()) {
auto* device = DeviceObjects().Get(queue->deviceInfo->self.id);
if (device == nullptr) {
return false;
}
return DoDeviceInjectError(reinterpret_cast<WGPUDevice>(device),
WGPUErrorType_OutOfMemory,
"Data size too large for write texture.");
}
mProcs.queueWriteBuffer(queue->handle, buffer->handle, bufferOffset, data,
static_cast<size_t>(size));
return true;
}
bool Server::DoQueueWriteTextureInternal(ObjectId queueId,
const WGPUTextureCopyView* destination,
const uint8_t* data,
size_t dataSize,
uint64_t dataSize,
const WGPUTextureDataLayout* dataLayout,
const WGPUExtent3D* writeSize) {
// The null object isn't valid as `self` so we can combine the check with the
@ -70,7 +81,18 @@ namespace dawn_wire { namespace server {
return false;
}
mProcs.queueWriteTexture(queue->handle, destination, data, dataSize, dataLayout, writeSize);
if (dataSize > std::numeric_limits<size_t>::max()) {
auto* device = DeviceObjects().Get(queue->deviceInfo->self.id);
if (device == nullptr) {
return false;
}
return DoDeviceInjectError(reinterpret_cast<WGPUDevice>(device),
WGPUErrorType_OutOfMemory,
"Data size too large for write texture.");
}
mProcs.queueWriteTexture(queue->handle, destination, data, static_cast<size_t>(dataSize),
dataLayout, writeSize);
return true;
}