On Fri, 6 Mar 2026 04:25:24 GMT, Yasumasa Suenaga <[email protected]> wrote:
> We saw the failure in serviceability/sa/TestJhsdbJstackMixedWithXComp.java on > Valhalla repo. `jhsdb jstack --mixed` could not unwind continuation call > frames as following: > > > * LingeredAppWithVirtualThread.run() bci:15 line:69 > (Interpreted frame) > * java.lang.Thread.runWith(java.lang.Object, > java.lang.Runnable) bci:5 line:1540 (Compiled frame [deoptimized]; > information may be imprecise) > * java.lang.VirtualThread.run(java.lang.Runnable) bci:62 > line:472 (Compiled frame [deoptimized]; information may be imprecise) > 0x00007fdad773cf98 <StubRoutines (continuation stubs)> > 0xfefefefefefefefe ???????? > > > I found that `frame::sender_for_compiled_frame()` in frame_x86.inline.hpp has > a special case if sender PC has return barrier entry, but SA does not handle > it. > > This is not only a problem on Valhalla. Same problem exists on JDK. So I want > to fix on JDK. > This PR passed serviceability/sa tests on Linux, and also > TestJhsdbJstackMixedWithXComp.java on Valhalla passed 100 times. > > This PR is assembled by following commits: > > * Follows continuation-related code in HotSpot, and use it on AMD64 SA code > * > https://github.com/openjdk/jdk/pull/30107/changes/4af559ee0dbc0bb61e0bcd91bb17459a5abf50ad > * Fix for AArch64 > * > https://github.com/openjdk/jdk/pull/30107/changes/2cff0fe40c52b88dd8d79ad1534e73b7b0f88f8d > * Fix for RISC-V > * > https://github.com/openjdk/jdk/pull/30107/changes/fdef0384577bb71d34371f11bc5878494f276857 > * Fix for PPC64 > * > https://github.com/openjdk/jdk/pull/30107/changes/5d9774d82c7e8c4d92eae8995ab014232ce9590e Overall this looks good and passes my testing. I can only test that it builds on riscv64. I can't run tests on that platform, and can't do builds or testing on s390 or PPC. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/Continuation.java line 41: > 39: public static boolean isSPInContinuation(ContinuationEntry entry, > Address sp) { > 40: return entry.getEntrySP().greaterThan(sp); > 41: } This could be made private. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/Continuation.java line 43: > 41: } > 42: > 43: public static ContinuationEntry getContinuationEntryForSP(JavaThread > thread, Address sp) { This also could be made private. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/Frame.java line 145: > 143: > 144: public void setSP(Address newSP) { > 145: throw new UnsupportedPlatformException("Continuation is not yet > implemented."); Although I understand that this method is not overridden on platforms that don't support continuations (s390), the relationship between a method with a generic name like "setSP" and a specific error messages about continuations is not obvious. If setSP() only only useful in the context of continuations, I'd suggest naming it something like setContinuationSP(), or you can leave it as setSP() but get rid of the reference to continuations in the error message. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/amd64/AMD64Frame.java line 282: > 280: // _chunk = > stackChunkHandle(Thread::current()->handle_area()->allocate_null_handle(), > true /* dummy */); > 281: // } > 282: I don't understand how this comment is relevant to the code below. Why is this here? ------------- PR Review: https://git.openjdk.org/jdk/pull/30107#pullrequestreview-3905865891 PR Review Comment: https://git.openjdk.org/jdk/pull/30107#discussion_r2898474442 PR Review Comment: https://git.openjdk.org/jdk/pull/30107#discussion_r2898475078 PR Review Comment: https://git.openjdk.org/jdk/pull/30107#discussion_r2897723439 PR Review Comment: https://git.openjdk.org/jdk/pull/30107#discussion_r2898482033
