JDK-8081800 : AbstractMethodError when evaluating a private method in an interface via debugger
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-06-03
  • Updated: 2016-11-04
  • Resolved: 2016-10-04
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 9
9 b143Fixed
Related Reports
Relates :  
JDK-8165827 - Support private interface methods in JNI, JDWP, JDI and JDB
Relates :  
JDK-8071453 - Allow interface methods to be private
Description
This is a method invocation issue on debugger backend in JDK 9.
Method invocation of a private method declared in an interface gives AbstractMethodError.
The issue can be reproduced via the attached test case. Unzip the project and run it via:
/opt/java/jdk1.9.0/bin/java -classpath .../AbstractMethodErrorTest/build/classes abstractmethoderrortest.AbstractMethodErrorTest
After
Breakpoint hit at abstractmethoderrortest.App:31
it throws:
com.sun.jdi.InvocationException: Exception occurred in target VM
	at com.sun.tools.jdi.ObjectReferenceImpl.invokeMethod(ObjectReferenceImpl.java:436)
	at abstractmethoderrortest.EventThread.testMethodInvoke(EventThread.java:362)
	at abstractmethoderrortest.EventThread.access$200(EventThread.java:57)
	at abstractmethoderrortest.EventThread$ThreadTrace.breakpointEvent(EventThread.java:321)
	at abstractmethoderrortest.EventThread.breakpointEvent(EventThread.java:474)
	at abstractmethoderrortest.EventThread.handleEvent(EventThread.java:414)
	at abstractmethoderrortest.EventThread.run(EventThread.java:121)
Exception: instance of java.lang.AbstractMethodError(id=157)

