8228630: Remove always true parameter to NoSafepointVerifier

Also remove NoGCVerifier since NoSafepointVerifier covers GC checking when not already at a safepoint and is a stronger check.

Reviewed-by: kbarrett, dholmes

src/hotspot/share/classfile/classLoaderDataGraph.cpp

@@ -309,8 +309,7 @@ class ClassLoaderDataGraphIterator : public StackObj {
                             // unless verifying at a safepoint.
 
 public:
-  ClassLoaderDataGraphIterator() : _next(ClassLoaderDataGraph::_head),
+  ClassLoaderDataGraphIterator() : _next(ClassLoaderDataGraph::_head) {
-     _nsv(true, !SafepointSynchronize::is_at_safepoint()) {
     _thread = Thread::current();
     assert_locked_or_safepoint(ClassLoaderDataGraph_lock);
   }

src/hotspot/share/code/compiledIC.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -52,8 +52,7 @@
 CompiledICLocker::CompiledICLocker(CompiledMethod* method)
   : _method(method),
     _behaviour(CompiledICProtectionBehaviour::current()),
-    _locked(_behaviour->lock(_method)),
+    _locked(_behaviour->lock(_method)) {
-    _nsv(true, !SafepointSynchronize::is_at_safepoint()) {
 }
 
 CompiledICLocker::~CompiledICLocker() {

src/hotspot/share/code/nmethod.cpp

@@ -1313,7 +1313,7 @@ bool nmethod::make_not_entrant_or_zombie(int state) {
   nmethodLocker nml(this);
   methodHandle the_method(method());
   // This can be called while the system is already at a safepoint which is ok
-  NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
+  NoSafepointVerifier nsv;
 
   // during patching, depending on the nmethod state we must notify the GC that
   // code has been unloaded, unregistering it. We cannot do this right while

src/hotspot/share/gc/z/zTracer.cpp

@@ -80,7 +80,7 @@ void ZTracer::initialize() {
 }
 
 void ZTracer::send_stat_counter(uint32_t counter_id, uint64_t increment, uint64_t value) {
-  NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
+  NoSafepointVerifier nsv;
 
   EventZStatisticsCounter e;
   if (e.should_commit()) {
@@ -92,7 +92,7 @@ void ZTracer::send_stat_counter(uint32_t counter_id, uint64_t increment, uint64_
 }
 
 void ZTracer::send_stat_sampler(uint32_t sampler_id, uint64_t value) {
-  NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
+  NoSafepointVerifier nsv;
 
   EventZStatisticsSampler e;
   if (e.should_commit()) {
@@ -103,7 +103,7 @@ void ZTracer::send_stat_sampler(uint32_t sampler_id, uint64_t value) {
 }
 
 void ZTracer::send_thread_phase(const char* name, const Ticks& start, const Ticks& end) {
-  NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
+  NoSafepointVerifier nsv;
 
   EventZThreadPhase e(UNTIMED);
   if (e.should_commit()) {
@@ -116,7 +116,7 @@ void ZTracer::send_thread_phase(const char* name, const Ticks& start, const Tick
 }
 
 void ZTracer::send_page_alloc(size_t size, size_t used, size_t free, size_t cache, bool nonblocking, bool noreserve) {
-  NoSafepointVerifier nsv(true, !SafepointSynchronize::is_at_safepoint());
+  NoSafepointVerifier nsv;
 
   EventZPageAllocation e;
   if (e.should_commit()) {

src/hotspot/share/opto/runtime.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1412,7 +1412,7 @@ address OptoRuntime::handle_exception_C(JavaThread* thread) {
 // However, there needs to be a safepoint check in the middle!  So compiled
 // safepoints are completely watertight.
 //
-// Thus, it cannot be a leaf since it contains the NoGCVerifier.
+// Thus, it cannot be a leaf since it contains the NoSafepointVerifier.
 //
 // *THIS IS NOT RECOMMENDED PROGRAMMING STYLE*
 //

src/hotspot/share/runtime/interfaceSupport.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -73,6 +73,14 @@ VMEntryWrapper::~VMEntryWrapper() {
   }
 }
 
+VMNativeEntryWrapper::VMNativeEntryWrapper() {
+  if (GCALotAtAllSafepoints) InterfaceSupport::check_gc_alot();
+}
+
+VMNativeEntryWrapper::~VMNativeEntryWrapper() {
+  if (GCALotAtAllSafepoints) InterfaceSupport::check_gc_alot();
+}
+
 long InterfaceSupport::_number_of_calls       = 0;
 long InterfaceSupport::_scavenge_alot_counter = 1;
 long InterfaceSupport::_fullgc_alot_counter   = 1;
@@ -292,40 +300,3 @@ void InterfaceSupport_init() {
   }
 #endif
 }
-
-#ifdef ASSERT
-// JRT_LEAF rules:
-// A JRT_LEAF method may not interfere with safepointing by
-//   1) acquiring or blocking on a Mutex or JavaLock - checked
-//   2) allocating heap memory - checked
-//   3) executing a VM operation - checked
-//   4) executing a system call (including malloc) that could block or grab a lock
-//   5) invoking GC
-//   6) reaching a safepoint
-//   7) running too long
-// Nor may any method it calls.
-JRTLeafVerifier::JRTLeafVerifier()
-  : NoSafepointVerifier(true, JRTLeafVerifier::should_verify_GC())
-{
-}
-
-JRTLeafVerifier::~JRTLeafVerifier()
-{
-}
-
-bool JRTLeafVerifier::should_verify_GC() {
-  switch (JavaThread::current()->thread_state()) {
-  case _thread_in_Java:
-    // is in a leaf routine, there must be no safepoint.
-    return true;
-  case _thread_in_native:
-    // A native thread is not subject to safepoints.
-    // Even while it is in a leaf routine, GC is ok
-    return false;
-  default:
-    // Leaf routines cannot be called from other contexts.
-    ShouldNotReachHere();
-    return false;
-  }
-}
-#endif // ASSERT

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

@@ -327,7 +327,7 @@ class ThreadInVMfromJavaNoAsyncException : public ThreadStateTransition {
   }
 };
 
-// Debug class instantiated in JRT_ENTRY and ITR_ENTRY macro.
+// Debug class instantiated in JRT_ENTRY macro.
 // Can be used to verify properties on enter/exit of the VM.
 
 #ifdef ASSERT
@@ -340,45 +340,17 @@ class VMEntryWrapper {
 
 class VMNativeEntryWrapper {
  public:
-  VMNativeEntryWrapper() {
+  VMNativeEntryWrapper();
-    if (GCALotAtAllSafepoints) InterfaceSupport::check_gc_alot();
+  ~VMNativeEntryWrapper();
-  }
-
-  ~VMNativeEntryWrapper() {
-    if (GCALotAtAllSafepoints) InterfaceSupport::check_gc_alot();
-  }
 };
 
-#endif
-
-
-// VM-internal runtime interface support
-
-// Definitions for JRT (Java (Compiler/Shared) Runtime)
-
-// JRT_LEAF currently can be called from either _thread_in_Java or
-// _thread_in_native mode. In _thread_in_native, it is ok
-// for another thread to trigger GC. The rest of the JRT_LEAF
-// rules apply.
-class JRTLeafVerifier : public NoSafepointVerifier {
-  static bool should_verify_GC();
- public:
-#ifdef ASSERT
-  JRTLeafVerifier();
-  ~JRTLeafVerifier();
-#else
-  JRTLeafVerifier() {}
-  ~JRTLeafVerifier() {}
-#endif
-};
-
-#ifdef ASSERT
-
 class RuntimeHistogramElement : public HistogramElement {
   public:
    RuntimeHistogramElement(const char* name);
 };
+#endif // ASSERT
 
+#ifdef ASSERT
 #define TRACE_CALL(result_type, header)                            \
   InterfaceSupport::_number_of_calls++;                            \
   if (CountRuntimeCalls) {                                         \
@@ -388,7 +360,7 @@ class RuntimeHistogramElement : public HistogramElement {
 #else
 #define TRACE_CALL(result_type, header)                            \
   /* do nothing */
-#endif
+#endif // ASSERT
 
 
 // LEAF routines do not lock, GC or throw exceptions
@@ -434,11 +406,24 @@ class RuntimeHistogramElement : public HistogramElement {
     VM_ENTRY_BASE(result_type, header, thread)                       \
     debug_only(VMEntryWrapper __vew;)
 
+// JRT_LEAF currently can be called from either _thread_in_Java or
+// _thread_in_native mode.
+//
+// JRT_LEAF rules:
+// A JRT_LEAF method may not interfere with safepointing by
+//   1) acquiring or blocking on a Mutex or JavaLock - checked
+//   2) allocating heap memory - checked
+//   3) executing a VM operation - checked
+//   4) executing a system call (including malloc) that could block or grab a lock
+//   5) invoking GC
+//   6) reaching a safepoint
+//   7) running too long
+// Nor may any method it calls.
 
 #define JRT_LEAF(result_type, header)                                \
   result_type header {                                               \
   VM_LEAF_BASE(result_type, header)                                  \
-  debug_only(JRTLeafVerifier __jlv;)
+  debug_only(NoSafepointVerifier __nsv;)
 
 
 #define JRT_ENTRY_NO_ASYNC(result_type, header)                      \

src/hotspot/share/runtime/safepointVerifiers.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,56 +23,29 @@
  */
 
 #include "precompiled.hpp"
-#include "runtime/safepointVerifiers.hpp"
 #include "gc/shared/collectedHeap.hpp"
 #include "memory/universe.hpp"
+#include "runtime/safepoint.hpp"
+#include "runtime/safepointVerifiers.hpp"
 #include "utilities/debug.hpp"
 
-// Implementation of NoGCVerifier
-
 #ifdef ASSERT
 
-NoGCVerifier::NoGCVerifier(bool verifygc) {
+NoSafepointVerifier::NoSafepointVerifier() : _thread(Thread::current()) {
-  _verifygc = verifygc;
+  _thread->_no_safepoint_count++;
-  if (_verifygc) {
-    CollectedHeap* h = Universe::heap();
-    assert(!h->is_gc_active(), "GC active during NoGCVerifier");
-    _old_invocations = h->total_collections();
-  }
 }
 
-
+NoSafepointVerifier::~NoSafepointVerifier() {
-NoGCVerifier::~NoGCVerifier() {
+  _thread->_no_safepoint_count--;
-  if (_verifygc) {
-    CollectedHeap* h = Universe::heap();
-    assert(!h->is_gc_active(), "GC active during NoGCVerifier");
-    if (_old_invocations != h->total_collections()) {
-      fatal("collection in a NoGCVerifier secured function");
-    }
-  }
 }
 
-PauseNoGCVerifier::PauseNoGCVerifier(NoGCVerifier * ngcv) {
+PauseNoSafepointVerifier::PauseNoSafepointVerifier(NoSafepointVerifier* nsv)
-  _ngcv = ngcv;
+    : _nsv(nsv) {
-  if (_ngcv->_verifygc) {
+  assert(_nsv->_thread == Thread::current(), "must be");
-    // if we were verifying, then make sure that nothing is
+  _nsv->_thread->_no_safepoint_count--;
-    // wrong before we "pause" verification
-    CollectedHeap* h = Universe::heap();
-    assert(!h->is_gc_active(), "GC active during NoGCVerifier");
-    if (_ngcv->_old_invocations != h->total_collections()) {
-      fatal("collection in a NoGCVerifier secured function");
-    }
-  }
 }
 
-
+PauseNoSafepointVerifier::~PauseNoSafepointVerifier() {
-PauseNoGCVerifier::~PauseNoGCVerifier() {
+  _nsv->_thread->_no_safepoint_count++;
-  if (_ngcv->_verifygc) {
-    // if we were verifying before, then reenable verification
-    CollectedHeap* h = Universe::heap();
-    assert(!h->is_gc_active(), "GC active during NoGCVerifier");
-    _ngcv->_old_invocations = h->total_collections();
-  }
 }
-
+#endif // ASSERT
-#endif

src/hotspot/share/runtime/safepointVerifiers.hpp

@@ -28,116 +28,31 @@
 #include "memory/allocation.hpp"
 #include "runtime/thread.hpp"
 
-// A NoGCVerifier object can be placed in methods where one assumes that
-// no garbage collection will occur. The destructor will verify this property
-// unless the constructor is called with argument false (not verifygc).
-//
-// The check will only be done in debug mode and if verifygc true.
-
-class NoGCVerifier: public StackObj {
- friend class PauseNoGCVerifier;
-
- protected:
-  bool _verifygc;
-  unsigned int _old_invocations;
-
- public:
-#ifdef ASSERT
-  NoGCVerifier(bool verifygc = true);
-  ~NoGCVerifier();
-#else
-  NoGCVerifier(bool verifygc = true) {}
-  ~NoGCVerifier() {}
-#endif
-};
-
-// A PauseNoGCVerifier is used to temporarily pause the behavior
-// of a NoGCVerifier object. If we are not in debug mode or if the
-// NoGCVerifier object has a _verifygc value of false, then there
-// is nothing to do.
-
-class PauseNoGCVerifier: public StackObj {
- private:
-  NoGCVerifier * _ngcv;
-
- public:
-#ifdef ASSERT
-  PauseNoGCVerifier(NoGCVerifier * ngcv);
-  ~PauseNoGCVerifier();
-#else
-  PauseNoGCVerifier(NoGCVerifier * ngcv) {}
-  ~PauseNoGCVerifier() {}
-#endif
-};
-
-
 // A NoSafepointVerifier object will throw an assertion failure if
 // the current thread passes a possible safepoint while this object is
 // instantiated. A safepoint, will either be: an oop allocation, blocking
 // on a Mutex or JavaLock, or executing a VM operation.
 //
-// If StrictSafepointChecks is turned off, it degrades into a NoGCVerifier
+class NoSafepointVerifier : public StackObj {
-//
-class NoSafepointVerifier : public NoGCVerifier {
  friend class PauseNoSafepointVerifier;
 
  private:
-  bool _activated;
   Thread *_thread;
  public:
-#ifdef ASSERT
+  NoSafepointVerifier()  NOT_DEBUG_RETURN;
-  NoSafepointVerifier(bool activated = true, bool verifygc = true ) :
+  ~NoSafepointVerifier() NOT_DEBUG_RETURN;
-    NoGCVerifier(verifygc),
-    _activated(activated) {
-    _thread = Thread::current();
-    if (_activated) {
-      _thread->_allow_safepoint_count++;
-    }
-  }
-
-  ~NoSafepointVerifier() {
-    if (_activated) {
-      _thread->_allow_safepoint_count--;
-    }
-  }
-#else
-  NoSafepointVerifier(bool activated = true, bool verifygc = true) : NoGCVerifier(verifygc){}
-  ~NoSafepointVerifier() {}
-#endif
 };
 
 // A PauseNoSafepointVerifier is used to temporarily pause the
-// behavior of a NoSafepointVerifier object. If we are not in debug
+// behavior of a NoSafepointVerifier object.
-// mode then there is nothing to do. If the NoSafepointVerifier
-// object has an _activated value of false, then there is nothing to
-// do for safepoint and allocation checking, but there may still be
-// something to do for the underlying NoGCVerifier object.
 
-class PauseNoSafepointVerifier : public PauseNoGCVerifier {
+class PauseNoSafepointVerifier : public StackObj {
  private:
-  NoSafepointVerifier * _nsv;
+  NoSafepointVerifier* _nsv;
 
  public:
-#ifdef ASSERT
+  PauseNoSafepointVerifier(NoSafepointVerifier* nsv) NOT_DEBUG_RETURN;
-  PauseNoSafepointVerifier(NoSafepointVerifier * nsv)
+  ~PauseNoSafepointVerifier() NOT_DEBUG_RETURN;
-    : PauseNoGCVerifier(nsv) {
-
-    _nsv = nsv;
-    if (_nsv->_activated) {
-      _nsv->_thread->_allow_safepoint_count--;
-    }
-  }
-
-  ~PauseNoSafepointVerifier() {
-    if (_nsv->_activated) {
-      _nsv->_thread->_allow_safepoint_count++;
-    }
-  }
-#else
-  PauseNoSafepointVerifier(NoSafepointVerifier * nsv)
-    : PauseNoGCVerifier(nsv) {}
-  ~PauseNoSafepointVerifier() {}
-#endif
 };
 
 #endif // SHARE_RUNTIME_SAFEPOINTVERIFIERS_HPP

src/hotspot/share/runtime/thread.cpp

@@ -250,7 +250,7 @@ Thread::Thread() {
 
   // plain initialization
   debug_only(_owned_locks = NULL;)
-  NOT_PRODUCT(_allow_safepoint_count = 0;)
+  NOT_PRODUCT(_no_safepoint_count = 0;)
   NOT_PRODUCT(_skip_gcalot = false;)
   _jvmti_env_iteration_count = 0;
   set_allocated_bytes(0);
@@ -1020,7 +1020,7 @@ bool Thread::owns_locks_but_compiled_lock() const {
 // 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 (!(_allow_safepoint_count == 0)) {
+  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) {
@@ -3543,7 +3543,7 @@ static inline void *prefetch_and_load_ptr(void **addr, intx prefetch_interval) {
 
 // All NonJavaThreads (i.e., every non-JavaThread in the system).
 void Threads::non_java_threads_do(ThreadClosure* tc) {
-  NoSafepointVerifier nsv(!SafepointSynchronize::is_at_safepoint(), false);
+  NoSafepointVerifier nsv;
   for (NonJavaThread::Iterator njti; !njti.end(); njti.step()) {
     tc->do_thread(njti.current());
   }

src/hotspot/share/runtime/thread.hpp

@@ -371,14 +371,15 @@ class Thread: public ThreadShadow {
 
  private:
 
-  // debug support for checking if code does allow safepoints or not
+  // Debug support for checking if code allows safepoints or not.
-  // GC points in the VM can happen because of allocation, invoking a VM operation, or blocking on
+  // Safepoints in the VM can happen because of allocation, invoking a VM operation, or blocking on
   // mutex, or blocking on an object synchronizer (Java locking).
-  // If !allow_safepoint(), then an assertion failure will happen in any of the above cases
+  // If _no_safepoint_count is non-zero, then an assertion failure will happen in any of
+  // the above cases.
   //
   // The class NoSafepointVerifier is used to set this counter.
   //
-  NOT_PRODUCT(int _allow_safepoint_count;)      // If 0, thread allow a safepoint to happen
+  NOT_PRODUCT(int _no_safepoint_count;)         // If 0, thread allow a safepoint to happen
 
   // Used by SkipGCALot class.
   NOT_PRODUCT(bool _skip_gcalot;)               // Should we elide gc-a-lot?