Disable the signal handlers set by AddressSanitizer.

Disable the signal handlers set by AddressSanitizer.

This seems to be the least invasive way to have the ASan SIGSEGV handler in every process
but the NaCl sandbox.
Calling __asan_disable_signal_handlers() from a process resets the currently installed
signal handler to SIG_DFL and sets AddressSanitizer's handle_segv flag to 0, allowing
to further reinstall any other signal handler.

BUG=nativeclient:2717, address-sanitizer:65

[Alexander Potapenko, 2012-04-16 13:10:58]

Please take a look.
Note that the corresponding bits for Clang are ready, but not landed yet (I'll
do so if we agree this is the correct way to deal with the ASan handlers in the
NaCl process).
When landed, they'll get to Chrome in a week or so (depending on the next Clang
roll).

[Alexander Potapenko, 2012-04-19 12:04:23]

A friendly ping.

[Mark Seaborn, 2012-04-19 16:16:39]

This looks OK in principle.  Can you run a try job, please?  I suspect it won't
test anything useful, though, because ADDRESS_SANITIZER isn't defined by the
NaCl build.

https://chromiumcodereview.appspot.com/10095012/diff/1/src/trusted/service_ru...
File src/trusted/service_runtime/sel_main_chrome.c (right):

https://chromiumcodereview.appspot.com/10095012/diff/1/src/trusted/service_ru...
src/trusted/service_runtime/sel_main_chrome.c:36: #ifdef ADDRESS_SANITIZER
Can you add a comment to explain what this is for, please?

I suspect we also want a TODO in here to say that when NaCl's signal handler is
enabled by default on Linux, we should replace this.  This is what we'd do:

 * NaCl's signal handler should run first;
 * when NaCl's signal handler notices the crash was in trusted code, it passes
control to the previously-registered ASAN signal handler.

Does that match what you'd expect?

What happens on Mac OS X, though?  Does ASAN still use a signal handler, and how
does it interact with Mac Breakpad's use of Mach exception handling?

https://chromiumcodereview.appspot.com/10095012/diff/1/src/trusted/service_ru...
src/trusted/service_runtime/sel_main_chrome.c:37: extern
__asan_disable_signal_handlers();
Surely "void __asan_disable_signal_handlers(void);"?  'extern' is redundant on
function prototypes (and IIRC our presubmit scripts reject it?).

Is there not a header file we can #include to get this?

https://chromiumcodereview.appspot.com/10095012/diff/1/src/trusted/service_ru...
src/trusted/service_runtime/sel_main_chrome.c:155:
__asan_disable_signal_handlers();
Can you add a comment to explain what this is for, please?

I suspect we also want a TODO in here to say that when NaCl's signal handler is
enabled by default on Linux, we should replace this.  This is what we'd do:

 * NaCl's signal handler should run first;
 * when NaCl's signal handler notices the crash was in trusted code, it passes
control to the previously-registered ASAN signal handler.

Does that match what you'd expect?

What happens on Mac OS X, though?  Does ASAN still use a signal handler, and how
does it interact with Mac Breakpad's use of Mach exception handling?

[pasko-google - do not use, 2012-04-19 18:43:50]

On 2012/04/19 16:16:39, Mark Seaborn wrote:
https://chromiumcodereview.appspot.com/10095012/diff/1/src/trusted/service_ru...
> src/trusted/service_runtime/sel_main_chrome.c:155:
> __asan_disable_signal_handlers();
> Can you add a comment to explain what this is for, please?
> 
> I suspect we also want a TODO in here to say that when NaCl's signal handler
is
> enabled by default on Linux, we should replace this.  This is what we'd do:
> 
>  * NaCl's signal handler should run first;
>  * when NaCl's signal handler notices the crash was in trusted code, it passes
> control to the previously-registered ASAN signal handler.
> 
> Does that match what you'd expect?

yes, that is the option we expect and more or less agreed on as per offline
discussion in msk

[Mark Seaborn, 2012-09-27 02:27:32]

I think this change was superseded by a later change, so I will close this
Rietveld issue.

[Alexander Potapenko, 2012-09-27 08:19:32]

On 2012/09/27 02:27:32, Mark Seaborn wrote:
> I think this change was superseded by a later change, so I will close this
> Rietveld issue.

Yes, that's right. Thank you.