finalFieldSetAllowed doing validation on class file with version less than 53

[mikezhang1234567890]

In resolvesupport.cpp the brief for the finalFieldSetAllowed states:

In class files with version 53 or later, setting of final fields is only allowed from initializer methods.

which implies that the check should not happen in class files with version 52 or less.

In https://github.com/eclipse/openj9/commit/315ed967602b64215195d8ab2d2aad78250c66ed, the check to do the validation is changed: https://github.com/eclipse/openj9/commit/315ed967602b64215195d8ab2d2aad78250c66ed#diff-b8265e29caabd4bcd2d6d544d5ee653cL83-R74

The old condition calls the function ramClassChecksFinalStores which includes a check for class file version >= 53, while the new condition does not do this check and runs the validation on all class file versions.

Ran into this when investigating https://github.com/eclipse/openj9/issues/6366

[DanHeidinga]

@mikezhang1234567890 The check moved and morphed somewhat. The new code sets the J9AccMethodAllowFinalFieldWrites method modifier for classfiles older than v53 in https://github.com/eclipse/openj9/blob/315ed967602b64215195d8ab2d2aad78250c66ed/runtime/bcutil/ROMClassWriter.cpp#L1213-L1215

Are you seeing unexpected failures or a behaviour change?

[mikezhang1234567890]

There is a behaviour change since the macro J9ROMMETHOD_ALLOW_FINAL_FIELD_WRITES for actually checking if we can write to final fields is https://github.com/eclipse/openj9/blob/85b0693e8754d1f1c03592f20c65e32da984b01c/runtime/oti/j9modifiers_api.h#L121-L122 and enforces that the field and method must both have the same staticness. Before, we didn't reach this point if the class file version was < 53.

From the javap output of the class causing the error (minus the constant pool and unnecessary methods):

Classfile /home/mike/0.88Tekxit3_Server/CLASSLOADER_TEMP/codechicken/multipart/handler/MultipartMod$.class
  Last modified Aug 1, 2019; size 3023 bytes
  MD5 checksum 202ff39df0852bb2f3de1697e4e12f87
  Compiled from "MultipartMod.scala"
public final class codechicken.multipart.handler.MultipartMod$
  minor version: 0
  major version: 51
  flags: ACC_PUBLIC, ACC_FINAL, ACC_SUPER

{
  public static final codechicken.multipart.handler.MultipartMod$ MODULE$;
    descriptor: Lcodechicken/multipart/handler/MultipartMod$;
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL

  private final java.lang.String modID;
    descriptor: Ljava/lang/String;
    flags: ACC_PRIVATE, ACC_FINAL

  private final java.lang.String deps;
    descriptor: Ljava/lang/String;
    flags: ACC_PRIVATE, ACC_FINAL

  public static {};
    descriptor: ()V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: new           #2                  // class codechicken/multipart/handler/MultipartMod$
         3: invokespecial #31                 // Method "<init>":()V
         6: return

  private codechicken.multipart.handler.MultipartMod$();
    descriptor: ()V
    flags: ACC_PRIVATE
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #101                // Method java/lang/Object."<init>":()V
         4: aload_0
         5: putstatic     #103                // Field MODULE$:Lcodechicken/multipart/handler/MultipartMod$;
         8: return
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       9     0  this   Lcodechicken/multipart/handler/MultipartMod$;
      LineNumberTable:
        line 38: 0
}
SourceFile: "MultipartMod.scala"
InnerClasses:
     public static #21= #18 of #20; //EventHandler=class net/minecraftforge/fml/common/Mod$EventHandler of class net/minecraftforge/fml/common/Mod
RuntimeVisibleAnnotations:
  0: #6(#7=s#8,#9=s#10,#11=s#12,#13=s#14,#15=s#16)
Error: unknown attribute
  Scala: length = 0x0

The static final field MODULE$ is set through the constructor which is called in the static initializer. On hotspot this class gets initialized normally, but on recent builds of OpenJ9, we get this error:

java.lang.IllegalAccessError: Attempt to set static final field codechicken/multipart/handler/MultipartMod$.MODULE$ from method "<init>" that is not <clinit>

[gacholio]

A simple testcase demonstrating the failure would be helpful.

[gacholio]

My test show this code to be working.

VM used to test:

openjdk version "11.0.4" 2019-07-16 OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.4+11-201908061830) Eclipse OpenJ9 VM AdoptOpenJDK (build master-85b0693e8, JRE 11 Linux amd64-64-Bit Compressed References 20190806_300 (JIT enabled, AOT enabled) OpenJ9 - 85b0693e8 OMR - 6e99760b JCL - 965d0c0df3 based on jdk-11.0.4+11)

The following zip has the test:

test.zip

[mikezhang1234567890]

Here is a small test similar to what the minecraft mod above is doing. Generator uses asm from here.

OpenJ9 VM used to test:

openjdk version "1.8.0_222"
OpenJDK Runtime Environment (build 1.8.0_222-201908060344-b10)
Eclipse OpenJ9 VM (build master-85b0693e8, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20190806_379 (JIT enabled, AOT enabled)
OpenJ9   - 85b0693e8
OMR      - 6e99760b
JCL      - 46e5e620 based on jdk8u222-b10)

Error:

Exception in thread "main" java.lang.IllegalAccessError: Attempt to set static final field FinalFieldsTest.MODULE$ from method "<init>" that is not <clinit>
    at FinalFieldsTest.<init>(Unknown Source)
    at FinalFieldsTest.<clinit>(Unknown Source)

Hotspot VM that passed:

openjdk version "1.8.0_222"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_222-201907290328-b10)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.222-b10, mixed mode)

fftest.zip

[gacholio]

Based on the stack trace, I'm not surprised - we are not allowing setting of final fields from the "wrong" initializer - instance fields only from <init> and static fields only from <clinit>.

@DanHeidinga @tajila

[gacholio]

What is the result of running this on a HotSpot JDK11 VM (I'll try it myself eventually!)?

[gacholio]

The issue appears to be that we're now enforcing that static fields be set only from static methods and instance fields only from instance methods, which is not the previous permissive behaviour.

[mikezhang1234567890]

Hotspot JDK11 VM runs the test normally.

openjdk version "11.0.5" 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.5+10-201908041809)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.5+10-201908041809, mixed mode)