Bugzilla – Full Text Bug Listing
| Summary: | NPE in FlowContext while building | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Benjamin Muskalla <b.muskalla> | ||||||||||||
| Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||
| Severity: | critical | ||||||||||||||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, jarthana, Olivier_Thomann, srikanth_sankaran | ||||||||||||
| Version: | 3.7 | Flags: | daniel_megert:
pmc_approved+
amj87.iitr: review+ Olivier_Thomann: review+ |
||||||||||||
| Target Milestone: | 3.6.1 | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Benjamin Muskalla
Created attachment 177903 [details]
project
Attached the project in question
This is a consequence of the fix for bug 321926. Ayushman, please follow up. Created attachment 177905 [details]
Simplified test case
Created attachment 177909 [details]
Untested patch
Some parameters that are irrelevant to org.eclipse.jdt.internal.compiler.flow.ExceptionHandlingFlowContext.recordHandlingException(ReferenceBinding, UnconditionalFlowInfo, TypeBinding, ASTNode, boolean)
are relevant to
org.eclipse.jdt.internal.compiler.flow.InitializationFlowContext.recordHandlingException(ReferenceBinding, UnconditionalFlowInfo, TypeBinding, ASTNode, boolean)
It is really the former call that we care about, but it was an oversight to
not have recognized that calls can dispatch to the latter.
It also errorneous to call the latter in the loop back context as this
can double report problems with unhandled exceptions.
Ayush, this patch is not tested and can be cleaner perhaps. Please take
a look.
Smaller test case:
import java.io.*;
public class X {
static {
try {
while(true) {
if (true)
throw new NumberFormatException();
else
throw new IOException();
}
} catch(IOException e ) {
// empty
}
}
}
Created attachment 177930 [details]
Cleaner patch + junit - under test
As an aside, it looks a bit odd that InitializationFlowContext extends ExceptionHandlingFlowContext - I am not sure there isn't an abstraction problem here -- Ignoring this issue for now. All tests pass, Ayush, please review. TIA. Created attachment 177955 [details]
Much simpler patch - under test
A much simpler patch per Ayush's suggestion.
We discussed the earlier patch and while it
concluded it would work properly, we also felt
it is needlessly complicated.
Since all we want to do is to update the entry
flow information for catch blocks to reflect
the flow data at the end of the loop, we need
to record only those exceptions that have a
catch block - this simply eliminates the calls
to org.eclipse.jdt.internal.compiler.flow.InitializationFlowContext.recordHandlingException(ReferenceBinding, UnconditionalFlowInfo, TypeBinding, ASTNode, boolean)
from ever happening again eliminating the possibility of the
NPE.
(In reply to comment #9) > Created an attachment (id=177955) [details] > Much simpler patch - under test All tests pass. Ayush please take a look. Looks good. Released in HEAD for 3.7 M2. Olivier, the patch is extremely simple, if we have a chance, we should deliver this for 3.6.1, do you agree ? (In reply to comment #12) > Released in HEAD for 3.7 M2. > Olivier, the patch is extremely simple, if we have a chance, we should > deliver this for 3.6.1, do you agree ? We have no choice as this is a regression over 3.6. The fix for 321926 created this problem and since this fix is in 3.6.1 we also must release this one to get back to normal. Daniel, we need PMC approval. This is the NPE I was referring to at the arch call this morning. >We have no choice as this is a regression over 3.6.
I agree, this is a must fix. +1 for 3.6.1 RC3.
Reopen to close as RESOLVED for 3.6.1. Released for 3.6.1. Same patch. Verified for 3.6.1 using build M20100901-1310. Thanks guys! Verified for 3.7M2 using I20100914-0100 |