[PATCH] Implement no_sanitize attribute.

[Aaron Ballman]

On Thu, May 14, 2015 at 4:37 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> On Thu, May 14, 2015 at 10:42:50AM -0400, Aaron Ballman wrote:
>> On Wed, May 13, 2015 at 6:40 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> > These constructs were added to clang in r237055 and r237056.
>>
>> Thank you; I was just slightly too out of date to have them. Having
>> fetched, I'm back on the same page.
>>
>> I don't think a variadic enumeration argument will be worth the effort
>> given the structure of Sanitizers.def. However, the current approach
>> is still incorrect. For instance, the attribute will not round-trip a
>> pretty printing, and it fails to diagnose illogical values that wind
>> up being ignored.
>
> FWIW, this is deliberate. We shouldn't diagnose unknown sanitizers because
> there ought to be a way to supply a no_sanitize attribute for any particular
> sanitizer in a way that is backwards compatible with older versions of Clang
> that do not support the sanitizer.

I understand the backwards compatibility argument, however, silently
ignoring attributes is something we generally try to warn on. The
benefit of having the warning is that it shows you times you
accidentally have a typo in your warning, which is trivial to
overlook. Users can turn the warning off if it turns out to be chatty
for them (I would make a new diagnostic for this, not attempt to reuse
an old one).

>
>> The semantic attribute needs to carry around the
>> valid original information the user provided, but I can see why you'd
>> also want it to carry it around in mask format.
>
> It seems that the mask will require custom serialization/deserialization
> support if we don't want it treated as a syntactic attribute, and there
> doesn't seem to be infrastructure for doing this right now. Instead I just
> added a method that calculates the mask on demand.

That is a good point; we do not have serialization support for that. I
wonder if other attributes have been tripped up by this (I will
check).

~Aaron

>> I think the correct approach for this is:
>>
>> 1) Make a VariadicStringArgument tablegen type (model it after
>> VariadicExprArgument, roughly). (Attr.td, ClangAttrEmitter.cpp)
>> 2) Implement no_sanitize in terms of a VariadicStringArgument (Attr.td)
>> 3) Add an additional public member to hold the SanitizerMask value (Attr.td)
>> 4) Issue diagnostics for invalid members when creating the sanitizer
>> mask value (SemaDeclAttr.cpp)
>> 5) Create the attribute, then attach the flattened SanitizerMask
>> value, then attach it to the declaration (SemaDeclAttr.cpp)
>
> Mostly followed this design, modulo the mask thing.
>
> Thanks,
> --
> Peter