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 <vapier@chromium.org>
This commit is contained in:
Jon Turney 2017-03-11 19:02:43 +00:00 committed by Mike Frysinger
parent dac2223398
commit 7ec3caf6c7
4 changed files with 28 additions and 6 deletions

View File

@ -922,9 +922,21 @@ bool CrashGenerationServer::GenerateDump(const ClientInfo& client,
client.assert_info(), client.assert_info(),
client.dump_type(), client.dump_type(),
true); true);
if (!dump_generator.GenerateDumpFile(dump_path)) { if (!dump_generator.GenerateDumpFile(dump_path)) {
return false; 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(); return dump_generator.WriteMinidump();
} }

View File

@ -271,12 +271,14 @@ MinidumpGenerator::MinidumpGenerator(
dump_type_(dump_type), dump_type_(dump_type),
is_client_pointers_(is_client_pointers), is_client_pointers_(is_client_pointers),
dump_path_(dump_path), dump_path_(dump_path),
uuid_generated_(false),
dump_file_(INVALID_HANDLE_VALUE), dump_file_(INVALID_HANDLE_VALUE),
full_dump_file_(INVALID_HANDLE_VALUE), full_dump_file_(INVALID_HANDLE_VALUE),
dump_file_is_internal_(false), dump_file_is_internal_(false),
full_dump_file_is_internal_(false), full_dump_file_is_internal_(false),
additional_streams_(NULL), additional_streams_(NULL),
callback_info_(NULL) { callback_info_(NULL) {
uuid_ = {0};
InitializeCriticalSection(&module_load_sync_); InitializeCriticalSection(&module_load_sync_);
InitializeCriticalSection(&get_proc_address_sync_); InitializeCriticalSection(&get_proc_address_sync_);
} }
@ -562,15 +564,17 @@ MinidumpGenerator::UuidCreateType MinidumpGenerator::GetCreateUuid() {
} }
bool MinidumpGenerator::GenerateDumpFilePath(wstring* file_path) { bool MinidumpGenerator::GenerateDumpFilePath(wstring* file_path) {
UUID id = {0}; if (!uuid_generated_) {
UuidCreateType create_uuid = GetCreateUuid(); UuidCreateType create_uuid = GetCreateUuid();
if (!create_uuid) { if (!create_uuid) {
return false; return false;
} }
create_uuid(&id); create_uuid(&uuid_);
wstring id_str = GUIDString::GUIDToWString(&id); uuid_generated_ = true;
}
wstring id_str = GUIDString::GUIDToWString(&uuid_);
*file_path = dump_path_ + TEXT("\\") + id_str + TEXT(".dmp"); *file_path = dump_path_ + TEXT("\\") + id_str + TEXT(".dmp");
return true; return true;

View File

@ -168,6 +168,10 @@ class MinidumpGenerator {
// Folder path to store dump files. // Folder path to store dump files.
std::wstring dump_path_; std::wstring dump_path_;
// UUID used to make dump file names.
UUID uuid_;
bool uuid_generated_;
// The file where the dump will be written. // The file where the dump will be written.
HANDLE dump_file_; HANDLE dump_file_;

View File

@ -247,6 +247,7 @@ TEST_F(ExceptionHandlerTest, InvalidParameterMiniDumpTest) {
EXPECT_EXIT(DoCrashInvalidParameter(), ::testing::ExitedWithCode(0), ""); EXPECT_EXIT(DoCrashInvalidParameter(), ::testing::ExitedWithCode(0), "");
ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty()); ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty());
ASSERT_TRUE(DoesPathExist(dump_file.c_str())); ASSERT_TRUE(DoesPathExist(dump_file.c_str()));
ASSERT_TRUE(DoesPathExist(full_dump_file.c_str()));
// Verify the dump for infos. // Verify the dump for infos.
DumpAnalysis mini(dump_file); DumpAnalysis mini(dump_file);
@ -318,6 +319,7 @@ TEST_F(ExceptionHandlerTest, PureVirtualCallMiniDumpTest) {
EXPECT_EXIT(DoCrashPureVirtualCall(), ::testing::ExitedWithCode(0), ""); EXPECT_EXIT(DoCrashPureVirtualCall(), ::testing::ExitedWithCode(0), "");
ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty()); ASSERT_TRUE(!dump_file.empty() && !full_dump_file.empty());
ASSERT_TRUE(DoesPathExist(dump_file.c_str())); ASSERT_TRUE(DoesPathExist(dump_file.c_str()));
ASSERT_TRUE(DoesPathExist(full_dump_file.c_str()));
// Verify the dump for infos. // Verify the dump for infos.
DumpAnalysis mini(dump_file); DumpAnalysis mini(dump_file);