Toolchain/binutils-gdb
1
0
Fork 0
forked from Imagelibrary/binutils-gdb
Code Pull Requests Activity

[remote/gdbserver] Don't lose signals when reconnecting.

Browse Source 
Currently, when GDB connects in all-stop mode, GDBserver always
responds to the status packet with a GDB_SIGNAL_TRAP, even if the
program is actually stopped for some other signal.

 (gdb) tar rem ...
 ...
 (gdb) c
 Program received signal SIGUSR1, User defined signal 1.
 (gdb) disconnect
 (gdb) tar rem ...
 (gdb) c

(Or a GDB crash instead of an explicit disconnect.)

This results in the program losing that signal on that last continue,
because gdb will tell the target to resume with no signal (to suppress
the GDB_SIGNAL_TRAP, due to 'handle SISGTRAP nopass'), and that will
actually suppress the real signal the program had stopped for
(SIGUSR1).  To fix that, I think we should make GDBserver report the
real signal the thread had stopped for in response to the status
packet:

 @item ?
 @cindex @samp{?} packet
 Indicate the reason the target halted.  The reply is the same as for
 step and continue.

But, that raises the question -- which thread are we reporting the
status for?  Due to how the RSP in all-stop works, we can only report
one status.  The status packet's response is a stop reply packet, so
it includes the thread identifier, so it's not a problem packet-wise.
However, GDBserver is currently always reporting the status for first
thread in the thread list, even though that may well not be the thread
that got the signal that caused the program to stop.  So the next
logical step would be to report the status for the
last_ptid/last_status thread (the last event reported to gdb), if it's
still around; and if not, fallback to some other thread.

There's an issue on the GDB side with that, though...

GDB currently always adds the thread reported in response to the
status query as the first thread in its list.  That means that if we
start with e.g.,

 (gdb) info threads
   3 Thread 1003 ...
 * 2 Thread 1002 ...
   1 Thread 1001 ...

And reconnect:

 (gdb) disconnect
 (gdb) tar rem ...

We end up with:

 (gdb) info threads
   3 Thread 1003 ...
   2 Thread 1001 ...
 * 1 Thread 1002 ...

Not a real big issue, but it's reasonably fixable, by having GDB
fetch/sync the thread list before fetching the status/'?', and then
using the status to select the right thread as current on the GDB
side.  Holes in the thread numbers are squashed before/after
reconnection (e.g., 2,3,5 becomes 1,2,3), but the order is preserved,
which I think is both good, and good enough.

