This is an archive of the discontinued LLVM Phabricator instance.

[ASan] Introduce the dump_instruction_bytes flag to print the faulting instruction upon SIGSEGV
ClosedPublic

Authored by glider on Sep 3 2014, 5:24 AM.

Details

Summary

When dump_instruction_bytes=1 and the instruction pointer doesn't point to the zero page, ASan prints 16 bytes starting at the instruction point.
When the instruction pointer points to the zero page, print a warning about that.
Also allow deadly signals to be received in signal handlers (previously ASan would just crash upon the second SEGV)

Diff Detail

Event Timeline

glider updated this revision to Diff 13201.Sep 3 2014, 5:24 AM
glider retitled this revision from to [ASan] Introduce the dump_instruction_bytes flag to print the faulting instruction upon SIGSEGV.
glider updated this object.
glider edited the test plan for this revision. (Show Details)
glider added reviewers: kcc, eugenis.
glider added a subscriber: Unknown Object (MLST).

What if instruction pc points to is not in the zero page, but is inaccessible? It would be sad to crash the program while printing ASan error report in this case.

lib/asan/asan_report.cc
162

You may check that pc < GetPageSizeCached() here.

658

We print pc above... do you really need this?

lib/asan/asan_rtl.cc
234

Wait, where is the default value for this flag?

lib/sanitizer_common/sanitizer_posix_libcdep.cc
151 ↗(On Diff #13201)

Can this go as a separate change?

What if instruction pc points to is not in the zero page, but is inaccessible? It would be sad to crash the program while printing ASan error report in this case.

glider added a comment.Sep 4 2014, 2:53 AM

What if instruction pc points to is not in the zero page, but is inaccessible? It would be sad to crash the program while printing ASan error report in this case.

If pc points to inaccessible page, then we're in a SEGV handler caused by this. We can't print anything meaningful besides the stack, and _Unwind_Backtrace() will crash on this pc right away.

lib/asan/asan_report.cc
162

Done.

658

Many people do not understand that this is just a SEGV handler, so they keep asking whether this crash is a false positive and why ASan doesn't print more info about it.
I'm not insisting on this piece, however.

lib/asan/asan_rtl.cc
234

Done.

lib/sanitizer_common/sanitizer_posix_libcdep.cc
151 ↗(On Diff #13201)

Done.

glider updated this revision to Diff 13242.Sep 4 2014, 2:58 AM

Addressed Alexey's comments

samsonov accepted this revision.Sep 4 2014, 12:00 PM
samsonov edited edge metadata.

Alright, let's do this if you feel it would improve user experience in certain cases.

This revision is now accepted and ready to land.Sep 4 2014, 12:00 PM

I take back my argument about crashes in _Unwind_Backtrace.
Apparently at least on OSX unwinding the stack from the unmapped page doesn't necessarily crash the unwinder. Thus we probably need to unprotect the faulting page before trying to dump instruction bytes.

Meanwhile I'll land the "zero page" hint separately.

glider updated this revision to Diff 13872.Sep 19 2014, 8:07 AM
glider edited edge metadata.

Call IsAccessibleMemoryRange() to make sure we don't step on unaccessible memory.
Disable the test on ARM.

LGTM

lib/asan/asan_report.cc
168

s/char/u8 ?

glider closed this revision.Sep 22 2014, 5:08 AM

r218243, thanks!