8359120: Improve warning message when fail to load hsdis library #25726

kurashige23 · 2025-06-10T13:38:03Z

This PR is improvement of warning message when fail to load hsdis library.

JDK-8287001 introduced a warning on hsdis library load failure. This is useful when the user executes -XX:+PrintAssembly, etc.

However, I think that when hs_err occurs, users might be confused by this warning printed by Xlog. Because users are not likely to know that hsdis is loaded for the [MachCode] section of the hs_err report, they may wonder, for example, "Why do I get warnings about hsdis load errors when -XX:+PrintAssembly is not specified?."

To clear up this confusion, I suggest printing a warning just before [MachCode].

sample output

If hs_err occurs and hsdis load fails without the option to specify where the hs_err report should be output, the following is output to the hs_err_pir log file:

.
.
native method entry point (kind = native)  [0x000001ae8753cec0, 0x000001ae8753dac0]  3072 bytes

Loading hsdis library failed, so undisassembled code is printed in the below [MachCode] section
[MachCode]
  0x000001ae8753cec0: 488b 4b08 | 0fb7 492e | 584c 8d74 | ccf8 6800 | 0000 0068 | 0000 0000 | 5055 488b | ec41 5548
  0x000001ae8753cee0: 8b43 084c | 8d68 3848 | 8b40 0868 | 0000 0000 | 5348 8b50 | 18
.
.

If -XX:+PrintAssembly is specified and hsdis load fails, the following is output to the stdout

$ java -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -version
OpenJDK 64-Bit Server VM warning: PrintAssembly is enabled; turning on DebugNonSafepoints to gain additional output

============================= C1-compiled nmethod ==============================
----------------------------------- Assembly -----------------------------------

Compiled method (c1) 57    2       3       java.lang.Object::<init> (1 bytes)
 total in heap  [0x0000024a08a00008,0x0000024a08a00208] = 512
.
.

[Constant Pool (empty)]


Loading hsdis library failed, so undisassembled code is printed in the below [MachCode] section
[MachCode]
[Instructions begin]
  0x0000024a08a00100: 6666 660f | 1f84 0000 | 0000 0066 | 6666 9066 | 6690 448b | 5208 443b
.
.
[Constant Pool (empty)]


Loading hsdis library failed, so undisassembled code is printed in the below [MachCode] section
[MachCode]
[Verified Entry Point]
  # {method} {0x00000000251a1898} 'toUnsignedInt' '(B)I' in 'java/lang/Byte
.
.

Since the warning added in this fix cover the role of warning introduced in JDK-8287001, I removed lines added in JDK-8287001 and JDK-8289421.

Testing:
・test/hotspot/jtreg/runtime/ErrorHandling on Windows Server 2019 (Some tests use fatdebug version, so I ran with fatdebug version)
・tested on Windows Server 2019 that each print_cr() in 3 routes prints warning just before [MachCode] and that JDK-8287001's warning isn't printed
・GHA testing

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8359120: Improve warning message when fail to load hsdis library (Enhancement - P4)

Reviewers

