On Sat, 12 Sep 2026 10:18:37 GMT, Fredrik Bredberg <[email protected]> wrote:
> 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). @TheRealMDoerr, @RealFYang, @offamitkumar Hi guys! Here is a profiler/`AsyncGetCallTrace()` related fix that avoids an otherwise failing `assert()`. Maybe you would like to take it for a spin on your platforms. The failing `assert()` is missing in s390 because loom support was missing when the `assert()` was added. I haven't added the `assert()` to s390 cause I think that should be done separately, but I did add the profiler/`AsyncGetCallTrace()` fix for s390x for symmetrical reasons. ------------- PR Comment: https://git.openjdk.org/jdk/pull/32850#issuecomment-5680995866
