From 3ca2fb91f5ed9905f5fe687e89a255c840c65ecf Mon Sep 17 00:00:00 2001
From: "Daniel D. Daugherty" <daniel.daugherty@oracle.com>
Date: Tue, 27 Jul 2021 14:55:47 -0400
Subject: [PATCH] 8271348: Add stronger sanity check of thread state when
 polling for safepoint/handshakes

---
 src/hotspot/share/runtime/safepoint.cpp       | 45 +++++++------------
 src/hotspot/share/runtime/safepoint.hpp       | 13 ++++++
 .../share/runtime/safepointMechanism.cpp      |  2 +
 3 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp
index 3db6cc8b3ee..b4c21652b38 100644
--- a/src/hotspot/share/runtime/safepoint.cpp
+++ b/src/hotspot/share/runtime/safepoint.cpp
@@ -705,45 +705,34 @@ void SafepointSynchronize::block(JavaThread *thread) {
   }
 
   JavaThreadState state = thread->thread_state();
+  assert(SafepointSynchronize::is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
   thread->frame_anchor()->make_walkable(thread);
 
   uint64_t safepoint_id = SafepointSynchronize::safepoint_counter();
-  // Check that we have a valid thread_state at this point
-  switch(state) {
-    case _thread_in_vm_trans:
-    case _thread_in_Java:        // From compiled code
-    case _thread_in_native_trans:
-    case _thread_blocked_trans:
-    case _thread_new_trans:
-
-      // We have no idea where the VMThread is, it might even be at next safepoint.
-      // So we can miss this poll, but stop at next.
 
-      // Load dependent store, it must not pass loading of safepoint_id.
-      thread->safepoint_state()->set_safepoint_id(safepoint_id); // Release store
+  // We have no idea where the VMThread is, it might even be at next safepoint.
+  // So we can miss this poll, but stop at next.
 
-      // This part we can skip if we notice we miss or are in a future safepoint.
-      OrderAccess::storestore();
-      // Load in wait barrier should not float up
-      thread->set_thread_state_fence(_thread_blocked);
+  // Load dependent store, it must not pass loading of safepoint_id.
+  thread->safepoint_state()->set_safepoint_id(safepoint_id); // Release store
 
-      _wait_barrier->wait(static_cast<int>(safepoint_id));
-      assert(_state != _synchronized, "Can't be");
+  // This part we can skip if we notice we miss or are in a future safepoint.
+  OrderAccess::storestore();
+  // Load in wait barrier should not float up
+  thread->set_thread_state_fence(_thread_blocked);
 
-      // If barrier is disarmed stop store from floating above loads in barrier.
-      OrderAccess::loadstore();
-      thread->set_thread_state(state);
+  _wait_barrier->wait(static_cast<int>(safepoint_id));
+  assert(_state != _synchronized, "Can't be");
 
-      // Then we reset the safepoint id to inactive.
-      thread->safepoint_state()->reset_safepoint_id(); // Release store
+  // If barrier is disarmed stop store from floating above loads in barrier.
+  OrderAccess::loadstore();
+  thread->set_thread_state(state);
 
-      OrderAccess::fence();
+  // Then we reset the safepoint id to inactive.
+  thread->safepoint_state()->reset_safepoint_id(); // Release store
 
-      break;
+  OrderAccess::fence();
 
-    default:
-     fatal("Illegal threadstate encountered: %d", state);
-  }
   guarantee(thread->safepoint_state()->get_safepoint_id() == InactiveSafepointCounter,
             "The safepoint id should be set only in block path");
 
diff --git a/src/hotspot/share/runtime/safepoint.hpp b/src/hotspot/share/runtime/safepoint.hpp
index c35f55f0c27..e73d029a2f8 100644
--- a/src/hotspot/share/runtime/safepoint.hpp
+++ b/src/hotspot/share/runtime/safepoint.hpp
@@ -126,6 +126,19 @@ class SafepointSynchronize : AllStatic {
                                     JavaThread *thread,
                                     uint64_t safepoint_count);
 
+  static bool is_a_block_safe_state(JavaThreadState state) {
+    // Check that we have a valid thread_state before blocking for safepoints
+    switch(state) {
+      case _thread_in_vm_trans:
+      case _thread_in_Java:        // From compiled code
+      case _thread_in_native_trans:
+      case _thread_blocked_trans:
+      case _thread_new_trans:
+        return true;
+      default:
+        return false;
+    }
+  }
   // Called when a thread voluntarily blocks
   static void block(JavaThread *thread);
 
diff --git a/src/hotspot/share/runtime/safepointMechanism.cpp b/src/hotspot/share/runtime/safepointMechanism.cpp
index 20e163a7f6c..b092f050539 100644
--- a/src/hotspot/share/runtime/safepointMechanism.cpp
+++ b/src/hotspot/share/runtime/safepointMechanism.cpp
@@ -118,6 +118,8 @@ void SafepointMechanism::process(JavaThread *thread, bool allow_suspend) {
   // local poll already checked, if used.
   bool need_rechecking;
   do {
+    JavaThreadState state = thread->thread_state();
+    guarantee(SafepointSynchronize::is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
     if (global_poll()) {
       // Any load in ::block() must not pass the global poll load.
       // Otherwise we might load an old safepoint counter (for example).
-- 
2.28.0