JDK-8028549 : Ensure consistent treatment of private/static methods during verification
  • Type: Bug
  • Component: specification
  • Sub-Component: vm
  • Affected Version: 7
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2013-11-18
  • Updated: 2014-02-26
  • Resolved: 2013-12-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8
8Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
During execution of an invokevirtual instruction, JVM implementations have always "skipped" private methods when performing method selection in the receiver's class hierarchy. This is for two reasons [1][2]. However, the JVMS7 definition of overriding used by invokevirtual allows a private method to override a non-private method such that a private method may be the result of selection; this was also the behavior specified in JVMS2. To bring the JVMS in line with implementations, the Lambda spec modifies 5.4.5 Method Overriding so that a private method is not deemed to override a non-private method.

For consistency, the type-checking verifier should deem that a private method does not override anything. Also, the type-checking verifier should deem that a static method does not override anything. This will involve short-circuiting doesNotOverrideFinalMethod:

doesNotOverrideFinalMethod(class('java/lang/Object', L), Method) :-
    isBootstrapLoader(L).

// NEW
doesNotOverrideFinalMethod(Class, Method) :-
    isPrivate(Method, Class).

// NEW
doesNotOverrideFinalMethod(Class, Method) :-
    isStatic(Method, Class).

doesNotOverrideFinalMethod(Class, Method) :-
    isNotPrivate(Method, Class),  // NEW
    isNotStatic(Method, Class),  // NEW
    doesNotOverrideFinalMethodOfSuperclass(Class, Method).

It is necessary to add a new intermediate clause between doesNotOverrideFinalMethod and finalMethodNotOverridden, because doesNotOverrideFinalMethod now assumes the method exists in the class whereas finalMethodNotOverridden needs to be able to recurse upwards into a class with no methods.

// NEW
doesNotOverrideFinalMethodOfSuperclass(Class, Method) :-
    classSuperClassName(Class, SuperclassName),
    classDefiningLoader(Class, L),
    loadedClass(SuperclassName, L, Superclass),
    classMethods(Superclass, MethodList),
    finalMethodNotOverridden(Method, Superclass, MethodList).

We skip over a superclass if it has a non-final method which "doesn't count" due to being private or static:

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    member(method(_, Name, Descriptor), SuperMethodList),
    isNotFinal(Method, Superclass),
    isPrivate(Method, Superclass),
    doesNotOverrideFinalMethodOfSuperclass(Superclass, Method).

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    member(method(_, Name, Descriptor), SuperMethodList),
    isNotFinal(Method, Superclass),
    isStatic(Method, Superclass),
    doesNotOverrideFinalMethodOfSuperclass(Superclass, Method).

Separately, if it is discovered that a (non-private, non-static) method overrides a final method, then the final method is really only overridden if it's non-static and non-private. If the final method is static or private, then it's not really overridden; per JLS7 13.4.17, it should be binary compatible to add 'final' to a static method. In fact, the 'final' modifier is an aberration; a static or private method cannot logically be overridden so the presence of 'final' is irrelevant. In Prolog terms:

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    member(method(_, Name, Descriptor), SuperMethodList),
    isFinal(Method, Superclass),
    isPrivate(Method, Superclass).

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    member(method(_, Name, Descriptor), SuperMethodList),
    isFinal(Method, Superclass),
    isStatic(Method, Superclass).

Finally, the "success" and "recursive" cases are similar to JLS7:

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    member(method(_, Name, Descriptor), SuperMethodList),
    isNotFinal(Method, Superclass),
    isNotStatic(Method, Superclass),  // NEW; we covered non-final static methods above
    isNotPrivate(Method, Superclass).  // NEW; we covered non-final private methods above

finalMethodNotOverridden(Method, Superclass, SuperMethodList) :-
    methodName(Method, Name),
    methodDescriptor(Method, Descriptor),
    notMember(method(_, Name, Descriptor), SuperMethodList),
    doesNotOverrideFinalMethodOfSuperclass(Superclass, Method).  // CHANGED

Need to add new accessors in 4.10.1.1:

isPrivate(Method, Class) - True iff Method in class Class is private.
isNotPrivate(Method, Class) - True iff Method in class Class is not private.
isStatic(Method, Class) - True iff Method in class Class is static.
isNotStatic(Method, Class) - True iff Method in class Class is not static.

[1] In the Java language, an overriding method cannot have less accessibility than an overridden method; it is a compile-time error if a private method [attempts to] override a non-private method. As such, the JVM need not be sympathetic to a class hierarchy where a private method "overrides" a non-private method. Still, it would be rather unforgiving for the JVM to give error (e.g. ICCE) upon detecting the private method, so JVM implementations simply act as if the private method didn't exist and continue climbing the class hierarchy.

[2] To the extent that private methods are legitimate in a class hierarchy, they should be invoked via invokespecial, not invokevirtual. So as in [1], it's not unreasonable for JVM implementations to act as if private methods are invisible during invokevirtual.
Comments
For completeness, here's a list of cases that seem to be in scope for this spec change (m2 is the final superclass method, m1 is the subclass method): - m1 is static. Safe to skip VerifyError. Addressed by doesNotOverrideFinalMethod change in the bug description. - m1 is private. Potentially-bad interaction with invokespecial. - m2 is static. Safe to skip VerifyError. Addressed by finalMethodNotOverridden change in the bug description. - m2 is private. Safe to skip VerifyError (no invokespecial concern here). Does not seem to be addressed by the rules in the bug description. - m2 is package-access. Potentially-bad interaction with invokespecial, just like when m1 is private. Finally, perhaps out of scope for this bug: - m1 is declared in an interface (m2 is, e.g., Object.notify). Safe to skip VerifyError (if m2 is the resolved method, selection in invokevirtual/interface/special will always search Object before interfaces).
19-11-2013

We need to determine whether an interaction with invokespecial is considered "unsafe" for private methods. (Generally, I believe the verifier's goal is to guarantee that, for a verified program, it will never be the case that invoke[virtual/interface/special] will i) resolve to a final method, and then ii) select some other method.) class C { public final void m() {} } class A extends C { private void m() {} } class Test extends A { ... invokespecial C.m()V ... // resolves to final C.m, selects private A.m. } If this is considered _unsafe_, then there should not be a spec change to allow private methods, and JDK-8028520 should be closed as "not a bug". If this is considered _safe_, then by the same argument, it is also safe for the verifier to allow a method to "match" a package-access method in a parent class declared in a different package. A spec change for that case should probably _also_ be included. (Here's the equivalent invokespecial "exploit":) package p1; public class C { /*package*/ final void m() {} } package p2; public class A extends C { public void m() {} } package p2; public class Test extends A { ... invokespecial C.m()V ... // resolves to final C.m, invokes A.m }
19-11-2013