Openj9 incorrectly permits duplicate fields

[tajila]

Given the following class

//Note: you cant do this with javac, need to use bytecode generator
class A {
   static int x;
   int x;
}

The RI will fail class loading with the following message:

Exception in thread "main" java.lang.ClassFormatError: Duplicate field name "x" with signature "I" in class file A

However, openj9 will not throw any errors

[DanHeidinga]

@tajila Does this get detected when running with -Xfuture? This duplicates check may be one of the more expensive ones that are only done under -Xfuture

[tajila]

@DanHeidinga This does not get detected with -Xfuture

[DanHeidinga]

Thanks for validating that Tobi. We'll need to look at the spec and see if we're missing a required check.

[DanHeidinga]

@ChengJin01 Can you take a look at this?

[ChengJin01]

@DanHeidinga, with the modified .class file generated from .jcod A.jcod.txt by asmtools.jar (A.jcod was generated by A.class file and the duplicate field int x was inserted to the fields of .jcod after that)

class A {
  0xCAFEBABE;
  0; // minor version
  55; // version
  [] { // Constant Pool
    ; // first element is empty
    Method #3 #12; // #1
    class #13; // #2
    class #14; // #3
    Utf8 "x"; // #4
    Utf8 "I"; // #5
    Utf8 "<init>"; // #6
    Utf8 "()V"; // #7
    Utf8 "Code"; // #8
    Utf8 "LineNumberTable"; // #9
    Utf8 "SourceFile"; // #10
    Utf8 "A.java"; // #11
    NameAndType #6 #7; // #12
    Utf8 "A"; // #13
    Utf8 "java/lang/Object"; // #14
  } // Constant Pool

  0x0021; // access
  #2;// this_cpx
  #3;// super_cpx

  [] { // Interfaces
  } // Interfaces

  [] { // Fields
    {  // field    <----------- static int x;
      0x0008; // access
      #4; // name_index
      #5; // descriptor_index
      [] { // Attributes
      } // Attributes
    }
    ;
    {  // field   <----------- int x; (inserted)
      0x0000; // access
      #4; // name_index
      #5; // descriptor_index
      [] { // Attributes
      } // Attributes
    }
  } // Fields

it shows our build (nightly) ended up with the same behaviour as RI as follows:

$ jdk11_hotspot/bin/java  -version
openjdk version "11.0.7" 2020-04-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.7+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.7+10, mixed mode)

$ ../jdk11_hotspot/bin/java   A
Error: LinkageError occurred while loading main class A
---> java.lang.ClassFormatError: Duplicate field name "x" with signature "I" in class file A

$ jdk11.0.8_7_openj9/bin/java  -version
openjdk version "11.0.8" 2020-07-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.8+7-202006152334)
Eclipse OpenJ9 VM AdoptOpenJDK (build master-3b048e5bc, JRE 11 Linux amd64-64-Bit Compressed References 20200615_660 (JIT enabled, AOT enabled)
OpenJ9   - 3b048e5bc
OMR      - 1db16ee21
JCL      - dfcd997d06 based on jdk-11.0.8+7)

$ ../jdk11.0.8_7_openj9/bin/java   A
Error: LinkageError occurred while loading main class A
---> java.lang.ClassFormatError: JVMCFRE021 duplicate field; class=A, offset=137

So there is nothing wrong in our code in terms of such error being captured.

[DanHeidinga]

@tajila which release did you have the problem with?

[tajila]

I created a test the produces an instance of

class Outer {
  private static final int y;
  private final int y;
  int z;
  public Outer();
}

using:

openjdk version "11.0.7" 2020-04-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.7+10)
Eclipse OpenJ9 VM AdoptOpenJDK (build openj9-0.20.0, JRE 11 Mac OS X amd64-64-Bit Compressed References 20200428_567 (JIT enabled, AOT enabled)
OpenJ9   - 05fa2d361
OMR      - d4365f371
JCL      - 838028fc9d based on jdk-11.0.7+10)

the resut is:

Tobis-MacBook-Pro:TestPrograms tobi$ java -cp classes.jar:asm-7.0-20181027.133552-5.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
test

using:

openjdk version "11.0.4" 2019-07-16
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.4+11)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.4+11, mixed mode)

the result is:

~/JDKs/bootjdk11/Contents/Home/bin/java -cp classes.jar:asm-7.0-20181027.133552-5.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
Exception in thread "main" java.lang.ClassFormatError: Duplicate field name "y" with signature "I" in class file Outer
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1192)
    at DefineAnonClass.defineClass(DefineAnonClass.java:66)
    at Scratch.main(Scratch.java:380)

Note: this is unrelated to anonclasses, I just so happened to be investigating another issue when I came across this.

Is it possible to upload file to github? If so I can post the jar

[tajila]

Here is a link to the jar, https://ibm.box.com/s/uqhaxkila29ni28vpyqr5o38g2e08adx

[ChengJin01]

[1] tried with the suspicious class (Outer.class from Outer.jcod.txt)

class Outer {
  0xCAFEBABE;
  0; // minor version
  55; // version
  [] { // Constant Pool
    ; // first element is empty
    Utf8 "Outer"; // #1
    class #1; // #2
    Utf8 "java/lang/Object"; // #3
    class #3; // #4
    Utf8 "Scratch.java"; // #5
    Utf8 "y"; // #6 <------------------
    Utf8 "I"; // #7
    Utf8 "z"; // #8
    Utf8 "<init>"; // #9
    Utf8 "()V"; // #10
    NameAndType #9 #10; // #11
    Method #4 #11; // #12
    NameAndType #8 #7; // #13
    Field #2 #13; // #14
    Utf8 "this"; // #15
    Utf8 "LOuter;"; // #16
    Utf8 "Code"; // #17
    Utf8 "LineNumberTable"; // #18
    Utf8 "LocalVariableTable"; // #19
    Utf8 "SourceFile"; // #20
    Utf8 "NestMembers"; // #21
    Utf8 "Outer$Inner"; // #22
    class #22; // #23
  } // Constant Pool

  0x0020; // access
  #2;// this_cpx
  #4;// super_cpx

  [] { // Interfaces
  } // Interfaces

