Re: [PATCH] [RFC, docs RFA] gdbserver and gdb.threads/attach-into-signal.exp.

Pedro Alves
2012-03-07T19:28:11Z

On 03/07/2012 06:53 PM, Eli Zaretskii wrote:

>> Date: Wed, 07 Mar 2012 18:25:18 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>> +z0/z1 conditional breakpoints extension
>> +
>> The z0/z1 breakpoint insertion packets have been extended to carry
>> a list of conditional expressions over to the remote stub depending on the
>> condition evaluation mode. The use of this extension can be controlled
>> via the "set remote conditional-breakpoints-packet" command.
>
> This should be a separate (obvious) commit, I Think.
>
>> +QProgramSignals:
>> +
>> + Signals the remote stub may pass to the debugged program, without
>> + GDB involvement.
>
> OK, but the style is different from all the other entries for
> packets. How about this small change:
>
> Specify the signals which the remote stub may pass ...

Okay, it ends up as:

QProgramSignals:

Specify the signals which the remote stub may pass to the debugged
program without GDB involvement.

I've folded that into the main patch, and applied it all, as below.

Thanks!

2012-03-07 Pedro Alves <palves@redhat.com>

gdb/doc/
* gdb.texinfo (General Query Packets): Document new
QProgramSignals packet.
* gdb.texinfo (Remote configuration): Mention
"program-signals-packet".

gdb/gdbserver/
* linux-low.c (get_detach_signal): New.
(linux_detach_one_lwp): Get rid of a pending SIGSTOP with SIGCONT.
Pass on pending signals to PTRACE_DETACH. Check the result of the
ptrace call.
* server.c (program_signals, program_signals_p): New.
(handle_general_set): Handle QProgramSignals.
* server.h (program_signals, program_signals_p): Declare.

gdb/
* NEWS: Mention QProgramSignals.
* inferior.h (update_signals_program_target): Declare.
* infrun.c: (update_signals_program_target): New.
(handle_command): Update the target of the new program signals
array changes.
* remote.c (PACKET_QProgramSignals): New enum.
(last_program_signals_packet): New global.
(remote_program_signals): New.
(remote_start_remote): Update the target with the program signals
list.
(remote_protocol_features): Add entry for QPassSignals.
(remote_open_1): Free anc clear last_program_signals_packet.
(init_remote_ops): Install remote_program_signals.
* target.c (update_current_target): Adjust.
(target_program_signals): New.
* target.h (struct target_ops) <to_program_signals>: New field.
(target_program_signals): Declare.
---
gdb/NEWS | 5 ++
gdb/doc/gdb.texinfo | 46 +++++++++++++++++++
gdb/gdbserver/linux-low.c | 107 ++++++++++++++++++++++++++++++++++++++++++---
gdb/gdbserver/server.c | 33 +++++++++++++-
gdb/gdbserver/server.h | 2 +
gdb/inferior.h | 2 +
gdb/infrun.c | 10 ++++
gdb/remote.c | 75 ++++++++++++++++++++++++++++++++
gdb/target.c | 31 +++++++++++++
gdb/target.h | 20 ++++++++
10 files changed, 322 insertions(+), 9 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 0d2e803..9c89346 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -97,6 +97,11 @@ z0/z1 conditional breakpoints extension
condition evaluation mode. The use of this extension can be controlled
via the "set remote conditional-breakpoints-packet" command.

+QProgramSignals:
+
+ Specify the signals which the remote stub may pass to the debugged
+ program without GDB involvement.
+
*** Changes in GDB 7.4

* GDB now handles ambiguous linespecs more consistently; the existing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 945b68d..e8bbded 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17514,6 +17514,10 @@ are:
@tab @code{QPassSignals}
@tab @code{handle @var{signal}}

+@item @code{program-signals}
+@tab @code{QProgramSignals}
+@tab @code{handle @var{signal}}
+
@item @code{hostio-close-packet}
@tab @code{vFile:close}
@tab @code{remote get}, @code{remote put}
@@ -34974,6 +34978,48 @@ 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 QProgramSignals: @var{signal} @r{[};@var{signal}@r{]}@dots{}
+@cindex signals the inferior may see, remote request
+@cindex @samp{QProgramSignals} packet
+@anchor{QProgramSignals}
+Each listed @var{signal} may be delivered to the inferior process.
+Others should be silently discarded.
+
+In some cases, the remote stub may need to decide whether to deliver a
+signal to the program or not without @value{GDBN} involvement. One
+example of that is while detaching --- the program's threads may have
+stopped for signals that haven't yet had a chance of being reported to
+@value{GDBN}, and so the remote stub can use the signal list specified
+by this packet to know whether to deliver or ignore those pending
+signals.
+
+This does not influence whether to deliver a signal as requested by a
+resumption packet (@pxref{vCont packet}).
+
+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. Multiple
+@samp{QProgramSignals} packets do not combine; any earlier
+@samp{QProgramSignals} list is completely replaced by the new list.
+
+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{QProgramSignals} is not supported
+by the stub.
+@end table
+
+Use of this packet is controlled by the @code{set remote program-signals}
+command (@pxref{Remote Configuration, set remote program-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
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4f8ec6b..997e5a0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -999,36 +999,127 @@ linux_kill (int pid)
return 0;
}

+/* Get pending signal of THREAD, for detaching purposes. This is the
+ signal the thread last stopped for, which we need to deliver to the
+ thread when detaching, otherwise, it'd be suppressed/lost. */
+
+static int
+get_detach_signal (struct thread_info *thread)
+{
+ enum target_signal signo = TARGET_SIGNAL_0;
+ int status;
+ struct lwp_info *lp = get_thread_lwp (thread);
+
+ if (lp->status_pending_p)
+ status = lp->status_pending;
+ else
+ {
+ /* If the thread had been suspended by gdbserver, and it stopped
+ cleanly, then it'll have stopped with SIGSTOP. But we don't
+ want to deliver that SIGSTOP. */
+ if (thread->last_status.kind != TARGET_WAITKIND_STOPPED
+ || thread->last_status.value.sig == TARGET_SIGNAL_0)
+ return 0;
+
+ /* Otherwise, we may need to deliver the signal we
+ intercepted. */
+ status = lp->last_status;
+ }
+
+ if (!WIFSTOPPED (status))
+ {
+ if (debug_threads)
+ fprintf (stderr,
+ "GPS: lwp %s hasn't stopped: no pending signal\n",
+ target_pid_to_str (ptid_of (lp)));
+ return 0;
+ }
+
+ /* Extended wait statuses aren't real SIGTRAPs. */
+ if (WSTOPSIG (status) == SIGTRAP && status >> 16 != 0)
+ {
+ if (debug_threads)
+ fprintf (stderr,
+ "GPS: lwp %s had stopped with extended "
+ "status: no pending signal\n",
+ target_pid_to_str (ptid_of (lp)));
+ return 0;
+ }
+
+ signo = target_signal_from_host (WSTOPSIG (status));
+
+ if (program_signals_p && !program_signals[signo])
+ {
+ if (debug_threads)
+ fprintf (stderr,
+ "GPS: lwp %s had signal %s, but it is in nopass state\n",
+ target_pid_to_str (ptid_of (lp)),
+ target_signal_to_string (signo));
+ return 0;
+ }
+ else if (!program_signals_p
+ /* If we have no way to know which signals GDB does not
+ want to have passed to the program, assume
+ SIGTRAP/SIGINT, which is GDB's default. */
+ && (signo == TARGET_SIGNAL_TRAP || signo == TARGET_SIGNAL_INT))
+ {
+ if (debug_threads)
+ fprintf (stderr,
+ "GPS: lwp %s had signal %s, "
+ "but we don't know if we should pass it. Default to not.\n",
+ target_pid_to_str (ptid_of (lp)),
+ target_signal_to_string (signo));
+ return 0;
+ }
+ else
+ {
+ if (debug_threads)
+ fprintf (stderr,
+ "GPS: lwp %s has pending signal %s: delivering it.\n",
+ target_pid_to_str (ptid_of (lp)),
+ target_signal_to_string (signo));
+
+ return WSTOPSIG (status);
+ }
+}
+
static int
linux_detach_one_lwp (struct inferior_list_entry *entry, void *args)
{
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
int pid = * (int *) args;
+ int sig;

if (ptid_get_pid (entry->id) != pid)
return 0;

- /* If this process is stopped but is expecting a SIGSTOP, then make
- sure we take care of that now. This isn't absolutely guaranteed
- to collect the SIGSTOP, but is fairly likely to. */
+ /* If there is a pending SIGSTOP, get rid of it. */
if (lwp->stop_expected)
{
- int wstat;
- /* Clear stop_expected, so that the SIGSTOP will be reported. */
+ if (debug_threads)
+ fprintf (stderr,
+ "Sending SIGCONT to %s\n",
+ target_pid_to_str (ptid_of (lwp)));
+
+ kill_lwp (lwpid_of (lwp), SIGCONT);
lwp->stop_expected = 0;
- linux_resume_one_lwp (lwp, 0, 0, NULL);
- linux_wait_for_event (lwp->head.id, &wstat, __WALL);
}

/* Flush any pending changes to the process's registers. */
regcache_invalidate_one ((struct inferior_list_entry *)
get_lwp_thread (lwp));

+ /* Pass on any pending signal for this thread. */
+ sig = get_detach_signal (thread);
+
/* Finally, let it resume. */
if (the_low_target.prepare_to_resume != NULL)
the_low_target.prepare_to_resume (lwp);
- ptrace (PTRACE_DETACH, lwpid_of (lwp), 0, 0);
+ if (ptrace (PTRACE_DETACH, lwpid_of (lwp), 0, sig) < 0)
+ error (_("Can't detach %s: %s"),
+ target_pid_to_str (ptid_of (lwp)),
+ strerror (errno));

delete_lwp (lwp);
return 0;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 0de3f52..586581c 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -59,6 +59,8 @@ int debug_threads;
int debug_hw_points;

int pass_signals[TARGET_SIGNAL_LAST];
+int program_signals[TARGET_SIGNAL_LAST];
+int program_signals_p;

jmp_buf toplevel;

@@ -455,6 +457,33 @@ handle_general_set (char *own_buf)
return;
}

+ if (strncmp ("QProgramSignals:", own_buf, strlen ("QProgramSignals:")) == 0)
+ {
+ int numsigs = (int) TARGET_SIGNAL_LAST, i;
+ const char *p = own_buf + strlen ("QProgramSignals:");
+ CORE_ADDR cursig;
+
+ program_signals_p = 1;
+
+ p = decode_address_to_semicolon (&cursig, p);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (i == cursig)
+ {
+ program_signals[i] = 1;
+ if (*p == '\0')
+ /* Keep looping, to clear the remaining signals. */
+ cursig = -1;
+ else
+ p = decode_address_to_semicolon (&cursig, p);
+ }
+ else
+ program_signals[i] = 0;
+ }
+ strcpy (own_buf, "OK");
+ return;
+ }
+
if (strcmp (own_buf, "QStartNoAckMode") == 0)
{
if (remote_debug)
@@ -1584,7 +1613,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
free (qsupported);
}

