Skip to content

Commit

Permalink
gdbserver: Leave already-vCont-resumed threads as they were
Browse files Browse the repository at this point in the history
Currently GDB never sends more than one action per vCont packet, when
connected in non-stop mode.  A follow up patch will change that, and
it exposed a gdbserver problem with the vCont handling.

For example, this in non-stop mode:

  => vCont;s:p1.1;c
  <= OK

Should be equivalent to:

  => vCont;s:p1.1
  <= OK
  => vCont;c
  <= OK

But gdbserver currently doesn't handle this.  In the latter case,
"vCont;c" makes gdbserver clobber the previous step request.  This
patch fixes that.

Note the server side must ignore resume actions for the thread that
has a pending %Stopped notification (and any other threads with events
pending), until GDB acks the notification with vStopped.  Otherwise,
e.g., the following case is mishandled:

 tromey#1 => g  (or any other packet)
 tromey#2 <= [registers]
 tromey#3 <= %Stopped T05 thread:p1.2
 tromey#4 => vCont s:p1.1;c
 tromey#5 <= OK

Above, the server must not resume thread p1.2 when it processes the
vCont.  GDB can't know that p1.2 stopped until it acks the %Stopped
notification.  (Otherwise it wouldn't send a default "c" action.)

(The vCont documentation already specifies this.)

Finally, special care must also be given to handling fork/vfork
events.  A (v)fork event actually tells us that two processes stopped
-- the parent and the child.  Until we follow the fork, we must not
resume the child.  Therefore, if we have a pending fork follow, we
must not send a global wildcard resume action (vCont;c).  We can still
send process-wide wildcards though.

(The comments above will be added as code comments to gdb in a follow
up patch.)

gdb/gdbserver/ChangeLog:
2016-10-26  Pedro Alves  <[email protected]>

	* linux-low.c (handle_extended_wait): Link parent/child fork
	threads.
	(linux_wait_1): Unlink them.
	(linux_set_resume_request): Ignore resume requests for
	already-resumed and unhandled fork child threads.
	* linux-low.h (struct lwp_info) <fork_relative>: New field.
	* server.c (in_queued_stop_replies_ptid, in_queued_stop_replies):
	New functions.
	(handle_v_requests) <vCont>: Don't call require_running.
	* server.h (in_queued_stop_replies): New declaration.
  • Loading branch information
palves committed Oct 26, 2016
1 parent ca6eff5 commit 5a04c4c
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 1 deletion.
13 changes: 13 additions & 0 deletions gdb/gdbserver/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
2016-10-26 Pedro Alves <[email protected]>

* linux-low.c (handle_extended_wait): Link parent/child fork
threads.
(linux_wait_1): Unlink them.
(linux_set_resume_request): Ignore resume requests for
already-resumed and unhandled fork child threads.
* linux-low.h (struct lwp_info) <fork_relative>: New field.
* server.c (in_queued_stop_replies_ptid, in_queued_stop_replies):
New functions.
(handle_v_requests) <vCont>: Don't call require_running.
* server.h (in_queued_stop_replies): New declaration.

2016-10-24 Yao Qi <[email protected]>

PR server/20733
Expand Down
59 changes: 59 additions & 0 deletions gdb/gdbserver/linux-low.c
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
event_lwp->status_pending_p = 1;
event_lwp->status_pending = wstat;

/* Link the threads until the parent event is passed on to
higher layers. */
event_lwp->fork_relative = child_lwp;
child_lwp->fork_relative = event_lwp;

/* If the parent thread is doing step-over with single-step
breakpoints, the list of single-step breakpoints are cloned
from the parent's. Remove them from the child process.
Expand Down Expand Up @@ -3853,6 +3858,15 @@ linux_wait_1 (ptid_t ptid,
{
/* If the reported event is an exit, fork, vfork or exec, let
GDB know. */

/* Break the unreported fork relationship chain. */
if (event_child->waitstatus.kind == TARGET_WAITKIND_FORKED
|| event_child->waitstatus.kind == TARGET_WAITKIND_VFORKED)
{
event_child->fork_relative->fork_relative = NULL;
event_child->fork_relative = NULL;
}

*ourstatus = event_child->waitstatus;
/* Clear the event lwp's waitstatus since we handled it already. */
event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
Expand Down Expand Up @@ -4654,6 +4668,51 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
continue;
}

