RFR: JDK-8302320: AsyncGetCallTrace obtains too few frames in sanity test [v2]

[Jorn Vernee]

On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger <duke at openjdk.org> wrote:

>> Extends the existing AsyncGetCallTrace test case and fixes the issue by modifying `MethodHandles` code.
>
> Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Implement addptr suggestion by @JohnVernee and @reinrich

Thanks for trying it out! I ran tier 1-4 here as well, and all failing tests are in `vmTestbase/vm/mlvm/indy/func/jvmti`. They all seem to be using `PopFrame` before a crash happens, so I think that might be where the problem is. I don't think it's impossible that there is other code out there that has it's own workaround for the incorrect unextended_sp.

Also, I later realized that interpreter frames also save the sp before before making a call (`interpreter_frame_last_sp_offset`), and I can find at least some code that seems to assume `caller.unextended_sp() == (intptr_t*)caller.at(frame::interpreter_frame_last_sp_offset)`, so we'd just be breaking another invariant with my "fix".

There's probably a bit of tech debt here (filed: https://bugs.openjdk.org/browse/JDK-8302745).

I had a closer look at the original fix, and it looks like the `unextended_sp` is only used to check the size of the caller frame for interpreter -> interpreter calls.  So, I think it's okay to relax the `unextende_sp >= sp` check (the invariant it tests turns out not to hold for otherwise valid stacks any way). But, in that case, please leave a comment that explains why we can have `sp > unextended_sp`.

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

PR: https://git.openjdk.org/jdk/pull/12535