patch: sanitizer summaries are redundant, don't emit them

[Kostya Serebryany]

LGTM


On Wed, Oct 23, 2013 at 9:25 AM, Nick Lewycky <nlewycky at google.com> wrote:

> On 22 October 2013 22:10, Kostya Serebryany <kcc at google.com> wrote:
>
>>
>> On Wed, Oct 23, 2013 at 8:16 AM, Nick Lewycky <nlewycky at google.com>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?

This patch will make asan print summaries with abrakadabra if there is not
symbolizer.
Could be totally fine though.


> 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: "

btw, why not use phabricator for reviews :) ?


>
> Any ideas?
>
> Nick
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131023/30930074/attachment.html>