5103339: Strengthen NoSafepointVerifier
Summary: Add NSV check at possible safepoint transition or places that could take out locks. Consolidate with clearing unhandled oops.
Reviewed-by: dholmes, rehn
--- a/src/hotspot/share/ci/ciMethod.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/ci/ciMethod.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -111,7 +111,7 @@
_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()) {
--- a/src/hotspot/share/code/nmethod.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/code/nmethod.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -2268,7 +2268,6 @@
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);
--- a/src/hotspot/share/code/vtableStubs.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/code/vtableStubs.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -234,7 +234,8 @@
}
// 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());
--- a/src/hotspot/share/gc/shared/memAllocator.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/gc/shared/memAllocator.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -143,7 +143,6 @@
// 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");
--- a/src/hotspot/share/interpreter/interpreterRuntime.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/interpreter/interpreterRuntime.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -1440,7 +1440,7 @@
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());
}
--- a/src/hotspot/share/oops/objArrayKlass.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/oops/objArrayKlass.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -328,7 +328,7 @@
// 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 @@
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);
}
--- a/src/hotspot/share/oops/typeArrayKlass.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/oops/typeArrayKlass.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -198,13 +198,13 @@
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);
}
--- a/src/hotspot/share/prims/jvmtiExport.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/prims/jvmtiExport.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -2279,7 +2279,9 @@
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 @@
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 @@
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) {
--- a/src/hotspot/share/prims/jvmtiTagMap.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/prims/jvmtiTagMap.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -520,7 +520,7 @@
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;
}
--- a/src/hotspot/share/prims/jvmtiThreadState.inline.hpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/prims/jvmtiThreadState.inline.hpp Wed Aug 14 10:07:00 2019 -0400
@@ -90,7 +90,9 @@
// 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;
}
--- a/src/hotspot/share/runtime/interfaceSupport.inline.hpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/interfaceSupport.inline.hpp Wed Aug 14 10:07:00 2019 -0400
@@ -87,13 +87,16 @@
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
--- a/src/hotspot/share/runtime/jniHandles.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/jniHandles.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -115,8 +115,6 @@
} else {
report_handle_allocation_failure(alloc_failmode, "global");
}
- } else {
- CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
}
return res;
@@ -140,8 +138,6 @@
} else {
report_handle_allocation_failure(alloc_failmode, "weak global");
}
- } else {
- CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
}
return res;
}
--- a/src/hotspot/share/runtime/mutex.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/mutex.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -50,13 +50,6 @@
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 @@
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
--- a/src/hotspot/share/runtime/os.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/os.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -1082,6 +1082,7 @@
} else {
st->print(INTPTR_FORMAT " is pointing into object: " , p2i(addr));
}
+ ResourceMark rm;
o->print_on(st);
return;
}
--- a/src/hotspot/share/runtime/thread.cpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/thread.cpp Wed Aug 14 10:07:00 2019 -0400
@@ -1010,26 +1010,32 @@
return false;
}
-
-#endif
-
-#ifndef PRODUCT
+// Checks safepoint allowed and clears unhandled oops at potential safepoints.
+void Thread::check_possible_safepoint() {
+ if (!is_Java_thread()) return;
+
+ if (_no_safepoint_count > 0) {
+ fatal("Possible safepoint reached by thread that does not allow it");
+ }
+#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 (_no_safepoint_count > 0) {
- fatal("Possible safepoint reached by thread that does not allow it");
- }
- if (is_Java_thread() && ((JavaThread*)this)->thread_state() != _thread_in_vm) {
+ if (!is_Java_thread()) return;
+
+ check_possible_safepoint();
+
+ if (((JavaThread*)this)->thread_state() != _thread_in_vm) {
fatal("LEAF method calling lock?");
}
-#ifdef ASSERT
- if (potential_vm_operation && is_Java_thread()
- && !Universe::is_bootstrapping()) {
+ if (potential_vm_operation && !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 @@
// 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");
--- a/src/hotspot/share/runtime/thread.hpp Wed Aug 14 11:14:54 2019 +0100
+++ b/src/hotspot/share/runtime/thread.hpp Wed Aug 14 10:07:00 2019 -0400
@@ -367,7 +367,7 @@
void set_missed_ic_stub_refill_verifier(ICRefillVerifier* verifier) {
_missed_ic_stub_refill_verifier = verifier;
}
-#endif
+#endif // ASSERT
private:
@@ -381,12 +381,13 @@
//
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 @@
// 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 @@
#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) {