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

[Remi Forax]

Hi Vladimir,
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*.

Rémi
* apart if we introduce a custom method handle.

----- 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