(M) RFR: 8081800: AbstractMethodError when evaluating a private method in an interface via debugger

[David Holmes]

Warning: long discussion, but in the end relatively simple code change. :)

Thanks to Karen for explaining vtables and itables and pointing out 
various tests to be executed; Coleen for the discussions around 
interface initialization and terminology, and pointing me to simple 
redefinition tests; and Stas Lukyanov for indicating the right JCK tests 
to run.

Bug: https://bugs.openjdk.java.net/browse/JDK-8081800

Background:

In JDK 8 default and static interface methods were added to the Java 
language. Private interface methods were also considered, and support in 
the VM was added, but were dropped due to schedule pressure. In Java 9 
private interface methods have now been enabled at the language-level 
and because the VM already supported invokespecial for private interface 
methods, the direct language use, and core reflection use, of these 
methods works fine. However, what was overlooked (and which the test 
case in this bug report highlighted) was that the other interfaces to 
the VM (JNI, JDWP, JDI, JVM TI) had not been updated to account for 
private interface method, and such usage did not work.

The updates to the specifications, plus some small JDI/JDWP related code 
changes are being handled under:

JDK-8165827 Support private interface methods in JDI, JDWP and JDB
https://bugs.openjdk.java.net/browse/JDK-8165827

This bug, although originally discovered via JDI/JDB, is being used to 
fix the underlying mechanics in the VM used by the JNI layer - after 
which the test in the bug report will run fine.

Problem:

Because private interface methods are only invocable via invokespecial 
(the JVMS goes to great lengths to explicitly prohibit all other 
invocation forms on them) they are in essence always statically bound 
and don't require lookup in either itables (for invokeinterface) or 
vtables (for general lookup). However, JNI etc, uses itables/vtables to 
perform their invocations, and what we got was behaviour where the 
private interface methods did have an itable entry, which made them 
appear to be regular abstract interface methods, and so they ended up 
with initial vtable entries that were set to throw AbstractMethodError 
on invocation (normally those vtable entries would be replaced by the 
concrete methods in the implementing class) - and that is what was 
observed via JDB. It turns out that depending on whether a class method 
with the same signature existed in a class implementing the interface, 
that you could also get IllegalAccessError (a path that actually crashes 
the debug VM due to an assertion failure in jni.cpp!).

Solution:

Private interface methods do not need, and should not have, an itable 
entry - they are never invoked via invokeinterface. (Thanks Karen)

Private interface methods can always be statically bound - 
Method::can_be_statically_bound() should return true, and their vtable 
entry should be Method::nonvirtual_vtable_index.

Private interface methods are not default methods and 
Method::is_default_method() should return false. There is a terminology 
confusion here that I address further down.

See the bug report for a detailed analysis of all the places where 
changing these Method properties may have had an affect.

Main webrev:

http://cr.openjdk.java.net/~dholmes/8081800/webrev.hotspot/

The main changes are in:
- klassVtable.cpp
- method.cpp

there are minor changes to comments and assertions in other files (the 
jni.cpp change was due to the crash I encountered that I referred to 
earlier). The change in linkResolver.cpp fixes an error in the tracing 
code as the bytecode need not be "invokeinterface" and clarifies it is 
an interface method (and adds a missing colon in the message) - there is 
a corresponding tweak to the logging/ItablesTest.java test.

I added new tests for JNI invocations of private, interface methods, and 
also to test JVM TI retransformation of private and default interface 
methods.

---

Terminology problem:

While working on this issue, and helping Coleen with:

8163969: Cyclic interface initialization causes JVM crash

it became apparent that there was a terminology error in the VM code 
with respect to default methods. A "default method" is very specifically 
a public interface method, marked with the default keyword, which has a 
method body defined. A static interface method also has a body, but is 
not a default method. A private interface method also has a body, but is 
not a default method. The JVMS refers to non-static, non-abstract 
interface methods - which covers default methods and private interface 
methods. But the code in the VM, primilarly in instanceKlass.cpp and 
classFileParser.cpp used the term "default methods" to mean 
"non-abstract and non-static" - which is wrong and potentially very 
confusing. So a second part of this change is to rename 
"has_default_methods" (and related variables) to 
"has_nonstatic_concrete_methods". This is somewhat of a mouthful, though 
less so than has_nonstatic_nonabstract_methods. Suggestions to 
abbreviate this to has_nsna_methods, or has_nans_methods, were rejected 
during pre-review.

The renaming webrev is here:

http://cr.openjdk.java.net/~dholmes/8081800/webrev.hotspot-rename/

and is best viewed via the patch file, where the renaming is more 
obvious. In classFileParser.cpp I also simplified the check for static 
interface methods in pre-java8 classfiles.

---

Testing:
  - JPRT
  - nsk.jdb/jdi/jdwp/jvmti
  - jtreg: com/sun/jdi (including InterfaceMethodsTest)
           runtime/SelectionResolution/
  - internal: vm.defmeth
  - JCK: subset of lang and vm tests that cover default/static/private 
interface methods
  - new tests

Together these tests cover interface method invocation at the language 
level, via core reflection, via MethodHandles, via JNI, via 
JDI/JDWP/JDB, and via JVM TI.

Thanks,
David