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

[John R Rose]

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

It is true that when an exception is being thrown the stack is clear.

The only reason C2 sometimes keeps (or used to keep) stuff on the stack along an exception path is that, when an exception in being thrown by a bytecode that is re-executable, C2 might cleverly issue an uncommon trap that re-executes the throwing bytecode.  In that case, having the stack unchanged is obviously important.  But call instructions are not re-executable, and any exception coming from a call (directly or indirectly) must clear the stack.

Even if there is a matching local handler in the method making the call, such as a `catch (NullPointerException _)` where a call might throw NPE, the correct move is to clear the stack when the throw is started (by the call), and then thread the JVMS (with empty stack) into the handler.

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

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