- sprintf (own_buf, "PacketSize=%x;QPassSignals+", PBUFSIZ - 1);
+ sprintf (own_buf,
+ "PacketSize=%x;QPassSignals+;QProgramSignals+",
+ PBUFSIZ - 1);

if (the_target->qxfer_libraries_svr4 != NULL)
strcat (own_buf, ";qXfer:libraries-svr4:read+");
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index fad58e8..a419c36 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -292,6 +292,8 @@ extern int server_waiting;
extern int debug_threads;
extern int debug_hw_points;
extern int pass_signals[];
+extern int program_signals[];
+extern int program_signals_p;

extern jmp_buf toplevel;

diff --git a/gdb/inferior.h b/gdb/inferior.h
index 9b2817f..1563111 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -645,6 +645,8 @@ extern struct inferior *add_inferior_with_spaces (void);

extern void update_observer_mode (void);

+extern void update_signals_program_target (void);
+
/* In some circumstances we allow a command to specify a numeric
signal. The idea is to keep these circumstances limited so that
users (and scripts) develop portable habits. For comparison,
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e383d77..103ef30 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -335,6 +335,15 @@ static unsigned char *signal_pass;
(flags)[signum] = 0; \
} while (0)

+/* Update the target's copy of SIGNAL_PROGRAM. The sole purpose of
+ this function is to avoid exporting `signal_program'. */
+
+void
+update_signals_program_target (void)
+{
+ target_program_signals ((int) TARGET_SIGNAL_LAST, signal_program);
+}
+
/* Value to pass to target_resume() to cause all threads to resume. */

