From: Julian Seward Date: Sun, 26 Jun 2011 09:36:38 +0000 (+0000) Subject: GDB server: X-Git-Tag: svn/VALGRIND_3_7_0~400 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64940ed44af89aedee072e0f0479e0b4a5687911;p=thirdparty%2Fvalgrind.git GDB server: * Fix bug in logic related to signal-passing * use SIGSTOP instead of SIGTRAP (avoid race condition) (Philippe Waroquiers, philippe.waroquiers@skynet.be). Bug 214909 comment 109. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11837 --- diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index 8171eefe2b..6a0a19671c 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -720,23 +720,24 @@ static int interrupts_non_interruptible = 0; ptrace. To do that, vgdb 'pushes' a call to invoke_gdbserver on the stack using ptrace. invoke_gdbserver must not return. Instead, it must call give_control_back_to_vgdb. - vgdb expects to receive a SIGTRAP, which this function generates. - When vgdb gets this SIGTRAP, it knows invoke_gdbserver call + vgdb expects to receive a SIGSTOP, which this function generates. + When vgdb gets this SIGSTOP, it knows invoke_gdbserver call is finished and can reset the Valgrind process in the state prior to the 'pushed call' (using ptrace again). This all works well. However, the user must avoid 'kill-9ing' vgdb during such a pushed call, otherwise - the SIGTRAP generated below will be seen by the Valgrind core, - instead of being handled by vgdb. When the Valgrind core gets - such a SIGTRAP, it will assert. */ + the SIGSTOP generated below will be seen by the Valgrind core, + instead of being handled by vgdb. The OS will then handle the SIGSTOP + by stopping the Valgrind process. + We use SIGSTOP as this process cannot be masked. */ static void give_control_back_to_vgdb(void) { - /* cause a SIGTRAP to be sent to ourself, so that vgdb takes control. + /* cause a SIGSTOP to be sent to ourself, so that vgdb takes control. vgdb will then restore the stack so as to resume the activity before the ptrace (typically do_syscall_WRK). */ - if (VG_(kill)(VG_(getpid)(), VKI_SIGTRAP) != 0) - vg_assert2(0, "SIGTRAP for vgdb could not be generated\n"); + if (VG_(kill)(VG_(getpid)(), VKI_SIGSTOP) != 0) + vg_assert2(0, "SIGSTOP for vgdb could not be generated\n"); /* If we arrive here, it means a call was pushed on the stack by vgdb, but during this call, vgdb and/or connection @@ -841,12 +842,15 @@ Bool VG_(gdbserver_report_signal) (Int sigNo, ThreadId tid) /* if gdbserver is currently not connected, then signal is to be given to the process */ if (!remote_connected()) { - dlog(1, "not connected => pass\n"); - return True; + dlog(1, "not connected => pass\n"); + return True; } + /* if gdb has informed gdbserver that this signal can be + passed directly without informing gdb, then signal is + to be given to the process. */ if (pass_signals[sigNo]) { dlog(1, "pass_signals => pass\n"); - return False; + return True; } /* indicate to gdbserver that there is a signal */ diff --git a/coregrind/vgdb.c b/coregrind/vgdb.c index 68e67970ab..54ce6cfd5a 100644 --- a/coregrind/vgdb.c +++ b/coregrind/vgdb.c @@ -1041,8 +1041,8 @@ Bool invoke_gdbserver (int pid) return False; } pid_of_save_regs_continued = True; - - stopped = waitstopped (pid, SIGTRAP, + /* Wait for SIGSTOP generated by m_gdbserver.c give_control_back_to_vgdb */ + stopped = waitstopped (pid, SIGSTOP, "waitpid status after PTRACE_CONT to invoke"); if (stopped) { /* Here pid has properly stopped on the break. */ @@ -1546,7 +1546,7 @@ void close_connection(int to_pid, int from_pid) call exit. But a ptraced process will be blocked in exit, waiting for the ptracing process to detach or die. vgdb cannot detach unconditionally as otherwise, in the normal - case, the valgrind process would die abnormally with SIGTRAP + case, the valgrind process would stop abnormally with SIGSTOP (as vgdb would not be there to catch it). vgdb can also not die unconditionally otherwise again, similar problem. So, we assume that most of the time, we arrive here in the normal @@ -1556,7 +1556,7 @@ void close_connection(int to_pid, int from_pid) signal to be sent after a few seconds. This means that in the normal case, the gdbserver code in valgrind process must have returned the control in less than the alarm nr of seconds, - otherwise, valgrind will die abnormally with SIGTRAP. */ + otherwise, valgrind will stop abnormally with SIGSTOP. */ (void) alarm (3); DEBUG(1, "joining with invoke_gdbserver_in_valgrind_thread\n"); diff --git a/gdbserver_tests/Makefile.am b/gdbserver_tests/Makefile.am index 91eb02e8d7..f1af31c02f 100644 --- a/gdbserver_tests/Makefile.am +++ b/gdbserver_tests/Makefile.am @@ -77,11 +77,20 @@ EXTRA_DIST = \ nlcontrolc.vgtest \ nlfork_chain.stderr.exp \ nlfork_chain.stdout.exp \ - nlfork_chain.vgtest + nlfork_chain.vgtest \ + nlpasssigalrm.vgtest \ + nlpasssigalrm.stderrB.exp \ + nlpasssigalrm.stderr.exp \ + nlpasssigalrm.stdinB.gdb \ + nlsigvgdb.vgtest \ + nlsigvgdb.stderr.exp \ + nlsigvgdb.stderrB.exp \ + nlsigvgdb.stdinB.gdb check_PROGRAMS = \ clean_after_fork \ fork_chain \ + passsigalrm \ sleepers \ main_pic \ t \ diff --git a/gdbserver_tests/filter_gdb b/gdbserver_tests/filter_gdb index 6439489e8c..3828ebb37a 100755 --- a/gdbserver_tests/filter_gdb +++ b/gdbserver_tests/filter_gdb @@ -68,6 +68,8 @@ sed -e '/Remote debugging using/,/vgdb launched process attached/d' -e 's/^>[> ]*//' \ -e '/^done\.$/d' \ -e 's/in _dl_sysinfo_int80 () from \/lib\/ld-linux.so.*/in syscall .../' \ + -e 's/in kill ().*$/in syscall .../' \ + -e 's/in \.kill ().*$/in syscall .../' \ -e 's/in _dl_sysinfo_int80 ()/in syscall .../' \ -e '/^ from \/lib\/ld-linux.so.*$/d' \ -e 's/\(0x........\) in ?? () from \/lib.*$/\1 in syscall .../' \ diff --git a/gdbserver_tests/nlpasssigalrm.stderr.exp b/gdbserver_tests/nlpasssigalrm.stderr.exp new file mode 100644 index 0000000000..fb23a97309 --- /dev/null +++ b/gdbserver_tests/nlpasssigalrm.stderr.exp @@ -0,0 +1,9 @@ +Nulgrind, the minimal Valgrind tool + +(action at startup) vgdb me ... + + +starting ... +ok: 1st SIGALRM received +ok: 2nd SIGALRM received + diff --git a/gdbserver_tests/nlpasssigalrm.stderrB.exp b/gdbserver_tests/nlpasssigalrm.stderrB.exp new file mode 100644 index 0000000000..ed5d4208ec --- /dev/null +++ b/gdbserver_tests/nlpasssigalrm.stderrB.exp @@ -0,0 +1,3 @@ +relaying data between gdb and process .... +vgdb-error value changed from 0 to 999999 +Remote connection closed diff --git a/gdbserver_tests/nlpasssigalrm.stdinB.gdb b/gdbserver_tests/nlpasssigalrm.stdinB.gdb new file mode 100644 index 0000000000..ef928983e0 --- /dev/null +++ b/gdbserver_tests/nlpasssigalrm.stdinB.gdb @@ -0,0 +1,17 @@ +# connect gdb to Valgrind gdbserver: +target remote | ./vgdb --wait=60 --vgdb-prefix=./vgdb-prefix-nlpasssigalrm +echo vgdb launched process attached\n +monitor vg.set vgdb-error 999999 +# +# +# ensure SIGALRM can be passed directly to the process, without +# going through gdb: +handle SIGALRM stop print pass +# +continue +# +# Here, gdb should have been informed of the 1st SIGALRM +# Tell the 2nd can be given directly +handle SIGALRM nostop noprint pass +continue +quit diff --git a/gdbserver_tests/nlpasssigalrm.stdoutB.exp b/gdbserver_tests/nlpasssigalrm.stdoutB.exp new file mode 100644 index 0000000000..a04d2a1163 --- /dev/null +++ b/gdbserver_tests/nlpasssigalrm.stdoutB.exp @@ -0,0 +1,8 @@ +Signal Stop Print Pass to program Description +SIGALRM Yes Yes Yes Alarm clock +Continuing. +Program received signal SIGALRM, Alarm clock. +0x........ in syscall ... +Signal Stop Print Pass to program Description +SIGALRM No No Yes Alarm clock +Continuing. diff --git a/gdbserver_tests/nlpasssigalrm.vgtest b/gdbserver_tests/nlpasssigalrm.vgtest new file mode 100644 index 0000000000..5c4e3952e4 --- /dev/null +++ b/gdbserver_tests/nlpasssigalrm.vgtest @@ -0,0 +1,15 @@ +# test that signals that must be "directly" passed by gdbserver +# are properly handled. +# GDB optimises "handle SIGNAL nostop noprint pass" +# by telling gdbserver (with QPassSignals packet) to pass +# it directly to the process, without roundtrip to gdb. + +prog: passsigalrm +vgopts: --tool=none --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-nlpasssigalrm +stderr_filter: filter_stderr +prereq: test -e gdb +progB: gdb +argsB: --quiet -l 60 --nx ./passsigalrm +stdinB: nlpasssigalrm.stdinB.gdb +stdoutB_filter: filter_gdb +stderrB_filter: filter_gdb diff --git a/gdbserver_tests/nlsigvgdb.stderr.exp b/gdbserver_tests/nlsigvgdb.stderr.exp new file mode 100644 index 0000000000..a47dc8966a --- /dev/null +++ b/gdbserver_tests/nlsigvgdb.stderr.exp @@ -0,0 +1,6 @@ +Nulgrind, the minimal Valgrind tool + +(action at startup) vgdb me ... + + +Reset valgrind output to log (orderly_finish) diff --git a/gdbserver_tests/nlsigvgdb.stderrB.exp b/gdbserver_tests/nlsigvgdb.stderrB.exp new file mode 100644 index 0000000000..672fea5c1f --- /dev/null +++ b/gdbserver_tests/nlsigvgdb.stderrB.exp @@ -0,0 +1,6 @@ +relaying data between gdb and process .... +vgdb-error value changed from 0 to 999999 +gdbserver: continuing in 5000 ms ... +gdbserver: continuing after wait ... +monitor command request to kill this process +Remote connection closed diff --git a/gdbserver_tests/nlsigvgdb.stdinB.gdb b/gdbserver_tests/nlsigvgdb.stdinB.gdb new file mode 100644 index 0000000000..d7001c3b0b --- /dev/null +++ b/gdbserver_tests/nlsigvgdb.stdinB.gdb @@ -0,0 +1,17 @@ +# connect gdb to Valgrind gdbserver: +target remote | ./vgdb --wait=60 --vgdb-prefix=./vgdb-prefix-nlsigvgdb +echo vgdb launched process attached\n +monitor vg.set vgdb-error 999999 +# +# +# simulate control-c in a few seconds +# The control-c will cause a character to be sent to gdbserver, causing +# an invocation while the gdbserver is already busy. +shell ./simulate_control_c --vgdb-prefix=./vgdb-prefix-nlsigvgdb 1 grep continuing nlsigvgdb.stderrB.out +# +monitor vg.wait 5000 +# +# kill the process now +monitor vg.kill +quit + diff --git a/gdbserver_tests/nlsigvgdb.vgtest b/gdbserver_tests/nlsigvgdb.vgtest new file mode 100644 index 0000000000..596cefabbc --- /dev/null +++ b/gdbserver_tests/nlsigvgdb.vgtest @@ -0,0 +1,15 @@ +# Tests the case when gdb sends a character to gdbserver while +# the gdbserver was forced invoked just before. +# gdbserver must send a signal to itself that is wait-ed for by vgdb. +# But if this signal is masked, then vgdb does not recuperate the control +# and Valgrind dies. See function give_control_back_to_vgdb in m_gdbserver.c +prog: sleepers +args: 1 10000000 0 -S-S-S-S +vgopts: --tool=none --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-nlsigvgdb +stderr_filter: filter_stderr +prereq: test -e gdb -a -f vgdb.ptraceinvoker +progB: gdb +argsB: --quiet -l 60 --nx ./sleepers +stdinB: nlsigvgdb.stdinB.gdb +stdoutB_filter: filter_gdb +stderrB_filter: filter_gdb diff --git a/gdbserver_tests/passsigalrm.c b/gdbserver_tests/passsigalrm.c new file mode 100644 index 0000000000..c6f1fb52c7 --- /dev/null +++ b/gdbserver_tests/passsigalrm.c @@ -0,0 +1,40 @@ +#include +#include +#include +static int sigalrm_received = 0; + +static void sigalrm_handler(int signr) +{ + sigalrm_received++; +} + +int main (int argc, char *argv[]) +{ + struct sigaction sa; + fprintf(stderr, "starting ...\n"); + sa.sa_handler = sigalrm_handler; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + sa.sa_restorer = NULL; + if (sigaction (SIGALRM, &sa, NULL) != 0) + perror("sigaction"); + if (kill(getpid(), SIGALRM) != 0) + perror("kill 1"); + + if (sigalrm_received == 1) + fprintf (stderr, "ok: 1st SIGALRM received\n"); + else + fprintf (stderr, "wrong 1st: unexpected value %d sigalrm_received\n", + sigalrm_received); + + if (kill(getpid(), SIGALRM) != 0) + perror("kill 2"); + + if (sigalrm_received == 2) + fprintf (stderr, "ok: 2nd SIGALRM received\n"); + else + fprintf (stderr, "wrong 2nd: unexpected value %d sigalrm_received\n", + sigalrm_received); + + return 0; +}