JDK-8157181 : Compilers accept modification of final fields outside initializer methods
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 7,8,9
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-05-17
  • Updated: 2017-11-29
  • Resolved: 2016-06-15
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 JDK 9
7u171Fixed 8u152Fixed 9 b127Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Sub Tasks
JDK-8177116 :  
Description
According to the Java VM Specification, the putstatic bytecode is allowed to modify a final field only
- (1) if the field is declared in the current class (the class that declares the current method) and
- (2) if the putstatic instruction appears in the class or interface initializer method <clinit> of the current class.

Otherwise an IllegalAccessError must be thrown.

Similar, the putfield bytecode is allowed to modify a final field only
- (1) if the field is declared in the current class and
- (2) if the instruction appears in an instance initializer method <init> of the current class.

Otherwise an IllegalAccesError must be thrown.

Currently, HotSpot checks only condition (1) but not (2):
http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/07a5eceac654/src/share/vm/interpreter/linkResolver.cpp#l910

Methods that do not satisfy condition (2) violate the assumptions of the compilers.  Compiling such methods results in different behavior of compiled and interpreted code (see a detailed example below).
Comments
Don't overlook setting via native code - ie the infamous System.setIn/setOut/setErr
21-07-2016

Only in Java SE 5.0 did the JVM Specification mandate that putfield/putstatic for a final field occur in the appropriate initialization method. (See the description of JDK-8159215 for details.) In the JVMS updates for Java SE 5.0, there is no suggestion that the mandate applies to >=49.0 class files only. If ever there was a mandate that should apply to all class file versions, this is it. I recommend the HotSpot fix in this bug does NOT inspect class file version.
30-06-2016

Ok, I see performance tests were run but not necessarily startup tests, which this would affect. Also, it looks like the check in the rewriter is not incorrect because assigning to an final field from another class is already detected.
29-06-2016

I see the performance tests were run. Were there any startup performance tests?
29-06-2016

If it has always been in the specification that final fields must always be initialized in the initialization method <clinit> or <init> for putstatic and putfield respectively, I don't think there should be a classfile version check. This is less likely than the case we found with 8145148: InterfaceMethod CP entry pointing to a class should cause ICCE (linked), and that was decision for this bug. If there's no classfile version check, then the code in the rewriter is unnecessary, I believe. Because this case will always trigger a link resolution error and the compiler should not have the opportunity to compile this code. I cannot convince myself that the rewriter code correct now and it slows down rewriting for all methods linked by the vm. I don't know if there have been startup time performance measurements done. It also has a regression https://bugs.openjdk.java.net/browse/JDK-8160551.
29-06-2016

Hi John, thank you for pointing that out! Should the verifier be extended to take care of the the special case you've mentioned? If yes, I'm happy to file a bug for that. Best regards, Zoltan
27-06-2016

ILW = breaking spec; only non-java compilers generating code violating java language rules; none = HLH = P2
15-06-2016

Hi David, I've implemented what you've suggested before, here is the code: http://cr.openjdk.java.net/~zmajo/8157181/webrev.05/ If you have time, could you please give me some feedback (maybe in email discussion on hotspot-compiler-dev)? Thanks a lot! Best regards, Zoltan
02-06-2016

ILW=breaking specification,defect encountered with custom-generated bytecode (i.e., bytecodes not generated by javac),no workaround=HMH=P1
02-06-2016

If this is a long standing non-compliance with the JVMS then its impact can not be considered high. The longer the VM and spec do not match the more likely we adjust the spec to match the VM behaviour. When that is not feasible then the usual approach is to enforce the correct rules as of the current classfile version, otherwise we risk introducing compatibility issues. Even if we make the correct behaviour the default, we will usually provide a flag to enable the old behaviour for compatibility purposes. Only if this is a security concern would we force the change to the new behaviour.
02-06-2016

Running a Jython-based application with the -Xcomp flag fails with a NullPointerException being thrown. Running the same program with -Xint or -Xmixed does not trigger that exception. The difference between the VM's behavior with -Xcomp and -Xmixed/-Xint is due to the compilation of the org.python.pycode._pyx0.<init> method by the C1 compiler. The org.python.pycode._pyx0.<init> method consists of the following bytecodes: 0 aload_0 1 invokespecial 42 <org/python/core/PyFunctionTable.<init>()V> 4 aload_0 5 putstatic 48 <org/python/pycode/_pyx0.self/Lorg/python/pycode/_pyx0;> 8 fast_aldc <string> 10 invokestatic 56 <org/python/core/Py.newString(Ljava/lang/String;)Lorg/python/core/PyString;> 13 putstatic 58 <org/python/pycode/_pyx0._0/Lorg/python/core/PyString;> 16 iconst_0 17 iconst_0 18 anewarray java/lang/String 21 astore_2 22 aload_2 23 aload_1 24 fast_aldc ? 26 iconst_0 27 iconst_0 28 iconst_0 29 getstatic 48 <org/python/pycode/_pyx0.self/Lorg/python/pycode/_pyx0;> 32 iconst_0 33 aconst_null 34 aconst_null 35 iconst_0 36 iconst_0 37 invokestatic 67 <org/python/core/Py.newCode(I[Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IZZLorg/python/core/PyFunctionTable;I[Ljava/lang/String;[Ljava/lang/String;II)Lorg/python/core/PyCode;> 40 putstatic 69 <org/python/pycode/_pyx0.f$0/Lorg/python/core/PyCode;> 43 return The class _pyx0 and the method <init> are generated at runtime by Jython. The method does not conform with the Java VM Specifictaion, because it sets the static final field _pyx0.self at bytecode @5. The initialization of that static final field is supposed to be performed in the class initializer method <clinit>. The <clinit> method is not generated for the class _pyx0. While compiling bytecode @29, the C1 compiler assumes that the value of the _pyx0.self field has already been set in the class initializer <clinit> method (as the class is initialized). The C1 compiler also assumes that the field will not change (as it is static final). Therefore, the C1 compiler omits reading the field _pyx0.self and passes the current value of the field ('null') to the org/ python/core/Py.newCode() method called at bytecode @37. (The correct value would be 'this'.) http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/07a5eceac654/src/share/vm/c1/c1_GraphBuilder.cpp#l1639 Using 'null' instead of a valid object results in a NullPointerException being thrown. With -Xint, the VM executes the putstatic to the static final field as any other putstatic (instead of throwing an IllegalAccessError, as required by the specification). That behavior is, however, expected (and acceptable) by Jython.
18-05-2016