RFR: 8311542: Consolidate the native stack printing code
Kim Barrett
kbarrett at openjdk.org
Fri Dec 6 02:55:39 UTC 2024
On Mon, 2 Dec 2024 07:36:59 GMT, David Holmes <dholmes at openjdk.org> wrote:
> We now print native stacks in a number of contexts, not just VMError reporting, and we have to try `os::platform_print_native_stack` else fall back to `VMError::print_native stack`.
>
> The refactoring adds a new `NativeStackPrinter` (NSP) helper class which can be constructed with some of the context information for the printing that will follow. This avoids the need for the print functions to have a large number of parameters. We have to expose both the top-level printing functionality and the "lower-level" frame-based stack walk as the error reporter needs access to that directly. To maintain the exact same format of output the NSP has to be aware of some error reporter usage requirements.
>
> I also had to expose a direct `frame` taking print function for the Debug.cpp `pns` case.
>
> Testing:
> - tiers 1 - 4
>
> Some frame management code was also moved from `VMError` to the `frame` class.
Looks good, except for some const issues. I'm approving as is if you decide not to
address those in this PR.
src/hotspot/share/runtime/frame.cpp line 1566:
> 1564: * @returns an invalid frame (i.e. fr.pc() === 0) if the caller cannot be obtained
> 1565: */
> 1566: frame frame::next_frame(frame fr, Thread* t) {
`frame` is a non-trivial type to copy, so it seems like it would be better to pass it by const-ref rather
than by value. That might trip over lots of const-correctness issues when accessing it though. If
pulling that thread looks like an excessive tangle then clean this up as a followup.
src/hotspot/share/utilities/nativeStackPrinter.cpp line 44:
> 42: }
> 43:
> 44: void NativeStackPrinter::print_stack_from_frame(outputStream* st, frame& fr,
Why is `frame` passed by non-const reference? This doesn't seem like a function that should be modifying
it? Though changing this might also run afoul of const-correctness issues when accessing the frame, so
feel free to defer to a followup if that's a problem.
-------------
Marked as reviewed by kbarrett (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22472#pullrequestreview-2483484677
PR Review Comment: https://git.openjdk.org/jdk/pull/22472#discussion_r1872487666
PR Review Comment: https://git.openjdk.org/jdk/pull/22472#discussion_r1872497196
More information about the hotspot-runtime-dev
mailing list