5103339: Strengthen NoSafepointVerifier

Add NSV check at possible safepoint transition or places that could take out locks.  Consolidate with clearing unhandled oops.

Reviewed-by: dholmes, rehn

src/hotspot/share/ci/ciMethod.cpp

@@ -111,7 +111,7 @@ ciMethod::ciMethod(const methodHandle& h_m, ciInstanceKlass* holder) :
       _can_be_parsed = false;
     }
   } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
+    DEBUG_ONLY(CompilerThread::current()->check_possible_safepoint());
   }
 
   if (h_m()->method_holder()->is_linked()) {

src/hotspot/share/code/nmethod.cpp

@@ -2268,7 +2268,6 @@ void nmethod::verify_interrupt_point(address call_site) {
   if (!is_not_installed()) {
     if (CompiledICLocker::is_safe(this)) {
       CompiledIC_at(this, call_site);
-      CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
     } else {
       CompiledICLocker ml_verify(this);
       CompiledIC_at(this, call_site);

src/hotspot/share/code/vtableStubs.cpp

@@ -234,7 +234,8 @@ address VtableStubs::find_stub(bool is_vtable_stub, int vtable_index) {
       }
       // Notify JVMTI about this stub. The event will be recorded by the enclosing
       // JvmtiDynamicCodeEventCollector and posted when this thread has released
-      // all locks.
+      // all locks. Only post this event if a new state is not required. Creating a new state would
+      // cause a safepoint and the caller of this code has a NoSafepointVerifier.
       if (JvmtiExport::should_post_dynamic_code_generated()) {
         JvmtiExport::post_dynamic_code_generated_while_holding_locks(is_vtable_stub? "vtable stub": "itable stub",
                                                                      s->code_begin(), s->code_end());

src/hotspot/share/gc/shared/memAllocator.cpp

@@ -143,7 +143,6 @@ void MemAllocator::Allocation::verify_before() {
   // Clear unhandled oops for memory allocation.  Memory allocation might
   // not take out a lock if from tlab, so clear here.
   Thread* THREAD = _thread;
-  CHECK_UNHANDLED_OOPS_ONLY(THREAD->clear_unhandled_oops();)
   assert(!HAS_PENDING_EXCEPTION, "Should not allocate with exception pending");
   debug_only(check_for_valid_allocation_state());
   assert(!Universe::heap()->is_gc_active(), "Allocation during gc not allowed");

src/hotspot/share/interpreter/interpreterRuntime.cpp

@@ -1440,7 +1440,7 @@ void SignatureHandlerLibrary::add(const methodHandle& method) {
         method->set_signature_handler(_handlers->at(handler_index));
       }
     } else {
-      CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
+      DEBUG_ONLY(Thread::current()->check_possible_safepoint());
       // use generic signature handler
       method->set_signature_handler(Interpreter::slow_signature_handler());
     }

src/hotspot/share/oops/objArrayKlass.cpp

@@ -328,7 +328,7 @@ Klass* ObjArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
 
   // lock-free read needs acquire semantics
   if (higher_dimension_acquire() == NULL) {
-    if (or_null)  return NULL;
+    if (or_null) return NULL;
 
     ResourceMark rm;
     JavaThread *jt = (JavaThread *)THREAD;
@@ -349,14 +349,13 @@ Klass* ObjArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
         assert(ak->is_objArray_klass(), "incorrect initialization of ObjArrayKlass");
       }
     }
-  } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
   }
 
   ObjArrayKlass *ak = ObjArrayKlass::cast(higher_dimension());
   if (or_null) {
     return ak->array_klass_or_null(n);
   }
+  THREAD->check_possible_safepoint();
   return ak->array_klass(n, THREAD);
 }
 

src/hotspot/share/oops/typeArrayKlass.cpp

@@ -198,13 +198,13 @@ Klass* TypeArrayKlass::array_klass_impl(bool or_null, int n, TRAPS) {
         assert(h_ak->is_objArray_klass(), "incorrect initialization of ObjArrayKlass");
       }
     }
-  } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
   }
+
   ObjArrayKlass* h_ak = ObjArrayKlass::cast(higher_dimension());
   if (or_null) {
     return h_ak->array_klass_or_null(n);
   }
+  THREAD->check_possible_safepoint();
   return h_ak->array_klass(n, THREAD);
 }
 

src/hotspot/share/prims/jvmtiExport.cpp

