QPassSignals patch to go with proposed protocol

Daniel Jacobowitz
2006-10-25T21:28:15Z

Here's the implementation of the remote protocol addition I just proposed
on the gdb@ list.

--
Daniel Jacobowitz
CodeSourcery

2006-10-25 Daniel Jacobowitz <dan@codesourcery.com>

* remote.c (PACKET_QPassSignals): New.
(last_pass_packet, remote_pass_signals): New.
(remote_protocol_features): Add QPassSignals.
(remote_query_supported): Correct an infinite loop.
(remote_open_1): Reset last_pass_packet.
(remote_resume): Call remote_pass_signals.
(_initialize_remote): Register "set remote pass-signals".

2006-10-25 Daniel Jacobowitz <dan@codesourcery.com>

* gdb.texinfo (Remote configuration): Document
"set remote pass-signals" and "show remote pass-signals".
(General Query Packets): Document QPassSignals. Fix
a typo.

2006-10-25 Daniel Jacobowitz <dan@codesourcery.com>

* linux-low.c (linux_wait_for_event): Reformat. Use the
pass_signals array.
* remote-utils.c (decode_address_to_semicolon): New.
* server.c (pass_signals, handle_general_set): New.
(handle_query): Mention QPassSignals for qSupported.
(main): Call handle_general_set.
* server.h (pass_signals, decode_address_to_semicolon): New.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.236
diff -u -p -r1.236 remote.c
--- remote.c 21 Oct 2006 17:59:08 -0000 1.236
+++ remote.c 25 Oct 2006 21:24:57 -0000
@@ -867,6 +867,7 @@ enum {
PACKET_qXfer_memory_map,
PACKET_qGetTLSAddr,
PACKET_qSupported,
+ PACKET_QPassSignals,
PACKET_MAX
};

@@ -1004,6 +1005,65 @@ record_currthread (int currthread)
}
}

+static char *last_pass_packet;
+
+/* If 'QPassSignals' is supported, tell the remote stub what signals
+ it can simply pass through to the inferior without reporting. */
+
+static void
+remote_pass_signals (void)
+{
+ if (remote_protocol_packets[PACKET_QPassSignals].support != PACKET_DISABLE)
+ {
+ char *pass_packet, *p;
+ int numsigs = (int) TARGET_SIGNAL_LAST;
+ int count = 0, i;
+
+ gdb_assert (numsigs < 256);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (signal_stop_state (i) == 0
+ && signal_print_state (i) == 0
+ && signal_pass_state (i) == 1)
+ count++;
+ }
+ pass_packet = xmalloc (count * 3 + strlen ("QPassSignals:") + 1);
+ strcpy (pass_packet, "QPassSignals:");
+ p = pass_packet + strlen (pass_packet);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (signal_stop_state (i) == 0
+ && signal_print_state (i) == 0
+ && signal_pass_state (i) == 1)
+ {
+ if (i >= 16)
+ *p++ = tohex (i >> 4);
+ *p++ = tohex (i & 15);
+ if (count)
+ *p++ = ';';
+ else
+ break;
+ count--;
+ }
+ }
+ *p = 0;
+ if (!last_pass_packet || strcmp (last_pass_packet, pass_packet))
+ {
+ struct remote_state *rs = get_remote_state ();
+ char *buf = rs->buf;
+
+ putpkt (pass_packet);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ packet_ok (buf, &remote_protocol_packets[PACKET_QPassSignals]);
+ if (last_pass_packet)
+ xfree (last_pass_packet);
+ last_pass_packet = pass_packet;
+ }
+ else
+ xfree (pass_packet);
+ }
+}
+
#define MAGIC_NULL_PID 42000

static void
@@ -2206,7 +2266,9 @@ static struct protocol_feature remote_pr
{ "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
PACKET_qXfer_auxv },
{ "qXfer:memory-map:read", PACKET_DISABLE, remote_supported_packet,
- PACKET_qXfer_memory_map }
+ PACKET_qXfer_memory_map },
+ { "QPassSignals", PACKET_DISABLE, remote_supported_packet,
+ PACKET_QPassSignals },
};

static void
@@ -2260,14 +2322,14 @@ remote_query_supported (void)
}
else
{
+ *end = '\0';
+ next = end + 1;
+
if (end == p)
{
warning (_("empty item in \"qSupported\" response"));
continue;
}
-
- *end = '\0';
- next = end + 1;
}

