r337470 - [Sema] Add a new warning, -Wmemset-transposed-args

Arthur O'Dwyer via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 26 13:22:25 PDT 2018


On Thu, Jul 26, 2018 at 12:52 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> Other than the (fixed) ffmpeg false positive, I see this firing in three
> places.
>
> One of them is in the libc tests for memset and bzero, where the 0
> argument is intentional.
> One of them is here: https://github.com/apache/xerces-c/blob/trunk/
> src/xercesc/util/XMLUTF16Transcoder.cpp#L114 (note that this code is
> correct).
> And one of them was a real bug where the second and third arguments of
> memset were transposed.
>
> That's an extremely low false positive rate, much lower than what we'd
> expect for an enabled-by-default warning. I find this extremely surprising;
> I would have expected the ratio of true-- to false-positives to be much
> much higher. But ultimately data wins out over expectations.
>
> How does our experience compare to other people's experiences? Are our
> findings abnormal? (They may well be; our corpus is very C++-heavy.) If
> this is indeed typical, we should reconsider whether to have these warnings
> enabled by default, as they do not meet our bar for false positives.
>

I am confused by the "low/high" and "meet/do not meet" in this comment.

Is this warning currently enabled by default?  Is it currently enabled by
-Wall?

Are you saying that "2-in-3-firings false positive" is sufficiently high
that this warning should be disabled by default? Are you saying that
"2-in-a-gazillion-lines false positive" is sufficiently low that this
warning should be included in -Wall? Are you saying something else?

My experience at Green Hills many years ago was that this warning caught a
ton of real bugs; but I don't remember if it was "on by default" or in our
equivalent of "-Wall" or what. I think it should be enabled by -Wall (or
-Wmost), and have no opinion as to whether it should also be on-by-default.

–Arthur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180726/e50b3e3c/attachment.html>


More information about the cfe-commits mailing list