  [] { // Fields
    {  // field   <----------   private static final int y
      0x001A; // access
      #6; // name_index
      #7; // descriptor_index
      [] { // Attributes
      } // Attributes
    }
    ;
    {  // field  <------------  private final int y
      0x0012; // access
      #6; // name_index
      #7; // descriptor_index
      [] { // Attributes
      } // Attributes
    }
    ;
    {  // field
      0x0000; // access
      #8; // name_index
      #7; // descriptor_index
      [] { // Attributes
      } // Attributes
    }
  } // Fields

it shows our nightly build ended up with the same ClassFormatError as RI when checking the class file itself as follows:

../jdk11_hotspot/bin/java   Outer
Error: LinkageError occurred while loading main class Outer
---> java.lang.ClassFormatError: Duplicate field name "y" with signature "I" in class file Outer

../jdk11.0.8_7_openj9/bin/java   Outer
Error: LinkageError occurred while loading main class Outer
---> java.lang.ClassFormatError: JVMCFRE021 duplicate field; class=Outer, offset=230

[2] with the jar file & Outer specified on the command line, our nightly build still ended up with the same ClassFormatError as RI:

../jdk11_hotspot/bin/java   -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Outer
Error: LinkageError occurred while loading main class Outer
---> java.lang.ClassFormatError: Duplicate field name "y" with signature "I" in class file Outer

../jdk11.0.8_7_openj9/bin/java  -XX:-ClassRelationshipVerifier -Xshareclasses:none  -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Outer
Error: LinkageError occurred while loading main class Outer
---> java.lang.ClassFormatError: JVMCFRE021 duplicate field; class=Outer, offset=230

[3] with the jar file & Scratch specified on the command line, RI did capture the error while the test passed with our nightly build :

jdk11_hotspot/bin/java   -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
Exception in thread "main" java.lang.ClassFormatError: Duplicate field name "y" with signature "I" in class file Outer
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1192)
    at DefineAnonClass.defineClass(DefineAnonClass.java:66)
    at Scratch.main(Scratch.java:380)

../jdk11.0.8_7_openj9/bin/java   -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
test (passed without ClassFormatError being captured)

../jdk11.0.8_7_openj9/bin/java  -XX:-ClassRelationshipVerifier  -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
test (passed without ClassFormatError being captured)

../jdk11.0.8_7_openj9/bin/java  -XX:-ClassRelationshipVerifier -Xshareclasses:none  -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
test (passed without ClassFormatError being captured)

Based on the investigation above, I suspect there suspicious code didn't get picked up in the static verification when specifying Scratch to be executed.

[ChengJin01]

Specified with -verbose:dynload and it shows that Outer.class was loaded during the test:

../jdk11.0.8_7_openj9/bin/java -verbose:dynload  
-XX:-ClassRelationshipVerifier -Xshareclasses:none  
-cp classes.jar:asm-7.0.jar 
--add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
JImage interface is using jimage library
...
<Loaded Outer>
<  Class size 343; ROM size 336; debug size 0>
<  Read time 0 usec; Load time 2 usec; Translate time 10 usec>
test

If so, the error should be captured in such case.

[tajila]

@ChengJin01 Your investigation has made me realize that this issue only occurs with unsafe classes. It seems that the RI performs verification with unsafe classes whereas we do not.

[tajila]

@DanHeidinga do we need to ensure compliance with the RI in terms of unsafe.defineClass ?

[ChengJin01]

