#1085 Null pointer dereference for code where it is not possible

2.0.3
closed-rejected
William Pugh
5
2014-06-19
2012-06-05
Michael Brizic
No

Re-opening this as a new ticket since the original was close without any opportunity to respond. This references original ticket #3531425. In that ticket, wpugh stated:

"OK, I can't tell because you didn't give a full test case. I believe your
issue as in the attached example.

FindBugs isn't warning about a possible null dereference in that line.
Rather, on that line it sees that c is being tested for null, and further
down in your code it sees a dereference of c and is warning about a null
dereference there. The precondition call is the place where the value is
known to be null.

The problem is that FindBugs doesn't understand that Contract.precondition
will always throw an exception if the second argument is false. Such
analysis is rather difficult.

Is the Contract.precondition method you are calling one from a standard
library?"

FindBugs reports the "Possible null pointer dereference" as Critical for the following line of code:

Contract.precondition("dataset connection is not null" , c != null && c.isConnected());

The Contract.precondition throws an Exception if the statement is not true, but if c is null, it won't execute isConnected() and if c is non-null then it should be fine.

It is not clear why FindBugs reports this as a possible null pointer dereference.

Attached is a snapshot of actual code being violated according to FindBugs. There you will see that it IS complaining about the line of code mentioned above. Also, the test case attached to 3531425 is essentially what Contract.precondition is doing. It is not a public standard library but rather a private one used throughout our company. In the attached snapshot it would appear that FindBugs is complaining both about the call to conn.isConnected within the call to Contract.precondition and then also FURTHER DOWN THE LINE when the call conn.querySQL(sql) is called as well.

So, do you still think that FindBugs cannot analyze this properly?

Discussion

  • Michael Brizic
    Michael Brizic
    2012-06-05

     
    Attachments
  • William Pugh
    William Pugh
    2012-06-06

    • labels: 914653 --> false positive
     
  • William Pugh
    William Pugh
    2012-06-06

    What tool are you using to display FIndBugs results? That display isn't from FindBugs.

    It is possible that the tool being used to display FindBugs results is not correctly identifying the line of the potential dereference, but simply noting the two SourcelineAnnotations in the BugInstance.

    FindBugs can't identify the position within a line where the problem occurs. It can only identify the byte code offset in the class file, and the associated source line.

    Can you provide the XML output from FindBugs for that BugInstance, obfuscating the appropriate class/method names for confidentially?

    I'm pretty sure that the issue you are seeing is the one I've described. We've analyzed hundreds of millions of lines of code, and have hundreds of test cases for null pointer deferences. In isolation, we will not report a null pointer warning for Contract.precondition("dataset connection is not null" , c != null && c.isConnected()); Try it.

     
  • Michael Brizic
    Michael Brizic
    2012-06-07

    I am requesting the data files from my techops team. I will attach shortly.

    Thanks for considering this.

     
  • Michael Brizic
    Michael Brizic
    2012-06-07

    Snapshot of XML output

     
    Attachments
  • Michael Brizic
    Michael Brizic
    2012-06-07

    I've added the findbugs XML output for exactly the same violations that I mentioned in this ticket. The tool we are using to display the violation is called Sonar. The first attachment is the image of how the findbugs XML output is displayed in Sonar. You can see the line numbers there match the line numbers from the XML

     
  • William Pugh
    William Pugh
    2012-06-08

    OK, it is exactly as I described. In the XML file, you can see for the source line annotation at line 181, is role is SOURCE_LINE_KNOWN_NULL, and the message is "Known null at line 181". For the annotation at line 192, the role is SOURCE_LINE_DEREF and the message is "Dereferenced at line 192".

    Sonor is ignoring the roles and messages generated by FindBugs. You need to file a bug report with Sonar.

     
  • William Pugh
    William Pugh
    2012-06-08

    • status: open --> closed-rejected
     
  • Michael Brizic
    Michael Brizic
    2012-06-08

    One additional question however:

    Based on what you say, only one of these appears to be incorrect, right? Line 181 should state "Known null at line 181" but if the conn is checked for null at line 181 why would line 192 say "Possible null pointer dereference"?

    I suppose I am looking for a bit more clarity on what exactly is the Sonar bug and whether FindBugs is in fact analyzing correctly.

     
  • Michael Brizic
    Michael Brizic
    2012-06-08

    • status: closed-rejected --> open-rejected
     
  • William Pugh
    William Pugh
    2012-06-08

    FindBugs detects a path in the control flow where conn is known to be null, the precondition method is called, and then conn is guaranteed to be dereferenced on line 192.

    The problem is that FindBugs doesn't do the inter procedural analysis to detect that if conn is null, a false value is passed into the precondition method, and the precondition method will throw an exception.

    So both statements by FindBugs are correct: that it may be null at line 181, and that it is dereferenced on line 192. The problem is that in analyzing control flow, FindBugs doesn't understand the possible thrown exception.

     
  • Michael Brizic
    Michael Brizic
    2012-06-10

    One more final question that occurred to me upon your last response. Line 181 is: SOURCE_LINE_KNOWN_NULL. How is this possible? If the conn object is checked for null and it is combined with an && where the object is dereferenced, how come FindBugs doesn't detect that if the second argument of && is executed then the first argument had to be true. This would seem independent of FindBugs needing to know that Precondition throws an exception.

     
  • Michael Brizic
    Michael Brizic
    2012-06-29

    Hi wpugh, wondering if you might be able to address my last comment.

    Also, FYI, I opened a bug against Sonar. They agreed it was a problem and will fix it in version 3.2
    Here is the issue if you need to point anyone else to it:
    http://jira.codehaus.org/browse/SONAR-3572

     
    • status: open-rejected --> closed-rejected
    • Group: --> 2.0.3