1410868 - Mac CrashGenerationClient should send exception context to server

Status: NEW

(Toolkit :: Crash Reporting, enhancement)

Description

[(not currently active) Ted Mielczarek]

Currently the Mac CrashGenerationClient doesn't send the actual exception context over to the server. `ExceptionHandler::WriteMinidumpWithException` takes a `task_context` parameter, but it doesn't get passed to `CrashGenerationClient::RequestDumpForException`:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc#375

I suspect this is because in the non-SIGABRT case it works fine--the actual crashing thread is halted (because it has crashed), so the minidump generator simply uses the CPU context it gets from the system for that thread as the exception context. For the SIGABRT case this falls down, though, because the top of the stack is Breakpad sending a message to the crash server, and the signal handler below that. Here's an example:
https://crash-stats.mozilla.com/report/index/a2e5766e-5d9c-4972-967b-13f1e0171019

In that crash, the top of the stack for the crashing thread should really be frame 10, `abort`.

We've never noticed this before because before spohl fixed bug 1405151 we didn't get useful crash reports for these SIGABRT crashes in child processes.

This should be possible to fix by passing the CPU context into `CrashGenerationClient::RequestDumpForException`:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc#375

adding it to the `ExceptionInfo` struct that gets sent in the IPC message:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h#50

copying the context data into that struct and sending it in the Mach message:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc#52

Then in `CrashGenerationServer::WaitForOneMessage`, the server should take the context from that struct and call `MinidumpGenerator::SetTaskContext` to pass it into the minidump-writing code:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/handler/minidump_generator.h#108
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.cc#122

It looks like we might also need to tweak the `MinidumpGenerator` code, as it seems to currently assume that a context will only ever be passed in to be used when writing out the dump info for the current thread:
https://dxr.mozilla.org/mozilla-central/rev/69e24c678a28dc46a4c1bda3ff04b2f6106ff71a/toolkit/crashreporter/breakpad-client/mac/handler/minidump_generator.cc#896

I think we could change that to be `if (task_context_ && target_thread == exception_thread_)` instead without harming anything. We'd need to test the in-process crashing case to make sure that doesn't break anything, and probably some scenarios where we write minidumps of a process without a crash involved (like hangs).