8385420: C2: SIGSEGV in compiled code due to missing ctrl

[merykitty]

Hi,

We have a load and its base is a CMoveP, the CMoveP merges 2 not-null values, one of which is a CastPP to not-null of the method parameter, while the other is a not-null constant. As a result, the CMoveP is not-null, and the graph is valid. However, during final_graph_reshaping, we try to remove CastPPNodes from the graph, this requires us to pin all of the depending accesses of that CastPP below the control input of the CastPP that is going away. The logic looks through some kinds of nodes, but it misses CMoveP. As a result, the load from the CastPP is not pinned correctly, and floats up above the null check.

This PR fixes the issue by adding CMove to the cases which require us looking through to look for accesses.

Testing:

• tier1-4,hs-comp-stress

Please take a look and leave your reviews, thanks a lot.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

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

Issue

• JDK-8385420: C2: SIGSEGV in compiled code due to missing ctrl (Bug - P3)

Reviewers

• Vladimir Ivanov (@iwanowww - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 31368

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back qamai! 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]

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

[openjdk]

@merykitty 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.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: hotspot-compiler. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 00: Full (b0904ea0)

[iwanowww]

Looks good.

What could be a validation check here to ensure no other problematic case is omitted? A node without control [1]?

[1]

default: 
   assert(use->in(0) != nullptr, "%s",  NodeClassNames[use->Opcode()]);

[dean-long]

I think keeping "switch" and "case" aligned is the more common HotSpot style. Also a comment explaining why we choose this subset of nodes would help.

[franciscoarturorivera371-cyber]

Hi @franciscoarturorivera371-cyber, thanks for making a comment in an OpenJDK project!

All comments and discussions in the OpenJDK Community must be made available under the OpenJDK Terms of Use. If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please Use "Add GitHub user franciscoarturorivera371-cyber" for the summary.

If you are not an OpenJDK Author, Committer or Reviewer, simply check the box below to accept the OpenJDK Terms of Use for your comments.

• I agree to the OpenJDK Terms of Use for all comments I make in a project in the OpenJDK GitHub organization.

Your comment will be automatically restored once you have accepted the OpenJDK Terms of Use.