<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 23, 2013 at 2:21 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On 23 October 2013 03:05, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<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">Hi!<div><br></div><div><div>+ if (Level == DL_Error) {</div>
<div>+ InternalScopedBuffer<char> summary(1024);</div><div>+ internal_snprintf(summary.data(), summary.size(),</div><div>+ "UndefinedBehaviorSanitizer: %s", Flag);</div>
<div>+ switch (Loc.getKind()) {</div><div>+ case Location::LK_Null:</div><div>+ break;</div><div>+ case Location::LK_Source: {</div><div>+ SourceLocation SLoc = Loc.getSourceLocation();</div><div>
+ internal_snprintf(summary.data(), summary.size(), "%s %s:%u:%u",</div><div>+ summary.data(),</div><div>+ SLoc.getFilename(), SLoc.getLine(), SLoc.getColumn());</div>
<div>+ break;</div><div>+ }</div><div>+ case Location::LK_Module: {</div><div>+ ModuleLocation MLoc = Loc.getModuleLocation();</div><div>+ internal_snprintf(</div><div>+ summary.data(), summary.size(), "%s (%s+0x%zx)", summary.data(),</div>
<div>+ StripPathPrefix(MLoc.getModuleName(),</div><div>+ __sanitizer::common_flags()->strip_path_prefix),</div><div>+ MLoc.getOffset());</div><div>+ break;</div>
<div>
+ }</div><div>+ case Location::LK_Memory:</div><div>+ internal_snprintf(summary.data(), summary.size(),</div><div>+ " %p", summary.data(), Loc.getMemoryLocation());</div>
<div>+ break;</div><div>+ }</div><div>+ __sanitizer_report_error_summary(summary.data());</div><div>+ }</div></div><div><br></div><div>This code looks too similar to renderLocation() in ubsan_diag.cc. Probably they both</div>
<div>should use PrintSourceLocation() and PrintModuleAndOffset() from sanitizer_common.cc, and the</div><div>latter two functions should be generalized to work with a buffer instead of simply printing the error message.</div>
</div></blockquote><div><br></div></div></div><div>It's not just renderLocation, the code in this if-statement is a second copy of the function up to before the renderMemorySnippet call.</div><div><br></div><div>Richard doesn't want this code to use any buffer at all (and indeed, up until this patch ubsan doesn't). Is there any way we can preserve that? Making PrintSourceLocation and PrintModuleAndOffset use intermediate buffers is going in the wrong direction -- it'd be best if we could avoid needing a buffer, because when this code runs the program is failing and we might not be able to get a buffer.</div>
</div></div></div></blockquote><div><br></div><div>It's a pretty strict limitation, this makes error reporting a lot harder. Usually we think that we can allocate (or at least mmap) a buffer. In fact, if we're using an internal symbolizer, it does all sorts of complex system calls, memory (de)allocation etc, which happen during the report printing. By the way, I've investigated a number of UBSan failures today and think that we should really move toward better error reports in UBSan - at least we should print the stack traces when error happens (if -fno-sanitize-recover is provided).</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div>Could we break up __sanitizer_report_error_summary into two functions, one that takes text as a stream, and a second one which flushes operation? I think the only problem is that it wouldn't be thread safe.</div>
</div></div></div></blockquote><div><br></div><div>I think CommonSanitizerReportMutex should protect us here - we use it to make sure two reports (from different or same sanitizer) are not intermixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="HOEnZb"><font color="#888888">
<div><br></div><div>Nick</div></font></span><div class="im"><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><span style="color:rgb(80,0,80)">On Wed, Oct 23, 2013 at 1:21 PM, Nick Lewycky </span><span dir="ltr" style="color:rgb(80,0,80)"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span><span style="color:rgb(80,0,80)"> wrote:</span><br>
</div></div><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><div><div dir="ltr"><div>On 22 October 2013 22:07, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@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"><div>On 22 October 2013 21:18, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@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">The attached patch makes ubsan emit summaries of errors it encounters. The format of these summaries is:<div>
UndefinedBehaviourSanitizer: signed-integer-overflow file:49:7</div><div>where the string is the flag name. Most of the patch is adding the flag names to all the reports all over.</div></div></blockquote><div><br></div>
</div><div>I've noticed a small bug, for load-invalid-value we always pick "enum" and never "bool". I would guess that's because ASTContext::getTypeSize(BoolTy) returns 8 instead of 1? </div><div>
<br>
</div><div>Richard, thoughts?</div></div></div></div></blockquote><div><br></div></div><div>Updated patch attached. It now detects bool sanitizer by looking at the Type as a string, and is otherwise updated for the changes in sanitizer-common.</div>
<span><font color="#888888">
<div><br></div><div>Nick</div></font></span><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">
<span><font color="#888888"><div>
<br></div><div>Nick</div></font></span><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>This patch is stacked on top of <a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131021/091535.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131021/091535.html</a> , or else ubsan's tests will fail.<br>
</div>
<div><br></div><div>Please review!<span><font color="#888888"><br></font></span></div><span><font color="#888888"><div><br></div><div>Nick</div></font></span></div>
</blockquote></div></div><br></div></div>
</blockquote></div></div><br></div></div>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</font></span></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>