Is it necessary to check subtype for invokeinterface of private method?

[Remi Forax]

----- Original Message -----
> From: "Liu, Xin" <xxinliu at amazon.com>
> To: "David Holmes" <david.holmes at oracle.com>, "hotspot-dev" <hotspot-dev at openjdk.java.net>
> Cc: "Simonis, Volker" <simonisv at amazon.de>, "Dean Long" <dean.long at oracle.com>
> Sent: Mercredi 17 Novembre 2021 09:22:10
> Subject: Re: Is it necessary to check subtype for invokeinterface of private method?

> Hi, David,
> 
> Thanks you the head-up! Now I understand why c2 generates the checkcast
> code for invokespecial and invokeinterface. It must conform to the JVM
> spec.
> 
> C1 does the checkcast for invokespecial right now.
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/c1/c1_GraphBuilder.cpp#L1874
> 
> ./test/jdk/java/lang/invoke/PrivateInterfaceCall.java is correct. The
> ICCE is expected. I will implement invokeinterface by book.
> 
> I don't intend the challenge the JVM spec. The private interface methods
> is new for me. I double think about this. There's no polymorphism for
> them, so c1/c2 can use relocInfo::opt_virtual_call_type to optimize the
> callsites of them, but the typecheck still needs to be in place!

Yes, the type check is due to the fact that the bytecode verifier does not verify if a value really implement an interface so a typecheck as to be inserted.
Usually, it means that the world seen by the compiler and the world seen by the VM are not the same hence the IncompatibleClassChangeError.

> 
> thanks,
> --lx

regards,
Rémi

> 
> 
> On 11/16/21 10:56 PM, David Holmes wrote:
>> CAUTION: This email originated from outside of the organization. Do not click
>> links or open attachments unless you can confirm the sender and know the
>> content is safe.
>> 
>> 
>> 
>> Hi Xin,
>> 
>> On 16/11/2021 7:44 pm, Liu, Xin wrote:
>>> Hi,
>>>
>>> I am working on the regex performance in JDK-8274983. Even though it
>>> begins with regex, the problem boils down to invokeinterface to the
>>> private interface methods. Before hidden class (JDK-8238358), lambda
>>> meta factory generates invokespecial for them. Now it generates
>>> invokeinterface instead. C1 doesn't recognize the new code pattern and
>>> generates an ic virtual call for the callsite. If many classes all
>>> implement a common interface, they trash the ic stub because the
>>> concrete classes are different. InvokePrivateInterfaceMethod.java with
>>> -XX:+TraceCallFixup can reveal this pathological slowness.
>>>
>>> Is it the intentional behavior of C1? I see that C2 actually generates
>>> checkcast code sequence for this case. I would like to patch up C1
>>> because C1 plays an important role for the startup time.
>> 
>> Can't comment on C1 issue.
>> 
>>> I have a patch to let C1 treats invokeinterface private interface
>>> methods as invokespecial. In other words, I treat the private interface
>>> methods as effective final. It runs pretty well until I encounter the
>>> regression ./test/jdk/java/lang/invoke/PrivateInterfaceCall.java. That
>>> leads me to the second question.
>>>
>>> In my understanding, the code unsafeCastI2() is essentially a typecast
>>> of a function pointer, isn't it?  My take of "unsafe" in "unsafeCastI2"
>>> is that its behavior undefined, then why we need to check ICCE here?
>> 
>> "unsafe" means that the validity of the cast can't be checked
>> immediately, but it will be checked when the result is actually used -
>> it is not "undefined behaviour". The VM and MethodHandle specifications
>> require the subtype check on the receiver:
>> 
>> JVMS 6.5 invoke_interface - Run-time Exception: Otherwise, if the class
>> of objectref does not implement the resolved interface, invokeinterface
>> throws an IncompatibleClassChangeError.
>> 
>>>      System.out.println("ICCE PrivateInterfaceCall.invokeDirect D1");
>>>      shouldThrowICCE(() ->
>>> PrivateInterfaceCall.invokeDirect(unsafeCastI2(new D1())))
>>>
>>>      static I2 unsafeCastI2(Object obj) {
>>>          try {
>>>              MethodHandle mh = MethodHandles.identity(Object.class);
>>>              mh = MethodHandles.explicitCastArguments(mh,
>>> mh.type().changeReturnType(I2.class));
>>>              return (I2)mh.invokeExact((Object) obj);
>>>          } catch (Throwable e) {
>>>              throw new Error(e);
>>>          }
>>>      }
>>>
>>> In real world, how much meaningful we detect the error if we
>>> accidentally invoke a private interface method where we actually don't
>>> implement that interface. I think it's only possible via methodhandle
>>> and jasm, right? if we say it's undefined behavior, I think we can skip
>>> typecheck. It would make lambda code faster.
>> 
>> I think there are potential security considerations here, but if you
>> want to make a case for change then email:
>> 
>>   jls-jvms-spec-comments at openjdk.java.net
>> 
>> Cheers,
>> David
>> 
>> 
>>> thanks,
>>> --lx