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

David Holmes david.holmes at oracle.com
Wed Sep 6 06:58:56 UTC 2017


On 6/09/2017 7:34 AM, David Holmes wrote:
> 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. 

Given we're dealing with an invokeinterface bytecode I can't see how JNI 
can possibly be involved, nor how we could fail to have a current class. 
I've added an assert and re-run current testing to see if it trips, but ...

> 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).

... we do seem to be missing basic testing in this area, so this code 
path is not being exercised at present. Additionally I need to force the 
taking of this path by using different classfile versions which I'm not 
setup to do.

David
-----

> 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