This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [pushed] Workaround remote targets that report an empty list to qfThreadInfo (Re: Cannot execute this command without a live selected thread.)
- From: Doug Evans <dje at google dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Sandra Loosemore <sandra at codesourcery dot com>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Wed, 29 Oct 2014 12:15:57 -0700
- Subject: Re: [pushed] Workaround remote targets that report an empty list to qfThreadInfo (Re: Cannot execute this command without a live selected thread.)
- Authentication-results: sourceware.org; auth=none
- References: <544A7648 dot 6060102 at codesourcery dot com> <544A7930 dot 4040909 at redhat dot com> <544A8741 dot 9090705 at codesourcery dot com> <544A8B0C dot 5000509 at redhat dot com> <544A8F15 dot 9000906 at redhat dot com> <544EA2C9 dot 8090902 at codesourcery dot com> <544F8792 dot 5050800 at redhat dot com>
On Tue, Oct 28, 2014 at 5:09 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/27/2014 07:53 PM, Sandra Loosemore wrote:
>> On 10/24/2014 11:40 AM, Pedro Alves wrote:
>>>
>>> Please give this a try.
>>>
>>> From 2062235a91a3c69e73c39b0f8a4f78f4ec396931 Mon Sep 17 00:00:00 2001
>>> From: Pedro Alves <palves@redhat.com>
>>> Date: Fri, 24 Oct 2014 18:27:14 +0100
>>> Subject: [PATCH] gdb/ 2014-10-24 Pedro Alves <palves@redhat.com>
>>>
>>> * remote.c (remote_thread_alive): New, factored out from ...
>>> (remote_thread_alive): ... this.
>>> (remote_update_thread_list): Bail out before deleting threads if
>>> the target returned an empty list, and, the current thread has a
>>> magic/fake ptid.
>>
>> This worked for me. The remaining ERRORs in the nios2-elf test results
>> are due to the GCC switch to C11.
>
> Thanks Sandra, now pushed.
Hi.
I was going to send the email appended at the bottom out,
but since the Subject line didn't specify a patch intended to be committed,
"[PATCH] mumble ..." or some such,
I didn't want to violate protocol.
>
> From 7d1a114c44db3d7055afe48868f939ba95a64b7b Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 28 Oct 2014 11:35:10 +0000
> Subject: [PATCH] Workaround remote targets that report an empty list to
> qfThreadInfo
>
> In https://sourceware.org/ml/gdb-patches/2014-10/msg00652.html, Sandra
> shows a target that was broken by the recent update_thread_list
> optimization:
>
> (gdb) target remote qa8-centos32-cs:10514
> ...
> (gdb) continue
> Continuing.
> Cannot execute this command without a live selected thread.
> (gdb)
>
> The error means that the current thread is in "exited" state when the
> continue command is processed. The root of the problem was found
> here:
>
> > Sending packet: $Hg0#df...Packet received:
> ...
> > Sending packet: $?#3f...Packet received: S00
> > Sending packet: $qfThreadInfo#bb...Packet received: l
> > Sending packet: $Hc-1#09...Packet received:
> > Sending packet: $qC#b4...Packet received: unset
>
> This target doesn't really support threads (no thread indication in
> stop reply packets; no support for qC), but then supports
> qfThreadInfo, and returns an empty thread list to GDB.
>
> See https://sourceware.org/ml/gdb-patches/2014-10/msg00665.html for
> why the target does that.
>
> As remote_update_thread_list deletes threads from GDB's list that are
> not found in the thread list that the target reports, the result is
> that GDB deletes the "fake" main thread that GDB added itself. (As
> that thread is currently selected, it is marked "exited" instead of
> being deleted straight away.)
>
> This commit avoids deleting the main thread in this scenario.
>
> gdb/
> 2014-10-27 Pedro Alves <palves@redhat.com>
>
> * remote.c (remote_thread_alive): New, factored out from ...
> (remote_thread_alive): ... this.
> (remote_update_thread_list): Bail out before deleting threads if
> the target returned an empty list, and, the current thread has a
> magic/fake ptid.
> ---
> gdb/ChangeLog | 8 ++++++++
> gdb/remote.c | 35 ++++++++++++++++++++++++++++++++---
> 2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 8a34118..b358dd7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,13 @@
> 2014-10-27 Pedro Alves <palves@redhat.com>
>
> + * remote.c (remote_thread_alive): New, factored out from ...
> + (remote_thread_alive): ... this.
> + (remote_update_thread_list): Bail out before deleting threads if
> + the target returned an empty list, and, the current thread has a
> + magic/fake ptid.
> +
> +2014-10-27 Pedro Alves <palves@redhat.com>
> +
> * infrun.c (handle_signal_stop): Also skip handlers when a random
> signal arrives while handling a "stepi" or a "nexti". Set the
> thread's 'step_after_step_resume_breakpoint' flag.
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 20f2988..4b9b099 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1842,11 +1842,11 @@ set_general_process (void)
> }
>
>
> -/* Return nonzero if the thread PTID is still alive on the remote
> - system. */
> +/* Return nonzero if this is the main thread that we made up ourselves
> + to model non-threaded targets as single-threaded. */
>
> static int
> -remote_thread_alive (struct target_ops *ops, ptid_t ptid)
> +remote_thread_always_alive (struct target_ops *ops, ptid_t ptid)
> {
> struct remote_state *rs = get_remote_state ();
> char *p, *endp;
> @@ -1861,6 +1861,23 @@ remote_thread_alive (struct target_ops *ops, ptid_t ptid)
> multi-threading. */
> return 1;
>
> + return 0;
> +}
> +
> +/* Return nonzero if the thread PTID is still alive on the remote
> + system. */
> +
> +static int
> +remote_thread_alive (struct target_ops *ops, ptid_t ptid)
> +{
> + struct remote_state *rs = get_remote_state ();
> + char *p, *endp;
> +
> + /* Check if this is a thread that we made up ourselves to model
> + non-threaded targets as single-threaded. */
> + if (remote_thread_always_alive (ops, ptid))
> + return 1;
> +
> p = rs->buf;
> endp = rs->buf + get_remote_packet_size ();
>
> @@ -2780,6 +2797,18 @@ remote_update_thread_list (struct target_ops *ops)
>
> got_list = 1;
>
> + if (VEC_empty (thread_item_t, context.items)
> + && remote_thread_always_alive (ops, inferior_ptid))
> + {
> + /* Some targets don't really support threads, but still
> + reply an (empty) thread list in response to the thread
> + listing packets, instead of replying "packet not
> + supported". Exit early so we don't delete the main
> + thread. */
> + do_cleanups (old_chain);
> + return;
> + }
> +
> /* CONTEXT now holds the current thread list on the remote
> target end. Delete GDB-side threads no longer found on the
> target. */
> --
> 1.9.3
>
>
---snip---
Pedro Alves writes:
> On 10/24/2014 06:23 PM, Pedro Alves wrote:
> > On 10/24/2014 06:07 PM, Sandra Loosemore wrote:
> >> Sending packet: $?#3f...Packet received: S00
> >> Sending packet: $qfThreadInfo#bb...Packet received: l
> >
> > Huh, I think this is the problem.
> >
> > So this target supports qfThreadInfo, but then it's returning
> > an empty thread list... remote_update_thread_list will delete
> > threads from GDB's list that are not found in the thread list that
> > the target reported. Why is the target reporting that empty list?
> >
> > See ab970af19746364a4f236bebc238ebb036adc898.
> >
> > We may be able add a workaround for this. :-/ Just in case,
> > can you confirm that the current thread's ptid (inferior_ptid)
> > is magic_null_ptid?
>
> Please give this a try.
Hi.
I don't have an opinion on whether to patch gdb, the stub, or both,
but if this patch does indeed address the issue and goes in
a few comments. :-)
I realize this was just a suggestion for Sandra to try,
but if it works it's not clear there'll be a follow-on formal patch.
> >From 2062235a91a3c69e73c39b0f8a4f78f4ec396931 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 24 Oct 2014 18:27:14 +0100
> Subject: [PATCH] gdb/ 2014-10-24 Pedro Alves <palves@redhat.com>
>
> * remote.c (remote_thread_alive): New, factored out from ...
Typo: remote_thread_always_alive.
> (remote_thread_alive): ... this.
> (remote_update_thread_list): Bail out before deleting threads if
> the target returned an empty list, and, the current thread has a
> magic/fake ptid.
> ---
> gdb/remote.c | 35 ++++++++++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 20f2988..affc7c2 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1842,11 +1842,11 @@ set_general_process (void)
> }
>
>
> -/* Return nonzero if the thread PTID is still alive on the remote
> - system. */
> +/* Return nonzero if this is the main thread that we made up ourselves
> + to model non-threaded targets as single-threaded. */
>
> static int
> -remote_thread_alive (struct target_ops *ops, ptid_t ptid)
> +remote_thread_always_alive (struct target_ops *ops, ptid_t ptid)
> {
The function comment and function name leave me the reader confused.
IOW: Which is it? Is it
(a) Return nonzero if this is the main thread that we made up ourselves,
or
(b) Return nonzero if the thread is always alive.
To the reader it's not clear the two are synonymous
and will remain so. Call sites also conflate the two
concepts.
Any rewording of the comment to clear things up would suffice.
E.g.,
/* Return nonzero if thread PTID is always alive.
E.g., If this is the main thread that we made up ourselves
to model non-threaded targets as single-threaded. */
[or some such]
Plus the function also has this test:
if (ptid_get_pid (ptid) != 0 && ptid_get_lwp (ptid) == 0)
/* The main thread is always alive. This can happen after a
vAttach, if the remote side doesn't support
multi-threading. */
return 1;
which isn't a made-up thread.
Another possibility is changing the name to something like
remote_ptid_is_main_thread or some such.
I guess I prefer the former though.
---snip---