From f6340643974f3e0cc3ab95fbbba51b23b8d9af31 Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Thu, 22 Oct 2020 10:42:00 -0500 Subject: [PATCH 1/6] reset signal handlers to their system defaults if handling crash with UseOSErrorReporting --- src/hotspot/os/posix/signals_posix.cpp | 11 +++++++++++ src/hotspot/os/posix/signals_posix.hpp | 5 +++++ src/hotspot/os/posix/vmError_posix.cpp | 6 +++++- src/hotspot/os/windows/vmError_windows.cpp | 5 ++++- src/hotspot/share/utilities/vmError.cpp | 13 +++++++++---- src/hotspot/share/utilities/vmError.hpp | 8 +++++--- 6 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/hotspot/os/posix/signals_posix.cpp b/src/hotspot/os/posix/signals_posix.cpp index 4eea0664566e5..2bc2cca0be631 100644 --- a/src/hotspot/os/posix/signals_posix.cpp +++ b/src/hotspot/os/posix/signals_posix.cpp @@ -1414,6 +1414,17 @@ void PosixSignals::hotspot_sigmask(Thread* thread) { } } +void PosixSignals::clear_signal_handlers() { + // set signal handlers back to their defaults + struct sigaction defaulthandler; + sigemptyset(&defaulthandler.sa_mask); + + defaulthandler.sa_handler = SIG_DFL; + for (int i = 0; i < NSIG + 1; i++) { + sigaction(i, &defaulthandler, NULL); + } +} + //////////////////////////////////////////////////////////////////////////////// // suspend/resume support diff --git a/src/hotspot/os/posix/signals_posix.hpp b/src/hotspot/os/posix/signals_posix.hpp index a2f7d955e1461..cf31f68b7c91b 100644 --- a/src/hotspot/os/posix/signals_posix.hpp +++ b/src/hotspot/os/posix/signals_posix.hpp @@ -48,6 +48,11 @@ class PosixSignals : public AllStatic { static int unblock_thread_signal_mask(const sigset_t *set); static void hotspot_sigmask(Thread* thread); + // set signal handlers back to their system defaults (i.e. no handler) + // to let the process die at the crash site (so that the OS can + // catch and process it) + static void clear_signal_handlers(); + static void print_signal_handler(outputStream* st, int sig, char* buf, size_t buflen); static address ucontext_get_pc(const ucontext_t* ctx); diff --git a/src/hotspot/os/posix/vmError_posix.cpp b/src/hotspot/os/posix/vmError_posix.cpp index 9c83d263e7184..4641bbb447d26 100644 --- a/src/hotspot/os/posix/vmError_posix.cpp +++ b/src/hotspot/os/posix/vmError_posix.cpp @@ -138,7 +138,7 @@ static void crash_handler(int sig, siginfo_t* info, void* ucVoid) { VMError::report_and_die(NULL, sig, pc, info, ucVoid); } -void VMError::reset_signal_handlers() { +void VMError::rearm_signal_handlers() { // install signal handlers for all synchronous program error signals sigset_t newset; sigemptyset(&newset); @@ -151,6 +151,10 @@ void VMError::reset_signal_handlers() { PosixSignals::unblock_thread_signal_mask(&newset); } +void VMError::clear_signal_handlers() { + PosixSignals::clear_signal_handlers(); +} + // Write a hint to the stream in case siginfo relates to a segv/bus error // and the offending address points into CDS archive. void VMError::check_failing_cds_access(outputStream* st, const void* siginfo) { diff --git a/src/hotspot/os/windows/vmError_windows.cpp b/src/hotspot/os/windows/vmError_windows.cpp index fb539a9e131d9..68f8c25c526a9 100644 --- a/src/hotspot/os/windows/vmError_windows.cpp +++ b/src/hotspot/os/windows/vmError_windows.cpp @@ -44,10 +44,13 @@ LONG WINAPI crash_handler(struct _EXCEPTION_POINTERS* exceptionInfo) { return EXCEPTION_CONTINUE_SEARCH; } -void VMError::reset_signal_handlers() { +void VMError::rearm_signal_handlers() { SetUnhandledExceptionFilter(crash_handler); } +void VMError::clear_signal_handlers() { +} + // Write a hint to the stream in case siginfo relates to a segv/bus error // and the offending address points into CDS archive. void VMError::check_failing_cds_access(outputStream* st, const void* siginfo) { diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 4a9ecebbec946..75e520c80cbf6 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -1429,14 +1429,19 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt os::check_dump_limit(buffer, sizeof(buffer)); - // reset signal handlers or exception filter; make sure recursive crashes - // are handled properly. - reset_signal_handlers(); + // Rearm signal handlers (Linux, macOS) or exception filter (Windows) + // to make sure that recursive crashes are handled properly. + rearm_signal_handlers(); } else { // If UseOsErrorReporting we call this for each level of the call stack // while searching for the exception handler. Only the first level needs // to be reported. - if (UseOSErrorReporting && log_done) return; + if (UseOSErrorReporting && log_done) { + // We already handled the signal once, so reset signal handlers + // to their defaults and let OS handle it after the process will die + clear_signal_handlers(); + return; + } // This is not the first error, see if it happened in a different thread // or in the same thread during error reporting. diff --git a/src/hotspot/share/utilities/vmError.hpp b/src/hotspot/share/utilities/vmError.hpp index 2e7b0c26d720f..92ad75ebe8b75 100644 --- a/src/hotspot/share/utilities/vmError.hpp +++ b/src/hotspot/share/utilities/vmError.hpp @@ -88,9 +88,11 @@ class VMError : public AllStatic { public: - // set signal handlers on Solaris/Linux or the default exception filter - // on Windows, to handle recursive crashes. - static void reset_signal_handlers(); + // reset signal handlers to the default exception filter + // to handle recursive crashes + static void rearm_signal_handlers(); + + static void clear_signal_handlers(); // handle -XX:+ShowMessageBoxOnError. buf is used to format the message string static void show_message_box(char* buf, int buflen); From cb403c98a8136461901b581a37b89f29cef089e7 Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Tue, 27 Oct 2020 11:40:26 -0500 Subject: [PATCH 2/6] Revert "reset signal handlers to their system defaults if handling crash with UseOSErrorReporting" This reverts commit f6340643974f3e0cc3ab95fbbba51b23b8d9af31. --- src/hotspot/os/posix/signals_posix.cpp | 11 ----------- src/hotspot/os/posix/signals_posix.hpp | 5 ----- src/hotspot/os/posix/vmError_posix.cpp | 6 +----- src/hotspot/os/windows/vmError_windows.cpp | 5 +---- src/hotspot/share/utilities/vmError.cpp | 13 ++++--------- src/hotspot/share/utilities/vmError.hpp | 8 +++----- 6 files changed, 9 insertions(+), 39 deletions(-) diff --git a/src/hotspot/os/posix/signals_posix.cpp b/src/hotspot/os/posix/signals_posix.cpp index 2bc2cca0be631..4eea0664566e5 100644 --- a/src/hotspot/os/posix/signals_posix.cpp +++ b/src/hotspot/os/posix/signals_posix.cpp @@ -1414,17 +1414,6 @@ void PosixSignals::hotspot_sigmask(Thread* thread) { } } -void PosixSignals::clear_signal_handlers() { - // set signal handlers back to their defaults - struct sigaction defaulthandler; - sigemptyset(&defaulthandler.sa_mask); - - defaulthandler.sa_handler = SIG_DFL; - for (int i = 0; i < NSIG + 1; i++) { - sigaction(i, &defaulthandler, NULL); - } -} - //////////////////////////////////////////////////////////////////////////////// // suspend/resume support diff --git a/src/hotspot/os/posix/signals_posix.hpp b/src/hotspot/os/posix/signals_posix.hpp index cf31f68b7c91b..a2f7d955e1461 100644 --- a/src/hotspot/os/posix/signals_posix.hpp +++ b/src/hotspot/os/posix/signals_posix.hpp @@ -48,11 +48,6 @@ class PosixSignals : public AllStatic { static int unblock_thread_signal_mask(const sigset_t *set); static void hotspot_sigmask(Thread* thread); - // set signal handlers back to their system defaults (i.e. no handler) - // to let the process die at the crash site (so that the OS can - // catch and process it) - static void clear_signal_handlers(); - static void print_signal_handler(outputStream* st, int sig, char* buf, size_t buflen); static address ucontext_get_pc(const ucontext_t* ctx); diff --git a/src/hotspot/os/posix/vmError_posix.cpp b/src/hotspot/os/posix/vmError_posix.cpp index 4641bbb447d26..9c83d263e7184 100644 --- a/src/hotspot/os/posix/vmError_posix.cpp +++ b/src/hotspot/os/posix/vmError_posix.cpp @@ -138,7 +138,7 @@ static void crash_handler(int sig, siginfo_t* info, void* ucVoid) { VMError::report_and_die(NULL, sig, pc, info, ucVoid); } -void VMError::rearm_signal_handlers() { +void VMError::reset_signal_handlers() { // install signal handlers for all synchronous program error signals sigset_t newset; sigemptyset(&newset); @@ -151,10 +151,6 @@ void VMError::rearm_signal_handlers() { PosixSignals::unblock_thread_signal_mask(&newset); } -void VMError::clear_signal_handlers() { - PosixSignals::clear_signal_handlers(); -} - // Write a hint to the stream in case siginfo relates to a segv/bus error // and the offending address points into CDS archive. void VMError::check_failing_cds_access(outputStream* st, const void* siginfo) { diff --git a/src/hotspot/os/windows/vmError_windows.cpp b/src/hotspot/os/windows/vmError_windows.cpp index 68f8c25c526a9..fb539a9e131d9 100644 --- a/src/hotspot/os/windows/vmError_windows.cpp +++ b/src/hotspot/os/windows/vmError_windows.cpp @@ -44,13 +44,10 @@ LONG WINAPI crash_handler(struct _EXCEPTION_POINTERS* exceptionInfo) { return EXCEPTION_CONTINUE_SEARCH; } -void VMError::rearm_signal_handlers() { +void VMError::reset_signal_handlers() { SetUnhandledExceptionFilter(crash_handler); } -void VMError::clear_signal_handlers() { -} - // Write a hint to the stream in case siginfo relates to a segv/bus error // and the offending address points into CDS archive. void VMError::check_failing_cds_access(outputStream* st, const void* siginfo) { diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 75e520c80cbf6..4a9ecebbec946 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -1429,19 +1429,14 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt os::check_dump_limit(buffer, sizeof(buffer)); - // Rearm signal handlers (Linux, macOS) or exception filter (Windows) - // to make sure that recursive crashes are handled properly. - rearm_signal_handlers(); + // reset signal handlers or exception filter; make sure recursive crashes + // are handled properly. + reset_signal_handlers(); } else { // If UseOsErrorReporting we call this for each level of the call stack // while searching for the exception handler. Only the first level needs // to be reported. - if (UseOSErrorReporting && log_done) { - // We already handled the signal once, so reset signal handlers - // to their defaults and let OS handle it after the process will die - clear_signal_handlers(); - return; - } + if (UseOSErrorReporting && log_done) return; // This is not the first error, see if it happened in a different thread // or in the same thread during error reporting. diff --git a/src/hotspot/share/utilities/vmError.hpp b/src/hotspot/share/utilities/vmError.hpp index 92ad75ebe8b75..2e7b0c26d720f 100644 --- a/src/hotspot/share/utilities/vmError.hpp +++ b/src/hotspot/share/utilities/vmError.hpp @@ -88,11 +88,9 @@ class VMError : public AllStatic { public: - // reset signal handlers to the default exception filter - // to handle recursive crashes - static void rearm_signal_handlers(); - - static void clear_signal_handlers(); + // set signal handlers on Solaris/Linux or the default exception filter + // on Windows, to handle recursive crashes. + static void reset_signal_handlers(); // handle -XX:+ShowMessageBoxOnError. buf is used to format the message string static void show_message_box(char* buf, int buflen); From 74d6c9a6d6bda2c66aafe7b022d3bcf8539bb7d7 Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Tue, 27 Oct 2020 12:00:50 -0500 Subject: [PATCH 3/6] Only use UseOsErrorReporting on Windows --- src/hotspot/share/utilities/vmError.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 4a9ecebbec946..134a088f62173 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -1433,10 +1433,12 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt // are handled properly. reset_signal_handlers(); } else { +#if defined(_WINDOWS) // If UseOsErrorReporting we call this for each level of the call stack // while searching for the exception handler. Only the first level needs // to be reported. if (UseOSErrorReporting && log_done) return; +#endif // This is not the first error, see if it happened in a different thread // or in the same thread during error reporting. @@ -1626,7 +1628,11 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt OnError = NULL; } +#if defined(_WINDOWS) if (!UseOSErrorReporting) { +#else + if (true) { +#endif // os::abort() will call abort hooks, try it first. static bool skip_os_abort = false; if (!skip_os_abort) { From b849b3c4abb1c07015a9fb3c0acafcb3291edf21 Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Wed, 28 Oct 2020 11:46:43 -0500 Subject: [PATCH 4/6] make UseOSErrorReporting flag Windows only --- src/hotspot/os/aix/globals_aix.hpp | 1 - src/hotspot/os/bsd/globals_bsd.hpp | 1 - src/hotspot/os/linux/globals_linux.hpp | 1 - src/hotspot/os/windows/globals_windows.hpp | 19 ++++++++++--------- src/hotspot/share/runtime/globals.hpp | 3 --- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/hotspot/os/aix/globals_aix.hpp b/src/hotspot/os/aix/globals_aix.hpp index b6b67a4f3bbf1..1203b09c883e1 100644 --- a/src/hotspot/os/aix/globals_aix.hpp +++ b/src/hotspot/os/aix/globals_aix.hpp @@ -88,7 +88,6 @@ // Use Use64KPages or Use16MPages instead. define_pd_global(bool, UseLargePages, false); define_pd_global(bool, UseLargePagesIndividualAllocation, false); -define_pd_global(bool, UseOSErrorReporting, false); define_pd_global(bool, UseThreadPriorities, true) ; #endif // OS_AIX_GLOBALS_AIX_HPP diff --git a/src/hotspot/os/bsd/globals_bsd.hpp b/src/hotspot/os/bsd/globals_bsd.hpp index 6c8939a6dc02b..b36173655df8a 100644 --- a/src/hotspot/os/bsd/globals_bsd.hpp +++ b/src/hotspot/os/bsd/globals_bsd.hpp @@ -44,7 +44,6 @@ // define_pd_global(bool, UseLargePages, false); define_pd_global(bool, UseLargePagesIndividualAllocation, false); -define_pd_global(bool, UseOSErrorReporting, false); define_pd_global(bool, UseThreadPriorities, true) ; #endif // OS_BSD_GLOBALS_BSD_HPP diff --git a/src/hotspot/os/linux/globals_linux.hpp b/src/hotspot/os/linux/globals_linux.hpp index 0e5c209c94008..138dcf5e10640 100644 --- a/src/hotspot/os/linux/globals_linux.hpp +++ b/src/hotspot/os/linux/globals_linux.hpp @@ -89,7 +89,6 @@ // define_pd_global(bool, UseLargePages, false); define_pd_global(bool, UseLargePagesIndividualAllocation, false); -define_pd_global(bool, UseOSErrorReporting, false); define_pd_global(bool, UseThreadPriorities, true) ; #endif // OS_LINUX_GLOBALS_LINUX_HPP diff --git a/src/hotspot/os/windows/globals_windows.hpp b/src/hotspot/os/windows/globals_windows.hpp index a712e102a760b..ed84118285cb5 100644 --- a/src/hotspot/os/windows/globals_windows.hpp +++ b/src/hotspot/os/windows/globals_windows.hpp @@ -28,24 +28,25 @@ // // Declare Windows specific flags. They are not available on other platforms. // -#define RUNTIME_OS_FLAGS(develop, \ - develop_pd, \ - product, \ - product_pd, \ - notproduct, \ - range, \ - constraint) +#define RUNTIME_OS_FLAGS(develop, \ + develop_pd, \ + product, \ + product_pd, \ + notproduct, \ + range, \ + constraint) \ + \ +product(bool, UseOSErrorReporting, false \ + "Let VM fatal error propagate to the OS (ie. WER on Windows)") // end of RUNTIME_OS_FLAGS - // // Defines Windows-specific default values. The flags are available on all // platforms, but they may have different default values on other platforms. // define_pd_global(bool, UseLargePages, false); define_pd_global(bool, UseLargePagesIndividualAllocation, true); -define_pd_global(bool, UseOSErrorReporting, false); // for now. define_pd_global(bool, UseThreadPriorities, true) ; #endif // OS_WINDOWS_GLOBALS_WINDOWS_HPP diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 4b77de989418c..4bbd1824c5be0 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -532,9 +532,6 @@ const intx ObjectAlignmentInBytes = 8; "error log in case of a crash.") \ range(0, (uint64_t)max_jlong/1000) \ \ - product_pd(bool, UseOSErrorReporting, \ - "Let VM fatal error propagate to the OS (ie. WER on Windows)") \ - \ product(bool, SuppressFatalErrorMessage, false, \ "Report NO fatal error message (avoid deadlock)") \ \ From c8badf58e7357584a3c14ef6999bc473dc514a0e Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Thu, 29 Oct 2020 09:48:50 -0500 Subject: [PATCH 5/6] last tweaks and fixes --- src/hotspot/os/windows/globals_windows.hpp | 2 +- src/hotspot/os/windows/os_windows.cpp | 2 +- src/hotspot/share/utilities/vmError.cpp | 8 ++------ 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/hotspot/os/windows/globals_windows.hpp b/src/hotspot/os/windows/globals_windows.hpp index ed84118285cb5..61157041f887b 100644 --- a/src/hotspot/os/windows/globals_windows.hpp +++ b/src/hotspot/os/windows/globals_windows.hpp @@ -36,7 +36,7 @@ range, \ constraint) \ \ -product(bool, UseOSErrorReporting, false \ +product(bool, UseOSErrorReporting, false, \ "Let VM fatal error propagate to the OS (ie. WER on Windows)") // end of RUNTIME_OS_FLAGS diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index 0416605e30903..73313434d3fdf 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -2354,7 +2354,7 @@ static inline void report_error(Thread* t, DWORD exception_code, address addr, void* siginfo, void* context) { VMError::report_and_die(t, exception_code, addr, siginfo, context); - // If UseOsErrorReporting, this will return here and save the error file + // If UseOSErrorReporting, this will return here and save the error file // somewhere where we can find it in the minidump. } diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 134a088f62173..4840a21281878 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -1434,7 +1434,7 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt reset_signal_handlers(); } else { #if defined(_WINDOWS) - // If UseOsErrorReporting we call this for each level of the call stack + // If UseOSErrorReporting we call this for each level of the call stack // while searching for the exception handler. Only the first level needs // to be reported. if (UseOSErrorReporting && log_done) return; @@ -1628,11 +1628,7 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt OnError = NULL; } -#if defined(_WINDOWS) - if (!UseOSErrorReporting) { -#else - if (true) { -#endif + if (WINDOWS_ONLY(!UseOsErrorReporting) NOT_WINDOWS(true)) { // os::abort() will call abort hooks, try it first. static bool skip_os_abort = false; if (!skip_os_abort) { From e21976a055d8a859b65cd2baa27f621cc0d9e3cc Mon Sep 17 00:00:00 2001 From: gerard-ziemski Date: Fri, 30 Oct 2020 10:51:21 -0500 Subject: [PATCH 6/6] Fixed one more leftover UseOsErrorReporting to UseOSErrorReporting --- src/hotspot/share/utilities/vmError.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/utilities/vmError.cpp b/src/hotspot/share/utilities/vmError.cpp index 4840a21281878..15a4b4667ed7e 100644 --- a/src/hotspot/share/utilities/vmError.cpp +++ b/src/hotspot/share/utilities/vmError.cpp @@ -1628,7 +1628,7 @@ void VMError::report_and_die(int id, const char* message, const char* detail_fmt OnError = NULL; } - if (WINDOWS_ONLY(!UseOsErrorReporting) NOT_WINDOWS(true)) { + if (WINDOWS_ONLY(!UseOSErrorReporting) NOT_WINDOWS(true)) { // os::abort() will call abort hooks, try it first. static bool skip_os_abort = false; if (!skip_os_abort) {