<div dir="ltr">On 22 October 2013 22:53, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">LGTM<br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">On Wed, Oct 23, 2013 at 9:25 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>On 22 October 2013 22:10, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote"><div>On Wed, Oct 23, 2013 at 8:16 AM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">__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.<div>
<br></div><div>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.</div>
</div></blockquote><div><br></div></div><div>But I've added summaries for a reason: they help to cluster bug reports.</div></div></div></div></blockquote><div><br></div></div><div>But asan prints one error and exits?</div>
</div></div></div></blockquote><div><br></div></div><div>Correct. Summaries help cluster reports from different runs. </div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>I'd really prefer to keep them by default. </div>
</div></div></div></blockquote><div><br></div></div><div>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.</div>
</div></div></div></blockquote><div><br></div></div><div>This happens only if the subprocess does not have a symbolizer, right?</div></div></div></div></blockquote><div><br></div><div>Right.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>This patch will make asan print summaries with abrakadabra if there is not symbolizer. <br></div>
<div>Could be totally fine though.</div></div></div></div></blockquote><div><br></div><div>With no symbolizer, the results look like:</div><div><br></div><div>SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 ??<br></div>
<div><br></div><div>which is pretty lousy, but does contain *some* information. We can certainly change what the summary says in the case where there is no </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra"><div class="gmail_quote"><div> 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.</div>
<div><br></div><div>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.</div></div></div></div></blockquote><div>
<br></div></div><div>[in another patch?] we can move printing SUMMARY: from __sanitizer_report_error_summary to ReportErrorSummary</div><div>and add an additional "const char *prefix" parameter to ReportErrorSummary. asan/tsan/msan will call it with prefix="SUMMARY: "</div>
</div></div></div></blockquote><div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>btw, why not use phabricator for reviews :) ? <br></div></div></div></div></blockquote><div><br></div><div>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 <span style="color:rgb(0,0,0);font-family:'Helvetica Neue',Arial,sans-serif;font-size:12px;line-height:14px;white-space:nowrap">D1960 </span>where double-clicking the line number wouldn't let me enter comments, even though I was logged in as the reviewer. Absolutely maddening.</div>
<div><br></div><div>Anyhow, I'll put the next patch in phab.</div><div><br></div><div>Nick</div><div><br></div></div></div></div>