8293851: hs_err should print more stack in hex dump #10282

shipilev · 2022-09-15T10:44:24Z

Similar to JDK-8217879, the debugging would be easier if we dump more stack in the hex dump. This is especially convenient when debugging Loom and seeing more of the raw stack in the crash log. Reading farther from current SP is generally safe, since stack banging mechanism provides us with accessible stack at all times for normal SPs. In fact, many platforms already read significantly more stack than x86.

We can homogenize the stack printing across all architectures. I believe the stack grows the same way on all supported arches (downwards), so we can read in one direction. Tell me if you still want to make that one selectable.

As the safeguard, JDK-8217994 gives us the facility to read the memory much more safely, so we can dump more stack without elevated risk of secondary crash, in case the stack is not readable.

x86_64 hs_err before:

Top of Stack: (sp=0x00007fdab92650b0)
0x00007fdab92650b0:   00000000ffffffff 00007fdb0c2d5c50
0x00007fdab92650c0:   00007fdab9265460 00007fdb12e4a106
0x00007fdab92650d0:   0000000000000001 00007fdb0c20e980
0x00007fdab92650e0:   0000000000000000 00007fdb0c2ff4c0

x86_64 hs_err after:

Top of Stack: (sp=0x00007f10ccb1d0b0)
0x00007f10ccb1d0b0:   00000000ffffffff 00007f11202d5c50
0x00007f10ccb1d0c0:   00007f10ccb1d460 00007f11266e4106
0x00007f10ccb1d0d0:   0000000000000001 00007f112020e980
0x00007f10ccb1d0e0:   0000000000000000 00007f11202ff4c0
0x00007f10ccb1d0f0:   00007f112020e980 00007f10ccb1d140
0x00007f10ccb1d100:   00007f11202fd4f0 00007f11202ffb80
0x00007f10ccb1d110:   00007f11202ffbc0 00007f11202fff68
0x00007f10ccb1d120:   00000000000003d8 00007f1000000002
0x00007f10ccb1d130:   00007f11202ff4c0 00007f10ccb1d550
0x00007f10ccb1d140:   00007f11202fd4f0 0000000300000007
0x00007f10ccb1d150:   00007f11202d5c50 00007f112020e980
0x00007f10ccb1d160:   00007f10ccb1d740 0000000000000000
0x00007f10ccb1d170:   00007f10a80668c0 00007f11ffffffff
0x00007f10ccb1d180:   00007f109c00d250 00007f1000000000
0x00007f10ccb1d190:   00007f109c00ecd8 00007f10a80404a0
0x00007f10ccb1d1a0:   0000000000000000 000000011d001001
0x00007f10ccb1d1b0:   0000000000000000 00007f11202ffc20
0x00007f10ccb1d1c0:   00007f11202ffbc0 0000000800000000
0x00007f10ccb1d1d0:   0000000100000003 0000000000000000
0x00007f10ccb1d1e0:   00007f11202ffc68 00007f1000000003
0x00007f10ccb1d1f0:   00007f10a803b1d0 0000002000000000
0x00007f10ccb1d200:   000000000000003b 0000004100000000
0x00007f10ccb1d210:   000000aaffffffff 00007f10ccb1d218
0x00007f10ccb1d220:   00000000000001e8 00007f1127d233e6
0x00007f10ccb1d230:   00007f110fee5f80 0000000000000000
0x00007f10ccb1d240:   00007f110fee5f80 00007f110fef2c50
0x00007f10ccb1d250:   00007f10a8040490 00007f10a8040490
0x00007f10ccb1d260:   00007f10a8040498 00007f110fee5f80
0x00007f10ccb1d270:   00007f1120000001 00007f10ccb1d218
0x00007f10ccb1d280:   00007f110fe65f80 0000000000000000
0x00007f10ccb1d290:   00007f110fe66051 00007f110fee5b18
0x00007f10ccb1d2a0:   00007f10a8040338 00007f10a804034c

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-8293851: hs_err should print more stack in hex dump

Reviewers

