RFR(S) 8153115: Move private interface check to linktime

[Igor Veresov]

> On Apr 5, 2016, at 12:04 PM, Karen Kinnear <karen.kinnear at oracle.com> wrote:
> 
> I am in agreement with Lois that the JVMS looks good with moving the exception.

Thanks!
> 
> With the checking I have done so far, I believe that linktime_resolve_static_method is only called with an invoke static - but after my next
> meeting I will check one more time. It might be worth adding a comment.

Ok, I added a comment to resolve_interface_method(): http://cr.openjdk.java.net/~iveresov/8153115/webrev.01/
Again, the bytecode argument here indicates the context, not the actual bytecode. Of course, for example, the method may be invoked with a method handle.

> 
> My concern is specific to the code in jvmciCompilerToVM.cpp there is code called resolveMethod that checks
> if holder_klass->is_interface -> LR::linktime_resolve_interface_method_or_null.
> 

That code needs fixing as well. We have the following issue filed for that: https://bugs.openjdk.java.net/browse/JDK-8152903 <https://bugs.openjdk.java.net/browse/JDK-8152903>
In the nutshell, resolveMethod() sort of emulates what happens during a virtual call (it is only called for invokevirtual and invokeinterface). It should do both linktime and runtime resolutions. The JVMCI version should work the same way as the CI version (see ciMethod::resolve_invoke() in ciMethod.cpp).

igor

> I think we need to study this one more closely - I suspect that you need a set of detailed tests that cover the
> corner cases here. I don’t know the code paths, but I would suggest following this approach of passing in the byte code,
> so that you get the correct behavior depending on the requesting byte code.
> 
> I am not sure what resolveMethod is doing here - since you seem to already have the resolved method and the receiver - so
> I could use help studying this a bit more to understand if this really is resolution or is really selection.
> 
> thanks,
> Karen
> 
>> On Apr 5, 2016, at 1:34 PM, Lois Foltan <lois.foltan at oracle.com> wrote:
>> 
>> 
>> On 4/5/2016 12:50 PM, Igor Veresov wrote:
>>> Hi Lois,
>>> 
>>> Thanks for looking at it. Yes, it passes all hotspot jtreg tests.
>>> 
>>> igor
>> Hi Igor,
>> 
>> Thanks for waiting on this.  A couple of comments:
>> 
>> - Section 6.5 Instructions for invokeinterface does indicate that a "Linking Exceptions" the VM can throw an ICCE if the resolved method is static or private.  So I think moving this exception from runtime to linktime is okay.
>> 
>> - I'm concerned about the change on line #998, #1030, #1316.  I don't think you are necessarily guaranteed to have the bytecodes that you are now passing to resolve_interface_method.  For example, line #998 within linktime_resolve_static_method, you may not have an invokestatic here, you may have another invoke* bytecode trying to invoke a static interface method.  Passing in Bytecodes::_invokestatic seems wrong, because even if the resolved method is static, "nostatics" was set to false.
>> 
>> Just curious did you also run the testbase default methods tests?
>> Lois
>> 
>>> 
>>>> On Apr 5, 2016, at 9:30 AM, Lois Foltan <lois.foltan at oracle.com> wrote:
>>>> 
>>>> Hi Igor,
>>>> 
>>>> I know you have two reviews for this but could you hold off committing until I or Karen Kinnear have a chance to review.  We both worked in this area a lot to support default methods in JDK 8. Also, have you run the hotspot/test/runtime/SelectionResolution tests on this?
>>>> 
>>>> Thanks,
>>>> Lois
>>>> 
>>>> On 4/1/2016 2:28 PM, Igor Veresov wrote:
>>>>> When invoking private interface methods with invokeinterface we throw ICCE. The check for that happens in the runtime part of the resolution, however, doing it at linktime seems like a better place, since the check doesn't depend on the receiver type. It also allows compiler interfaces that rely on linktime resolution to detect inconsistencies during parsing (see ciEnv::lookup_method() (CI) and ConstantPool.lookupMethod() (JVMCI) that are affected).
>>>>> 
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8153115
>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8153115/webrev.00/
>>>>> 
>>>>> Thanks,
>>>>> igor
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160405/e6fa7453/attachment.html>