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? 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