Andrew Dinn (@adinn - Reviewer)
Stefan Karlsson (@stefank - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10282

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

Using diff file

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

bridgekeeper · 2022-09-15T10:46:20Z

👋 Welcome back shade! 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 · 2022-09-15T10:49:47Z

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

hotspot-runtime

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 · 2022-09-15T10:52:33Z

Webrevs

01: Full - Incremental (a9c35f88)
00: Full (98a27d9c)

adinn

Looks great!

openjdk · 2022-09-15T11:14:44Z

@shipilev This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8293851: hs_err should print more stack in hex dump

Reviewed-by: adinn, stefank

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 28 new commits pushed to the master branch:

f42caef: 8293550: Optionally add get-task-allow entitlement to macos binaries
5feca68: 8293840: RISC-V: Remove cbuf parameter from far_call/far_jump/trampoline_call
39cd163: 8293578: Duplicate ldc generated by javac
7765942: 8290367: Update default value and extend the scope of com.sun.jndi.ldap.object.trustSerialData system property
11e7d53: 8293819: sun/util/logging/PlatformLoggerTest.java failed with "RuntimeException: Retrieved backing PlatformLogger level null is not the expected CONFIG"
141d5f5: 8293767: AWT test TestSinhalaChar.java has old SCCS markings
3beca2d: 8291600: [vectorapi] vector cast op check is not always needed for vector mask cast
9a40b76: 8293842: IPv6-only systems throws UnsupportedOperationException for several socket/TCP options
bb9aa4e: 8293813: ProblemList com/sun/jdi/JdbLastErrorTest.java on windows-x64 in Xcomp mode
4cec141: 8291509: Minor cleanup could be done in sun.security
... and 18 more: https://git.openjdk.org/jdk/compare/7f3250d71c4866a64eb73f52140c669fe90f122f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

stefank

+1

I would consider hiding sizeof(intptr_t) from the call sites. Seems like unnecessary noise to me.

tstuefe

I dislike a bit that the new function lives in os.hpp since it is a rather specific function used in one place only and would better live locally in vmError.cpp.

I guess it needs to be this way if you want to keep the patch small, because we have no platform-independent API for extracting PC and SP from a context (which would be trivial to implement, but I don't know how far you want to go).

src/hotspot/share/runtime/os.hpp

shipilev · 2022-09-15T14:55:15Z

I guess it needs to be this way if you want to keep the patch small, because we have no platform-independent API for extracting PC and SP from a context (which would be trivial to implement, but I don't know how far you want to go).

One of the reasons why print_instruction is separate and called via os_cpu-specific code is that arches select the unit size knowing the instruction size. Granted, we can do the helpers that expose both SP, PC, and insn unit size.

shipilev · 2022-09-15T14:55:46Z

I would consider hiding sizeof(intptr_t) from the call sites. Seems like unnecessary noise to me.

Right. Stack dump is one of those things where we expect word-granularity (almost?) everywhere. Done in new commit.

shipilev · 2022-09-16T10:46:08Z

@tstuefe -- happy with current PR, or?

adinn

Still ok

shipilev · 2022-09-19T07:36:10Z

All right, I am integrating.

/integrate

openjdk · 2022-09-19T07:38:19Z

Going to push as commit cbd0688.
Since your change was applied there have been 43 commits pushed to the master branch:

04d7b7d: 8293503: gc/metaspace/TestMetaspacePerfCounters.java#Epsilon-64 failed assertGreaterThanOrEqual: expected MMM >= NNN
d77c464: 8293891: gc/g1/mixedgc/TestOldGenCollectionUsage.java (still) assumes that GCs take 1ms minimum
d7c1a76: 8293861: G1: Disable preventive GCs by default
43f7f47: 8293499: Provide jmod --compress option
26e08cf: 8293844: C2: Verify Location::{oop,normal} types in PhaseOutput::FillLocArray
357a2cc: 8293937: x86: Drop LP64 conditions from clearly x86_32 code
b1ed40a: 8293466: libjsig should ignore non-modifying sigaction calls
b6ff8fa: 8292073: NMT: remove unused constructor parameter from MallocHeader
cfd44bb: 8293218: serviceability/tmtools/jstat/GcNewTest.java fails with "Error in the percent calculation"
01e7b88: 8290917: x86: Memory-operand arithmetic instructions have too low costs
... and 33 more: https://git.openjdk.org/jdk/compare/7f3250d71c4866a64eb73f52140c669fe90f122f...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-09-19T07:38:34Z

@shipilev Pushed as commit cbd0688.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Fix

98a27d9

openjdk bot added the rfr Pull request is ready for review label Sep 15, 2022

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Sep 15, 2022

adinn approved these changes Sep 15, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 15, 2022

stefank approved these changes Sep 15, 2022

View reviewed changes

tstuefe reviewed Sep 15, 2022

View reviewed changes

src/hotspot/share/runtime/os.hpp Outdated Show resolved Hide resolved

shipilev added 2 commits September 15, 2022 16:36

Drop stack unitsize

8d0ffff

Rename to print_tos

a9c35f8

stefank approved these changes Sep 16, 2022

View reviewed changes

adinn reviewed Sep 16, 2022

View reviewed changes

adinn approved these changes Sep 16, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Sep 19, 2022

openjdk bot closed this Sep 19, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 19, 2022

shipilev deleted the JDK-8293851-hexdump-sp branch October 21, 2022 09:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8293851: hs_err should print more stack in hex dump #10282

8293851: hs_err should print more stack in hex dump #10282

shipilev commented Sep 15, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

mlbridge bot commented Sep 15, 2022 •

edited

Loading

adinn left a comment

openjdk bot commented Sep 15, 2022 •

edited

Loading

stefank left a comment

tstuefe left a comment

shipilev commented Sep 15, 2022

shipilev commented Sep 15, 2022 •

edited

Loading

shipilev commented Sep 16, 2022

adinn left a comment

shipilev commented Sep 19, 2022

openjdk bot commented Sep 19, 2022

openjdk bot commented Sep 19, 2022

8293851: hs_err should print more stack in hex dump #10282

8293851: hs_err should print more stack in hex dump #10282

Conversation

shipilev commented Sep 15, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 15, 2022

openjdk bot commented Sep 15, 2022

mlbridge bot commented Sep 15, 2022 • edited Loading

Webrevs

adinn left a comment

Choose a reason for hiding this comment

openjdk bot commented Sep 15, 2022 • edited Loading

stefank left a comment

Choose a reason for hiding this comment

tstuefe left a comment

Choose a reason for hiding this comment

shipilev commented Sep 15, 2022

shipilev commented Sep 15, 2022 • edited Loading

shipilev commented Sep 16, 2022

adinn left a comment

Choose a reason for hiding this comment

shipilev commented Sep 19, 2022

openjdk bot commented Sep 19, 2022

openjdk bot commented Sep 19, 2022

shipilev commented Sep 15, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Sep 15, 2022 •

edited

Loading

openjdk bot commented Sep 15, 2022 •

edited

Loading

shipilev commented Sep 15, 2022 •

edited

Loading