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>
diff --git a/src/resolver/resolver_validation.cc b/src/resolver/resolver_validation.cc
index 6921a51..117eb25 100644
--- a/src/resolver/resolver_validation.cc
+++ b/src/resolver/resolver_validation.cc
@@ -263,7 +263,8 @@
// 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 @@
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());
diff --git a/src/resolver/storage_class_layout_validation_test.cc b/src/resolver/storage_class_layout_validation_test.cc
index 3ad7d3f..467608d 100644
--- a/src/resolver/storage_class_layout_validation_test.cc
+++ b/src/resolver/storage_class_layout_validation_test.cc
@@ -263,6 +263,63 @@
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 {