@tajila , I double-checked Scratch.class in your classes.jar file: Scratch_class.txt as follows

  public static void main(java.lang.String[]) throws java.lang.Exception, java.lang.Throwable;
    descriptor: ([Ljava/lang/String;)V
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Exceptions:
      throws java.lang.Exception, java.lang.Throwable
    Code:
      stack=4, locals=6, args_size=1
         0: new           #36                 // class java/io/File
         3: dup
         4: ldc           #38                 // String Outer.class
         6: invokespecial #40                 // Method java/io/File."<init>":(Ljava/lang/String;)V
         9: astore_1
        10: aload_1
        11: invokevirtual #43                 // Method java/io/File.createNewFile:()Z
        14: pop
        15: new           #36                 // class java/io/File
        18: dup
        19: ldc           #47                 // String Outer$Inner.class
        21: invokespecial #40                 // Method java/io/File."<init>":(Ljava/lang/String;)V
        24: astore_2
        25: aload_2
        26: invokevirtual #43                 // Method java/io/File.createNewFile:()Z
        29: pop
        30: new           #49                 // class java/io/FileOutputStream
        33: dup
        34: aload_1
        35: invokespecial #51                 // Method java/io/FileOutputStream."<init>":(Ljava/io/File;)V
        38: astore_3
        39: new           #49                 // class java/io/FileOutputStream
        42: dup
        43: aload_2
        44: invokespecial #51                 // Method java/io/FileOutputStream."<init>":(Ljava/io/File;)V
        47: astore        4
        49: aload_3
        50: invokestatic  #54                 // Method OuterDump.dump:()[B
        53: invokevirtual #60                 // Method java/io/FileOutputStream.write:([B)V
        56: aload         4
        58: invokestatic  #64                 // Method Outer$InnerDump.dump:()[B
        61: invokevirtual #60                 // Method java/io/FileOutputStream.write:([B)V
        64: ldc           #67                 // String Outer
-->  66: invokestatic  #54                 // Method OuterDump.dump:()[B
        69: ldc           #1                  // class Scratch
        71: invokevirtual #69                 // Method java/lang/Class.getClassLoader:()Ljava/lang/ClassLoader;
        74: ldc           #1                  // class Scratch
        76: invokevirtual #75                 // Method java/lang/Class.getProtectionDomain:()Ljava/security/ProtectionDomain;
---> 79: invokestatic  #79                 // Method DefineAnonClass.defineClass:(Ljava/lang/String;[BLjava/lang/ClassLoader;Ljava/security/ProtectionDomain;)Ljava/lang/Class;
        82: astore        5
        84: aload         5
        86: invokevirtual #85                 // Method java/lang/Class.newInstance:()Ljava/lang/Object;
        89: pop
        90: getstatic     #89                 // Field java/lang/System.out:Ljava/io/PrintStream;
        93: ldc           #95                 // String test
        95: invokevirtual #97                 // Method java/io/PrintStream.println:(Ljava/lang/String;)V
        98: return

along with DefineAnonClass.class

  public static java.lang.Class<?> defineClass(java.lang.String, java.lang.String, java.lang.ClassLoader, java.security.ProtectionDomain) throws java.lang.Throwable;
    descriptor: (Ljava/lang/String;Ljava/lang/String;Ljava/lang/ClassLoader;Ljava/security/ProtectionDomain;)Ljava/lang/Class;
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Exceptions:
      throws java.lang.Throwable
    Signature: #94                          // (Ljava/lang/String;Ljava/lang/String;Ljava/lang/ClassLoader;Ljava/security/ProtectionDomain;)Ljava/lang/Class<*>;
    Code:
      stack=7, locals=6, args_size=4
         0: aload_1
         1: invokestatic  #61                 // Method getClassBytes:(Ljava/lang/String;)[B
         4: astore        4
         6: getstatic     #30                 // Field unsafe:Ljdk/internal/misc/Unsafe;
         9: aload_0
        10: aload         4
        12: iconst_0
        13: aload         4
        15: arraylength
        16: aload_2
        17: aload_3
---> 18: invokevirtual #95    // Method jdk/internal/misc/Unsafe.defineClass:(Ljava/lang/String;[BIILjava/lang/ClassLoader;Ljava/security/ProtectionDomain;)Ljava/lang/Class;
        21: astore        5
        23: aload         5
        25: areturn

So the test is to call unsafe.defineClass() via DefineAnonClass.defineClass() with the passed-in classbytes of Outer.class (generated by OuterDump.dump) as argument.

There should be no JVM Spec or unsafe API specification clarifying whether to do the verification on the passed-in class bytes for unsafe.defineClass. In normal case, we might need to figure how to sort this out to match the RI's behavior.

[ChengJin01]

The problem should be related to the code in internalLoadROMClass() at /runtime/bcutil/defineclass.c:

---> if ((0 == (loadData->options & J9_FINDCLASS_FLAG_UNSAFE)) &&
        (0 != (vm->runtimeFlags & J9_RUNTIME_VERIFY))) {
        translationFlags |= BCT_StaticVerification;
    }

Debugging shows the static verification (in which case ClassFormatError won't be captured for the duplicate field of Outer.class) was disabled for the unsafe classes.

I didn't find out the reason behind it as the piece of code has been there for a long while. Simply removing the check of J9_FINDCLASS_FLAG_UNSAFE ended up with a compilation error on the anonymous class:

Compiling 4 files for BUILD_JIGSAW_TOOLS
Exception in thread "main" java/lang/ClassFormatError: JVMCFRE045 constant pool index out of range; class=, offset=655
    at jdk/internal/misc/Unsafe.defineAnonymousClass0 (java.base@9/NativeMethod:4294967295)
    at jdk/internal/misc/Unsafe.defineAnonymousClass (java.base@9/Unsafe.java:1537)
    at java/lang/invoke/InnerClassLambdaMetafactory.spinInnerClass (java.base@9/InnerClassLambdaMetafactory.java:331)
    at java/lang/invoke/InnerClassLambdaMetafactory.buildCallSite (java.base@9/InnerClassLambdaMetafactory.java:195)
    at java/lang/invoke/LambdaMetafactory.metafactory (java.base@9/LambdaMetafactory.java:329)
    at java/lang/invoke/MethodHandle.invokeBsm (java.base@9/MethodHandle.java:969)
    at java/lang/invoke/MethodHandle.resolveInvokeDynamic (java.base@9/MethodHandle.java:1093)
    at jdk/internal/module/SystemModuleFinders$1.find (java.base@9/SystemModuleFinders.java:215)
    at jdk/internal/module/ModuleBootstrap.boot (java.base@9/ModuleBootstrap.java:227)
    at java/lang/ClassLoader.initializeClassLoaders (java.base@9/ClassLoader.java:211)
    at java/lang/Thread.initialize (java.base@9/Thread.java:430)
    at java/lang/Thread.<init> (java.base@9/Thread.java:155)

If we determine to enable the static verification for unsafe classes, the issue with the anonymous classes should be handled accordingly.

[DanHeidinga]

Good find @ChengJin01. Can you dump the classfile that causes the ClassFormatError? I want to see what interfaces it implements / superclasses to see if there is a better source of information to base that decision on

[tajila]

Thanks for your investigation @ChengJin01

We should try to match the RI behaviour in this regard as it would simplify some of the logic required by valueTypes to handle flattened instance and static fields.

[ChengJin01]

Good find @ChengJin01. Can you dump the classfile that causes the ClassFormatError? I want to see what interfaces it implements / superclasses to see if there is a better source of information to base that decision on

The ClassFormatError occurred in the compilation. I need to check whether it is possible to add -Xdump options in the corresponding scripts to trigger the dump in such case.

[ChengJin01]

@DanHeidinga , just generated the dumps (shared to you via box) based on the command line recorded in the failure_Logs

 /../jdk/bin/java -Xint -Xdump:system+snap+java:events=systhrow,filter=java/lang
/ClassFormatError,request=exclusive+prepwalk  -Xms64M -Xmx1600M -XX:ThreadStackSize=1536 
-XX:+UseSerialGC -Xms32M -Xmx512M -XX:TieredStopAtLevel=1 -cp 
/.../build/linux-x86_64-normal-server-slowdebug/buildtools/tools_jigsaw_classes --add-exports 
java.base/jdk.internal.module=ALL-UNNAMED build.tools.jigsaw.AddPackagesAttribute 
/.../openj9-openjdk-jdk11/build/linux-x86_64-normal-server-slowdebug/jdk

JVMDUMP039I Processing dump event "systhrow", detail "java/lang/ClassFormatError" at 2020/06/18 12:55:51 - please wait.
JVMDUMP032I JVM requested System dump using '/.../dumps/core.20200618.125551.9512.0001.dmp' in response to an event
JVMPORT030W /proc/sys/kernel/core_pattern setting "|/usr/share/apport/apport %p %s %c %d %P %E" specifies that the core dump is to be piped to an external program.  Attempting to rename either core or core.9516.

JVMDUMP010I System dump written to /.../dumps/core.20200618.125551.9512.0001.dmp
JVMDUMP032I JVM requested Java dump using '/.../dumps/javacore.20200618.125551.9512.0002.txt' in response to an event
JVMDUMP010I Java dump written to /.../dumps/javacore.20200618.125551.9512.0002.txt
JVMDUMP032I JVM requested Snap dump using '/.../dumps/Snap.20200618.125551.9512.0003.trc' in response to an event
JVMDUMP010I Snap dump written to /.../dumps/Snap.20200618.125551.9512.0003.trc
JVMDUMP013I Processed dump event "systhrow", detail "java/lang/ClassFormatError".
Exception in thread "main" java/lang/ClassFormatError: JVMCFRE045 constant pool index out of range; class=, offset=655

[ChengJin01]

@DanHeidinga , according to the dumped classbytes at anoclass_class.txt from the generated core dump

Classfile  anoclass.class
  Last modified Jun. 18, 2020; size 2054 bytes
  MD5 checksum 5391efe50e914408e568c564b4269db6
final class jdk.internal.module.SystemModuleFinders$1$$Lambda$1 implements java.security.PrivilegedAction <----
  minor version: 0
  major version: 52
  flags: (0x1030) ACC_FINAL, ACC_SUPER, ACC_SYNTHETIC
  this_class: #2                          // jdk/internal/module/SystemModuleFinders$1$$Lambda$1
  super_class: #4                         // java/lang/Object <--------
  interfaces: 1, fields: 2, methods: 3, attributes: 0
Constant pool:
   #1 = Utf8               jdk/internal/module/SystemModuleFinders$1$$Lambda$1

The class with ClassFormatError is jdk.internal.module.SystemModuleFinders$1$$Lambda$1 The interface is java.security.PrivilegedAction and the superclass is java/lang/Object.

[ChengJin01]

Verified the dumped class file on both our nightly build and RI:

$ ../jdk11.0.8_7_openj9/bin/java  -version
openjdk version "11.0.8" 2020-07-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.8+7-202006152334)
Eclipse OpenJ9 VM AdoptOpenJDK (build master-3b048e5bc, JRE 11 Linux amd64-64-Bit Compressed References 20200615_660 (JIT enabled, AOT enabled)
OpenJ9   - 3b048e5bc
OMR      - 1db16ee21
JCL      - dfcd997d06 based on jdk-11.0.8+7)

$ ../jdk11.0.8_7_openj9/bin/java   anoclass
Error: LinkageError occurred while loading main class anoclass
    java.lang.ClassFormatError: JVMCFRE118 extra bytes after EOF; class=anoclass, offset=814

$ ../jdk11_hotspot/bin/java  -version
openjdk version "11.0.7" 2020-04-14
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.7+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.7+10, mixed mode)

