JDK-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers #14240

dafedafe · 2023-05-31T07:06:27Z

Problem

The AbortVMOnException flag prints the stack trace instead of aborting when the code gets compiled with C1.
This only happens when the exception gets unwound, all methods in the call stack are compiled with C1 and there are no exception handlers for that exception.
This happens here (for x86. Other platform's implementations are similar):

jdk/src/hotspot/cpu/x86/c1_Runtime1_x86.cpp

Lines 1209 to 1215 in 4d47069

    
           case unwind_exception_id: 
        
             { __ set_info("unwind_exception", dont_gc_arguments); 
        
               // note: no stubframe since we are about to leave the current 
        
               //       activation and we are calling a leaf VM function only. 
        
               generate_unwind_exception(sasm); 
        
             } 
        
             break;

Solution

To catch this situation, if AbortVMOnException is set, a call to check if there is a match is added when unwinding the exception. This is done in the void Runtime1::generate_unwind_exception which is the sole call of the unwind_exception_id case.

On a few architectures a move instruction is needed (e.g. __ mov(rscratch1, exception_oop)) to copy the exception oop into a different register that doesn't conflict with the argument of the C calling convention. This is checked with asserts in most architectures but not for aarch64 (new enhancement issue: JDK-8310020).

A regression test is added as well.

Testing

This fix includes changes for x86_32/64 and aarch64, which I could test thoroughly but also for arm, ppc, riscv, and s390 for which I would need some help with 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-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers (Bug - P4)

Reviewers

Tobias Hartmann (@TobiHartmann - Reviewer) ⚠️ Review applies to d899bb83
Amit Kumar (@offamitkumar - Committer) ⚠️ Review applies to 8baa842e
Fei Yang (@RealFYang - Reviewer)
Martin Doerr (@TheRealMDoerr - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14240

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

Using diff file

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

Webrev

Link to Webrev Comment

… -XX:TieredStopAtLevel=1,2,3

…Exception set

…k to JTReg test

This reverts commit 9a3feac.

bridgekeeper · 2023-05-31T07:07:36Z

👋 Welcome back dafedafe! 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 · 2023-05-31T07:09:03Z

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

hotspot

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.

…nstruction

…oid using Handle

…rove test

mlbridge · 2023-06-14T08:38:33Z

Webrevs

TobiHartmann

Looks good to me!

openjdk · 2023-06-14T11:28:23Z

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

8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers

Reviewed-by: thartmann, amitkumar, fyang, mdoerr

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

658c337: 8309296: jdk/jfr/event/runtime/TestAgentEvent.java fails due to "missing" dynamic JavaAgent
70e1c92: 8310297: assert(static_cast(result) == thing) with ctw
574c6f0: 8310376: Extend SetupTarget macro with DIR parameter
7ea22cd: 8310369: UTIL_ARG_WITH fails when arg is disabled
01623f6: 8310133: Effectivelly final condition not enforced in guards for binding variables from the same case
a15db1a: 8302865: Illegal bytecode for break from if with instanceof pattern matching condition
67fbd87: 8310143: RandomCommandsTest fails due to unexpected VM exit code after JDK-8282797
9eed049: 8310321: make JDKOPT_CHECK_CODESIGN_PARAMS more verbose
3faba07: 8307145: windowsaccessbridge.dll erroneously includes private methods in its C API
886ac1c: 8308606: C2 SuperWord: remove alignment checks when not required
... and 129 more: https://git.openjdk.org/jdk/compare/4d66d977450e083214da3dba6ad4ed851c6c1cb4...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.

dafedafe · 2023-06-15T09:03:54Z

This change includes changes in c1_Runtime for platforms (arm32, ppc, riscv, and s390) that I unfortunately couldn't test.
@RealFYang could you or someone you know please run some tests (e.g. tier1-3) for RISCV to make sure my change doesn't break anything? @bulasevich could I ask you the same for ARM32 and @TheRealMDoerr for PPC please? Thank you very much!

dafedafe · 2023-06-15T09:20:07Z

@offamitkumar I noticed you have a few s390x commits. Could I ask you to run a few tests for s390? Thank you very much.

offamitkumar · 2023-06-15T09:30:01Z

@dafedafe s390x tier1 test looks good. I have gone through the changes as well.

LGTM.

offamitkumar · 2023-06-15T10:13:29Z

Hi @dafedafe, On Z15 machine, TestAbortVmOnException.java is failing with below error:

----------System.err:(36/2119)----------                                                                                                                    
 stdout: [#                                                                                                                                                 
# A fatal error has been detected by the Java Runtime Environment:                                                                                          
#                                                                                                                                                           
#  Internal Error (/home/amit/jdk/src/hotspot/share/oops/compressedOops.inline.hpp:138), pid=2449542, tid=2449543                                          
#  assert(!is_null(v)) failed: narrow klass value can never be zero
#
# JRE version: OpenJDK Runtime Environment (22.0) (fastdebug build 22-internal-adhoc.amit.jdk)                                                             
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 22-internal-adhoc.amit.jdk, compiled mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, l$nux-s390x)
# Problematic frame:
# V  [libjvm.so+0x4bf1bc]  Runtime1::check_abort_on_vm_exception(oopDesc*)+0x83c                                                                           
#
# Core dump will be written. Default location: Core dumps may be processed with "/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h %e" (or dumping to $home/amit/jdk/build/linux-s390x-server-fastdebug/test-support/jtreg_test_hotspot_jtreg_runtime_ErrorHandling_TestAbortVmOnException_java/scratch/0/core.244$542)
#
# An error report file with more information is saved as:
# /home/amit/jdk/build/linux-s390x-server-fastdebug/test-support/jtreg_test_hotspot_jtreg_runtime_ErrorHandling_TestAbortVmOnException_java/scratch/0/hs_er$_pid2449542.log
[1.053s][warning][os] Loading hsdis library failed
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#
];
 stderr: []
 exitValue = 134

java.lang.RuntimeException: 'fatal error: Saw MyAbortException, aborting' missing from stdout/stderr                                                       
        at jdk.test.lib.process.OutputAnalyzer.shouldContain(OutputAnalyzer.java:221)                                                                      
        at TestAbortVmOnException.parseOutput(TestAbortVmOnException.java:76)
        at TestAbortVmOnException.main(TestAbortVmOnException.java:50)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)                                           
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)                                                  
        at java.base/java.lang.Thread.run(Thread.java:1583)

