8226690: SIGSEGV in MetadataOnStackClosure::do_metadata

Dont create nmethod if classes have been redefined since compilation start.

Reviewed-by: sspitsyn, dlong, eosterlund, gdub

Author: coleenp

src/hotspot/share/ci/ciEnv.cpp

@@ -154,6 +154,7 @@ ciEnv::ciEnv(CompileTask* task)
   _the_null_string = NULL;
   _the_min_jint_string = NULL;
 
+  _jvmti_redefinition_count = 0;
   _jvmti_can_hotswap_or_post_breakpoint = false;
   _jvmti_can_access_local_variables = false;
   _jvmti_can_post_on_exceptions = false;
@@ -209,6 +210,7 @@ ciEnv::ciEnv(Arena* arena) : _ciEnv_arena(mtCompiler) {
   _the_null_string = NULL;
   _the_min_jint_string = NULL;
 
+  _jvmti_redefinition_count = 0;
   _jvmti_can_hotswap_or_post_breakpoint = false;
   _jvmti_can_access_local_variables = false;
   _jvmti_can_post_on_exceptions = false;
@@ -231,13 +233,19 @@ void ciEnv::cache_jvmti_state() {
   VM_ENTRY_MARK;
   // Get Jvmti capabilities under lock to get consistant values.
   MutexLocker mu(JvmtiThreadState_lock);
+  _jvmti_redefinition_count             = JvmtiExport::redefinition_count();
   _jvmti_can_hotswap_or_post_breakpoint = JvmtiExport::can_hotswap_or_post_breakpoint();
   _jvmti_can_access_local_variables     = JvmtiExport::can_access_local_variables();
   _jvmti_can_post_on_exceptions         = JvmtiExport::can_post_on_exceptions();
   _jvmti_can_pop_frame                  = JvmtiExport::can_pop_frame();
 }
 
 bool ciEnv::jvmti_state_changed() const {
+  // Some classes were redefined
+  if (_jvmti_redefinition_count != JvmtiExport::redefinition_count()) {
+    return true;
+  }
+
   if (!_jvmti_can_access_local_variables &&
       JvmtiExport::can_access_local_variables()) {
     return true;
@@ -254,6 +262,7 @@ bool ciEnv::jvmti_state_changed() const {
       JvmtiExport::can_pop_frame()) {
     return true;
   }
+
   return false;
 }
 

src/hotspot/share/ci/ciEnv.hpp

@@ -68,6 +68,7 @@ class ciEnv : StackObj {
   int   _name_buffer_len;
 
   // Cache Jvmti state
+  uint64_t _jvmti_redefinition_count;
   bool  _jvmti_can_hotswap_or_post_breakpoint;
   bool  _jvmti_can_access_local_variables;
   bool  _jvmti_can_post_on_exceptions;

src/hotspot/share/code/nmethod.cpp

@@ -2192,6 +2192,17 @@ class VerifyOopsClosure: public OopClosure {
   virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); }
 };
 
+class VerifyMetadataClosure: public MetadataClosure {
+ public:
+  void do_metadata(Metadata* md) {
+    if (md->is_method()) {
+      Method* method = (Method*)md;
+      assert(!method->is_old(), "Should not be installing old methods");
+    }
+  }
+};
+
+
 void nmethod::verify() {
 
   // Hmm. OSR methods can be deopted but not marked as zombie or not_entrant
@@ -2255,6 +2266,10 @@ void nmethod::verify() {
   Universe::heap()->verify_nmethod(this);
 
   verify_scopes();
+
+  CompiledICLocker nm_verify(this);
+  VerifyMetadataClosure vmc;
+  metadata_do(&vmc);
 }
 
 

src/hotspot/share/jvmci/jvmciEnv.cpp

@@ -44,13 +44,18 @@ JVMCICompileState::JVMCICompileState(CompileTask* task):
   _failure_reason_on_C_heap(false) {
   // Get Jvmti capabilities under lock to get consistent values.
   MutexLocker mu(JvmtiThreadState_lock);
+  _jvmti_redefinition_count             = JvmtiExport::redefinition_count();
   _jvmti_can_hotswap_or_post_breakpoint = JvmtiExport::can_hotswap_or_post_breakpoint() ? 1 : 0;
   _jvmti_can_access_local_variables     = JvmtiExport::can_access_local_variables() ? 1 : 0;
   _jvmti_can_post_on_exceptions         = JvmtiExport::can_post_on_exceptions() ? 1 : 0;
   _jvmti_can_pop_frame                  = JvmtiExport::can_pop_frame() ? 1 : 0;
 }
 
 bool JVMCICompileState::jvmti_state_changed() const {
+  // Some classes were redefined
+  if (jvmti_redefinition_count() != JvmtiExport::redefinition_count()) {
+    return true;
+  }
   if (!jvmti_can_access_local_variables() &&
       JvmtiExport::can_access_local_variables()) {
     return true;

src/hotspot/share/jvmci/jvmciEnv.hpp

@@ -94,6 +94,7 @@ class JVMCICompileState : public ResourceObj {
   // Cache JVMTI state. Defined as bytes so that reading them from Java
   // via Unsafe is well defined (the C++ type for bool is implementation
   // defined and may not be the same as a Java boolean).
+  uint64_t _jvmti_redefinition_count;
   jbyte  _jvmti_can_hotswap_or_post_breakpoint;
   jbyte  _jvmti_can_access_local_variables;
   jbyte  _jvmti_can_post_on_exceptions;
@@ -113,6 +114,7 @@ class JVMCICompileState : public ResourceObj {
   CompileTask* task() { return _task; }
 
   bool  jvmti_state_changed() const;
+  uint64_t jvmti_redefinition_count() const          { return  _jvmti_redefinition_count; }
   bool  jvmti_can_hotswap_or_post_breakpoint() const { return  _jvmti_can_hotswap_or_post_breakpoint != 0; }
   bool  jvmti_can_access_local_variables() const     { return  _jvmti_can_access_local_variables != 0; }
   bool  jvmti_can_post_on_exceptions() const         { return  _jvmti_can_post_on_exceptions != 0; }

src/hotspot/share/prims/jvmtiExport.cpp

@@ -304,7 +304,7 @@ bool              JvmtiExport::_can_hotswap_or_post_breakpoint            = fals
 bool              JvmtiExport::_can_modify_any_class                      = false;
 bool              JvmtiExport::_can_walk_any_space                        = false;
 
-bool              JvmtiExport::_has_redefined_a_class                     = false;
+uint64_t          JvmtiExport::_redefinition_count                        = 0;
 bool              JvmtiExport::_all_dependencies_are_recorded             = false;
 
 //

src/hotspot/share/prims/jvmtiExport.hpp

@@ -173,10 +173,10 @@ class JvmtiExport : public AllStatic {
   // one or more classes during the lifetime of the VM. The flag should
   // only be set by the friend class and can be queried by other sub
   // systems as needed to relax invariant checks.
-  static bool _has_redefined_a_class;
+  static uint64_t _redefinition_count;
   friend class VM_RedefineClasses;
-  inline static void set_has_redefined_a_class() {
-    JVMTI_ONLY(_has_redefined_a_class = true;)
+  inline static void increment_redefinition_count() {
+    JVMTI_ONLY(_redefinition_count++;)
   }
   // Flag to indicate if the compiler has recorded all dependencies. When the
   // can_redefine_classes capability is enabled in the OnLoad phase then the compiler
@@ -188,10 +188,13 @@ class JvmtiExport : public AllStatic {
 
  public:
   inline static bool has_redefined_a_class() {
-    JVMTI_ONLY(return _has_redefined_a_class);
+    JVMTI_ONLY(return _redefinition_count != 0);
     NOT_JVMTI(return false);
   }
 
+  // Only set in safepoint, so no memory ordering needed.
+  inline static uint64_t redefinition_count() { return _redefinition_count; }
+
   inline static bool all_dependencies_are_recorded() {
     return _all_dependencies_are_recorded;
   }

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -232,9 +232,9 @@ void VM_RedefineClasses::doit() {
     ResolvedMethodTable::adjust_method_entries(&trace_name_printed);
   }
 
-  // Set flag indicating that some invariants are no longer true.
+  // Increment flag indicating that some invariants are no longer true.
   // See jvmtiExport.hpp for detailed explanation.
-  JvmtiExport::set_has_redefined_a_class();
+  JvmtiExport::increment_redefinition_count();
 
   // check_class() is optionally called for product bits, but is
   // always called for non-product bits.

src/hotspot/share/runtime/sharedRuntime.cpp

@@ -1269,6 +1269,7 @@ bool SharedRuntime::resolve_sub_helper_internal(methodHandle callee_method, cons
     // will be supported.
     if (!callee_method->is_old() &&
         (callee == NULL || (callee->is_in_use() && callee_method->code() == callee))) {
+      NoSafepointVerifier nsv;
 #ifdef ASSERT
       // We must not try to patch to jump to an already unloaded method.
       if (dest_entry_point != 0) {