From f14eda221fe73959b8a71d50448643e330df5976 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Fri, 24 Mar 2017 18:06:08 -0400 Subject: [PATCH] win: Be more careful about exit codes in end_to_end_test.py This is like 270490ff79df, but for things run by end_to_end_test.py, and things run for it by crash_other_program.exe. Change-Id: Iabf3c762c50f41eb61ab31f714c646364196e745 Reviewed-on: https://chromium-review.googlesource.com/458822 Commit-Queue: Mark Mentovai Reviewed-by: Scott Graham --- handler/win/crash_other_program.cc | 45 +++++++++++++++++------- snapshot/win/end_to_end_test.py | 55 ++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 31 deletions(-) diff --git a/handler/win/crash_other_program.cc b/handler/win/crash_other_program.cc index 012b4ba3..ced82859 100644 --- a/handler/win/crash_other_program.cc +++ b/handler/win/crash_other_program.cc @@ -18,6 +18,7 @@ #include "base/files/file_path.h" #include "base/logging.h" +#include "base/strings/stringprintf.h" #include "client/crashpad_client.h" #include "test/paths.h" #include "test/win/child_launcher.h" @@ -29,20 +30,21 @@ namespace crashpad { namespace test { namespace { +constexpr DWORD kCrashAndDumpTargetExitCode = 0xdeadbea7; + bool CrashAndDumpTarget(const CrashpadClient& client, HANDLE process) { DWORD target_pid = GetProcessId(process); - HANDLE thread_snap_raw = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0); - if (thread_snap_raw == INVALID_HANDLE_VALUE) { - LOG(ERROR) << "CreateToolhelp32Snapshot"; + ScopedFileHANDLE thread_snap(CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0)); + if (!thread_snap.is_valid()) { + PLOG(ERROR) << "CreateToolhelp32Snapshot"; return false; } - ScopedFileHANDLE thread_snap(thread_snap_raw); THREADENTRY32 te32; te32.dwSize = sizeof(THREADENTRY32); if (!Thread32First(thread_snap.get(), &te32)) { - LOG(ERROR) << "Thread32First"; + PLOG(ERROR) << "Thread32First"; return false; } @@ -55,9 +57,12 @@ bool CrashAndDumpTarget(const CrashpadClient& client, HANDLE process) { if (te32.tpBasePri == 9) { ScopedKernelHANDLE thread( OpenThread(kXPThreadAllAccess, false, te32.th32ThreadID)); + if (!thread.is_valid()) { + PLOG(ERROR) << "OpenThread"; + return false; + } if (!client.DumpAndCrashTargetProcess( - process, thread.get(), 0xdeadbea7)) { - LOG(ERROR) << "DumpAndCrashTargetProcess failed"; + process, thread.get(), kCrashAndDumpTargetExitCode)) { return false; } return true; @@ -65,6 +70,7 @@ bool CrashAndDumpTarget(const CrashpadClient& client, HANDLE process) { } } while (Thread32Next(thread_snap.get(), &te32)); + LOG(ERROR) << "target not found"; return false; } @@ -73,7 +79,7 @@ int CrashOtherProgram(int argc, wchar_t* argv[]) { if (argc == 2 || argc == 3) { if (!client.SetHandlerIPCPipe(argv[1])) { - LOG(ERROR) << "SetHandler"; + LOG(ERROR) << "SetHandlerIPCPipe"; return EXIT_FAILURE; } } else { @@ -96,15 +102,28 @@ int CrashOtherProgram(int argc, wchar_t* argv[]) { return EXIT_FAILURE; } + DWORD expect_exit_code; if (argc == 3 && wcscmp(argv[2], L"noexception") == 0) { - client.DumpAndCrashTargetProcess(child.process_handle(), 0, 0); - return EXIT_SUCCESS; + expect_exit_code = CrashpadClient::kTriggeredExceptionCode; + if (!client.DumpAndCrashTargetProcess(child.process_handle(), 0, 0)) + return EXIT_FAILURE; } else { - if (CrashAndDumpTarget(client, child.process_handle())) - return EXIT_SUCCESS; + expect_exit_code = kCrashAndDumpTargetExitCode; + if (!CrashAndDumpTarget(client, child.process_handle())) { + return EXIT_FAILURE; + } } - return EXIT_FAILURE; + DWORD exit_code = child.WaitForExit(); + if (exit_code != expect_exit_code) { + LOG(ERROR) << base::StringPrintf( + "incorrect exit code, expected 0x%x, observed 0x%x", + expect_exit_code, + exit_code); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; } } // namespace diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 42999df8..5afdac38 100755 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -22,6 +22,7 @@ import subprocess import sys import tempfile import time +import win32con g_temp_dirs = [] @@ -80,23 +81,23 @@ def GetCdbPath(): return None -def GetDumpFromProgram(out_dir, pipe_name, executable_name, *args): +def GetDumpFromProgram( + out_dir, pipe_name, executable_name, expect_exit_code, *args): """Initialize a crash database, and run |executable_name| connecting to a crash handler. If pipe_name is set, crashpad_handler will be started first. If pipe_name is empty, the executable is responsible for starting crashpad_handler. *args will be passed after other arguments to - executable_name. Returns the minidump generated by crashpad_handler for - further testing. + executable_name. If the child process does not exit with |expect_exit_code|, + an exception will be raised. Returns the path to the minidump generated by + crashpad_handler for further testing. """ test_database = MakeTempDir() handler = None try: - if subprocess.call( + subprocess.check_call( [os.path.join(out_dir, 'crashpad_database_util.exe'), '--create', - '--database=' + test_database]) != 0: - print 'could not initialize report database' - return None + '--database=' + test_database]) if pipe_name is not None: handler = subprocess.Popen([ @@ -113,12 +114,16 @@ def GetDumpFromProgram(out_dir, pipe_name, executable_name, *args): printed = True time.sleep(0.1) - subprocess.call([os.path.join(out_dir, executable_name), pipe_name] + - list(args)) + exit_code = subprocess.call( + [os.path.join(out_dir, executable_name), pipe_name] + list(args)) else: - subprocess.call([os.path.join(out_dir, executable_name), - os.path.join(out_dir, 'crashpad_handler.com'), - test_database] + list(args)) + exit_code = subprocess.call( + [os.path.join(out_dir, executable_name), + os.path.join(out_dir, 'crashpad_handler.com'), + test_database] + + list(args)) + if exit_code != expect_exit_code: + raise CalledProcessError(exit_code, executable_name) out = subprocess.check_output([ os.path.join(out_dir, 'crashpad_database_util.exe'), @@ -135,24 +140,38 @@ def GetDumpFromProgram(out_dir, pipe_name, executable_name, *args): def GetDumpFromCrashyProgram(out_dir, pipe_name): - return GetDumpFromProgram(out_dir, pipe_name, 'crashy_program.exe') + return GetDumpFromProgram(out_dir, + pipe_name, + 'crashy_program.exe', + win32con.EXCEPTION_ACCESS_VIOLATION) def GetDumpFromOtherProgram(out_dir, pipe_name, *args): - return GetDumpFromProgram(out_dir, pipe_name, 'crash_other_program.exe', - *args) + return GetDumpFromProgram( + out_dir, pipe_name, 'crash_other_program.exe', 0, *args) def GetDumpFromSignal(out_dir, pipe_name, *args): - return GetDumpFromProgram(out_dir, pipe_name, 'crashy_signal.exe', *args) + STATUS_FATAL_APP_EXIT = 0x40000015 # Not known by win32con. + return GetDumpFromProgram(out_dir, + pipe_name, + 'crashy_signal.exe', + STATUS_FATAL_APP_EXIT, + *args) def GetDumpFromSelfDestroyingProgram(out_dir, pipe_name): - return GetDumpFromProgram(out_dir, pipe_name, 'self_destroying_program.exe') + return GetDumpFromProgram(out_dir, + pipe_name, + 'self_destroying_program.exe', + win32con.EXCEPTION_BREAKPOINT) def GetDumpFromZ7Program(out_dir, pipe_name): - return GetDumpFromProgram(out_dir, pipe_name, 'crashy_z7_loader.exe') + return GetDumpFromProgram(out_dir, + pipe_name, + 'crashy_z7_loader.exe', + win32con.EXCEPTION_ACCESS_VIOLATION) class CdbRun(object):