8255741: Zero: print signal name in unhandled signal handler #1001

shipilev · 2020-11-02T10:15:24Z

At least one test -- runtime/Safepoint/TestAbortVMOnSafepointTimeout.java -- fails on Zero with:

# Internal Error (/home/shade/trunks/jdk/src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp:250), pid=560280, tid=560281
# fatal error: caught unhandled signal 4

The test expects "SIGKILL (sent by kill)" in the test output.

Additional testing:

TestAbortVMOnSafepointTimeout.java now passes on Linux x86_64 Zero.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x64	Linux x86	Windows x64	macOS x64
Build	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)	✔️ (9/9 passed)

Issue

JDK-8255741: Zero: print signal name in unhandled signal handler

Reviewers

Thomas Stuefe (@tstuefe - Reviewer) ⚠️ Review applies to 89de8c2

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1001/head:pull/1001
$ git checkout pull/1001

bridgekeeper · 2020-11-02T10:16:38Z

👋 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 · 2020-11-02T10:19:23Z

@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 · 2020-11-02T10:23:59Z

Webrevs

01: Full - Incremental (3356bd3)
00: Full (89de8c2)

tstuefe

Okay I guess, but note that I am currently rewriting all that coding for https://bugs.openjdk.java.net/browse/JDK-8255711 . Which would, as a side effect, fix this bug too.

