JDK-8272570: C2: crash in PhaseCFG::global_code_motion #5140

casparcwang · 2021-08-17T11:28:08Z

In PhaseCFG::global_code_motion, it allocate recalc_pressure_nodes array. condition is at least one block has more than 10 nodes:

  if (OptoRegScheduling) {
    for (uint i = 0; i < number_of_blocks(); i++) {
      Block* block = get_block(i);
      if (block->number_of_nodes() > 10) {
        block_size_threshold_ok = true;
        break;
      }
    }
  }

In PhaseCFG::select, it uses recalc_pressure_nodes array, if block's node exceed 10:

bool block_size_threshold_ok = (block->number_of_nodes() > 10) ? true : false;

In this case, block#4's node size is 10 at GCM begin, but increased to 11 later. PhaseCFG::select use null recalc_pressure_nodes array, this leads to crash.

# --- schedule_local B4, before: ---
#   31: Region              31    6
#   22: MergeMem          NULL    0   17    0   23   25 NULL NULL
#   25: storeImmB            3   17   24 NULL
#   23: storeImmB            3   17   24 NULL
#    3: MachProj             4
#    4: CallLeafDirect      31    0   17   18    0   19    0 NULL
#    5: MachProj             4
#   19: loadConD          NULL   20 NULL NULL
#   20: MachConstantBaseNode   11 NULL NULL NULL
#    2: Ret                  3   21   22   18   26 NULL NULL NULL
#
#   ready cnt: -2  latency:  0    31: Region
#   ready cnt:  2  latency:  0    22: MergeMem
#   ready cnt:  1  latency:  0    25: storeImmB
#   ready cnt:  1  latency:  0    23: storeImmB
#   ready cnt:  1  latency:  0     3: MachProj
#   ready cnt:  1  latency:  0     4: CallLeafDirect
#   ready cnt:  1  latency:  0     5: MachProj
#   ready cnt:  1  latency:  0    19: loadConD
#   ready cnt:  0  latency:  0    20: MachConstantBaseNode
#   ready cnt:  0  latency:  0     2: Ret
#   ready list: 20
#    select 20: MachConstantBaseNode, latency:0 20  MachConstantBaseNode  ===  11  [[ 19 ]] 
#   ready list: 19
#    select 19: loadConD, latency:0 19  loadConD  === _  20  [[ 4 ]] dblcon:1.000000
#   ready list: 4
#    select 4: CallLeafDirect, latency:0 4  CallLeafDirect  ===  31  0  17  18  0  19  0  [[ 5  3 ]] dtan # double/half ( double, half ) !jvms: SDD0::foo @ bci:23 (line 10)
#   ready list: 23 25
select node_size # B4, 11
#   31: Region              31    6
#   20: MachConstantBaseNode   11 NULL NULL NULL
#   19: loadConD          NULL   20 NULL NULL
#    4: CallLeafDirect      31    0   17   18    0   19    0 NULL
#    5: MachProj             4
#    3: MachProj             4
#   35: MachProj             4
#    5: MachProj             4
#   19: loadConD          NULL   20 NULL NULL
#   20: MachConstantBaseNode   11 NULL NULL NULL
#    2: Ret                  3   21   22   18   26 NULL NULL NULL
#
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f14bf3ccf81, pid=31936, tid=31949

Progress

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

Issue

JDK-8272570: C2: crash in PhaseCFG::global_code_motion

Reviewers

Tobias Hartmann (@TobiHartmann - Reviewer)
Rickard Bäckman (@rickard - Reviewer) ⚠️ Review applies to 57a88f5

Contributors

Hui Shi <hshi@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5140/head:pull/5140
$ git checkout pull/5140

Update a local copy of the PR:
$ git checkout pull/5140
$ git pull https://git.openjdk.java.net/jdk pull/5140/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5140

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5140.diff

bridgekeeper · 2021-08-17T11:29:27Z

👋 Welcome back casparcwang! 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 · 2021-08-17T11:31:30Z

@casparcwang 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 · 2021-08-17T11:45:47Z

Webrevs

casparcwang · 2021-08-17T13:01:02Z

/contributor add hshi

openjdk · 2021-08-17T13:01:45Z

@casparcwang
Contributor Hui Shi <hshi@openjdk.org> successfully added.

dean-long · 2021-08-17T23:34:59Z

@casparcwang @hshi same question as the other bug: does the test assert in a debug build?

TobiHartmann

So this code assumes that the number of nodes per block does not change. Where did we add a new node?

