Skip to content

Commit 95e1ccc

Browse files
author
duke
committed
Automatic merge of jdk:master into master
2 parents a646fe9 + e7a2d5c commit 95e1ccc

File tree

3 files changed

+61
-67
lines changed

3 files changed

+61
-67
lines changed

src/hotspot/os/posix/signals_posix.cpp

Lines changed: 54 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -451,46 +451,62 @@ extern "C" JNIEXPORT int JVM_handle_linux_signal(int signo, siginfo_t* siginfo,
451451
int abort_if_unrecognized);
452452
#endif
453453

454-
#if defined(AIX)
455454

456-
// Set thread signal mask (for some reason on AIX sigthreadmask() seems
457-
// to be the thing to call; documentation is not terribly clear about whether
458-
// pthread_sigmask also works, and if it does, whether it does the same.
459-
bool set_thread_signal_mask(int how, const sigset_t* set, sigset_t* oset) {
460-
const int rc = ::pthread_sigmask(how, set, oset);
461-
// return value semantics differ slightly for error case:
462-
// pthread_sigmask returns error number, sigthreadmask -1 and sets global errno
463-
// (so, pthread_sigmask is more theadsafe for error handling)
464-
// But success is always 0.
465-
return rc == 0 ? true : false;
455+
///// Synchronous (non-deferrable) error signals (ILL, SEGV, FPE, BUS, TRAP):
456+
457+
// These signals are special because they cannot be deferred and, if they
458+
// happen while delivery is blocked for the receiving thread, will cause UB
459+
// (in practice typically resulting in sudden process deaths or hangs, see
460+
// JDK-8252533). So we must take care never to block them when we cannot be
461+
// absolutely sure they won't happen. In practice, this is always.
462+
//
463+
// Relevant Posix quote:
464+
// "The behavior of a process is undefined after it ignores a SIGFPE, SIGILL,
465+
// SIGSEGV, or SIGBUS signal that was not generated by kill(), sigqueue(), or
466+
// raise()."
467+
//
468+
// We also include SIGTRAP in that list of never-to-block-signals. While not
469+
// mentioned by the Posix documentation, in our (SAPs) experience blocking it
470+
// causes similar problems. Beside, during normal operation - outside of error
471+
// handling - SIGTRAP may be used for implicit NULL checking, so it makes sense
472+
// to never block it.
473+
//
474+
// We deal with those signals in two ways:
475+
// - we just never explicitly block them, which includes not accidentally blocking
476+
// them via sa_mask when establishing signal handlers.
477+
// - as an additional safety measure, at the entrance of a signal handler, we
478+
// unblock them explicitly.
479+
480+
static void add_error_signals_to_set(sigset_t* set) {
481+
sigaddset(set, SIGILL);
482+
sigaddset(set, SIGBUS);
483+
sigaddset(set, SIGFPE);
484+
sigaddset(set, SIGSEGV);
485+
sigaddset(set, SIGTRAP);
486+
}
487+
488+
static void remove_error_signals_from_set(sigset_t* set) {
489+
sigdelset(set, SIGILL);
490+
sigdelset(set, SIGBUS);
491+
sigdelset(set, SIGFPE);
492+
sigdelset(set, SIGSEGV);
493+
sigdelset(set, SIGTRAP);
466494
}
467495

468-
// Function to unblock all signals which are, according
469-
// to POSIX, typical program error signals. If they happen while being blocked,
470-
// they typically will bring down the process immediately.
471-
bool unblock_program_error_signals() {
496+
// Unblock all signals whose delivery cannot be deferred and which, if they happen
497+
// while delivery is blocked, would cause crashes or hangs (JDK-8252533).
498+
void PosixSignals::unblock_error_signals() {
472499
sigset_t set;
473500
sigemptyset(&set);
474-
sigaddset(&set, SIGILL);
475-
sigaddset(&set, SIGBUS);
476-
sigaddset(&set, SIGFPE);
477-
sigaddset(&set, SIGSEGV);
478-
return set_thread_signal_mask(SIG_UNBLOCK, &set, NULL);
501+
add_error_signals_to_set(&set);
502+
::pthread_sigmask(SIG_UNBLOCK, &set, NULL);
479503
}
480504

481-
#endif
482-
483505
// Renamed from 'signalHandler' to avoid collision with other shared libs.
484506
static void javaSignalHandler(int sig, siginfo_t* info, void* uc) {
485507
assert(info != NULL && uc != NULL, "it must be old kernel");
486508

487-
// TODO: reconcile the differences between Linux/BSD vs AIX here!
488-
#if defined(AIX)
489-
// Never leave program error signals blocked;
490-
// on all our platforms they would bring down the process immediately when
491-
// getting raised while being blocked.
492-
unblock_program_error_signals();
493-
#endif
509+
PosixSignals::unblock_error_signals();
494510

495511
int orig_errno = errno; // Preserve errno value over signal handler.
496512
#if defined(BSD)
@@ -504,6 +520,9 @@ static void javaSignalHandler(int sig, siginfo_t* info, void* uc) {
504520
}
505521

506522
static void UserHandler(int sig, void *siginfo, void *context) {
523+
524+
PosixSignals::unblock_error_signals();
525+
507526
// Ctrl-C is pressed during error reporting, likely because the error
508527
// handler fails to abort. Let VM die immediately.
509528
if (sig == SIGINT && VMError::is_error_reported()) {
@@ -702,23 +721,7 @@ void* os::signal(int signal_number, void* handler) {
702721
struct sigaction sigAct, oldSigAct;
703722

704723
sigfillset(&(sigAct.sa_mask));
705-
706-
#if defined(AIX)
707-
// Do not block out synchronous signals in the signal handler.
708-
// Blocking synchronous signals only makes sense if you can really
709-
// be sure that those signals won't happen during signal handling,
710-
// when the blocking applies. Normal signal handlers are lean and
711-
// do not cause signals. But our signal handlers tend to be "risky"
712-
// - secondary SIGSEGV, SIGILL, SIGBUS' may and do happen.
713-
// On AIX, PASE there was a case where a SIGSEGV happened, followed
714-
// by a SIGILL, which was blocked due to the signal mask. The process
715-
// just hung forever. Better to crash from a secondary signal than to hang.
716-
sigdelset(&(sigAct.sa_mask), SIGSEGV);
717-
sigdelset(&(sigAct.sa_mask), SIGBUS);
718-
sigdelset(&(sigAct.sa_mask), SIGILL);
719-
sigdelset(&(sigAct.sa_mask), SIGFPE);
720-
sigdelset(&(sigAct.sa_mask), SIGTRAP);
721-
#endif
724+
remove_error_signals_from_set(&(sigAct.sa_mask));
722725

723726
sigAct.sa_flags = SA_RESTART|SA_SIGINFO;
724727
sigAct.sa_handler = CAST_TO_FN_PTR(sa_handler_t, handler);
@@ -1099,6 +1102,7 @@ void set_signal_handler(int sig, bool set_installed) {
10991102

11001103
struct sigaction sigAct;
11011104
sigfillset(&(sigAct.sa_mask));
1105+
remove_error_signals_from_set(&(sigAct.sa_mask));
11021106
sigAct.sa_handler = SIG_DFL;
11031107
if (!set_installed) {
11041108
sigAct.sa_flags = SA_SIGINFO|SA_RESTART;
@@ -1303,10 +1307,6 @@ bool PosixSignals::is_sig_ignored(int sig) {
13031307
}
13041308
}
13051309

1306-
int PosixSignals::unblock_thread_signal_mask(const sigset_t *set) {
1307-
return pthread_sigmask(SIG_UNBLOCK, set, NULL);
1308-
}
1309-
13101310
address PosixSignals::ucontext_get_pc(const ucontext_t* ctx) {
13111311
#if defined(AIX)
13121312
return os::Aix::ucontext_get_pc(ctx);
@@ -1470,10 +1470,13 @@ static void suspend_save_context(OSThread *osthread, siginfo_t* siginfo, ucontex
14701470
// Currently only ever called on the VMThread and JavaThreads (PC sampling)
14711471
//
14721472
static void SR_handler(int sig, siginfo_t* siginfo, ucontext_t* context) {
1473+
14731474
// Save and restore errno to avoid confusing native code with EINTR
14741475
// after sigsuspend.
14751476
int old_errno = errno;
14761477

1478+
PosixSignals::unblock_error_signals();
1479+
14771480
Thread* thread = Thread::current_or_null_safe();
14781481
assert(thread != NULL, "Missing current thread in SR_handler");
14791482

@@ -1567,6 +1570,7 @@ int PosixSignals::SR_initialize() {
15671570

15681571
// SR_signum is blocked by default.
15691572
pthread_sigmask(SIG_BLOCK, NULL, &act.sa_mask);
1573+
remove_error_signals_from_set(&(act.sa_mask));
15701574

15711575
if (sigaction(SR_signum, &act, 0) == -1) {
15721576
return -1;

src/hotspot/os/posix/signals_posix.hpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ class PosixSignals : public AllStatic {
4444
static bool is_sig_ignored(int sig);
4545
static void signal_sets_init();
4646

47-
// unblocks the signal masks for current thread
48-
static int unblock_thread_signal_mask(const sigset_t *set);
4947
static void hotspot_sigmask(Thread* thread);
5048

5149
static void print_signal_handler(outputStream* st, int sig, char* buf, size_t buflen);
@@ -64,6 +62,11 @@ class PosixSignals : public AllStatic {
6462

6563
// sun.misc.Signal support
6664
static void jdk_misc_signal_init();
65+
66+
// Unblock all signals whose delivery cannot be deferred and which, if they happen
67+
// while delivery is blocked, would cause crashes or hangs (see JDK-8252533).
68+
static void unblock_error_signals();
69+
6770
};
6871

6972
#endif // OS_POSIX_SIGNALS_POSIX_HPP

src/hotspot/os/posix/vmError_posix.cpp

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,8 @@ address VMError::get_resetted_sighandler(int sig) {
101101
}
102102

103103
static void crash_handler(int sig, siginfo_t* info, void* ucVoid) {
104-
// unmask current signal
105-
sigset_t newset;
106-
sigemptyset(&newset);
107-
sigaddset(&newset, sig);
108-
// also unmask other synchronous signals
109-
for (int i = 0; i < NUM_SIGNALS; i++) {
110-
sigaddset(&newset, SIGNALS[i]);
111-
}
112-
PosixSignals::unblock_thread_signal_mask(&newset);
104+
105+
PosixSignals::unblock_error_signals();
113106

114107
// support safefetch faults in error handling
115108
ucontext_t* const uc = (ucontext_t*) ucVoid;
@@ -139,16 +132,10 @@ static void crash_handler(int sig, siginfo_t* info, void* ucVoid) {
139132
}
140133

141134
void VMError::reset_signal_handlers() {
142-
// install signal handlers for all synchronous program error signals
143-
sigset_t newset;
144-
sigemptyset(&newset);
145-
146135
for (int i = 0; i < NUM_SIGNALS; i++) {
147136
save_signal(i, SIGNALS[i]);
148137
os::signal(SIGNALS[i], CAST_FROM_FN_PTR(void *, crash_handler));
149-
sigaddset(&newset, SIGNALS[i]);
150138
}
151-
PosixSignals::unblock_thread_signal_mask(&newset);
152139
}
153140

154141
// Write a hint to the stream in case siginfo relates to a segv/bus error

0 commit comments

Comments
 (0)