Bugzilla – Full Text Bug Listing
| Summary: | NPE in ReferenceBinding.binarySearch | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Stephan Herrmann <stephan.herrmann> |
| Component: | Core | Assignee: | Sebastian Zarnekow <sebastian.zarnekow> |
| Status: | VERIFIED FIXED | QA Contact: | Stephan Herrmann <stephan.herrmann> |
| Severity: | normal | ||
| Priority: | P3 | CC: | manoj.palat |
| Version: | 4.13 | ||
| Target Milestone: | 4.13 M3 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=544921 https://bugs.eclipse.org/bugs/show_bug.cgi?id=540738 https://git.eclipse.org/r/146812 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=714070211387f0595cd8df499e20cf559116fcea |
||
| Whiteboard: | |||
|
Description
Stephan Herrmann
I'll look into this tomorrow. The attachement has two source files:
A.java
```
package test;
import java.util.Map;
import java.util.Map.Entry;
public class A {
void a(Map<String,String> a) {
for (Entry<String, String> iterable_element : a.entrySet()) {
}
}
}
```
B.java
```
package test;
import java.util.HashMap;
public class B {
void test() {
new A().a(new HashMap<String, String>());
}
}
```
Looking into the example, I see a SplitPackageBinding in the callstack of the shape `package java.util (from java.prefs, java.base)`. I wonder, where this stems from, since java.util is not a package defined in java.prefs.
```
module java.prefs {
requires java.xml;
exports java.util.prefs;
uses java.util.prefs.PreferencesFactory;
}
```
The split package leads to a ProblemReferenceBinding with the correct closest match (java.util.Map$Entry).
```
ProblemType:[compoundName=java.util.Map$Entry][problemID=ProblemReasons.Ambiguous][closestMatch=public static interface java.util.Map$Entry<K extends java.lang.Object, V extends java.lang.Object>
extends java.lang.Object
enclosing type : java.util.Map<K,V>
/* methods */
[unresolved] public abstract K getKey()
[unresolved] public abstract V getValue()
[unresolved] public abstract V setValue(V)
[unresolved] public abstract boolean equals(java.lang.Object)
[unresolved] public abstract int hashCode()
[unresolved] public static Unresolved type java.util.Comparator<Entry<K,V>> comparingByKey()
[unresolved] public static Unresolved type java.util.Comparator<Entry<K,V>> comparingByValue()
[unresolved] public static Unresolved type java.util.Comparator<Entry<K,V>> comparingByKey(Unresolved type java.util.Comparator<? super K>)
[unresolved] public static Unresolved type java.util.Comparator<Entry<K,V>> comparingByValue(Unresolved type java.util.Comparator<? super V>)
]
```
If we guard against the NPE, the compiler will report an error on the import of java.util.Map.Entry.
I'll make the code null-safe but it won't be a happy face for the given scenario.
New Gerrit change created: https://git.eclipse.org/r/146812 The underlying lookup of packages / modules is indeed wrong. This has been investigated in bug 549647, with the conclusion, that insufficient information in file ct.sym (with undocumented and changing format) is the root cause (hence NOT_ECLIPSE). The general issue in JDT is: ProblemReferenceBinding has become a candidate for memberTypes. Perhaps you can create a reliable test with a "real" split package problem? Perhaps the test would also need to trigger: - loading the toplevel class (Map) from the declaring module itself, so that ambiguity is avoided - then loading the member class (Map$Entry) from a module that has the package conflict. The original code in BinaryTypeBinding.getMemberType did use the sourceName field, too. What would be a valid approach for a correct lookup? Using the closestZype to unwrap the ProblemReferenceBinding? Sorry, I don’t understand your idea for the test. Could you please share some more insights. Is there an existing test that I could look at to understand your proposal better? (In reply to Sebastian Zarnekow from comment #5) > Sorry, I don’t understand your idea for the test. Could you please share > some more insights. Is there an existing test that I could look at to > understand your proposal better? I thought the following would work: mod1 exports pa, pb: pa/CA pa/CA$Inner (static?) pb/CSub extends CA // <- use this to trigger loading CA as a valid binding mod2 exports pa: pa/CX mod3 requires mod1, mod2: // <- causes split package pa (from mod1, mod2) import pb.CSub; import pa.CA.Inner; // <- use this to trigger Inner being a problem binding in that case I had hoped to - see CA.Inner as a ProblemReferenceBinding(Ambiguous) - let CA.getMemberType("Inner") trigger sorting with a ProblemReferenceBinding => NPE But since CA$Inner is found in a SplitPackageBinding we only report that conflict and don't let resolving of the import succeed => the ProblemReferenceBinding is never stored. Ergo: I'm no longer sure the NPE can be triggered without a prior bug in resolving. Still causing NPE due to a distant bug is not nice ... > The original code in BinaryTypeBinding.getMemberType did use the sourceName > field, too. What would be a valid approach for a correct lookup? Using the > closestZype to unwrap the ProblemReferenceBinding? I'd suggest to set ProblemReferenceBinding#sourceName in that class's constructor if available. (In reply to Stephan Herrmann from comment #6) > I'd suggest to set ProblemReferenceBinding#sourceName in that class's > constructor if available. I like that idea. I'll add this in addition to the null-safe sorting / search in ReferenceBinding (seems like closestMatch can be null in some cases, at least there are null guards in the ProblemReferenceBinding). About the tests: I toyed around with it but did not manage to reproduce an NPE yet. I'll work on that, though. (In reply to Sebastian Zarnekow from comment #7) > About the tests: I toyed around with it but did not manage to reproduce an > NPE yet. I'll work on that, though. From comment 2 I assume you did succeed to reproduce? Perhaps only in a runtime workbench, not in a test? Did you see in the other bug, that the specific bug is triggered by --release 10 when running on JRE 11? (In reply to Stephan Herrmann from comment #8) > (In reply to Sebastian Zarnekow from comment #7) > > About the tests: I toyed around with it but did not manage to reproduce an > > NPE yet. I'll work on that, though. > > From comment 2 I assume you did succeed to reproduce? Perhaps only in a > runtime workbench, not in a test? Yes, exactly. I saw the NPE in a runtime workbench, never in a test. > > Did you see in the other bug, that the specific bug is triggered by > --release 10 when running on JRE 11? Yes I saw that and the runtime / interactive test confirmed this. So far I am still struggling with the test infrastructure. I did not yet figure out how to configure projects in the tests in the very same way as in the attached zip. (In reply to Sebastian Zarnekow from comment #9) > Yes I saw that and the runtime / interactive test confirmed this. So far I > am still struggling with the test infrastructure. I did not yet figure out > how to configure projects in the tests in the very same way as in the > attached zip. Patch set #8 slightly adjusts the project configuration to match our findings in bug 549647. With this I was able to see the NPE in the test if - I revert the fixes - run tests on openjdk 11 Running on Oracle JDK 11 doesn't reproduce because for random reasons this JRE lets modules appear in a lucky order, whereas openjdk 11 makes java.util a split package of java.logging, java.base, java.prefs (in that order!). Now, with the fix, in the bad case we still see the effects of bug 549647, so perhaps we need to detect the JRE tests are running on to select between two sets of expectations. Let's first see, what jenkins says... Bottom line: yes, we have a regression tests which - under certain conditions - may reproduce the issue, and if it does the problem is fixed by the change in gerrit. (In reply to Stephan Herrmann from comment #10) > Patch set #8 slightly adjusts the project configuration to match our > findings in bug 549647. With this I was able to see the NPE in the test if > - I revert the fixes > - run tests on openjdk 11 These are great news! Thank you very much for helping out. I've to admit that using a different JDK did not come to my mind at all. > so perhaps we need to detect the JRE tests are running on to select between > two sets of expectations. If that's not possible, we could assert for no markers and catch the AssertionError to verify that the markers for the other scenario are present. Or is there a reliable means in test infrastructure to assert that we are running with openjdk 11? In existing tests I see us checking System.getProperty("java.vm.name").contains("OpenJDK"), but to make things worse, the bogus compile errors are not deterministically reported, and can occur (rarely) also on Oracle JDK 11.
For a pragmatic solution I let the test accept both outcomes when run on JRE 11 of any provenance.
Gerrit change https://git.eclipse.org/r/146812 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=714070211387f0595cd8df499e20cf559116fcea (In reply to Eclipse Genie from comment #13) > Gerrit change https://git.eclipse.org/r/146812 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=714070211387f0595cd8df499e20cf559116fcea Fix released for 4.13 M3 Thanks Thank you very much for helping with the analysis and the test setup! Verified for Eclipse 2019-09 (4.13) M3 with Build id: I20190820-1800 |