From fab4801e1e88772e97586c7eae8d1290a26f61ff Mon Sep 17 00:00:00 2001 From: Alex Pankhurst Date: Wed, 20 Jul 2022 10:36:27 -0700 Subject: [PATCH] [fuchsia] Fix ubsan issues Fuchsia's undefined behavior sanitizer was detecting unaligned accesses to 8 byte aligned data in Crashpad tests because various MINIDUMP_* structs are packed with 4 byte alignment. This change copies unaligned data in tests to local variable that can be safely used to check values. Example errors: ''' [../../third_party/crashpad/minidump/minidump_thread_name_list_writer_test.cc:95:3]: runtime error: reference binding to misaligned address 0x461e104cfbd4 for type 'const RVA64' (aka 'const unsigned long'), which requires 8 byte aligment ''' ''' ''' Change-Id: I3c0905aa9eab810c00d57f1e9e54bb8eaaff54b0 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3775293 Reviewed-by: Joshua Peraza Commit-Queue: Alex Pankhurst --- .../minidump_thread_name_list_writer_test.cc | 13 +- minidump/minidump_writable_test.cc | 128 ++++++++++-------- 2 files changed, 79 insertions(+), 62 deletions(-) diff --git a/minidump/minidump_thread_name_list_writer_test.cc b/minidump/minidump_thread_name_list_writer_test.cc index 265c12e6..43ed5270 100644 --- a/minidump/minidump_thread_name_list_writer_test.cc +++ b/minidump/minidump_thread_name_list_writer_test.cc @@ -91,10 +91,19 @@ void ExpectThreadName(const MINIDUMP_THREAD_NAME* expected, const MINIDUMP_THREAD_NAME* observed, const std::string& file_contents, const std::string& expected_thread_name) { + // Copy RvaOfThreadName into a local variable because + // |MINIDUMP_THREAD_NAME::RvaOfThreadName| requires 8-byte alignment but the + // struct itself is 4-byte algined. + const auto rva_of_thread_name = [&observed] { + RVA64 data = 0; + memcpy(&data, &observed->RvaOfThreadName, sizeof(RVA64)); + return data; + }(); + EXPECT_EQ(observed->ThreadId, expected->ThreadId); - EXPECT_NE(observed->RvaOfThreadName, 0u); + EXPECT_NE(rva_of_thread_name, 0u); const std::string observed_thread_name = base::UTF16ToUTF8( - MinidumpStringAtRVAAsString(file_contents, observed->RvaOfThreadName)); + MinidumpStringAtRVAAsString(file_contents, rva_of_thread_name)); EXPECT_EQ(observed_thread_name, expected_thread_name); } diff --git a/minidump/minidump_writable_test.cc b/minidump/minidump_writable_test.cc index 2032b063..3d466d78 100644 --- a/minidump/minidump_writable_test.cc +++ b/minidump/minidump_writable_test.cc @@ -698,16 +698,25 @@ class TTestLocationDescriptorMinidumpWritable final template struct TLocationDescriptorAndData { MinidumpLocationDescriptorType location_descriptor; - char string[1]; + const char* string; }; template -const TLocationDescriptorAndData* TLDDAtIndex( - const std::string& string, +TLocationDescriptorAndData TLDDAtIndex( + const std::string& str, size_t index) { - return reinterpret_cast< - const TLocationDescriptorAndData*>( - &string[index]); + const MinidumpLocationDescriptorType* location_descriptor = + reinterpret_cast(&str[index]); + + const char* string = reinterpret_cast( + &str[index] + + offsetof(TLocationDescriptorAndData, + string)); + + return TLocationDescriptorAndData{ + *location_descriptor, + string, + }; } template @@ -746,9 +755,9 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { EXPECT_TRUE(location_descriptor_writable.WriteEverything(&string_file)); ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize + 1); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, 0u); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + EXPECT_EQ(ldd.location_descriptor.DataSize, 0u); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); location_descriptor_writable.Verify(); } @@ -761,10 +770,10 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { EXPECT_TRUE(location_descriptor_writable.WriteEverything(&string_file)); ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize + 1); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 1); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); location_descriptor_writable.Verify(); } @@ -778,11 +787,11 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { EXPECT_TRUE(location_descriptor_writable.WriteEverything(&string_file)); ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize + 3); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 3); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); - EXPECT_STREQ("zz", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); + EXPECT_STREQ("zz", ldd.string); location_descriptor_writable.Verify(); } @@ -800,17 +809,19 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize * 2 + 6); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 3); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); - EXPECT_STREQ("yy", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); + EXPECT_STREQ("yy", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize + 4); - EXPECT_EQ(ldd->location_descriptor.DataSize, + + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 2); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("x", ldd->string); + + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("x", ldd.string); parent.Verify(); } @@ -827,16 +838,18 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize * 2 + 7); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 3); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("www", ldd->string); + + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("www", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize + 4); - EXPECT_EQ(ldd->location_descriptor.DataSize, 0u); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); - EXPECT_STREQ("vv", ldd->string); + + EXPECT_EQ(ldd.location_descriptor.DataSize, 0u); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); + EXPECT_STREQ("vv", ldd.string); parent.Verify(); } @@ -854,17 +867,16 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize * 2 + 13); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 5); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 8); - EXPECT_STREQ("uuuu", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 8); + EXPECT_STREQ("uuuu", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize + 8); - EXPECT_EQ(ldd->location_descriptor.DataSize, + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 5); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); - EXPECT_STREQ("tttt", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); + EXPECT_STREQ("tttt", ldd.string); parent.Verify(); } @@ -893,37 +905,33 @@ TYPED_TEST(MinidumpWritableLocationDescriptor, LocationDescriptor) { ASSERT_EQ(string_file.string().size(), kMinidumpLocationDescriptorSize * 5 + 18); - const LocationDescriptorAndData* ldd = LDDAtIndex(string_file.string(), 0); - EXPECT_EQ(ldd->location_descriptor.DataSize, + LocationDescriptorAndData ldd = LDDAtIndex(string_file.string(), 0); + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 2); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("s", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("s", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize + 4); - EXPECT_EQ(ldd->location_descriptor.DataSize, 0u); - EXPECT_EQ(ldd->location_descriptor.Rva, 0u); - EXPECT_STREQ("r", ldd->string); + EXPECT_EQ(ldd.location_descriptor.DataSize, 0u); + EXPECT_EQ(ldd.location_descriptor.Rva, 0u); + EXPECT_STREQ("r", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize * 2 + 8); - EXPECT_EQ(ldd->location_descriptor.DataSize, + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 2); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("q", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("q", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize * 3 + 12); - EXPECT_EQ(ldd->location_descriptor.DataSize, + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 2); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("p", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("p", ldd.string); ldd = LDDAtIndex(string_file.string(), kMinidumpLocationDescriptorSize * 4 + 16); - EXPECT_EQ(ldd->location_descriptor.DataSize, + EXPECT_EQ(ldd.location_descriptor.DataSize, kMinidumpLocationDescriptorSize + 2); - EXPECT_EQ(ldd->location_descriptor.Rva, - kMinidumpLocationDescriptorSize + 4); - EXPECT_STREQ("o", ldd->string); + EXPECT_EQ(ldd.location_descriptor.Rva, kMinidumpLocationDescriptorSize + 4); + EXPECT_STREQ("o", ldd.string); parent.Verify(); } }