On Mon, Apr 20, 2015 at 9:06 PM, Oliver Chang <[email protected]> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