$ ../jdk11_hotspot/bin/java    anoclass
Error: Could not find or load main class anoclass
Caused by: java.lang.NoClassDefFoundError: 
jdk/internal/module/SystemModuleFinders$1$$Lambda$1 (wrong name: anoclass)

It seems ClassFormatError was captured by our build while RI just let it passed in terms of static verification.

[ChengJin01]

Debugging shows the ClassFormatErrorcame from checkPool in which case info->slot1 (212) went beyond the number of constant pool (36) at the cpEntry (at index = 35) of the constant pool:

checkPool(J9CfrClassFile* classfile, U_8* segment, U_8* poolStart, I_32 *maxBootstrapMethodIndex, U_32 flags)
{
        case CFR_CONSTANT_String:
            /* Must be a UTF8. */
            if ((!(info->slot1)) || (info->slot1 > count)) {   count = 36 and info->slot1 = 212
                errorCode = J9NLS_CFR_ERR_BAD_INDEX__ID; <----- ClassFormatError was thrown out
                goto _errorFound;
            }

Looking at the class file that only contains 35 cpEntries in the constant pool:

class jdk/internal/module/SystemModuleFinders$1$$Lambda$1 {
  0xCAFEBABE;
  0; // minor version
  52; // version
  [] { // Constant Pool <--- totally 35 cpEntries
    ; // first element is empty
    Utf8 "jdk/internal/module/SystemModuleFinders$1$$Lambda$1"; // #1
    class #1; // #2
    Utf8 "java/lang/Object"; // #3
    class #3; // #4
    Utf8 "java/security/PrivilegedAction"; // #5
    class #5; // #6
    Utf8 "arg$1"; // #7
    Utf8 "Ljava/lang/module/ModuleFinder;"; // #8
    Utf8 "arg$2"; // #9
    Utf8 "Ljava/lang/String;"; // #10
    Utf8 "<init>"; // #11
    Utf8 "(Ljava/lang/module/ModuleFinder;Ljava/lang/String;)V"; // #12
    Utf8 "()V"; // #13
    NameAndType #11 #13; // #14
    Method #4 #14; // #15
    NameAndType #7 #8; // #16
    Field #2 #16; // #17
    NameAndType #9 #10; // #18
    Field #2 #18; // #19
    Utf8 "get$Lambda"; // #20
    Utf8 "(Ljava/lang/module/ModuleFinder;Ljava/lang/String;)Ljava/security/PrivilegedAction;"; // #21
    NameAndType #11 #12; // #22
    Method #2 #22; // #23
    Utf8 "run"; // #24
    Utf8 "()Ljava/lang/Object;"; // #25
    Utf8 "Ljava/lang/invoke/LambdaForm$Hidden;"; // #26
    Utf8 "jdk/internal/module/SystemModuleFinders$1"; // #27
    class #27; // #28
    Utf8 "lambda$find$0"; // #29
    Utf8 "(Ljava/lang/module/ModuleFinder;Ljava/lang/String;)Ljava/util/Optional;"; // #30
    NameAndType #29 #30; // #31
    Method #28 #31; // #32
    Utf8 "Code"; // #33
    Utf8 "RuntimeVisibleAnnotations"; // #34
  } // Constant Pool

This is because the code in j9bcutil_readClassFileBytes at /runtime/bcutil/cfreader.c was modified to support anoClass by adding one more cpEntry for anoClassName as follows:

j9bcutil_readClassFileBytes(...)
{
...
    CHECK_EOF(2); /* constantPoolCount */
    NEXT_U16(classfile->constantPoolCount, index);

    constantPoolAllocationSize = classfile->constantPoolCount; <------
  ...
    VERBOSE_START(ParseClassFileConstantPool);
    /* Space for the constant pool */

    if (J9_ARE_ANY_BITS_SET(findClassFlags, J9_FINDCLASS_FLAG_ANON)) {
        /* Preemptively add new entry to the end of the constantPool for the modified anonClassName.
         * If it turns out we dont need it, simply reduce the constantPoolCount by 1, which is
         * cheaper than allocating twice.
         *
         * Can't modify the classfile->constantPoolCount until after readPool is called so
         * the classfile is properly parsed. Instead use a temp variable to track the real
         * size for allocation, and re-assign to classfile->constantPoolCount later.
         *
         * If the size of the constantPool is MAX_CONSTANT_POOL_SIZE then throw an OOM.
         */
        if (constantPoolAllocationSize == MAX_CONSTANT_POOL_SIZE) {
            Trc_BCU_j9bcutil_readClassFileBytes_MaxCPCount();
            return BCT_ERR_OUT_OF_MEMORY;
        }
        constantPoolAllocationSize += 1;
    }
  ...
    /* Read the pool. */
    if ((result = readPool(classfile, data, dataEnd, segment, segmentEnd, &index, &freePointer)) != 0) {
        Trc_BCU_j9bcutil_readClassFileBytes_Exit(result);
        return result;
    }
    endOfConstantPool = index;
    VERBOSE_END(ParseClassFileConstantPool);

    classfile->constantPoolCount = constantPoolAllocationSize; <---- add one more cpEntry for the modified anonClassName

The intention of this code is to initialize the new cpEntry at ROMClassBuilder::buildROMClass which occurs after the static verification is finished:

ROMClassBuilder::buildROMClass(ROMClassCreationContext *context)
{
...
---> result = prepareAndLaydown( &bufferManager, &classFileParser, context );
...

ROMClassBuilder::prepareAndLaydown( BufferManager *bufferManager, ClassFileParser *classFileParser, ROMClassCreationContext *context )
{
...
    bool isLambda = false;
    if (context->isClassAnon()) {
----> BuildResult res = handleAnonClassName(classFileParser->getParsedClassFile(), &isLambda);
        if (OK != res) {
            return res;
        }
    }
...
ROMClassBuilder::handleAnonClassName(J9CfrClassFile *classfile, bool *isLambda)
{
...
    U_16 newUtfCPEntry = classfile->constantPoolCount - 1; /* last cpEntry is reserved for anonClassName utf */
...
    J9CfrConstantPoolInfo *anonClassName = &classfile->constantPool[newUtfCPEntry];
    /*
     * alloc an array for the new name with the following format:
     * [className]/[ROMClassAddress]\0
     */

    if ((0 == _anonClassNameBufferSize) || (newAnonClassNameLength > _anonClassNameBufferSize)) {
        j9mem_free_memory(_anonClassNameBuffer);
        _anonClassNameBuffer = (U_8 *)j9mem_allocate_memory(newAnonClassNameLength, J9MEM_CATEGORY_CLASSES);
        if (NULL == _anonClassNameBuffer) {
            result = OutOfMemory;
            goto done;
        }
        _anonClassNameBufferSize = newAnonClassNameLength;
    }
    constantPool[newUtfCPEntry].bytes = _anonClassNameBuffer;
...

In such case, it was supposed to avoid updating the number of constant pool (classfile->constantPoolCount) with constantPoolAllocationSize (in which case it failed to do so) till all constant pool related verifications were finished; otherwise, the check went wrong when reaching the initialized slot in the constant pool as mentioned above.

To address the problem, the code classfile->constantPoolCount = constantPoolAllocationSize needs to be moved close to the end of j9bcutil_readClassFileBytes after all constant pool related verifications as follows:

...
VERBOSE_END(ParseClassFileCheckClass);

    VERBOSE_START(ParseClassFileVerifyClass);
    /* Static verification. This is the first half of "Pass 2". */
    if (0 != (flags & CFR_StaticVerification)) {
        if((result = (I_32)(verifyFunction)(portLib, classfile, segment, segmentEnd, freePointer, vmVersionShifted, flags, &hasRET)) != 0) {
            Trc_BCU_j9bcutil_readClassFileBytes_Exit(result);
            return result;
        }
    } else {
        /* Special checking to look for jsr's if -noverify - the verifyFunction normally scans and tags
         * for inlining methods and classes that contain jsr's unless the class file version is 51 or
         * greater as jsr / ret are always illegal in that case
         */
        if (classfile->majorVersion < 51) {
            hasRET = checkForJsrs(classfile);
        }
    }
    VERBOSE_END(ParseClassFileVerifyClass);

    classfile->constantPoolCount = constantPoolAllocationSize; <-----

    VERBOSE_START(ParseClassFileInlineJSRs);

With the fix above (along with the static verification enabled for unsafe classes), the compilation error disappeared and the failing test case passed as expected (pretty much the same result as RI)

$ ../jdk11_openj9_build/bin/java -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
Exception in thread "main" java.lang.ClassFormatError: JVMCFRE021 duplicate field; class=Outer, offset=230
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1510)
    at DefineAnonClass.defineClass(DefineAnonClass.java:66)
    at Scratch.main(Scratch.java:380)

$ ../jdk11_hotspot/bin/java   -cp classes.jar:asm-7.0.jar --add-opens java.base/jdk.internal.misc=ALL-UNNAMED   Scratch
Exception in thread "main" java.lang.ClassFormatError: Duplicate field name "y" with signature "I" in class file Outer
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1192)
    at DefineAnonClass.defineClass(DefineAnonClass.java:66)
    at Scratch.main(Scratch.java:380)

Given the static verification for unsafe classes was disabled for quite a long while (probably never enabled before), I need to launch personal builds on Java 8 & 11 to see whether there is any potential issue with the fix above.

[ChengJin01]

A couple of test failures with Unsafe.defineClass() were detected in jsr335_interfacePrivateMethod in personal builds (Java 8 & 11). e.g. (the failing class was generated in OpenJDK extension / the same failing tests passed on RI)

java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; class=jdk/internal/reflect/GeneratedMethodAccessor19, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
---> jdk/internal/reflect/GeneratedMethodAccessor19.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; 
@20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1510) <-----
    at java.base/jdk.internal.reflect.ClassDefiner.defineClass(ClassDefiner.java:63)
    at java.base/jdk.internal.reflect.MethodAccessorGenerator$1.run(MethodAccessorGenerator.java:400)
    at java.base/jdk.internal.reflect.MethodAccessorGenerator$1.run(MethodAccessorGenerator.java:394)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:678)
    at java.base/jdk.internal.reflect.MethodAccessorGenerator.generate(MethodAccessorGenerator.java:393)
    at java.base/jdk.internal.reflect.MethodAccessorGenerator.generateMethod(MethodAccessorGenerator.java:75)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:53)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.openj9.test.jsr335.interfacePrivateMethod.Test_ReflectionAndMethodHandles.getLookupObject(Test_ReflectionAndMethodHandles.java:62)
    at org.openj9.test.jsr335.interfacePrivateMethod.Test_ReflectionAndMethodHandles.test_unreflect_child_public_static_method(Test_ReflectionAndMethodHandles.java:678)

agains the code at /runtime/bcverify/staticverify.c

checkBytecodeStructure (J9CfrClassFile * classfile, UDATA methodIndex, UDATA length, U_8 * map, J9CfrError * error, U_32 flags, I_32 *hasRET)
{
...
        case CFR_BC_invokestatic:
            ...
            if (info->tag != CFR_CONSTANT_Methodref) {
                if (((flags & BCT_MajorClassFileVersionMask) >= BCT_Java8MajorVersionShifted)
                && (bc != CFR_BC_invokevirtual) && (info->tag == CFR_CONSTANT_InterfaceMethodref)
                ) {
           ...
                    /* Valid - take no action */
                } else {
VerifyError was thrown out ------>errorType = J9NLS_CFR_ERR_BC_NOT_METHODREF__ID; 
                    /* Jazz 82615: Set the constant pool index to show up in the error message framework */
                    errorDataIndex = index;
                    goto _verifyError;
                }
            }

(1) Looking at the triggered dump:

(gdb) frame 14
#14 checkBytecodeStructure (classfile=0x7f5d1962b030, methodIndex=1, length=45, map=0x7f5d1962b978 "\001\001", 
    error=0x7f5d1962b030, flags=822477312, hasRET=0x7f5d3ea6833c <j9javaArrayOfObject_load+274>) at staticverify.c:872
(gdb) list
867                      */
868                     /* Valid - take no action */
869                 } else {
870                     errorType = J9NLS_CFR_ERR_BC_NOT_METHODREF__ID;
871                     printf("\ncheckBytecodeStructure: index = %d, cpCount = %d, info->tag = %d\n", (int)index, 
(int)cpCount, info->tag);

(gdb) p/x flags & BCT_MajorClassFileVersionMask
$12 = 0x31000000    <---- #define BCT_Java5MajorVersionShifted  0x31000000
                                            the class version = 0x31 = 49 (Java 5)
(gdb) p  bc
$14 = 184   <------------- #define CFR_BC_invokestatic 184
(gdb) p/x  info->tag
$1 = 0xb  <----      #define CFR_CONSTANT_InterfaceMethodref  11

against the Spec from JVM 5 to JVM 8:

https://jcp.org/aboutJava/communityprocess/maintenance/jsr924/JVMS-SE5.0-Ch4-ClassFile.pdf
4.10.1 Static Constraints
 The indexbyte operands of each invokevirtual, invokespecial, and invokestatic
instruction must represent a valid index into the constant_pool table. 
The constant pool entry referenced by that index must be of type CONSTANT_Methodref.

jvms7.pdf
4.9.1 Static Constraints
The indexbyte operands of each invokevirtual, invokespecial, and invokestatic 
instruction must represent a valid index into the constant_pool table.
The  constant pool entry referenced by that index must be of type CONSTANT_Methodref.

jvms8.pdf
4.9.1 Static Constraints
The indexbyte operands of each invokespecial and invokestatic instruction 
must represent a valid index into the constant_pool table. 
If the class file version number is less than 52.0, the constant pool entry
referenced by that index must be of type CONSTANT_Methodref; 
if the class file version number is 52.0 or above, the constant pool entry 
referenced by that index must be of type CONSTANT_Methodref 
or CONSTANT_InterfaceMethodref. <---- firstly introduced since JVM 8 Spec

In the case of invokestatic, the check of CONSTANT_InterfaceMethodrefon the constant pool entry was firstly introduced since JVM 8 Spec, which means our code there follows the Spec to do the correct check.

(2) Looking at the dumped class bytes: anoclass2_class.txt

public class jdk.internal.reflect.GeneratedMethodAccessor9 extends jdk.internal.reflect.MethodAccessorImpl
  minor version: 0
  major version: 49 <--------- Java 5
  flags: (0x0001) ACC_PUBLIC
  this_class: #2                          // jdk/internal/reflect/GeneratedMethodAccessor9
  super_class: #4                         // jdk/internal/reflect/MethodAccessorImpl
  interfaces: 0, fields: 0, methods: 2, attributes: 0
Constant pool:
   #1 = Utf8               jdk/internal/reflect/GeneratedMethodAccessor9
   #2 = Class              #1             // jdk/internal/reflect/GeneratedMethodAccessor9
   #3 = Utf8               jdk/internal/reflect/MethodAccessorImpl
   #4 = Class              #3             // jdk/internal/reflect/MethodAccessorImpl
   #5 = Utf8               ITestChild
   #6 = Class              #5             // ITestChild
   #7 = Utf8               getLookup
   #8 = Utf8               ()Ljava/lang/invoke/MethodHandles$Lookup;
   #9 = NameAndType        #7:#8          // getLookup:()Ljava/lang/invoke/MethodHandles$Lookup;
---> #10 = InterfaceMethodref #6.#9          // ITestChild.getLookup:()Ljava/lang/invoke/MethodHandles$Lookup;
  #11 = Utf8               invoke
...
{
  public jdk.internal.reflect.GeneratedMethodAccessor9();
    descriptor: ()V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #34                 // Method jdk/internal/reflect/MethodAccessorImpl."<init>":()V
         4: return

  public java.lang.Object invoke(java.lang.Object, java.lang.Object[]) throws java.lang.reflect.InvocationTargetException;
    descriptor: (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=4, locals=3, args_size=3
  ...
        19: athrow
----> 20: invokestatic  #10                 // InterfaceMethod ITestChild.getLookup:()Ljava/lang/invoke/MethodHandles$Lookup;

It seems the body of the generated class follows the JVM 8 Spec while the class version was filled up with Java 5.

(3) Given that the class was defined via Unsafe.defineClass() and OpenJ9 JDK & RI share pretty much the same OpenJDK extension at this point, I wrote a simple test to see how the it goes with RI.

import java.io.*;
import jdk.internal.misc.Unsafe;

public class UnSafeTest {
  public static void main(String[] args) {
        try {           
            InputStream classFile = UnSafeTest.class.getResourceAsStream("anoclass2.class");
            byte[] classFileBytes = new byte[classFile.available()];
            classFile.read(classFileBytes);
                        System.out.println("classFileBytes.length = " + classFileBytes.length);

            Unsafe unsafe = Unsafe.getUnsafe();
            Class<?> generatedClass = unsafe.defineClass("jdk.internal.reflect.GeneratedMethodAccessor9", classFileBytes, 0, classFileBytes.length, null, null);
                        System.out.println("*** PASSED ***");
        } catch (Exception | VerifyError e) {
                        System.out.println("*** FAILED!!! ***");
            e.printStackTrace();
        }
   }
}

$ ../jdk11_hotspot/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED  UnSafeTest
classFileBytes.length = 783
*** PASSED ***  (ignored the class version check for unsafe classes in this case)

$ ../jdk11_openj9_build/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED  UnSafeTest
classFileBytes.length = 783
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; class=jdk/internal/reflect/GeneratedMethodAccessor9, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    jdk/internal/reflect/GeneratedMethodAccessor9.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1510)
    at UnSafeTest.main(UnSafeTest.java:13)

After manually changing the class version to 0x34 (52 / Java 8), both RI and RI passed without any exception

$ ../jdk11_hotspot/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED  UnSafeTest
classFileBytes.length = 783
*** PASSED ***

$ ../jdk11_openj9_build/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED  UnSafeTest
classFileBytes.length = 783
*** PASSED ***

Based on the investigation above, it turns out RI didn't follow the JVM 8 Spec to check the class version for the unsafe classses.

Given that JVM 8 Spec doesn't explain whether the rule of checking the class version applies to the unsafe classses, we should change our code as follows to match RI's behaviour:

     * Note: ignore the class file version if the class is defined via Unsafe
     */
    if (!J9_ARE_ANY_BITS_SET(findClassFlags, J9_FINDCLASS_FLAG_UNSAFE)) { <-------
        flags &= ~BCT_MajorClassFileVersionMask;
        flags |= ((UDATA) classfile->majorVersion) << BCT_MajorClassFileVersionMaskShift;
    }

With the fix above, all failing tests in jsr335_interfacePrivateMethod passed as expected

jdk11_openj9_debug_v7/bin/java   -cp ".../openj9_test/test/TKG/../../jvmtest/TestConfig/resources:
.../openj9_test/test/TKG/../TKG/lib/testng.jar:
.../openj9_test/test/TKG/../TKG/lib/jcommander.jar:
.../openj9_test/test/TKG/../../jvmtest/functional/Java8andUp/GeneralTest.jar:
.../openj9_test/test/TKG/../TKG/lib/asm-all.jar" 
org.testng.TestNG -d ".../openj9_test/test/TKG/../TKG/
test_output_15929707577021/jsr335_interfacePrivateMethod_1" 
".../openj9_test/test/TKG/../../jvmtest/functional/Java8andUp/testng.xml" 
-testnames jsr335InterfacePrivateMethod -groups level.sanity
...
===============================================
    jsr335InterfacePrivateMethod
    Tests run: 66, Failures: 0, Skips: 0
===============================================

Will launch personal builds again to see what else related to Unsafe needs to be addressed.

[DanHeidinga]

@ChengJin01 What does Hotspot do for a v49 class when defined by a normal classloader rather than by Unsafe? I'm wondering if this is a general difference for old classfiles or specific to unsafe

[ChengJin01]

@DanHeidinga, with the test case with a normal classloader as follows:

public class ClassLoaderTest  extends ClassLoader {

    public  Class<?> load(final String className) throws ClassNotFoundException {
        Class<?> loadedClass = null;
        try {
     InputStream classFile = ClassLoaderTest.class.getResourceAsStream("GeneratedMethodAccessor9.class");
     byte[] classFileBytes = new byte[classFile.available()];
     classFile.read(classFileBytes);
         System.out.println("classFileBytes.length = " + classFileBytes.length);
---->  loadedClass = defineClass(className, classFileBytes, 0, classFileBytes.length); 
          System.out.println("class version = " + classFileBytes[7]);
          System.out.println("*** PASSED ***");
    } catch (IOException | VerifyError e) {
             System.out.println("*** FAILED!!! ***");
         e.printStackTrace();
    }
       return loadedClass;
    }

  public static void main(String[] args) throws ClassNotFoundException {
            ClassLoaderTest cltest = new ClassLoaderTest();
            Class<?> loadedClass = cltest.load("GeneratedMethodAccessor9");
   }
}

and UnSafeTest as verified previously:

public class UnSafeTest {
  public static void main(String[] args) {
        try {           
            InputStream classFile = UnSafeTest.class.getResourceAsStream("GeneratedMethodAccessor9.class");
            byte[] classFileBytes = new byte[classFile.available()];
            classFile.read(classFileBytes);
                        System.out.println("classFileBytes.length = " + classFileBytes.length);
                        System.out.println("class version = " + classFileBytes[7]);
            Unsafe unsafe = Unsafe.getUnsafe();
------->      Class<?> generatedClass = unsafe.defineClass("GeneratedMethodAccessor9", classFileBytes, 0, classFileBytes.length, null, null);
                        System.out.println("*** PASSED ***");
        } catch (Exception | VerifyError e) {
                        System.out.println("*** FAILED!!! ***");
            e.printStackTrace();
        }
   }
}

Hotspot ended up with same result on both class version = v49 (0x31 / Java 5) and v52 (0x34 /Java 8)

[1] GeneratedMethodAccessor9.class with v49:
$ ../jdk11_hotspot/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED   UnSafeTest
classFileBytes.length = 739
class version = 49
*** PASSED ***
$ ../jdk11_hotspot/bin/java   ClassLoaderTest
classFileBytes.length = 739
class version = 49
*** PASSED ***

[2] GeneratedMethodAccessor9.class with v52:
$ ../jdk11_hotspot/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED   UnSafeTest
classFileBytes.length = 739
class version = 52
*** PASSED ***
$ ../jdk11_hotspot/bin/java   ClassLoaderTest
classFileBytes.length = 739
class version = 52
*** PASSED ***

It means Hotspot totally ignores the check of class version in this case, whether the class is defined by a normal classloader or unsafe.

Comparably, our build (without the fix to ignore the class version) ended up with VerifyError on v49 (UnSafeTest & ClassLoaderTest) and passed on v52 (UnSafeTest & ClassLoaderTest):

[1] GeneratedMethodAccessor9.class with v49:
$ ...jdk11_openj9_build/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED   UnSafeTest
classFileBytes.length = 739
class version = 49
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; class=GeneratedMethodAccessor9, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    GeneratedMethodAccessor9.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1510)
    at UnSafeTest.main(UnSafeTest.java:13)

$ ../jdk11_openj9_build/bin/java   ClassLoaderTest
classFileBytes.length = 739
class version = 49
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; class=GeneratedMethodAccessor9, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    GeneratedMethodAccessor9.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/java.lang.ClassLoader.defineClassImpl(Native Method)
    at java.base/java.lang.ClassLoader.defineClassInternal(ClassLoader.java:476)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:437)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:389)
    at ClassLoaderTest.load(ClassLoaderTest.java:12)
    at ClassLoaderTest.main(ClassLoaderTest.java:23)