#define RESUME_ALL minus_one_ptid
@@ -6363,6 +6372,7 @@ Are you sure you want to change it? "),
{
signal_cache_update (-1);
target_pass_signals ((int) TARGET_SIGNAL_LAST, signal_pass);
+ target_program_signals ((int) TARGET_SIGNAL_LAST, signal_program);

if (from_tty)
{
diff --git a/gdb/remote.c b/gdb/remote.c
index 45fec17..b3a331e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1259,6 +1259,7 @@ enum {
PACKET_qGetTLSAddr,
PACKET_qSupported,
PACKET_QPassSignals,
+ PACKET_QProgramSignals,
PACKET_qSearch_memory,
PACKET_vAttach,
PACKET_vRun,
@@ -1669,6 +1670,65 @@ remote_pass_signals (int numsigs, unsigned char *pass_signals)
}
}

+/* The last QProgramSignals packet sent to the target. We bypass
+ sending a new program signals list down to the target if the new
+ packet is exactly the same as the last we sent. IOW, we only let
+ the target know about program signals list changes. */
+
+static char *last_program_signals_packet;
+
+/* If 'QProgramSignals' is supported, tell the remote stub what
+ signals it should pass through to the inferior when detaching. */
+
+static void
+remote_program_signals (int numsigs, unsigned char *signals)
+{
+ if (remote_protocol_packets[PACKET_QProgramSignals].support != PACKET_DISABLE)
+ {
+ char *packet, *p;
+ int count = 0, i;
+
+ gdb_assert (numsigs < 256);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (signals[i])
+ count++;
+ }
+ packet = xmalloc (count * 3 + strlen ("QProgramSignals:") + 1);
+ strcpy (packet, "QProgramSignals:");
+ p = packet + strlen (packet);
+ for (i = 0; i < numsigs; i++)
+ {
+ if (signal_pass_state (i))
+ {
+ if (i >= 16)
+ *p++ = tohex (i >> 4);
+ *p++ = tohex (i & 15);
+ if (count)
+ *p++ = ';';
+ else
+ break;
+ count--;
+ }
+ }
+ *p = 0;
+ if (!last_program_signals_packet
+ || strcmp (last_program_signals_packet, packet) != 0)
+ {
+ struct remote_state *rs = get_remote_state ();
+ char *buf = rs->buf;
+
+ putpkt (packet);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+ packet_ok (buf, &remote_protocol_packets[PACKET_QProgramSignals]);
+ xfree (last_program_signals_packet);
+ last_program_signals_packet = packet;
+ }
+ else
+ xfree (packet);
+ }
+}
+
/* If PTID is MAGIC_NULL_PTID, don't set any thread. If PTID is
MINUS_ONE_PTID, set the thread to -1, so the stub returns the
thread. If GEN is set, set the general thread, if not, then set
@@ -3246,6 +3306,10 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
getpkt (&rs->buf, &rs->buf_size, 0);
}

+ /* Let the target know which signals it is allowed to pass down to
+ the program. */
+ update_signals_program_target ();
+
/* Next, if the target can specify a description, read it. We do
this before anything involving memory or registers. */
target_find_description ();
@@ -3803,6 +3867,8 @@ static struct protocol_feature remote_protocol_features[] = {
PACKET_qXfer_traceframe_info },
{ "QPassSignals", PACKET_DISABLE, remote_supported_packet,
PACKET_QPassSignals },
+ { "QProgramSignals", PACKET_DISABLE, remote_supported_packet,
+ PACKET_QProgramSignals },
{ "QStartNoAckMode", PACKET_DISABLE, remote_supported_packet,
PACKET_QStartNoAckMode },
{ "multiprocess", PACKET_DISABLE, remote_multi_process_feature, -1 },
@@ -4073,6 +4139,11 @@ remote_open_1 (char *name, int from_tty,
xfree (last_pass_packet);
last_pass_packet = NULL;

+ /* Make sure we send the program signals list the next time we
+ resume. */
+ xfree (last_program_signals_packet);
+ last_program_signals_packet = NULL;
+
remote_fileio_reset ();
reopen_exec_file ();
reread_symbols ();
@@ -10819,6 +10890,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_load = generic_load;
remote_ops.to_mourn_inferior = remote_mourn;
remote_ops.to_pass_signals = remote_pass_signals;
+ remote_ops.to_program_signals = remote_program_signals;
remote_ops.to_thread_alive = remote_thread_alive;
remote_ops.to_find_new_threads = remote_threads_info;
remote_ops.to_pid_to_str = remote_pid_to_str;
@@ -11263,6 +11335,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QPassSignals],
"QPassSignals", "pass-signals", 0);

