This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

fix gdbserver re-attach (extended-remote attach.exp)


I've applied the gdbserver patch below.

While running the testsuite with gdbserver in extended-remote mode, I stumbled on attach.exp
failing with timeouts, where gdbserver wouldn't reply to a vAttach request.  It was
attaching with success, but then failing to stop the inferior, and reporting
back the stop to gdb.  What happens is that the test does an attach, does some
usual debugging, involving resuming the inferior, and then runs the inferior till
exit.  That part went well.  Then the test attaches to some other inferior, and
this failed ("attach" times out).  What happened is that linux-low.c:linux_wait_1
has this bit:

  /* If we were only supposed to resume one thread, only wait for
     that thread - if it's still alive.  If it died, however - which
     can happen if we're coming from the thread death case below -
     then we need to make sure we restart the other threads.  We could
     pick a thread at random or restart all; restarting all is less
     arbitrary.  */
  if (!non_stop
      && !ptid_equal (cont_thread, null_ptid)
      && !ptid_equal (cont_thread, minus_one_ptid))
    {
      struct thread_info *thread;

      thread = (struct thread_info *) find_inferior_id (&all_threads,
							cont_thread);

      /* No stepping, no signal - unless one is pending already, of course.  */
      if (thread == NULL)
	{
	  struct thread_resume resume_info;
	  resume_info.thread = minus_one_ptid;
	  resume_info.kind = resume_continue;
	  resume_info.sig = 0;
	  linux_resume (&resume_info, 1);   <<<<<<<<<<<
	}
      else
	ptid = cont_thread;
    }


and we were reaching the linux_resume marked above.  My first reaction was,
well, why are we reaching here with cont_thread set?  That variable should only
be used when the target does _not_ support vCont, it is set by the older
Hc packet.  Turns out that is not true, and note the comment pasted
above -- this really implementing scheduler locking, but right within the
target, instead of at the caller (and that resume is highly questionable, no
doubt a consequence of there being no support for reporting a thread exit
to gdb).

cont_thread is also set in server.c:handle_v_cont, like:

  /* Still used in occasional places in the backend.  */
  if (n == 1
      && !ptid_equal (resume_info[0].thread, minus_one_ptid)
      && resume_info[0].kind != resume_stop)
    cont_thread = resume_info[0].thread;
  else
    cont_thread = minus_one_ptid;

If I remove these bits, no longer setting cont_thread in the vCont
packets, then schedlock.exp starts failing.  Fixing this will require
some surgery, so I'm taking the easy route for now, so to concentrate
on getting the board file for extended-remote runs more fit for
general consumption first.  Clear `cont_thread' when attaching, and
still leave the schedlocking implementation in place.

gdb/gdbserver/
2012-01-13  Pedro Alves  <palves@redhat.com>

	* server.c (attach_inferior): Clear `cont_thread'.

---
 gdb/gdbserver/server.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f312a5c..bebccf5 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -338,6 +338,10 @@ attach_inferior (int pid)
      whichever we were told to attach to.  */
   signal_pid = pid;

+  /* Clear this so the backend doesn't get confused, thinking
+     CONT_THREAD died, and it needs to resume all threads.  */
+  cont_thread = null_ptid;
+
   if (!non_stop)
     {
       last_ptid = mywait (pid_to_ptid (pid), &last_status, 0, 0);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]