8157181: Compilers accept modification of final fields outside initializer methods

Mon, 12 Dec 2016 12:53:38 +0100

author
zmajo
date
Mon, 12 Dec 2016 12:53:38 +0100
changeset 8664
00cbb581da94
parent 8658
f8a5d01c0929
child 8665
8cc2e2729cce

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 file | annotate | diff | comparison | revisions
src/share/vm/ci/ciField.hpp file | annotate | diff | comparison | revisions
src/share/vm/ci/ciFlags.hpp file | annotate | diff | comparison | revisions
src/share/vm/interpreter/rewriter.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/method.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/method.hpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/fieldDescriptor.hpp file | annotate | diff | comparison | revisions
src/share/vm/utilities/accessFlags.hpp file | annotate | diff | comparison | revisions
     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);

mercurial