RFR: 8187221 [Nestmates] Virtual invocation for private interface methods

David Holmes david.holmes at oracle.com
Tue Sep 5 21:34:02 UTC 2017


Hi Maurizio,

On 5/09/2017 11:20 PM, Maurizio Cimadamore wrote:
> Langtools change looks good,

You made it so simple for me to change. :)

> VM changes also make sense. One question - 
> I wonder why the old code assumed that link_info.current_klass() could 
> be null? I see that your new code expects that not to be the case 
> (otherwise the code would crash), and that's probably a consequence of 
> the fact that you're using InstanceKlass::cast which I see asserts that 
> value passed in is not NULL. So does it mean that this value cannot ever 
> be NULL and that the old code was 'bogus' ?

My take was that the current_klass can not possibly be null - but 
looking further I think that was wishful thinking. We allow for a NULL 
caller class:

   // Case where we just find the method and don't check access against 
the current class
   LinkInfo(Klass* resolved_klass, Symbol*name, Symbol* signature) :
     _resolved_klass(resolved_klass),
     _name(name), _signature(signature), _current_klass(NULL), 
_current_method(methodHandle()),
     _check_access(false), _tag(JVM_CONSTANT_Invalid) {}


and I see some use of this from JavaCalls and MethodHandle code. It may 
be that if this code is called from JNI that there is no current klass. 
That particularly code path may not be tested yet ... which suggests 
there is no test that tries to trigger the ICCE, because it would now 
not throw (such a test would have to be compiled with an earlier 
classfile version).

I'll have to dig deeper on this.

Thanks,
David

> Maurizio
> 
> 
> On 05/09/17 13:34, David Holmes wrote:
>> webrev: http://cr.openjdk.java.net/~dholmes/8187221/
>>
>> Following up from JDK-8186763 this issue handles item #2:
>>
>> 2. Use invokeinterface for private interface method invocations, 
>> instead of invokespecial
>>
>> This involves three basic pieces:
>>
>> 1. javac issues invokeinterface instead of invokespecial
>> 2. Verifier rules requiring invokespecial are relaxed for latest class 
>> file version (exact version TBD based on release timing)
>> 3. Method resolution locates the expected method.
>>
>> As part of this change I'm also isolating the changes that allowed 
>> invokespecial to be used for nestmate method invocations. In the near 
>> future that code will be removed so that we are near final-form.
>>
>> Actual file changes are very simple in the end:
>>
>> src/share/vm/classfile/verifier.cpp
>>
>> Restore original (pre-nestmate) logic for invokespecial and add in the 
>> nestmate-enabling logic under the UseNewCode guard.
>>
>> ---
>>
>> src/share/vm/interpreter/linkResolver.cpp
>>
>> LinkResolver::resolve_interface_method: Only prohibit invokeinterface 
>> for private interface methods if : 
>> InstanceKlass::cast(current_klass)->major_version() < 
>> VIRTUAL_PRIVATE_ACCESS_VERSION
>>
>> LinkResolver::runtime_resolve_interface_method: Don't resolve in the 
>> receiver class if dealing with a private interface method
>>
>> ---
>>
>> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
>>
>> Removed the check for interface methods when generating virtual calls.
>>
>> ---
>>
>> With these changes in place the following test options will test the 
>> use of invokevirtual and invokeinterface for private method invocations:
>>
>> -javacoption:-XDdisablePrivateAccessors 
>> -javacoption:-XDvirtualizePrivateAccess
>>
>> If you still want to experiment with the invokespecial changes use:
>>
>> -javacoption:-XDdisablePrivateAccessors -javaoption:-XX:+UseNewCode
>>
>> (note the second one is a java option not javac)
>>
>> Next iteration I plan on removing the invokespecial related changes.
>>
>> ---
>>
>> Testing so far:
>>  - all runtime jtreg tests
>>  - langtools jtreg tests
>>
>> Cheers,
>> David
>> -----
> 


More information about the valhalla-dev mailing list