[2] GeneratedMethodAccessor9.class with v52:
$ ../jdk11_openj9_build/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED   UnSafeTest
classFileBytes.length = 739
class version = 52
*** PASSED ***

$ ../jdk11_openj9_build/bin/java   ClassLoaderTest
classFileBytes.length = 739
class version = 52
*** PASSED ***

As for the JVM 8 Spec, my understanding is that the check on CONSTANT_InterfaceMethodref was effective immediately when it was introduced in JVM 8 Spec, in which case the previous rule on this was automatically discarded, regardless of the class version. e.g. Hotspot still works fine with v46 on both UnSafeTest and ClassLoaderTest:

GeneratedMethodAccessor9.class with `v46`:
$ ../jdk11_hotspot/bin/java   --add-exports java.base/jdk.internal.misc=ALL-UNNAMED   UnSafeTest
classFileBytes.length = 739
class version = 46  <---- JDK 1.2
*** PASSED ***
$ ../jdk11_hotspot/bin/java   ClassLoaderTest
classFileBytes.length = 739
class version = 46 <---- JDK 1.2
*** PASSED ***

If we'd like to match RI's behaviour at this point, the following code should change from BCT_Java8MajorVersionShifted to BCT_Java2MajorVersionShifted (given BCT_Java2MajorVersionShifted is the minimal value in our code) or totally remove the check on the class version:

