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 larger default blacklist for one 
sanitizer without inhibiting the other sanitizers. This would be useful if we 
find a bug in a runtime check: we could temporarily work around the issue by 
deploying a new blacklist, instead of changing the compiler or build system. It 
won't be possible to do this if blacklist updates can introduce false negatives.

Also, as a separate point, I think a design which permits false negatives is 
worrisome in and of itself, and is worth fixing.

> Consider extending the blacklist syntax instead, the same way Valgrind does.

I like this idea. However, I think that it would require most of the changes in 
this patch, with some additional work to change SpecialCaseList.

> Blacklist entries could be annotated with a list of sanitizers they apply to, 
> like
> 
> asan,ubsan : src: file.cc:line
> 
> or an even less verbose way using sections
> 
> [asan]
> src:
> src:
>  [msan]
> src:
> 
> As an extra benefit, all default blacklists can be merged into one.

It would be nice to combine the default blacklists into one file with separate 
sections for each sanitizer. I'll look into this (hopefully next week).


https://reviews.llvm.org/D32842



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to