Mon, 12 Dec 2016 12:53:38 +0100
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
1.1 --- a/src/share/vm/ci/ciField.cpp Thu Dec 08 15:12:58 2016 +0300 1.2 +++ b/src/share/vm/ci/ciField.cpp Mon Dec 12 12:53:38 2016 +0100 1.3 @@ -207,7 +207,7 @@ 1.4 // Check to see if the field is constant. 1.5 bool is_final = this->is_final(); 1.6 bool is_stable = FoldStableValues && this->is_stable(); 1.7 - if (_holder->is_initialized() && (is_final || is_stable)) { 1.8 + if (_holder->is_initialized() && ((is_final && !has_initialized_final_update()) || is_stable)) { 1.9 if (!this->is_static()) { 1.10 // A field can be constant if it's a final static field or if 1.11 // it's a final non-static field of a trusted class (classes in
2.1 --- a/src/share/vm/ci/ciField.hpp Thu Dec 08 15:12:58 2016 +0300 2.2 +++ b/src/share/vm/ci/ciField.hpp Mon Dec 12 12:53:38 2016 +0100 2.3 @@ -124,22 +124,8 @@ 2.4 return _holder->is_shared() && !is_static(); 2.5 } 2.6 2.7 - // Is this field a constant? 2.8 - // 2.9 - // Clarification: A field is considered constant if: 2.10 - // 1. The field is both static and final 2.11 - // 2. The canonical holder of the field has undergone 2.12 - // static initialization. 2.13 - // 3. If the field is an object or array, then the oop 2.14 - // in question is allocated in perm space. 2.15 - // 4. The field is not one of the special static/final 2.16 - // non-constant fields. These are java.lang.System.in 2.17 - // and java.lang.System.out. Abomination. 2.18 - // 2.19 - // A field is also considered constant if it is marked @Stable 2.20 - // and is non-null (or non-zero, if a primitive). 2.21 - // For non-static fields, the null/zero check must be 2.22 - // arranged by the user, as constant_value().is_null_or_zero(). 2.23 + // Is this field a constant? See ciField::initialize_from() for details 2.24 + // about how a field is determined to be constant. 2.25 bool is_constant() { return _is_constant; } 2.26 2.27 // Get the constant value of this field. 2.28 @@ -176,6 +162,9 @@ 2.29 bool is_stable () { return flags().is_stable(); } 2.30 bool is_volatile () { return flags().is_volatile(); } 2.31 bool is_transient () { return flags().is_transient(); } 2.32 + // The field is modified outside of instance initializer methods 2.33 + // (or class/initializer methods if the field is static). 2.34 + bool has_initialized_final_update() { return flags().has_initialized_final_update(); } 2.35 2.36 bool is_call_site_target() { 2.37 ciInstanceKlass* callsite_klass = CURRENT_ENV->CallSite_klass();
3.1 --- a/src/share/vm/ci/ciFlags.hpp Thu Dec 08 15:12:58 2016 +0300 3.2 +++ b/src/share/vm/ci/ciFlags.hpp Mon Dec 12 12:53:38 2016 +0100 3.3 @@ -46,20 +46,25 @@ 3.4 3.5 public: 3.6 // Java access flags 3.7 - bool is_public () const { return (_flags & JVM_ACC_PUBLIC ) != 0; } 3.8 - bool is_private () const { return (_flags & JVM_ACC_PRIVATE ) != 0; } 3.9 - bool is_protected () const { return (_flags & JVM_ACC_PROTECTED ) != 0; } 3.10 - bool is_static () const { return (_flags & JVM_ACC_STATIC ) != 0; } 3.11 - bool is_final () const { return (_flags & JVM_ACC_FINAL ) != 0; } 3.12 - bool is_synchronized() const { return (_flags & JVM_ACC_SYNCHRONIZED) != 0; } 3.13 - bool is_super () const { return (_flags & JVM_ACC_SUPER ) != 0; } 3.14 - bool is_volatile () const { return (_flags & JVM_ACC_VOLATILE ) != 0; } 3.15 - bool is_transient () const { return (_flags & JVM_ACC_TRANSIENT ) != 0; } 3.16 - bool is_native () const { return (_flags & JVM_ACC_NATIVE ) != 0; } 3.17 - bool is_interface () const { return (_flags & JVM_ACC_INTERFACE ) != 0; } 3.18 - bool is_abstract () const { return (_flags & JVM_ACC_ABSTRACT ) != 0; } 3.19 - bool is_strict () const { return (_flags & JVM_ACC_STRICT ) != 0; } 3.20 - bool is_stable () const { return (_flags & JVM_ACC_FIELD_STABLE) != 0; } 3.21 + bool is_public () const { return (_flags & JVM_ACC_PUBLIC ) != 0; } 3.22 + bool is_private () const { return (_flags & JVM_ACC_PRIVATE ) != 0; } 3.23 + bool is_protected () const { return (_flags & JVM_ACC_PROTECTED ) != 0; } 3.24 + bool is_static () const { return (_flags & JVM_ACC_STATIC ) != 0; } 3.25 + bool is_final () const { return (_flags & JVM_ACC_FINAL ) != 0; } 3.26 + bool is_synchronized () const { return (_flags & JVM_ACC_SYNCHRONIZED ) != 0; } 3.27 + bool is_super () const { return (_flags & JVM_ACC_SUPER ) != 0; } 3.28 + bool is_volatile () const { return (_flags & JVM_ACC_VOLATILE ) != 0; } 3.29 + bool is_transient () const { return (_flags & JVM_ACC_TRANSIENT ) != 0; } 3.30 + bool is_native () const { return (_flags & JVM_ACC_NATIVE ) != 0; } 3.31 + bool is_interface () const { return (_flags & JVM_ACC_INTERFACE ) != 0; } 3.32 + bool is_abstract () const { return (_flags & JVM_ACC_ABSTRACT ) != 0; } 3.33 + bool is_strict () const { return (_flags & JVM_ACC_STRICT ) != 0; } 3.34 + bool is_stable () const { return (_flags & JVM_ACC_FIELD_STABLE ) != 0; } 3.35 + // In case the current object represents a field, return true if 3.36 + // the field is modified outside of instance initializer methods 3.37 + // (or class/initializer methods if the field is static) and false 3.38 + // otherwise. 3.39 + bool has_initialized_final_update() const { return (_flags & JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE) != 0; }; 3.40 3.41 // Conversion 3.42 jint as_int() { return _flags; }
4.1 --- a/src/share/vm/interpreter/rewriter.cpp Thu Dec 08 15:12:58 2016 +0300 4.2 +++ b/src/share/vm/interpreter/rewriter.cpp Mon Dec 12 12:53:38 2016 +0100 4.3 @@ -396,10 +396,45 @@ 4.4 break; 4.5 } 4.6 4.7 + case Bytecodes::_putstatic : 4.8 + case Bytecodes::_putfield : { 4.9 + if (!reverse) { 4.10 + // Check if any final field of the class given as parameter is modified 4.11 + // outside of initializer methods of the class. Fields that are modified 4.12 + // are marked with a flag. For marked fields, the compilers do not perform 4.13 + // constant folding (as the field can be changed after initialization). 4.14 + // 4.15 + // The check is performed after verification and only if verification has 4.16 + // succeeded. Therefore, the class is guaranteed to be well-formed. 4.17 + InstanceKlass* klass = method->method_holder(); 4.18 + u2 bc_index = Bytes::get_Java_u2(bcp + prefix_length + 1); 4.19 + constantPoolHandle cp(method->constants()); 4.20 + Symbol* field_name = cp->name_ref_at(bc_index); 4.21 + Symbol* field_sig = cp->signature_ref_at(bc_index); 4.22 + Symbol* ref_class_name = cp->klass_name_at(cp->klass_ref_index_at(bc_index)); 4.23 + 4.24 + if (klass->name() == ref_class_name) { 4.25 + fieldDescriptor fd; 4.26 + klass->find_field(field_name, field_sig, &fd); 4.27 + if (fd.access_flags().is_final()) { 4.28 + if (fd.access_flags().is_static()) { 4.29 + assert(c == Bytecodes::_putstatic, "must be putstatic"); 4.30 + if (!method->is_static_initializer()) { 4.31 + fd.set_has_initialized_final_update(true); 4.32 + } 4.33 + } else { 4.34 + assert(c == Bytecodes::_putfield, "must be putfield"); 4.35 + if (!method->is_object_initializer()) { 4.36 + fd.set_has_initialized_final_update(true); 4.37 + } 4.38 + } 4.39 + } 4.40 + } 4.41 + } 4.42 + } 4.43 + // fall through 4.44 case Bytecodes::_getstatic : // fall through 4.45 - case Bytecodes::_putstatic : // fall through 4.46 case Bytecodes::_getfield : // fall through 4.47 - case Bytecodes::_putfield : // fall through 4.48 case Bytecodes::_invokevirtual : // fall through 4.49 case Bytecodes::_invokestatic : 4.50 case Bytecodes::_invokeinterface:
5.1 --- a/src/share/vm/oops/method.cpp Thu Dec 08 15:12:58 2016 +0300 5.2 +++ b/src/share/vm/oops/method.cpp Mon Dec 12 12:53:38 2016 +0100 5.3 @@ -590,7 +590,7 @@ 5.4 } 5.5 5.6 bool Method::is_initializer() const { 5.7 - return name() == vmSymbols::object_initializer_name() || is_static_initializer(); 5.8 + return is_object_initializer() || is_static_initializer(); 5.9 } 5.10 5.11 bool Method::has_valid_initializer_flags() const { 5.12 @@ -606,6 +606,9 @@ 5.13 has_valid_initializer_flags(); 5.14 } 5.15 5.16 +bool Method::is_object_initializer() const { 5.17 + return name() == vmSymbols::object_initializer_name(); 5.18 +} 5.19 5.20 objArrayHandle Method::resolved_checked_exceptions_impl(Method* this_oop, TRAPS) { 5.21 int length = this_oop->checked_exceptions_length();
6.1 --- a/src/share/vm/oops/method.hpp Thu Dec 08 15:12:58 2016 +0300 6.2 +++ b/src/share/vm/oops/method.hpp Mon Dec 12 12:53:38 2016 +0100 6.3 @@ -627,6 +627,9 @@ 6.4 // valid static initializer flags. 6.5 bool is_static_initializer() const; 6.6 6.7 + // returns true if the method name is <init> 6.8 + bool is_object_initializer() const; 6.9 + 6.10 // compiled code support 6.11 // NOTE: code() is inherently racy as deopt can be clearing code 6.12 // simultaneously. Use with caution.
7.1 --- a/src/share/vm/runtime/fieldDescriptor.hpp Thu Dec 08 15:12:58 2016 +0300 7.2 +++ b/src/share/vm/runtime/fieldDescriptor.hpp Mon Dec 12 12:53:38 2016 +0100 7.3 @@ -106,6 +106,7 @@ 7.4 bool is_field_access_watched() const { return access_flags().is_field_access_watched(); } 7.5 bool is_field_modification_watched() const 7.6 { return access_flags().is_field_modification_watched(); } 7.7 + bool has_initialized_final_update() const { return access_flags().has_field_initialized_final_update(); } 7.8 bool has_generic_signature() const { return access_flags().field_has_generic_signature(); } 7.9 7.10 void set_is_field_access_watched(const bool value) { 7.11 @@ -118,6 +119,11 @@ 7.12 update_klass_field_access_flag(); 7.13 } 7.14 7.15 + void set_has_initialized_final_update(const bool value) { 7.16 + _access_flags.set_has_field_initialized_final_update(value); 7.17 + update_klass_field_access_flag(); 7.18 + } 7.19 + 7.20 // Initialization 7.21 void reinitialize(InstanceKlass* ik, int index); 7.22
8.1 --- a/src/share/vm/utilities/accessFlags.hpp Thu Dec 08 15:12:58 2016 +0300 8.2 +++ b/src/share/vm/utilities/accessFlags.hpp Mon Dec 12 12:53:38 2016 +0100 8.3 @@ -76,11 +76,12 @@ 8.4 // These bits must not conflict with any other field-related access flags 8.5 // (e.g., ACC_ENUM). 8.6 // Note that the class-related ACC_ANNOTATION bit conflicts with these flags. 8.7 - JVM_ACC_FIELD_ACCESS_WATCHED = 0x00002000, // field access is watched by JVMTI 8.8 - JVM_ACC_FIELD_MODIFICATION_WATCHED = 0x00008000, // field modification is watched by JVMTI 8.9 - JVM_ACC_FIELD_INTERNAL = 0x00000400, // internal field, same as JVM_ACC_ABSTRACT 8.10 - JVM_ACC_FIELD_STABLE = 0x00000020, // @Stable field, same as JVM_ACC_SYNCHRONIZED 8.11 - JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE = 0x00000800, // field has generic signature 8.12 + JVM_ACC_FIELD_ACCESS_WATCHED = 0x00002000, // field access is watched by JVMTI 8.13 + JVM_ACC_FIELD_MODIFICATION_WATCHED = 0x00008000, // field modification is watched by JVMTI 8.14 + JVM_ACC_FIELD_INTERNAL = 0x00000400, // internal field, same as JVM_ACC_ABSTRACT 8.15 + JVM_ACC_FIELD_STABLE = 0x00000020, // @Stable field, same as JVM_ACC_SYNCHRONIZED and JVM_ACC_SUPER 8.16 + JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE = 0x00000100, // (static) final field updated outside (class) initializer, same as JVM_ACC_NATIVE 8.17 + JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE = 0x00000800, // field has generic signature 8.18 8.19 JVM_ACC_FIELD_INTERNAL_FLAGS = JVM_ACC_FIELD_ACCESS_WATCHED | 8.20 JVM_ACC_FIELD_MODIFICATION_WATCHED | 8.21 @@ -150,6 +151,8 @@ 8.22 bool is_field_access_watched() const { return (_flags & JVM_ACC_FIELD_ACCESS_WATCHED) != 0; } 8.23 bool is_field_modification_watched() const 8.24 { return (_flags & JVM_ACC_FIELD_MODIFICATION_WATCHED) != 0; } 8.25 + bool has_field_initialized_final_update() const 8.26 + { return (_flags & JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE) != 0; } 8.27 bool on_stack() const { return (_flags & JVM_ACC_ON_STACK) != 0; } 8.28 bool is_internal() const { return (_flags & JVM_ACC_FIELD_INTERNAL) != 0; } 8.29 bool is_stable() const { return (_flags & JVM_ACC_FIELD_STABLE) != 0; } 8.30 @@ -229,6 +232,15 @@ 8.31 atomic_clear_bits(JVM_ACC_FIELD_MODIFICATION_WATCHED); 8.32 } 8.33 } 8.34 + 8.35 + void set_has_field_initialized_final_update(const bool value) { 8.36 + if (value) { 8.37 + atomic_set_bits(JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE); 8.38 + } else { 8.39 + atomic_clear_bits(JVM_ACC_FIELD_INITIALIZED_FINAL_UPDATE); 8.40 + } 8.41 + } 8.42 + 8.43 void set_field_has_generic_signature() 8.44 { 8.45 atomic_set_bits(JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE);