8252148: vmError::controlled_crash should be #ifdef ASSERT and move tests to gtest

Reviewed-by: iklam, stuefe

Author: coleenp

src/hotspot/share/gc/shared/gc_globals.hpp

@@ -290,9 +290,6 @@
   develop(uintx, MetadataAllocationFailALotInterval, 1000,                  \
           "Metadata allocation failure a lot interval")                     \
                                                                             \
-  product(bool, ExecutingUnitTests, false,                                  \
-          "Whether the JVM is running unit tests or not")                   \
-                                                                            \
   product(bool, UseTLAB, true,                                              \
           "Use thread-local object allocation")                             \
                                                                             \

src/hotspot/share/prims/jni.cpp

@@ -3800,10 +3800,14 @@ static jint JNI_CreateJavaVM_inner(JavaVM **vm, void **penv, void *args) {
 
 #ifndef PRODUCT
     if (ReplayCompiles) ciReplay::replay(thread);
+#endif
 
+#ifdef ASSERT
     // Some platforms (like Win*) need a wrapper around these test
     // functions in order to properly handle error conditions.
-    VMError::test_error_handler();
+    if (ErrorHandlerTest != 0) {
+      VMError::controlled_crash(ErrorHandlerTest);
+    }
 #endif
 
     // Since this is not a JVM_ENTRY we have to set the thread state manually before leaving.

src/hotspot/share/runtime/globals.hpp

@@ -493,19 +493,24 @@ const intx ObjectAlignmentInBytes = 8;
   develop(bool, ZapFillerObjects, trueInDebug,                              \
           "Zap filler objects with 0xDEAFBABE")                             \
                                                                             \
-  notproduct(uintx, ErrorHandlerTest, 0,                                    \
+  product(bool, ExecutingUnitTests, false,                                  \
+          "Whether the JVM is running unit tests or not")                   \
+                                                                            \
+  develop(uintx, ErrorHandlerTest, 0,                                       \
           "If > 0, provokes an error after VM initialization; the value "   \
-          "determines which error to provoke. See test_error_handler() "    \
+          "determines which error to provoke. See controlled_crash() "      \
           "in vmError.cpp.")                                                \
+          range(0, 17)                                                      \
                                                                             \
-  notproduct(uintx, TestCrashInErrorHandler, 0,                             \
+  develop(uintx, TestCrashInErrorHandler, 0,                                \
           "If > 0, provokes an error inside VM error handler (a secondary " \
-          "crash). see test_error_handler() in vmError.cpp")                \
+          "crash). see controlled_crash() in vmError.cpp")                  \
+          range(0, 17)                                                      \
                                                                             \
-  notproduct(bool, TestSafeFetchInErrorHandler, false,                      \
+  develop(bool, TestSafeFetchInErrorHandler, false   ,                      \
           "If true, tests SafeFetch inside error handler.")                 \
                                                                             \
-  notproduct(bool, TestUnresponsiveErrorHandler, false,                     \
+  develop(bool, TestUnresponsiveErrorHandler, false,                        \
           "If true, simulates an unresponsive error handler.")              \
                                                                             \
   develop(bool, Verbose, false,                                             \

src/hotspot/share/utilities/debug.cpp

@@ -238,6 +238,36 @@ void report_vm_error(const char* file, int line, const char* error_msg)
   report_vm_error(file, line, error_msg, "%s", "");
 }
 
+
+static void print_error_for_unit_test(const char* message, const char* detail_fmt, va_list detail_args) {
+#ifdef ASSERT
+  if (ExecutingUnitTests) {
+    char detail_msg[256];
+    if (detail_fmt != NULL) {
+      // Special handling for the sake of gtest death tests which expect the assert
+      // message to be printed in one short line to stderr (see TEST_VM_ASSERT_MSG) and
+      // cannot be tweaked to accept our normal assert message.
+      va_list detail_args_copy;
+      va_copy(detail_args_copy, detail_args);
+      jio_vsnprintf(detail_msg, sizeof(detail_msg), detail_fmt, detail_args_copy);
+
+      // the VM assert tests look for "assert failed: "
+      if (message == NULL) {
+        fprintf(stderr, "assert failed: %s", detail_msg);
+      } else {
+        if (strlen(detail_msg) > 0) {
+          fprintf(stderr, "assert failed: %s: %s", message, detail_msg);
+        } else {
+          fprintf(stderr, "assert failed: Error: %s", message);
+        }
+      }
+      ::fflush(stderr);
+      va_end(detail_args_copy);
+    }
+  }
+#endif // ASSERT
+}
+
 void report_vm_error(const char* file, int line, const char* error_msg, const char* detail_fmt, ...)
 {
   if (Debugging || error_is_suppressed(file, line)) return;
@@ -250,20 +280,7 @@ void report_vm_error(const char* file, int line, const char* error_msg, const ch
   }
 #endif // CAN_SHOW_REGISTERS_ON_ASSERT
 
-#ifdef ASSERT
-  if (detail_fmt != NULL && ExecutingUnitTests) {
-    // Special handling for the sake of gtest death tests which expect the assert
-    // message to be printed in one short line to stderr (see TEST_VM_ASSERT_MSG) and
-    // cannot be tweaked to accept our normal assert message.
-    va_list detail_args_copy;
-    va_copy(detail_args_copy, detail_args);
-    ::fputs("assert failed: ", stderr);
-    ::vfprintf(stderr, detail_fmt, detail_args_copy);
-    ::fputs("\n", stderr);
-    ::fflush(stderr);
-    va_end(detail_args_copy);
-  }
-#endif
+  print_error_for_unit_test(error_msg, detail_fmt, detail_args);
 
   VMError::report_and_die(Thread::current_or_null(), context, file, line, error_msg, detail_fmt, detail_args);
   va_end(detail_args);
@@ -285,6 +302,9 @@ void report_fatal(const char* file, int line, const char* detail_fmt, ...)
     context = g_assertion_context;
   }
 #endif // CAN_SHOW_REGISTERS_ON_ASSERT
+
+  print_error_for_unit_test("fatal error", detail_fmt, detail_args);
+
   VMError::report_and_die(Thread::current_or_null(), context, file, line, "fatal error", detail_fmt, detail_args);
   va_end(detail_args);
 }
@@ -294,6 +314,9 @@ void report_vm_out_of_memory(const char* file, int line, size_t size,
   if (Debugging) return;
   va_list detail_args;
   va_start(detail_args, detail_fmt);
+
+  print_error_for_unit_test(NULL, detail_fmt, detail_args);
+
   VMError::report_and_die(Thread::current_or_null(), file, line, size, vm_err_type, detail_fmt, detail_args);
   va_end(detail_args);
 

src/hotspot/share/utilities/vmError.cpp

@@ -444,7 +444,7 @@ void VMError::report(outputStream* st, bool _verbose) {
                    "Runtime Environment to continue.");
     }
 
-#ifndef PRODUCT
+#ifdef ASSERT
   // Error handler self tests
 
   // test secondary error handling. Test it twice, to test that resetting
@@ -503,7 +503,7 @@ void VMError::report(outputStream* st, bool _verbose) {
         st->print_cr("not possible; skipped.");
       }
     }
-#endif // PRODUCT
+#endif // ASSERT
 
   STEP("printing type of error")
 
@@ -1737,7 +1737,7 @@ bool VMError::check_timeout() {
 
 }
 
-#ifndef PRODUCT
+#ifdef ASSERT
 typedef void (*voidfun_t)();
 
 // Crash with an authentic sigfpe
@@ -1765,47 +1765,13 @@ static void crash_with_segfault() {
 
 } // end: crash_with_segfault
 
-void VMError::test_error_handler() {
-  controlled_crash(ErrorHandlerTest);
-}
-
 // crash in a controlled way:
-// how can be one of:
-// 1,2 - asserts
-// 3,4 - guarantee
-// 5-7 - fatal
-// 8 - vm_exit_out_of_memory
-// 9 - ShouldNotCallThis
-// 10 - ShouldNotReachHere
-// 11 - Unimplemented
-// 12,13 - (not guaranteed) crashes
+// 1  - assert
+// 2  - guarantee
 // 14 - SIGSEGV
 // 15 - SIGFPE
 void VMError::controlled_crash(int how) {
-  if (how == 0) return;
-
-  // If asserts are disabled, use the corresponding guarantee instead.
-  NOT_DEBUG(if (how <= 2) how += 2);
-
-  const char* const str = "hello";
-  const size_t      num = (size_t)os::vm_page_size();
-
-  const char* const eol = os::line_separator();
-  const char* const msg = "this message should be truncated during formatting";
-  char * const dataPtr = NULL;  // bad data pointer
-  const void (*funcPtr)(void);  // bad function pointer
 
-#if defined(PPC64) && !defined(ABI_ELFv2)
-  struct FunctionDescriptor functionDescriptor;
-
-  functionDescriptor.set_entry((address) 0xF);
-  funcPtr = (const void(*)()) &functionDescriptor;
-#else
-  funcPtr = (const void(*)()) 0xF;
-#endif
-
-  // Keep this in sync with test/hotspot/jtreg/runtime/ErrorHandling/ErrorHandler.java
-  // which tests cases 1 thru 13.
   // Case 14 is tested by test/hotspot/jtreg/runtime/ErrorHandling/SafeFetchInErrorHandlingTest.java.
   // Case 15 is tested by test/hotspot/jtreg/runtime/ErrorHandling/SecondaryErrorTest.java.
   // Case 16 is tested by test/hotspot/jtreg/runtime/ErrorHandling/ThreadsListHandleInErrorHandlingTest.java.
@@ -1821,30 +1787,10 @@ void VMError::controlled_crash(int how) {
   }
 
   switch (how) {
-    case  1: vmassert(str == NULL, "expected null"); break;
-    case  2: vmassert(num == 1023 && *str == 'X',
-                      "num=" SIZE_FORMAT " str=\"%s\"", num, str); break;
-    case  3: guarantee(str == NULL, "expected null"); break;
-    case  4: guarantee(num == 1023 && *str == 'X',
-                       "num=" SIZE_FORMAT " str=\"%s\"", num, str); break;
-    case  5: fatal("expected null"); break;
-    case  6: fatal("num=" SIZE_FORMAT " str=\"%s\"", num, str); break;
-    case  7: fatal("%s%s#    %s%s#    %s%s#    %s%s#    %s%s#    "
-                   "%s%s#    %s%s#    %s%s#    %s%s#    %s%s#    "
-                   "%s%s#    %s%s#    %s%s#    %s%s#    %s",
-                   msg, eol, msg, eol, msg, eol, msg, eol, msg, eol,
-                   msg, eol, msg, eol, msg, eol, msg, eol, msg, eol,
-                   msg, eol, msg, eol, msg, eol, msg, eol, msg); break;
-    case  8: vm_exit_out_of_memory(num, OOM_MALLOC_ERROR, "ChunkPool::allocate"); break;
-    case  9: ShouldNotCallThis(); break;
-    case 10: ShouldNotReachHere(); break;
-    case 11: Unimplemented(); break;
-    // There's no guarantee the bad data pointer will crash us
-    // so "break" out to the ShouldNotReachHere().
-    case 12: *dataPtr = '\0'; break;
-    // There's no guarantee the bad function pointer will crash us
-    // so "break" out to the ShouldNotReachHere().
-    case 13: (*funcPtr)(); break;
+    case 1: assert(how == 0, "test assert"); break;
+    case 2: guarantee(how == 0, "test guarantee"); break;
+
+    // The other cases are unused.
     case 14: crash_with_segfault(); break;
     case 15: crash_with_sigfpe(); break;
     case 16: {
@@ -1858,19 +1804,11 @@ void VMError::controlled_crash(int how) {
         fatal("Force crash with a nested ThreadsListHandle.");
       }
     }
-    case 18: {
-      // Check for assert when allocating from resource area without a
-      // ResourceMark.  There must not be a ResourceMark on the
-      // current stack when invoking this test case.
-      ResourceArea* area = Thread::current()->resource_area();
-      assert(area->nesting() == 0, "unexpected ResourceMark");
-      area->allocate_bytes(100);
-      break;
-    }
-
-    default: tty->print_cr("ERROR: %d: unexpected test_num value.", how);
+    default:
+      // If another number is given, give a generic crash.
+      fatal("Crashing with number %d", how);
   }
-  tty->print_cr("VMError::controlled_crash: survived intentional crash. Did you suppress the assert?");
+  tty->print_cr("controlled_crash: survived intentional crash. Did you suppress the assert?");
   ShouldNotReachHere();
 }
-#endif // !PRODUCT
+#endif // !ASSERT

src/hotspot/share/utilities/vmError.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, 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
@@ -183,9 +183,7 @@ class VMError : public AllStatic {
   // Support for avoiding multiple asserts
   static bool is_error_reported();
 
-  // Test vmassert(), fatal(), guarantee(), etc.
-  NOT_PRODUCT(static void test_error_handler();)
-  NOT_PRODUCT(static void controlled_crash(int how);)
+  DEBUG_ONLY(static void controlled_crash(int how);)
 
   // returns an address which is guaranteed to generate a SIGSEGV on read,
   // for test purposes, which is not NULL and contains bits in every word

test/hotspot/gtest/gc/g1/test_g1ServiceThread.cpp

@@ -160,19 +160,19 @@ TEST_VM(G1ServiceTaskQueue, add_ordered) {
 
 #ifdef ASSERT
 TEST_VM_ASSERT_MSG(G1ServiceTaskQueue, pop_empty,
-    "Should never try to verify empty queue") {
+    ".*Should never try to verify empty queue") {
   G1ServiceTaskQueue queue;
   queue.pop();
 }
 
 TEST_VM_ASSERT_MSG(G1ServiceTaskQueue, peek_empty,
-    "Should never try to verify empty queue") {
+    ".*Should never try to verify empty queue") {
   G1ServiceTaskQueue queue;
   queue.peek();
 }
 
 TEST_VM_ASSERT_MSG(G1ServiceTaskQueue, set_time_in_queue,
-    "Not allowed to update time while in queue") {
+    ".*Not allowed to update time while in queue") {
   G1ServiceTaskQueue queue;
   TestTask a(100);
   queue.add_ordered(&a);

test/hotspot/gtest/metaspace/test_allocationGuard.cpp

@@ -44,7 +44,7 @@ using metaspace::Settings;
 //  Note: We use TEST_VM_ASSERT_MSG. However, an assert is only triggered if allocation
 //  guards are enabled; if guards are disabled for the gtests, this test would fail.
 //  So for that case, we trigger a fake assert.
-TEST_VM_ASSERT_MSG(metaspace, test_overwriter, "Corrupt block") {
+TEST_VM_ASSERT_MSG(metaspace, test_overwriter, ".*failed: Corrupt block") {
 
   if (Settings::use_allocation_guard()) {
     MetaspaceGtestContext context;

test/hotspot/gtest/metaspace/test_blocktree.cpp

@@ -218,7 +218,7 @@ TEST_VM(metaspace, BlockTree_print_test) {
 }
 
 // Test that an overwritten node would result in an assert and a printed tree
-TEST_VM_ASSERT_MSG(metaspace, BlockTree_overwriter_test, "Invalid node") {
+TEST_VM_ASSERT_MSG(metaspace, BlockTree_overwriter_test, ".*failed: Invalid node") {
   static const size_t sizes1[] = { 30, 17, 0 };
   static const size_t sizes2[] = { 12, 12, 0 };
 

test/hotspot/gtest/runtime/test_mutex_rank.cpp

@@ -45,7 +45,7 @@ TEST_OTHER_VM(MutexRank, mutex_lock_rank_in_order) {
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderA,
-                   "Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
+                   ".* Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);
 
@@ -59,7 +59,7 @@ TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderA,
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderB,
-                   "Attempting to acquire lock mutex_rankB/50 out of order with lock mutex_rankA/50 -- possible deadlock") {
+                   ".* Attempting to acquire lock mutex_rankB/50 out of order with lock mutex_rankA/50 -- possible deadlock") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);
 
@@ -89,7 +89,7 @@ TEST_OTHER_VM(MutexRank, mutex_trylock_rank_out_of_orderA) {
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, mutex_trylock_rank_out_of_orderB,
-                   "Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
+                   ".* Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);
 
@@ -119,7 +119,7 @@ TEST_OTHER_VM(MutexRank, monitor_wait_rank_in_order) {
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order,
-                   "Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
+                   ".* Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
                    "-- possible deadlock. Should wait on the least ranked monitor from all owned locks.") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);
@@ -135,7 +135,7 @@ TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order,
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order_trylock,
-                   "Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
+                   ".* Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
                    "-- possible deadlock. Should wait on the least ranked monitor from all owned locks.") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);
@@ -151,7 +151,7 @@ TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order_trylock,
 }
 
 TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_special,
-                   "Attempting to wait on monitor monitor_rank_special_minus_one/5 while holding lock monitor_rank_special/6 "
+                   ".* Attempting to wait on monitor monitor_rank_special_minus_one/5 while holding lock monitor_rank_special/6 "
                    "-- possible deadlock. Should not block\\(wait\\) while holding a lock of rank special.") {
   JavaThread* THREAD = JavaThread::current();
   ThreadInVMfromNative invm(THREAD);