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

Fix OOB access while dumping struct layout for invalid storage class layout 

A one letter typo would lead to invalid memory access in the very
specific case of outputting the layout for a struct within a struct with
field alignment padding, and the inner struct has more members than the
outer.

Bug: tint:1344
Bug: oss-fuzz:72642
Change-Id: I749e3fb172e78a20ece68b40be1a0a57dc5746f4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72642
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
This commit is contained in:
Antonio Maiorano 2021-12-13 23:36:38 +00:00 committed by Tint LUCI CQ
parent f885941100
commit 9b9132c715
2 changed files with 60 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/resolver/resolver_validation.cc
View File

@ -263,7 +263,8 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str,
// TODO(amaiorano): Output struct and member decorations so that this output
// can be copied verbatim back into source
auto get_struct_layout_string = [&](const sem::Struct* st) -> std::string {
auto get_struct_layout_string = [this, member_name_of, type_name_of](
const sem::Struct* st) -> std::string {
std::stringstream ss;
if (st->Members().empty()) {
@ -308,7 +309,7 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str,
auto* const m = st->Members()[i];
// Output field alignment padding, if any
auto* const prev_member = (i == 0) ? nullptr : str->Members()[i - 1];
auto* const prev_member = (i == 0) ? nullptr : st->Members()[i - 1];
if (prev_member) {
uint32_t padding =
m->Offset() - (prev_member->Offset() + prev_member->Size());

57
src/resolver/storage_class_layout_validation_test.cc
View File

@ -263,6 +263,63 @@ TEST_F(ResolverStorageClassLayoutValidationTest,
22:24 note: see declaration of variable)");
}
// See https://crbug.com/tint/1344
TEST_F(ResolverStorageClassLayoutValidationTest,
UniformBuffer_MembersOffsetNotMultipleOf16_InnerMoreMembersThanOuter) {
// struct Inner {
// a : i32;
// b : i32;
// c : i32;
// [[align(1), size(5)]] scalar : i32;
// };
//
// [[block]]
// struct Outer {
// inner : Inner;
// scalar : i32;
// };
//
// [[group(0), binding(0)]]
// var<uniform> a : Outer;
Structure(Source{{12, 34}}, "Inner",
{
Member("a", ty.i32()),
Member("b", ty.i32()),
Member("c", ty.i32()),
Member("scalar", ty.i32(), {MemberAlign(1), MemberSize(5)}),
});
Structure(Source{{34, 56}}, "Outer",
{
Member(Source{{56, 78}}, "inner", ty.type_name("Inner")),
Member(Source{{78, 90}}, "scalar", ty.i32()),
},
{StructBlock()});
Global(Source{{22, 24}}, "a", ty.type_name("Outer"),
ast::StorageClass::kUniform, GroupAndBinding(0, 0));
ASSERT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(78:90 error: uniform storage requires that the number of bytes between the start of the previous member of type struct and the current member be a multiple of 16 bytes, but there are currently 20 bytes between 'inner' and 'scalar'. Consider setting [[align(16)]] on this member
34:56 note: see layout of struct:
/* align(4) size(24) */ struct Outer {
/* offset( 0) align(4) size(20) */ inner : Inner;
/* offset(20) align(4) size( 4) */ scalar : i32;
/* */ };
12:34 note: and layout of previous member struct:
/* align(4) size(20) */ struct Inner {
/* offset( 0) align(4) size( 4) */ a : i32;
/* offset( 4) align(4) size( 4) */ b : i32;
/* offset( 8) align(4) size( 4) */ c : i32;
/* offset(12) align(1) size( 5) */ scalar : i32;
/* offset(17) align(1) size( 3) */ // -- implicit struct size padding --;
/* */ };
22:24 note: see declaration of variable)");
}
TEST_F(ResolverStorageClassLayoutValidationTest,
UniformBuffer_MembersOffsetNotMultipleOf16_SuggestedFix) {
// struct Inner {