RFR: 8277602: Deopt code does not extend the stack enough if the caller is an optimize entry blob [v2]

[Jorn Vernee]

On Thu, 25 Nov 2021 18:48:45 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Deoptimization code does not recreate c2i adapter 'frames'. For compiled callers this means that the stack needs to be adjusted manually to make room for the parameters when the callee is converted to an interpreter frame (essentially emulating what a c2i adapter would do).
>> 
>> To check if the caller does a compiled call, the current code uses `frame::is_compiled_frame()`, which is true if the codeblob of the caller frame is an instance of `CompiledMethod`.
>> 
>> However, optimized entry blobs also do compiled calls, are not detected by this test, and therefore don't get their stack adjusted correctly.
>> 
>> To address this, I've added a new `frame::is_compiled_caller` function to determine if the caller is doing a compiled call, and I use that in the deopt code instead of `is_compiled_frame`.
>> 
>> This patch also removes an old workaround that tried to fix the issue by allocating some spill space in the optimized entry blob frame, but this only accounts for the first argument. If there are more arguments we still have a problem. The suggested patch fixes this the right way I think.
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: run-test-jdk_foreign on Windows x64 and Linux x64 (afaik these are the only tests that use optimized entry blobs)
>
> Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Fix after merge
>  - Merge branch 'master' into Deopt_Stack_Fix
>  - Add test + asserts
>  - Properly handle optimized entry frame callers during deopt

I've added a test and a couple of asserts that catch the case I'm trying to fix (mostly so that the test fails in a more obvious way). If I revert the fix in deoptimization.cpp the latter assert fires (that's the case I found during debugging as well), and when I re-add the fix tests pass again.

I ran this through tier1-3 as well.

The asserts I've added only check for 'overflow' in the case of compiled callers. I played around with adding a similar check for interpreted callers as well, but I wasn't able to provoke an assertion failure with that, and I'm not 100% what the right check should be. I suspect interpreted callers are rare when we deopt due to an uncommon trap. For these reasons, I've left the asserts to check for overflow in the case of compiled callers only, for now.

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

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