[cfe-dev] RFC clang analyzer false positives (for loop)

[Jonathan Roelofs via cfe-dev]

On 8/26/16 9:37 AM, Artem Dergachev via cfe-dev wrote:
> On 8/26/16 5:51 PM, Joerg Sonnenberger via cfe-dev wrote:
>> On Fri, Aug 26, 2016 at 02:36:31PM +0000, Daniel Marjamäki wrote:
>>> However if most warnings for a particular use case are in practice
>>> false positives I say these should be dropped. So I think the
>>> question should be if most warnings are false positives? Do you think
>>> your true positive ratio in your own code is good? Is it higher than
>>> 50%?
>>>
>>>> But the example you have given is *not* wrong. It is completely
>>>> correct.
>>> Imho it is a false positive if I can easily see with human
>>> intelligence that the warning is wrong.
>> But you can't without adding additional assumptions that are not part of
>> the code. In this case "nr must be positive". That's why it is not a
>> false positive -- you have to add external knowledge. If the constraint
>> can't be handled by the analyzer, that's again a different situation.
>> That's e.g. a problem when the initialisation happens in a different
>> function and the analyzer can't or doesn't want to see it. Another case
>> is if the constraint depends on a non-trivial computation and the
>> analyzer isn't smart enough to figure it out. All those are examples of
>> false positives: the information is available, but the system doesn't
>> use them. It is not a false positive if information is missing in first
>> place.
>>
> Suppose we analyzed the program from the very main(), scanned all call
> sites in all translation units, and proven that the path is never
> reached in this program. Then the positive would have been gone. So
> perhaps this positive is not unavoidable, and i don't think this
> information is missing - just the analyzer was not taught to use it. If
> it was a library interface function, then yeah, the user is known to be
> able to throw arbitrary stuff into it. But perhaps the library
> documentation will suggest that "if nr is non-positive, the behavior is
> undefined"... no longer sure if it's true or false this time.
>
> But i'd also consider classifying positives as "useful" and "useless",
> rather than "true" or "false". For example, when the code needs to be
> fixed, this positive is good. When it requires documenting (documenting
> contracts through asserts included), it's also probably useful, but
> perhaps less useful - there was no bug in the program, so the user
> wouldn't notice the change. This particular positive, depending on the
> circumstances, may or may not be very useful. Assertion is worth adding,
> yeah. But the software's end user may not benefit from it, so the
> manager may avoid supporting the usage of such tool in the project, if
> most positives need to be suppressed rather than fixed.

I think OP's particular case, the assertion *is* the correct suppression.


Jon

> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded