From: Philippe Waroquiers Date: Thu, 29 Mar 2012 21:56:47 +0000 (+0000) Subject: Fix bug 297078 gdbserver signal handling problems caused by diff vki nr/gdb nr and X-Git-Tag: svn/VALGRIND_3_8_0~383 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=323ba3d8f00575e7c4e0291bae06b720ec201a45;p=thirdparty%2Fvalgrind.git Fix bug 297078 gdbserver signal handling problems caused by diff vki nr/gdb nr and non reset of "C-ontinued" signal * To allow vki signame to be used in debuglog: - pub_core_signals.h : added prototype for Char *VG_(signame) - m_signals.c : changed static const Char *signame(Int sigNo) to const Char *VG_(signame)(Int sigNo) * valgrind-low.c : when the signal to report to gdb has been reported, clear it so that it is not reported anymore afterwards. * m_gdbserver.c: when checking in pass_signals if signal can be passed without gdb interaction, do a conversion from vki nr to gdb nr when indexing (as pass_signals[] is indexed by gdb_nr). * various gdbserver files: - used vki_ prefix for some args and variables to clarify - better debuglog tracing * modified nlpasssigalrm.vgtest to test SIGCHLD signal handling followed by a break (to see SIGTRAP is properly given to gdb). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12470 --- diff --git a/NEWS b/NEWS index e221491a57..1f4bd55555 100644 --- a/NEWS +++ b/NEWS @@ -78,7 +78,8 @@ where XXXXXX is the bug number as listed below. 294190 --vgdb-error=xxx can be out of sync with errors shown to the user 295799 Missing \n with get_vbits in gdbserver when line is % 80 and there are some unaddressable bytes n-i-bz s390x: Shadow registers can now be examined using vgdb - +297078 gdbserver signal handling problems caused by diff vki nr/gdb nr + and non reset of "C-ontinued" signal Release 3.7.0 (5 November 2011) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index 36e0b27624..f3d3d507f0 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -42,6 +42,7 @@ #include "pub_core_libcassert.h" #include "pub_tool_libcbase.h" #include "pub_core_libcsignal.h" +#include "pub_core_signals.h" #include "pub_tool_machine.h" // VG_(fnptr_to_fnentry) #include "pub_tool_debuginfo.h" #include "pub_core_scheduler.h" @@ -835,9 +836,14 @@ Bool VG_(gdbserver_activity) (ThreadId tid) return ret; } -Bool VG_(gdbserver_report_signal) (Int sigNo, ThreadId tid) +Bool VG_(gdbserver_report_signal) (Int vki_sigNo, ThreadId tid) { - dlog(1, "signal %d tid %d\n", sigNo, tid); + dlog(1, "VG core calling VG_(gdbserver_report_signal) " + "vki_nr %d %s gdb_nr %d %s tid %d\n", + vki_sigNo, VG_(signame)(vki_sigNo), + target_signal_from_host (vki_sigNo), + target_signal_to_name(target_signal_from_host (vki_sigNo)), + tid); /* if gdbserver is currently not connected, then signal is to be given to the process */ @@ -848,19 +854,19 @@ Bool VG_(gdbserver_report_signal) (Int sigNo, ThreadId tid) /* 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]) { + if (pass_signals[target_signal_from_host(vki_sigNo)]) { dlog(1, "pass_signals => pass\n"); return True; } /* indicate to gdbserver that there is a signal */ - gdbserver_signal_encountered (sigNo); + gdbserver_signal_encountered (vki_sigNo); /* let gdbserver do some work, e.g. show the signal to the user */ call_gdbserver (tid, signal_reason); /* ask gdbserver what is the final decision */ - if (gdbserver_deliver_signal (sigNo)) { + if (gdbserver_deliver_signal (vki_sigNo)) { dlog(1, "gdbserver deliver signal\n"); return True; } else { diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 84a2e1b290..2736419c8c 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -34,7 +34,7 @@ unsigned long step_thread; unsigned long thread_from_wait; unsigned long old_thread_from_wait; -int pass_signals[TARGET_SIGNAL_LAST]; +int pass_signals[TARGET_SIGNAL_LAST]; /* indexed by gdb signal nr */ /* for a gdbserver integrated in valgrind, resuming the process consists in returning the control to valgrind. @@ -412,7 +412,8 @@ void handle_set (char *arg_own_buf, int *new_packet_len_p) if (to == NULL) to = end; decode_address (&sig, from, to - from); pass_signals[(int)sig] = 1; - dlog(1, "pass_signal %d\n", (int)sig); + dlog(1, "pass_signal gdb_nr %d %s\n", + (int)sig, target_signal_to_name(sig)); from = to; if (*from == ';') from++; } diff --git a/coregrind/m_gdbserver/server.h b/coregrind/m_gdbserver/server.h index dc1b1280e5..a88bc03050 100644 --- a/coregrind/m_gdbserver/server.h +++ b/coregrind/m_gdbserver/server.h @@ -230,19 +230,19 @@ struct thread_info; gdbserver by calling call_gdbserver. On return, call gdbserver_deliver_signal to effectively deliver the signal or not. */ -extern void gdbserver_signal_encountered (Int sigNo); +extern void gdbserver_signal_encountered (Int vki_sigNo); /* between these two calls, call call_gdbserver */ /* If gdbserver_deliver_signal True, then gdb did not ask to ignore the signal, so signal can be delivered to the guest. */ -extern Bool gdbserver_deliver_signal (Int sigNo); +extern Bool gdbserver_deliver_signal (Int vki_sigNo); /* To optimise signal handling, gdb can instruct gdbserver to - not stop on some signals. In the below, a 1 indicates the signal + not stop on some signals. In the below, a 1 indicates the gdb_nr signal has to be passed directly to the guest, without asking gdb. A 0 indicates gdb has to be consulted to see if signal has or has not to be passed. The gdb consultation is to be done using the above two functions. */ -extern int pass_signals[]; +extern int pass_signals[]; /* indexed by gdb signal nr */ #include "target.h" diff --git a/coregrind/m_gdbserver/valgrind-low.c b/coregrind/m_gdbserver/valgrind-low.c index 5d3f708434..59f5284c9f 100644 --- a/coregrind/m_gdbserver/valgrind-low.c +++ b/coregrind/m_gdbserver/valgrind-low.c @@ -438,17 +438,17 @@ static CORE_ADDR stop_pc; */ static CORE_ADDR resume_pc; -static int signal_to_report; +static int vki_signal_to_report; -void gdbserver_signal_encountered (Int sigNo) +void gdbserver_signal_encountered (Int vki_sigNo) { - signal_to_report = sigNo; + vki_signal_to_report = vki_sigNo; } -static int signal_to_deliver; -Bool gdbserver_deliver_signal (Int sigNo) +static int vki_signal_to_deliver; +Bool gdbserver_deliver_signal (Int vki_sigNo) { - return sigNo == signal_to_deliver; + return vki_sigNo == vki_signal_to_deliver; } static @@ -479,10 +479,12 @@ unsigned char valgrind_wait (char *ourstatus) and with a signal TRAP (i.e. a breakpoint), unless there is a signal to report. */ *ourstatus = 'T'; - if (signal_to_report == 0) + if (vki_signal_to_report == 0) sig = TARGET_SIGNAL_TRAP; - else - sig = target_signal_from_host(signal_to_report); + else { + sig = target_signal_from_host(vki_signal_to_report); + vki_signal_to_report = 0; + } if (vgdb_interrupted_tid != 0) tst = VG_(get_ThreadState) (vgdb_interrupted_tid); @@ -523,7 +525,7 @@ void valgrind_resume (struct thread_resume *resume_info) C2v(stopped_data_address)); VG_(set_watchpoint_stop_address) ((Addr) 0); } - signal_to_deliver = resume_info->sig; + vki_signal_to_deliver = resume_info->sig; stepping = resume_info->step; resume_pc = (*the_low_target.get_pc) (); diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c index 4112c539e5..f73b8f371f 100644 --- a/coregrind/m_signals.c +++ b/coregrind/m_signals.c @@ -240,8 +240,6 @@ static void async_signalhandler ( Int sigNo, vki_siginfo_t *info, static void sigvgkill_handler ( Int sigNo, vki_siginfo_t *info, struct vki_ucontext * ); -static const Char *signame(Int sigNo); - /* Maximum usable signal. */ Int VG_(max_signal) = _VKI_NSIG; @@ -1080,18 +1078,18 @@ SysRes VG_(do_sys_sigaction) ( Int signo, bad_signo_reserved: if (VG_(showing_core_errors)() && !VG_(clo_xml)) { VG_(umsg)("Warning: ignored attempt to set %s handler in sigaction();\n", - signame(signo)); + VG_(signame)(signo)); VG_(umsg)(" the %s signal is used internally by Valgrind\n", - signame(signo)); + VG_(signame)(signo)); } return VG_(mk_SysRes_Error)( VKI_EINVAL ); bad_sigkill_or_sigstop: if (VG_(showing_core_errors)() && !VG_(clo_xml)) { VG_(umsg)("Warning: ignored attempt to set %s handler in sigaction();\n", - signame(signo)); + VG_(signame)(signo)); VG_(umsg)(" the %s signal is uncatchable\n", - signame(signo)); + VG_(signame)(signo)); } return VG_(mk_SysRes_Error)( VKI_EINVAL ); } @@ -1272,7 +1270,7 @@ void push_signal_frame ( ThreadId tid, const vki_siginfo_t *siginfo, if (VG_(clo_trace_signals)) VG_(dmsg)("delivering signal %d (%s) to thread %d: " "on ALT STACK (%p-%p; %ld bytes)\n", - sigNo, signame(sigNo), tid, tst->altstack.ss_sp, + sigNo, VG_(signame)(sigNo), tid, tst->altstack.ss_sp, (UChar *)tst->altstack.ss_sp + tst->altstack.ss_size, (Word)tst->altstack.ss_size ); @@ -1300,7 +1298,7 @@ void push_signal_frame ( ThreadId tid, const vki_siginfo_t *siginfo, } -static const Char *signame(Int sigNo) +const Char *VG_(signame)(Int sigNo) { static Char buf[20]; @@ -1524,7 +1522,7 @@ static void default_action(const vki_siginfo_t *info, ThreadId tid) VG_(umsg)( "\n" "Process terminating with default action of signal %d (%s)%s\n", - sigNo, signame(sigNo), core ? ": dumping core" : ""); + sigNo, VG_(signame)(sigNo), core ? ": dumping core" : ""); /* Be helpful - decode some more details about this fault */ if (is_signal_from_kernel(tid, sigNo, info->si_code)) { @@ -1669,7 +1667,7 @@ static void deliver_signal ( ThreadId tid, const vki_siginfo_t *info, if (VG_(clo_trace_signals)) VG_(dmsg)("delivering signal %d (%s):%d to thread %d\n", - sigNo, signame(sigNo), info->si_code, tid ); + sigNo, VG_(signame)(sigNo), info->si_code, tid ); if (sigNo == VG_SIGVGKILL) { /* If this is a SIGVGKILL, we're expecting it to interrupt any @@ -2186,7 +2184,7 @@ void sync_signalhandler_from_user ( ThreadId tid, signal and what we should do about it, we really can't continue unless we get it. */ VG_(umsg)("Signal %d (%s) appears to have lost its siginfo; " - "I can't go on.\n", sigNo, signame(sigNo)); + "I can't go on.\n", sigNo, VG_(signame)(sigNo)); VG_(printf)( " This may be because one of your programs has consumed your ration of\n" " siginfo structures. For more information, see:\n" @@ -2338,7 +2336,7 @@ void sync_signalhandler_from_kernel ( ThreadId tid, */ VG_(dmsg)("VALGRIND INTERNAL ERROR: Valgrind received " "a signal %d (%s) - exiting\n", - sigNo, signame(sigNo)); + sigNo, VG_(signame)(sigNo)); VG_(dmsg)("si_code=%x; Faulting address: %p; sp: %#lx\n", info->si_code, info->VKI_SIGINFO_si_addr, diff --git a/coregrind/pub_core_signals.h b/coregrind/pub_core_signals.h index dae2c3fe92..f2188f1ccf 100644 --- a/coregrind/pub_core_signals.h +++ b/coregrind/pub_core_signals.h @@ -40,6 +40,9 @@ /* Highest signal the kernel will let us use */ extern Int VG_(max_signal); +/* Returns the name of the vki signal sigNo */ +extern const Char *VG_(signame)(Int sigNo); + /* Use high signals because native pthreads wants to use low */ #define VG_SIGVGKILL (VG_(max_signal)-0) #define VG_SIGVGRTUSERMAX (VG_(max_signal)-1) diff --git a/gdbserver_tests/nlpasssigalrm.stdinB.gdb b/gdbserver_tests/nlpasssigalrm.stdinB.gdb index ee2b4e1ef5..a8f14c794b 100644 --- a/gdbserver_tests/nlpasssigalrm.stdinB.gdb +++ b/gdbserver_tests/nlpasssigalrm.stdinB.gdb @@ -2,6 +2,8 @@ target remote | ./vgdb --wait=60 --vgdb-prefix=./vgdb-prefix-nlpasssigalrm echo vgdb launched process attached\n monitor v.set vgdb-error 999999 +break passsigalrm.c:43 +break passsigalrm.c:44 # # # ensure SIGALRM can be passed directly to the process, without @@ -14,4 +16,9 @@ continue # Tell the 2nd can be given directly handle SIGALRM nostop noprint pass continue +# Here, we expect to stop on the breakme +p breakme +continue +p breakme +continue quit diff --git a/gdbserver_tests/nlpasssigalrm.stdoutB.exp b/gdbserver_tests/nlpasssigalrm.stdoutB.exp index a04d2a1163..98eac299a7 100644 --- a/gdbserver_tests/nlpasssigalrm.stdoutB.exp +++ b/gdbserver_tests/nlpasssigalrm.stdoutB.exp @@ -1,3 +1,5 @@ +Breakpoint 1 at 0x........: file passsigalrm.c, line 43. +Breakpoint 2 at 0x........: file passsigalrm.c, line 44. Signal Stop Print Pass to program Description SIGALRM Yes Yes Yes Alarm clock Continuing. @@ -6,3 +8,11 @@ Program received signal SIGALRM, Alarm clock. Signal Stop Print Pass to program Description SIGALRM No No Yes Alarm clock Continuing. +Breakpoint 1, main (argc=1, argv=0x........) at passsigalrm.c:43 +43 breakme++; +$1 = 0 +Continuing. +Breakpoint 2, main (argc=1, argv=0x........) at passsigalrm.c:44 +44 return 0; +$2 = 1 +Continuing. diff --git a/gdbserver_tests/passsigalrm.c b/gdbserver_tests/passsigalrm.c index a625eb6910..6493a85f77 100644 --- a/gdbserver_tests/passsigalrm.c +++ b/gdbserver_tests/passsigalrm.c @@ -1,13 +1,16 @@ #include #include #include -static int sigalrm_received = 0; +#include +static int sigalrm_received = 0; static void sigalrm_handler(int signr) { sigalrm_received++; } +static int breakme = 0; + int main (int argc, char *argv[]) { struct sigaction sa; @@ -36,5 +39,7 @@ int main (int argc, char *argv[]) fprintf (stderr, "wrong 2nd: unexpected value %d sigalrm_received\n", sigalrm_received); + system("../tests/true"); + breakme++; return 0; }