From: Julian Seward Date: Sun, 14 Apr 2002 02:29:29 +0000 (+0000) Subject: Take notice of SA_RESTART flags on signals, so as to deal (at least X-Git-Tag: svn/VALGRIND_1_0_3~384 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d8e7eaed1592f6b68ecc0ae77bc3c30e17780134;p=thirdparty%2Fvalgrind.git Take notice of SA_RESTART flags on signals, so as to deal (at least partially properly) with blocking system calls interrupted by signals. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@62 --- diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h index 52706e0395..5735b1c009 100644 --- a/coregrind/vg_include.h +++ b/coregrind/vg_include.h @@ -588,7 +588,7 @@ extern void VG_(do__NR_sigprocmask) ( Int how, vki_ksigset_t* set ); /* Modify the current thread's state once we have detected it is returning from a signal handler. */ -extern void VG_(signal_returns) ( ThreadId ); +extern Bool VG_(signal_returns) ( ThreadId ); /* Handy utilities to block/restore all host signals. */ extern void VG_(block_all_host_signals) diff --git a/coregrind/vg_kerneliface.h b/coregrind/vg_kerneliface.h index 9ec236acb5..360c259dfa 100644 --- a/coregrind/vg_kerneliface.h +++ b/coregrind/vg_kerneliface.h @@ -134,6 +134,7 @@ typedef /* Copied from /usr/src/linux-2.4.9-13/include/asm/errno.h */ +#define VKI_EINTR 4 /* Interrupted system call */ #define VKI_EINVAL 22 /* Invalid argument */ #define VKI_ENOMEM 12 /* Out of memory */ diff --git a/coregrind/vg_scheduler.c b/coregrind/vg_scheduler.c index ecf44668aa..cb7d5a943c 100644 --- a/coregrind/vg_scheduler.c +++ b/coregrind/vg_scheduler.c @@ -35,15 +35,24 @@ #include "valgrind.h" /* for VG_USERREQ__MAKE_NOACCESS and VG_USERREQ__DO_LEAK_CHECK */ -/* BORKAGE as of 11 Apr 02 +/* BORKAGE/ISSUES as of 14 Apr 02 -Note! This implementation is so poor as to not be suitable for use by -anyone at all! +Note! This pthreads implementation is so poor as to not be +suitable for use by anyone at all! -- properly save scheduler private state in signal delivery frames. +- Currently, when a signal is run, just the ThreadStatus.status fields + are saved in the signal frame, along with the CPU state. Question: + should I also save and restore: + ThreadStatus.joiner + ThreadStatus.waited_on_mid + ThreadStatus.awaken_at + ThreadStatus.retval + Currently unsure, and so am not doing so. -- signals interrupting read/write and nanosleep, and take notice - of SA_RESTART or not +- Signals interrupting read/write and nanosleep: SA_RESTART settings. + Read/write correctly return with EINTR when SA_RESTART isn't + specified and they are interrupted by a signal. nanosleep just + pretends signals don't exist -- should be fixed. - when a thread is done mark its stack as noaccess @@ -1657,6 +1666,44 @@ static void do_pthread_mutex_destroy ( ThreadId tid, } +/* vthread tid is returning from a signal handler; modify its + stack/regs accordingly. */ +static +void handle_signal_return ( ThreadId tid ) +{ + Char msg_buf[100]; + Bool restart_blocked_syscalls = VG_(signal_returns)(tid); + + if (restart_blocked_syscalls) + /* Easy; we don't have to do anything. */ + return; + + if (vg_threads[tid].status == VgTs_WaitFD) { + vg_assert(vg_threads[tid].m_eax == __NR_read + || vg_threads[tid].m_eax == __NR_write); + /* read() or write() interrupted. Force a return with EINTR. */ + vg_threads[tid].m_eax = -VKI_EINTR; + vg_threads[tid].status = VgTs_Runnable; + if (VG_(clo_trace_sched)) { + VG_(sprintf)(msg_buf, + "read() / write() interrupted by signal; return EINTR" ); + print_sched_event(tid, msg_buf); + } + return; + } + + if (vg_threads[tid].status == VgTs_WaitFD) { + vg_assert(vg_threads[tid].m_eax == __NR_nanosleep); + /* We interrupted a nanosleep(). The right thing to do is to + write the unused time to nanosleep's second param and return + EINTR, but I'm too lazy for that. */ + return; + } + + /* All other cases? Just return. */ +} + + /* --------------------------------------------------------------------- Handle non-trivial client requests. ------------------------------------------------------------------ */ @@ -1725,11 +1772,9 @@ void do_nontrivial_clientreq ( ThreadId tid ) vg_threads[tid].m_edx = VG_(handle_client_request) ( arg ); break; - case VG_USERREQ__SIGNAL_RETURNS: - /* vthread tid is returning from a signal handler; - modify its stack/regs accordingly. */ - VG_(signal_returns)(tid); - break; + case VG_USERREQ__SIGNAL_RETURNS: + handle_signal_return(tid); + break; default: VG_(printf)("panic'd on private request = 0x%x\n", arg[0] ); diff --git a/coregrind/vg_signals.c b/coregrind/vg_signals.c index 33ff722294..4687e64e42 100644 --- a/coregrind/vg_signals.c +++ b/coregrind/vg_signals.c @@ -45,6 +45,7 @@ receive a signal for which the corresponding handler is NULL. */ void* VG_(sighandler)[VKI_KNSIG]; + /* For each signal, either: -- VG_SIGIDLE if not pending and not running -- Handler address if pending @@ -58,6 +59,15 @@ void* VG_(sighandler)[VKI_KNSIG]; void* VG_(sigpending)[VKI_KNSIG]; +/* For each signal that we have a handler for (ie, for those for which + the VG_(sighandler) entry is non-NULL), record whether or not the + client asked for syscalls to be restartable (SA_RESTART) if + interrupted by this signal. We need to consult this when a signal + returns, if it should happen that the signal which we delivered has + interrupted a system call. */ +Bool vg_sig_sarestart[VKI_KNSIG]; + + /* --------------------------------------------------------------------- Handy utilities to block/restore all host signals. ------------------------------------------------------------------ */ @@ -248,9 +258,10 @@ Int vg_pop_signal_frame ( ThreadId tid ) /* A handler is returning. Restore the machine state from the stacked VgSigContext and continue with whatever was going on before the - handler ran. */ + handler ran. Returns the SA_RESTART syscall-restartability-status + of the delivered signal. */ -void VG_(signal_returns) ( ThreadId tid ) +Bool VG_(signal_returns) ( ThreadId tid ) { Int sigNo; vki_ksigset_t saved_procmask; @@ -286,7 +297,10 @@ void VG_(signal_returns) ( ThreadId tid ) /* Unlock and return. */ VG_(restore_host_signals)( &saved_procmask ); - /* Scheduler now can resume this thread, or perhaps some other. */ + /* Scheduler now can resume this thread, or perhaps some other. + Tell the scheduler whether or not any syscall interrupted by + this signal should be restarted, if possible, or no. */ + return vg_sig_sarestart[sigNo]; } @@ -508,8 +522,11 @@ void VG_(sigstartup_actions) ( void ) } /* Set initial state for the signal simulation. */ - for (i = 1; i < VKI_KNSIG; i++) - VG_(sighandler[i]) = VG_(sigpending[i]) = NULL; + for (i = 1; i < VKI_KNSIG; i++) { + VG_(sighandler)[i] = NULL; + VG_(sigpending)[i] = NULL; + vg_sig_sarestart[i] = True; /* An easy default */ + } for (i = 1; i < VKI_KNSIG; i++) { @@ -526,8 +543,14 @@ void VG_(sigstartup_actions) ( void ) if ((sa.ksa_flags & VKI_SA_ONSTACK) != 0) VG_(unimplemented) ("signals on an alternative stack (SA_ONSTACK)"); - VG_(sighandler[i]) = sa.ksa_handler; + + VG_(sighandler)[i] = sa.ksa_handler; sa.ksa_handler = &VG_(oursignalhandler); + /* Save the restart status, then set it to restartable. */ + vg_sig_sarestart[i] + = (sa.ksa_flags & VKI_SA_RESTART) ? True : False; + sa.ksa_flags |= VKI_SA_RESTART; + ret = VG_(ksigaction)(i, &sa, NULL); vg_assert(ret == 0); } @@ -561,6 +584,8 @@ void VG_(sigshutdown_actions) ( void ) VG_(block_all_host_signals)( &saved_procmask ); /* copy the sim signal actions to the real ones. */ + /* Hmm, this isn't accurate. Doesn't properly restore the + SA_RESTART flag nor SA_ONSTACK. */ for (i = 1; i < VKI_KNSIG; i++) { if (i == VKI_SIGKILL || i == VKI_SIGSTOP) continue; if (VG_(sighandler)[i] == NULL) continue; @@ -630,6 +655,9 @@ void VG_(do__NR_sigaction) ( ThreadId tid ) ("signals on an alternative stack (SA_ONSTACK)"); new_action->ksa_flags |= VKI_SA_ONSTACK; VG_(sighandler)[param1] = new_action->ksa_handler; + vg_sig_sarestart[param1] + = (new_action->ksa_flags & VKI_SA_RESTART) ? True : False; + new_action->ksa_flags |= VKI_SA_RESTART; new_action->ksa_handler = &VG_(oursignalhandler); } } @@ -655,6 +683,11 @@ void VG_(do__NR_sigaction) ( ThreadId tid ) sig stack, unset the bit for anything we pass back to it. */ old_action->ksa_flags &= ~VKI_SA_ONSTACK; + /* Restore the SA_RESTART flag to whatever we snaffled. */ + if (vg_sig_sarestart[param1]) + old_action->ksa_flags |= VKI_SA_RESTART; + else + old_action->ksa_flags &= ~VKI_SA_RESTART; } } diff --git a/vg_include.h b/vg_include.h index 52706e0395..5735b1c009 100644 --- a/vg_include.h +++ b/vg_include.h @@ -588,7 +588,7 @@ extern void VG_(do__NR_sigprocmask) ( Int how, vki_ksigset_t* set ); /* Modify the current thread's state once we have detected it is returning from a signal handler. */ -extern void VG_(signal_returns) ( ThreadId ); +extern Bool VG_(signal_returns) ( ThreadId ); /* Handy utilities to block/restore all host signals. */ extern void VG_(block_all_host_signals) diff --git a/vg_kerneliface.h b/vg_kerneliface.h index 9ec236acb5..360c259dfa 100644 --- a/vg_kerneliface.h +++ b/vg_kerneliface.h @@ -134,6 +134,7 @@ typedef /* Copied from /usr/src/linux-2.4.9-13/include/asm/errno.h */ +#define VKI_EINTR 4 /* Interrupted system call */ #define VKI_EINVAL 22 /* Invalid argument */ #define VKI_ENOMEM 12 /* Out of memory */ diff --git a/vg_scheduler.c b/vg_scheduler.c index ecf44668aa..cb7d5a943c 100644 --- a/vg_scheduler.c +++ b/vg_scheduler.c @@ -35,15 +35,24 @@ #include "valgrind.h" /* for VG_USERREQ__MAKE_NOACCESS and VG_USERREQ__DO_LEAK_CHECK */ -/* BORKAGE as of 11 Apr 02 +/* BORKAGE/ISSUES as of 14 Apr 02 -Note! This implementation is so poor as to not be suitable for use by -anyone at all! +Note! This pthreads implementation is so poor as to not be +suitable for use by anyone at all! -- properly save scheduler private state in signal delivery frames. +- Currently, when a signal is run, just the ThreadStatus.status fields + are saved in the signal frame, along with the CPU state. Question: + should I also save and restore: + ThreadStatus.joiner + ThreadStatus.waited_on_mid + ThreadStatus.awaken_at + ThreadStatus.retval + Currently unsure, and so am not doing so. -- signals interrupting read/write and nanosleep, and take notice - of SA_RESTART or not +- Signals interrupting read/write and nanosleep: SA_RESTART settings. + Read/write correctly return with EINTR when SA_RESTART isn't + specified and they are interrupted by a signal. nanosleep just + pretends signals don't exist -- should be fixed. - when a thread is done mark its stack as noaccess @@ -1657,6 +1666,44 @@ static void do_pthread_mutex_destroy ( ThreadId tid, } +/* vthread tid is returning from a signal handler; modify its + stack/regs accordingly. */ +static +void handle_signal_return ( ThreadId tid ) +{ + Char msg_buf[100]; + Bool restart_blocked_syscalls = VG_(signal_returns)(tid); + + if (restart_blocked_syscalls) + /* Easy; we don't have to do anything. */ + return; + + if (vg_threads[tid].status == VgTs_WaitFD) { + vg_assert(vg_threads[tid].m_eax == __NR_read + || vg_threads[tid].m_eax == __NR_write); + /* read() or write() interrupted. Force a return with EINTR. */ + vg_threads[tid].m_eax = -VKI_EINTR; + vg_threads[tid].status = VgTs_Runnable; + if (VG_(clo_trace_sched)) { + VG_(sprintf)(msg_buf, + "read() / write() interrupted by signal; return EINTR" ); + print_sched_event(tid, msg_buf); + } + return; + } + + if (vg_threads[tid].status == VgTs_WaitFD) { + vg_assert(vg_threads[tid].m_eax == __NR_nanosleep); + /* We interrupted a nanosleep(). The right thing to do is to + write the unused time to nanosleep's second param and return + EINTR, but I'm too lazy for that. */ + return; + } + + /* All other cases? Just return. */ +} + + /* --------------------------------------------------------------------- Handle non-trivial client requests. ------------------------------------------------------------------ */ @@ -1725,11 +1772,9 @@ void do_nontrivial_clientreq ( ThreadId tid ) vg_threads[tid].m_edx = VG_(handle_client_request) ( arg ); break; - case VG_USERREQ__SIGNAL_RETURNS: - /* vthread tid is returning from a signal handler; - modify its stack/regs accordingly. */ - VG_(signal_returns)(tid); - break; + case VG_USERREQ__SIGNAL_RETURNS: + handle_signal_return(tid); + break; default: VG_(printf)("panic'd on private request = 0x%x\n", arg[0] ); diff --git a/vg_signals.c b/vg_signals.c index 33ff722294..4687e64e42 100644 --- a/vg_signals.c +++ b/vg_signals.c @@ -45,6 +45,7 @@ receive a signal for which the corresponding handler is NULL. */ void* VG_(sighandler)[VKI_KNSIG]; + /* For each signal, either: -- VG_SIGIDLE if not pending and not running -- Handler address if pending @@ -58,6 +59,15 @@ void* VG_(sighandler)[VKI_KNSIG]; void* VG_(sigpending)[VKI_KNSIG]; +/* For each signal that we have a handler for (ie, for those for which + the VG_(sighandler) entry is non-NULL), record whether or not the + client asked for syscalls to be restartable (SA_RESTART) if + interrupted by this signal. We need to consult this when a signal + returns, if it should happen that the signal which we delivered has + interrupted a system call. */ +Bool vg_sig_sarestart[VKI_KNSIG]; + + /* --------------------------------------------------------------------- Handy utilities to block/restore all host signals. ------------------------------------------------------------------ */ @@ -248,9 +258,10 @@ Int vg_pop_signal_frame ( ThreadId tid ) /* A handler is returning. Restore the machine state from the stacked VgSigContext and continue with whatever was going on before the - handler ran. */ + handler ran. Returns the SA_RESTART syscall-restartability-status + of the delivered signal. */ -void VG_(signal_returns) ( ThreadId tid ) +Bool VG_(signal_returns) ( ThreadId tid ) { Int sigNo; vki_ksigset_t saved_procmask; @@ -286,7 +297,10 @@ void VG_(signal_returns) ( ThreadId tid ) /* Unlock and return. */ VG_(restore_host_signals)( &saved_procmask ); - /* Scheduler now can resume this thread, or perhaps some other. */ + /* Scheduler now can resume this thread, or perhaps some other. + Tell the scheduler whether or not any syscall interrupted by + this signal should be restarted, if possible, or no. */ + return vg_sig_sarestart[sigNo]; } @@ -508,8 +522,11 @@ void VG_(sigstartup_actions) ( void ) } /* Set initial state for the signal simulation. */ - for (i = 1; i < VKI_KNSIG; i++) - VG_(sighandler[i]) = VG_(sigpending[i]) = NULL; + for (i = 1; i < VKI_KNSIG; i++) { + VG_(sighandler)[i] = NULL; + VG_(sigpending)[i] = NULL; + vg_sig_sarestart[i] = True; /* An easy default */ + } for (i = 1; i < VKI_KNSIG; i++) { @@ -526,8 +543,14 @@ void VG_(sigstartup_actions) ( void ) if ((sa.ksa_flags & VKI_SA_ONSTACK) != 0) VG_(unimplemented) ("signals on an alternative stack (SA_ONSTACK)"); - VG_(sighandler[i]) = sa.ksa_handler; + + VG_(sighandler)[i] = sa.ksa_handler; sa.ksa_handler = &VG_(oursignalhandler); + /* Save the restart status, then set it to restartable. */ + vg_sig_sarestart[i] + = (sa.ksa_flags & VKI_SA_RESTART) ? True : False; + sa.ksa_flags |= VKI_SA_RESTART; + ret = VG_(ksigaction)(i, &sa, NULL); vg_assert(ret == 0); } @@ -561,6 +584,8 @@ void VG_(sigshutdown_actions) ( void ) VG_(block_all_host_signals)( &saved_procmask ); /* copy the sim signal actions to the real ones. */ + /* Hmm, this isn't accurate. Doesn't properly restore the + SA_RESTART flag nor SA_ONSTACK. */ for (i = 1; i < VKI_KNSIG; i++) { if (i == VKI_SIGKILL || i == VKI_SIGSTOP) continue; if (VG_(sighandler)[i] == NULL) continue; @@ -630,6 +655,9 @@ void VG_(do__NR_sigaction) ( ThreadId tid ) ("signals on an alternative stack (SA_ONSTACK)"); new_action->ksa_flags |= VKI_SA_ONSTACK; VG_(sighandler)[param1] = new_action->ksa_handler; + vg_sig_sarestart[param1] + = (new_action->ksa_flags & VKI_SA_RESTART) ? True : False; + new_action->ksa_flags |= VKI_SA_RESTART; new_action->ksa_handler = &VG_(oursignalhandler); } } @@ -655,6 +683,11 @@ void VG_(do__NR_sigaction) ( ThreadId tid ) sig stack, unset the bit for anything we pass back to it. */ old_action->ksa_flags &= ~VKI_SA_ONSTACK; + /* Restore the SA_RESTART flag to whatever we snaffled. */ + if (vg_sig_sarestart[param1]) + old_action->ksa_flags |= VKI_SA_RESTART; + else + old_action->ksa_flags &= ~VKI_SA_RESTART; } }