/runtime/bcverify/staticverify.c
checkBytecodeStructure (J9CfrClassFile * classfile, UDATA methodIndex, UDATA length, U_8 * map, J9CfrError * error, U_32 flags, I_32 *hasRET)
{
...
            case CFR_BC_invokestatic:
                 ...
            if (info->tag != CFR_CONSTANT_Methodref) {
set up the check on JDK 1.2 or remove -->  if (((flags & BCT_MajorClassFileVersionMask) >= BCT_Java8MajorVersionShifted)
                      && (bc != CFR_BC_invokevirtual) && (info->tag == CFR_CONSTANT_InterfaceMethodref)
                ) {
                    /* JVMS 4.9.1 Static Constraints:
                     * The indexbyte operands of each invokespecial and invokestatic instruction must represent
                     * a valid index into the constant_pool table. The constant pool entry referenced by that
                     * index must be either of type CONSTANT_Methodref or of type CONSTANT_InterfaceMethodref.
                     */
                    /* Valid - take no action */
                } else {
                    errorType = J9NLS_CFR_ERR_BC_NOT_METHODREF__ID;
                    /* Jazz 82615: Set the constant pool index to show up in the error message framework */
                    errorDataIndex = index;
                    goto _verifyError;
                }
            }

[DanHeidinga]

@ChengJin01 Can you rerun the test above passing -Xverify:all to rule out any "optimizations" that avoid verifying locally defined classes? It shouldn't be the case but better to be 100% sure

[DanHeidinga]

If we ignore the check only for classes from Unsafe (my preference), does it apply with new JDKs as well? Does jdk14 need the check to be ignored as well? Trying to gauge how to limit the effect of this to as small a set of classes (unsafe) & jdk releases as possible (ie: only 8 & 11 if possible)

[ChengJin01]

@DanHeidinga,

In order to avoid loading class with the duplicate name in test, I renamed GeneratedMethodAccessor9.class to anoClassTest.class and did a couple of experiments with the following test cases:

[1] ran previous tests to check anoClassTest.class with v49 and with -Xverify:all specified:

$ ../bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest
classFileBytes.length = 727
class version = 49
*** PASSED ***

$ ../jdk11_hotspot/bin/java -Xverify:all -cp  .:  ClassLoaderTest
classFileBytes.length = 727
class version = 49
*** PASSED ***

[2] ran UnSafeTest2 with the current class loader (loading the test case) specified:

public class UnSafeTest2 {
  public static void main(String[] args) {
        try {           
            InputStream classFile = UnSafeTest.class.getResourceAsStream("anoClassTest.class");
            byte[] classFileBytes = new byte[classFile.available()];
            classFile.read(classFileBytes);
                        System.out.println("classFileBytes.length = " + classFileBytes.length);
                        System.out.println("class version = " + classFileBytes[7]);
            Unsafe unsafe = Unsafe.getUnsafe();
            Class<?> generatedClass = unsafe.defineClass("anoClassTest", classFileBytes, 0, classFileBytes.length, UnSafeTest.class.getClassLoader(), null); <---------------
                        System.out.println("*** PASSED ***");
        } catch (Exception | VerifyError e) {
                        System.out.println("*** FAILED!!! ***");
            e.printStackTrace();
        }
   }
}

which ended up with the same result as previously (different from our build):

$ ../jdk11_hotspot/bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest2
classFileBytes.length = 727
class version = 49
*** PASSED ***

../jdk11_openj9_build/bin/java  --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest2
classFileBytes.length = 727
class version = 49
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref;
 class=anoClassTest, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    anoClassTest.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/jdk.internal.misc.Unsafe.defineClass0(Native Method)
    at java.base/jdk.internal.misc.Unsafe.defineClass(Unsafe.java:1510)
    at UnSafeTest2.main(UnSafeTest2.java:13)

[3] ran UnSafeTest3 to call Unsafe.allocateInstance() instead of Unsafe.defineClass()

public class UnSafeTest3 {
  public static void main(String[] args) {
        try {           
            Unsafe unsafe = Unsafe.getUnsafe();
                        unsafe.allocateInstance(anoClassTest.class); <-----------
                        System.out.println("*** PASSED ***");
        } catch (Exception | VerifyError e) {
                        System.out.println("*** FAILED!!! ***");
            e.printStackTrace();
        }
   }
}

which ended up with VerifyError as follows, regardless of -Xverify:all:

$ ../jdk11_hotspot/bin/java  --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest3
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature: 
(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/jdk.internal.misc.Unsafe.allocateInstance(Native Method)
    at UnSafeTest3.main(UnSafeTest3.java:8)

$ ../bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest3
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature:
 (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/jdk.internal.misc.Unsafe.allocateInstance(Native Method)
    at UnSafeTest3.main(UnSafeTest3.java:8)

$ ../jdk11_openj9_build/bin/java  --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest3
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; 
class=anoClassTest, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    anoClassTest.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/java.lang.ClassLoader.defineClassImpl(Native Method) <------
    at java.base/java.lang.ClassLoader.defineClassInternal(ClassLoader.java:476)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:437)
    at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
    at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:1110)
    at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:898)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:806)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:764)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:1078)
    at UnSafeTest3.main(UnSafeTest3.java:8)

