RFR: 8267807: C2: Downcast receiver to target holder during inlining

[Vladimir Ivanov]

On Wed, 2 Jun 2021 23:11:41 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> I think you need a CheckCastPP as the comment (written by me long ago IIRC) suggests.
>> 
>> I’d like to believe you can trust the type of a CHA receiver, but I don’t.  What the IR says about interface types is (last time I looked) just as untrustworthy as what the verifier thinks about them.
>> 
>> (We should fix this decisively someday.  Or did we do this cleanup already and missed it?  We have had dozens of bugs
>> in early C2 because of untrustworthy interface types.)
>> 
>> Suppose CHA (of default methods, yay!) says that since x is interface Foo, then default Foo::m is the only possible target
>> of x.m.  Before we execute Foo::m we still need a type check of x, because it could be any Object whatsoever.
>> 
>> I suggest adding the CheckCastPP in, in all modes, with fallback to de-opt not VM crash.  The IR types are saying there is not a dominating check.
>
> I closely looked at the relevant logic and couldn't find any possibility to bypass receiver subtype check. 
> 
> There are 2 call site shapes possible:
> 
> 1. `invokevirtual C.m RECV => J.m DEFAULT` (`J.m` - resolved and selected method)
> 
> Verifier guarantees that RECV <: C (REFC) and method resolution ensures that C <: J (REFC <: DECC).
> 
> 
> 2. `invokeinterface I.m RECV => J.m DEFAULT` (`J.m` - resolved and selected method)
> 
> `I` (REFC) is required to be an interface (structural constraint of `invokeinterface` instruction).
> 
> Then, here's the relevant code:
> 
>   ciInstanceKlass* actual_receiver = resolved_iklass;
>   ...
>   ciInstanceKlass *ikl = receiver_type->klass()->as_instance_klass();
>   if (ikl->is_loaded() && ikl->is_initialized() && !ikl->is_interface() &&
>       (ikl == actual_receiver || ikl->is_subtype_of(actual_receiver))) {
>     // ikl is a same or better type than the original actual_receiver,
>     // e.g. static receiver from bytecodes.
>     actual_receiver = ikl;
>     // Is the actual_receiver exact?
>     actual_receiver_is_exact = receiver_type->klass_is_exact();
>   }
> 
>   ciInstanceKlass*   calling_klass = caller->holder();
>   ciMethod* cha_monomorphic_target = resolved_method->find_monomorphic_target(calling_klass, resolved_iklass, actual_receiver, check_access);
> 
> ...
> 
> ciMethod* ciMethod::find_monomorphic_target(...) {
>   ...
>   if (actual_recv->is_interface()) {
>     // %%% We cannot trust interface types, yet.  See bug 6312651.
>     return NULL;
>   }
> 
> 
> 
> `receiver_type` can represent either an interface (1a) or a class (1b `ikl`).
> 
> 1a. But if `ikl->is_interface()`, then `actual_receiver` stays as an interface (`I`, REFC) and `find_monomorphic_target()` returns NULL.
> 
> 1b. If `ikl` is a class, then `ikl != actual_receiver` and depending on whether `ikl->is_subtype_of(actual_receiver))`, `actual_receiver` is either REFC (interface) or `receiver_type`.
> 
> `find_monomorphic_target()` can succeed only in the latter case (`actual_receiver` is a class and not an interface), but previous static subtype check guarantees that the receiver type (a class) implements the interface.  
> 
> Q.E.D

So, CHA is still kept conservative when interface types on receiver are encountered. 
Currently, only single implementor case is optimised, but there are proper receiver subtype checks in place.

It should be quite straightforward to relax aforementioned constraints on receiver types (only classes, no interfaces), but then we would require additional receiver subtype check accompanying `Compile::optimize_virtual_call()` calls.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4212