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

[Igor Veresov]

> On Apr 5, 2016, at 10:44 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
> 
>> 
>> On Apr 5, 2016, at 10:34 AM, 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.
>> 
> 
> I looked at the call graphs of these guys and linktime_resolve_X_method() methods actually seem to only called within invokeX contexts. But may be I missed something. Can you give an example of a path that may cause, say, linktime_resolve_static_method() be invoked for non-invokestatic instruction?

Actually, the easier way to think about it, would be: The answer returned by resolve_interface_method() is result of a method resolution in the interface class “as if” it were invoked by the given bytecode instruction. It of course doesn’t not mean that the invocation is really a result of the said instruction. The context is. As you may see the logic around “nostatic” did not change and the logic around resolve_interface_method() being called within the invokeinterface context is what we want it to be.

The same effect could have been achieved by adding another bool argument to resolve_interface_method() to indicate a question within the invokeinterface context. But passing a bytecode makes it an easier to read code.

igor 

> 
>> Just curious did you also run the testbase default methods tests?
> 
> Yes, within the context of a closed project. That’s actually what made these changes necessary.
> 
> igor
> 
>> 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/98d68ed5/attachment-0001.html>