@@ -2279,7 +2279,9 @@ void JvmtiExport::post_dynamic_code_generated_while_holding_locks(const char* na
                                                                   address code_begin, address code_end)
 {
   // register the stub with the current dynamic code event collector
-  JvmtiThreadState* state = JvmtiThreadState::state_for(JavaThread::current());
+  // Cannot take safepoint here so do not use state_for to get
+  // jvmti thread state.
+  JvmtiThreadState* state = JavaThread::current()->jvmti_thread_state();
   // state can only be NULL if the current thread is exiting which
   // should not happen since we're trying to post an event
   guarantee(state != NULL, "attempt to register stub via an exiting thread");
@@ -2294,7 +2296,7 @@ void JvmtiExport::record_vm_internal_object_allocation(oop obj) {
   if (thread != NULL && thread->is_Java_thread())  {
     // Can not take safepoint here.
     NoSafepointVerifier no_sfpt;
-    // Can not take safepoint here so can not use state_for to get
+    // Cannot take safepoint here so do not use state_for to get
     // jvmti thread state.
     JvmtiThreadState *state = ((JavaThread*)thread)->jvmti_thread_state();
     if (state != NULL) {
@@ -2318,7 +2320,7 @@ void JvmtiExport::record_sampled_internal_object_allocation(oop obj) {
   if (thread != NULL && thread->is_Java_thread())  {
     // Can not take safepoint here.
     NoSafepointVerifier no_sfpt;
-    // Can not take safepoint here so can not use state_for to get
+    // Cannot take safepoint here so do not use state_for to get
     // jvmti thread state.
     JvmtiThreadState *state = ((JavaThread*)thread)->jvmti_thread_state();
     if (state != NULL) {

src/hotspot/share/prims/jvmtiTagMap.cpp

@@ -520,7 +520,7 @@ JvmtiTagMap* JvmtiTagMap::tag_map_for(JvmtiEnv* env) {
       tag_map = new JvmtiTagMap(env);
     }
   } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
+    DEBUG_ONLY(Thread::current()->check_possible_safepoint());
   }
   return tag_map;
 }

src/hotspot/share/prims/jvmtiThreadState.inline.hpp

@@ -90,7 +90,9 @@ inline JvmtiThreadState* JvmtiThreadState::state_for(JavaThread *thread) {
     // check again with the lock held
     state = state_for_while_locked(thread);
   } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
+    // Check possible safepoint even if state is non-null.
+    // (Note: the thread argument isn't the current thread)
+    DEBUG_ONLY(JavaThread::current()->check_possible_safepoint());
   }
   return state;
 }

src/hotspot/share/runtime/interfaceSupport.inline.hpp

@@ -87,13 +87,16 @@ class ThreadStateTransition : public StackObj {
     assert(from != _thread_in_native, "use transition_from_native");
     assert((from & 1) == 0 && (to & 1) == 0, "odd numbers are transitions states");
     assert(thread->thread_state() == from, "coming from wrong thread state");
+
+    // Check NoSafepointVerifier
+    // This also clears unhandled oops if CheckUnhandledOops is used.
+    thread->check_possible_safepoint();
+
     // Change to transition state and ensure it is seen by the VM thread.
     thread->set_thread_state_fence((JavaThreadState)(from + 1));
 
     SafepointMechanism::block_if_requested(thread);
     thread->set_thread_state(to);
-
-    CHECK_UNHANDLED_OOPS_ONLY(thread->clear_unhandled_oops();)
   }
 
   // Same as above, but assumes from = _thread_in_Java. This is simpler, since we

src/hotspot/share/runtime/jniHandles.cpp

@@ -115,8 +115,6 @@ jobject JNIHandles::make_global(Handle obj, AllocFailType alloc_failmode) {
     } else {
       report_handle_allocation_failure(alloc_failmode, "global");
     }
-  } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
   }
 
   return res;
@@ -140,8 +138,6 @@ jobject JNIHandles::make_weak_global(Handle obj, AllocFailType alloc_failmode) {
     } else {
       report_handle_allocation_failure(alloc_failmode, "weak global");
     }
-  } else {
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
   }
   return res;
 }

src/hotspot/share/runtime/mutex.cpp

