OpenE2K
/
binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Tweak handling of remote errors in response to resumption packet 

With current master, on a Fedora 27 machine with a kernel with buggy
watchpoint support, I see:

  (gdb) PASS: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: hardware breakpoints work
  continue
  Continuing.
  warning: Remote failure reply: E01
  Remote communication error.  Target disconnected.: Connection reset by peer.
  (gdb) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: watchpoints work
  continue
  The program is not being run.
  (gdb) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: breakpoint after the first fork (the program is no longer running)

The FAILs themselves aren't what's interesting here.  What is
interesting is that with the main multi-target patch applied, I was getting this:

  (gdb) PASS: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: hardware breakpoints work
  continue
  Continuing.
  warning: Remote failure reply: E01
  /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/inferior.c:285: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: watchpoints work (GDB internal error)

The problem is that in remote_target::wait_as, we're hitting this:

  switch (buf[0])
    {
    case 'E':		/* Error of some sort.	*/
      /* We're out of sync with the target now.  Did it continue or
	 not?  Not is more likely, so report a stop.  */
      rs->waiting_for_stop_reply = 0;

      warning (_("Remote failure reply: %s"), buf);
      status->kind = TARGET_WAITKIND_STOPPED;
      status->value.sig = GDB_SIGNAL_0;
      break;

which leaves event_ptid as null_ptid.  At the end of the function, we then reach:

  else if (status->kind != TARGET_WAITKIND_EXITED
	   && status->kind != TARGET_WAITKIND_SIGNALLED)
    {
      if (event_ptid != null_ptid)
	record_currthread (rs, event_ptid);
      else
	event_ptid = inferior_ptid;                 <<<<< here
    }

and the trouble is that with the multi-target patch, we'll get here
with inferior_ptid as null_ptid too.  That is done exactly to find
these implicit assumptions that inferior_ptid is a good choice for
default thread, which isn't generaly true.

I first thought of fixing this in the "case 'E'" path, but, given that
this "event_ptid = inferior_ptid" path is also taken when the remote
target does not support threads at all, no thread-related packets or
extensions, it's better to fix it in latter path, to handle all
scenarios that miss reporting a thread.

That's what this patch does.

gdb/ChangeLog:
2020-01-10  Pedro Alves  <palves@redhat.com>

	* remote.c (first_remote_resumed_thread): New.
	(remote_target::wait_as): Use it as default event_ptid instead of
	inferior_ptid.
This commit is contained in:
Pedro Alves 2020-01-10 20:05:52 +00:00
parent 735fc2ca68
commit 31ba933ec6
2 changed files with 18 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
gdb/ChangeLog
View File

@ -1,3 +1,9 @@
2020-01-10 Pedro Alves <palves@redhat.com>
* remote.c (first_remote_resumed_thread): New.
(remote_target::wait_as): Use it as default event_ptid instead of
inferior_ptid.
2020-01-10 Pedro Alves <palves@redhat.com>
* infrun.c (handle_no_resumed): Use all_non_exited_inferiors.

13
gdb/remote.c
View File

@ -7692,6 +7692,17 @@ remote_target::wait_ns (ptid_t ptid, struct target_waitstatus *status, int optio
}
}
/* Return the first resumed thread. */
static ptid_t
first_remote_resumed_thread ()
{
for (thread_info *tp : all_non_exited_threads (minus_one_ptid))
if (tp->resumed)
return tp->ptid;
return null_ptid;
}
/* Wait until the remote machine stops, then return, storing status in
STATUS just as `wait' would. */
@ -7828,7 +7839,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status, int options)
if (event_ptid != null_ptid)
record_currthread (rs, event_ptid);
else
event_ptid = inferior_ptid;
event_ptid = first_remote_resumed_thread ();
}
else
/* A process exit. Invalidate our notion of current thread. */