Bug 299900

Summary: [null]Missing potential null warnings for variable on the right of an OR conditional expression
Product: [Eclipse Project] JDT Reporter: Ayushman Jain <amj87.iitr>
Component: CoreAssignee: Ayushman Jain <amj87.iitr>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Olivier_Thomann
Version: 3.6Flags: Olivier_Thomann: review+
Target Milestone: 3.6 M5   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
proposed fix v0.5 + regression tests Olivier_Thomann: iplog+

Description Ayushman Jain CLA 2010-01-18 02:20:10 EST
Build Identifier: I20091210-1301

The null analysis has a flaw while evaluating the OR conditional expression for null info. The variable(s) on the left have their null info properly in place when the analysis of the OR completes, but the same cannot be said about variables(s) on the right. This is because after the analysis of left and right expressions, to combine both we use FlowInfo.conditional, which merges the info when true and the info when false. Within that we use mergedWith() to combine the true info from left expression with true info from right expression.

Note that mergedWith takes the intersection of definitely null variables in the merging streams. So, since the stream of left expression has no info about what is null in the stream of right expression, the variables that are definitely null in right info (when true) are missed out when merging. Ex:

void foo(Object a, Object b) {
  if(a == null || b == null) {
      System.out.print(b.toString());  //Missing potential null access warning
  }
}

In the above case b is definitely null in 'right info when true', but this info about b gets discarded in the merging. This leads to the missing warning(s) further in the code.
Another ex:

void foo(Object a, Object b) {
  if(a == null || b == null) {}
  System.out.print(b.toString());  //Missing potential null access warning
}

In both cases warnings for a are correct.


Reproducible: Always

Steps to Reproduce:
1. in Preference-> Java -> Compiler -> Errors/warnings -> Potential programming problems, enable the warnings null pointer access and potential null pointer access.
2. Try the above two examples.
3. No potential null warning for b, but warning correctly reported for a.
Comment 1 Ayushman Jain CLA 2010-01-18 04:22:07 EST
Created attachment 156362 [details]
proposed fix v0.5 + regression tests

In order to use mergedWith() and avoid losing definitely null info in the right info when true, we should add that info to a temporary stream which has the left info as well, and use that stream to merge with the right info. This solves the problem.
Comment 2 Olivier Thomann CLA 2010-01-21 12:05:49 EST
Released patch for 3.6M5.
Added regression tests:
org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest#testBug299900a
org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest#testBug299900b
Comment 3 Olivier Thomann CLA 2010-01-25 14:28:49 EST
Verified for 3.6M5 using I20100125-0800