encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WireCmd: disallow optional members with length="otherMember" 

This didn't make sense since these members can be specified to have no
data by setting the length to 0.

 - Prevent uses of this patten by adding an assert in WireCmd.cpp's
   generator.
 - Fix SetBindGroup dynamicOffset to not be optional but default to
   nullptr instead.

This issues would cause a read of uninitialized pointers becaus the
generator code looked like this:

  SetBindGroupCmd cmd;
  cmd.dynamicOffsetCount = record.dynamicOffsetCount; // 1
  bool has_dynamicOffset = record.has_dynamicOffsetl // false
  if (has_dynamicOffset) {
    cmd.dynamicOffsets = ...;
  }
  // Oh no! dynamicOffsets contains garbage even if dynamicOffsetCount
  // is set to 1. dawn_native will happily read it.

Bug: chromium:1220036
Change-Id: I5c468b639f671cef3be2fa64667a0bf114fc902b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/54643
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Corentin Wallez 2021-06-17 16:04:29 +00:00 committed by Dawn LUCI CQ
parent 36edf8d850
commit 75f554d973
2 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
dawn.json
View File

@ -572,7 +572,7 @@
{"name": "group index", "type": "uint32_t"},
{"name": "group", "type": "bind group"},
{"name": "dynamic offset count", "type": "uint32_t", "default": "0"},
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "optional": true}
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "default": "nullptr"}
]
},
{
@ -1229,7 +1229,7 @@
{"name": "group index", "type": "uint32_t"},
{"name": "group", "type": "bind group"},
{"name": "dynamic offset count", "type": "uint32_t", "default": "0"},
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "optional": true}
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "default": "nullptr"}
]
},
{
@ -1393,7 +1393,7 @@
{"name": "group index", "type": "uint32_t"},
{"name": "group", "type": "bind group"},
{"name": "dynamic offset count", "type": "uint32_t", "default": "0"},
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "optional": true}
{"name": "dynamic offsets", "type": "uint32_t", "annotation": "const*", "length": "dynamic offset count", "default": "nullptr"}
]
},
{

4
generator/templates/dawn_wire/WireCmd.cpp
View File

@ -361,6 +361,10 @@ namespace {
{% set memberName = as_varName(member.name) %}
{% if member.type.category != "object" and member.optional %}
//* Non-constant length optional members use length=0 to denote they aren't present.
//* Otherwise we could have length=N and has_member=false, causing reads from an
//* uninitialized pointer.
{{ assert(member.length == "constant") }}
bool has_{{memberName}} = transfer->has_{{memberName}};
record->{{memberName}} = nullptr;
if (has_{{memberName}})