/* Ignore (wildcard) resume requests for already-resumed
threads. */
if (r->resume[ndx].kind != resume_stop
&& thread->last_resume_kind != resume_stop)
{
if (debug_threads)
debug_printf ("already %s LWP %ld at GDB's request\n",
(thread->last_resume_kind
== resume_step)
? "stepping"
: "continuing",
lwpid_of (thread));
continue;
}

/* Don't let wildcard resumes resume fork children that GDB
does not yet know are new fork children. */
if (lwp->fork_relative != NULL)
{
struct inferior_list_entry *inf, *tmp;
struct lwp_info *rel = lwp->fork_relative;

if (rel->status_pending_p
&& (rel->waitstatus.kind == TARGET_WAITKIND_FORKED
|| rel->waitstatus.kind == TARGET_WAITKIND_VFORKED))
{
if (debug_threads)
debug_printf ("not resuming LWP %ld: has queued stop reply\n",
lwpid_of (thread));
continue;
}
}

/* If the thread has a pending event that has already been
reported to GDBserver core, but GDB has not pulled the
event out of the vStopped queue yet, likewise, ignore the
(wildcard) resume request. */
if (in_queued_stop_replies (entry->id))
{
if (debug_threads)
debug_printf ("not resuming LWP %ld: has queued stop reply\n",
lwpid_of (thread));
continue;
}

lwp->resume = &r->resume[ndx];
thread->last_resume_kind = lwp->resume->kind;

Expand Down
6 changes: 6 additions & 0 deletions gdb/gdbserver/linux-low.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,12 @@ struct lwp_info
information or exit status until it can be reported to GDB. */
struct target_waitstatus waitstatus;

/* A pointer to the fork child/parent relative. Valid only while
the parent fork event is not reported to higher layers. Used to
avoid wildcard vCont actions resuming a fork child before GDB is
notified about the parent's fork event. */
struct lwp_info *fork_relative;

/* When stopped is set, this is where the lwp last stopped, with
decr_pc_after_break already accounted for. If the LWP is
running, this is the address at which the lwp was resumed. */
Expand Down
33 changes: 32 additions & 1 deletion gdb/gdbserver/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,38 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
prepare_resume_reply (own_buf, vstop->ptid, &vstop->status);
}

/* QUEUE_iterate callback helper for in_queued_stop_replies. */

static int
in_queued_stop_replies_ptid (QUEUE (notif_event_p) *q,
QUEUE_ITER (notif_event_p) *iter,
struct notif_event *event,
void *data)
{
ptid_t filter_ptid = *(ptid_t *) data;
struct vstop_notif *vstop_event = (struct vstop_notif *) event;

if (ptid_match (vstop_event->ptid, filter_ptid))
return 0;

/* Don't resume fork children that GDB does not know about yet. */
if ((vstop_event->status.kind == TARGET_WAITKIND_FORKED
|| vstop_event->status.kind == TARGET_WAITKIND_VFORKED)
&& ptid_match (vstop_event->status.value.related_pid, filter_ptid))
return 0;

return 1;
}

/* See server.h. */

int
in_queued_stop_replies (ptid_t ptid)
{
return !QUEUE_iterate (notif_event_p, notif_stop.queue,
in_queued_stop_replies_ptid, &ptid);
}

struct notif_server notif_stop =
{
"vStopped", "Stop", NULL, vstop_notif_reply,
Expand Down Expand Up @@ -2947,7 +2979,6 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)

if (startswith (own_buf, "vCont;"))
{
require_running (own_buf);
handle_v_cont (own_buf);
return;
}
Expand Down
4 changes: 4 additions & 0 deletions gdb/gdbserver/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ extern int handle_target_event (int err, gdb_client_data client_data);
/* Get rid of the currently pending stop replies that match PTID. */
extern void discard_queued_stop_replies (ptid_t ptid);

/* Returns true if there's a pending stop reply that matches PTID in
the vStopped notifications queue. */
extern int in_queued_stop_replies (ptid_t ptid);

#include "remote-utils.h"

#include "utils.h"
Expand Down

0 comments on commit 5a04c4c

Please sign in to comment.