On Wed, Apr 22, 2015 at 12:37 PM, Aaron Ballman <[email protected]> wrote:
> 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). 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 > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > -- Alexey Samsonov [email protected]
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
