Menu

#1420 False false positive reported by researchers?

3.x
closed-invalid
None
1
2015-10-04
2015-09-20
Thrawn
No

Researchers in 2008 compared FindBugs to Coverity, but one of their sample 'false positive' reports from FindBugs doesn't look to me like a false positive at all.

The relevant code (in the JEdit codebase) was:
char thech = 0;
try
{
thech = (char)m_input.read();
}
catch (IOException e)
{
compressedStreamEOF();
}
if (thech == -1)
{
compressedStreamEOF();
}

FindBugs reported a bad comparison of a non-negative value with a negative constant, but the researchers thought that it just didn't realise that the read() method could return a negative value. Looks to me like they were wrong, because the local variable is a char, unsigned by definition. Anyone else's thoughts?

Paper is at http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.149.9997&rep=rep1&type=pdf

Discussion

  • Tagir Valeev

    Tagir Valeev - 2015-10-04
    • status: open --> closed-invalid
    • assigned_to: Tagir Valeev
     
  • Tagir Valeev

    Tagir Valeev - 2015-10-04

    You're right, these "researchers" are completely wrong:

    The example of identified false positives was about the bad comparison of nonnegative value with negative constant. FindBugs can only do an intra-procedural analysis and cannot detect the return value of the function. In this example, FindBugs cannot detect the possibility that the local variable can have a negative value from a return value of a function. We categorized this case as a false positive.

    As you correctly mentioned, this "Bad comparison" warning was triggered due to incorrect (char) cast (the bug would have been reported even if thech were int, the real cause which triggered the bug report is (char) cast). The bug actually appears in the code. If m_input.read() returns -1, it will be converted to 65535, thus comparison is never true. And for some bug reports FindBugs actually does pretty smart interprocedural analysis.

    So feel free to contact them and point to their mistake.

     

    Last edit: Tagir Valeev 2015-10-04

Log in to post a comment.