Page MenuHomePhabricator

[asan] On OS X, log reports to syslog and os_trace
AbandonedPublic

Authored by kubamracek on Aug 12 2015, 8:43 AM.

Details

Summary

When ASan currently detects a bug, by default it will only print out the text of the report to stderr. This patch changes this behavior and writes the full text of the report to syslog before we terminate the process. It also calls os_trace (Activity Tracing available on OS X and iOS) with a message saying that the report is available in syslog. This is useful, because this message will be shown in the crash log.

For this to work, the patch makes sure we store the full report into error_message_buffer unconditionally, and it also strips out ANSI escape sequences from the report (they are used when producing colored reports).

Diff Detail

Event Timeline

kubamracek updated this revision to Diff 31945.Aug 12 2015, 8:43 AM
kubamracek retitled this revision from to [asan] On OS X, log reports to syslog and os_trace.
kubamracek updated this object.
kubamracek added reviewers: samsonov, glider, kcc.
kubamracek added subscribers: llvm-commits, samsonov, glider and 2 others.
filcab added a subscriber: filcab.Aug 12 2015, 9:23 AM
filcab added inline comments.
lib/asan/asan_report.cc
708

Is this error_message_buffer_size == 0 something we should reasonably expect?

kubamracek updated this revision to Diff 31951.Aug 12 2015, 9:47 AM

Is this error_message_buffer_size == 0 something we should reasonably expect?

Not really. At this point, something was definitely already written to the message buffer. Removed the check.

kcc added inline comments.Aug 12 2015, 2:48 PM
lib/asan/asan_report.cc
631

OMG, no.
Instead of removing the ANSI escapes can't we just not emit them?

707

Please move large ifdef-ed chunks of code into separate functions

zaks.anna added inline comments.Aug 12 2015, 3:39 PM
lib/asan/asan_report.cc
631

We want the colored report to be printed but store the color-less report in the syslog. The buffer we get here, 'error_message_buffer', is exactly what gets printed. So simply using a different decorator would not work, unless we want to generate the report twice..

kcc added inline comments.Aug 12 2015, 3:54 PM
lib/asan/asan_report.cc
631

So, you say that a user is going to let asan print the output to screen (not to file!!!)
and will still need to have the sys log entry?

Well, ok...

Please write a unit test for it (probably in sanitizer_common_test.cc)

kcc edited edge metadata.Aug 12 2015, 3:58 PM

also, consider moving all of the mac-specific code to sanitizer_mac.cc

samsonov added inline comments.Aug 12 2015, 4:30 PM
lib/asan/asan_report.cc
53

Is this function called under some mutex? Can we check for that?

721

So, does it mean that error_report_callback is invoked with ANSI escape sequences? Maybe, we don't need them there either?

So, does it mean that error_report_callback is invoked with ANSI escape sequences? Maybe, we don't need
them there either?

Yes and I don't think it would be easy to change. Here is my understanding of how this works. The users of Printf decide how everything should be decorated. Sometimes the messages are added to a temporary buffer with the ANSI escape sequences already in place even before calling Printf.
Ex:
static void DescribeAccessToHeapChunk(AsanChunkView chunk, uptr addr,

                                    uptr access_size) {
sptr offset;
Decorator d;
InternalScopedString str(4096);
str.append("%s", d.Location());
str.append("%s", d.Location());
if (chunk.AddrIsAtLeft(addr, access_size, &offset)) {
  str.append("%p is located %zd bytes to the left of", (void *)addr, offset);
...
str.append("%s", d.EndLocation());
Printf("%s", str.data());

}
The error_report_callback callback is called at the last step in the implementation of SharedPrintfCode; we are passed the same buffer and have no control over the decoration at that point:

RawWrite(buffer);
if (common_flags()->log_to_syslog)
  WriteToSyslog(buffer);
CallPrintfAndReportCallback(buffer);

Interesting.. looks like logging to syslog for Android is done directly right there on every Printf and the logging is done with ANSI in place...

If we log there, everything not only "error reports", will be logged.
Also, should we consider logging line by line, syslog does not work well with new line characters?

kcc added a comment.Aug 12 2015, 6:18 PM

Interesting.. looks like logging to syslog for Android is done directly right there on every Printf and the logging is done with ANSI in place...

I don't think that we ever print error messages on android to the screen,
i.e. the ANSI colors are disabled and not printed at all

glider added inline comments.Aug 13 2015, 5:02 AM
lib/asan/asan_report.cc
631

We can store the report in a linked list of strings with ANSI characters emitted to separate strings and marked somehow. Then we can easily reconstruct the report without markup.

kcc added inline comments.Aug 17 2015, 6:04 PM
lib/asan/asan_report.cc
631

OMG, no!

I was looking at this and… Shouldn't something like this be in sanitizer_common, and then all the sanitizers could use it?
It seems to me that our method of reporting errors is different from sanitizer to sanitizer, and changes like this would need to be done again for other sanitizers, which doesn't seem very nice.

kcc added a comment.Aug 24 2015, 1:09 PM

I was looking at this and… Shouldn't something like this be in sanitizer_common, and then all the sanitizers could use it?
It seems to me that our method of reporting errors is different from sanitizer to sanitizer, and changes like this would need to be done again for other sanitizers, which doesn't seem very nice.

Yes, as I've also mentioned above we should move most of this Mac-specific code to sanitizer_mac.cc (which is in sanitizer_common).