> Hi, could I have a review of this patch. > > This patch fixes [JDK-8395826](https://bugs.openjdk.org/browse/JDK-8395826) > by consistently using `PC - 1` to lookup native sender in mixed jhsdb jstack > on Linux AMD64 and AArch64. > > Currently, `SA` first looks up DWARF at the exact sender PC and retries with > `PC - 1` only if parsing throws. However, a native return address can > coincide with the start of the next function. In that case, the lookup can > succeed, and selects the wrong FDE, so the retry is never triggered. > > The patch uses `PC - 1` for `native caller library`, `DWARF` and `symbol` > lookups while preserving the actual frame PC. Top-frame PCs, PCs restored > from signal contexts, recognized signal trampoline PCs, and code-cache (Java) > PCs remain unchanged. > > For clarity, `PCs restored from signal contexts` and `Recognized signal > trampoline PCs` are different: > - `PCs restored from signal contexts`: When delivering a signal, the Linux > kernel saves the interrupted thread’s register state in an on-stack > `ucontext` and transfers control to the signal handler. When SA unwinds a > signal trampoline frame, it reads the saved PC and other registers from that > context to reconstruct the interrupted frame. This PC represents an execution > location, not a normal call return address, so it is used unchanged. See the > real scenario bellow: > > ... > 0x00007ffff713dcbc VMError::report_and_die(Thread*, ...) + 0x5b > 0x00007ffff6e2b0ba JVM_handle_linux_signal + 0x409 > 0x00007ffff7c45330 __restore_rt <signal trampoline> <--- > current frame, uses `PCs restored from signal contexts` to find the sender > 0x00007ffff7e1b114 Java_jdk_test_lib_apps_LingeredApp_crash + 0x4 <--- > the sender > ... > 0x00007ffff7fb425d ThreadJavaMain + 0xc > 0x00007ffff7c9cb84 start_thread + 0x383 > 0x00007ffff7d29ecc __clone3 + 0x2b > > > - `Recognized signal trampoline PCs`: The Linux kernel sets the signal > trampoline’s address as the signal handler’s return address. If the recovered > senderPC is recognized as a signal trampoline, SA uses it unchanged. Unlike a > normal call return address, it has no associated call site immediately before > it; subtracting 1 could move the lookup outside the trampoline and into > another function. See the real scenario bellow: > > ... > 0x00007ffff713dcbc VMError::report_and_die(Thread*, ...) + 0x5b > 0x00007ffff6e2b0ba JVM_handle_linux_signal + 0x409 <--- > current frame, uses `Recognized signal trampoline PCs` to find the sender > 0x00007ffff7c45330 __restore_rt <signal t...
Long Yang has updated the pull request incrementally with one additional commit since the last revision: Move the isSignalTrampoline check to just after stripPAC ------------- Changes: - all: https://git.openjdk.org/jdk/pull/33620/files - new: https://git.openjdk.org/jdk/pull/33620/files/029a7d51..9a6ea3d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=33620&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=33620&range=01-02 Stats: 32 lines in 1 file changed: 8 ins; 18 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/33620.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/33620/head:pull/33620 PR: https://git.openjdk.org/jdk/pull/33620
