On Thu, 16 Feb 2023 14:25:15 GMT, Johannes Bechberger <d...@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
