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