From b4b9e7d9a138b09505cbec28a9a2f6cc9271ed3a Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Wed, 8 Aug 2018 16:45:59 +0000 Subject: [PATCH] Bug 1481009 Part 3 - Report recording/replaying processes crashes as if they happened in the middleman, r=gsvelto. --HG-- extra : rebase_source : fe9d098d759f49773ff81c9c38c02134e464bfcd --- .../crash_generation_client.cc | 10 ++++++++ .../crash_generation_server.cc | 4 +--- .../crash_generation_server.h | 1 + .../mac/crash_generation/moz.build | 1 + toolkit/crashreporter/nsExceptionHandler.cpp | 23 +++++++++++++++++++ toolkit/recordreplay/ipc/DisabledIPC.cpp | 6 +++++ toolkit/recordreplay/ipc/ParentIPC.cpp | 10 ++++++++ toolkit/recordreplay/ipc/ParentIPC.h | 3 +++ 8 files changed, 55 insertions(+), 3 deletions(-) diff --git a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc index c5732d873b6c..adfd30ba7570 100644 --- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc +++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc @@ -32,6 +32,8 @@ #include "mac/crash_generation/crash_generation_server.h" #include "common/mac/MachIPC.h" +#include "mozilla/recordreplay/ChildIPC.h" + namespace google_breakpad { bool CrashGenerationClient::RequestDumpForException( @@ -53,6 +55,14 @@ bool CrashGenerationClient::RequestDumpForException( info.exception_type = exception_type; info.exception_code = exception_code; info.exception_subcode = exception_subcode; + info.child_pid = getpid(); + + // When recording/replaying, associate minidumps with the middleman process + // so that the UI process can find them. + if (mozilla::recordreplay::IsRecordingOrReplaying()) { + info.child_pid = mozilla::recordreplay::child::MiddlemanProcessId(); + } + message.SetData(&info, sizeof(info)); const mach_msg_timeout_t kSendTimeoutMs = 2 * 1000; diff --git a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.cc b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.cc index 59ead321d6ce..f4354ea67842 100644 --- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.cc +++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.cc @@ -110,9 +110,7 @@ bool CrashGenerationServer::WaitForOneMessage() { mach_port_t crashing_thread = message.GetTranslatedPort(1); mach_port_t handler_thread = message.GetTranslatedPort(2); mach_port_t ack_port = message.GetTranslatedPort(3); - pid_t remote_pid = -1; - pid_for_task(remote_task, &remote_pid); - ClientInfo client(remote_pid); + ClientInfo client(info.child_pid); bool result; std::string dump_path; diff --git a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h index 85bd5b5e33e4..c331d77214f1 100644 --- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h +++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h @@ -52,6 +52,7 @@ struct ExceptionInfo { int32_t exception_type; int32_t exception_code; int32_t exception_subcode; + int32_t child_pid; }; class CrashGenerationServer { diff --git a/toolkit/crashreporter/breakpad-client/mac/crash_generation/moz.build b/toolkit/crashreporter/breakpad-client/mac/crash_generation/moz.build index 5932bca5b873..74212f15efb6 100644 --- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/moz.build +++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/moz.build @@ -12,6 +12,7 @@ UNIFIED_SOURCES += [ FINAL_LIBRARY = 'xul' LOCAL_INCLUDES += [ + '/ipc/chromium/src', '/toolkit/crashreporter/breakpad-client', '/toolkit/crashreporter/google-breakpad/src', ] diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp index 1fd7adc06da3..2abc128d90bf 100644 --- a/toolkit/crashreporter/nsExceptionHandler.cpp +++ b/toolkit/crashreporter/nsExceptionHandler.cpp @@ -94,6 +94,7 @@ using mozilla::InjectCrashRunnable; #include "mozilla/IOInterposer.h" #include "mozilla/mozalloc_oom.h" #include "mozilla/WindowsDllBlocklist.h" +#include "mozilla/recordreplay/ParentIPC.h" #if defined(XP_MACOSX) CFStringRef reporterClientAppID = CFSTR("org.mozilla.crashreporter"); @@ -3252,6 +3253,24 @@ OnChildProcessDumpRequested(void* aContext, } } +#ifdef XP_MACOSX + +// Middleman processes do not have their own crash generation server. +// Any crashes in the middleman's children are forwarded to the UI process. +static bool +MaybeForwardCrashesIfMiddleman() +{ + if (recordreplay::IsMiddleman()) { + childCrashNotifyPipe = + mozilla::Smprintf("gecko-crash-server-pipe.%i", + static_cast(recordreplay::parent::ParentProcessId())).release(); + return true; + } + return false; +} + +#endif // XP_MACOSX + static bool OOPInitialized() { @@ -3340,6 +3359,10 @@ OOPInit() &dumpPath); #elif defined(XP_MACOSX) + if (MaybeForwardCrashesIfMiddleman()) { + return; + } + childCrashNotifyPipe = mozilla::Smprintf("gecko-crash-server-pipe.%i", static_cast(getpid())).release(); diff --git a/toolkit/recordreplay/ipc/DisabledIPC.cpp b/toolkit/recordreplay/ipc/DisabledIPC.cpp index e3c0e378806c..3bb0fe7b7026 100644 --- a/toolkit/recordreplay/ipc/DisabledIPC.cpp +++ b/toolkit/recordreplay/ipc/DisabledIPC.cpp @@ -165,6 +165,12 @@ GetArgumentsForChildProcess(base::ProcessId aMiddlemanPid, uint32_t aChannelId, MOZ_CRASH(); } +base::ProcessId +ParentProcessId() +{ + MOZ_CRASH(); +} + } // namespace parent } // namespace recordreplay diff --git a/toolkit/recordreplay/ipc/ParentIPC.cpp b/toolkit/recordreplay/ipc/ParentIPC.cpp index c9f2f01dcede..6a36e8e894db 100644 --- a/toolkit/recordreplay/ipc/ParentIPC.cpp +++ b/toolkit/recordreplay/ipc/ParentIPC.cpp @@ -849,6 +849,14 @@ MainThreadMessageLoop() return gMainThreadMessageLoop; } +static base::ProcessId gParentPid; + +base::ProcessId +ParentProcessId() +{ + return gParentPid; +} + void InitializeMiddleman(int aArgc, char* aArgv[], base::ProcessId aParentPid, const base::SharedMemoryHandle& aPrefsHandle, @@ -856,6 +864,8 @@ InitializeMiddleman(int aArgc, char* aArgv[], base::ProcessId aParentPid, { MOZ_RELEASE_ASSERT(NS_IsMainThread()); + gParentPid = aParentPid; + // Construct the message that will be sent to each child when starting up. IntroductionMessage* msg = IntroductionMessage::New(aParentPid, aArgc, aArgv); diff --git a/toolkit/recordreplay/ipc/ParentIPC.h b/toolkit/recordreplay/ipc/ParentIPC.h index 06c139fdf740..548944da6e8c 100644 --- a/toolkit/recordreplay/ipc/ParentIPC.h +++ b/toolkit/recordreplay/ipc/ParentIPC.h @@ -37,6 +37,9 @@ const char* SaveAllRecordingsDirectory(); // Middleman process API +// Get the pid of the UI process. +base::ProcessId ParentProcessId(); + // Save the recording up to the current point in execution. void SaveRecording(const ipc::FileDescriptor& aFile);