name_end = strchr (p, '=');
@@ -2354,6 +2416,10 @@ remote_open_1 (char *name, int from_tty,

unpush_target (target);

+ /* Make sure we send the passed signals list the next time we resume. */
+ xfree (last_pass_packet);
+ last_pass_packet = NULL;
+
remote_fileio_reset ();
reopen_exec_file ();
reread_symbols ();
@@ -2728,6 +2794,9 @@ remote_resume (ptid_t ptid, int step, en
if (deprecated_target_resume_hook)
(*deprecated_target_resume_hook) ();

+ /* Update the inferior on signals to silently pass, if they've changed. */
+ remote_pass_signals ();
+
/* The vCont packet doesn't need to specify threads via Hc. */
if (remote_vcont_resume (ptid, step, siggnal))
return;
@@ -6270,6 +6339,9 @@ Show the maximum size of the address (in
add_packet_config_cmd (&remote_protocol_packets[PACKET_vCont],
"vCont", "verbose-resume", 0);

+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QPassSignals],
+ "QPassSignals", "pass-signals", 0);
+
add_packet_config_cmd (&remote_protocol_packets[PACKET_qSymbol],
"qSymbol", "symbol-lookup", 0);

Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.365
diff -u -p -r1.365 gdb.texinfo
--- doc/gdb.texinfo 21 Oct 2006 13:06:03 -0000 1.365
+++ doc/gdb.texinfo 25 Oct 2006 21:25:04 -0000
@@ -12886,6 +12886,19 @@ details about this packet. The default
@item show remote supported-packets
@kindex show remote supported-packets
Show the current setting of @samp{qSupported} packet usage.
+
+@item set remote pass-signals
+@kindex set remote pass-signals
+@cindex inform remote target of signals passed to the inferior
+This command enables or disables the use of the @samp{QPassSignals}
+packet, which improves performance when debugging applications with
+high frequency signals (such as timers). @xref{General Query Packets,
+QPassSignals}, for more details about this packet. The default is to
+use @samp{QPassSignals} only if it is reported by @samp{qSupported}.
+
+@item show remote pass-signals
+@kindex show remote pass-signals
+Show the current setting of @samp{QPassSignals} packet usage.
@end table

@node remote stub
@@ -23653,6 +23666,37 @@ Don't use this packet; use the @samp{qTh

Reply: see @code{remote.c:remote_unpack_thread_info_response()}.

+@item QPassSignals @var{signal} @r{[};@var{signal}@r{]}@dots{}
+@cindex pass signals to inferior, remote request
+@cindex @samp{QPassSignals} packet
+Each listed @var{signal}, using the same signal numbering used in
+continue packets and stop responses, should be passed directly to the
+inferior process. Each @var{signal} list item should be strictly
+greater than the previous item. These signals do not need to stop the
+inferior, or be reported to @value{GDBN}. All other signals should be
+reported to @value{GDBN}. Multiple @samp{QPassSignals} packets do not
+combine; any earlier @samp{QPassSignals} list is completely replaced
+by the new list. This packet improves performance when using
+@samp{handle @var{signal} nostop noprint pass}.
+
+Reply:
+@table @samp
+@item OK
+The request succeeded.
+
+@item E @var{nn}
+An error occurred. @var{nn} are hex digits.
+
+@item
+An empty reply indicates that @samp{QPassSignals} is not supported by
+the stub.
+@end table
+
+Use of this packet is controlled by the @code{set remote pass-signals}
+command (@pxref{Remote configuration, set remote pass-signals}).
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
@item qRcmd,@var{command}
@cindex execute remote command, remote request
@cindex @samp{qRcmd} packet
@@ -23796,6 +23840,11 @@ These are the currently defined stub fea
@tab @samp{-}
@tab Yes

+@item @samp{QPassSignals}
+@tab No
+@tab @samp{-}
+@tab Yes
+
@end multitable

These are the currently defined stub features, in more detail:
@@ -23912,7 +23961,7 @@ auxiliary vector}, and @ref{Remote confi
read-aux-vector-packet}. Note @var{annex} must be empty.

This packet is not probed by default; the remote stub must request it,
-by suppling an appropriate @samp{qSupported} response (@pxref{qSupported}).
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
@end table

