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

[Liu, Xin]

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!

thanks,
--lx


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