(The real bug here is that we invoke fatal() instead of calling VMError::report_and_die(), which means we won't see the real register content from the fault point in the hs-err file).

See current draft: #982

Part of the change will be to heave fatal error handler invocation out of platform dependent up to shared code (and note how I took explicit care to preserve robot and cat on zero :) .

Cheers, Thomas

tstuefe

LGTM

shipilev · 2020-11-02T10:56:15Z

Thanks! I was not aware of #982 -- that would be very good to have. Maybe push this simple patch to ease backporting? Making Zero pass tier1 in 11u would be nice. I suspect #982 would not be backportable?

openjdk · 2020-11-02T10:59:40Z

@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:

8255741: Zero: print signal name in unhandled signal handler

Reviewed-by: stuefe

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 60 new commits pushed to the master branch:

acb5f65: 8211958: Broken links in java.desktop files
bc6085b: 8255578: [JVMCI] be more careful about reflective reads of Class.componentType.
05bcd67: 8255529: Remove unused methods from java.util.zip.ZipFile
d93e3a7: 8255760: Shenandoah: match constants style in ShenandoahMarkTask fallback
3e89f72: 8255237: ZGC: Bulk free garbage pages during relocation set selection
6dac8d2: 8255671: Bidi.reorderVisually has misleading exception messages
2f7d34f: 8255616: Disable AOT and Graal in Oracle OpenJDK
0e19ded: 8255401: Shenandoah: Allow oldval and newval registers to overlap in cmpxchg_oop()
a3aad11: 8255400: Shenandoah: C2 failures after JDK-8255000
1769c48: 8255471: ZGC: Rework root iterators and closures
... and 50 more: https://git.openjdk.java.net/jdk/compare/4b20e460dc19e4e1533eb5d177f3b701ccbb483a...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.

mlbridge · 2020-11-02T12:30:09Z

Mailing list message from Thomas St��fe on hotspot-runtime-dev:

Sure, push away, I approved both fixes. #982 would be backportable too but
require much more work.

Thanks, Thomas

On Mon, Nov 2, 2020 at 12:01 PM Aleksey Shipilev <shade at openjdk.java.net>
wrote:

shipilev · 2020-11-03T07:24:18Z

/integrate

openjdk · 2020-11-03T07:30:06Z

@shipilev Since your change was applied there have been 73 commits pushed to the master branch:

1580574: 8255672: Replace PhaseTransform::eqv by pointer equality check
e7a2d5c: 8252533: Signal handlers should run with synchronous error signals unblocked
6d36b4b: 8255743: Relax SIGFPE match in in runtime/ErrorHandling/SecondaryErrorTest.java
f0eeca9: 8255718: Zero: VM should know it runs in interpreter-only mode
9beb866: 8233561: [TESTBUG] Swing text test bug8014863.java fails on macos
fe4e6b3: 8196089: javax/swing/Action/8133039/bug8133039.java fails
50357d1: 8254723: add diagnostic command to write Linux perf map file
f97ec35: 8255785: X11 libraries should not be required by configure for headless only
184db64: 8255732: OpenJDK fails to build if $A is set to a value with spaces
c774741: 8255695: Some JVMTI calls in the jdwp debug agent are using FUNC_PTR instead of JVMTI_FUNC_PTR
... and 63 more: https://git.openjdk.java.net/jdk/compare/4b20e460dc19e4e1533eb5d177f3b701ccbb483a...master

Your commit was automatically rebased without conflicts.

Pushed as commit aa2862a.

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

Reviewed-by: stuefe

…g to pointer In the cases like: ``` UNSAFE.putLong(address + off1 + 1030, lseed); UNSAFE.putLong(address + 1023, lseed); UNSAFE.putLong(address + off2 + 1001, lseed); ``` Unsafe intrinsifies direct memory access using a long as the base address, generating a `CastX2P` node converting long to pointer in C2. Then we get optoassembly code like: ``` ldr R10, [R15, openjdk#120] # int ! Field: address ldr R11, [R16, openjdk#136] # int ! Field: off1 ldr R12, [R16, openjdk#144] # int ! Field: off2 add R11, R11, R10 mov R11, R11 # long -> ptr add R12, R12, R10 mov R10, R10 # long -> ptr add R11, R11, openjdk#1030 # ptr str R17, [R11] # int add R10, R10, openjdk#1023 # ptr str R17, [R10] # int mov R10, R12 # long -> ptr add R10, R10, openjdk#1001 # ptr str R17, [R10] # int ``` In aarch64, the conversion from long to pointer could be a nop but C2 doesn't know it. On the existing code, we do nothing for `mov dst src` only when `dst` == `src` [1], then we have assembly: ``` ldr x10, [x15,openjdk#120] ldp x11, x12, [x16,openjdk#136] add x11, x11, x10 add x12, x12, x10 add x11, x11, #0x406 str x17, [x11] add x10, x10, #0x3ff str x17, [x10] mov x10, x12 <--- extra register copy add x10, x10, #0x3e9 str x17, [x10] ``` There is still one extra register copy, which we're trying to remove in this patch. This patch folds `CastX2P` into memory operands by introducing `indirectX2P` and `indOffX2P`. We also create a new opclass `iRegPorL2P` to remove extra copies from `CastX2P` in pointer addition. Tier 1~3 passed on aarch64. No obvious change in size of libjvm.so [1] https://github.com/openjdk/jdk/blob/5c612c230b0a852aed5fd36e58b82ebf2e1838af/src/hotspot/cpu/aarch64/aarch64.ad#L7906

8255741: Zero: print signal name in unhandled signal handler

89de8c2

openjdk bot added the rfr Pull request is ready for review label Nov 2, 2020

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Nov 2, 2020

tstuefe reviewed Nov 2, 2020

View reviewed changes

tstuefe approved these changes Nov 2, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 2, 2020

Minor stylistic change

3356bd3

openjdk bot closed this Nov 3, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 3, 2020

shipilev deleted the JDK-8255741-zero-print-signal-name branch November 3, 2020 07:29

openjdk-notifier bot referenced this pull request Nov 3, 2020

8255741: Zero: print signal name in unhandled signal handler

aa2862a

Reviewed-by: stuefe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8255741: Zero: print signal name in unhandled signal handler #1001

8255741: Zero: print signal name in unhandled signal handler #1001

Uh oh!

shipilev commented Nov 2, 2020 •

edited by openjdk bot

Loading

Uh oh!

bridgekeeper bot commented Nov 2, 2020

Uh oh!

openjdk bot commented Nov 2, 2020

Uh oh!

mlbridge bot commented Nov 2, 2020 •

edited

Loading

Uh oh!

tstuefe left a comment

Uh oh!

tstuefe left a comment

Uh oh!

shipilev commented Nov 2, 2020

Uh oh!

openjdk bot commented Nov 2, 2020 •

edited

Loading

Uh oh!

mlbridge bot commented Nov 2, 2020

Uh oh!

shipilev commented Nov 3, 2020

Uh oh!

openjdk bot commented Nov 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

8255741: Zero: print signal name in unhandled signal handler #1001

8255741: Zero: print signal name in unhandled signal handler #1001

Uh oh!

Conversation

shipilev commented Nov 2, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Testing

Issue

Reviewers

Download

Uh oh!

bridgekeeper bot commented Nov 2, 2020

Uh oh!

openjdk bot commented Nov 2, 2020

Uh oh!

mlbridge bot commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

tstuefe left a comment

Choose a reason for hiding this comment

Uh oh!

tstuefe left a comment

Choose a reason for hiding this comment

Uh oh!

shipilev commented Nov 2, 2020

Uh oh!

openjdk bot commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Nov 2, 2020

Uh oh!

shipilev commented Nov 3, 2020

Uh oh!

openjdk bot commented Nov 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

shipilev commented Nov 2, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 2, 2020 •

edited

Loading

openjdk bot commented Nov 2, 2020 •

edited

Loading