[PATCH] asan: handle SIGABRT

[Todd Lipcon]

================
Comment at: lib/asan/asan_posix.cc:90
@@ +89,3 @@
+  UnmapShadowMappings();
+  InvokeDefaultSignalHandler(SIGABRT);
+}
----------------
Alexander Potapenko wrote:
> Once you unmap the shadow mappings, it's unsafe to invoke the default signal handler, because the instrumented code will try to access the shadow memory.
The default signal handler, though, is in the kernel -- not user code. So, it won't run instrumented code, it'll just generate a core dump and exit with the right return code. Right?

I'm explicitly setting to SIG_DFL, not restoring whatever signal handler the user might have installed.

================
Comment at: lib/asan/asan_report.cc:526
@@ -525,1 +525,3 @@
 
+void ReportSIGABRT(uptr pc, uptr sp, uptr bp) {
+  Decorator d;
----------------
Alexey Samsonov wrote:
> No, generally ASan should fail the program immediately after it prints the error report - that's why all "ReportFoo" functions are noreturn and we have ScopedInErrorReport object. Among the other things, it makes sure that no two error reports are printed simultaneously, otherwise bad things may happen - the code in "ReportFoo" functions is thread-unsafe.
The issue is that if you die with "Die()" then you don't get the core dump at all. I'd like to actually get core dumps on SEGV/ABRT so I can debug things, but I want the core dumps to be _without_ the shadow mapping.

With this patch, you can get this behavior by turning on abort_on_error -- the flow is:

- user code accesses bad memory
-- ReportSEGV generates the report
-- Die() calls abort(), generating a SIGABRT
--- HandleSIGABRT gets called, reports SIGABRT, unmaps memory
--- HandleSIGABRT re-kills self with SIGABRT after setting to SIG_DFL
---- kernel generates core dump (without shadow mappings)
- user happily debugs their application from the corefile

As for ensuring thread-safety, I put that check in the signal handler -- it uses an atomic swap to make sure that only one thread will proceed into this code

================
Comment at: lib/asan/asan_rtl.cc:151
@@ -154,2 +150,3 @@
   f->handle_segv = ASAN_NEEDS_SEGV;
+  f->handle_abrt = ASAN_NEEDS_SEGV;
   f->allow_user_segv_handler = false;
----------------
Alexey Samsonov wrote:
> Why? I don't think SIGABRT handler should be on by default.
Why not? Unmapping the shadow memory on abort makes corefiles possible on 64-bit (and could enable us to then get rid of the other code which sets RLIMIT_CORE->0 on those systems)

My general use case is that I have an integration test build which runs all of my tests with ASAN enabled. Sometimes my tests fail due to a race which is difficult to reproduce, but a corefile would allow us to debug it the next morning. Currently it's impossible to get corefiles out of this build, but with this patch we're able to.


http://llvm-reviews.chandlerc.com/D1123