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 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 52706e0..5735b1c 100644
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -588,7 +588,7 @@
/* 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 9ec236a..360c259 100644
--- a/coregrind/vg_kerneliface.h
+++ b/coregrind/vg_kerneliface.h
@@ -134,6 +134,7 @@
/* 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 ecf4466..cb7d5a9 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 @@
}
+/* 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 @@
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 33ff722..4687e64 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_(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 @@
/* 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 @@
/* 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 @@
}
/* 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 @@
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 @@
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 @@
("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 @@
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 52706e0..5735b1c 100644
--- a/vg_include.h
+++ b/vg_include.h
@@ -588,7 +588,7 @@
/* 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 9ec236a..360c259 100644
--- a/vg_kerneliface.h
+++ b/vg_kerneliface.h
@@ -134,6 +134,7 @@
/* 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 ecf4466..cb7d5a9 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 @@
}
+/* 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 @@
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 33ff722..4687e64 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_(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 @@
/* 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 @@
/* 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 @@
}
/* 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 @@
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 @@
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 @@
("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 @@
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;
}
}