RFR: 8275638: GraphKit::combine_exception_states fails with "matching stack sizes" assert [v3]

[Dean Long]

On Tue, 7 Dec 2021 08:25:50 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Root cause is identical to 8273165 AFIU: late inline of a virtual call
>> can throw from 2 different paths (null check and the call
>> itself). That breaks because the logic for exceptions expects the
>> stack for all paths that throw exceptions to have the same stack size.
>> 
>> AFAIU, the stack doesn't matter exception handling: either the
>> exception is caught by a exception handler and then the stack is
>> popped and the exception is pushed or, the exception is rethrown to
>> the caller in which case the current stack is also popped (that is the
>> jvm state for the current method). As a consequence the fix I propose
>> is to ignore the stack in GraphKit::combine_exception_states().
>> 
>> AFAIU, the same fix would work for 8273165 but I left the current work
>> around as is: not sure if we want to be conservative for now or not
>
> Roland Westrelin 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 six additional commits since the last revision:
> 
>  - comment
>  - Merge branch 'master' into JDK-8275638
>  - alternate fix
>  - make test runnable with release build
>  - more
>  - fix

Marked as reviewed by dlong (Reviewer).

Yes, I agree that's the current difference between parse-time inlining and late inlining as implemented today.  I'd just like to know why late inlining must combine the exceptions states into a single state.  But that's not a blocker for your fix.

John wrote me and said your fix looks OK (thought not a general solution).  For your test, he suggested adding a 3rd case with extra call depth: "test3 -> mcaller -> m".  Also, I believe @vnkozlov will take a look too.

It would be nice if some of the assumptions the code is making could be checked, which could be done in a separate RFE with perhaps a more general solution:
- an exception state with cleared stack won't be used to reexecute a bytecode (uncommon trap)
- an exception state with uncleared stack must be used to reexecute
- safepoint stack size matches computed interpreter oopmap stack size (VerifyStack logic)

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

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