Bugzilla – Full Text Bug Listing
| Summary: | [compiler] method reference on null object should throw NPE at runtime (JLS compliance) | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Damien Gibou <damien.gibou> |
| Component: | Core | Assignee: | Sasikanth Bharadwaj <sasikanth.bharadwaj> |
| Status: | VERIFIED FIXED | QA Contact: | Stephan Herrmann <stephan.herrmann> |
| Severity: | major | ||
| Priority: | P3 | CC: | b.michael, damien.gibou, daniel_megert, jacek.s.gajek, jarthana, Lars.Vogel, manoj.palat, stephan.herrmann |
| Version: | 4.8 | ||
| Target Milestone: | 4.7.3 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://git.eclipse.org/r/114968 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=4fea24ab96200c9f682b58c91a53876aa7173c22 https://git.eclipse.org/r/115585 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=2cadb681dd72aa94e234bc831c9ea2a850982e32 https://bugs.eclipse.org/bugs/show_bug.cgi?id=537735 |
||
| Whiteboard: | |||
Thanks
Also the ecj-generated code would trigger NPE, if m() would say "s.get()". It's the earlier NPE in main() that is missing. Thanks for the JLS reference!
Comparing compiled bytes from ecj vs. javac:
--- ecj ---
0: aconst_null
1: astore_1
2: aload_1
3: invokedynamic #20, 0 // InvokeDynamic #0:get:(Ljava/lang/Object;)Ljava/util/function/Supplier;
8: invokestatic #21 // Method m:(Ljava/util/function/Supplier;)V
11: getstatic #23 // Field java/lang/System.out:Ljava/io/PrintStream;
14: ldc #29 // String A NPE should have been thrown !!!!!
16: invokevirtual #31 // Method java/io/PrintStream.println:(Ljava/lang/String;)V
19: return
---
--- javac ---
0: aconst_null
1: astore_1
2: aload_1
3: dup
4: invokevirtual #2 // Method java/lang/Object.getClass:()Ljava/lang/Class;
7: pop
8: invokedynamic #3, 0 // InvokeDynamic #0:get:(Ljava/lang/Object;)Ljava/util/function/Supplier;
13: invokestatic #4 // Method m:(Ljava/util/function/Supplier;)V
16: getstatic #5 // Field java/lang/System.out:Ljava/io/PrintStream;
19: ldc #6 // String A NPE should have been thrown !!!!!
21: invokevirtual #7 // Method java/io/PrintStream.println:(Ljava/lang/String;)V
24: return
---
javac is generating an extra getClass() call, which is an idiom of conditionally triggering an NPE.
For information, we have been bitten by this bug during a tree-wide conversion of ‘()- >m.f()’ to ‘m::f()’ (suggested by static code analysis e.g. Sonar). The subtle semantic difference (NPE at call site instead of functional interface invocation, _if any_) was hidden when running the tests from the IDE (Eclipse, obviously). However, the same tests were failing when run on the CI (using javac through maven or gradle…) I've bumped into this bug in code which relies on deserialization.
@Test
public void nullGoodBehaviour() {
Object o = null;
try {
Supplier<String> s = o::toString;
Assert.fail();
} catch (NullPointerException ignored) {
// here it should go.
}
}
@Test
public void nullBadBehaviour() {
Object o = null;
Supplier<String> s = o::toString;
Assert.assertNull(o);
Assert.assertNotNull(s);
try {
s.get();
Assert.fail();
} catch (NullPointerException ignored) {
// here it goes
} catch (RuntimeException e) {
Assert.fail();
}
}
Thanks for the examples. Please see that the bug is scheduled for Photon M4, while currently Java 9 work has priority. moving it to M5 New Gerrit change created: https://git.eclipse.org/r/114968 Gerrit change https://git.eclipse.org/r/114968 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=4fea24ab96200c9f682b58c91a53876aa7173c22 (In reply to comment #7) > Gerrit change https://git.eclipse.org/r/114968 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=4fea24ab96200c9f682b58c91a53876aa7173c22 Following the same idiom of generating getClass on the receiver. Java9 generates a call to Objects.requireNonNull instead of getClass(), but I've not followed that because Object.getClass() seemed easier to generate. Resolving... Should probably go into 4.7.3. @J, what do you think? (In reply to Sasikanth Bharadwaj from comment #9) > Should probably go into 4.7.3. @J, what do you think? +1 for 4.7.3 Reopening for backport to 4.7.3 New Gerrit change created: https://git.eclipse.org/r/115585 Gerrit change https://git.eclipse.org/r/115585 was merged to [R4_7_maintenance]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=2cadb681dd72aa94e234bc831c9ea2a850982e32 Released for 4.7.3 Verified for 4.8 M5 with build I20180123-1010 Verified for 4.7.3 using build M20180214-1700 |
The code below does not behave as expected (at runtime) when compiled with the Eclipse JDT compiler: import java.util.function.Supplier; public class MethodRef { public static void m(Supplier<?> s) { // } public static void main(String[] args) { Object ref = null; m(ref::toString); System.out.println("A NPE should have been thrown !!!!!"); } } JDT Compiler: the code executes with no errors. Javac: NPE at execution, as expected, according to my understanding of https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.13.3 First, if the method reference expression begins with an ExpressionName or a Primary, this subexpression is evaluated. If the subexpression evaluates to null, a NullPointerException is raised, and the method reference expression completes abruptly.