Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[23][Preview][Primitive type patterns] VerifyError with instanceof record patterns with conversion from double to Long #3129

Closed
jarthana opened this issue Oct 21, 2024 · 5 comments · Fixed by #3342
Assignees
Milestone

Comments

@jarthana
Copy link
Member

Testcase:

record Record(Long i) {}
public class X  {
    public static double convert(Record r) {
    	return r instanceof Record(double d) ? d : 0;
    }
    public static void main(String[] args) {}
}

Exception thrown when run:

Caused by: java.lang.VerifyError: Inconsistent stackmap frames at branch target 35
Exception Details:
  Location:
    X.convert(LRecord;)D @15: ifnull
  Reason:
    Current frame's stack size doesn't match stackmap.
  Current Frame:
    bci: @15
    flags: { }
    locals: { 'Record' }
    stack: { 'java/lang/Long', 'java/lang/Long' }
  Stackmap Frame:
    bci: @35
    flags: { }
    locals: { 'Record' }
    stack: { }

Note: It's also cool to see the compiler error about "Internal inconsistency: Inappropriate operand stack size encountered during translation". After some more testing, may be we can make that an error instead of warning - @srikanth-sankaran ?

@srikanth-sankaran
Copy link
Contributor

Byte code generated:

  public static double convert(Record);
    descriptor: (LRecord;)D
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Code:
      stack=4, locals=3, args_size=1
         0: aload_0                                
         1: instanceof    #16                 // class Record
         4: ifeq          35                        // <---------------  reach branch target with stackDepth == 0 
         7: aload_0
         8: checkcast     #16                 // class Record
        11: invokevirtual #18                 // Method Record.i:()Ljava/lang/Long;
        14: dup
        15: ifnull        35                     //  <---------------  reach branch target with stackDepth == 1
        18: iconst_1
        19: ifne          26
        22: pop
        23: goto          35
        26: invokevirtual #22                 // Method java/lang/Long.longValue:()J
        29: l2d
        30: dstore_1
        31: dload_1
        32: goto          36
        35: dconst_0
        36: dreturn
        37: new           #28                 // class java/lang/MatchException
        40: dup_x1
        41: swap
        42: dup
        43: invokevirtual #30                 // Method java/lang/Throwable.toString:()Ljava/lang/String;
        46: swap
        47: invokespecial #36                 // Method java/lang/MatchException."<init>":(Ljava/lang/String;Ljava/lang/Throwable;)V
        50: athrow

It seems we can reach bci with stack depth being 0 through one arc and stack depth being 1 through another arc.

@srikanth-sankaran
Copy link
Contributor

See @bci 4 and @bci 15 and @bci 35 in the assembly dump above

@srikanth-sankaran srikanth-sankaran changed the title VerifyError with instanceof record patterns with conversion from double to Long [Primitive type patterns] VerifyError with instanceof record patterns with conversion from double to Long Oct 22, 2024
@srikanth-sankaran srikanth-sankaran added this to the 4.34 milestone Oct 22, 2024
@srikanth-sankaran
Copy link
Contributor

@stephan-herrmann - thanks for taking a look

@srikanth-sankaran
Copy link
Contributor

Note: It's also cool to see the compiler error about "Internal inconsistency: Inappropriate operand stack size encountered during translation". After some more testing, may be we can make that an error instead of warning - @srikanth-sankaran ?

It was the plan originally that we would as we generate code call out verification failures - but that is a project whose time has to come :)

We can consider the warning -> error change at some point. yes.

@srikanth-sankaran srikanth-sankaran changed the title [Primitive type patterns] VerifyError with instanceof record patterns with conversion from double to Long [23][Preview][Primitive type patterns] VerifyError with instanceof record patterns with conversion from double to Long Oct 22, 2024
@srikanth-sankaran
Copy link
Contributor

This is same as issue #3181 except for the indirection through the type variable. Which is worth capturing as a separate test.

The codeStream.instance_of(scope.getJavaLangObject()); fix outlined in #3301 also solves this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants