8157181: Compilers accept modification of final fields outside initializer methods

Summary: Track initialized final field updates; disable constant folding if an update is detected.
Reviewed-by: vlivanov, dnsimon, forax, never, kvn, coleenp

src/share/vm/ci/ciField.cpp

@@ -207,7 +207,7 @@ void ciField::initialize_from(fieldDescriptor* fd) {
   // Check to see if the field is constant.
   bool is_final = this->is_final();
   bool is_stable = FoldStableValues && this->is_stable();
-  if (_holder->is_initialized() && (is_final || is_stable)) {
+  if (_holder->is_initialized() && ((is_final && !has_initialized_final_update()) || is_stable)) {
     if (!this->is_static()) {
       // A field can be constant if it's a final static field or if
       // it's a final non-static field of a trusted class (classes in

src/share/vm/ci/ciField.hpp

@@ -124,22 +124,8 @@ public:
     return _holder->is_shared() && !is_static();
   }
 
-  // Is this field a constant?
-  //
-  // Clarification: A field is considered constant if:
-  //   1. The field is both static and final
-  //   2. The canonical holder of the field has undergone
-  //      static initialization.
-  //   3. If the field is an object or array, then the oop
-  //      in question is allocated in perm space.
-  //   4. The field is not one of the special static/final
-  //      non-constant fields.  These are java.lang.System.in
-  //      and java.lang.System.out.  Abomination.
-  //
-  // A field is also considered constant if it is marked @Stable
-  // and is non-null (or non-zero, if a primitive).
-  // For non-static fields, the null/zero check must be
-  // arranged by the user, as constant_value().is_null_or_zero().
+  // Is this field a constant? See ciField::initialize_from() for details
+  // about how a field is determined to be constant.
   bool is_constant() { return _is_constant; }
 
   // Get the constant value of this field.
@@ -176,6 +162,9 @@ public:
   bool is_stable      () { return flags().is_stable(); }
   bool is_volatile    () { return flags().is_volatile(); }
   bool is_transient   () { return flags().is_transient(); }
+  // The field is modified outside of instance initializer methods
+  // (or class/initializer methods if the field is static).
+  bool has_initialized_final_update() { return flags().has_initialized_final_update(); }
 
   bool is_call_site_target() {
     ciInstanceKlass* callsite_klass = CURRENT_ENV->CallSite_klass();

src/share/vm/ci/ciFlags.hpp

@@ -46,20 +46,25 @@ private:
 
 public:
   // Java access flags
-  bool is_public      () const         { return (_flags & JVM_ACC_PUBLIC      ) != 0; }
-  bool is_private     () const         { return (_flags & JVM_ACC_PRIVATE     ) != 0; }
-  bool is_protected   () const         { return (_flags & JVM_ACC_PROTECTED   ) != 0; }
-  bool is_static      () const         { return (_flags & JVM_ACC_STATIC      ) != 0; }
-  bool is_final       () const         { return (_flags & JVM_ACC_FINAL       ) != 0; }
-  bool is_synchronized() const         { return (_flags & JVM_ACC_SYNCHRONIZED) != 0; }
-  bool is_super       () const         { return (_flags & JVM_ACC_SUPER       ) != 0; }
-  bool is_volatile    () const         { return (_flags & JVM_ACC_VOLATILE    ) != 0; }
-  bool is_transient   () const         { return (_flags & JVM_ACC_TRANSIENT   ) != 0; }
-  bool is_native      () const         { return (_flags & JVM_ACC_NATIVE      ) != 0; }
-  bool is_interface   () const         { return (_flags & JVM_ACC_INTERFACE   ) != 0; }
-  bool is_abstract    () const         { return (_flags & JVM_ACC_ABSTRACT    ) != 0; }
-  bool is_strict      () const         { return (_flags & JVM_ACC_STRICT      ) != 0; }
-  bool is_stable      () const         { return (_flags & JVM_ACC_FIELD_STABLE) != 0; }
+  bool is_public               () const { return (_flags & JVM_ACC_PUBLIC                    ) != 0; }
+  bool is_private              () const { return (_flags & JVM_ACC_PRIVATE                   ) != 0; }
+  bool is_protected            () const { return (_flags & JVM_ACC_PROTECTED                 ) != 0; }
+  bool is_static               () const { return (_flags & JVM_ACC_STATIC                    ) != 0; }
+  bool is_final                () const { return (_flags & JVM_ACC_FINAL                     ) != 0; }
+  bool is_synchronized         () const { return (_flags & JVM_ACC_SYNCHRONIZED              ) != 0; }
+  bool is_super                () const { return (_flags & JVM_ACC_SUPER                     ) != 0; }
+  bool is_volatile             () const { return (_flags & JVM_ACC_VOLATILE                  ) != 0; }
+  bool is_transient            () const { return (_flags & JVM_ACC_TRANSIENT                 ) != 0; }
+  bool is_native               () const { return (_flags & JVM_ACC_NATIVE                    ) != 0; }
+  bool is_interface            () const { return (_flags & JVM_ACC_INTERFACE                 ) != 0; }
+  bool is_abstract             () const { return (_flags & JVM_ACC_ABSTRACT                  ) != 0; }
+  bool is_strict               () const { return (_flags & JVM_ACC_STRICT                    ) != 0; }
+  bool is_stable               () const { return (_flags & JVM_ACC_FIELD_STABLE              ) != 0; }
+  // In case the current object represents a field, return true if
+  // the field is modified outside of instance initializer methods
+  // (or class/initializer methods if the field is static) and false
+  // otherwise.
+  bool has_initialized_final_update() const { return (_flags & JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE) != 0; };
 
   // Conversion
   jint   as_int()                      { return _flags; }

src/share/vm/interpreter/rewriter.cpp

@@ -396,10 +396,45 @@ void Rewriter::scan_method(Method* method, bool reverse, bool* invokespecial_err
           break;
         }
 
+        case Bytecodes::_putstatic      :
+        case Bytecodes::_putfield       : {
+          if (!reverse) {
+            // Check if any final field of the class given as parameter is modified
+            // outside of initializer methods of the class. Fields that are modified
+            // are marked with a flag. For marked fields, the compilers do not perform
+            // constant folding (as the field can be changed after initialization).
+            //
+            // The check is performed after verification and only if verification has
+            // succeeded. Therefore, the class is guaranteed to be well-formed.
+            InstanceKlass* klass = method->method_holder();
+            u2 bc_index = Bytes::get_Java_u2(bcp + prefix_length + 1);
+            constantPoolHandle cp(method->constants());
+            Symbol* field_name = cp->name_ref_at(bc_index);
+            Symbol* field_sig = cp->signature_ref_at(bc_index);
+            Symbol* ref_class_name = cp->klass_name_at(cp->klass_ref_index_at(bc_index));
+
+            if (klass->name() == ref_class_name) {
+              fieldDescriptor fd;
+              klass->find_field(field_name, field_sig, &fd);
+              if (fd.access_flags().is_final()) {
+                if (fd.access_flags().is_static()) {
+                  assert(c == Bytecodes::_putstatic, "must be putstatic");
+                  if (!method->is_static_initializer()) {
+                    fd.set_has_initialized_final_update(true);
+                  }
+                } else {
+                  assert(c == Bytecodes::_putfield, "must be putfield");
+                  if (!method->is_object_initializer()) {
+                    fd.set_has_initialized_final_update(true);
+                  }
+                }
+              }
+            }
+          }
+        }
+        // fall through
         case Bytecodes::_getstatic      : // fall through
-        case Bytecodes::_putstatic      : // fall through
         case Bytecodes::_getfield       : // fall through
-        case Bytecodes::_putfield       : // fall through
         case Bytecodes::_invokevirtual  : // fall through
         case Bytecodes::_invokestatic   :
         case Bytecodes::_invokeinterface:

src/share/vm/oops/method.cpp

@@ -590,7 +590,7 @@ bool Method::is_constant_getter() const {
 }
 
 bool Method::is_initializer() const {
-  return name() == vmSymbols::object_initializer_name() || is_static_initializer();
+  return is_object_initializer() || is_static_initializer();
 }
 
 bool Method::has_valid_initializer_flags() const {
@@ -606,6 +606,9 @@ bool Method::is_static_initializer() const {
          has_valid_initializer_flags();
 }
 
+bool Method::is_object_initializer() const {
+   return name() == vmSymbols::object_initializer_name();
+}
 
 objArrayHandle Method::resolved_checked_exceptions_impl(Method* this_oop, TRAPS) {
   int length = this_oop->checked_exceptions_length();

src/share/vm/oops/method.hpp

@@ -627,6 +627,9 @@ class Method : public Metadata {
   // valid static initializer flags.
   bool is_static_initializer() const;
 
+  // returns true if the method name is <init>
+  bool is_object_initializer() const;
+
   // compiled code support
   // NOTE: code() is inherently racy as deopt can be clearing code
   // simultaneously. Use with caution.

src/share/vm/runtime/fieldDescriptor.hpp

@@ -106,6 +106,7 @@ class fieldDescriptor VALUE_OBJ_CLASS_SPEC {
   bool is_field_access_watched()  const    { return access_flags().is_field_access_watched(); }
   bool is_field_modification_watched() const
                                            { return access_flags().is_field_modification_watched(); }
+  bool has_initialized_final_update() const { return access_flags().has_field_initialized_final_update(); }
   bool has_generic_signature()    const    { return access_flags().field_has_generic_signature(); }
 
   void set_is_field_access_watched(const bool value) {
@@ -118,6 +119,11 @@ class fieldDescriptor VALUE_OBJ_CLASS_SPEC {
     update_klass_field_access_flag();
   }
 
+  void set_has_initialized_final_update(const bool value) {
+    _access_flags.set_has_field_initialized_final_update(value);
+    update_klass_field_access_flag();
+  }
+
   // Initialization
   void reinitialize(InstanceKlass* ik, int index);
 

src/share/vm/utilities/accessFlags.hpp

@@ -76,11 +76,12 @@ enum {
   // These bits must not conflict with any other field-related access flags
   // (e.g., ACC_ENUM).
   // Note that the class-related ACC_ANNOTATION bit conflicts with these flags.
-  JVM_ACC_FIELD_ACCESS_WATCHED       = 0x00002000,  // field access is watched by JVMTI
-  JVM_ACC_FIELD_MODIFICATION_WATCHED = 0x00008000,  // field modification is watched by JVMTI
-  JVM_ACC_FIELD_INTERNAL             = 0x00000400,  // internal field, same as JVM_ACC_ABSTRACT
-  JVM_ACC_FIELD_STABLE               = 0x00000020,  // @Stable field, same as JVM_ACC_SYNCHRONIZED
-  JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE = 0x00000800, // field has generic signature
+  JVM_ACC_FIELD_ACCESS_WATCHED            = 0x00002000, // field access is watched by JVMTI
+  JVM_ACC_FIELD_MODIFICATION_WATCHED      = 0x00008000, // field modification is watched by JVMTI
+  JVM_ACC_FIELD_INTERNAL                  = 0x00000400, // internal field, same as JVM_ACC_ABSTRACT
+  JVM_ACC_FIELD_STABLE                    = 0x00000020, // @Stable field, same as JVM_ACC_SYNCHRONIZED and JVM_ACC_SUPER
+  JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE  = 0x00000100, // (static) final field updated outside (class) initializer, same as JVM_ACC_NATIVE
+  JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE     = 0x00000800, // field has generic signature
 
   JVM_ACC_FIELD_INTERNAL_FLAGS       = JVM_ACC_FIELD_ACCESS_WATCHED |
                                        JVM_ACC_FIELD_MODIFICATION_WATCHED |
@@ -150,6 +151,8 @@ class AccessFlags VALUE_OBJ_CLASS_SPEC {
   bool is_field_access_watched() const  { return (_flags & JVM_ACC_FIELD_ACCESS_WATCHED) != 0; }
   bool is_field_modification_watched() const
                                         { return (_flags & JVM_ACC_FIELD_MODIFICATION_WATCHED) != 0; }
+  bool has_field_initialized_final_update() const
+                                        { return (_flags & JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE) != 0; }
   bool on_stack() const                 { return (_flags & JVM_ACC_ON_STACK) != 0; }
   bool is_internal() const              { return (_flags & JVM_ACC_FIELD_INTERNAL) != 0; }
   bool is_stable() const                { return (_flags & JVM_ACC_FIELD_STABLE) != 0; }
@@ -229,6 +232,15 @@ class AccessFlags VALUE_OBJ_CLASS_SPEC {
                                            atomic_clear_bits(JVM_ACC_FIELD_MODIFICATION_WATCHED);
                                          }
                                        }
+
+  void set_has_field_initialized_final_update(const bool value) {
+    if (value) {
+      atomic_set_bits(JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE);
+    } else {
+      atomic_clear_bits(JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE);
+    }
+  }
+
   void set_field_has_generic_signature()
                                        {
                                          atomic_set_bits(JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE);