[PATCH] Add a no_sanitize_vptr function attribute.

Wed Apr 22 14:28:38 PDT 2015

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

> On Wed, Apr 22, 2015 at 4:12 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> >
> >
> > 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.
>
> To me, using the existing attributes makes more sense. For starters,
> they're self-documenting as to what sanitizer is being affected. This
> means we don't have to be as careful about adding selective features
> to the sanitizers (the namespace becomes the sanitizer, and not all
> sanitizers).
>

The sanitizers are structured in a different way. From Clang point of view,
it's not the case that there are large
"sanitizers", each of them having a set of "features". Instead, we have
small
"sanitizers" and "groups of sanitizers". They are completely defined by the
file
include/clang/Basic/Sanitizers.def. You may say that we have "flat
namespace".

-fsanitize=cfi is essentially a short way of writing of
-fsanitize=cfi-cast-strict,cfi-derived-cast,cfi-unrelated-cast,cfi-nvcall,cfi-vcall

As I understand Richard's proposal, it's a new attribute, which takes a
list of strings
as its arguments, and has the same semantics as -fsanitize= flag.

ASan, for instance, is a single sanitizer (once again, from Clang's
driver/codegen point of view). Now, it
actually has some experimental functionality, but it's protected by runtime
flags, which is a very different story.

> Also, how much do we have to worry about feature names across
> sanitizers? e.g., asan and msan both share a feature that does
> slightly different things under the same name?

For background, eventually I want to move the sanitizer attributes out
> of clang entirely (by having a plugin system for attributes), and have
> them under the control of the individual sanitizers. Having all
> sanitizers listed together makes this goal considerably more
> challenging. By having each sanitizer segregate its own list of
> features, it makes a more clear layering.
>
> ~Aaron
>
> >
> >> 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
>

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