Chris,

Thanks for looking into this. See my answers inline.

Best regards,
Vladimir Ivanov

On 1/15/14 9:51 PM, Christian Thalinger wrote:
> [I’m resending something I sent earlier today to Vladimir directly.]
>
> On Jan 15, 2014, at 7:31 AM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> 
> wrote:
>
>> http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8031502
>>
>> InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm
>> when invoking a method from Object declared in an interface.
>>
>> The problem is the following:
>>    (1) java.lang.CharSequence interface declares abstract method "String
>> toString()";
>>
>>    (2) after 8014013 fix, VM resolves
>> CharSequence::toString()/invokeInterface to
>> CharSequence::toString()/invokeVirtual;
>
> Without having looked at the changes of 8014013, why did the invoke type 
> change?  Is it an optimization that was added?
>

After 8014013, LinkResolver::resolve_interface_call returns virtual 
(_call_kind = CallInfo::vtable_call), instead of interface method and 
MethodHandles::init_method_MemberName uses it as is (without any fine 
tuning, which was done before).

It's caused by the following:
   - LinkResolver::linktime_resolve_interface_method returns 
CharSequence::toString method, but it has vtable index, instead of 
itable index;

   - LinkResolver::runtime_resolve_interface_method looks at resolved 
method and sets _call_kind to vtable_call, since resolved method doesn't 
have itable index.

   - then MethodHandles::init_method_MemberName looks at CallInfo passed 
in and sets the reference kind flag to JVM_REF_invokeVirtual.

That's how the conversion from invokeInterface to invokeVirtual occurs.

I did a quick debugging session with pre-8014013 hotspot to check how it 
worked before, but don't remember all the details now.

>>
>>    (3) during LambdaForm compilation, CharSequence is considered
>> statically invocable (see
>> InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for
>> CharSequence::toString() is issued, which is wrong (invokevirtual throws
>> ICCE if it references an interface);
>>
>> The fix is straightforward: during LambdaForm compilation, switch back
>> from invokevirtual to invokeinterface instruction when invoking a method
>> on an interface.
>
> I find this risky.  Right now MemberName is only used in a couple places in 
> java.lang.invoke but in the future it might be used for other things (e.g. 
> java.lang.reflect).  The information MemberName contains should be correct 
> and usable without changing.  Otherwise all users have to patch the 
> information the same way as you do in your patch.
>
> I would like to see the VM passing correct information (whatever the 
> definition of correct is here).
>

It's an interesting question what kind of correctness is required for 
MemberName and I don't know the answer. Looking through the code, I got 
an impression MemberName isn't designed to provide information to be 
used "as is" for bytecode generation, because, at least:
   - MemberName::referenceKindIsConsistent already expects 
(clazz.isInterface() && refKind == REF_invokeVirtual && 
isObjectPublicMethod()) case;

   - InvokeBytecodeGenerator::emitStaticInvoke already has a special 
case for REF_invokeSpecial referenceKind, converting it to 
REF_invokeVirtual;

Best regards,
Vladimir Ivanov
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to