On Thu, 15 Oct 2026 02:18:04 GMT, Long Yang <[email protected]> wrote:
> Hi, could I have a review of this patch. This patch fixes > [JDK-8391637](https://bugs.openjdk.org/browse/JDK-8391637). > > Due to my GitHub account settings (Enable Copilot Code Review for all my > PRs), my previous [PR](https://github.com/openjdk/jdk/pull/33358) had Copilot > Review enabled by default. I have disabled it and now open this new PR, with > no changes made to the code. > > On `risc-v`, > [LinuxRISCV64CFrame.sender()](https://github.com/openjdk/jdk/blob/c368bac40e185398fbc40dc4d75a457f7a36f954/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/riscv64/LinuxRISCV64CFrame.java#L70) > validates only FP alignment, then dereferences FP for SP, link, and return > address. An aligned arbitrary FP (such as 0x90) can therefore cause > UnmappedAddressException, and lead to incomplete stack for the target thread. > > `aarch64` and `amdf64`already use `dwarf` to unwind native frames, see > https://github.com/openjdk/jdk/blob/c368bac40e185398fbc40dc4d75a457f7a36f954/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/amd64/LinuxAMD64CFrame.java#L36 > https://github.com/openjdk/jdk/blob/c368bac40e185398fbc40dc4d75a457f7a36f954/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/aarch64/LinuxAARCH64CFrame.java#L38 > > This patch makes `risc-v` to unwind with `dwarf` too. > > The `risc-v` version basically has the same design as the `aarch64` version. > There are also several differences , primarily due to differences in calling > conventions. Almost every difference is accompanied by an explanatory > comment. > > I have ran tests in `test/hotspot/jtreg/serviceability/sa` on `risc-v` and > `amd64`, all passed. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Thanks for this patch! I have a couple of additional comments. Could we add `-fasynchronous-unwind-tables` to the RISC-V build flags for the JVM, JDK libraries and native tests? Older supported toolchains do not enable these tables by default, so mixed-stack unwinding may stop in JDK native code. diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4 index 04a475c65b63..e2a30c5cfbd5 100644 --- a/make/autoconf/flags-cflags.m4 +++ b/make/autoconf/flags-cflags.m4 @@ -724,6 +724,12 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_CPU_DEP], else AC_MSG_RESULT([no]) fi + + if test "x$FLAGS_OS" = xlinux && test "x$FLAGS_CPU" = xriscv64; then + # SA needs asynchronous unwind tables to unwind native frames. They are + # not enabled by default on riscv64 before GCC 13 and Clang 17. + $1_CFLAGS_CPU="${$1_CFLAGS_CPU} -fasynchronous-unwind-tables" + fi fi if test "x$TOOLCHAIN_TYPE" = xgcc; then Could we also enable all three configurations of `TestJhsdbJstackMixedWithXComp` on riscv64, with the same QEMU exclusion as `TestJhsdbJstackMixed`? src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.hpp line 37: > 35: #elif defined(__aarch64__) > 36: #include "dwarf_regs_aarch64.h" > 37: #elif defined(riscv64) Suggestion: #elif defined(__riscv) && __riscv_xlen == 64 Minor: maybe we can use `defined(__riscv) && __riscv_xlen == 64`? `defined(riscv64)` works with the build-provided `-Driscv64`, but the adjacent x86_64 and aarch64 branches use compiler predefined macros. Using the corresponding RISC-V macros would keep this consistent. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/riscv64/LinuxRISCV64CFrame.java line 100: > 98: } > 99: } > 100: } Could we honor the CIE return-address column in both the stack lookup and the register fallback? GCC's `__riscv_save_*` helpers use x5, while `getReturnAddressOffsetFromCFA()` still queries x1. Depending on where the thread stops, this can throw or skip the helper's immediate caller. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/linux/riscv64/LinuxRISCV64CFrame.java line 174: > 172: ? null > 173: : fp().getAddressAt(C_FRAME_LINK_OFFSET * > VM.getVM().getAddressSize()); > 174: } Could `getSenderFP()` catch the same memory-read exceptions as `getSenderPC()`? It is called before the `senderPC == null` check, so an invalid FP can throw even after `getSenderPC()` has returned null to terminate the walk. ------------- PR Comment: https://git.openjdk.org/jdk/pull/33374#issuecomment-6183910197 PR Review Comment: https://git.openjdk.org/jdk/pull/33374#discussion_r4278774858 PR Review Comment: https://git.openjdk.org/jdk/pull/33374#discussion_r4278780300 PR Review Comment: https://git.openjdk.org/jdk/pull/33374#discussion_r4278783696
