[PATCH] Add a no_sanitize_vptr function attribute.

[Alexey Samsonov]

On Wed, Apr 22, 2015 at 12:37 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> 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).


Not really. There are two kinds of arguments to -fsanitize= options:
1) inidividual sanitizers ("address", "thread", "vptr", etc.)
2) sets of sanitizers ("undefined", "cfi", etc.).

We can introduce a single no_sanitize("list", "of", "sanitizers") that
would be flexible
enough for your use cases:
[[clang::no_sanitize("vptr", "shift")]]  // disable just "vptr" and "shift"
checks in the function
[[clang::no_sanitize("undefined")]]  // disable all checks introduced by
UBSan (-fsanitize=undefined)
[[clang::no_sanitize("undefined", "address", "memory")]]  // Disable UBSan,
MSan, ASan instrumentation (but use TSan, if the code will be compiled with
-fsanitize=thread).

Then no_sanitize_address will be just a synonym for no_sanitize("address"),
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
>



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150422/58da1370/attachment.html>