RFR : JDK-8166110 : C2 crash in compiling method handles

[Vladimir Ivanov]

Remi,

> in the long term, c2 (and c1) should be fixed to stop to inline switch branches that have never been taken,
> the dynamic language I currently maintain tranforms all switch to a tree of GWTs because of that,
> i think we will need this improvement to implement pattern matching in 10 anyway*.

Yes, C2 can do better when optimizing switches (e.g., [1]), but 
profiling information is not our friend here. Even if it says all 
branches are equally taken, type paradoxes arise from decisions in the 
context of the current compilation where only one branch is always taken 
at runtime.

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8058192

> ----- Mail original -----
>> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
>> À: "Dmitry Chuyko" <dmitry.chuyko at oracle.com>, hotspot-compiler-dev at openjdk.java.net
>> Envoyé: Vendredi 3 Février 2017 17:59:53
>> Objet: Re: RFR : JDK-8166110 : C2 crash in compiling method handles
>
>> The fix looks good.
>>
>> More detailed description of the problem:
>>
>> MH.invokeBasic() and MH.linkTo* signature-polymorphic are inherently
>> unsafe. There are no type checks involved and it's user responsibility
>> to ensure there are no type mismatches possible. They are available only
>> to trusted code (in java.lang.invoke), which ensures the signatures
>> always match.
>>
>> Unfortunately, even though type mismatches aren't possible at runtime,
>> JIT-compilers can encounter them in paradoxical situations. It usually
>> happens when optimizing effectively dead code.
>>
>> BoundMethodHandle.arg(I) demonstrates one of the problematic code shapes:
>>
>> final Object arg(int i) {
>>     switch (speciesData().fieldType(i)) {
>>     case L_TYPE: return
>> speciesData().getters[i].invokeBasic(this);
>>     case I_TYPE: return (int)
>> speciesData().getters[i].invokeBasic(this);
>>     case J_TYPE: return (long)
>> speciesData().getters[i].invokeBasic(this);
>>     case F_TYPE: return (float)
>> speciesData().getters[i].invokeBasic(this);
>>     case D_TYPE: return (double)
>> speciesData().getters[i].invokeBasic(this);
>> }
>>
>> If C2 can constant fold speciesData().getters[i], it will try to inline
>> through invokeBasic() on all execution paths, though only 1 of them is
>> taken. On all other paths the signatures won't match and it can lead to
>> type paradoxes on IR level though the code is effectively dead.
>>
>> Though it was observed only with invokeBasic() and I don't see such
>> situations is possible with MH.linkTo*(), for consistency the fix
>> touches MH linkers as well.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 2/3/17 5:19 PM, Dmitry Chuyko wrote:
>>> Summary: some method handles can be used with actual parameters not
>>> matching their signatures. It's too expensive to check this on call but
>>> possible to check during inlining. Inlined MHs with wrong usage caused
>>> further assertion failures in debug builds. The fix adds checks for
>>> signatures match during MH inlining so it fails if they are not
>>> matching. New test checks linkToStatic and invokeBasic cases.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8166110
>>>
>>> Webrev: http://cr.openjdk.java.net/~vlivanov/dchuyko/8166110/webrev.00/
>>>
>>> Testing: new test, Hotspot and JDK tests, promotion benchmarks.
>>>
>>> Thanks,
>>> -Dmitry