@@ -50,13 +50,6 @@ void Monitor::check_safepoint_state(Thread* thread, bool do_safepoint_check) {
 void Monitor::lock(Thread * self) {
   check_safepoint_state(self, true);
 
-#ifdef CHECK_UNHANDLED_OOPS
-  // Clear unhandled oops in JavaThreads so we get a crash right away.
-  if (self->is_active_Java_thread()) {
-    self->clear_unhandled_oops();
-  }
-#endif // CHECK_UNHANDLED_OOPS
-
   DEBUG_ONLY(check_prelock_state(self, true));
   assert(_owner != self, "invariant");
 
@@ -196,11 +189,6 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
   guarantee(self->is_active_Java_thread(), "invariant");
   assert_wait_lock_state(self);
 
-#ifdef CHECK_UNHANDLED_OOPS
-  // Clear unhandled oops in JavaThreads so we get a crash right away.
-  self->clear_unhandled_oops();
-#endif // CHECK_UNHANDLED_OOPS
-
   int wait_status;
   // conceptually set the owner to NULL in anticipation of
   // abdicating the lock in wait

src/hotspot/share/runtime/os.cpp

@@ -1082,6 +1082,7 @@ void os::print_location(outputStream* st, intptr_t x, bool verbose) {
       } else {
         st->print(INTPTR_FORMAT " is pointing into object: " , p2i(addr));
       }
+      ResourceMark rm;
       o->print_on(st);
       return;
     }

src/hotspot/share/runtime/thread.cpp

@@ -1010,26 +1010,32 @@ bool Thread::owns_locks_but_compiled_lock() const {
   return false;
 }
 
+// Checks safepoint allowed and clears unhandled oops at potential safepoints.
+void Thread::check_possible_safepoint() {
+  if (!is_Java_thread()) return;
 
-#endif
+  if (_no_safepoint_count > 0) {
-
+    fatal("Possible safepoint reached by thread that does not allow it");
-#ifndef PRODUCT
+  }
+#ifdef CHECK_UNHANDLED_OOPS
+  // Clear unhandled oops in JavaThreads so we get a crash right away.
+  clear_unhandled_oops();
+#endif // CHECK_UNHANDLED_OOPS
+}
 
 // The flag: potential_vm_operation notifies if this particular safepoint state could potentially
 // invoke the vm-thread (e.g., an oop allocation). In that case, we also have to make sure that
 // no locks which allow_vm_block's are held
 void Thread::check_for_valid_safepoint_state(bool potential_vm_operation) {
-  // Check if current thread is allowed to block at a safepoint
+  if (!is_Java_thread()) return;
-  if (_no_safepoint_count > 0) {
+
-    fatal("Possible safepoint reached by thread that does not allow it");
+  check_possible_safepoint();
-  }
+
-  if (is_Java_thread() && ((JavaThread*)this)->thread_state() != _thread_in_vm) {
+  if (((JavaThread*)this)->thread_state() != _thread_in_vm) {
     fatal("LEAF method calling lock?");
   }
 
-#ifdef ASSERT
+  if (potential_vm_operation && !Universe::is_bootstrapping()) {
-  if (potential_vm_operation && is_Java_thread()
-      && !Universe::is_bootstrapping()) {
     // Make sure we do not hold any locks that the VM thread also uses.
     // This could potentially lead to deadlocks
     for (Monitor *cur = _owned_locks; cur; cur = cur->next()) {
@@ -1052,9 +1058,8 @@ void Thread::check_for_valid_safepoint_state(bool potential_vm_operation) {
     // We could enter a safepoint here and thus have a gc
     InterfaceSupport::check_gc_alot();
   }
-#endif
 }
-#endif
+#endif // ASSERT
 
 bool Thread::is_in_stack(address adr) const {
   assert(Thread::current() == this, "is_in_stack can only be called from current thread");

src/hotspot/share/runtime/thread.hpp

@@ -367,7 +367,7 @@ class Thread: public ThreadShadow {
   void set_missed_ic_stub_refill_verifier(ICRefillVerifier* verifier) {
     _missed_ic_stub_refill_verifier = verifier;
   }
-#endif
+#endif // ASSERT
 
  private:
 
@@ -381,12 +381,13 @@ class Thread: public ThreadShadow {
   //
   NOT_PRODUCT(int _no_safepoint_count;)         // If 0, thread allow a safepoint to happen
 
+ private:
   // Used by SkipGCALot class.
   NOT_PRODUCT(bool _skip_gcalot;)               // Should we elide gc-a-lot?
 
+  friend class GCLocker;
   friend class NoSafepointVerifier;
   friend class PauseNoSafepointVerifier;
-  friend class GCLocker;
 
   volatile void* _polling_page;                 // Thread local polling page
 
@@ -757,9 +758,12 @@ protected:
   // Deadlock detection
   ResourceMark* current_resource_mark()          { return _current_resource_mark; }
   void set_current_resource_mark(ResourceMark* rm) { _current_resource_mark = rm; }
-#endif
+#endif // ASSERT
 
-  void check_for_valid_safepoint_state(bool potential_vm_operation) PRODUCT_RETURN;
+  // These functions check conditions on a JavaThread before possibly going to a safepoint,
+  // including NoSafepointVerifier.
+  void check_for_valid_safepoint_state(bool potential_vm_operation) NOT_DEBUG_RETURN;
+  void check_possible_safepoint() NOT_DEBUG_RETURN;
 
  private:
   volatile int _jvmti_env_iteration_count;
@@ -1221,7 +1225,7 @@ class JavaThread: public Thread {
 #ifdef ASSERT
   // verify this JavaThread hasn't be published in the Threads::list yet
   void verify_not_published();
-#endif
+#endif // ASSERT
 
   //JNI functiontable getter/setter for JVMTI jni function table interception API.
   void set_jni_functions(struct JNINativeInterface_* functionTable) {