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
