On Wed, Oct 23, 2013 at 2:21 PM, Nick Lewycky <[email protected]> wrote:
> On 23 October 2013 03:05, Alexey Samsonov <[email protected]> wrote: > >> Hi! >> >> + if (Level == DL_Error) { >> + InternalScopedBuffer<char> summary(1024); >> + internal_snprintf(summary.data(), summary.size(), >> + "UndefinedBehaviorSanitizer: %s", Flag); >> + switch (Loc.getKind()) { >> + case Location::LK_Null: >> + break; >> + case Location::LK_Source: { >> + SourceLocation SLoc = Loc.getSourceLocation(); >> + internal_snprintf(summary.data(), summary.size(), "%s %s:%u:%u", >> + summary.data(), >> + SLoc.getFilename(), SLoc.getLine(), >> SLoc.getColumn()); >> + break; >> + } >> + case Location::LK_Module: { >> + ModuleLocation MLoc = Loc.getModuleLocation(); >> + internal_snprintf( >> + summary.data(), summary.size(), "%s (%s+0x%zx)", >> summary.data(), >> + StripPathPrefix(MLoc.getModuleName(), >> + >> __sanitizer::common_flags()->strip_path_prefix), >> + MLoc.getOffset()); >> + break; >> + } >> + case Location::LK_Memory: >> + internal_snprintf(summary.data(), summary.size(), >> + " %p", summary.data(), >> Loc.getMemoryLocation()); >> + break; >> + } >> + __sanitizer_report_error_summary(summary.data()); >> + } >> >> This code looks too similar to renderLocation() in ubsan_diag.cc. >> Probably they both >> should use PrintSourceLocation() and PrintModuleAndOffset() from >> sanitizer_common.cc, and the >> latter two functions should be generalized to work with a buffer instead >> of simply printing the error message. >> > > It's not just renderLocation, the code in this if-statement is a second > copy of the function up to before the renderMemorySnippet call. > > 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. > 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). > 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. > I think CommonSanitizerReportMutex should protect us here - we use it to make sure two reports (from different or same sanitizer) are not intermixed. > > Nick > > On Wed, Oct 23, 2013 at 1:21 PM, Nick Lewycky <[email protected]> wrote: >> >>> On 22 October 2013 22:07, Nick Lewycky <[email protected]> wrote: >>> >>>> On 22 October 2013 21:18, Nick Lewycky <[email protected]> wrote: >>>> >>>>> The attached patch makes ubsan emit summaries of errors it encounters. >>>>> The format of these summaries is: >>>>> UndefinedBehaviourSanitizer: signed-integer-overflow file:49:7 >>>>> where the string is the flag name. Most of the patch is adding the >>>>> flag names to all the reports all over. >>>>> >>>> >>>> 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? >>>> >>>> Richard, thoughts? >>>> >>> >>> 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. >>> >>> Nick >>> >>> >>>> >>>> Nick >>>> >>>> This patch is stacked on top of >>>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131021/091535.html >>>>> , >>>>> or else ubsan's tests will fail. >>>>> >>>>> Please review! >>>>> >>>>> Nick >>>>> >>>> >>>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >> >> >> -- >> Alexey Samsonov, MSK >> > > -- Alexey Samsonov, MSK
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
