RFR: 8336042: Caller/callee param size mismatch in deoptimization causes crash [v2]

[Dean Long]

On Thu, 13 Feb 2025 18:57:24 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix bounds checks
>
> src/hotspot/share/runtime/deoptimization.cpp line 645:
> 
>> 643:     methodHandle method(current, deopt_sender.interpreter_frame_method());
>> 644:     Bytecode_invoke cur = Bytecode_invoke_check(method, deopt_sender.interpreter_frame_bci());
>> 645:     if (cur.is_invokedynamic() || cur.is_invokehandle()) {
> 
> Can you elaborate, please, why invokedynamic case is not needed anymore?

As far as I can tell, it was never needed.  If an invokedynamic or invokehandle adds an appendix, then it will show up in the callee, and will be reflected in the caller args size, so there is no mismatch.  As far as the JVM is concerned, an invokedynamic/invokehandle looks like a call to a JVM-generated adapter.  The only way for invokedynamic/invokehandle to cause an argument mismatch is if the JVM resolved the call-site to an adapter that was actually a MethodHandle linker.  That is the exception I describe in the comment below.  If we ever allowed the JVM to do that, then several other checks would also need to be fixed.
For the record, this code used to call cur.is_method_handle_invoke(), which was also wrong, but at least it had a name closer to what we would want.  Ideally, something like is_method_handle_linker_invoke() that checks for linkToVirtual, linkToStatic, linkToSpecial, and linkToInterface would have been better.
The old comment about "arbitrary chains of calls" seems to be left over from an early JSR292 feature known as Ricochet Frames.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23557#discussion_r1955228726