TobiHartmann · 2021-08-18T08:08:10Z

test/hotspot/jtreg/compiler/c2/TestGCMRecalcPressureNodes.java

+ * @summary crash in PhaseCFG::global_code_motion
+ * @requires vm.compiler2.enabled
+ *
+ * @run main/othervm TestGCMRecalcPressureNodes


Test can be executed in agent VM mode.

no, the test directly crashes with command java TestGCMRecalcPressureNodes. I will remove othervm

casparcwang · 2021-08-19T03:45:42Z

@casparcwang @hshi same question as the other bug: does the test assert in a debug build?

The test assert in debug build. I have upload the fast debug crash log in JBS.

casparcwang · 2021-08-19T04:12:51Z

So this code assumes that the number of nodes per block does not change. Where did we add a new node?

Thank you for your review.

schedule_local will call sched_call, and it will create MachProjNode. That's where node 35: MachProj comes from.

#0  Node::Init (this=0x7fff5805c910, req=1) at /data/openjdk/jdk_dev/src/hotspot/share/opto/node.cpp:309
#1  0x00007ffff62206a9 in Node::Node (this=0x7fff5805c910, n0=0x7fff58059ef0) at /data/openjdk/jdk_dev/src/hotspot/share/opto/node.cpp:356
#2  0x00007ffff56c7703 in ProjNode::ProjNode (this=0x7fff5805c910, src=0x7fff58059ef0, con=8, io_use=false) at /data/openjdk/jdk_dev/src/hotspot/share/opto/multnode.hpp:68
#3  0x00007ffff56c77b6 in MachProjNode::MachProjNode (this=0x7fff5805c910, multi=0x7fff58059ef0, con=8, out=..., ideal_reg=999) at /data/openjdk/jdk_dev/src/hotspot/share/opto/machnode.hpp:739
#4  0x00007ffff605e003 in PhaseCFG::sched_call (this=0x7fff981242d0, block=0x7fff5805bc68, node_cnt=6, worklist=..., ready_cnt=..., mcall=0x7fff58059ef0, next_call=...) at /data/openjdk/jdk_dev/src/hotspot/share/opto/lcm.cpp:863
#5  0x00007ffff605f258 in PhaseCFG::schedule_local (this=0x7fff981242d0, block=0x7fff5805bc68, ready_cnt=..., next_call=..., recalc_pressure_nodes=0x0) at /data/openjdk/jdk_dev/src/hotspot/share/opto/lcm.cpp:1148
#6  0x00007ffff5cf7951 in PhaseCFG::global_code_motion (this=0x7fff981242d0) at /data/openjdk/jdk_dev/src/hotspot/share/opto/gcm.cpp:1601
#7  0x00007ffff5cf7c0c in PhaseCFG::do_global_code_motion (this=0x7fff981242d0) at /data/openjdk/jdk_dev/src/hotspot/share/opto/gcm.cpp:1642
#8  0x00007ffff5a83eac in Compile::Code_Gen (this=0x7fff98126cb0) at /data/openjdk/jdk_dev/src/hotspot/share/opto/compile.cpp:2721
#9  0x00007ffff5a7b0ac in Compile::Compile (this=0x7fff98126cb0, ci_env=0x7fff98127a20, target=0x7fff580bc9f8, osr_bci=-1, subsume_loads=true, do_escape_analysis=true, eliminate_boxing=true, do_locks_coarsening=true, install_code=true, 
    directive=0x7ffff01f1b80) at /data/openjdk/jdk_dev/src/hotspot/share/opto/compile.cpp:829
