[runtime] Don't call mono_arch_handle_exception() when chaining
authorT.J. Purtell <tj@mobisocial.us>
Fri, 15 Aug 2014 20:43:36 +0000 (13:43 -0700)
committerJonathan Pryor <jonpryor@vt.edu>
Tue, 26 Aug 2014 03:54:36 +0000 (23:54 -0400)
commit5d07b77a67f61576318a30e8b1c5f65f7f26b1cf
tree9ff419d221e2c4430f79f611eb939441fda44293
parent9e7713121b36cf961c4ccd596c57734eaac6e911
[runtime] Don't call mono_arch_handle_exception() when chaining

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=18763#c12
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=18763#c14
Context: http://stackoverflow.com/a/1789879/83444

mono_set_crash_chaining() was added in 8ffc4070 in order to better
support Android crash logs.

Crash logging in Android is "weird" (something we didn't know at the
time of 8ffc4070); when a process crashes on Android, ideally:

 1. The Android signal handler is executed,
 2. Bionic will attempt to connect to /system/bin/debuggerd.
 3. debuggerd will try to connect to the crashing process, then
    retrieve "useful" information from the crashing process (stack
    trace, register values, etc.)

The problem with 8ffc4070 was that there is a "window" between steps
(1) and (3) during which the crashing process continues to execute but
debuggerd hasn't attached to the process. It is possible that the
crashing process can thus exit before debuggerd has had a chance to
attach, killing the purpose of the crash logs.

Case in point: consider mono_sigsegv_signal_handler(). In 8ffc4070,
if mono_do_crash_chaining is TRUE, then Mono will execute:

  mono_handle_native_sigsegv (SIGSEGV, ctx);
  mono_chain_signal (SIG_HANDLER_PARAMS);
  mono_arch_handle_exception (ctx, NULL);

Because mono_chain_signal() doesn't block, abort the process, or
otherwise do anything to prevent execution from continuing, execution
will hit mono_arch_handle_exception(), which will exit the process.
If you're unlucky, this will happen before debuggerd has had a chance
to attach to the process.

The obvious fix is to prevent mono_arch_handle_exception() from being
invoked. The question is, *how*? jonp suggested [0] that after
mono_chain_signal() returns then _exit(2) or a busy loop could be
invoked. Unfortunately, _exit(2) makes the process exit explicit, and
~reliably prevents debuggerd from doing anything.

A busy loop doesn't work because it just causes the process to hang
within signal handler context, and debuggerd is never able to log
anything about the crashed process.

It would seem that the signal handler itself must exit without causing
the process to exit so that debuggerd has a chance to do ANYTHING,
otherwise nothing will happen reliably.

Thanks to T.J. Purtell for investigating and providing the patch.

[0]: https://github.com/mono/mono/pull/1206#issuecomment-52357802

--- Original commit message [formatted] ---

re #18763: After the chained handler executes, it may return to allow
the crashed instruction to be restarted or it may have taken
corrective action so the instruction can be resumed successfully.

For example, bionic resets the signal handler for SIGSEGV to default
expecting that the kernel will restart the offending instruction and
cause the kernel to abort the process.  Don't do mono exception
handling after chaining to support these types of signal management.
mono/mini/mini.c