[4] ran ClassForNameTest to check anoClassTest.class with v49:

public class ClassForNameTest  {

 public static void main(String[] args) 
    {
        try 
        {
            Class<?> cls = Class.forName("anoClassTest", true, ClassForNameTest.class.getClassLoader());
            System.out.println("*** PASSED ***");
        }
        catch (ClassNotFoundException | VerifyError e) 
        {
            System.out.println("*** FAILED!!! ***");
            e.printStackTrace();
        }
    }
}

which ended up with VerifyError as follows, regardless of -Xverify:all:

$ ../jdk11_hotspot/bin/java -cp  .:  ClassForNameTest
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature: (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/java.lang.Class.forName0(Native Method) <----------
    at java.base/java.lang.Class.forName(Class.java:398)
    at ClassForNameTest.main(ClassForNameTest.java:9)

$ ../jdk11_hotspot/bin/java -Xverify:all -cp  .:  ClassForNameTest
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature: 
(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/java.lang.Class.forName0(Native Method) <----------
    at java.base/java.lang.Class.forName(Class.java:398)
    at ClassForNameTest.main(ClassForNameTest.java:9)

$ /jdk11_openj9_build/bin/java -cp  .:  ClassForNameTest
*** FAILED!!! ***
java.lang.VerifyError: JVMCFRE116 invoke bytecode must reference a Methodref; 
class=anoClassTest, method=invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;, pc=20
Exception Details:
  Location:
    anoClassTest.invoke(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object; @20: JBinvokestatic
  Reason:
    Wrong type at the index 10 of constant pool
  Exception Handler Table:
    bci [0, 20] => handler: 24
    bci [0, 20] => handler: 24
    bci [20, 23] => handler: 36
    at java.base/java.lang.ClassLoader.defineClassImpl(Native Method) <------
    at java.base/java.lang.ClassLoader.defineClassInternal(ClassLoader.java:476)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:437)
    at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
    at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:1110)
    at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:898)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:806)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:764)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:1078)
    at java.base/java.lang.Class.forNameImpl(Native Method)
    at java.base/java.lang.Class.forName(Class.java:412)