#10 0x00007ffff5966b80 in C2Compiler::compile_method (this=0x7ffff02f0910, env=0x7fff98127a20, target=0x7fff580bc9f8, entry_bci=-1, install_code=true, directive=0x7ffff01f1b80) at /data/openjdk/jdk_dev/src/hotspot/share/opto/c2compiler.cpp:107
#11 0x00007ffff5a9831a in CompileBroker::invoke_compiler_on_method (task=0x7ffff04d7c00) at /data/openjdk/jdk_dev/src/hotspot/share/compiler/compileBroker.cpp:2290
#12 0x00007ffff5a96ead in CompileBroker::compiler_thread_loop () at /data/openjdk/jdk_dev/src/hotspot/share/compiler/compileBroker.cpp:1965
#13 0x00007ffff5ab77d7 in CompilerThread::thread_entry (thread=0x7ffff02f0f30, __the_thread__=0x7ffff02f0f30) at /data/openjdk/jdk_dev/src/hotspot/share/compiler/compilerThread.cpp:59
#14 0x00007ffff6530f97 in JavaThread::thread_main_inner (this=0x7ffff02f0f30) at /data/openjdk/jdk_dev/src/hotspot/share/runtime/thread.cpp:1269
#15 0x00007ffff6530e2d in JavaThread::run (this=0x7ffff02f0f30) at /data/openjdk/jdk_dev/src/hotspot/share/runtime/thread.cpp:1252
#16 0x00007ffff652e6d2 in Thread::call_run (this=0x7ffff02f0f30) at /data/openjdk/jdk_dev/src/hotspot/share/runtime/thread.cpp:360
#17 0x00007ffff6275e17 in thread_native_entry (thread=0x7ffff02f0f30) at /data/openjdk/jdk_dev/src/hotspot/os/linux/os_linux.cpp:720
#18 0x00007ffff79b0e25 in start_thread (arg=0x7fff98128700) at pthread_create.c:308
#19 0x00007ffff74d935d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113

…t c2 crash

TobiHartmann

Thanks for the explanation! The fix looks reasonable to me.

TobiHartmann · 2021-08-20T06:45:05Z

test/hotspot/jtreg/compiler/c2/TestGCMRecalcPressureNodes.java

+ * @summary crash in PhaseCFG::global_code_motion
+ * @requires vm.compiler2.enabled
+ *
+ * @run main TestGCMRecalcPressureNodes


Please move the test to test/hotspot/jtreg/compiler/regalloc/ and the corresponding package.

I have move it to regalloc directory.

test/hotspot/jtreg/compiler/c2/TestGCMRecalcPressureNodes.java

TobiHartmann · 2021-08-20T06:48:22Z

/reviewers 2

openjdk · 2021-08-20T06:48:41Z

⚠️ @casparcwang the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout JDK-8272570
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

openjdk · 2021-08-20T06:48:44Z

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

8272570: C2: crash in PhaseCFG::global_code_motion

Co-authored-by: Hui Shi <hshi@openjdk.org>
Reviewed-by: thartmann, rbackman

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

b17b821: 8272639: jpackaged applications using microphone on mac
0e7288f: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions
6ace805: 8272856: DoubleFlagWithIntegerValue uses G1GC-only flag
c547ead: 8272916: Copyright year was modified unintentionally in jlink.properties and ImagePluginStack.java
aaedac6: 8265261: java/nio/file/Files/InterruptCopy.java fails with java.lang.RuntimeException: Copy was not interrupted
d34f17c: 8270195: Add missing links between methods of JavaFX properties
f608e81: 8264322: Generate CDS archive when creating custom JDK image
f681d65: 8272806: [macOS] "Apple AWT Internal Exception" when input method is changed
6e0328f: 8272725: G1: add documentation on needs_remset_update_t vs bool
2309b7d: 8253178: Replace LinkedList Impl in net.http.FilterFactory
... and 60 more: https://git.openjdk.java.net/jdk/compare/0e3fde6c3c2f5c05777b79ff5eb1188014269b0f...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@TobiHartmann, @rickard) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2021-08-20T06:50:15Z

@TobiHartmann
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

TobiHartmann · 2021-08-20T06:54:39Z

src/hotspot/share/opto/lcm.cpp

  int cand_cnt = 0; // Candidate count
-  bool block_size_threshold_ok = (block->number_of_nodes() > 10) ? true : false;
+  bool block_size_threshold_ok = recalc_pressure_nodes != NULL &&
+                                 (block->number_of_nodes() > 10) ? true : false;


And this really should be:
block_size_threshold_ok = (recalc_pressure_nodes != NULL) && (block->number_of_nodes() > 10);

casparcwang · 2021-08-20T07:12:00Z

Thanks for the explanation! The fix looks reasonable to me.

Thank you for the review and suggestion, I have modify the code according to your suggestion.

TobiHartmann

Looks good.

rickard · 2021-08-20T08:35:31Z

Looks good to me too.

… option(s) found, need to specify /othervm"

casparcwang · 2021-08-23T08:31:55Z

Looks good to me too.

Thank you for your review.

casparcwang · 2021-08-25T01:13:43Z

/integrate

openjdk · 2021-08-25T01:15:07Z

@casparcwang
Your change (at version 57c7d8e) is now ready to be sponsored by a Committer.

DamonFool · 2021-08-25T01:15:14Z

/sponsor

openjdk · 2021-08-25T01:16:57Z

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

