From d90b598125322ebc191dd09a3a606cb6426dbbce Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 15 Oct 2015 14:16:27 +0800 Subject: [PATCH] win: Hook up V8 to breakpad This fixes the crashes happens from V8 not caught by the crash reporter, for more context, see http://code.google.com/p/v8/issues/detail?id=3597. Fix #2365. --- atom/app/atom_main_delegate.cc | 8 ++ .../crash_reporter/crash_reporter_win.cc | 124 +++++++++++++++++- .../crash_reporter/crash_reporter_win.h | 6 + 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/atom/app/atom_main_delegate.cc b/atom/app/atom_main_delegate.cc index 44f2048c09..1c35bced3e 100644 --- a/atom/app/atom_main_delegate.cc +++ b/atom/app/atom_main_delegate.cc @@ -19,6 +19,10 @@ #include "content/public/common/content_switches.h" #include "ui/base/resource/resource_bundle.h" +#if defined(OS_WIN) +#include "atom/common/crash_reporter/crash_reporter_win.h" +#endif + namespace atom { namespace { @@ -69,6 +73,10 @@ bool AtomMainDelegate::BasicStartupComplete(int* exit_code) { // Logging with pid and timestamp. logging::SetLogItems(true, false, true, false); +#if defined(OS_WIN) + crash_reporter::SetupV8CodeRangeHook(); +#endif + #if defined(DEBUG) && defined(OS_LINUX) // Enable convient stack printing. base::debug::EnableInProcessStackDumping(); diff --git a/atom/common/crash_reporter/crash_reporter_win.cc b/atom/common/crash_reporter/crash_reporter_win.cc index be096da80e..8ea5b6689c 100644 --- a/atom/common/crash_reporter/crash_reporter_win.cc +++ b/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,8 +43,101 @@ const MINIDUMP_TYPE kSmallDumpType = static_cast( 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( + static_cast(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(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(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(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(start))); +} + +void UnregisterNonABICompliantCodeRange(void* start) { + ExceptionHandlerRecord* record = + reinterpret_cast(start); + + CHECK(RtlDeleteFunctionTable(&record->runtime_function)); +} +#endif // _WIN64 + } // namespace +void SetupV8CodeRangeHook() { + gin::Debug::SetCodeRangeCreatedCallback(RegisterNonABICompliantCodeRange); + gin::Debug::SetCodeRangeDeletedCallback(UnregisterNonABICompliantCodeRange); +} + CrashReporterWin::CrashReporterWin() { } @@ -63,14 +175,12 @@ 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))); @@ -83,6 +193,14 @@ 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, diff --git a/atom/common/crash_reporter/crash_reporter_win.h b/atom/common/crash_reporter/crash_reporter_win.h index 72b9411d21..1725ca3283 100644 --- a/atom/common/crash_reporter/crash_reporter_win.h +++ b/atom/common/crash_reporter/crash_reporter_win.h @@ -17,6 +17,9 @@ template struct DefaultSingletonTraits; namespace crash_reporter { +// Hook up V8 to breakpad. +void SetupV8CodeRangeHook(); + class CrashReporterWin : public CrashReporter { public: static CrashReporterWin* GetInstance(); @@ -29,6 +32,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;