Menu
▾
▴
[Valgrind-developers] Valgrind: r13896 - in /trunk: ./ coregrind/ gdbserver_tests/
|
From: <sv...@va...> - 2014-04-15 22:35:38
|
Author: philippe
Date: Tue Apr 15 22:35:23 2014
New Revision: 13896
Log:
vgdb must not transmit signals when gdbserver has been ptrace-invoked.
Most of the time, Valgrind masks async signals, and polls for such
signals at regular interval.
There is a very narrow range of code (around client syscall logic)
where such signals are unmasked (as they must be able to interrupt
syscalls).
This is the only range of code where Valgrind is expecting to
receive such a signal.
When gdbserver is ptraced invoked, Valgrind is artificially made
to jump out of this code portion. Signals are not masked.
When ptraceing valgrind, vgdb will get these signals but cannot
transmit them immediately, otherwise they arrive in range
of code where they are not expected (causing internal error
in syscall logic) and/or causing gdbserver syscalls to be
interrupted.
3 solutions to solve that were looked at:
1. have the gdbserver code masking signals "as quickly as possible".
Easy to implement, but this leaves still a small window
of code where signals are not masked and would cause a problem.
2. have vgdb setting the SIGMASK of valgrind before invoking
gdbserver.
This would be easy to implement, but changing the SIGMASK
of the ptrace-d process is only available on very recent kernels.
3. have vgdb queuing signals, and only transmitting them once
gdbserver invocation is finished.
This 3rd solution has been implemented.
Added:
trunk/gdbserver_tests/nlvgdbsigqueue.stderr.exp
trunk/gdbserver_tests/nlvgdbsigqueue.stderrB.exp
trunk/gdbserver_tests/nlvgdbsigqueue.stdinB.gdb
trunk/gdbserver_tests/nlvgdbsigqueue.stdoutB.exp
trunk/gdbserver_tests/nlvgdbsigqueue.vgtest
trunk/gdbserver_tests/send_signal (with props)
Modified:
trunk/NEWS
trunk/coregrind/vgdb-invoker-ptrace.c
trunk/coregrind/vgdb.c
trunk/coregrind/vgdb.h
trunk/gdbserver_tests/Makefile.am
trunk/gdbserver_tests/sleepers.c
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Tue Apr 15 22:35:23 2014
@@ -77,6 +77,7 @@
n-i-bz Fix KVM_CREATE_IRQCHIP ioctl handling
n-i-bz s390x: Fix memory corruption for multithreaded applications
n-i-bz vex arm->IR: allow PC as basereg in some LDRD cases
+n-i-bz internal error in Valgrind if vgdb transmit signals when ptrace invoked
Release 3.9.0 (31 October 2013)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Modified: trunk/coregrind/vgdb-invoker-ptrace.c
==============================================================================
--- trunk/coregrind/vgdb-invoker-ptrace.c (original)
+++ trunk/coregrind/vgdb-invoker-ptrace.c Tue Apr 15 22:35:23 2014
@@ -60,8 +60,18 @@
# error "unexpected wordsize"
#endif
+// if > 0, pid for which registers have to be restored.
+// if == 0, means we have not yet called setregs (or have already
+// restored the registers).
+static int pid_of_save_regs = 0;
/* True if we have continued pid_of_save_regs after PTRACE_ATTACH. */
static Bool pid_of_save_regs_continued = False;
+// When setregs has been called to change the registers of pid_of_save_regs,
+// vgdb cannot transmit the signals intercepted during ptrace.
+// So, we queue them, and will deliver them when detaching.
+// See function waitstopped for more info.
+static int signal_queue_sz = 0;
+static siginfo_t *signal_queue;
/* True when loss of connection indicating that the Valgrind
process is dying. */
@@ -252,9 +262,44 @@
break;
/* pid received a signal which is not the signal we are waiting for.
- We continue pid, transmitting this signal. */
- DEBUG(1, "waitstopped PTRACE_CONT with signal %d\n", signal_received);
- res = ptrace (PTRACE_CONT, pid, NULL, signal_received);
+ If we have not (yet) changed the registers of the inferior
+ or we have (already) reset them, we can transmit the signal.
+
+ If we have already set the registers of the inferior, we cannot
+ transmit the signal, as this signal would arrive when the
+ gdbserver code runs. And valgrind only expects signals to
+ arrive in a small code portion around
+ client syscall logic, where signal are unmasked (see e.g.
+ m_syswrap/syscall-x86-linux.S ML_(do_syscall_for_client_WRK).
+
+ As ptrace is forcing a call to gdbserver by jumping
+ 'out of this region', signals are not masked, but
+ will arrive outside of the allowed/expected code region.
+ So, if we have changed the registers of the inferior, we
+ rather queue the signal to transmit them when detaching,
+ after having restored the registers to the initial values. */
+ if (pid_of_save_regs) {
+ siginfo_t *newsiginfo;
+
+ // realloc a bigger queue, and store new signal at the end.
+ // This is not very efficient but we assume not many sigs are queued.
+ signal_queue_sz++;
+ signal_queue = vrealloc(signal_queue, sizeof(siginfo_t) * signal_queue_sz);
+ newsiginfo = signal_queue + (signal_queue_sz - 1);
+
+ res = ptrace (PTRACE_GETSIGINFO, pid, NULL, newsiginfo);
+ if (res != 0) {
+ ERROR(errno, "PTRACE_GETSIGINFO failed: signal lost !!!!\n");
+ signal_queue_sz--;
+ } else
+ DEBUG(1, "waitstopped PTRACE_CONT, queuing signal %d"
+ " si_signo %d si_pid %d\n",
+ signal_received, newsiginfo->si_signo, newsiginfo->si_pid);
+ res = ptrace (PTRACE_CONT, pid, NULL, 0);
+ } else {
+ DEBUG(1, "waitstopped PTRACE_CONT with signal %d\n", signal_received);
+ res = ptrace (PTRACE_CONT, pid, NULL, signal_received);
+ }
if (res != 0) {
ERROR(errno, "waitstopped PTRACE_CONT\n");
return False;
@@ -447,8 +492,6 @@
}
}
-// if > 0, pid for which registers have to be restored.
-static int pid_of_save_regs = 0;
static struct user user_save;
// The below indicates if ptrace_getregs (and ptrace_setregs) can be used.
@@ -588,6 +631,11 @@
static
void restore_and_detach (pid_t pid)
{
+ int res;
+
+ DEBUG(1, "restore_and_detach pid %d pid_of_save_regs %d\n",
+ pid, pid_of_save_regs);
+
if (pid_of_save_regs) {
/* In case the 'main pid' has been continued, we need to stop it
before resetting the registers. */
@@ -602,10 +650,32 @@
ERROR(errno, "setregs restore registers pid %d after cont\n",
pid_of_save_regs);
}
+
+ /* Now, we transmit all the signals we have queued. */
+ if (signal_queue_sz > 0) {
+ int i;
+ for (i = 0; i < signal_queue_sz; i++) {
+ DEBUG(1, "PTRACE_CONT to transmit queued signal %d\n",
+ signal_queue[i].si_signo);
+ res = ptrace (PTRACE_CONT, pid_of_save_regs, NULL,
+ signal_queue[i].si_signo);
+ if (res != 0)
+ ERROR(errno, "PTRACE_CONT with signal %d\n",
+ signal_queue[i].si_signo);
+ if (!stop(pid_of_save_regs, "sigstop after transmit sig"))
+ DEBUG(0, "Could not sigstop after transmit sig");
+ }
+ free (signal_queue);
+ signal_queue = NULL;
+ signal_queue_sz = 0;
+ }
pid_of_save_regs = 0;
} else {
DEBUG(1, "PTRACE_SETREGS restore registers: no pid\n");
}
+ if (signal_queue)
+ ERROR (0, "One or more signals queued were not delivered. "
+ "First signal: %d\n", signal_queue);
detach_from_all_threads(pid);
}
Modified: trunk/coregrind/vgdb.c
==============================================================================
--- trunk/coregrind/vgdb.c (original)
+++ trunk/coregrind/vgdb.c Tue Apr 15 22:35:23 2014
@@ -87,8 +87,6 @@
#define VS_vgdb_pid (shared32 != NULL ? shared32->vgdb_pid : shared64->vgdb_pid)
-/* Calls malloc (size). Exits if memory can't be allocated. */
-static
void *vmalloc(size_t size)
{
void * mem = malloc(size);
@@ -97,8 +95,6 @@
return mem;
}
-/* Calls realloc (size). Exits if memory can't be allocated. */
-static
void *vrealloc(void *ptr,size_t size)
{
void * mem = realloc(ptr, size);
Modified: trunk/coregrind/vgdb.h
==============================================================================
--- trunk/coregrind/vgdb.h (original)
+++ trunk/coregrind/vgdb.h Tue Apr 15 22:35:23 2014
@@ -64,6 +64,11 @@
fflush(stderr), \
exit(1))
+/* Calls malloc (size). Exits if memory can't be allocated. */
+extern void *vmalloc(size_t size);
+/* Calls realloc (size). Exits if memory can't be allocated. */
+extern void *vrealloc(void *ptr,size_t size);
+
/* Will be set to True when any condition indicating we have to shutdown
is encountered. */
extern Bool shutting_down;
Modified: trunk/gdbserver_tests/Makefile.am
==============================================================================
--- trunk/gdbserver_tests/Makefile.am (original)
+++ trunk/gdbserver_tests/Makefile.am Tue Apr 15 22:35:23 2014
@@ -119,7 +119,12 @@
nlsigvgdb.vgtest \
nlsigvgdb.stderr.exp \
nlsigvgdb.stderrB.exp \
- nlsigvgdb.stdinB.gdb
+ nlsigvgdb.stdinB.gdb \
+ nlvgdbsigqueue.vgtest \
+ nlvgdbsigqueue.stderrB.exp \
+ nlvgdbsigqueue.stderr.exp \
+ nlvgdbsigqueue.stdinB.gdb \
+ nlvgdbsigqueue.stdoutB.exp
check_PROGRAMS = \
clean_after_fork \
Added: trunk/gdbserver_tests/nlvgdbsigqueue.stderr.exp
==============================================================================
--- trunk/gdbserver_tests/nlvgdbsigqueue.stderr.exp (added)
+++ trunk/gdbserver_tests/nlvgdbsigqueue.stderr.exp Tue Apr 15 22:35:23 2014
@@ -0,0 +1,11 @@
+Nulgrind, the minimal Valgrind tool
+
+(action at startup) vgdb me ...
+
+
+loops/sleep_ms/burn/threads_spec: 1000000000 1000000000 1 BSBSBSBS
+Brussels ready to sleep and/or burn
+London ready to sleep and/or burn
+Petaouchnok ready to sleep and/or burn
+main ready to sleep and/or burn
+Gdb request to kill this process
Added: trunk/gdbserver_tests/nlvgdbsigqueue.stderrB.exp
==============================================================================
(empty)
Added: trunk/gdbserver_tests/nlvgdbsigqueue.stdinB.gdb
==============================================================================
--- trunk/gdbserver_tests/nlvgdbsigqueue.stdinB.gdb (added)
+++ trunk/gdbserver_tests/nlvgdbsigqueue.stdinB.gdb Tue Apr 15 22:35:23 2014
@@ -0,0 +1,29 @@
+# connect gdb to Valgrind gdbserver:
+target remote | ./vgdb --wait=60 --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue
+echo vgdb launched process attached\n
+monitor v.set vgdb-error 999999
+#
+#
+# simulate control-c in a 1 second
+shell ./simulate_control_c --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue 1 grep main nlvgdbsigqueue.stderr.out
+#
+# send SIGUSR1/SIGUSR1 in a few seconds, when vgdb is attached
+shell ./send_signal USR1 --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue 2
+shell ./send_signal USR1 --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue 3
+#
+echo continuing to have vgdb interrupted by simulate_control_c\n
+continue
+#
+# Now vgdb should have received the interrupt, and so has
+# attached to the sleeping process.
+# wait for the USR sig to be sent, that will be queued by vgdb.
+shell sleep 4
+# continue, so as to have vgdb sending queued signals when PTRACE_DETACHing
+echo continuing to receive first SIGUSR1\n
+continue
+# simulate a control c to afterwards stop the execution
+shell ./simulate_control_c --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue 1
+echo continuing to receive second SIGUSR1\n
+continue
+kill
+quit
Added: trunk/gdbserver_tests/nlvgdbsigqueue.stdoutB.exp
==============================================================================
--- trunk/gdbserver_tests/nlvgdbsigqueue.stdoutB.exp (added)
+++ trunk/gdbserver_tests/nlvgdbsigqueue.stdoutB.exp Tue Apr 15 22:35:23 2014
@@ -0,0 +1,14 @@
+continuing to have vgdb interrupted by simulate_control_c
+Continuing.
+Program received signal SIGUSR1, User defined signal 1.
+0x........ in syscall ...
+continuing to receive first SIGUSR1
+Continuing.
+[New Thread ....]
+Program received signal SIGUSR1, User defined signal 1.
+0x........ in syscall ...
+continuing to receive second SIGUSR1
+Continuing.
+Program received signal SIGTRAP, Trace/breakpoint trap.
+0x........ in syscall ...
+Kill the program being debugged? (y or n) [answered Y; input not from terminal]
Added: trunk/gdbserver_tests/nlvgdbsigqueue.vgtest
==============================================================================
--- trunk/gdbserver_tests/nlvgdbsigqueue.vgtest (added)
+++ trunk/gdbserver_tests/nlvgdbsigqueue.vgtest Tue Apr 15 22:35:23 2014
@@ -0,0 +1,14 @@
+# test :
+# sending signal when all threads are in WaitSys
+# vgdb must queue these signals and deliver them before PTRACE_DETACHing.
+# sleepers is started with argument so that it will mostly sleep.
+prog: sleepers
+args: 1000000000 1000000000 1 BSBSBSBS
+vgopts: --tool=none --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-nlvgdbsigqueue
+stderr_filter: filter_stderr
+prereq: test -e gdb -a -f vgdb.invoker
+progB: gdb
+argsB: --quiet -l 60 --nx ./sleepers
+stdinB: nlvgdbsigqueue.stdinB.gdb
+stdoutB_filter: filter_gdb
+stderrB_filter: filter_make_empty
Added: trunk/gdbserver_tests/send_signal
==============================================================================
--- trunk/gdbserver_tests/send_signal (added)
+++ trunk/gdbserver_tests/send_signal Tue Apr 15 22:35:23 2014
@@ -0,0 +1,17 @@
+#! /bin/sh
+
+# send_signal sends signal $1 to the Valgrind process using prefix $2 in $3 seconds
+SIG=$1
+shift
+PREFIX=$1
+shift
+SLEEP=$1
+shift
+VPID=`./vgdb -l $PREFIX 2>&1 | awk '{print $2}' | sed -e 's/--pid=//'`
+if [ "$VPID" = "" ]
+then
+ echo "send_signal could not determine the valgrind pid with " $PREFIX
+ exit 1
+fi
+(sleep $SLEEP
+ kill -s $SIG $VPID) &
Modified: trunk/gdbserver_tests/sleepers.c
==============================================================================
--- trunk/gdbserver_tests/sleepers.c (original)
+++ trunk/gdbserver_tests/sleepers.c Tue Apr 15 22:35:23 2014
@@ -8,11 +8,11 @@
#include <sys/types.h>
#include <sys/syscall.h>
#include <sched.h>
-
+#include <signal.h>
static int loops = 15; // each thread+main will do this amount of loop
static int sleepms = 1000; // in each loop, will sleep "sleepms" milliseconds
static int burn = 0; // after each sleep, will burn cpu in a tight 'burn' loop
-
+static void setup_sigusr_handler(void); // sigusr1 and 2 sigaction setup.
static pid_t gettid()
{
@@ -133,7 +133,7 @@
struct spec b, l, p, m;
char *some_mem __attribute__((unused)) = malloc(100);
setaffinity();
-
+ setup_sigusr_handler();
if (argc > 1)
loops = atoi(argv[1]);
@@ -194,3 +194,24 @@
return 0;
}
+
+static int sigusr1_received = 0;
+static void sigusr1_handler(int signr)
+{
+ sigusr1_received++;
+}
+static void setup_sigusr_handler(void)
+{
+ struct sigaction sa;
+ sa.sa_handler = sigusr1_handler;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = 0;
+
+ if (sigaction (SIGUSR1, &sa, NULL) != 0)
+ perror("sigaction SIGUSR1");
+
+ sa.sa_handler = SIG_IGN;
+ if (sigaction (SIGUSR2, &sa, NULL) != 0)
+ perror("sigaction SIGUSR2");
+}
+
|