Manuel Hässig (@mhaessig - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25726/head:pull/25726
$ git checkout pull/25726

Update a local copy of the PR:
$ git checkout pull/25726
$ git pull https://git.openjdk.org/jdk.git pull/25726/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25726

View PR using the GUI difftool:
$ git pr show -t 25726

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25726.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-06-10T13:38:49Z

👋 Welcome back tkurashige! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-06-10T13:39:56Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-06-10T13:40:35Z

@kurashige23 The following label will be automatically applied to this pull request:

hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-06-10T13:44:04Z

Webrevs

01: Full - Incremental (6ff4f9b5)
00: Full (45584ba7)

mhaessig

Thank you for working on this. I have been confused by this myself and think this is a great improvement. I do have a few comments and questions, though.

Currently, I do not understand exactly how your new message is only printed when hsdis is not loaded. Do we only emit a MachCode section if hsdis is not loaded?

mhaessig · 2025-06-19T09:04:18Z

src/hotspot/share/compiler/disassembler.cpp

-  } else {
-    log_warning(os)("Loading hsdis library failed");


Personally, I would leave this warning. It does not hurt, and perhaps someone is depending on it to detect if hsdis is installed correctly.

I thought your comment made sense.
I fixed it.

mhaessig · 2025-06-19T09:10:53Z

src/hotspot/share/code/nmethod.cpp

@@ -3488,6 +3488,9 @@ void nmethod::decode2(outputStream* ost) const {
  if (use_compressed_format && ! compressed_with_comments) {
    const_cast<nmethod*>(this)->print_constant_pool(st);

+    st->bol();
+    st->cr();
+    st->print_cr("Loading hsdis library failed, so undisassembled code is printed in the below [MachCode] section");


Some comments on the message:

I don't think we should use [MachCode] inside the square brackets apart from marking the start of a section. Otherwise, tools parsing a hs_err file incorrectly identify the start of the MachCode section.

Personally, I find the language of the message a bit clunky. Here are a few suggestions I would personally prefer:
Loading hsdis library failed, unable to show disassembled code in MachCode section or Note: Unable to display disassembled code because loading of hsdis library failed..

I don't think we should use [MachCode]

You're right. I will stop using [MachCode].

Here are a few suggestions I would personally prefer:

Thank you for your suggestion.
I prefer the former expression.
However, if hsdis is loaded successfully, the disassembled code will appear in [Disassembly] instead of [MachCode], so I don't think "unable to show disassembled code in MachCode section" is appropriate.
Do you think "undisassembled" is clunky? "undisassembled" is also used in src/hotspot/share/code/nmethod.cpp#L3428, so I don't think it's too clunky.
So, for example, how about Loading hsdis library failed, undisassembled code is shown in MachCode section? If "undisassembled" is clunky, I think it could be "not disassembled" for example.

I do happen to dislike "undisassembled", but I was not aware of its usage in other places in the codebase. I'm fine with the message now.

kurashige23 · 2025-06-23T11:58:47Z

Thank you for working on this. I have been confused by this myself and think this is a great improvement. I do have a few comments and questions, though.

Currently, I do not understand exactly how your new message is only printed when hsdis is not loaded. Do we only emit a MachCode section if hsdis is not loaded?

Thank you for your comment!

Yes, my research shows that we only emit a MachCode section if hsdis is not loaded.

is_abstract() in src/hotspot/share/compiler/disassembler.hpp#L83 determines if hsdis is not loaded. is_abstract() returns true if loading hsdis fails.

  static bool is_abstract() {
    if (!_tried_to_load_library) {
      load_library();
    }
    return ! _library_usable;
  }

There are three direct processes to output the MachCode section:

1. is called when is_abstract() at src/hotspot/share/compiler/disassembler.cpp#L889 returns true.

For example, if loading hsdis fails to load and hs_err occurs

2. is called when is_abstract() at src/hotspot/share/code/nmethod.cpp#L3444 returns true and compressed_with_comments at src/hotspot/share/code/nmethod.cpp#L3445 is false.

For example, if only -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly is specified

3. is called when is_abstract() at src/hotspot/share/code/nmethod.cpp#L3444 returns true and compressed_with_comments at src/hotspot/share/code/nmethod.cpp#L3445 is true.

For example, -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=show-comment:off,show-block-comment:off is specified

mhaessig · 2025-06-23T12:13:58Z

Thank you for addressing my comments and for your explanation!

The changes look good to me, but nonetheless I kicked off some testing on our side and I'll get back to you with the results.

mhaessig

Testing passed. Looks good to me!

kurashige23 · 2025-06-24T12:18:23Z

@mhaessig

Thank you for your review.

I think I need approval from 1 reviewer, could you introduce someone?
If not, I will wait for someone to check this PR.

Thanks.

TobiHartmann · 2025-06-27T10:17:26Z

Don't we already have such error reporting code in Disassembler::load_library?

jdk/src/hotspot/share/compiler/disassembler.cpp

Line 863 in c33c1cf

"PrintAssembly defaults to abstract disassembly.");

kurashige23 and others added 3 commits June 9, 2025 16:37

Improve hsdis load error message

9321f4b

Merge branch 'openjdk:master' into hsdis_load_err_msg

c01de1d

Remove include of log.hpp

45584ba

openjdk bot changed the title ~~JDK-8359120: Improve warning message when fail to load hsdis library~~ 8359120: Improve warning message when fail to load hsdis library Jun 10, 2025

openjdk bot added the rfr Pull request is ready for review label Jun 10, 2025

openjdk bot added the hotspot-compiler [email protected] label Jun 10, 2025

mhaessig suggested changes Jun 19, 2025

View reviewed changes

Fix message and revert lines for Xlog

6ff4f9b

mhaessig approved these changes Jun 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8359120: Improve warning message when fail to load hsdis library #25726

8359120: Improve warning message when fail to load hsdis library #25726

Uh oh!

kurashige23 commented Jun 10, 2025 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Jun 10, 2025

Uh oh!

openjdk bot commented Jun 10, 2025

Uh oh!

openjdk bot commented Jun 10, 2025

Uh oh!

mlbridge bot commented Jun 10, 2025 •

edited

Loading

Uh oh!

mhaessig left a comment

Uh oh!

mhaessig Jun 19, 2025

Uh oh!

kurashige23 Jun 23, 2025

Uh oh!

mhaessig Jun 19, 2025

Uh oh!

kurashige23 Jun 23, 2025

Uh oh!

mhaessig Jun 23, 2025

Uh oh!

kurashige23 commented Jun 23, 2025

Uh oh!

mhaessig commented Jun 23, 2025

Uh oh!

mhaessig left a comment

Uh oh!

kurashige23 commented Jun 24, 2025

Uh oh!

TobiHartmann commented Jun 27, 2025

Uh oh!

Uh oh!

8359120: Improve warning message when fail to load hsdis library #25726

Are you sure you want to change the base?

8359120: Improve warning message when fail to load hsdis library #25726

Uh oh!

Conversation

kurashige23 commented Jun 10, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jun 10, 2025

Uh oh!

openjdk bot commented Jun 10, 2025

Uh oh!

openjdk bot commented Jun 10, 2025

Uh oh!

mlbridge bot commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

mhaessig left a comment

Choose a reason for hiding this comment

Uh oh!

mhaessig Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

kurashige23 Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

mhaessig Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

kurashige23 Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

mhaessig Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

kurashige23 commented Jun 23, 2025

Uh oh!

mhaessig commented Jun 23, 2025

Uh oh!

mhaessig left a comment

Choose a reason for hiding this comment

Uh oh!

kurashige23 commented Jun 24, 2025

Uh oh!

TobiHartmann commented Jun 27, 2025

Uh oh!

Uh oh!

kurashige23 commented Jun 10, 2025 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 10, 2025 •

edited

Loading