b17b821: 8272639: jpackaged applications using microphone on mac
0e7288f: 8267125: AES Galois CounterMode (GCM) interleaved implementation using AVX512 + VAES instructions
6ace805: 8272856: DoubleFlagWithIntegerValue uses G1GC-only flag
c547ead: 8272916: Copyright year was modified unintentionally in jlink.properties and ImagePluginStack.java
aaedac6: 8265261: java/nio/file/Files/InterruptCopy.java fails with java.lang.RuntimeException: Copy was not interrupted
d34f17c: 8270195: Add missing links between methods of JavaFX properties
f608e81: 8264322: Generate CDS archive when creating custom JDK image
f681d65: 8272806: [macOS] "Apple AWT Internal Exception" when input method is changed
6e0328f: 8272725: G1: add documentation on needs_remset_update_t vs bool
2309b7d: 8253178: Replace LinkedList Impl in net.http.FilterFactory
... and 60 more: https://git.openjdk.java.net/jdk/compare/0e3fde6c3c2f5c05777b79ff5eb1188014269b0f...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-08-25T01:17:11Z

@DamonFool @casparcwang Pushed as commit 0f428ca.

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

casparcwang added 2 commits August 17, 2021 19:10

Fix nullptr in lcm

d3537b5

Add test

ad3867f

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Aug 17, 2021

casparcwang changed the title ~~C2: crash in PhaseCFG::global_code_motion~~ JDK-8272570: C2: crash in PhaseCFG::global_code_motion Aug 17, 2021

openjdk bot added the rfr Pull request is ready for review label Aug 17, 2021

casparcwang added 2 commits August 17, 2021 20:27

Change copyright

8c6343b

Rename test name

8fbadf4

TobiHartmann suggested changes Aug 18, 2021

View reviewed changes

Remove othervm to speed up test, and make the loop count larger to le…

64a1176

…t c2 crash

TobiHartmann approved these changes Aug 20, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 20, 2021

openjdk bot removed the ready Pull request is ready to be integrated label Aug 20, 2021

TobiHartmann reviewed Aug 20, 2021

View reviewed changes

make the code clean, and fix the test

57a88f5

TobiHartmann approved these changes Aug 20, 2021

View reviewed changes

rickard approved these changes Aug 20, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 20, 2021

ci failure due to "TEST RESULT: Error. Parse Exception: [-Xbatch]: vm…

57c7d8e

… option(s) found, need to specify /othervm"

TobiHartmann approved these changes Aug 23, 2021

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 25, 2021

openjdk bot closed this Aug 25, 2021

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 sponsor Pull request is ready to be sponsored labels Aug 25, 2021

JDK-8272570: C2: crash in PhaseCFG::global_code_motion #5140

JDK-8272570: C2: crash in PhaseCFG::global_code_motion #5140

Uh oh!

Conversation

casparcwang commented Aug 17, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper bot commented Aug 17, 2021

Uh oh!

openjdk bot commented Aug 17, 2021

Uh oh!

mlbridge bot commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

casparcwang commented Aug 17, 2021

Uh oh!

openjdk bot commented Aug 17, 2021

Uh oh!

dean-long commented Aug 17, 2021

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

TobiHartmann Aug 18, 2021

Choose a reason for hiding this comment

Uh oh!

casparcwang Aug 19, 2021

Choose a reason for hiding this comment

Uh oh!

casparcwang commented Aug 19, 2021

Uh oh!

casparcwang commented Aug 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

TobiHartmann Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

casparcwang Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TobiHartmann commented Aug 20, 2021

Uh oh!

openjdk bot commented Aug 20, 2021

Uh oh!

openjdk bot commented Aug 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Aug 20, 2021

Uh oh!

TobiHartmann Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

casparcwang commented Aug 20, 2021

Uh oh!

TobiHartmann left a comment

Choose a reason for hiding this comment

Uh oh!

rickard commented Aug 20, 2021

Uh oh!

casparcwang commented Aug 23, 2021

Uh oh!

casparcwang commented Aug 25, 2021

Uh oh!

openjdk bot commented Aug 25, 2021

Uh oh!

DamonFool commented Aug 25, 2021

Uh oh!

openjdk bot commented Aug 25, 2021

Uh oh!

openjdk bot commented Aug 25, 2021

casparcwang commented Aug 17, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 17, 2021 •

edited

Loading

casparcwang commented Aug 19, 2021 •

edited

Loading

openjdk bot commented Aug 20, 2021 •

edited

Loading