From eaefbe1f25cb09238feb037e4a2f138076ca353f Mon Sep 17 00:00:00 2001 From: Philippe Waroquiers Date: Sat, 24 Sep 2016 12:06:34 +0000 Subject: [PATCH] Fix 361615 - Inconsistent termination for multithreaded process terminated by signal Test program by earl_chew git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15982 --- NEWS | 1 + coregrind/m_main.c | 6 +- coregrind/m_scheduler/scheduler.c | 5 -- coregrind/m_signals.c | 9 +-- coregrind/pub_core_scheduler.h | 4 +- none/tests/Makefile.am | 4 +- none/tests/pth_term_signal.c | 91 +++++++++++++++++++++++++++ none/tests/pth_term_signal.stderr.exp | 1 + none/tests/pth_term_signal.vgtest | 2 + 9 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 none/tests/pth_term_signal.c create mode 100644 none/tests/pth_term_signal.stderr.exp create mode 100644 none/tests/pth_term_signal.vgtest diff --git a/NEWS b/NEWS index 6254946dfb..5231a5c1b2 100644 --- a/NEWS +++ b/NEWS @@ -143,6 +143,7 @@ where XXXXXX is the bug number as listed below. 361226 s390x: risbgn (EC59) not implemented 361253 [s390x] ex_clone.c:42: undefined reference to `pthread_create' 361354 ppc64[le]: wire up separate socketcalls system calls +361615 Inconsistent termination for multithreaded process terminated by signal 361926 Unhandled Solaris syscall: sysfs(84) 362009 Valgrind dumps core on unimplemented functionality before threads are created 362329 Valgrind does not support the IBM POWER ISA 3.0 instructions, part 3/5 diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 80f347e6c1..baf2ebf38c 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -2705,7 +2705,11 @@ void shutdown_actions_NORETURN( ThreadId tid, sys_exit, do likewise; if the (last) thread stopped due to a fatal signal, terminate the entire system with that same fatal signal. */ VG_(debugLog)(1, "core_os", - "VG_(terminate_NORETURN)(tid=%u)\n", tid); + "VG_(terminate_NORETURN)(tid=%u) schedretcode %s" + " os_state.exit_code %d fatalsig %d\n", + tid, VG_(name_of_VgSchedReturnCode)(tids_schedretcode), + VG_(threads)[tid].os_state.exitcode, + VG_(threads)[tid].os_state.fatalsig); switch (tids_schedretcode) { case VgSrc_ExitThread: /* the normal way out (Linux, Solaris) */ diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index 9aa854db6e..727275c29a 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -1653,11 +1653,6 @@ VgSchedReturnCode VG_(scheduler) ( ThreadId tid ) } -/* - This causes all threads to forceably exit. They aren't actually - dead by the time this returns; you need to call - VG_(reap_threads)() to wait for them. - */ void VG_(nuke_all_threads_except) ( ThreadId me, VgSchedReturnCode src ) { ThreadId tid; diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c index 9146d0c5a7..168b681e97 100644 --- a/coregrind/m_signals.c +++ b/coregrind/m_signals.c @@ -1654,8 +1654,8 @@ static Bool is_signal_from_kernel(ThreadId tid, int signum, int si_code) /* Perform the default action of a signal. If the signal is fatal, it - marks all threads as needing to exit, but it doesn't actually kill - the process or thread. + terminates all other threads, but it doesn't actually kill + the process and calling thread. If we're not being quiet, then print out some more detail about fatal signals (esp. core dumping signals). @@ -1933,12 +1933,13 @@ static void default_action(const vki_siginfo_t *info, ThreadId tid) VG_(setrlimit)(VKI_RLIMIT_CORE, &zero); } - /* stash fatal signal in main thread */ // what's this for? //VG_(threads)[VG_(master_tid)].os_state.fatalsig = sigNo; - /* everyone dies */ + /* everyone but tid dies */ VG_(nuke_all_threads_except)(tid, VgSrc_FatalSig); + VG_(reap_threads)(tid); + /* stash fatal signal in this thread */ VG_(threads)[tid].exitreason = VgSrc_FatalSig; VG_(threads)[tid].os_state.fatalsig = sigNo; } diff --git a/coregrind/pub_core_scheduler.h b/coregrind/pub_core_scheduler.h index 0851950958..aa9610e43d 100644 --- a/coregrind/pub_core_scheduler.h +++ b/coregrind/pub_core_scheduler.h @@ -51,7 +51,9 @@ extern void VG_(exit_thread)(ThreadId tid); If it isn't blocked in a syscall, has no effect on the thread. */ extern void VG_(get_thread_out_of_syscall)(ThreadId tid); -/* Nuke all threads except tid. */ +/* This causes all threads except tid to forceably exit. They aren't actually + dead by the time this returns; you need to call + VG_(reap_threads)() to wait for them. */ extern void VG_(nuke_all_threads_except) ( ThreadId me, VgSchedReturnCode reason ); diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 7059e67090..11bc3fa958 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -167,6 +167,7 @@ EXTRA_DIST = \ pth_rwlock.stderr.exp pth_rwlock.vgtest \ pth_stackalign.stderr.exp \ pth_stackalign.stdout.exp pth_stackalign.vgtest \ + pth_term_signal.stderr.exp pth_term_signal.vgtest \ rcrl.stderr.exp rcrl.stdout.exp rcrl.vgtest \ readline1.stderr.exp readline1.stdout.exp \ readline1.vgtest \ @@ -224,7 +225,7 @@ check_PROGRAMS = \ pselect_sigmask_null \ pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple \ pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock \ - pth_stackalign \ + pth_stackalign pth_term_signal\ rcrl readline1 \ require-text-symbol \ res_search resolv \ @@ -315,6 +316,7 @@ if VGCONF_OS_IS_SOLARIS pth_rwlock_CFLAGS += --std=c99 endif pth_stackalign_LDADD = -lpthread +pth_term_signal_LDADD = -lpthread res_search_LDADD = -lresolv -lpthread resolv_CFLAGS = $(AM_CFLAGS) resolv_LDADD = -lresolv -lpthread diff --git a/none/tests/pth_term_signal.c b/none/tests/pth_term_signal.c new file mode 100644 index 0000000000..9d8f05cc30 --- /dev/null +++ b/none/tests/pth_term_signal.c @@ -0,0 +1,91 @@ +#include +#include +#include +#include +#include + +#include + +void * +slavethread(void *arg) +{ + sigset_t sigmask; + + if (sigfillset(&sigmask)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + _exit(255); + } + + if (pthread_sigmask(SIG_UNBLOCK, &sigmask, 0)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + _exit(255); + } + + while (1) + sleep(1); +} + +void +childprocess() +{ + pthread_t slave; + + if (pthread_create(&slave, 0, &slavethread, 0)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + _exit(255); + } + + while (1) + sleep(1); +} + +int main(int argc, char **argv) +{ + sigset_t sigmask; + + if (sigfillset(&sigmask)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + return 255; + } + + if (pthread_sigmask(SIG_BLOCK, &sigmask, 0)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + return 255; + } + + int childpid = fork(); + + if (-1 == childpid) + { + fprintf(stderr, "Error line %u\n", __LINE__); + return 255; + } + + if ( ! childpid) + childprocess(); + + if (kill(childpid, SIGTERM)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + return 255; + } + + int status; + if (childpid != waitpid(childpid, &status, 0)) + { + fprintf(stderr, "Error line %u\n", __LINE__); + return 255; + } + + assert(WIFSIGNALED(status)); + + fprintf(stderr, "Signal %d\n", WTERMSIG(status)); + assert(WTERMSIG(status) == SIGTERM); + + return 0; +} diff --git a/none/tests/pth_term_signal.stderr.exp b/none/tests/pth_term_signal.stderr.exp new file mode 100644 index 0000000000..272cf3186f --- /dev/null +++ b/none/tests/pth_term_signal.stderr.exp @@ -0,0 +1 @@ +Signal 15 diff --git a/none/tests/pth_term_signal.vgtest b/none/tests/pth_term_signal.vgtest new file mode 100644 index 0000000000..9591605767 --- /dev/null +++ b/none/tests/pth_term_signal.vgtest @@ -0,0 +1,2 @@ +prog: pth_term_signal +vgopts: -q -- 2.47.2