Merge pull request #3092 from atom/crash-reporter

Fix problems of crash-reporter on Windows

atom/app/atom_main_delegate.cc

@@ -21,6 +21,15 @@
 
 namespace atom {
 
+namespace {
+
+bool IsBrowserProcess(base::CommandLine* cmd) {
+  std::string process_type = cmd->GetSwitchValueASCII(switches::kProcessType);
+  return process_type.empty();
+}
+
+}  // namespace
+
 AtomMainDelegate::AtomMainDelegate() {
 }
 
@@ -28,11 +37,14 @@ AtomMainDelegate::~AtomMainDelegate() {
 }
 
 bool AtomMainDelegate::BasicStartupComplete(int* exit_code) {
+  auto command_line = base::CommandLine::ForCurrentProcess();
+
   logging::LoggingSettings settings;
 #if defined(OS_WIN)
   // On Windows the terminal returns immediately, so we add a new line to
   // prevent output in the same line as the prompt.
-  std::wcout << std::endl;
+  if (IsBrowserProcess(command_line))
+    std::wcout << std::endl;
 #if defined(DEBUG)
   // Print logging to debug.log on Windows
   settings.logging_dest = logging::LOG_TO_ALL;
@@ -47,7 +59,6 @@ bool AtomMainDelegate::BasicStartupComplete(int* exit_code) {
 #endif  // !defined(OS_WIN)
 
   // Only enable logging when --enable-logging is specified.
-  auto command_line = base::CommandLine::ForCurrentProcess();
   if (!command_line->HasSwitch(switches::kEnableLogging)) {
     settings.logging_dest = logging::LOG_NONE;
     logging::SetMinLogLevel(logging::LOG_NUM_SEVERITIES);
@@ -83,7 +94,7 @@ void AtomMainDelegate::PreSandboxStartup() {
   }
 
   // Only append arguments for browser process.
-  if (!process_type.empty())
+  if (!IsBrowserProcess(command_line))
     return;
 
 #if defined(OS_WIN)

atom/browser/native_window.cc

@@ -116,7 +116,8 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) {
   } else if (options.Get(switches::kCenter, &center) && center) {
     Center();
   }
-  extensions::SizeConstraints size_constraints;
+  // On Linux and Window we may already have maximum size defined.
+  extensions::SizeConstraints size_constraints(GetContentSizeConstraints());
   int min_height = 0, min_width = 0;
   if (options.Get(switches::kMinHeight, &min_height) |
       options.Get(switches::kMinWidth, &min_width)) {

atom/browser/native_window_views.cc

@@ -390,7 +390,10 @@ gfx::Size NativeWindowViews::GetContentSize() {
 void NativeWindowViews::SetContentSizeConstraints(
     const extensions::SizeConstraints& size_constraints) {
   NativeWindow::SetContentSizeConstraints(size_constraints);
-  window_->OnSizeConstraintsChanged();
+  // widget_delegate() is only available after Init() is called, we make use of
+  // this to determine whether native widget has initialized.
+  if (window_ && window_->widget_delegate())
+    window_->OnSizeConstraintsChanged();
 #if defined(USE_X11)
   if (resizable_)
     old_size_constraints_ = size_constraints;

atom/common/crash_reporter/crash_reporter_win.cc

@@ -11,6 +11,25 @@
 #include "base/memory/singleton.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/public/common/result_codes.h"
+#include "gin/public/debug.h"
+#include "sandbox/win/src/nt_internals.h"
+
+#pragma intrinsic(_AddressOfReturnAddress)
+#pragma intrinsic(_ReturnAddress)
+
+#ifdef _WIN64
+// See http://msdn.microsoft.com/en-us/library/ddssxxy8.aspx
+typedef struct _UNWIND_INFO {
+  unsigned char Version : 3;
+  unsigned char Flags : 5;
+  unsigned char SizeOfProlog;
+  unsigned char CountOfCodes;
+  unsigned char FrameRegister : 4;
+  unsigned char FrameOffset : 4;
+  ULONG ExceptionHandler;
+} UNWIND_INFO, *PUNWIND_INFO;
+#endif
 
 namespace crash_reporter {
 
@@ -24,6 +43,94 @@ const MINIDUMP_TYPE kSmallDumpType = static_cast<MINIDUMP_TYPE>(
 const wchar_t kWaitEventFormat[] = L"$1CrashServiceWaitEvent";
 const wchar_t kPipeNameFormat[] = L"\\\\.\\pipe\\$1 Crash Service";
 
+typedef NTSTATUS (WINAPI* NtTerminateProcessPtr)(HANDLE ProcessHandle,
+                                                 NTSTATUS ExitStatus);
+char* g_real_terminate_process_stub = NULL;
+
+void TerminateProcessWithoutDump() {
+  // Patched stub exists based on conditions (See InitCrashReporter).
+  // As a side note this function also gets called from
+  // WindowProcExceptionFilter.
+  if (g_real_terminate_process_stub == NULL) {
+    ::TerminateProcess(::GetCurrentProcess(), content::RESULT_CODE_KILLED);
+  } else {
+    NtTerminateProcessPtr real_terminate_proc =
+        reinterpret_cast<NtTerminateProcessPtr>(
+            static_cast<char*>(g_real_terminate_process_stub));
+    real_terminate_proc(::GetCurrentProcess(), content::RESULT_CODE_KILLED);
+  }
+}
+
+#ifdef _WIN64
+int CrashForExceptionInNonABICompliantCodeRange(
+    PEXCEPTION_RECORD ExceptionRecord,
+    ULONG64 EstablisherFrame,
+    PCONTEXT ContextRecord,
+    PDISPATCHER_CONTEXT DispatcherContext) {
+  EXCEPTION_POINTERS info = { ExceptionRecord, ContextRecord };
+  if (!CrashReporter::GetInstance())
+    return EXCEPTION_CONTINUE_SEARCH;
+  return static_cast<CrashReporterWin*>(CrashReporter::GetInstance())->
+      CrashForException(&info);
+}
+
+struct ExceptionHandlerRecord {
+  RUNTIME_FUNCTION runtime_function;
+  UNWIND_INFO unwind_info;
+  unsigned char thunk[12];
+};
+
+void RegisterNonABICompliantCodeRange(void* start, size_t size_in_bytes) {
+  ExceptionHandlerRecord* record =
+      reinterpret_cast<ExceptionHandlerRecord*>(start);
+
+  // We assume that the first page of the code range is executable and
+  // committed and reserved for breakpad. What could possibly go wrong?
+
+  // All addresses are 32bit relative offsets to start.
+  record->runtime_function.BeginAddress = 0;
+  record->runtime_function.EndAddress =
+      base::checked_cast<DWORD>(size_in_bytes);
+  record->runtime_function.UnwindData =
+      offsetof(ExceptionHandlerRecord, unwind_info);
+
+  // Create unwind info that only specifies an exception handler.
+  record->unwind_info.Version = 1;
+  record->unwind_info.Flags = UNW_FLAG_EHANDLER;
+  record->unwind_info.SizeOfProlog = 0;
+  record->unwind_info.CountOfCodes = 0;
+  record->unwind_info.FrameRegister = 0;
+  record->unwind_info.FrameOffset = 0;
+  record->unwind_info.ExceptionHandler =
+      offsetof(ExceptionHandlerRecord, thunk);
+
+  // Hardcoded thunk.
+  // mov imm64, rax
+  record->thunk[0] = 0x48;
+  record->thunk[1] = 0xb8;
+  void* handler = &CrashForExceptionInNonABICompliantCodeRange;
+  memcpy(&record->thunk[2], &handler, 8);
+
+  // jmp rax
+  record->thunk[10] = 0xff;
+  record->thunk[11] = 0xe0;
+
+  // Protect reserved page against modifications.
+  DWORD old_protect;
+  CHECK(VirtualProtect(
+      start, sizeof(ExceptionHandlerRecord), PAGE_EXECUTE_READ, &old_protect));
+  CHECK(RtlAddFunctionTable(
+      &record->runtime_function, 1, reinterpret_cast<DWORD64>(start)));
+}
+
+void UnregisterNonABICompliantCodeRange(void* start) {
+  ExceptionHandlerRecord* record =
+      reinterpret_cast<ExceptionHandlerRecord*>(start);
+
+  CHECK(RtlDeleteFunctionTable(&record->runtime_function));
+}
+#endif  // _WIN64
+
 }  // namespace
 
 CrashReporterWin::CrashReporterWin() {
@@ -63,26 +170,46 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name,
   // to allow any previous handler to detach in the correct order.
   breakpad_.reset();
 
-  int handler_types = google_breakpad::ExceptionHandler::HANDLER_EXCEPTION |
-      google_breakpad::ExceptionHandler::HANDLER_PURECALL;
   breakpad_.reset(new google_breakpad::ExceptionHandler(
       temp_dir.value(),
       FilterCallback,
       MinidumpCallback,
       this,
-      handler_types,
+      google_breakpad::ExceptionHandler::HANDLER_ALL,
       kSmallDumpType,
       pipe_name.c_str(),
       GetCustomInfo(product_name, version, company_name)));
 
   if (!breakpad_->IsOutOfProcess())
     LOG(ERROR) << "Cannot initialize out-of-process crash handler";
+
+#ifdef _WIN64
+  // Hook up V8 to breakpad.
+  {
+    // gin::Debug::SetCodeRangeCreatedCallback only runs the callback when
+    // Isolate is just created, so we have to manually run following code here.
+    void* code_range = nullptr;
+    size_t size = 0;
+    v8::Isolate::GetCurrent()->GetCodeRange(&code_range, &size);
+    if (code_range && size)
+      RegisterNonABICompliantCodeRange(code_range, size);
+  }
+  gin::Debug::SetCodeRangeDeletedCallback(UnregisterNonABICompliantCodeRange);
+#endif
 }
 
 void CrashReporterWin::SetUploadParameters() {
   upload_parameters_["platform"] = "win32";
 }
 
+int CrashReporterWin::CrashForException(EXCEPTION_POINTERS* info) {
+  if (breakpad_) {
+    breakpad_->WriteMinidumpForException(info);
+    TerminateProcessWithoutDump();
+  }
+  return EXCEPTION_CONTINUE_SEARCH;
+}
+
 // static
 bool CrashReporterWin::FilterCallback(void* context,
                                       EXCEPTION_POINTERS* exinfo,

atom/common/crash_reporter/crash_reporter_win.h

@@ -29,6 +29,9 @@ class CrashReporterWin : public CrashReporter {
                     bool skip_system_crash_handler) override;
   void SetUploadParameters() override;
 
+  // Crashes the process after generating a dump for the provided exception.
+  int CrashForException(EXCEPTION_POINTERS* info);
+
  private:
   friend struct DefaultSingletonTraits<CrashReporterWin>;
 

atom/common/crash_reporter/win/crash_service.cc

@@ -311,7 +311,7 @@ bool CrashService::Initialize(const base::string16& application_name,
   // service is initialized.
   base::string16 wait_name = ReplaceStringPlaceholders(
       kWaitEventFormat, application_name, NULL);
-  HANDLE wait_event = ::CreateEventW(NULL, TRUE, FALSE, wait_name.c_str());
+  HANDLE wait_event = ::CreateEventW(NULL, TRUE, TRUE, wait_name.c_str());
   ::SetEvent(wait_event);
 
   return true;

spec/api-crash-reporter-spec.coffee

@@ -15,9 +15,6 @@ describe 'crash-reporter module', ->
   beforeEach -> w = new BrowserWindow(show: false)
   afterEach -> w.destroy()
 
-  # It is not working on 64bit Windows.
-  return if process.platform is 'win32' and process.arch is 'x64'
-
   # It is not working for mas build.
   return if process.mas