diff options
author | Pedro Alves <palves@redhat.com> | 2020-01-10 20:05:52 +0000 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2020-01-10 20:05:52 +0000 |
commit | 31ba933ec6a14b1f7fdbd9ba51f759e2c0537b9d (patch) | |
tree | f808b25a9a7627df1acc414e32e9ceaf37fc9f67 /gdb | |
parent | 735fc2ca685b55bf1debbfcea6d2ab544e58a530 (diff) | |
download | fsf-binutils-gdb-31ba933ec6a14b1f7fdbd9ba51f759e2c0537b9d.zip fsf-binutils-gdb-31ba933ec6a14b1f7fdbd9ba51f759e2c0537b9d.tar.gz fsf-binutils-gdb-31ba933ec6a14b1f7fdbd9ba51f759e2c0537b9d.tar.bz2 |
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.
Diffstat (limited to 'gdb')
-rw-r--r-- | gdb/ChangeLog | 6 | ||||
-rw-r--r-- | gdb/remote.c | 13 |
2 files changed, 18 insertions, 1 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index fb7b93f..9d63feb 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,11 @@ 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. 2020-01-10 Pedro Alves <palves@redhat.com> diff --git a/gdb/remote.c b/gdb/remote.c index 51fa5da..fa940df 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -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. */ |