@table @samp
Index: gdbserver/linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.47
diff -u -p -r1.47 linux-low.c
--- gdbserver/linux-low.c 17 Oct 2006 16:02:27 -0000 1.47
+++ gdbserver/linux-low.c 25 Oct 2006 21:25:05 -0000
@@ -498,69 +498,76 @@ linux_wait_for_event (struct thread_info
current_inferior = (struct thread_info *)
find_inferior_id (&all_threads, event_child->tid);

- if (using_threads)
+ /* Check for thread exit. */
+ if (using_threads && ! WIFSTOPPED (wstat))
{
- /* Check for thread exit. */
- if (! WIFSTOPPED (wstat))
- {
- if (debug_threads)
- fprintf (stderr, "Thread %ld (LWP %ld) exiting\n",
- event_child->tid, event_child->head.id);
-
- /* If the last thread is exiting, just return. */
- if (all_threads.head == all_threads.tail)
- return wstat;
-
- dead_thread_notify (event_child->tid);
-
- remove_inferior (&all_processes, &event_child->head);
- free (event_child);
- remove_thread (current_inferior);
- current_inferior = (struct thread_info *) all_threads.head;
-
- /* If we were waiting for this particular child to do something...
- well, it did something. */
- if (child != NULL)
- return wstat;
+ if (debug_threads)
+ fprintf (stderr, "Thread %ld (LWP %ld) exiting\n",
+ event_child->tid, event_child->head.id);

- /* Wait for a more interesting event. */
- continue;
- }
+ /* If the last thread is exiting, just return. */
+ if (all_threads.head == all_threads.tail)
+ return wstat;
+
+ dead_thread_notify (event_child->tid);
+
+ remove_inferior (&all_processes, &event_child->head);
+ free (event_child);
+ remove_thread (current_inferior);
+ current_inferior = (struct thread_info *) all_threads.head;
+
+ /* If we were waiting for this particular child to do something...
+ well, it did something. */
+ if (child != NULL)
+ return wstat;

- if (WIFSTOPPED (wstat)
- && WSTOPSIG (wstat) == SIGSTOP
- && event_child->stop_expected)
- {
- if (debug_threads)
- fprintf (stderr, "Expected stop.\n");
- event_child->stop_expected = 0;
- linux_resume_one_process (&event_child->head,
- event_child->stepping, 0, NULL);
- continue;
- }
+ /* Wait for a more interesting event. */
+ continue;
+ }

- /* FIXME drow/2002-06-09: Get signal numbers from the inferior's
- thread library? */
- if (WIFSTOPPED (wstat)
- && (WSTOPSIG (wstat) == __SIGRTMIN
- || WSTOPSIG (wstat) == __SIGRTMIN + 1))
- {
- siginfo_t info, *info_p;
+ if (using_threads
+ && WIFSTOPPED (wstat)
+ && WSTOPSIG (wstat) == SIGSTOP
+ && event_child->stop_expected)
+ {
+ if (debug_threads)
+ fprintf (stderr, "Expected stop.\n");
+ event_child->stop_expected = 0;
+ linux_resume_one_process (&event_child->head,
+ event_child->stepping, 0, NULL);
+ continue;
+ }

- if (debug_threads)
- fprintf (stderr, "Ignored signal %d for %ld (LWP %ld).\n",
- WSTOPSIG (wstat), event_child->tid,
- event_child->head.id);
+ /* If GDB is not interested in this signal, don't stop other
+ threads, and don't report it to GDB. Just resume the
+ inferior right away. We do this for threading-related
+ signals as well as any that GDB specifically requested
+ we ignore. But never ignore SIGSTOP if we sent it
+ ourselves. */
+ /* FIXME drow/2002-06-09: Get signal numbers from the inferior's
+ thread library? */
+ if (WIFSTOPPED (wstat)
+ && ((using_threads && (WSTOPSIG (wstat) == __SIGRTMIN
+ || WSTOPSIG (wstat) == __SIGRTMIN + 1))
+ || (pass_signals[target_signal_from_host (WSTOPSIG (wstat))]
+ && (WSTOPSIG (wstat) != SIGSTOP
+ || !event_child->sigstop_sent))))
+ {
+ siginfo_t info, *info_p;

- if (ptrace (PTRACE_GETSIGINFO, event_child->lwpid, 0, &info) == 0)
- info_p = &info;
- else
- info_p = NULL;
- linux_resume_one_process (&event_child->head,
- event_child->stepping,
- WSTOPSIG (wstat), info_p);
- continue;
- }
+ if (debug_threads)
+ fprintf (stderr, "Ignored signal %d for %ld (LWP %ld).\n",
+ WSTOPSIG (wstat), event_child->tid,
+ event_child->head.id);
+
+ if (ptrace (PTRACE_GETSIGINFO, event_child->lwpid, 0, &info) == 0)
+ info_p = &info;
+ else
+ info_p = NULL;
+ linux_resume_one_process (&event_child->head,
+ event_child->stepping,
+ WSTOPSIG (wstat), info_p);
+ continue;
}

/* If this event was not handled above, and is not a SIGTRAP, report
Index: gdbserver/remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.33
diff -u -p -r1.33 remote-utils.c
--- gdbserver/remote-utils.c 17 Oct 2006 16:02:27 -0000 1.33
+++ gdbserver/remote-utils.c 25 Oct 2006 21:25:05 -0000
@@ -296,6 +296,22 @@ decode_address (CORE_ADDR *addrp, const
*addrp = addr;
}

+const char *
+decode_address_to_semicolon (CORE_ADDR *addrp, const char *start)
+{
+ const char *end;
+
+ end = start;
+ while (*end != '\0' && *end != ';')
+ end++;
+
+ decode_address (addrp, start, end - start);
+
+ if (*end == ';')
+ end++;
+ return end;
+}
+
/* Convert number NIB to a hex digit. */

static int
Index: gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.40
diff -u -p -r1.40 server.c
--- gdbserver/server.c 17 Oct 2006 16:02:27 -0000 1.40
+++ gdbserver/server.c 25 Oct 2006 21:25:05 -0000
@@ -36,6 +36,8 @@ unsigned long old_thread_from_wait;
int extended_protocol;
int server_waiting;

+int pass_signals[TARGET_SIGNAL_LAST];
+
jmp_buf toplevel;

/* The PID of the originally created or attached inferior. Used to
@@ -157,6 +159,40 @@ write_qxfer_response (char *buf, unsigne
PBUFSIZ - 2) + 1;
}

+/* Handle all of the extended 'Q' packets. */
+void
+handle_general_set (char *own_buf)
+{
+ if (strncmp ("QPassSignals:", own_buf, strlen ("QPassSignals:")) == 0)
+ {
+ int numsigs = (int) TARGET_SIGNAL_LAST, i;
+ const char *p = own_buf + strlen ("QPassSignals:");
+ CORE_ADDR cursig;
+
+ p = decode_address_to_semicolon (&cursig, p);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (i == cursig)
+ {
+ pass_signals[i] = 1;
+ if (*p == '\0')
+ /* Keep looping, to clear the remaining signals. */
+ cursig = -1;
+ else
+ p = decode_address_to_semicolon (&cursig, p);
+ }
+ else
+ pass_signals[i] = 0;
+ }
+ strcpy (own_buf, "OK");
+ return;
+ }
+
+ /* Otherwise we didn't know what packet it was. Say we didn't
+ understand it. */
+ own_buf[0] = 0;
+}
+
/* Handle all of the extended 'q' packets. */
void
handle_query (char *own_buf, int *new_packet_len_p)
@@ -246,7 +282,7 @@ handle_query (char *own_buf, int *new_pa
if (strncmp ("qSupported", own_buf, 10) == 0
&& (own_buf[10] == ':' || own_buf[10] == '\0'))
{
- sprintf (own_buf, "PacketSize=%x", PBUFSIZ - 1);
+ sprintf (own_buf, "PacketSize=%x;QPassSignals+", PBUFSIZ - 1);

if (the_target->read_auxv != NULL)
strcat (own_buf, ";qXfer:auxv:read+");
@@ -599,6 +635,9 @@ main (int argc, char *argv[])
case 'q':
handle_query (own_buf, &new_packet_len);
break;
+ case 'Q':
+ handle_general_set (own_buf);
+ break;
case 'd':
remote_debug = !remote_debug;
break;
Index: gdbserver/server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.26
diff -u -p -r1.26 server.h
--- gdbserver/server.h 17 Oct 2006 16:02:27 -0000 1.26
+++ gdbserver/server.h 25 Oct 2006 21:25:05 -0000
@@ -129,6 +129,7 @@ extern unsigned long step_thread;
extern unsigned long thread_from_wait;
extern unsigned long old_thread_from_wait;
extern int server_waiting;
+extern int pass_signals[];

extern jmp_buf toplevel;

@@ -153,6 +154,7 @@ void new_thread_notify (int id);
void dead_thread_notify (int id);
void prepare_resume_reply (char *buf, char status, unsigned char sig);

+const char *decode_address_to_semicolon (CORE_ADDR *addrp, const char *start);
void decode_address (CORE_ADDR *addrp, const char *start, int len);
void decode_m_packet (char *from, CORE_ADDR * mem_addr_ptr,
unsigned int *len_ptr);