+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QProgramSignals],
+ "QProgramSignals", "program-signals", 0);
+
add_packet_config_cmd (&remote_protocol_packets[PACKET_qSymbol],
"qSymbol", "symbol-lookup", 0);

diff --git a/gdb/target.c b/gdb/target.c
index 88703ea..cffea2c 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -639,6 +639,7 @@ update_current_target (void)
/* Do not inherit to_mourn_inferior. */
INHERIT (to_can_run, t);
/* Do not inherit to_pass_signals. */
+ /* Do not inherit to_program_signals. */
/* Do not inherit to_thread_alive. */
/* Do not inherit to_find_new_threads. */
/* Do not inherit to_pid_to_str. */
@@ -2728,6 +2729,36 @@ target_pass_signals (int numsigs, unsigned char *pass_signals)
}
}

+void
+target_program_signals (int numsigs, unsigned char *program_signals)
+{
+ struct target_ops *t;
+
+ for (t = current_target.beneath; t != NULL; t = t->beneath)
+ {
+ if (t->to_program_signals != NULL)
+ {
+ if (targetdebug)
+ {
+ int i;
+
+ fprintf_unfiltered (gdb_stdlog, "target_program_signals (%d, {",
+ numsigs);
+
+ for (i = 0; i < numsigs; i++)
+ if (program_signals[i])
+ fprintf_unfiltered (gdb_stdlog, " %s",
+ target_signal_to_name (i));
+
+ fprintf_unfiltered (gdb_stdlog, " })\n");
+ }
+
+ (*t->to_program_signals) (numsigs, program_signals);
+ return;
+ }
+ }
+}
+
/* Look through the list of possible targets for a target that can
follow forks. */

diff --git a/gdb/target.h b/gdb/target.h
index ed2bcdd..50a0ea6 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -511,6 +511,10 @@ struct target_ops
target_* macro. */
void (*to_pass_signals) (int, unsigned char *);

+ /* Documentation of this routine is provided with the
+ corresponding target_* function. */
+ void (*to_program_signals) (int, unsigned char *);
+
int (*to_thread_alive) (struct target_ops *, ptid_t ptid);
void (*to_find_new_threads) (struct target_ops *);
char *(*to_pid_to_str) (struct target_ops *, ptid_t);
@@ -1261,6 +1265,22 @@ void target_mourn_inferior (void);

extern void target_pass_signals (int nsig, unsigned char *pass_signals);

+/* Set list of signals the target may pass to the inferior. This
+ directly maps to the "handle SIGNAL pass/nopass" setting.
+
+ PROGRAM_SIGNALS is an array of size NSIG, indexed by target signal
+ number (enum target_signal). For every signal whose entry in this
+ array is non-zero, the target is allowed to pass the signal to the
+ inferior. Signals not present in the array shall be silently
+ discarded. This does not influence whether to pass signals to the
+ inferior as a result of a target_resume call. This is useful in
+ scenarios where the target needs to decide whether to pass or not a
+ signal to the inferior without GDB core involvement, such as for
+ example, when detaching (as threads may have been suspended with
+ pending signals not reported to GDB). */
+
+extern void target_program_signals (int nsig, unsigned char *program_signals);
+
/* Check to see if a thread is still alive. */

extern int target_thread_alive (ptid_t ptid);