Skip to content

Conversation

benoitmaillard
Copy link
Contributor

@benoitmaillard benoitmaillard commented Aug 20, 2025

This is a draft PR meant for discussion.

Issue summary

The bug is reproducible with the following code

java -XX:CompileCommand="compileonly,*A::test" -Xcomp -XX:+IgnoreUnrecognizedVMOptions -XX:+StressReflectiveCode TestSubTypeOfAbstractClass.java
public class TestSubTypeOfAbstractClass {

    abstract class A {
        public static A get_null() {
            return null;
        }

        public static boolean test() {
            // NullCheck -> CastPP with type A:NotNull
            // But A is abstract with no subclass, hence this type is impossible
            return get_null() instanceof A;
        }
    }

    public static void main(String[] args) {
        for (int i = 0; i < 10_000; i++ ) {
            A.test();
        }
    }
}

The type compiler/types/TestSubTypeOfAbstractClass$A originates from the TypeFunc of the CallStaticJavaNode.
During parsing of instanceof, in GraphKit::gen_instanceof, we generate two branches for the LHS (one for the case where it is null, one for the other case). In the not null
branch, we end up with a CastPPNode that gets assigned the impossible type TestSubTypeOfAbstractClass$A:NotNull.

This then becomes a problem in SubTypeCheckNode::verify. There, we compare the result of calling Value on the SubTypeCheckNode and performing the sub type check with
a pointer comparison. For Value, we end up in SubTypeCheckNode::sub. There, we detect that the
class on the RHS does not have any subclass (we add a compile dependency for that) and return TypeInt::CC_GT (which is false).
However, when doing the pointer comparison we compute the class of the LHS with LoadKlass, and then compare it to the class on the
RHS with CmpPNode. This constant folds to TypeInt::CC_EQ (true) as the type of the LHS is TestSubTypeOfAbstractClass$A:NotNull.

These two results contradict each other, which results in hitting the assert. However they are both correct in a way, as this path
will never be executed anyway.

A previous attempt at fixing this issue was made, but was later backed out because it caused a regression.


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-8323727: [REDO] C2 compilation fails with assert(verify(phase)) failed: missing Value() optimization (Bug - P3)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26861

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

Using diff file

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

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 20, 2025

👋 Welcome back bmaillard! 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
Copy link

openjdk bot commented Aug 20, 2025

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

@openjdk
Copy link

openjdk bot commented Aug 20, 2025

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

@benoitmaillard benoitmaillard changed the title NOTES: hint to where the issue arises 8323727 Aug 20, 2025
@openjdk openjdk bot changed the title 8323727 8323727: [REDO] C2 compilation fails with assert(verify(phase)) failed: missing Value() optimization Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant