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

[Roland Westrelin]

On Tue, 14 Dec 2021 03:34:36 GMT, Dean Long <dlong at openjdk.org> wrote:

> I'm still staring at the code trying to figure out why parse-time inlining can avoid with this problem, but late inlining can't.

With parse time inlining, when the call is processed, 2 exception states are added. One for the null check before the call and one for exceptions thrown after the call. They have different sp so they can't be combined. Either the exception is caught in the current method in which case, the 2 exception states are processed independently in Parse::catch_inline_exceptions() at which point the expression stacks are popped for each one of them. Or the exception is passed on the caller in which case, the expression stacks are also popped in Parse::throw_to_exit().

With late inlining, the call to Parse::catch_inline_exceptions() or Parse::throw_to_exit() happened before the call site is processed and that one assumed a single exception state. So when late inlining occurs, exception states must be combined while with parse time inlining they don't need to be combined.

FWIW, my understanding of what happens comes from running the tests of this PR with and without late inlining and stepping through the code.

I'm unclear where we go from here. My opinion is that, by going over the history of the code, you figured out why expression handling preserves the stack. And AFAICT, you also found that the code that needed the stack preserved was removed. So I would go with a fix that pops the stack when an exception is thrown, have it go through as much testing as possible and integrate that fix if testing is clean. Whether we want to have the exception on stack in exception states or as an extra edge (as it is today) is something that could be revisited later.

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

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