However (yes, there's more...), the previous GDB that was connected
might have had gdbserver running in non-stop mode, or could have left
gdbserver doing disconnected tracing (which also forces non-stop), and
if the new gdb/connection is in all-stop mode, we can end up with more
than one thread with a signal to report back to gdb.  As we can only
report one thread/status (in the all-stop RSP variant; the non-stop
variant doesn't have this issue), we get to do what we do at every
other place we have this situation -- leave events we can't report
right now as pending, so that the next resume picks them up.

Note all this ammounts to a QoI change, within the existing framework.
There's really no RSP change here.

The only user visible change (other than that the signal is program is
stopped at isn't lost / is passed to the program), is in "info
program", that now can show the signal the program stopped for.  Of
course, the next resume will respect the pass/nopass setting for the
signal in question.  It'd be reasonable to have the initial connection
tell the user the program was stopped with a signal, similar to when
we load a core to debug, but I'm leaving that out for a future change.
I think we'll need to either change how handle_inferior_event & co
handle stop_soon, or maybe bypass them completely (like
fork-child.c:startup_inferior) for that.

Tested on x86_64 Fedora 17.

gdb/gdbserver/
2014-01-08  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (struct thread_info) <status_pending_p>: New field.
	* server.c (visit_actioned_threads, handle_pending_status): New
	function.
	(handle_v_cont): Factor out parts to ...
	(resume): ... this new function.  If in all-stop, and a thread
	being resumed has a pending status, report it without actually
	resuming.
	(myresume): Adjust to use the new 'resume' function.
	(clear_pending_status_callback, set_pending_status_callback)
	(find_status_pending_thread_callback): New functions.
	(handle_status): Handle the case of multiple threads having
	interesting statuses to report.  Report threads' real last signal
	instead of always reporting GDB_SIGNAL_TRAP.  Look for a thread
	with an interesting thread to report the status for, instead of
	always reporting the status of the first thread.

gdb/
2014-01-08  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_add_thread): Add threads silently if starting
	up.
	(remote_notice_new_inferior): If in all-stop, and starting up,
	don't call notice_new_inferior.
	(get_current_thread): New function, factored out from ...
	(add_current_inferior_and_thread): ... this.  Adjust.
	(remote_start_remote) <all-stop>: Fetch the thread list.  If we
	found any thread, then select the remote's current thread as GDB's
	current thread too.

gdb/testsuite/
2014-01-08  Pedro Alves  <palves@redhat.com>

	* gdb.threads/reconnect-signal.c: New file.
	* gdb.threads/reconnect-signal.exp: New file.
This commit is contained in:
Pedro Alves
2014-01-08 18:55:51 +00:00
parent 143e9f4a65
commit b7ea362b02
8 changed files with 431 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

216
gdb/gdbserver/server.c
View File

@@ -2016,6 +2016,63 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
}
static void gdb_wants_all_threads_stopped (void);
static void resume (struct thread_resume *actions, size_t n);
/* Call CALLBACK for any thread to which ACTIONS applies to. Returns
true if CALLBACK returns true. Returns false if no matching thread
is found or CALLBACK results false. */
static int
visit_actioned_threads (const struct thread_resume *actions,
size_t num_actions,
int (*callback) (const struct thread_resume *,
struct thread_info *))
{
struct inferior_list_entry *entry;
for (entry = all_threads.head; entry != NULL; entry = entry->next)
{
size_t i;
for (i = 0; i < num_actions; i++)
{
const struct thread_resume *action = &actions[i];
if (ptid_equal (action->thread, minus_one_ptid)
|| ptid_equal (action->thread, entry->id)
|| ((ptid_get_pid (action->thread)
== ptid_get_pid (entry->id))
&& ptid_get_lwp (action->thread) == -1))
{
struct thread_info *thread = (struct thread_info *) entry;
if ((*callback) (action, thread))
return 1;
}
}
}
return 0;
}
/* Callback for visit_actioned_threads. If the thread has a pending
status to report, report it now. */
static int
handle_pending_status (const struct thread_resume *resumption,
struct thread_info *thread)
{
if (thread->status_pending_p)
{
thread->status_pending_p = 0;
last_status = thread->last_status;
last_ptid = thread->entry.id;
prepare_resume_reply (own_buf, last_ptid, &last_status);
return 1;
}
return 0;
}
/* Parse vCont packets. */
void
@@ -2128,12 +2185,34 @@ handle_v_cont (char *own_buf)
cont_thread = minus_one_ptid;
set_desired_inferior (0);
if (!non_stop)
enable_async_io ();
(*the_target->resume) (resume_info, n);
resume (resume_info, n);
free (resume_info);
return;
err:
write_enn (own_buf);
free (resume_info);
return;
}
/* Resume target with ACTIONS, an array of NUM_ACTIONS elements. */
static void
resume (struct thread_resume *actions, size_t num_actions)
{
if (!non_stop)
{
/* Check if among the threads that GDB wants actioned, there's
one with a pending status to report. If so, skip actually
resuming/stopping and report the pending event
immediately. */
if (visit_actioned_threads (actions, num_actions, handle_pending_status))
return;
enable_async_io ();
}
(*the_target->resume) (actions, num_actions);
if (non_stop)
write_ok (own_buf);
@@ -2157,12 +2236,6 @@ handle_v_cont (char *own_buf)
|| last_status.kind == TARGET_WAITKIND_SIGNALLED)
mourn_inferior (find_process_pid (ptid_get_pid (last_ptid)));
}
return;
err:
write_enn (own_buf);
free (resume_info);
return;
}
/* Attach to a new program. Return 1 if successful, 0 if failure. */
@@ -2422,31 +2495,7 @@ myresume (char *own_buf, int step, int sig)
n++;
}
if (!non_stop)
enable_async_io ();
(*the_target->resume) (resume_info, n);
if (non_stop)
write_ok (own_buf);
else
{
last_ptid = mywait (minus_one_ptid, &last_status, 0, 1);
if (last_status.kind != TARGET_WAITKIND_EXITED
&& last_status.kind != TARGET_WAITKIND_SIGNALLED)
{
current_inferior->last_resume_kind = resume_stop;
current_inferior->last_status = last_status;
}
prepare_resume_reply (own_buf, last_ptid, &last_status);
disable_async_io ();
if (last_status.kind == TARGET_WAITKIND_EXITED
|| last_status.kind == TARGET_WAITKIND_SIGNALLED)
mourn_inferior (find_process_pid (ptid_get_pid (last_ptid)));
}
resume (resume_info, n);
}
/* Callback for for_each_inferior. Make a new stop reply for each
@@ -2536,6 +2585,48 @@ gdb_reattached_process (struct inferior_list_entry *entry)
process->gdb_detached = 0;
}
/* Callback for for_each_inferior. Clear the thread's pending status
flag. */
static void
clear_pending_status_callback (struct inferior_list_entry *entry)
{
struct thread_info *thread = (struct thread_info *) entry;
thread->status_pending_p = 0;
}
/* Callback for for_each_inferior. If the thread is stopped with an
interesting event, mark it as having a pending event. */
static void
set_pending_status_callback (struct inferior_list_entry *entry)
{
struct thread_info *thread = (struct thread_info *) entry;
if (thread->last_status.kind != TARGET_WAITKIND_STOPPED
|| (thread->last_status.value.sig != GDB_SIGNAL_0
/* A breakpoint, watchpoint or finished step from a previous
GDB run isn't considered interesting for a new GDB run.
If we left those pending, the new GDB could consider them
random SIGTRAPs. This leaves out real async traps. We'd
have to peek into the (target-specific) siginfo to
distinguish those. */
&& thread->last_status.value.sig != GDB_SIGNAL_TRAP))
thread->status_pending_p = 1;
}
/* Callback for find_inferior. Return true if ENTRY (a thread) has a
pending status to report to GDB. */
static int
find_status_pending_thread_callback (struct inferior_list_entry *entry, void *data)
{
struct thread_info *thread = (struct thread_info *) entry;
return thread->status_pending_p;
}
/* Status handler for the '?' packet. */
static void
@@ -2544,13 +2635,15 @@ handle_status (char *own_buf)
/* GDB is connected, don't forward events to the target anymore. */
for_each_inferior (&all_processes, gdb_reattached_process);
discard_queued_stop_replies (-1);
for_each_inferior (&all_threads, clear_pending_status_callback);
/* In non-stop mode, we must send a stop reply for each stopped
thread. In all-stop mode, just send one for the first stopped
thread we find. */
if (non_stop)
{
discard_queued_stop_replies (-1);
find_inferior (&all_threads, queue_stop_reply_callback, NULL);
/* The first is sent immediatly. OK is sent if there is no
@@ -2560,18 +2653,53 @@ handle_status (char *own_buf)
}
else
{
struct inferior_list_entry *thread = NULL;
pause_all (0);
stabilize_threads ();
gdb_wants_all_threads_stopped ();
if (all_threads.head)
{
struct target_waitstatus status;
/* We can only report one status, but we might be coming out of
non-stop -- if more than one thread is stopped with
interesting events, leave events for the threads we're not
reporting now pending. They'll be reported the next time the
threads are resumed. Start by marking all interesting events
as pending. */
for_each_inferior (&all_threads, set_pending_status_callback);
status.kind = TARGET_WAITKIND_STOPPED;
status.value.sig = GDB_SIGNAL_TRAP;
prepare_resume_reply (own_buf,
all_threads.head->id, &status);
/* Prefer the last thread that reported an event to GDB (even if
that was a GDB_SIGNAL_TRAP). */
if (last_status.kind != TARGET_WAITKIND_IGNORE
&& last_status.kind != TARGET_WAITKIND_EXITED
&& last_status.kind != TARGET_WAITKIND_SIGNALLED)
thread = find_inferior_id (&all_threads, last_ptid);
/* If the last event thread is not found for some reason, look
for some other thread that might have an event to report. */
if (thread == NULL)
thread = find_inferior (&all_threads,
find_status_pending_thread_callback, NULL);
/* If we're still out of luck, simply pick the first thread in
the thread list. */
if (thread == NULL)
thread = all_threads.head;
if (thread != NULL)
{
struct thread_info *tp = (struct thread_info *) thread;
/* We're reporting this event, so it's no longer
pending. */
tp->status_pending_p = 0;
/* GDB assumes the current thread is the thread we're
reporting the status for. */
general_thread = thread->id;
set_desired_inferior (1);
gdb_assert (tp->last_status.kind != TARGET_WAITKIND_IGNORE);
prepare_resume_reply (own_buf, tp->entry.id, &tp->last_status);
}
else
strcpy (own_buf, "W00");