patch: make ubsan report error summaries

[Kostya Serebryany]

On Thu, Oct 24, 2013 at 2:39 AM, Nick Lewycky <nlewycky at google.com> wrote:

> On 23 October 2013 15:29, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> I'm not really happy about including the 'summary' in the normal output
>> -- it's ugly and redundant. If we want to provide this to people who want
>> summaries, that's fine, but the default should either be that this output
>> goes nowhere, or that the summary text is exactly the 'runtime error:' line.
>>
>
> Okay, how about this direction. Let's add a sanitizer_common flag which
> controls whether summaries are printed. The sanitizers can check the flag
> and decide not to call the API (ie., to avoid requiring a buffer) and the
> default implementation of the API checks the flag again and does nothing if
> called with the flag off. Then we turn that flag on with the RUN lines to
> test our summary emission, but it defaults to off for users that don't want
> it.
>
> +cc Kostya. Does that work for both of you? I agree with Richard that (my
> idea of "normal") normal users would find this output redundant, hence the
> idea to default the flag to off.
>
I'd prefer to keep the current behavior the default on asan/tsan/msan, but
it's totally ok to add a flag to disable summaries if wanted.

>
> Nick
>
> On Wed, Oct 23, 2013 at 2:51 PM, Nick Lewycky <nlewycky at google.com> wrote:
>>
>>> On 23 October 2013 11:18, Richard Smith <richard at metafoo.co.uk> wrote:
>>>
>>>> On Wed, Oct 23, 2013 at 2:21 AM, Nick Lewycky <nlewycky at google.com>wrote:
>>>>
>>>>> On 22 October 2013 22:07, Nick Lewycky <nlewycky at google.com> wrote:
>>>>>
>>>>>> On 22 October 2013 21:18, Nick Lewycky <nlewycky at google.com> 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.
>>>>>
>>>>
>>>> This does the wrong thing for typedefs of bool. Can we emit a flag as
>>>> part of the static info to say whether this was the bool sanitizer or the
>>>> enum sanitizer? Otherwise, I don't see how we can distinguish the
>>>> typedef-for-bool case from the enum-with-underlying-type-bool case.
>>>>
>>>
>>> Done. Patch attached!
>>>
>>> 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
>>>>> cfe-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131024/c427bcd2/attachment.html>