[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In https://reviews.llvm.org/D32842#868505, @vlad.tsyrklevich wrote: > @vsk Why don't I take a look at implementing the blacklist selection methods > @eugenis mentioned on top of this change now so that we can skip ahead and > merge something everyone is satisfied with?

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-09-12 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added a comment. @vsk Why don't I take a look at implementing the blacklist selection methods @eugenis mentioned on top of this change now so that we can skip ahead and merge something everyone is satisfied with? https://reviews.llvm.org/D32842

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. @eugenis I gave the idea of annotating blacklist entries with a list of sanitizers they apply to some thought. It would be a nice follow-up to this change, but would depend on it. As an initial step, I think we should move forward with this patch, since it makes it

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 114882. vsk added a comment. - Rebase to ToT. https://reviews.llvm.org/D32842 Files: include/clang/AST/ASTContext.h include/clang/Basic/LangOptions.h include/clang/Basic/SanitizerBlacklist.h include/clang/Driver/SanitizerArgs.h lib/AST/ASTContext.cpp

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-07-31 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In https://reviews.llvm.org/D32842#826383, @shenhan wrote: > Ping? Can we make a decision on this? > I've this simple one D35849: [UBSan] Provide default blacklist filename for > UBSan which, depending on this, shall be > discarded or

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-07-31 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment. Ping? Can we make a decision on this? I've this simple one D35849: [UBSan] Provide default blacklist filename for UBSan which, depending on this, shall be discarded or move forward. If this CL stalls, I'll seek to proceed with

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-07-25 Thread Han Shen via Phabricator via cfe-commits
shenhan added a comment. Thanks. Can you update "SanitizerArgs::collectDefaultBlacklists" to include "ubsan_blacklist.txt"? https://reviews.llvm.org/D32842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-30 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In https://reviews.llvm.org/D32842#768038, @eugenis wrote: > This change scares me a bit, to be honest. Is this really that big of a > problem? Blacklists are not supposed to be big, maybe we can tolerate a few > false negatives? I'd like to make it possible to deploy a

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-30 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment. This change scares me a bit, to be honest. Is this really that big of a problem? Blacklists are not supposed to be big, maybe we can tolerate a few false negatives? Consider extending the blacklist syntax instead, the same way Valgrind does. Blacklist entries could be

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @vsk Looks reasonable to me, although I would say such a change should probably require more tests. However, I am not a C++ expert, and I am still not overly familiar with a codebase, so I'm not sure whether I should be a single reviewer.

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. Ping. https://reviews.llvm.org/D32842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-05 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added reviewers: kcc, kubamracek. vsk added a subscriber: zaks.anna. vsk added a comment. @zaks.anna suggested some more reviewers who may be interested. https://reviews.llvm.org/D32842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32842: Specify which sanitizers are covered by a sanitizer blacklist

2017-05-03 Thread Vedant Kumar via Phabricator via cfe-commits
vsk created this revision. Sanitizer blacklists currently apply to all enabled sanitizers. E.g if multiple sanitizers are enabled, it isn't possible to specify a blacklist for one sanitizer and not another. This makes it impossible to load default blacklists for more than one sanitizer, because