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
