fix: use crashpad on Windows (#18483)

* Initial changes to use crashpad for windows

* Remove crashpad patch

* Report error when failed to connect pipe

* Allow crashpad to communicate with named pipe

* Add patch to make crashpad named pipe work

* Windows also needs crashReporter on main process

* Call SetUnhandledExceptionFilter in node process

Node can also use crash reporter.

* Do not treat node process as browser process

* No more need to manually start crash service

* Use base::StringPrintf for better readbility

* Print error when pipe name not available

* Make sure pipe name is updated

Note that the crashpad may be started after renderer process gets
created.

* Fix some tests

* Update node

* Exclude crashpad files on Linux and MAS

* Fix lint warning

* Remove unused checks

* kCrashpadPipeName is only available on Windows

* Fix uploadToServer tests

* Fix extra params tests

* Fix getCrashesDirectory tests

* Run crashReporter tests on CI

* Style fixes

* Update crashreporter docs

* Rename InitBreakpad to Init

* Add comment for process_type_.empty() and UTF16ToASCII to UTF16ToUTF8.

* Update build.gn include crashpad headers

* Address comment https://github.com/electron/electron/pull/18483#discussion_r290887898

* Avoid using api::WebContents

* Put kRunAsNode in atom_constants

* Remove duplicate settings on upload params

* Fix building on macOS

* Update description for crashpad_pid_check.patch
This commit is contained in:
Nitish Sakhawalkar 2019-06-12 23:42:21 -07:00 committed by Cheng Zhao
parent ddec3c0e78
commit f98454e5dd
39 changed files with 561 additions and 812 deletions

View file

@ -67,7 +67,6 @@ fix_disable_usage_of_setapplicationisdaemon_and.patch
disable_network_services_by_default.patch
unsandboxed_ppapi_processes_skip_zygote.patch
patch_the_ensure_gn_version_py_script_to_work_on_mac_ci.patch
partially_revert_fb4ab3be3fc0024d00358d5b7816d3215a8ff20c.patch
build_add_electron_tracing_category.patch
add_contentgpuclient_precreatemessageloop_callback.patch
disable_custom_libcxx_on_windows.patch
@ -77,3 +76,4 @@ fix_breakpad_symbol_generation_on_linux_arm.patch
frame_host_manager.patch
cross_site_document_resource_handler.patch
woa_compiler_workaround.patch
crashpad_pid_check.patch

View file

@ -0,0 +1,38 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Tue Jun 4 11:30:12 JST 2019
Subject: crashpad_pid_check.patch
When both browser process and renderer process are connecting to the pipe,
the API may return the PID of browser process as real_pid, which is different
from the PID of renderer process.
This is caused by the crashReporter getting started after the sanbox, after
we redesign crashReporter's API to make it alwasy start before the
sanbox, we can remove this patch.
See following links for more:
https://github.com/electron/electron/pull/18483#discussion_r292703588
https://github.com/electron/electron/pull/18483#issuecomment-501090683
diff --git a/third_party/crashpad/crashpad/util/win/exception_handler_server.cc b/third_party/crashpad/crashpad/util/win/exception_handler_server.cc
index 2593ff2de032..e89b8ff675be 100644
--- a/third_party/crashpad/crashpad/util/win/exception_handler_server.cc
+++ b/third_party/crashpad/crashpad/util/win/exception_handler_server.cc
@@ -448,9 +448,16 @@ bool ExceptionHandlerServer::ServiceClientConnection(
DWORD real_pid = 0;
if (get_named_pipe_client_process_id(service_context.pipe(), &real_pid) &&
message.registration.client_process_id != real_pid) {
+ // Electron: When both browser process and renderer process are connecting
+ // to the pipe, the API may return the PID of browser process as real_pid,
+ // which is different from the PID of renderer process.
+ //
+ // I don't understand why Chromium does not have this issue.
+#if 0
LOG(ERROR) << "forged client pid, real pid: " << real_pid
<< ", got: " << message.registration.client_process_id;
return false;
+#endif
}
}

View file

@ -1,141 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Nitish Sakhawalkar <nitsakh@icloud.com>
Date: Fri, 19 Apr 2019 14:01:12 -0700
Subject: Partially revert fb4ab3be3fc0024d00358d5b7816d3215a8ff20c
This change partially reverts the changes made in gin in fb4ab3be3fc0024d00358d5b7816d3215a8ff20c. Electron uses breakpad for windows, while chromium has moved to crashpad. Once electron uses crashpad for windows, we can get rid of this change.
diff --git a/gin/debug_impl.cc b/gin/debug_impl.cc
index ca0577ea4caabce42bb4ec1aad8062b59eaaa8e4..5c3b7ffc932f063c8ad458d92643564edba393dc 100644
--- a/gin/debug_impl.cc
+++ b/gin/debug_impl.cc
@@ -8,6 +8,10 @@ namespace gin {
namespace {
v8::JitCodeEventHandler g_jit_code_event_handler = NULL;
+#if defined(OS_WIN)
+Debug::CodeRangeCreatedCallback g_code_range_created_callback = NULL;
+Debug::CodeRangeDeletedCallback g_code_range_deleted_callback = NULL;
+#endif
} // namespace
// static
@@ -17,9 +21,13 @@ void Debug::SetJitCodeEventHandler(v8::JitCodeEventHandler event_handler) {
#if defined(OS_WIN)
// static
-void Debug::SetUnhandledExceptionCallback(
- v8::UnhandledExceptionCallback callback) {
- v8::V8::SetUnhandledExceptionCallback(callback);
+void Debug::SetCodeRangeCreatedCallback(CodeRangeCreatedCallback callback) {
+ g_code_range_created_callback = callback;
+}
+
+// static
+void Debug::SetCodeRangeDeletedCallback(CodeRangeDeletedCallback callback) {
+ g_code_range_deleted_callback = callback;
}
#endif
@@ -28,4 +36,16 @@ v8::JitCodeEventHandler DebugImpl::GetJitCodeEventHandler() {
return g_jit_code_event_handler;
}
+#if defined(OS_WIN)
+// static
+Debug::CodeRangeCreatedCallback DebugImpl::GetCodeRangeCreatedCallback() {
+ return g_code_range_created_callback;
+}
+
+// static
+Debug::CodeRangeDeletedCallback DebugImpl::GetCodeRangeDeletedCallback() {
+ return g_code_range_deleted_callback;
+}
+#endif
+
} // namespace gin
diff --git a/gin/debug_impl.h b/gin/debug_impl.h
index b0b7931f3e10592577858f048eab37d4eff4c18f..b88c0b6c0896f60c6421023fe5783b614d596769 100644
--- a/gin/debug_impl.h
+++ b/gin/debug_impl.h
@@ -13,6 +13,10 @@ namespace gin {
class DebugImpl {
public:
static v8::JitCodeEventHandler GetJitCodeEventHandler();
+#if defined(OS_WIN)
+ static Debug::CodeRangeCreatedCallback GetCodeRangeCreatedCallback();
+ static Debug::CodeRangeDeletedCallback GetCodeRangeDeletedCallback();
+#endif
};
} // namespace gin
diff --git a/gin/isolate_holder.cc b/gin/isolate_holder.cc
index 4dd48f6a871cccb374c58adacb8ad9a80da89a5a..ec61b873d4e2dcdca833c8503beabb88d0798f2d 100644
--- a/gin/isolate_holder.cc
+++ b/gin/isolate_holder.cc
@@ -91,9 +91,31 @@ IsolateHolder::IsolateHolder(
isolate_memory_dump_provider_.reset(
new V8IsolateMemoryDumpProvider(this, task_runner));
+#if defined(OS_WIN)
+ {
+ void* code_range;
+ size_t size;
+ isolate_->GetCodeRange(&code_range, &size);
+ Debug::CodeRangeCreatedCallback callback =
+ DebugImpl::GetCodeRangeCreatedCallback();
+ if (code_range && size && callback)
+ callback(code_range, size);
+ }
+#endif
}
IsolateHolder::~IsolateHolder() {
+#if defined(OS_WIN)
+ {
+ void* code_range;
+ size_t size;
+ isolate_->GetCodeRange(&code_range, &size);
+ Debug::CodeRangeDeletedCallback callback =
+ DebugImpl::GetCodeRangeDeletedCallback();
+ if (code_range && callback)
+ callback(code_range);
+ }
+#endif
isolate_memory_dump_provider_.reset();
isolate_data_.reset();
isolate_->Dispose();
diff --git a/gin/public/debug.h b/gin/public/debug.h
index 4e567876f7ac0f010783eded5e57f8b2293542d8..8c2eee341c3bc64331926e258e1fd20080373a80 100644
--- a/gin/public/debug.h
+++ b/gin/public/debug.h
@@ -24,11 +24,25 @@ class GIN_EXPORT Debug {
static void SetJitCodeEventHandler(v8::JitCodeEventHandler event_handler);
#if defined(OS_WIN)
- /* Sets a callback that is invoked for exceptions that arise in V8-generated
- * code (jitted code or embedded builtins).
+ typedef void (__cdecl *CodeRangeCreatedCallback)(void*, size_t);
+
+ /* Sets a callback that is invoked for every new code range being created.
+ *
+ * On Win64, exception handling in jitted code is broken due to the fact
+ * that JS stack frames are not ABI compliant. It is possible to install
+ * custom handlers for the code range which holds the jitted code to work
+ * around this issue.
+ *
+ * https://code.google.com/p/v8/issues/detail?id=3598
+ */
+ static void SetCodeRangeCreatedCallback(CodeRangeCreatedCallback callback);
+
+ typedef void (__cdecl *CodeRangeDeletedCallback)(void*);
+
+ /* Sets a callback that is invoked for every previously registered code range
+ * when it is deleted.
*/
- static void SetUnhandledExceptionCallback(
- v8::UnhandledExceptionCallback callback);
+ static void SetCodeRangeDeletedCallback(CodeRangeDeletedCallback callback);
#endif
};