A simple, practically zero cost fallback crash handler for Crashpad handler process.

components/crash/content/app/fallback_crash_handler_launcher_win.cc

Index: components/crash/content/app/fallback_crash_handler_launcher_win.cc
diff --git a/components/crash/content/app/fallback_crash_handler_launcher_win.cc b/components/crash/content/app/fallback_crash_handler_launcher_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..67bc58ff6729ea4ea7ada9130ce9d8719ba21a32
--- /dev/null
+++ b/components/crash/content/app/fallback_crash_handler_launcher_win.cc
@@ -0,0 +1,127 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.

[scottmg, 2017/01/06 18:29:04]

2017

[Sigurður Ásgeirsson, 2017/01/06 20:59:10]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/crash/content/app/fallback_crash_handler_launcher_win.h"
+
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/win/win_util.h"
+
+namespace crash_reporter {
+
+FallbackCrashHandlerLauncher::FallbackCrashHandlerLauncher() {
+  memset(&exception_pointers_, 0, sizeof(exception_pointers_));
+}
+
+FallbackCrashHandlerLauncher::~FallbackCrashHandlerLauncher() {}
+
+bool FallbackCrashHandlerLauncher::Initialize(
+    const base::CommandLine& program,
+    const base::FilePath& crashpad_database) {
+  // Open an inheritable handle to self. This will be inherited to the handler.
+  DWORD kAccessMask =

[scottmg, 2017/01/06 18:29:04]

const DWORD if using k..., otherwise access_mask.

[Sigurður Ásgeirsson, 2017/01/06 20:59:10]

Done.

+      PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | PROCESS_DUP_HANDLE;
+  self_process_handle_.Set(
+      OpenProcess(kAccessMask, TRUE, ::GetCurrentProcessId()));
+  if (!self_process_handle_.IsValid())
+    return false;
+
+  // Setup the startup info for inheriting the self process handle into the
+  // fallback crash handler.
+  if (!startup_info_.InitializeProcThreadAttributeList(1))
+    return false;
+
+  HANDLE raw_self_process_handle = self_process_handle_.Get();
+  if (!startup_info_.UpdateProcThreadAttribute(
+          PROC_THREAD_ATTRIBUTE_HANDLE_LIST, &raw_self_process_handle,
+          sizeof(raw_self_process_handle))) {
+    return false;
+  }
+
+  // Build the command line from a copy of the command line passed in.
+  base::CommandLine cmd_line(program);
+  cmd_line.AppendSwitchPath("database", crashpad_database);
+  cmd_line.AppendSwitchASCII(
+      "exception-pointers",
+      base::UintToString(reinterpret_cast<uintptr_t>(&exception_pointers_)));
+  cmd_line.AppendSwitchASCII(
+      "process", base::UintToString(
+                     base::win::HandleToUint32(self_process_handle_.Get())));
+
+  std::wstring str_cmd_line = cmd_line.GetCommandLineString();
+
+  // Append the for now abortive thread argument manually.
+  str_cmd_line.append(L" --thread=");
+
+  // Store the command line string for easy use later.
+  cmd_line_.assign(str_cmd_line.begin(), str_cmd_line.end());
+
+  // Reserve space for appending the thread id later at crash time, without
+  // requiring an allocation to happen.
+  cmd_line_.reserve(cmd_line_.size() + 32);
+
+  return true;
+}
+
+DWORD FallbackCrashHandlerLauncher::LaunchAndWaitForHandler(
+    EXCEPTION_POINTERS* exception_pointers) {
+  DCHECK(!cmd_line_.empty());
+  DCHECK_EQ('=', cmd_line_.back());
+  // This program has crashed. Try and not use anything but the stack.
+
+  // Append the current thread's ID to the command line, without incurring
+  // an allocation.
+  wchar_t buf[32];
+  int len = wsprintf(buf, L"%d", GetCurrentThreadId());

[scottmg, 2017/01/06 18:29:04]

How about doing a resize() in Initialize() and then wsprintf directly into
&cmd_line_[N], followed by resize()ing down to the final size?

(Just to avoid the 32 bytes on the stack really, I guess, but it makes me feel a
little less likely that something's going to allocate.)

[Sigurður Ásgeirsson, 2017/01/06 20:59:10]

Good idea, done!

+  // Append the thread ID and the terminating zero to the command line string.
+  cmd_line_.insert(cmd_line_.end(), buf, buf + len + 1);
+
+  // Copy the exception pointers to our member variable, whose address is
+  // already baked into the command line.
+  exception_pointers_ = *exception_pointers;
+
+  // Launch the pre-cooked command line.
+  DWORD error = ERROR_SUCCESS;
+  PROCESS_INFORMATION process_info = {};
+  if (!CreateProcess(nullptr,                       // Application name.
+                     &cmd_line_[0],                 // Command line.
+                     nullptr,                       // Process attributes.
+                     nullptr,                       // Thread attributes.
+                     true,                          // Inherit handles.
+                     0,                             // Creation flags.
+                     nullptr,                       // Environment.
+                     nullptr,                       // Current directory.
+                     startup_info_.startup_info(),  // Startup info.
+                     &process_info)) {
+    error = GetLastError();

[scottmg, 2017/01/06 18:29:04]

return GetLastError() seems simpler here, and move the DWORD error declaration
down to the WFSO line.

[Sigurður Ásgeirsson, 2017/01/06 20:59:10]

Done.

+    return error;
+  }
+
+  // Wait on the fallback crash handler process. The expectation is that this
+  // will never return, as the fallback crash handler will terminate this
+  // process. For testing, and full-on belt and suspenders, cover for this
+  // returning.
+  error = WaitForSingleObject(process_info.hProcess, INFINITE);
+  if (error != WAIT_OBJECT_0) {

[scottmg, 2017/01/06 18:29:04]

Please add (or add a note to add) UMA that tracks if this happens. (And maybe
some other things near here that aren't likely to cause additional chaos.)

[Sigurður Ásgeirsson, 2017/01/06 20:59:10]

Added a TODO - maybe it'll be saner to do this from the caller also.

+    // This should never happen, barring handle abuse.
+    NOTREACHED();
+    error = GetLastError();
+  } else {
+    // On successful wait, return the exit code of the fallback crash handler
+    // process.
+    if (!GetExitCodeProcess(process_info.hProcess, &error)) {
+      // This should never happen, barring handle abuse.
+      NOTREACHED();
+      error = GetLastError();
+    }
+  }
+
+  // Close the handles returned from CreateProcess.
+  CloseHandle(process_info.hProcess);
+  CloseHandle(process_info.hThread);
+
+  return error;
+}
+
+}  // namespace crash_reporter