On 22 October 2013 22:53, Kostya Serebryany <[email protected]> wrote: > LGTM > > > On Wed, Oct 23, 2013 at 9:25 AM, Nick Lewycky <[email protected]> wrote: > >> On 22 October 2013 22:10, Kostya Serebryany <[email protected]> wrote: >> >>> >>> On Wed, Oct 23, 2013 at 8:16 AM, Nick Lewycky <[email protected]>wrote: >>> >>>> __sanitizer_report_error_summary is the replaceable interface for >>>> reporting an error. This patch changes asan a tiny bit to always emit a >>>> summary whenever it encounters an error. This is important, otherwise a >>>> library user of asan will not be aware of errors that were merely printed. >>>> >>>> Also, all these summaries are redundant with the non-summarized >>>> printouts. Make the default weak implementation of report error summary not >>>> do anything. This means we need to remove the check for SUMMARY lines from >>>> the tests. This is suboptimal for testing, but printing out these redundant >>>> lines is just bad UI. >>>> >>> >>> But I've added summaries for a reason: they help to cluster bug reports. >>> >> >> But asan prints one error and exits? >> > > Correct. Summaries help cluster reports from different runs. > > >> >> >>> I'd really prefer to keep them by default. >>> >> >> I've attached an updated patch that puts the summaries back. What's left >> in the patch will fix a bug where a subprocess may catch an asan error and >> die, but that error would be lost by the parent process. >> > > This happens only if the subprocess does not have a symbolizer, right? >
Right. This patch will make asan print summaries with abrakadabra if there is not > symbolizer. > Could be totally fine though. > With no symbolizer, the results look like: SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 ?? which is pretty lousy, but does contain *some* information. We can certainly change what the summary says in the case where there is no The only way to make sure we catch errors in all subprocesses is to make >> sure they call into sanitizer_report_error_summary if there's any error. >> >> And we need ubsan to do this for the same reason. UBSan already has >> one-line errors. We want to print those, preferably without "SUMMARY:" >> before them. >> > > [in another patch?] we can move printing SUMMARY: from > __sanitizer_report_error_summary to ReportErrorSummary > and add an additional "const char *prefix" parameter to > ReportErrorSummary. asan/tsan/msan will call it with prefix="SUMMARY: " > Okay, that works. But there's one other difference Richard brought up: ReportErrorSummary expects complete error text, while printing is assumed to be continually appending. UBSan manages to print out each piece of the error individually and never allocates any buffers. I could change UBSan's Printf calls to instead keep appending to an internal buffer and then feed the buffer to __sanitizer_report_error_summary when we're done. I just wanted to point out the difference in these APIs, in case you can think of a way we could do this without require a buffer somewhere. btw, why not use phabricator for reviews :) ? > I have to search for instructions every time I want to use it. When it works it's fine, but I had a fit reviewing D1960 where double-clicking the line number wouldn't let me enter comments, even though I was logged in as the reviewer. Absolutely maddening. Anyhow, I'll put the next patch in phab. Nick
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