The method invocation is performed in EventThread.testMethodInvoke().
Comments
Method::is_default_method is used in the following places, so we have to verify that excluding private interface methods does not affect the correctness of the code: ./share/vm/classfile/defaultMethods.cpp: if (entry.first->is_default_method()) { This code already works on a set of methods that has excluded private interface methods. --- ./share/vm/oops/klassVtable.cpp: - assert(super_method->is_default_method() || super_method->is_overpass() super_method can not be a private interface method, so operation of is_default_method() is unchanged. - if (old_method->is_default_method()) { This is only used in class redefinition to update the vtable/itable as needed, which doesn't apply to private interface methods, so use of is_default_method is correct. A new test verifies this. - if (m->is_default_method()) { This prints "default" for default methods so the change fixes a bug here. - if (m->is_default_method()) { This prints "default" for default methods so the change fixes a bug here. --- ./share/vm/oops/method.cpp: if (is_overpass() || is_default_method()) return false; This is in the context of: bool Method::is_final_method(AccessFlags class_access_flags) const { if (is_overpass() || is_default_method()) return false; return is_final() || class_access_flags.is_final(); } This would potentially treat final private interface methods differently as we do not return false immediately. However you can not (at the language level) declare a private final interface method, nor a final interface. As future work we should reexamine this logic as all private methods are implicitly final and should treated as so. --- ./share/vm/oops/method.cpp: if (is_default_method()) { This prints "default" for default methods so the change fixes a bug here. --- ./share/vm/oops/instanceKlass.cpp: ((defaults_mode != skip_defaults) || !m->is_default_method())) { The full expression already skips private methods: if (m != NULL && m->is_public() && !m->is_static() && ((defaults_mode != skip_defaults) || !m->is_default_method())) { --- ./share/vm/prims/methodHandles.cpp: if (m->is_default_method()) { This prints "default" for default methods so the change fixes a bug here. --- ./share/vm/ci/ciMethod.cpp: if (root_m->get_Method()->is_default_method()) { return NULL; } This is in the context of "// Disable CHA for default methods for now". However there is earlier code that is intended to catch private or final methods: // Is it private or final? if (root_m->can_be_statically_bound()) { __ return root_m; } and that is the path we take. --- ./share/vm/code/dependencies.cpp: assert(fm->is_default_method(), "sanity"); This occurs in code called from Dependencies::find_unique_concrete_method which in turn is called from two places: 1. In ciMethod.cpp after we have already excluded private interface methods (as they are statically bound ciMethods) 2. In jvmciCompilerToVM.cpp in code that can not be called for interface methods --- So all usages check out okay.
27-09-2016
With these changes Method::can_be_statically_bound now reports true due to vtable_index() == nonvirtual_vtable_index. This is what we want as private interface methods are statically bound (and if all private methods were treated as final the same would be true). But we have to check all uses to ensure code will not be surprised if it encounters a private interface method. ./share/vm/ci/ciMethod.cpp: - _can_be_statically_bound = h_m()->can_be_statically_bound(); Establishes ciMethod property based on Method property. However we later already have: if (!_can_be_statically_bound && h_m()->is_private()) _can_be_statically_bound = true; So ciMethod reports can_be_ statically_bound for private interface methods both before and after this change. ./share/vm/opto/doCall.cpp ./share/vm/shark/sharkTopLevelBlock.cpp ./share/vm/ci/bcEscapeAnalyzer.cpp ./share/vm/c1/c1_GraphBuilder.cpp: All relate to ciMethod so ok. --- ./share/vm/runtime/sharedRuntime.cpp: bool static_bound = call_info.resolved_method()->can_be_statically_bound(); Only called for invokevirtual, so should never encounter a private interface method. --- ./share/vm/runtime/sharedRuntime.cpp: This is in SharedRuntime::handle_ic_miss_helper: // Compiler1 can produce virtual call sites that can actually be statically bound // If we fell thru to below we would think that the site was going megamorphic // when in fact the site can never miss. Worse because we'd think it was megamorphic // we'd try and do a vtable dispatch however methods that can be statically bound // don't have vtable entries (vtable_index < 0) and we'd blow up. So we force a // reresolution of the call site (as if we did a handle_wrong_method and not an // plain ic_miss) and the site will be converted to an optimized virtual call site // never to miss again. I don't believe C2 will produce code like this but if it // did this would still be the correct thing to do for it too, hence no ifdef. // if (call_info.resolved_method()->can_be_statically_bound()) { methodHandle callee_method = SharedRuntime::reresolve_call_site(thread, CHECK_(methodHandle())); It is unclear if we could ever reach here for a private interface method, but if we did then this seem the right thing to do. --- ./share/vm/oops/cpCache.cpp: assert(method->can_be_statically_bound(), ""); case Bytecodes::_invokevirtual: { if (!is_vtable_call) { assert(method->can_be_statically_bound(), ""); ... } else { assert(!method->can_be_statically_bound(), ""); Private interface methods can't be called via invokevirtual, so we should not reach here. --- ./share/vm/oops/method.cpp: assert(m->can_be_statically_bound(), ""); Relates to creation of MethodHandle intrinsic. --- ./share/vm/interpreter/linkResolver.cpp: - CallKind kind = (vtable_index >= 0 && !resolved_method->can_be_statically_bound() ? CallInfo::vtable_call : CallInfo::direct_call); vtable_index will not be >= 0 for private interface methods so the clause is not reachable in this case - and a direct call is the correct choice. - if (resolved_method->can_be_statically_bound()) { kind = CallInfo::direct_call; } This is the correct selection. assert(resolved_method->can_be_statically_bound(), "cannot override this method"); Code should never be reachable for a private interface method.
26-09-2016
We do in fact initially set the vtable index to nonvirtual_vtable_index, but later logic then overwrites that with pending_vtable_index, which later leads to the throwing of AbstractMethodError. So I think the most appropriate, and conservative, fix here is to tweak update_inherited_vtable by changing this: } else if (klass->is_interface()) { allocate_new = false; // see note below in needs_new_vtable_entry // An interface never allocates new vtable slots, only inherits old ones. // This method will either be assigned its own itable index later, // or be assigned an inherited vtable index in the loop below. // default methods inherited by classes store their vtable indices // in the inheritor's default_vtable_indices. // default methods inherited by interfaces may already have a // valid itable index, if so, don't change it. // Overpass methods in an interface will be assigned an itable index later // by an inheriting class if (!is_default || !target_method()->has_itable_index()) { target_method()->set_vtable_index(Method::pending_itable_index); } to add a private check: } else if (klass->is_interface()) { allocate_new = false; // see note below in needs_new_vtable_entry // An interface never allocates new vtable slots, only inherits old ones. // This method will either be assigned its own itable index later, // or be assigned an inherited vtable index in the loop below. // default methods inherited by classes store their vtable indices // in the inheritor's default_vtable_indices. // default methods inherited by interfaces may already have a // valid itable index, if so, don't change it. // Overpass methods in an interface will be assigned an itable index later // by an inheriting class + // Private interface methods have no itable index and are always invoked nonvirtually ! if ((!is_default || !target_method()->has_itable_index()) && !target_method()->is_private()) { target_method()->set_vtable_index(Method::pending_itable_index); } which combined with not adding an itable index fixes the main issue - we will always do a nonvirtual invocation. As a side issue I also have to tweak an assertion in Method::can_be_statically_bound to recognize that private interface methods are also non-virtual. And I chose to fix Method::is_default_method to not claim private interface methods are default methods! Luckily we don't seem to use that method to make any major decisions! But there is one case where it disables CHA in ciMethod.cpp. // Disable CHA for default methods for now if (root_m->get_Method()->is_default_method()) { return NULL; } so that change might expose other issues if CHA can't actually deal with private interface methods. Then on the JDB side I have to make it aware of private interface methods so that a correct attempt at a nonvirtual invocation doesn't get rejected.
07-09-2016
Karen writes: If you call invoke interface on a private (or static) method, you should get an IncompatibleClassChangeError (not an AbstractMethodError). The rule for private interface methods, is that they must be invoked using invoke special - not invoke interface, so they are handled completely differently than public default interface methods - which can be invoked either via invoke virtual or invoke interface. Private methods in interfaces internally do not show up in the default methods list or the vtable for implementations of an interface - public concrete interface methods (default methods) do show up in both the default methods list and the vtable. The order of processing for an interface is: 1) initialize_vtable: update_inherited_vtable should mark method->vtable/itable_index as pending_itable_index 2) initialize_table: assign_itable_indices_for_interface - today I believe this initializes itable indices for private interface methods One of my goals going forward is to remove private interface methods from the itable completely. I did a quick prototype last spring but got interrupted before I had time to do thorough testing. The only change I added was this. It would be great if that were to be part of your fix for this. This both reduces the size of the table (memory savings) and should prevent setting an index for the private method. in klassVtable.cpp: interface_method_needs_itable_index: added if (m->is_private()) return false; // interface private methods require invokeSpecial 3) for The itable is a selection cache for the invoke interface instruction, and only exists for instantiable classes, i.e. interfaces themselves do not have itables. It is organized by walking all transitive interfaces: interface0, method0 index, method1 index, ��� interface1, method0 index, method1 index, ��� ��� with an extra null entry The interface method itself though will have a unique itable index recorded. See method.hpp for VtableIndexFlag. To save memory, vtable and itable indices share the same word, so vtable indices are positive, and itable indices start at -10 and grow down. I don���t know the meaning of 0 for this value - which I think Jaroslav said they were seeing. I would have expected pending_itable_index which should be assigned in update_inherited_vtable for the interface itself. But - if what Jaroslav means is that they found a 0 in the itable index for that method for the implementor of the interface (as distinct from the method���s recorded index in the declarer) ��� then I think we are running into the logic in initialize_table_for_interface in which we ���Leave it empty for AbstractMethodError���. The logic there to me looks like it should happen if target->is_abstract. Does the test have an abstract private method? If so, if you had a runtime direct byte code access - worth testing - I would expect to get an ICCE first at link time, before checking for abstract. It is quite likely we didn���t get all the alternate accessors right when adding these (it is too complex partly because the spec is complex and partly because the spec evolved right up to the end) 1) direct bytecodes - invoke interface: resolution search order: 1. lookup_method_in_klasses: starts with current interface and so will find local private interface method 2. lookup_method_in_klasses 3. lookup_method_in_interfaces calls ik->lookup_method_in_all_interfaces - does NOT return static or private methods, so not inherited - throws ICCE so resolution does not cache any itable index 2) reflection: - I believe we modified the code generating version to explicitly generate invoke special for private methods in interfaces 3) jni: in jni_invoke_nonstatic - I believe a JNI call should be using JNI_NONVIRTUAL for JNI invoke special, i.e. for private interface method accesses. - if there is a call using JNI_VIRTUAL which checks for an itable_index ��� if it has an itable index (which I would expect it to have today) - and you find a null for the method, it should throw AbstractMethodError - that code looks missing? ��� if you put in the change I suggested above, then you would expect an invalid_table_index, and we should throw ICCE (otherwise expect assert ���no valid vtable index" 4) methodHandles - - default method tests run in 4 modes. I think the methodHandle mode is part of default rt-nightly - but Christian would know for sure - 292 struggled with distinguishing AME and ICCE. 4) JVMTI/JDI I don���t know where this code is either - Serguei is back from vacation - or Dmitry might know
02-09-2016
It seems that JDI is ignorant of private interface methods. The test code is executing a virtual invocation mechanism in JDI for a method that should only be invoked via INVOKESPECIAL. This presently leads to the code that see this as an unimplemented interface method and throws AbstractMethodError. According to Karen the correct behaviour should be to throw IncompatibleClassChangeError. If we use a non-virtual invocation then we get an IllegalArgumentException because JDI thinks non-virtual, non-static, interface invocation must be for default methods - again it has no knowledge of private interface methods. There seem to be multiple gaps in the integration of private interface methods, both on the VM side, and in the JDI Java code.
01-09-2016
I think the jni_invoke_nonstatic code has been updated since Jaroslav commented on this. I'm also not very knowledgeable in this area but am investigating. A seperate JNI test is probably needed. Meanwhile I'm trying to figure where the JVMTI/JDWP code is that would generate the AbstractMethodError ?
26-08-2016
oh no, I don't know this code that well either. We'll triage it and get it assigned to the most knowlegeable people. Thanks for the initial analysis.
16-06-2015
This problem goes deeper than JVMTI/JDI. The private interface methods don't work that well with JNI at all. Seems to have something to do with the private interface methods being marked as having the assigned itable index (as opposed to the default interface methods) In vm/prims/jni.cpp#jni_invoke_nonstatic() methods on L1131 the method to be invoked is checked for the presence of the itable index. The private interface method seems to be wrongly marked to contain the itable index and the index is 0 (while in fact, the method on index 0 in the itable is NULL). I would expect the private interface methods being treated the same way the default interface methods are. And here I am getting out of my competency and rather let Coleen take a look.
16-06-2015