This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: QPassSignals patch to go with proposed protocol
On Thu, Oct 26, 2006 at 02:55:09AM -0400, Eli Zaretskii wrote:
> What happened with the indentation here? Some lines use spaces,
> others use TABs and spaces.
Thanks, fixed.
> > +@cindex inform remote target of signals passed to the inferior
>
> This index entry is too long.
Is this better?
@cindex inform remote target of unnecessary signals
> > +Each listed @var{signal}, using the same signal numbering used in
>
> Too many uses of ``using'', ``used'', etc. I suggest to split this
> sentence in two, and mention the signal numbering only in the second
> one. Like this, for example:
>
> Each listed SIGNAL should be passed directly to the inferior
> process. Signals are numbered identically to continue packets and
> stop responses (*note ...).
>
> The *note at the end is to suggest a cross-reference to the place
> where the signal numbering is described.
There wasn't such a description, but I've added a small one and changed
the text as suggested.
> Why is the implementation only for Linux? Is there something
> platform-dependent here?
The exact mechanism is platform dependent, because the platform
specific wait routine in gdbserver is responsible for stopping all
threads when a signal is received (just like in GDB). The only
platforms currently supported by gdbserver are Linux and Win32, and
signals are rather less important on Windows, which seems to use other
mechanisms.
--
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 26 Oct 2006 12:30:56 -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 26 Oct 2006 12:30:59 -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 unnecessary signals
+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
@@ -23392,8 +23405,8 @@ The @samp{C}, @samp{c}, @samp{S}, @samp{
receive any of the below as a reply. In the case of the @samp{C},
@samp{c}, @samp{S} and @samp{s} packets, that reply is only returned
when the target halts. In the below the exact meaning of @dfn{signal
-number} is poorly defined. In general one of the UNIX signal
-numbering conventions is used.
+number} is defined by the header @file{include/gdb/signals.h} in the
+@value{GDBN} source code.
As in the description of request packets, we include spaces in the
reply templates for clarity; these are not part of the reply packet's
@@ -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} should be passed directly to the inferior process.
+Signals are numbered identically to continue packets and stop replies
+(@pxref{Stop Reply Packets}). 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 26 Oct 2006 12:30:59 -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 26 Oct 2006 12:31:00 -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 26 Oct 2006 12:31:00 -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 26 Oct 2006 12:31:00 -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);