[PATCH] Add a no_sanitize_vptr function attribute.

[Aaron Ballman]

On Mon, Apr 20, 2015 at 9:06 PM, Oliver Chang <ochang at google.com> wrote:
> In http://reviews.llvm.org/D9059#158733, @samsonov wrote:
>
>> In http://reviews.llvm.org/D9059#157697, @ochang wrote:
>>
>> > In http://reviews.llvm.org/D9059#157331, @rsmith wrote:
>> >
>> > > I'm not convinced that adding one attribute per sanitizer is the right design here -- it doesn't seem to scale very well. Have you considered an attribute like
>> > >
>> > >   __attribute__((no_sanitize("list,of,sanitizers"))) void fn() { ... }
>> > >
>> > >
>> > > where the list is parsed as if it were specified as `-fno-sanitize=list,of,sanitizers` on the command line?
>> >
>> >
>> > This does seem like a much better way of doing this. Should I change this in this patch?
>> >
>> > What does everyone else think?
>>
>>
>> I agree with this suggestion. It would be cool to have a single attribute like this, and later deprecate no_sanitize_address, no_sanitize_thread and no_sanitize_memory.
>
>
> I can put together another patch in the next few days, unless someone else (more experienced) wants to take this?

I'd like to make sure I'm on the same page for the design. I was under
the impression that your original patch was attempting to suppress a
feature of a sanitizer, not an entire sanitizer itself. I think the
suggestions have been that we should have a single attribute which
allows you to turn off individual ubsan sanitizer features, and
another attribute (someday) to turn of entire sanitizers (which
replaces no_sanitize_thread, etc). I'm wondering if we can do
something like this:

1) Add a new no_sanitize_undefined attribute that turns off all ubsan
checks for a method, and,
2) Have the attribute take an optional, variadic list of sanitizer
feature strings to fine-tune what gets turned off.
3) Someday, if needed, add the optional list functionality to the
no_sanitize_address, no_sanitize_memory, and no_sanitize_thread
attributes.
4) Someday add [[clang::no_sanitize("list", "of", "sanitizers")]] for
turning off entire sanitizers.

This allows you to do:

[[clang::no_sanitize_undefined("vptr", "something_else")]] void f() {}
// Turns off just vptr and something_else ubsan instrumentation
[[clang::no_sanitize_undefined]] void g() {} // Turns off all ubsan
instrumentation for this function

// Someday
[[clang::no_sanitize("ubsan", "asan")]] void h() {} // Turns off all
ubsan and asan instrumentation, but allows tsan and msan.

I think this provides maximal flexibility with minimal worry about
name clashes between sanitizer features. It comes with the handy
benefit of not needing to deprecate the no_sanitize_blah attributes,
but still scales.

~Aaron

>
>
> http://reviews.llvm.org/D9059
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits