Don't memcpy zero bytes in generated WireCmd.cpp
Due to a C language bug, memcpy is undefined on empty inputs when the
pointers are NULL. See https://davidben.net/2024/01/15/empty-slices.html
for details. This code triggers UBSan bugs like the following:
https://luci-milo.appspot.com/ui/inv/build-8749056521619502353/test-results?q=WebGPUSwapBufferProviderTest.ReuseSwapBuffers&sortby=&groupby=
For most of these in Chromium, I've been fixing them to use spans, or at
least std::copy and std::copy_n, which do not have this language bug and
are specialized as memcpy when the type is trivially copyable. However,
when I tried to do that, I ran into issues with WGPUColor and
WGPUColorTransfer being different types. There's a lot of synthesized
code here I can't follow so, instead, just guard the memcpy with a zero
check to fix the undefined behavior.
Bug: chromium:40248746
Change-Id: I4041bfb750a3e0fcf56353bffefc0a2715603475
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/188900
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/generator/templates/dawn/wire/WireCmd.cpp b/generator/templates/dawn/wire/WireCmd.cpp
index 3cadd30..0c122d2 100644
--- a/generator/templates/dawn/wire/WireCmd.cpp
+++ b/generator/templates/dawn/wire/WireCmd.cpp
@@ -308,9 +308,14 @@
WIRE_TRY(buffer->NextN(memberLength, &memberBuffer));
{% if member.type.is_wire_transparent %}
- memcpy(
- memberBuffer, record.{{memberName}},
- {{member_transfer_sizeof(member)}} * memberLength);
+ //* memcpy is not defined for null pointers, even when the length is zero.
+ //* conflicts with the common practice to use (nullptr, 0) to represent an
+ //* span. Guard memcpy with a zero check to work around this language bug.
+ if (memberLength != 0) {
+ memcpy(
+ memberBuffer, record.{{memberName}},
+ {{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
@@ -436,15 +441,20 @@
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);
+ //* memcpy is not defined for null pointers, even when the length is zero.
+ //* conflicts with the common practice to use (nullptr, 0) to represent an
+ //* span. Guard memcpy with a zero check to work around this language bug.
+ if (memberLength != 0) {
+ //* 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
