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 <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>

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);
+  ScopedFileHANDLE thread_snap(CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0));
-  if (thread_snap_raw == INVALID_HANDLE_VALUE) {
+  if (!thread_snap.is_valid()) {
-    LOG(ERROR) << "CreateToolhelp32Snapshot";
+    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)) {
+                process, thread.get(), kCrashAndDumpTargetExitCode)) {
-          LOG(ERROR) << "DumpAndCrashTargetProcess failed";
           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);
+    expect_exit_code = CrashpadClient::kTriggeredExceptionCode;
-    return EXIT_SUCCESS;
+    if (!client.DumpAndCrashTargetProcess(child.process_handle(), 0, 0))
+      return EXIT_FAILURE;
   } else {
-    if (CrashAndDumpTarget(client, child.process_handle()))
+    expect_exit_code = kCrashAndDumpTargetExitCode;
-      return EXIT_SUCCESS;
+    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

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
+  executable_name. If the child process does not exit with |expect_exit_code|,
-  further testing.
+  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:
+         '--database=' + test_database])
-      print 'could not initialize report database'
-      return None
 
     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] +
+      exit_code = subprocess.call(
-                      list(args))
+          [os.path.join(out_dir, executable_name), pipe_name] + list(args))
     else:
-      subprocess.call([os.path.join(out_dir, executable_name),
+      exit_code = subprocess.call(
-                       os.path.join(out_dir, 'crashpad_handler.com'),
+          [os.path.join(out_dir, executable_name),
-                       test_database] + list(args))
+           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',
+  return GetDumpFromProgram(
-                            *args)
+      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):