From 44de18ca37f694ba6afe9cc409199224c09e3186 Mon Sep 17 00:00:00 2001 From: Braden Kell Date: Wed, 9 Dec 2020 22:21:38 +0000 Subject: [PATCH] Fix instances of undefined behavior This change removes several unaligned accesses, as well a null pointer offset and an out of bounds array access. Bug: fuchsia:46805 Change-Id: I0110d0b7faf672655d978894b868760eee7b2988 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2583025 Commit-Queue: Scott Graham Reviewed-by: Scott Graham --- BUILD.gn | 13 --- minidump/BUILD.gn | 2 - minidump/minidump_exception_writer_test.cc | 31 +++--- minidump/minidump_handle_writer_test.cc | 63 ++++++------ minidump/minidump_memory_info_writer_test.cc | 31 +++--- minidump/minidump_misc_info_writer_test.cc | 27 ++++-- minidump/minidump_module_writer_test.cc | 97 +++++++++++-------- minidump/minidump_system_info_writer_test.cc | 23 +++-- minidump/minidump_thread_writer_test.cc | 46 +++++---- .../test/minidump_memory_writer_test_util.cc | 18 +++- snapshot/BUILD.gn | 2 - third_party/googletest/BUILD.gn | 1 - util/BUILD.gn | 2 - util/file/file_io.cc | 14 +-- util/net/http_transport_socket.cc | 2 +- 15 files changed, 207 insertions(+), 165 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 7c0a7940..1ec9b975 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -20,19 +20,6 @@ config("crashpad_config") { include_dirs = [ "." ] } -# TODO(fuchsia:46805): Remove this once instances of UB have been cleaned up. -config("disable_ubsan") { - if (crashpad_is_in_fuchsia) { - cflags = [ "-fno-sanitize=undefined" ] - } - visibility = [ - "snapshot:snapshot", - "minidump:minidump_test", - "third_party/googletest:googletest", - "util:util", - ] -} - if (crashpad_is_in_chromium || crashpad_is_in_fuchsia) { test("crashpad_tests") { deps = [ diff --git a/minidump/BUILD.gn b/minidump/BUILD.gn index 6217edeb..459fd6b7 100644 --- a/minidump/BUILD.gn +++ b/minidump/BUILD.gn @@ -186,6 +186,4 @@ source_set("minidump_test") { if (crashpad_is_win) { cflags = [ "/wd4201" ] # nonstandard extension used : nameless struct/union } - - configs += [ "..:disable_ubsan" ] } diff --git a/minidump/minidump_exception_writer_test.cc b/minidump/minidump_exception_writer_test.cc index eed8f104..72837bba 100644 --- a/minidump/minidump_exception_writer_test.cc +++ b/minidump/minidump_exception_writer_test.cc @@ -69,22 +69,27 @@ void ExpectExceptionStream(const MINIDUMP_EXCEPTION_STREAM* expected, const MinidumpContextX86** context) { EXPECT_EQ(observed->ThreadId, expected->ThreadId); EXPECT_EQ(observed->__alignment, 0u); - EXPECT_EQ(observed->ExceptionRecord.ExceptionCode, - expected->ExceptionRecord.ExceptionCode); - EXPECT_EQ(observed->ExceptionRecord.ExceptionFlags, - expected->ExceptionRecord.ExceptionFlags); - EXPECT_EQ(observed->ExceptionRecord.ExceptionRecord, - expected->ExceptionRecord.ExceptionRecord); - EXPECT_EQ(observed->ExceptionRecord.ExceptionAddress, - expected->ExceptionRecord.ExceptionAddress); - EXPECT_EQ(observed->ExceptionRecord.NumberParameters, - expected->ExceptionRecord.NumberParameters); + + // Copy the ExceptionRecords so that their uint64_t members can be accessed + // with the proper alignment. + const MINIDUMP_EXCEPTION observed_exception = observed->ExceptionRecord; + const MINIDUMP_EXCEPTION expected_exception = expected->ExceptionRecord; + + EXPECT_EQ(observed_exception.ExceptionCode, expected_exception.ExceptionCode); + EXPECT_EQ(observed_exception.ExceptionFlags, + expected_exception.ExceptionFlags); + EXPECT_EQ(observed_exception.ExceptionRecord, + expected_exception.ExceptionRecord); + EXPECT_EQ(observed_exception.ExceptionAddress, + expected_exception.ExceptionAddress); + EXPECT_EQ(observed_exception.NumberParameters, + expected_exception.NumberParameters); EXPECT_EQ(observed->ExceptionRecord.__unusedAlignment, 0u); for (size_t index = 0; - index < base::size(observed->ExceptionRecord.ExceptionInformation); + index < base::size(observed_exception.ExceptionInformation); ++index) { - EXPECT_EQ(observed->ExceptionRecord.ExceptionInformation[index], - expected->ExceptionRecord.ExceptionInformation[index]); + EXPECT_EQ(observed_exception.ExceptionInformation[index], + expected_exception.ExceptionInformation[index]); } *context = MinidumpWritableAtLocationDescriptor( file_contents, observed->ThreadContext); diff --git a/minidump/minidump_handle_writer_test.cc b/minidump/minidump_handle_writer_test.cc index 19a1036b..0776ae02 100644 --- a/minidump/minidump_handle_writer_test.cc +++ b/minidump/minidump_handle_writer_test.cc @@ -110,18 +110,17 @@ TEST(MinidumpHandleDataWriter, OneHandle) { GetHandleDataStream(string_file.string(), &handle_data_stream)); EXPECT_EQ(handle_data_stream->NumberOfDescriptors, 1u); - const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor = - reinterpret_cast( - &handle_data_stream[1]); - EXPECT_EQ(handle_descriptor->Handle, handle_snapshot.handle); + MINIDUMP_HANDLE_DESCRIPTOR handle_descriptor; + memcpy(&handle_descriptor, &handle_data_stream[1], sizeof(handle_descriptor)); + EXPECT_EQ(handle_descriptor.Handle, handle_snapshot.handle); EXPECT_EQ(base::UTF16ToUTF8(MinidumpStringAtRVAAsString( - string_file.string(), handle_descriptor->TypeNameRva)), + string_file.string(), handle_descriptor.TypeNameRva)), handle_snapshot.type_name); - EXPECT_EQ(handle_descriptor->ObjectNameRva, 0u); - EXPECT_EQ(handle_descriptor->Attributes, handle_snapshot.attributes); - EXPECT_EQ(handle_descriptor->GrantedAccess, handle_snapshot.granted_access); - EXPECT_EQ(handle_descriptor->HandleCount, handle_snapshot.handle_count); - EXPECT_EQ(handle_descriptor->PointerCount, handle_snapshot.pointer_count); + EXPECT_EQ(handle_descriptor.ObjectNameRva, 0u); + EXPECT_EQ(handle_descriptor.Attributes, handle_snapshot.attributes); + EXPECT_EQ(handle_descriptor.GrantedAccess, handle_snapshot.granted_access); + EXPECT_EQ(handle_descriptor.HandleCount, handle_snapshot.handle_count); + EXPECT_EQ(handle_descriptor.PointerCount, handle_snapshot.pointer_count); } TEST(MinidumpHandleDataWriter, RepeatedTypeName) { @@ -168,34 +167,34 @@ TEST(MinidumpHandleDataWriter, RepeatedTypeName) { GetHandleDataStream(string_file.string(), &handle_data_stream)); EXPECT_EQ(handle_data_stream->NumberOfDescriptors, 2u); - const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor = - reinterpret_cast( - &handle_data_stream[1]); - EXPECT_EQ(handle_descriptor->Handle, handle_snapshot.handle); + MINIDUMP_HANDLE_DESCRIPTOR handle_descriptor; + memcpy(&handle_descriptor, &handle_data_stream[1], sizeof(handle_descriptor)); + EXPECT_EQ(handle_descriptor.Handle, handle_snapshot.handle); EXPECT_EQ(base::UTF16ToUTF8(MinidumpStringAtRVAAsString( - string_file.string(), handle_descriptor->TypeNameRva)), + string_file.string(), handle_descriptor.TypeNameRva)), handle_snapshot.type_name); - EXPECT_EQ(handle_descriptor->ObjectNameRva, 0u); - EXPECT_EQ(handle_descriptor->Attributes, handle_snapshot.attributes); - EXPECT_EQ(handle_descriptor->GrantedAccess, handle_snapshot.granted_access); - EXPECT_EQ(handle_descriptor->HandleCount, handle_snapshot.handle_count); - EXPECT_EQ(handle_descriptor->PointerCount, handle_snapshot.pointer_count); + EXPECT_EQ(handle_descriptor.ObjectNameRva, 0u); + EXPECT_EQ(handle_descriptor.Attributes, handle_snapshot.attributes); + EXPECT_EQ(handle_descriptor.GrantedAccess, handle_snapshot.granted_access); + EXPECT_EQ(handle_descriptor.HandleCount, handle_snapshot.handle_count); + EXPECT_EQ(handle_descriptor.PointerCount, handle_snapshot.pointer_count); - const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor2 = - reinterpret_cast( - reinterpret_cast(&handle_data_stream[1]) + - sizeof(MINIDUMP_HANDLE_DESCRIPTOR)); - EXPECT_EQ(handle_descriptor2->Handle, handle_snapshot2.handle); + MINIDUMP_HANDLE_DESCRIPTOR handle_descriptor2; + memcpy(&handle_descriptor2, + reinterpret_cast(&handle_data_stream[1]) + + sizeof(MINIDUMP_HANDLE_DESCRIPTOR), + sizeof(handle_descriptor2)); + EXPECT_EQ(handle_descriptor2.Handle, handle_snapshot2.handle); EXPECT_EQ(base::UTF16ToUTF8(MinidumpStringAtRVAAsString( - string_file.string(), handle_descriptor2->TypeNameRva)), + string_file.string(), handle_descriptor2.TypeNameRva)), handle_snapshot2.type_name); - EXPECT_EQ(handle_descriptor2->ObjectNameRva, 0u); - EXPECT_EQ(handle_descriptor2->Attributes, handle_snapshot2.attributes); - EXPECT_EQ(handle_descriptor2->GrantedAccess, handle_snapshot2.granted_access); - EXPECT_EQ(handle_descriptor2->HandleCount, handle_snapshot2.handle_count); - EXPECT_EQ(handle_descriptor2->PointerCount, handle_snapshot2.pointer_count); + EXPECT_EQ(handle_descriptor2.ObjectNameRva, 0u); + EXPECT_EQ(handle_descriptor2.Attributes, handle_snapshot2.attributes); + EXPECT_EQ(handle_descriptor2.GrantedAccess, handle_snapshot2.granted_access); + EXPECT_EQ(handle_descriptor2.HandleCount, handle_snapshot2.handle_count); + EXPECT_EQ(handle_descriptor2.PointerCount, handle_snapshot2.pointer_count); - EXPECT_EQ(handle_descriptor2->TypeNameRva, handle_descriptor->TypeNameRva); + EXPECT_EQ(handle_descriptor2.TypeNameRva, handle_descriptor.TypeNameRva); } } // namespace diff --git a/minidump/minidump_memory_info_writer_test.cc b/minidump/minidump_memory_info_writer_test.cc index 634d3f1a..e45ea573 100644 --- a/minidump/minidump_memory_info_writer_test.cc +++ b/minidump/minidump_memory_info_writer_test.cc @@ -74,7 +74,11 @@ TEST(MinidumpMemoryInfoWriter, Empty) { ASSERT_NO_FATAL_FAILURE( GetMemoryInfoListStream(string_file.string(), &memory_info_list)); - EXPECT_EQ(memory_info_list->NumberOfEntries, 0u); + uint64_t number_of_entries; + memcpy(&number_of_entries, + &memory_info_list->NumberOfEntries, + sizeof(number_of_entries)); + EXPECT_EQ(number_of_entries, 0u); } TEST(MinidumpMemoryInfoWriter, OneRegion) { @@ -115,16 +119,21 @@ TEST(MinidumpMemoryInfoWriter, OneRegion) { ASSERT_NO_FATAL_FAILURE( GetMemoryInfoListStream(string_file.string(), &memory_info_list)); - EXPECT_EQ(memory_info_list->NumberOfEntries, 1u); - const MINIDUMP_MEMORY_INFO* memory_info = - reinterpret_cast(&memory_info_list[1]); - EXPECT_EQ(memory_info->BaseAddress, mmi.BaseAddress); - EXPECT_EQ(memory_info->AllocationBase, mmi.AllocationBase); - EXPECT_EQ(memory_info->AllocationProtect, mmi.AllocationProtect); - EXPECT_EQ(memory_info->RegionSize, mmi.RegionSize); - EXPECT_EQ(memory_info->State, mmi.State); - EXPECT_EQ(memory_info->Protect, mmi.Protect); - EXPECT_EQ(memory_info->Type, mmi.Type); + uint64_t number_of_entries; + memcpy(&number_of_entries, + &memory_info_list->NumberOfEntries, + sizeof(number_of_entries)); + EXPECT_EQ(number_of_entries, 1u); + + MINIDUMP_MEMORY_INFO memory_info; + memcpy(&memory_info, &memory_info_list[1], sizeof(memory_info)); + EXPECT_EQ(memory_info.BaseAddress, mmi.BaseAddress); + EXPECT_EQ(memory_info.AllocationBase, mmi.AllocationBase); + EXPECT_EQ(memory_info.AllocationProtect, mmi.AllocationProtect); + EXPECT_EQ(memory_info.RegionSize, mmi.RegionSize); + EXPECT_EQ(memory_info.State, mmi.State); + EXPECT_EQ(memory_info.Protect, mmi.Protect); + EXPECT_EQ(memory_info.Type, mmi.Type); } } // namespace diff --git a/minidump/minidump_misc_info_writer_test.cc b/minidump/minidump_misc_info_writer_test.cc index b7ea3d04..7e93fc5f 100644 --- a/minidump/minidump_misc_info_writer_test.cc +++ b/minidump/minidump_misc_info_writer_test.cc @@ -171,20 +171,27 @@ void ExpectMiscInfoEqual( ExpectMiscInfoEqual( reinterpret_cast(expected), reinterpret_cast(observed)); - EXPECT_EQ(observed->XStateData.SizeOfInfo, expected->XStateData.SizeOfInfo); - EXPECT_EQ(observed->XStateData.ContextSize, expected->XStateData.ContextSize); - EXPECT_EQ(observed->XStateData.EnabledFeatures, - expected->XStateData.EnabledFeatures); + + MINIDUMP_MISC_INFO_5 expected_misc_info, observed_misc_info; + memcpy(&expected_misc_info, expected, sizeof(expected_misc_info)); + memcpy(&observed_misc_info, observed, sizeof(observed_misc_info)); + + EXPECT_EQ(observed_misc_info.XStateData.SizeOfInfo, + expected_misc_info.XStateData.SizeOfInfo); + EXPECT_EQ(observed_misc_info.XStateData.ContextSize, + expected_misc_info.XStateData.ContextSize); + EXPECT_EQ(observed_misc_info.XStateData.EnabledFeatures, + expected_misc_info.XStateData.EnabledFeatures); for (size_t feature_index = 0; - feature_index < base::size(observed->XStateData.Features); + feature_index < base::size(observed_misc_info.XStateData.Features); ++feature_index) { SCOPED_TRACE(base::StringPrintf("feature_index %" PRIuS, feature_index)); - EXPECT_EQ(observed->XStateData.Features[feature_index].Offset, - expected->XStateData.Features[feature_index].Offset); - EXPECT_EQ(observed->XStateData.Features[feature_index].Size, - expected->XStateData.Features[feature_index].Size); + EXPECT_EQ(observed_misc_info.XStateData.Features[feature_index].Offset, + expected_misc_info.XStateData.Features[feature_index].Offset); + EXPECT_EQ(observed_misc_info.XStateData.Features[feature_index].Size, + expected_misc_info.XStateData.Features[feature_index].Size); } - EXPECT_EQ(observed->ProcessCookie, expected->ProcessCookie); + EXPECT_EQ(observed_misc_info.ProcessCookie, expected_misc_info.ProcessCookie); } TEST(MinidumpMiscInfoWriter, Empty) { diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index 71db3ac1..e40d1af1 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/minidump/minidump_module_writer_test.cc @@ -213,56 +213,68 @@ void ExpectModule(const MINIDUMP_MODULE* expected, const char* expected_debug_name, uint32_t expected_debug_type, bool expected_debug_utf16) { - EXPECT_EQ(observed->BaseOfImage, expected->BaseOfImage); - EXPECT_EQ(observed->SizeOfImage, expected->SizeOfImage); - EXPECT_EQ(observed->CheckSum, expected->CheckSum); - EXPECT_EQ(observed->TimeDateStamp, expected->TimeDateStamp); - EXPECT_EQ(observed->VersionInfo.dwSignature, - implicit_cast(VS_FFI_SIGNATURE)); - EXPECT_EQ(observed->VersionInfo.dwStrucVersion, - implicit_cast(VS_FFI_STRUCVERSION)); - EXPECT_EQ(observed->VersionInfo.dwFileVersionMS, - expected->VersionInfo.dwFileVersionMS); - EXPECT_EQ(observed->VersionInfo.dwFileVersionLS, - expected->VersionInfo.dwFileVersionLS); - EXPECT_EQ(observed->VersionInfo.dwProductVersionMS, - expected->VersionInfo.dwProductVersionMS); - EXPECT_EQ(observed->VersionInfo.dwProductVersionLS, - expected->VersionInfo.dwProductVersionLS); - EXPECT_EQ(observed->VersionInfo.dwFileFlagsMask, - expected->VersionInfo.dwFileFlagsMask); - EXPECT_EQ(observed->VersionInfo.dwFileFlags, - expected->VersionInfo.dwFileFlags); - EXPECT_EQ(observed->VersionInfo.dwFileOS, expected->VersionInfo.dwFileOS); - EXPECT_EQ(observed->VersionInfo.dwFileType, expected->VersionInfo.dwFileType); - EXPECT_EQ(observed->VersionInfo.dwFileSubtype, - expected->VersionInfo.dwFileSubtype); - EXPECT_EQ(observed->VersionInfo.dwFileDateMS, - expected->VersionInfo.dwFileDateMS); - EXPECT_EQ(observed->VersionInfo.dwFileDateLS, - expected->VersionInfo.dwFileDateLS); - EXPECT_EQ(observed->Reserved0, 0u); - EXPECT_EQ(observed->Reserved1, 0u); + MINIDUMP_MODULE expected_module, observed_module; + memcpy(&expected_module, expected, sizeof(expected_module)); + memcpy(&observed_module, observed, sizeof(observed_module)); - EXPECT_NE(observed->ModuleNameRva, 0u); + EXPECT_EQ(observed_module.BaseOfImage, expected_module.BaseOfImage); + EXPECT_EQ(observed_module.SizeOfImage, expected_module.SizeOfImage); + EXPECT_EQ(observed_module.CheckSum, expected_module.CheckSum); + EXPECT_EQ(observed_module.TimeDateStamp, expected_module.TimeDateStamp); + EXPECT_EQ(observed_module.VersionInfo.dwSignature, + implicit_cast(VS_FFI_SIGNATURE)); + EXPECT_EQ(observed_module.VersionInfo.dwStrucVersion, + implicit_cast(VS_FFI_STRUCVERSION)); + EXPECT_EQ(observed_module.VersionInfo.dwFileVersionMS, + expected_module.VersionInfo.dwFileVersionMS); + EXPECT_EQ(observed_module.VersionInfo.dwFileVersionLS, + expected_module.VersionInfo.dwFileVersionLS); + EXPECT_EQ(observed_module.VersionInfo.dwProductVersionMS, + expected_module.VersionInfo.dwProductVersionMS); + EXPECT_EQ(observed_module.VersionInfo.dwProductVersionLS, + expected_module.VersionInfo.dwProductVersionLS); + EXPECT_EQ(observed_module.VersionInfo.dwFileFlagsMask, + expected_module.VersionInfo.dwFileFlagsMask); + EXPECT_EQ(observed_module.VersionInfo.dwFileFlags, + expected_module.VersionInfo.dwFileFlags); + EXPECT_EQ(observed_module.VersionInfo.dwFileOS, + expected_module.VersionInfo.dwFileOS); + EXPECT_EQ(observed_module.VersionInfo.dwFileType, + expected_module.VersionInfo.dwFileType); + EXPECT_EQ(observed_module.VersionInfo.dwFileSubtype, + expected_module.VersionInfo.dwFileSubtype); + EXPECT_EQ(observed_module.VersionInfo.dwFileDateMS, + expected_module.VersionInfo.dwFileDateMS); + EXPECT_EQ(observed_module.VersionInfo.dwFileDateLS, + expected_module.VersionInfo.dwFileDateLS); + + uint64_t reserved0, reserved1; + memcpy(&reserved0, &observed_module.Reserved0, sizeof(reserved0)); + memcpy(&reserved1, &observed_module.Reserved1, sizeof(reserved1)); + + EXPECT_EQ(reserved0, 0u); + EXPECT_EQ(reserved1, 0u); + + EXPECT_NE(observed_module.ModuleNameRva, 0u); base::string16 observed_module_name_utf16 = - MinidumpStringAtRVAAsString(file_contents, observed->ModuleNameRva); + MinidumpStringAtRVAAsString(file_contents, observed_module.ModuleNameRva); base::string16 expected_module_name_utf16 = base::UTF8ToUTF16(expected_module_name); EXPECT_EQ(observed_module_name_utf16, expected_module_name_utf16); - ASSERT_NO_FATAL_FAILURE(ExpectCodeViewRecord(&observed->CvRecord, + ASSERT_NO_FATAL_FAILURE(ExpectCodeViewRecord(&observed_module.CvRecord, file_contents, expected_pdb_name, expected_pdb_uuid, expected_pdb_timestamp, expected_pdb_age)); - ASSERT_NO_FATAL_FAILURE(ExpectMiscellaneousDebugRecord(&observed->MiscRecord, - file_contents, - expected_debug_name, - expected_debug_type, - expected_debug_utf16)); + ASSERT_NO_FATAL_FAILURE( + ExpectMiscellaneousDebugRecord(&observed_module.MiscRecord, + file_contents, + expected_debug_name, + expected_debug_type, + expected_debug_utf16)); } // ExpectModuleWithBuildIDCv() is like ExpectModule( but expects the module to @@ -300,8 +312,13 @@ void ExpectModuleWithBuildIDCv(const MINIDUMP_MODULE* expected, expected->VersionInfo.dwFileDateMS); EXPECT_EQ(observed->VersionInfo.dwFileDateLS, expected->VersionInfo.dwFileDateLS); - EXPECT_EQ(observed->Reserved0, 0u); - EXPECT_EQ(observed->Reserved1, 0u); + + uint64_t reserved0, reserved1; + memcpy(&reserved0, &observed->Reserved0, sizeof(reserved0)); + memcpy(&reserved1, &observed->Reserved1, sizeof(reserved1)); + + EXPECT_EQ(reserved0, 0u); + EXPECT_EQ(reserved1, 0u); EXPECT_NE(observed->ModuleNameRva, 0u); base::string16 observed_module_name_utf16 = diff --git a/minidump/minidump_system_info_writer_test.cc b/minidump/minidump_system_info_writer_test.cc index 99c599c1..206a8e41 100644 --- a/minidump/minidump_system_info_writer_test.cc +++ b/minidump/minidump_system_info_writer_test.cc @@ -109,8 +109,11 @@ TEST(MinidumpSystemInfoWriter, Empty) { EXPECT_EQ(system_info->Cpu.X86CpuInfo.VersionInformation, 0u); EXPECT_EQ(system_info->Cpu.X86CpuInfo.FeatureInformation, 0u); EXPECT_EQ(system_info->Cpu.X86CpuInfo.AMDExtendedCpuFeatures, 0u); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[0], 0u); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[1], 0u); + + CPU_INFORMATION other_cpu_info; + memcpy(&other_cpu_info, &system_info->Cpu, sizeof(other_cpu_info)); + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[0], 0u); + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[1], 0u); EXPECT_EQ(csd_version->Buffer[0], '\0'); } @@ -234,10 +237,11 @@ TEST(MinidumpSystemInfoWriter, AMD64_Mac) { EXPECT_EQ(system_info->BuildNumber, kOSVersionBuild); EXPECT_EQ(system_info->PlatformId, kOS); EXPECT_EQ(system_info->SuiteMask, 0u); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[0], - kCPUFeatures[0]); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[1], - kCPUFeatures[1]); + + CPU_INFORMATION other_cpu_info; + memcpy(&other_cpu_info, &system_info->Cpu, sizeof(other_cpu_info)); + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[0], kCPUFeatures[0]); + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[1], kCPUFeatures[1]); } TEST(MinidumpSystemInfoWriter, X86_CPUVendorFromRegisters) { @@ -457,9 +461,12 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_AMD64) { EXPECT_EQ(system_info->BuildNumber, expect_system_info.BuildNumber); EXPECT_EQ(system_info->PlatformId, expect_system_info.PlatformId); EXPECT_EQ(system_info->SuiteMask, expect_system_info.SuiteMask); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[0], + + CPU_INFORMATION other_cpu_info; + memcpy(&other_cpu_info, &system_info->Cpu, sizeof(other_cpu_info)); + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[0], expect_system_info.Cpu.OtherCpuInfo.ProcessorFeatures[0]); - EXPECT_EQ(system_info->Cpu.OtherCpuInfo.ProcessorFeatures[1], + EXPECT_EQ(other_cpu_info.OtherCpuInfo.ProcessorFeatures[1], expect_system_info.Cpu.OtherCpuInfo.ProcessorFeatures[1]); for (size_t index = 0; index < strlen(kOSVersionBuild); ++index) { diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index e682b53d..3f3d56e8 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/minidump/minidump_thread_writer_test.cc @@ -108,33 +108,41 @@ void ExpectThread(const MINIDUMP_THREAD* expected, const std::string& file_contents, const MINIDUMP_MEMORY_DESCRIPTOR** stack, const void** context_base) { - EXPECT_EQ(observed->ThreadId, expected->ThreadId); - EXPECT_EQ(observed->SuspendCount, expected->SuspendCount); - EXPECT_EQ(observed->PriorityClass, expected->PriorityClass); - EXPECT_EQ(observed->Priority, expected->Priority); - EXPECT_EQ(observed->Teb, expected->Teb); + MINIDUMP_THREAD expected_thread, observed_thread; + memcpy(&expected_thread, expected, sizeof(expected_thread)); + memcpy(&observed_thread, observed, sizeof(observed_thread)); - EXPECT_EQ(observed->Stack.StartOfMemoryRange, - expected->Stack.StartOfMemoryRange); - EXPECT_EQ(observed->Stack.Memory.DataSize, expected->Stack.Memory.DataSize); + EXPECT_EQ(observed_thread.ThreadId, expected_thread.ThreadId); + EXPECT_EQ(observed_thread.SuspendCount, expected_thread.SuspendCount); + EXPECT_EQ(observed_thread.PriorityClass, expected_thread.PriorityClass); + EXPECT_EQ(observed_thread.Priority, expected_thread.Priority); + EXPECT_EQ(observed_thread.Teb, expected_thread.Teb); + + EXPECT_EQ(observed_thread.Stack.StartOfMemoryRange, + expected_thread.Stack.StartOfMemoryRange); + EXPECT_EQ(observed_thread.Stack.Memory.DataSize, + expected_thread.Stack.Memory.DataSize); if (stack) { - ASSERT_NE(observed->Stack.Memory.DataSize, 0u); - ASSERT_NE(observed->Stack.Memory.Rva, 0u); + ASSERT_NE(observed_thread.Stack.Memory.DataSize, 0u); + ASSERT_NE(observed_thread.Stack.Memory.Rva, 0u); ASSERT_GE(file_contents.size(), - observed->Stack.Memory.Rva + observed->Stack.Memory.DataSize); + observed_thread.Stack.Memory.Rva + + observed_thread.Stack.Memory.DataSize); *stack = &observed->Stack; } else { - EXPECT_EQ(observed->Stack.StartOfMemoryRange, 0u); - EXPECT_EQ(observed->Stack.Memory.DataSize, 0u); - EXPECT_EQ(observed->Stack.Memory.Rva, 0u); + EXPECT_EQ(observed_thread.Stack.StartOfMemoryRange, 0u); + EXPECT_EQ(observed_thread.Stack.Memory.DataSize, 0u); + EXPECT_EQ(observed_thread.Stack.Memory.Rva, 0u); } - EXPECT_EQ(observed->ThreadContext.DataSize, expected->ThreadContext.DataSize); - ASSERT_NE(observed->ThreadContext.DataSize, 0u); - ASSERT_NE(observed->ThreadContext.Rva, 0u); + EXPECT_EQ(observed_thread.ThreadContext.DataSize, + expected_thread.ThreadContext.DataSize); + ASSERT_NE(observed_thread.ThreadContext.DataSize, 0u); + ASSERT_NE(observed_thread.ThreadContext.Rva, 0u); ASSERT_GE(file_contents.size(), - observed->ThreadContext.Rva + expected->ThreadContext.DataSize); - *context_base = &file_contents[observed->ThreadContext.Rva]; + observed_thread.ThreadContext.Rva + + expected_thread.ThreadContext.DataSize); + *context_base = &file_contents[observed_thread.ThreadContext.Rva]; } TEST(MinidumpThreadWriter, OneThread_x86_NoStack) { diff --git a/minidump/test/minidump_memory_writer_test_util.cc b/minidump/test/minidump_memory_writer_test_util.cc index f198abeb..4e38cfa6 100644 --- a/minidump/test/minidump_memory_writer_test_util.cc +++ b/minidump/test/minidump_memory_writer_test_util.cc @@ -38,12 +38,20 @@ void TestMinidumpMemoryWriter::SetShouldFailRead(bool should_fail) { void ExpectMinidumpMemoryDescriptor( const MINIDUMP_MEMORY_DESCRIPTOR* expected, const MINIDUMP_MEMORY_DESCRIPTOR* observed) { - EXPECT_EQ(observed->StartOfMemoryRange, expected->StartOfMemoryRange); - EXPECT_EQ(observed->Memory.DataSize, expected->Memory.DataSize); - if (expected->Memory.Rva != 0) { + MINIDUMP_MEMORY_DESCRIPTOR expected_descriptor; + MINIDUMP_MEMORY_DESCRIPTOR observed_descriptor; + + memcpy(&expected_descriptor, expected, sizeof(expected_descriptor)); + memcpy(&observed_descriptor, observed, sizeof(observed_descriptor)); + + EXPECT_EQ(observed_descriptor.StartOfMemoryRange, + expected_descriptor.StartOfMemoryRange); + EXPECT_EQ(observed_descriptor.Memory.DataSize, + expected_descriptor.Memory.DataSize); + if (expected_descriptor.Memory.Rva != 0) { constexpr uint32_t kMemoryAlignment = 16; - EXPECT_EQ(observed->Memory.Rva, - (expected->Memory.Rva + kMemoryAlignment - 1) & + EXPECT_EQ(observed_descriptor.Memory.Rva, + (expected_descriptor.Memory.Rva + kMemoryAlignment - 1) & ~(kMemoryAlignment - 1)); } } diff --git a/snapshot/BUILD.gn b/snapshot/BUILD.gn index 79f5eeb1..82f8dd76 100644 --- a/snapshot/BUILD.gn +++ b/snapshot/BUILD.gn @@ -239,8 +239,6 @@ crashpad_static_library("snapshot") { public_configs = [ "..:crashpad_config" ] - configs = [ "..:disable_ubsan" ] - public_deps = [ ":context" ] deps = [ diff --git a/third_party/googletest/BUILD.gn b/third_party/googletest/BUILD.gn index 4ad44a43..f51db0e9 100644 --- a/third_party/googletest/BUILD.gn +++ b/third_party/googletest/BUILD.gn @@ -28,7 +28,6 @@ if (crashpad_is_in_chromium) { group("googletest") { testonly = true public_deps = [ "//third_party/googletest:gtest" ] - public_configs = [ "../..:disable_ubsan" ] } group("googlemock") { testonly = true diff --git a/util/BUILD.gn b/util/BUILD.gn index 00622ef2..adedf954 100644 --- a/util/BUILD.gn +++ b/util/BUILD.gn @@ -565,8 +565,6 @@ crashpad_static_library("util") { public_configs = [ "..:crashpad_config" ] - configs = [ "..:disable_ubsan" ] - # Include generated files starting with "util". if (crashpad_is_in_fuchsia) { include_dirs = [ "$root_gen_dir/third_party/crashpad" ] diff --git a/util/file/file_io.cc b/util/file/file_io.cc index c1da6722..3bd86cc4 100644 --- a/util/file/file_io.cc +++ b/util/file/file_io.cc @@ -66,11 +66,12 @@ class FileIOWriteAll final : public internal::WriteAllInternal { namespace internal { bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) { - char* buffer_c = static_cast(buffer); + uintptr_t buffer_int = reinterpret_cast(buffer); size_t total_bytes = 0; size_t remaining = size; while (remaining > 0) { - FileOperationResult bytes_read = Read(buffer_c, remaining, can_log); + FileOperationResult bytes_read = + Read(reinterpret_cast(buffer_int), remaining, can_log); if (bytes_read < 0) { return false; } @@ -81,7 +82,7 @@ bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) { break; } - buffer_c += bytes_read; + buffer_int += bytes_read; remaining -= bytes_read; total_bytes += bytes_read; } @@ -96,17 +97,18 @@ bool ReadExactlyInternal::ReadExactly(void* buffer, size_t size, bool can_log) { } bool WriteAllInternal::WriteAll(const void* buffer, size_t size) { - const char* buffer_c = static_cast(buffer); + uintptr_t buffer_int = reinterpret_cast(buffer); while (size > 0) { - FileOperationResult bytes_written = Write(buffer_c, size); + FileOperationResult bytes_written = + Write(reinterpret_cast(buffer_int), size); if (bytes_written < 0) { return false; } DCHECK_NE(bytes_written, 0); - buffer_c += bytes_written; + buffer_int += bytes_written; size -= bytes_written; } diff --git a/util/net/http_transport_socket.cc b/util/net/http_transport_socket.cc index 0451d2cd..ba9c25d0 100644 --- a/util/net/http_transport_socket.cc +++ b/util/net/http_transport_socket.cc @@ -415,7 +415,7 @@ bool WriteRequest(Stream* stream, } } - write_start = buf.crlf - size_len; + write_start = static_cast(buf.crlf) - size_len; write_size = size_len + sizeof(buf.crlf) + data_bytes + kCRLFSize; } else { // When not using chunked encoding, only use buf.data.