JavaTest Message: Test threw exception: java.lang.RuntimeException
JavaTest Message: shutting down test

result: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: 'fatal error: Saw MyAbortException, aborting' missing from stdout/std$rr

@RealLucy you may want to look at it.

src/hotspot/cpu/ppc/c1_Runtime1_ppc.cpp

dafedafe · 2023-06-15T11:20:44Z

Hi @dafedafe, On Z15 machine, TestAbortVmOnException.java is failing with below error:

----------System.err:(36/2119)----------                                                                                                                    
 stdout: [#                                                                                                                                                 
# A fatal error has been detected by the Java Runtime Environment:                                                                                          
#                                                                                                                                                           
#  Internal Error (/home/amit/jdk/src/hotspot/share/oops/compressedOops.inline.hpp:138), pid=2449542, tid=2449543                                          
#  assert(!is_null(v)) failed: narrow klass value can never be zero

@offamitkumar I think I used the wrong register for the exception oop. It should be fixed now. Could you please run the tests again? Thanks a lot.

offamitkumar

@offamitkumar I think I used the wrong register for the exception oop. It should be fixed now.

Thanks, for fixing it. Testing done for tier1-test with fastdebug, slowdebug on Z15 machine. No new failure appeared.

src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp

bulasevich · 2023-06-16T20:14:30Z

@bulasevich could I ask you the same for ARM32

Hi @dafedafe, ARM32 tier1 tests are good. The new test works Ok.

dafedafe · 2023-06-20T07:16:54Z

@offamitkumar I think I used the wrong register for the exception oop. It should be fixed now.

Thanks, for fixing it. Testing done for tier1-test with fastdebug, slowdebug on Z15 machine. No new failure appeared.

@offamitkumar thanks a lot for testing!

dafedafe · 2023-06-20T07:17:47Z

@bulasevich could I ask you the same for ARM32

Hi @dafedafe, ARM32 tier1 tests are good. The new test works Ok.

@bulasevich thanks a lot for testing!

TheRealMDoerr

PPC64 part looks good. Saving and restoring all registers is a bit of an overkill, but it's only a diagnostic feature. So, ok.

dafedafe · 2023-06-21T12:24:22Z

@RealFYang @offamitkumar @TobiHartmann @TheRealMDoerr thanks a lot for your reviews!

dafedafe · 2023-06-21T12:25:32Z

/integrate

openjdk · 2023-06-21T12:27:05Z

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

658c337: 8309296: jdk/jfr/event/runtime/TestAgentEvent.java fails due to "missing" dynamic JavaAgent
70e1c92: 8310297: assert(static_cast(result) == thing) with ctw
574c6f0: 8310376: Extend SetupTarget macro with DIR parameter
7ea22cd: 8310369: UTIL_ARG_WITH fails when arg is disabled
01623f6: 8310133: Effectivelly final condition not enforced in guards for binding variables from the same case
a15db1a: 8302865: Illegal bytecode for break from if with instanceof pattern matching condition
67fbd87: 8310143: RandomCommandsTest fails due to unexpected VM exit code after JDK-8282797
9eed049: 8310321: make JDKOPT_CHECK_CODESIGN_PARAMS more verbose
3faba07: 8307145: windowsaccessbridge.dll erroneously includes private methods in its C API
886ac1c: 8308606: C2 SuperWord: remove alignment checks when not required
... and 129 more: https://git.openjdk.org/jdk/compare/4d66d977450e083214da3dba6ad4ed851c6c1cb4...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-06-21T12:27:15Z

@dafedafe Pushed as commit 826dcb5.

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

dafedafe added 14 commits May 15, 2023 08:21

JDK-8264899: add check for abort as first asm call in C1 if AbortVMOn…

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

4470d1e

…Exception set

JDK-8264899: rename method and add JTReg test

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

ef3a0e6

JDK-8264899: add call to other platform macroassembler

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

b8804f9

JDK-8264899: add UnlockDiagnosticVMOptions and remove exit value chec…

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

93fa7c2

…k to JTReg test

Merge branch 'master' into JDK-8264899

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

a640ea1

JDK-8264899: remove extra whitespace

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

5a6546b

JDK-8264899: don't use a handle

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

9a3feac

Revert "JDK-8264899: don't use a handle"

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

a8138b0

This reverts commit 9a3feac.

JDK-8264899: use handle and reorder runtime1 stubs

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

5226c3e

JDK-8264899: fix stubframe arguments

Verified

This commit was signed with the committer’s verified signature.

robjtede Rob Ede

GPG key ID: 97C636207D3EF933

Verified
Learn about vigilant mode

b763a00

JDK-8264899: fix save_live_registers arguments for x86

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

42e2bb0

JDK-8264899: update copyright year

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

1a0184b

JDK-8264899: remove does_not_return argument

239c233

openjdk bot added the hotspot label May 31, 2023

dafedafe added 6 commits June 5, 2023 11:00

JDK-8264899: explicit enter/leave instead of StubFrame to avoid ret i…

ac20419

…nstruction

Merge branch 'master' into JDK-8264899

2feb407

JDK-8264899: use JRT_LEAF for check_abort_on_vm_exception call and av…

2a2f52d

…oid using Handle

JDK-8264899: use call_VM_leaf for check_abort_on_vm_exception and imp…

ba4adf3

…rove test

Merge branch 'master' into JDK-8264899

c634515

JDK-8264899: move AbortVMOnException to generate_unwind_exception

375c366

dafedafe added 2 commits June 14, 2023 08:33

JDK-8264899: fix PPC macroassembler

73bce2c

JDK-8264899: fix RISC-V macroassembler

3a02c87

dafedafe marked this pull request as ready for review June 14, 2023 08:32

openjdk bot added the rfr label Jun 14, 2023

dafedafe added 2 commits June 14, 2023 11:08

JDK-8264899: fix test source style

75d662b

JDK-8264899: remove stub reordering

d899bb8

TobiHartmann approved these changes Jun 14, 2023

View reviewed changes

openjdk bot added the ready label Jun 14, 2023

TheRealMDoerr reviewed Jun 15, 2023

View reviewed changes

src/hotspot/cpu/ppc/c1_Runtime1_ppc.cpp Outdated Show resolved Hide resolved

dafedafe added 2 commits June 15, 2023 12:35

JDK-8310020: remove unneeded mr ppc instruction

cbe0111

JDK-8264899: fix exception oop register for s390

8baa842

offamitkumar approved these changes Jun 15, 2023

View reviewed changes

RealFYang suggested changes Jun 16, 2023

View reviewed changes

src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp Show resolved Hide resolved

JDK-8264899: avoid copying argument register for riscv

ea110a9

RealFYang approved these changes Jun 20, 2023

View reviewed changes

dafedafe requested a review from TheRealMDoerr June 21, 2023 06:56

TheRealMDoerr approved these changes Jun 21, 2023

View reviewed changes

openjdk bot added the integrated label Jun 21, 2023

openjdk bot closed this Jun 21, 2023

openjdk bot removed ready rfr labels Jun 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers #14240

JDK-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers #14240

dafedafe commented May 31, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented May 31, 2023

openjdk bot commented May 31, 2023

mlbridge bot commented Jun 14, 2023 •

edited

Loading

TobiHartmann left a comment

openjdk bot commented Jun 14, 2023 •

edited

Loading

dafedafe commented Jun 15, 2023

dafedafe commented Jun 15, 2023

offamitkumar commented Jun 15, 2023

offamitkumar commented Jun 15, 2023

dafedafe commented Jun 15, 2023

offamitkumar left a comment

bulasevich commented Jun 16, 2023

dafedafe commented Jun 20, 2023

dafedafe commented Jun 20, 2023

TheRealMDoerr left a comment •

edited

Loading

dafedafe commented Jun 21, 2023

dafedafe commented Jun 21, 2023

openjdk bot commented Jun 21, 2023

openjdk bot commented Jun 21, 2023

	case unwind_exception_id:
	{ __ set_info("unwind_exception", dont_gc_arguments);
	// note: no stubframe since we are about to leave the current
	// activation and we are calling a leaf VM function only.
	generate_unwind_exception(sasm);
	}
	break;

JDK-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers #14240

JDK-8264899: C1: -XX:AbortVMOnException does not work if all methods in the call stack are compiled with C1 and there are no exception handlers #14240

Conversation

dafedafe commented May 31, 2023 • edited by openjdk bot Loading

Problem

Solution

Testing

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented May 31, 2023

openjdk bot commented May 31, 2023

mlbridge bot commented Jun 14, 2023 • edited Loading

Webrevs

TobiHartmann left a comment

Choose a reason for hiding this comment

openjdk bot commented Jun 14, 2023 • edited Loading

dafedafe commented Jun 15, 2023

dafedafe commented Jun 15, 2023

offamitkumar commented Jun 15, 2023

offamitkumar commented Jun 15, 2023

dafedafe commented Jun 15, 2023

offamitkumar left a comment

Choose a reason for hiding this comment

bulasevich commented Jun 16, 2023

dafedafe commented Jun 20, 2023

dafedafe commented Jun 20, 2023

TheRealMDoerr left a comment • edited Loading

Choose a reason for hiding this comment

dafedafe commented Jun 21, 2023

dafedafe commented Jun 21, 2023

openjdk bot commented Jun 21, 2023

openjdk bot commented Jun 21, 2023

dafedafe commented May 31, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 14, 2023 •

edited

Loading

openjdk bot commented Jun 14, 2023 •

edited

Loading

TheRealMDoerr left a comment •

edited

Loading