> This PR will strengthen the ability of `frame::safe_for_sender()` to validate 
> an asynchronous sampled frame. Before this strengthen 
> `frame::safe_for_sender()` could accept a faulty frame and later the 
> `is_older()` check would cause an assertion to fail in `frame::sender()`.
> 
> The added strengthening validates the sampled frame's sender FP, so 
> `AsyncGetCallTrace()` can reject faulty frames. This was enough to prevent 
> the assertion in my gprof-ng reproducer.
> 
> However: Reading @caoman's comment in 
> [JDK-8382486](https://bugs.openjdk.org/browse/JDK-8382486) I realized that 
> the strengthening of `frame::safe_for_sender()` was not enough. A sender's FP 
> that is within the stack and older than the current frame might slip through 
> the strengthening of `frame::safe_for_sender()` but it might still point to 
> something that is not a valid walkable stack frame. This is because a 
> profiler samples the registers at an arbitrary point, which means that the id 
> of a sampled frame might not be comparable with the current, so the 
> `is_older()` assertion might still fail in `frame::sender()`.
> 
> The solution was to mark all profiler/`AsyncGetCallTrace()` related register 
> maps as async and treat them as unreliable in the `frame::sender()` assert.
> 
> The strengthening of `frame::safe_for_sender()` is still in this PR because 
> it's a good way for `AsyncGetCallTrace()` to reject faulty frames before 
> calling `frame::sender()`.
> 
> Unfortunately I have only been able to reproduce the failure by running 
> `gprofng collect app` on a `x86` built java machine running the Derby test, 
> but that has on the other hand been a very steady reproducer. So all changes 
> in other platforms are just copies of the changes in `x86`. But since it's a 
> generic code change I trust that it will work for all the other platforms as 
> well.
> 
> Passes tier1-3 on supported platforms.
> All other platforms (`riscv64`, `ppc64le` and `s390x`) has been tested with 
> `TEST=serviceability/AsyncGetCallTrace/MyPackage/ASGCTBaseTest.java` using 
> QEMU.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Fredrik Bredberg has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains four additional 
commits since the last revision:

 - Updated copyright date
 - s390 fix
 - Merge branch 'master' into 8382486_assert_failed_in_frame_sender
 - 8382486: assert failed in frame::sender

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/32850/files
  - new: https://git.openjdk.org/jdk/pull/32850/files/7fee04a1..4883076c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=32850&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=32850&range=00-01

  Stats: 52679 lines in 1094 files changed: 28823 ins; 15960 del; 7896 mod
  Patch: https://git.openjdk.org/jdk/pull/32850.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/32850/head:pull/32850

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

Reply via email to