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 e221491a570187af79cbf1765c927e927858d237..1f4bd555555b7346d16d4165608c8409f83083b4 100644 (file)
--- 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 36e0b27624076b98a9b0c6faa6f806f1963bda73..f3d3d507f0758243d68116d64c24a7327f5003cc 100644 (file)
--- 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 84a2e1b290d493f38b1f7556fa51c31a28627283..2736419c8cc4712b61a968db89a32615614ecfb5 100644 (file)
--- 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 dc1b1280e5f307b78545f6f4616b122deafbe9dd..a88bc03050e2caa5f357da3b16fb37a6a8749eb0 100644 (file)
--- 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 5d3f70843459a67ebbb26f6fa542663eed8b4d93..59f5284c9f5edafe2ef373368fa8da4377958a6f 100644 (file)
--- 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 4112c539e5554fc8a03cc35cf96f63f22860494e..f73b8f371f6e08cd8d976217c80c85e1b035542a 100644 (file)
--- 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 dae2c3fe92b9517c7df2e577658b42c843868008..f2188f1ccf4e54263f91085da934b041a52aa6a9 100644 (file)
--- 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 ee2b4e1ef5caf54911945a7eb3be6b58e1c40ed5..a8f14c794bd01e2235fbefa78205701bd8b0fa5f 100644 (file)
--- 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 a04d2a116301cd43e200cdb29dcf8b209db3a68e..98eac299a7edc8cf3ee29b57d0127f87c460f2df 100644 (file)
--- 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 a625eb6910a6b08dc1e4cd00c92f51157eb0b73e..6493a85f7751c10130901a295edc59174b2c9abf 100644 (file)
--- a/gdbserver_tests/passsigalrm.c
+++ b/gdbserver_tests/passsigalrm.c
@@ -1,13 +1,16 @@
 #include <stdio.h>
 #include <signal.h>
 #include <unistd.h>
-static int sigalrm_received = 0;
+#include <stdlib.h>
 
+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;
 }