[cfe-dev] [StaticAnalyser][RFC] New checker, -Wsign-compare without the noise

[Daniel Marjamäki via cfe-dev]

>> I attach a simple proof of concept checker. It will just warn if there is an assignment and RHS is a known negative value. Do you have opinions about the design? Should some alternative approach be used?

> You seem to have substituted a lot of false positives for a lot of false negatives.

Thanks for looking!

It is just a proof of concept patch. I wanted it to be accurate. The finished patch will have fewer false negatives for sure.

My primary goal is to avoid obvious FPs. When it is obvious that the signed value can't be negative there should be no warning about loss of sign.

If that means the amount of FPs will be acceptable on real code, I am fine with that.

I have been told that we shouldn't even warn for:

    void foo(int x) {
        unsigned y;
        y = x;
    }

.. as developers often know that some function parameters are always positive. if we want to properly warn about that we should lookup all "foo" function calls and see if the parameter is negative sometimes.

Best regards,
Daniel Marjamäki

..................................................................................................................
Daniel Marjamäki Senior Engineer
Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden

Mobile:                 +46 (0)709 12 42 62
E-mail:                 Daniel.Marjamaki<mailto:Daniel.Marjamaki at evidente.se>@evidente.se<mailto:Daniel.Marjamaki at evidente.se>

www.evidente.se
________________________________
Från: Matthew Del Buono [mpdelbuono at gmail.com]
Skickat: den 1 september 2015 17:47
Till: Daniel Marjamäki
Kopia: cfe-dev at lists.llvm.org
Ämne: Re: [cfe-dev] [StaticAnalyser][RFC] New checker, -Wsign-compare without the noise


I attach a simple proof of concept checker. It will just warn if there is an assignment and RHS is a known negative value. Do you have opinions about the design? Should some alternative approach be used?


You seem to have substituted a lot of false positives for a lot of false negatives.

Why not look to see if the value cannot be provably non-negative? That would resolve the false positive you showed above, but still capture cases where the case only *might* be negative (rather than being guaranteed to be negative). In fact, I think the most interesting case is those where an off-by-one error or some other hidden defect has caused a scenario where almost all cases are positive, but left a couple cases open that the developer did not realize could be negative. Warning in these scenarios that the conversion could be unsafe would be very powerful, I think.

- Matthew P. Del Buono

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150901/275817fb/attachment.html>