8349945: Implement strict static fields (proposed JVM feature)

src/hotspot/share/c1/c1_Runtime1.cpp

@@ -1112,6 +1112,7 @@ JRT_ENTRY(void, Runtime1::patch_code(JavaThread* current, C1StubId stub_id ))
   bool deoptimize_for_atomic = false;
   bool deoptimize_for_null_free = false;
   bool deoptimize_for_flat = false;
+  bool deoptimize_for_strict_static = false;
   int patch_field_offset = -1;
   Klass* init_klass = nullptr; // klass needed by load_klass_patching code
   Klass* load_klass = nullptr; // klass needed by load_klass_patching code
@@ -1165,6 +1166,9 @@ JRT_ENTRY(void, Runtime1::patch_code(JavaThread* current, C1StubId stub_id ))
     // field because it was unknown at compile time.
     deoptimize_for_flat = result.is_flat();
 
+    // Strict statics may require tracking if their class is not fully initialized.
+    // For now we can bail out of the compiler and let the interpreter handle it.
+    deoptimize_for_strict_static = result.is_strict_static_unset();
   } else if (load_klass_or_mirror_patch_id) {
     Klass* k = nullptr;
     switch (code) {
@@ -1237,7 +1241,11 @@ JRT_ENTRY(void, Runtime1::patch_code(JavaThread* current, C1StubId stub_id ))
     ShouldNotReachHere();
   }
 
-  if (deoptimize_for_volatile || deoptimize_for_atomic || deoptimize_for_null_free || deoptimize_for_flat) {
+  if (deoptimize_for_volatile  ||
+      deoptimize_for_atomic    ||
+      deoptimize_for_null_free ||
+      deoptimize_for_flat      ||
+      deoptimize_for_strict_static) {
     // At compile time we assumed the field wasn't volatile/atomic but after
     // loading it turns out it was volatile/atomic so we have to throw the
     // compiled code out and let it be regenerated.
@@ -1254,6 +1262,9 @@ JRT_ENTRY(void, Runtime1::patch_code(JavaThread* current, C1StubId stub_id ))
       if (deoptimize_for_flat) {
         tty->print_cr("Deoptimizing for patching flat field reference");
       }
+      if (deoptimize_for_strict_static) {
+        tty->print_cr("Deoptimizing for patching strict static field reference");
+      }
     }
 
     // It's possible the nmethod was invalidated in the last

src/hotspot/share/ci/ciField.cpp

@@ -479,6 +479,18 @@ bool ciField::will_link(ciMethod* accessing_method,
   fieldDescriptor result;
   LinkResolver::resolve_field(result, link_info, bc, false, CHECK_AND_CLEAR_(false));
 
+  // Strict statics may require tracking if their class is not fully initialized.
+  // For now we can bail out of the compiler and let the interpreter handle it.
+  if (is_static && result.is_strict_static_unset()) {
+    // If we left out this logic, we would get (a) spurious <clinit>
+    // failures for C2 code because compiled putstatic would not write
+    // the "unset" bits, and (b) missed failures for too-early reads,
+    // since the compiled getstatic would not check the "unset" bits.
+    // Test C1 on <clinit> with "-XX:TieredStopAtLevel=2 -Xcomp -Xbatch".
+    // Test C2 on <clinit> with "-XX:-TieredCompilation -Xcomp -Xbatch".
+    return false;
+  }
+
   // update the hit-cache, unless there is a problem with memory scoping:
   if (accessing_method->holder()->is_shared() || !is_shared()) {
     if (is_put) {

src/hotspot/share/classfile/classFileParser.cpp

@@ -1514,6 +1514,9 @@ void ClassFileParser::parse_fields(const ClassFileStream* const cfs,
     if (fi.field_flags().is_contended()) {
       _has_contended_fields = true;
     }
+    if (access_flags.is_strict() && access_flags.is_static()) {
+      _has_strict_static_fields = true;
+    }
     _temp_field_info->append(fi);
   }
   assert(_temp_field_info->length() == length, "Must be");
@@ -5289,6 +5292,7 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
   // Not yet: supers are done below to support the new subtype-checking fields
   ik->set_nonstatic_field_size(_layout_info->_nonstatic_field_size);
   ik->set_has_nonstatic_fields(_layout_info->_has_nonstatic_fields);
+  ik->set_has_strict_static_fields(_has_strict_static_fields);
 
   if (_layout_info->_is_naturally_atomic) {
     ik->set_is_naturally_atomic();
@@ -5600,6 +5604,7 @@ ClassFileParser::ClassFileParser(ClassFileStream* stream,
   _has_localvariable_table(false),
   _has_final_method(false),
   _has_contended_fields(false),
+  _has_strict_static_fields(false),
   _has_inline_type_fields(false),
   _is_naturally_atomic(false),
   _must_be_atomic(true),
@@ -6238,6 +6243,27 @@ void ClassFileParser::post_process_parsed_stream(const ClassFileStream* const st
   _fields_status =
     MetadataFactory::new_array<FieldStatus>(_loader_data, _temp_field_info->length(),
                                             FieldStatus(0), CHECK);
+
+  // Strict static fields track initialization status from the beginning of time.
+  // After this class runs <clinit>, they will be verified as being "not unset".
+  // See Step 8 of InstanceKlass::initialize_impl.
+  if (_has_strict_static_fields) {
+    bool found_one = false;
+    for (int i = 0; i < _temp_field_info->length(); i++) {
+      FieldInfo& fi = *_temp_field_info->adr_at(i);
+      if (fi.access_flags().is_strict() && fi.access_flags().is_static()) {
+        found_one = true;
+        if (fi.initializer_index() != 0) {
+          // skip strict static fields with ConstantValue attributes
+        } else {
+          _fields_status->adr_at(fi.index())->update_strict_static_unset(true);
+          _fields_status->adr_at(fi.index())->update_strict_static_unread(true);
+        }
+      }
+    }
+    assert(found_one == _has_strict_static_fields,
+           "correct prediction = %d", (int)_has_strict_static_fields);
+  }
 }
 
 void ClassFileParser::set_klass(InstanceKlass* klass) {

src/hotspot/share/classfile/classFileParser.hpp

@@ -208,6 +208,7 @@ class ClassFileParser {
   bool _has_localvariable_table;
   bool _has_final_method;
   bool _has_contended_fields;
+  bool _has_strict_static_fields;
 
   bool _has_inline_type_fields;
   bool _is_naturally_atomic;

src/hotspot/share/classfile/stackMapFrame.cpp

@@ -228,6 +228,8 @@ bool StackMapFrame::is_assignable_to(
   // Check that assert unset fields are compatible
   bool compatible = verify_unset_fields_compatibility(target->assert_unset_fields());
   if (!compatible) {
+    print_strict_fields(assert_unset_fields());
+    print_strict_fields(target->assert_unset_fields());
     *ctx = ErrorContext::strict_fields_mismatch(target->offset(),
         (StackMapFrame*)this, (StackMapFrame*)target);
     return false;

src/hotspot/share/interpreter/interpreterRuntime.cpp

@@ -792,6 +792,7 @@ void InterpreterRuntime::resolve_get_put(Bytecodes::Code bytecode, int field_ind
   bool uninitialized_static = is_static && !klass->is_initialized();
   bool has_initialized_final_update = info.field_holder()->major_version() >= 53 &&
                                       info.has_initialized_final_update();
+  bool strict_static_final = info.is_strict() && info.is_static() && info.is_final();
   assert(!(has_initialized_final_update && !info.access_flags().is_final()), "Fields with initialized final updates must be final");
 
   Bytecodes::Code get_code = (Bytecodes::Code)0;
@@ -805,6 +806,19 @@ void InterpreterRuntime::resolve_get_put(Bytecodes::Code bytecode, int field_ind
     if ((is_put && !has_initialized_final_update) || !info.access_flags().is_final()) {
         put_code = ((is_static) ? Bytecodes::_putstatic : Bytecodes::_putfield);
     }
+    assert(!info.is_strict_static_unset(), "after initialization, no unset flags");
+  } else if (is_static && (info.is_strict_static_unset() || strict_static_final)) {
+    // During <clinit>, closely track the state of strict statics.
+    // 1. if we are reading an uninitialized strict static, throw
+    // 2. if we are writing one, clear the "unset" flag
+    //
+    // Note: If we were handling an attempted write of a null to a
+    // null-restricted strict static, we would NOT clear the "unset"
+    // flag.  This does not happen today, but will with NR fields.
+    assert(klass->is_being_initialized(), "else should have thrown");
+    assert(klass->is_reentrant_initialization(THREAD),
+      "<clinit> must be running in current thread");
+    klass->notify_strict_static_access(info.index(), is_put, CHECK);
   }
 
   ResolvedFieldEntry* entry = pool->resolved_field_entry_at(field_index);

src/hotspot/share/oops/fieldInfo.hpp

@@ -313,6 +313,8 @@ class FieldStatus {
   enum FieldStatusBitPosition {
     _fs_access_watched,       // field access is watched by JVMTI
     _fs_modification_watched, // field modification is watched by JVMTI
+    _fs_strict_static_unset,  // JVM_ACC_STRICT static field has not yet been set
+    _fs_strict_static_unread, // SS field has not yet been read (EnforceStrictStatics=2 only)
     _initialized_final_update // (static) final field updated outside (class) initializer
   };
 
@@ -331,12 +333,16 @@ class FieldStatus {
   FieldStatus(u1 flags) { _flags = flags; }
   u1 as_uint() { return _flags; }
 
-  bool is_access_watched()        { return test_flag(_fs_access_watched); }
-  bool is_modification_watched()  { return test_flag(_fs_modification_watched); }
+  bool is_access_watched()           { return test_flag(_fs_access_watched); }
+  bool is_modification_watched()     { return test_flag(_fs_modification_watched); }
+  bool is_strict_static_unset()      { return test_flag(_fs_strict_static_unset); }
+  bool is_strict_static_unread()     { return test_flag(_fs_strict_static_unread); }
   bool is_initialized_final_update() { return test_flag(_initialized_final_update); }
 
   void update_access_watched(bool z);
   void update_modification_watched(bool z);
+  void update_strict_static_unset(bool z);
+  void update_strict_static_unread(bool z);
   void update_initialized_final_update(bool z);
 };
 

src/hotspot/share/oops/fieldInfo.inline.hpp

@@ -184,8 +184,10 @@ inline void FieldStatus::update_flag(FieldStatusBitPosition pos, bool z) {
   else atomic_clear_bits(_flags, flag_mask(pos));
 }
 
-inline void FieldStatus::update_access_watched(bool z) { update_flag(_fs_access_watched, z); }
-inline void FieldStatus::update_modification_watched(bool z) { update_flag(_fs_modification_watched, z); }
+inline void FieldStatus::update_access_watched(bool z)           { update_flag(_fs_access_watched, z); }
+inline void FieldStatus::update_modification_watched(bool z)     { update_flag(_fs_modification_watched, z); }
+inline void FieldStatus::update_strict_static_unset(bool z)      { update_flag(_fs_strict_static_unset, z); }
+inline void FieldStatus::update_strict_static_unread(bool z)     { update_flag(_fs_strict_static_unread, z); }
 inline void FieldStatus::update_initialized_final_update(bool z) { update_flag(_initialized_final_update, z); }
 
 #endif // SHARE_OOPS_FIELDINFO_INLINE_HPP

src/hotspot/share/oops/instanceKlass.cpp

@@ -1509,6 +1509,36 @@ void InstanceKlass::initialize_impl(TRAPS) {
       }
       call_class_initializer(THREAD);
     }
+
+    if (has_strict_static_fields() && !HAS_PENDING_EXCEPTION) {
+      // Step 9 also verifies that strict static fields have been initialized.
+      // Status bits were set in ClassFileParser::post_process_parsed_stream.
+      // After <clinit>, bits must all be clear, or else we must throw an error.
+      // This is an extremely fast check, so we won't bother with a timer.
+      assert(fields_status() != nullptr, "");
+      Symbol* bad_strict_static = nullptr;
+      for (int index = 0; index < fields_status()->length(); index++) {
+        // Very fast loop over single byte array looking for a set bit.
+        if (fields_status()->adr_at(index)->is_strict_static_unset()) {
+          // This strict static field has not been set by the class initializer.
+          // Note that in the common no-error case, we read no field metadata.
+          // We only unpack it when we need to report an error.
+          FieldInfo fi = field(index);
+          bad_strict_static = fi.name(constants());
+          if (debug_logging_enabled) {
+            ResourceMark rm(jt);
+            const char* msg = format_strict_static_message(bad_strict_static);
+            log_debug(class, init)("%s", msg);
+          } else {
+            // If we are not logging, do not bother to look for a second offense.
+            break;
+          }
+        }
+      }
+      if (bad_strict_static != nullptr) {
+        throw_strict_static_exception(bad_strict_static, "is unset after initialization of", THREAD);
+      }
+    }
   }
 
   // Step 9
@@ -1561,6 +1591,88 @@ void InstanceKlass::set_initialization_state_and_notify(ClassState state, TRAPS)
   }
 }
 
+void InstanceKlass::notify_strict_static_access(int field_index, bool is_writing, TRAPS) {
+  guarantee(field_index >= 0 && field_index < fields_status()->length(), "valid field index");
+  DEBUG_ONLY(FieldInfo debugfi = field(field_index));
+  assert(debugfi.access_flags().is_strict(), "");
+  assert(debugfi.access_flags().is_static(), "");
+  FieldStatus& fs = *fields_status()->adr_at(field_index);
+  LogTarget(Trace, class, init) lt;
+  if (lt.is_enabled()) {
+    ResourceMark rm(THREAD);
+    LogStream ls(lt);
+    FieldInfo fi = field(field_index);
+    ls.print("notify %s %s %s%s ",
+             external_name(), is_writing? "Write" : "Read",
+             fs.is_strict_static_unset() ? "Unset" : "(set)",
+             fs.is_strict_static_unread() ? "+Unread" : "");
+    fi.print(&ls, constants());
+  }
+  if (fs.is_strict_static_unset()) {
+    assert(fs.is_strict_static_unread(), "ClassFileParser resp.");
+    // If it is not set, there are only two reasonable things we can do here:
+    // - mark it set if this is putstatic
+    // - throw an error (Read-Before-Write) if this is getstatic
+    //
+    // A third less-reasonable thing is note an internal error if the
+    // JDK reflection logic has allowed another thread to sneak into
+    // the <clinit> critical section.
+    if (!is_reentrant_initialization(THREAD)) {
+      // The unset state is (or should be) transient, and observable only in one
+      // thread during the execution of <clinit>.  Something is wrong here, and
+      // we should just throw.
+      if (is_in_error_state()) {
+        oop init_error = get_initialization_error(THREAD);
+        if (init_error != nullptr) {
+          THROW_OOP(init_error);
+        }
+      }
+      THROW_MSG(vmSymbols::java_lang_InternalError(), "unscoped access to strict static");
+    } else if (is_writing) {
+      // clear the "unset" bit, since the field is actually going to be written
+      fs.update_strict_static_unset(false);
+    } else {
+      // throw an IllegalStateException, since we are reading before writing
+      // see also InstanceKlass::initialize_impl, Step 8 (at end)
+      Symbol* bad_strict_static = field(field_index).name(constants());
+      throw_strict_static_exception(bad_strict_static, "is unset before first read in", CHECK);
+    }
+  } else {
+    // Experimentally, enforce additional proposed conditions after the first write.
+    FieldInfo fi = field(field_index);
+    bool is_final = fi.access_flags().is_final();
+    if (is_final) {
+      // no final write after read, so observing a constant freezes it, as if <clinit> ended early
+      // (maybe we could trust the constant a little earlier, before <clinit> ends)
+      if (is_writing && !fs.is_strict_static_unread()) {
+        Symbol* bad_strict_static = fi.name(constants());
+        throw_strict_static_exception(bad_strict_static, "is set after read (as final) in", CHECK);
+      } else if (!is_writing && fs.is_strict_static_unread()) {
+        // log the read (this requires an extra status bit, with extra tests on it)
+        fs.update_strict_static_unread(false);
+      }
+    }
+  }
+}
+
+void InstanceKlass::throw_strict_static_exception(Symbol* field_name, const char* when, TRAPS) {
+  ResourceMark rm(THREAD);
+  const char* msg = format_strict_static_message(field_name, when);
+  // FIXME: Maybe replace IllegalStateException with something more precise.
+  // Perhaps a new-fangled UninitializedFieldException?
+  THROW_MSG(vmSymbols::java_lang_IllegalStateException(), msg);
+}
+
+const char* InstanceKlass::format_strict_static_message(Symbol* field_name, const char* when) {
+  stringStream ss;
+  // we can use similar format strings for both -Xlog:class+init and for the ISE throw
+  ss.print("Strict static \"%s\" %s %s",
+           field_name->as_C_string(),
+           when == nullptr ? "is unset in" : when,
+           external_name());
+  return ss.as_string();
+}
+
 // Update hierarchy. This is done before the new klass has been added to the SystemDictionary. The Compile_lock
 // is grabbed, to ensure that the compiler is not using the class hierarchy.
 void InstanceKlass::add_to_hierarchy(JavaThread* current) {

src/hotspot/share/oops/instanceKlass.hpp

@@ -863,6 +863,13 @@ class InstanceKlass: public Klass {
   inline u2 next_method_idnum();
   void set_initial_method_idnum(u2 value)             { _idnum_allocated_count = value; }
 
+  // runtime support for strict statics
+  bool has_strict_static_fields() const     { return _misc_flags.has_strict_static_fields(); }
+  void set_has_strict_static_fields(bool b) { _misc_flags.set_has_strict_static_fields(b); }
+  void notify_strict_static_access(int field_index, bool is_writing, TRAPS);
+  const char* format_strict_static_message(Symbol* field_name, const char* doing_what = nullptr);
+  void throw_strict_static_exception(Symbol* field_name, const char* when, TRAPS);
+
   // generics support
   Symbol* generic_signature() const;
   u2 generic_signature_index() const;

src/hotspot/share/oops/instanceKlassFlags.hpp

@@ -59,6 +59,8 @@ class InstanceKlassFlags {
     flag(is_naturally_atomic                , 1 << 16) /* loaded/stored in one instruction*/ \
     flag(must_be_atomic                     , 1 << 17) /* doesn't allow tearing */ \
     flag(has_loosely_consistent_annotation  , 1 << 18) /* the class has the LooselyConsistentValue annotation WARNING: it doesn't automatically mean that the class allows tearing */ \
+    flag(is_implicitly_constructible        , 1 << 19) /* the class has the ImplicitlyConstrutible annotation */ \
+    flag(has_strict_static_fields           , 1 << 20) /* True if strict static fields declared */ \
     /* end of list */
 
   /* (*) An inline type is considered empty if it contains no non-static fields or

src/hotspot/share/prims/jni.cpp

@@ -2055,8 +2055,11 @@ JNI_ENTRY(jfieldID, jni_GetStaticFieldID(JNIEnv *env, jclass clazz,
   k->initialize(CHECK_NULL);
 
   fieldDescriptor fd;
+  // strict fields need special tracking during <clinit>; do not hand them out so early
   if (!k->is_instance_klass() ||
-      !InstanceKlass::cast(k)->find_field(fieldname, signame, true, &fd)) {
+      !InstanceKlass::cast(k)->find_field(fieldname, signame, true, &fd) ||
+      // strict fields need special tracking during <clinit>; do not hand them out so early
+      (fd.access_flags().is_strict() && !InstanceKlass::cast(k)->is_initialized())) {
     THROW_MSG_NULL(vmSymbols::java_lang_NoSuchFieldError(), (char*) name);
   }
 

src/hotspot/share/prims/unsafe.cpp

@@ -836,6 +836,27 @@ UNSAFE_ENTRY(jboolean, Unsafe_ShouldBeInitialized0(JNIEnv *env, jobject unsafe,
 }
 UNSAFE_END
 
+UNSAFE_ENTRY(void, Unsafe_NotifyStrictStaticAccess0(JNIEnv *env, jobject unsafe, jobject clazz,
+  jlong sfoffset, jboolean writing)) {
+  assert(clazz != nullptr, "clazz must not be null");
+
+  oop mirror = JNIHandles::resolve_non_null(clazz);
+  Klass* klass = java_lang_Class::as_Klass(mirror);
+
+  if (klass != nullptr && klass->is_instance_klass()) {
+    InstanceKlass* ik = InstanceKlass::cast(klass);
+    fieldDescriptor fd;
+    if (ik->find_local_field_from_offset((int)sfoffset, true, &fd)) {
+      // Note: The Unsafe API takes an OFFSET, but the InstanceKlass wants the INDEX.
+      // We could surface field indexes into Unsafe, but that's too much churn.
+      ik->notify_strict_static_access(fd.index(), writing, CHECK);
+      return;
+    }
+  }
+  THROW(vmSymbols::java_lang_InternalError());
+}
+UNSAFE_END
+
 static void getBaseAndScale(int& base, int& scale, jclass clazz, TRAPS) {
   assert(clazz != nullptr, "clazz must not be null");
 
@@ -1191,6 +1212,7 @@ static JNINativeMethod jdk_internal_misc_Unsafe_methods[] = {
     {CC "setMemory0",         CC "(" OBJ "JJB)V",        FN_PTR(Unsafe_SetMemory0)},
 
     {CC "shouldBeInitialized0", CC "(" CLS ")Z",         FN_PTR(Unsafe_ShouldBeInitialized0)},
+    {CC "notifyStrictStaticAccess0", CC "(" CLS "JZ)V",  FN_PTR(Unsafe_NotifyStrictStaticAccess0)},
 
     {CC "fullFence",          CC "()V",                  FN_PTR(Unsafe_FullFence)},
 };