From 7ec3caf6c7cdff0a99cd34249acfdc61f76f2d86 Mon Sep 17 00:00:00 2001 From: Jon Turney Date: Sat, 11 Mar 2017 19:02:43 +0000 Subject: Fix Windows client ExceptionHandlerTest tests ExceptionHandlerTest.InvalidParameterMiniDumpTest and ExceptionHandlerTest.PureVirtualCallMiniDumpTest both also exercise a feature that if the MiniDumpWithFullMemory MINIDUMP_TYPE is used, both UUID.dmp and UUID-full.dmp files are written. This is currently broken, and requesting a minidump with MiniDumpWithFullMemory MINIDUMP_TYPE fails, as the file handle for the full dump is not set. Call GenerateFullDumpFile() if MiniDumpWithFullMemory is requested, to generate a filename for the full dump file and set the file handle. Currently GenerateFullDumpFile() also generates another UUID for the full dump filename, so also make the private method MinidumpGenerator::GenerateDumpFilePath() idempotent (so the same UUID is reused) (Note that calling Generate(|Full)DumpFile() more than once is not permitted, so there's no behaviour where this changed the UUID to preserve) BUG= Change-Id: I74304f38b398f53da1c24f368dedfba8463da9e5 Reviewed-on: https://chromium-review.googlesource.com/452978 Reviewed-by: Mike Frysinger --- .../windows/crash_generation/crash_generation_server.cc | 12 ++++++++++++ .../windows/crash_generation/minidump_generator.cc | 16 ++++++++++------ src/client/windows/crash_generation/minidump_generator.h | 4 ++++ src/client/windows/unittests/exception_handler_test.cc | 2 ++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc index bb0968fe..0af213ba 100644 --- a/src/client/windows/crash_generation/crash_generation_server.cc +++ b/src/client/windows/crash_generation/crash_generation_server.cc @@ -922,9 +922,21 @@ bool CrashGenerationServer::GenerateDump(const ClientInfo& client, client.assert_info(), client.dump_type(), true); + if (!dump_generator.GenerateDumpFile(dump_path)) { return false; } + + // If the client requests a full memory dump, we will write a normal mini + // dump and a full memory dump. Both dump files use the same uuid as file + // name prefix. + if (client.dump_type() & MiniDumpWithFullMemory) { + std::wstring full_dump_path; + if (!dump_generator.GenerateFullDumpFile(&full_dump_path)) { + return false; + } + } + return dump_generator.WriteMinidump(); } diff --git a/src/client/windows/crash_generation/minidump_generator.cc b/src/client/windows/crash_generation/minidump_generator.cc index 786c9b93..100e365a 100644 --- a/src/client/windows/crash_generation/minidump_generator.cc +++ b/src/client/windows/crash_generation/minidump_generator.cc @@ -271,12 +271,14 @@ MinidumpGenerator::MinidumpGenerator( dump_type_(dump_type), is_client_pointers_(is_client_pointers), dump_path_(dump_path), + uuid_generated_(false), dump_file_(INVALID_HANDLE_VALUE), full_dump_file_(INVALID_HANDLE_VALUE), dump_file_is_internal_(false), full_dump_file_is_internal_(false), additional_streams_(NULL), callback_info_(NULL) { + uuid_ = {0}; InitializeCriticalSection(&module_load_sync_); InitializeCriticalSection(&get_proc_address_sync_); } @@ -562,15 +564,17 @@ MinidumpGenerator::UuidCreateType MinidumpGenerator::GetCreateUuid() { } bool MinidumpGenerator::GenerateDumpFilePath(wstring* file_path) { - UUID id = {0}; + if (!uuid_generated_) { + UuidCreateType create_uuid = GetCreateUuid(); + if (!create_uuid) { + return false; + } - UuidCreateType create_uuid = GetCreateUuid(); - if (!create_uuid) { - return false; + create_uuid(&uuid_); + uuid_generated_ = true; } - create_uuid(&id); - wstring id_str = GUIDString::GUIDToWString(&id); + wstring id_str = GUIDString::GUIDToWString(&uuid_); *file_path = dump_path_ + TEXT("\\") + id_str + TEXT(".dmp"); return true; diff --git a/src/client/windows/crash_generation/minidump_generator.h b/src/client/windows/crash_generation/minidump_generator.h index a3c12305..a707c0bb 100644 --- a/src/client/windows/crash_generation/minidump_generator.h +++ b/src/client/windows/crash_generation/minidump_generator.h @@ -168,6 +168,10 @@ class MinidumpGenerator { // Folder path to store dump files. std::wstring dump_path_; + // UUID used to make dump file names. + UUID uuid_; + bool uuid_generated_; + // The file where the dump will be written. HANDLE dump_file_; diff --git a/src/client/windows/unittests/exception_handler_test.cc b/src/client/windows/unittests/exception_handler_test.cc index 55275323..0e8ee9ff 100644 --- a/src/client/windows/unittests/exception_handler_test.cc +++ b/src/client/windows/unittests/exception_handler_test.cc @@ -247,6 +247,7 @@ TEST_F(ExceptionHandlerTest, InvalidParameterMiniDumpTest) { EXPECT_EXIT(DoCrashInvalidParameter(), ::testing::ExitedWithCode(0), ""); ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty()); ASSERT_TRUE(DoesPathExist(dump_file.c_str())); + ASSERT_TRUE(DoesPathExist(full_dump_file.c_str())); // Verify the dump for infos. DumpAnalysis mini(dump_file); @@ -318,6 +319,7 @@ TEST_F(ExceptionHandlerTest, PureVirtualCallMiniDumpTest) { EXPECT_EXIT(DoCrashPureVirtualCall(), ::testing::ExitedWithCode(0), ""); ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty()); ASSERT_TRUE(DoesPathExist(dump_file.c_str())); + ASSERT_TRUE(DoesPathExist(full_dump_file.c_str())); // Verify the dump for infos. DumpAnalysis mini(dump_file); -- cgit v1.2.1