[chrome_elf] Big ELF cleanup. Part 1.

chrome_elf/crash/crash_helper.cc

Index: chrome_elf/crash/crash_helper.cc
diff --git a/chrome_elf/crash/crash_helper.cc b/chrome_elf/crash/crash_helper.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0d575d05e8a1f40d30a7a7a1331b345361c4074b
--- /dev/null
+++ b/chrome_elf/crash/crash_helper.cc
@@ -0,0 +1,150 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome_elf/crash/crash_helper.h"
+
+#include <assert.h>
+#include <windows.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+#include "chrome/app/chrome_crash_reporter_client_win.h"
+#include "chrome_elf/hook_util/hook_util.h"
+#include "components/crash/content/app/crashpad.h"
+#include "components/crash/core/common/crash_keys.h"
+#include "third_party/crashpad/crashpad/client/crashpad_client.h"
+
+namespace {
+
+// Gets the exe name from the full path of the exe.
+std::wstring GetExeName() {
+  std::wstring file_name;
+  file_name.resize(MAX_PATH);
+  if (!::GetModuleFileNameW(nullptr, &file_name[0], MAX_PATH)) {
+    assert(false);
+    return std::wstring();
+  }
+  size_t last_slash_pos = file_name.find_last_of(L'\\');
+  if (last_slash_pos != std::wstring::npos) {
+    file_name = file_name.substr(last_slash_pos + 1,
+                                 file_name.length() - last_slash_pos);
+  }
+  std::transform(file_name.begin(), file_name.end(), file_name.begin(),
+                 ::tolower);
+  return file_name;
+}
+
+// Global pointer to a vector of crash reports.
+// This structure will be initialized in InitializeCrashReportingForProcess()
+// and cleaned up in DllDetachCrashReportingCleanup().
+std::vector<crash_reporter::Report>* g_crash_reports = nullptr;
+
+// chrome_elf loads early in the process and initializes Crashpad. That in turn
+// uses the SetUnhandledExceptionFilter API to set a top level exception
+// handler for the process. When the process eventually initializes, CRT sets
+// an exception handler which calls TerminateProcess which effectively bypasses
+// us. Ideally we want to be at the top of the unhandled exception filter
+// chain. However we don't have a good way of intercepting the
+// SetUnhandledExceptionFilter API in the sandbox. EAT patching kernel32 or
+// kernelbase should ideally work. However the kernel32 kernelbase dlls are
+// prebound which causes EAT patching to not work. Sidestep works. However it
+// is only supported for 32 bit. For now we use IAT patching for the
+// executable.
+// TODO(ananta).
+// Check if it is possible to fix EAT patching or use sidestep patching for
+// 32 bit and 64 bit for this purpose.
+elf_hook::IATHook g_set_unhandled_exception_filter;

[robertshield, 2016/08/03 04:52:45]

Technically this violates
https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables.

As is, this will be destroyed at some point after DLLMain with
dll_process_detach gets called. It would be cleaner / marginally hypothetically
safer to call Unhook explicitly during dll_process_detach instead of having it
happen during static destruction.

Following from that, you could make this a raw pointer and new / delete it
during process attach / detach. 

[penny, 2016/08/04 18:55:35]

I fully agree.  I was planning to change this to be like g_crash_reports in my
next CL.  Was trying split up many changes into multiple smaller CLs for your
sanity.  ;D  However, it's small to add this now, so done!  Fully agree.

+
+// Hook function, which ignores the request to set an unhandled-exception
+// filter.
+LPTOP_LEVEL_EXCEPTION_FILTER WINAPI
+SetUnhandledExceptionFilterPatch(LPTOP_LEVEL_EXCEPTION_FILTER filter) {
+  // Don't set the exception filter. Please see above for comments.
+  return nullptr;
+}
+
+}  // namespace
+
+//------------------------------------------------------------------------------
+// Public chrome_elf crash APIs
+//------------------------------------------------------------------------------
+
+namespace elf_crash {
+
+// NOTE: This function will be called from DllMain during DLL_PROCESS_ATTACH
+// (while we have the loader lock), so do not misbehave.
+void InitializeCrashReporting() {
+  // We want to initialize crash reporting only in chrome.exe
+  if (GetExeName() != L"chrome.exe") {
+#ifdef _DEBUG
+    assert(false);
+#endif  // _DEBUG
+    return;
+  }
+
+#ifdef _DEBUG
+  assert(g_crash_reports == nullptr);
+#endif  // _DEBUG
+  // No global objects with destructors, so using a global pointer.
+  // DllMain on detach will clean this up.
+  g_crash_reports = new std::vector<crash_reporter::Report>;
+
+  ChromeCrashReporterClient::InitializeCrashReportingForProcess();
+}
+
+// NOTE: This function will be called from DllMain during DLL_PROCESS_DETACH
+// (while we have the loader lock), so do not misbehave.
+void ShutdownCrashReporting() {
+  if (g_crash_reports != nullptr) {
+    g_crash_reports->clear();
+    delete g_crash_reports;
+  }
+}
+
+// Please refer to the comment on g_set_unhandled_exception_filter for more
+// information about why we intercept the SetUnhandledExceptionFilter API.
+void DisableSetUnhandledExceptionFilter() {
+  if (g_set_unhandled_exception_filter.Hook(
+          GetModuleHandle(nullptr), "kernel32.dll",
+          "SetUnhandledExceptionFilter",
+          SetUnhandledExceptionFilterPatch) != NO_ERROR) {
+#ifdef _DEBUG
+    assert(false);
+#endif  //_DEBUG
+  }
+}
+
+int GenerateCrashDump(EXCEPTION_POINTERS* exception_pointers) {
+  crashpad::CrashpadClient::DumpWithoutCrash(
+      *(exception_pointers->ContextRecord));
+  return EXCEPTION_CONTINUE_SEARCH;
+}
+
+}  // namespace elf_crash
+
+//------------------------------------------------------------------------------
+// Exported crash APIs for the rest of the process.
+//------------------------------------------------------------------------------
+
+// This helper is invoked by code in chrome.dll to retrieve the crash reports.
+// See CrashUploadListCrashpad. Note that we do not pass a std::vector here,
+// because we do not want to allocate/free in different modules. The returned
+// pointer is read-only.
+extern "C" __declspec(dllexport) void GetCrashReportsImpl(

[robertshield, 2016/08/03 04:52:45]

Thinking about lifecycle here, it is important to ensure that anyone who calls
this doesn't attempt to use the returned |reports| after chrome_elf.dll has
unloaded (since they'll be referencing memory that we free during DLL unload).
ELF *should* be the last thing unloaded, so this *should* be fine in chrome.exe.


Would need to be careful if this was called in unit tests that dynamically load
/ unload elf. 

This is more of an unactionable musing... only thing I might suggest is adding a
note to the comment here to be careful not to use the returned data after this
DLL has been unloaded. 

[penny, 2016/08/04 18:55:35]

Done.

+    const crash_reporter::Report** reports,
+    size_t* report_count) {
+  crash_reporter::GetReports(g_crash_reports);
+  *reports = g_crash_reports->data();
+  *report_count = g_crash_reports->size();
+}
+
+// This helper is invoked by debugging code in chrome to register the client
+// id.
+extern "C" __declspec(dllexport) void SetMetricsClientId(
+    const char* client_id) {
+  if (client_id)
+    crash_keys::SetMetricsClientIdFromGUID(client_id);
+}