RFR: 8367862: debug.cpp: Do not print help message for methods not compiled in

[Manuel Hässig]

On Wed, 17 Sep 2025 11:27:27 GMT, Kerem Kat <krk at openjdk.org> wrote:

> The `help` command in `debug.cpp` was out of date, listing only a fraction of the available functions. This and other commands callable from gdb via `call help()`.
> 
> I added all commands to the help message, except `pns2` as it is documented as not being useful when called from gdb.
> 
> Also fixed is the message for the conditional `pns` command appearing in PRODUCT builds.

Thank you for providing this improvement, @krk! This sure is a good quality of life improvement.

I do have a few suggestions below. Also, please update either the JBS issue description or the PR title so they match up.

src/hotspot/share/utilities/debug.cpp line 661:

> 659:   tty->print_cr("  findm(intptr_t pc)  - finds Method*");
> 660:   tty->print_cr("  find(intptr_t x)    - finds & prints nmethod/stub/bytecode/oop based on pointer into it");
> 661:   tty->print_cr("  findpc(intptr_t x)  - finds & prints nmethod/stub/bytecode/oop based on pointer into it (verbose)");

Since you are touching this, it would be good to make the grammar consistent. My suggestion is to do it like above and drop the "s" on all verbs to describe to the user what they can do with the function.
Suggestion:

  tty->print_cr("  findnm(intptr_t pc) - find nmethod*");
  tty->print_cr("  findm(intptr_t pc)  - find Method*");
  tty->print_cr("  find(intptr_t x)    - find & print nmethod/stub/bytecode/oop based on pointer into it");
  tty->print_cr("  findpc(intptr_t x)  - find & print nmethod/stub/bytecode/oop based on pointer into it (verbose)");

Could you do that for the rest as well?

src/hotspot/share/utilities/debug.cpp line 667:

> 665:   tty->print_cr("                   pns($sp, $rbp, $pc) on Linux/amd64 or");
> 666:   tty->print_cr("                   pns($sp, $ebp, $pc) on Linux/x86 or");
> 667:   tty->print_cr("                   pns($sp, $fp, $pc)  on Linux/AArch64 or");

Since you are already ifdefing, you could go all the way and only print one of the pns examples:
Suggestion:

  tty->print_cr("  pns(void* sp, void* fp, void* pc) - print native (i.e. mixed) stack trace. E.g.");
#ifdef LINUX
  AMD64_ONLY(tty->print_cr("                   pns($sp, $rbp, $pc) on Linux/amd64");)
  IA32_ONLY(tty->print_cr("                   pns($sp, $ebp, $pc) on Linux/x86");)
  AARCH64_ONLY(tty->print_cr("                   pns($sp, $fp, $pc)  on Linux/AArch64");)

(this is not a complete suggestion, because Github won't let me...)

src/hotspot/share/utilities/debug.cpp line 673:

> 671:   tty->print_cr("                 - in gdb do 'set overload-resolution off' before calling pns()");
> 672:   tty->print_cr("                 - in dbx do 'frame 1' before calling pns()");
> 673: #endif

Suggestion:

#endif // !PRODUCT

Nit: this is my readability pet peeve.

src/hotspot/share/utilities/debug.cpp line 703:

> 701: 
> 702:   tty->print_cr("compiler debugging");
> 703:   tty->print_cr("  debug()  - to set things up for compiler debugging");

Suggestion:

  tty->print_cr("  debug()  - set things up for compiler debugging");

One last grammar nit.

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

Changes requested by mhaessig (Committer).

PR Review: https://git.openjdk.org/jdk/pull/27341#pullrequestreview-3243305992
PR Review Comment: https://git.openjdk.org/jdk/pull/27341#discussion_r2361903475
PR Review Comment: https://git.openjdk.org/jdk/pull/27341#discussion_r2361900556
PR Review Comment: https://git.openjdk.org/jdk/pull/27341#discussion_r2361906096
PR Review Comment: https://git.openjdk.org/jdk/pull/27341#discussion_r2361910106