Based on the results above, it turns out the verification was ignored when calling Unsafe.defineClass() in RI. Given the difference of the implementation between RI and OpenJ9 at this point, we can safely ignore the check on Unsafe.defineClass() and the rest of the situations are already handled by ClassLoader.defineClass().

As for Java 14, RI ended up with the same result Java 11

$ ../jdk14_hotspot/bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest
classFileBytes.length = 727
class version = 49
*** PASSED ***

$ ../jdk14_hotspot/bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest2
classFileBytes.length = 727
class version = 49
*** PASSED ***

$ ../jdk14_hotspot/bin/java -Xverify:all --add-exports java.base/jdk.internal.misc=ALL-UNNAMED     -cp  .:    UnSafeTest3
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature: 
(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/jdk.internal.misc.Unsafe.allocateInstance(Native Method)
    at UnSafeTest3.main(UnSafeTest3.java:8)

$ ../jdk11_hotspot/bin/java -Xverify:all -cp  .:  ClassForNameTest
*** FAILED!!! ***
java.lang.VerifyError: (class: anoClassTest, method: invoke signature: 
(Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
    at java.base/java.lang.Class.forName0(Native Method)
    at java.base/java.lang.Class.forName(Class.java:398)
    at ClassForNameTest.main(ClassForNameTest.java:9)

So technically we should ignore the check of Unsafe.defineClass() on Java 14 given OpenJ9 share the same OpenJDK14 extension as RI at this point but we can limit the fix to Java 8 & 11 to avoid potential issues.

[ChengJin01]

Will launch personal builds with the latest changes to double-check again whether everything works good as expected.