Cannot execute this command without a live selected thread.

  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.

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.
---
 gdb/remote.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)
  

On 10/24/2014 11:40 AM, Pedro Alves wrote:
> On 10/24/2014 06:23 PM, Pedro Alves wrote:
>>
>> 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.

The patch works fine for hand-testing of a hello world program (can load 
and run it and stop at breakpoint on main, at least).  I've kicked off a 
full set of tests.

-Sandra
  

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.
 > 
 > >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.
 > ---
 >  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)
 >  {
 >    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.  */

I looked at the current remote_thread_alive.
It has this:

  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;

pid != 0 && lwp == 0 -> main thread?
That sounds odd.
Do you know why the test is the way it is?
  

On 10/24/2014 08:19 PM, Doug Evans wrote:

> I looked at the current remote_thread_alive.
> It has this:
> 
>   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;
> 
> pid != 0 && lwp == 0 -> main thread?
> That sounds odd.
> Do you know why the test is the way it is?

If it's the 0 part you're calling out as odd, it's that way
because we didn't have a thread id back when we created
the thread:

static void
extended_remote_attach_1 (struct target_ops *target, const char *args,
			  int from_tty)
{
  struct remote_state *rs = get_remote_state ();
  int pid;
  char *wait_status = NULL;

  pid = parse_pid_to_attach (args);

...
  set_current_inferior (remote_add_inferior (0, pid, 1));

  inferior_ptid = pid_to_ptid (pid);

...
    {
      /* Now, if we have thread information, update inferior_ptid.  */
      inferior_ptid = remote_current_thread (inferior_ptid);

      /* Add the main thread to the thread list.  */
      add_thread_silent (inferior_ptid);
    }
...


Later on, when we get the first stop event back, we may or may not
find a thread id to use:

static void
remote_notice_new_inferior (ptid_t currthread, int running)
{
...
      if (ptid_is_pid (inferior_ptid)
	  && pid == ptid_get_pid (inferior_ptid))
	{
	  /* inferior_ptid has no thread member yet.  This can happen
	     with the vAttach -> remote_wait,"TAAthread:" path if the
	     stub doesn't support qC.  This is the first stop reported
	     after an attach, so this is the main thread.  Update the
	     ptid in the thread list.  */
	  if (in_thread_list (pid_to_ptid (pid)))
	    thread_change_ptid (inferior_ptid, currthread);
	  else
	    {
	      remote_add_thread (currthread, running);
	      inferior_ptid = currthread;
	    }
	  return;
	}

If we never see any stop reply with a thread id, or the target
doesn't support any thread listing packets, it must be that the
target doesn't really support threads, so we shouldn't ever delete
that thread, for we made it up.  We use "pid != 0 && lwp == 0"
rather than magic_null_ptid as the former carries more info, for
including the PID that the user specified on "attach PID" (and a stop
reply with a thread id may come along), so we can put that PID in
inferior->pid too and display it in "info inferiors", etc., and preserve
the invariant that starting from a ptid we can find the corresponding
inferior, matching by pid.  We shouldn't ask the target whether
that thread is alive, as it's a thread id we made up.

BTW, we do the same in native debugging.  E.g., see inf-ptrace.c:

  inferior_ptid = pid_to_ptid (pid);

  /* Always add a main thread.  If some target extends the ptrace
     target, it should decorate the ptid later with more info.  */
  add_thread_silent (inferior_ptid);

If the inferior is truly non-threaded, and doesn't have any other
threads, it's main/single thread can well end up with a ptid with only
the pid field set; there's no conflict with using (pid,0,0) to refer
to all threads of the process as there'll be only one in that
process anyway.

Thanks,
Pedro Alves
  

Pedro Alves writes:
 > On 10/24/2014 08:19 PM, Doug Evans wrote:
 > 
 > > I looked at the current remote_thread_alive.
 > > It has this:
 > > 
 > >   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;
 > > 
 > > pid != 0 && lwp == 0 -> main thread?
 > > That sounds odd.
 > > Do you know why the test is the way it is?
 > 
 > If it's the 0 part you're calling out as odd, it's that way
 > because we didn't have a thread id back when we created
 > the thread:
 > 
 > static void
 > extended_remote_attach_1 (struct target_ops *target, const char *args,
 > 			  int from_tty)
 > {
 >   struct remote_state *rs = get_remote_state ();
 >   int pid;
 >   char *wait_status = NULL;
 > 
 >   pid = parse_pid_to_attach (args);
 > 
 > ...
 >   set_current_inferior (remote_add_inferior (0, pid, 1));
 > 
 >   inferior_ptid = pid_to_ptid (pid);
 > 
 > ...
 >     {
 >       /* Now, if we have thread information, update inferior_ptid.  */
 >       inferior_ptid = remote_current_thread (inferior_ptid);
 > 
 >       /* Add the main thread to the thread list.  */
 >       add_thread_silent (inferior_ptid);
 >     }
 > ...
 > 
 > 
 > Later on, when we get the first stop event back, we may or may not
 > find a thread id to use:
 > 
 > static void
 > remote_notice_new_inferior (ptid_t currthread, int running)
 > {
 > ...
 >       if (ptid_is_pid (inferior_ptid)
 > 	  && pid == ptid_get_pid (inferior_ptid))
 > 	{
 > 	  /* inferior_ptid has no thread member yet.  This can happen
 > 	     with the vAttach -> remote_wait,"TAAthread:" path if the
 > 	     stub doesn't support qC.  This is the first stop reported
 > 	     after an attach, so this is the main thread.  Update the
 > 	     ptid in the thread list.  */
 > 	  if (in_thread_list (pid_to_ptid (pid)))
 > 	    thread_change_ptid (inferior_ptid, currthread);
 > 	  else
 > 	    {
 > 	      remote_add_thread (currthread, running);
 > 	      inferior_ptid = currthread;
 > 	    }
 > 	  return;
 > 	}
 > 
 > If we never see any stop reply with a thread id, or the target
 > doesn't support any thread listing packets, it must be that the
 > target doesn't really support threads, so we shouldn't ever delete
 > that thread, for we made it up.  We use "pid != 0 && lwp == 0"
 > rather than magic_null_ptid as the former carries more info, for
 > including the PID that the user specified on "attach PID" (and a stop
 > reply with a thread id may come along), so we can put that PID in
 > inferior->pid too and display it in "info inferiors", etc., and preserve
 > the invariant that starting from a ptid we can find the corresponding
 > inferior, matching by pid.  We shouldn't ask the target whether
 > that thread is alive, as it's a thread id we made up.
 > 
 > BTW, we do the same in native debugging.  E.g., see inf-ptrace.c:
 > 
 >   inferior_ptid = pid_to_ptid (pid);
 > 
 >   /* Always add a main thread.  If some target extends the ptrace
 >      target, it should decorate the ptid later with more info.  */
 >   add_thread_silent (inferior_ptid);
 > 
 > If the inferior is truly non-threaded, and doesn't have any other
 > threads, it's main/single thread can well end up with a ptid with only
 > the pid field set; there's no conflict with using (pid,0,0) to refer
 > to all threads of the process as there'll be only one in that
 > process anyway.

Thanks, that's helpful.

Not all targets use ptid.lwp.
Does remote.c not support targets that use tid instead of lwp?
I guess not.
  

On 10/24/2014 09:02 PM, Doug Evans wrote:
> Pedro Alves writes:
>  > On 10/24/2014 08:19 PM, Doug Evans wrote:
>  > 
>  > > I looked at the current remote_thread_alive.
>  > > It has this:
>  > > 
>  > >   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;
>  > > 
>  > > pid != 0 && lwp == 0 -> main thread?
>  > > That sounds odd.
>  > > Do you know why the test is the way it is?
>  > 
>  > If it's the 0 part you're calling out as odd, it's that way
>  > because we didn't have a thread id back when we created
>  > the thread:
>  > 
>  > static void
>  > extended_remote_attach_1 (struct target_ops *target, const char *args,
>  > 			  int from_tty)
>  > {
>  >   struct remote_state *rs = get_remote_state ();
>  >   int pid;
>  >   char *wait_status = NULL;
>  > 
>  >   pid = parse_pid_to_attach (args);
>  > 
>  > ...
>  >   set_current_inferior (remote_add_inferior (0, pid, 1));
>  > 
>  >   inferior_ptid = pid_to_ptid (pid);
>  > 
>  > ...
>  >     {
>  >       /* Now, if we have thread information, update inferior_ptid.  */
>  >       inferior_ptid = remote_current_thread (inferior_ptid);
>  > 
>  >       /* Add the main thread to the thread list.  */
>  >       add_thread_silent (inferior_ptid);
>  >     }
>  > ...
>  > 
>  > 
>  > Later on, when we get the first stop event back, we may or may not
>  > find a thread id to use:
>  > 
>  > static void
>  > remote_notice_new_inferior (ptid_t currthread, int running)
>  > {
>  > ...
>  >       if (ptid_is_pid (inferior_ptid)
>  > 	  && pid == ptid_get_pid (inferior_ptid))
>  > 	{
>  > 	  /* inferior_ptid has no thread member yet.  This can happen
>  > 	     with the vAttach -> remote_wait,"TAAthread:" path if the
>  > 	     stub doesn't support qC.  This is the first stop reported
>  > 	     after an attach, so this is the main thread.  Update the
>  > 	     ptid in the thread list.  */
>  > 	  if (in_thread_list (pid_to_ptid (pid)))
>  > 	    thread_change_ptid (inferior_ptid, currthread);
>  > 	  else
>  > 	    {
>  > 	      remote_add_thread (currthread, running);
>  > 	      inferior_ptid = currthread;
>  > 	    }
>  > 	  return;
>  > 	}
>  > 
>  > If we never see any stop reply with a thread id, or the target
>  > doesn't support any thread listing packets, it must be that the
>  > target doesn't really support threads, so we shouldn't ever delete
>  > that thread, for we made it up.  We use "pid != 0 && lwp == 0"
>  > rather than magic_null_ptid as the former carries more info, for
>  > including the PID that the user specified on "attach PID" (and a stop
>  > reply with a thread id may come along), so we can put that PID in
>  > inferior->pid too and display it in "info inferiors", etc., and preserve
>  > the invariant that starting from a ptid we can find the corresponding
>  > inferior, matching by pid.  We shouldn't ask the target whether
>  > that thread is alive, as it's a thread id we made up.
>  > 
>  > BTW, we do the same in native debugging.  E.g., see inf-ptrace.c:
>  > 
>  >   inferior_ptid = pid_to_ptid (pid);
>  > 
>  >   /* Always add a main thread.  If some target extends the ptrace
>  >      target, it should decorate the ptid later with more info.  */
>  >   add_thread_silent (inferior_ptid);
>  > 
>  > If the inferior is truly non-threaded, and doesn't have any other
>  > threads, it's main/single thread can well end up with a ptid with only
>  > the pid field set; there's no conflict with using (pid,0,0) to refer
>  > to all threads of the process as there'll be only one in that
>  > process anyway.
> 
> Thanks, that's helpful.
> 
> Not all targets use ptid.lwp.

All process_stratum targets do.  remote.c is a process_stratum
target, and I think that last to be converted:

commit ba3481708d3f18e77ab6c000385b131c76d2733e
Author:     Pedro Alves <palves@redhat.com>
AuthorDate: Wed Feb 19 18:25:40 2014 +0000
Commit:     Pedro Alves <palves@redhat.com>
CommitDate: Wed Feb 19 18:25:40 2014 +0000

    remote.c: Use the ptid.lwp field to store remote thread ids rather than ptid.tid.

    From GDB's perspective, independently of how the target really
    implements threads, gdb/remote sees all threads as if kernel/system
    threads.  A rationale along theses lines led to gdbserver storing
    thread ids in ptid.lwp in all ports.

    Because remote.c is currently using ptid.tid, we can't make gdbserver
    and gdb share bits of remote-specific code that manipulates ptids
    (e.g., write_ptid/read_ptid).

    This patch thus makes remote.c use ptid.lwp instead of ptid.tid.

    I believe that on the GDB side too, it's best that we standardize on
    process_stratum targets using the ptid.lwp field to store thread ids
    anyway.  The idea being leave the ptid.tid field free for any
    thread_stratum target that might want to sit on top.

> Does remote.c not support targets that use tid instead of lwp?
> I guess not.

Not sure what you mean by "support" here.

Thanks,
Pedro Alves
  

Pedro Alves writes:
 > > Not all targets use ptid.lwp.
 > 
 > All process_stratum targets do.

windows-nat.c doesn't
(at least I remember seeing all calls to ptid_build there
passing 0 for lwp).
Could be missing something of course.

 >     I believe that on the GDB side too, it's best that we standardize on
 >     process_stratum targets using the ptid.lwp field to store thread ids
 >     anyway.  The idea being leave the ptid.tid field free for any
 >     thread_stratum target that might want to sit on top.

The language in the comment in ptid.h waffles a bit:

   process_stratum targets that handle threading themselves should
   prefer using the ptid.lwp field, leaving the ptid.tid field for any
   thread_stratum target that might want to sit on top.

Can we make this more of a rule than just a "should prefer"?
[and fix targets to follow]
  

[note subject change]

Doug Evans writes:
 > Pedro Alves writes:
 >  > > Not all targets use ptid.lwp.
 >  > 
 >  > All process_stratum targets do.
 > 
 > windows-nat.c doesn't
 > (at least I remember seeing all calls to ptid_build there
 > passing 0 for lwp).
 > Could be missing something of course.
 > 
 >  >     I believe that on the GDB side too, it's best that we standardize on
 >  >     process_stratum targets using the ptid.lwp field to store thread ids
 >  >     anyway.  The idea being leave the ptid.tid field free for any
 >  >     thread_stratum target that might want to sit on top.
 > 
 > The language in the comment in ptid.h waffles a bit:
 > 
 >    process_stratum targets that handle threading themselves should
 >    prefer using the ptid.lwp field, leaving the ptid.tid field for any
 >    thread_stratum target that might want to sit on top.
 > 
 > Can we make this more of a rule than just a "should prefer"?
 > [and fix targets to follow]

Oh, btw, another question I've been wanting to ask ...

One goal we have is to remove libthread_db on linux.
There are two reasons we still have it, pthread_t and thread local vars,
though those can be solved.

Long term, at least in linux-land,
do we still want to keep thread_stratum?
Knowing the answer to this will help save some typing.
[If one did want to remove thread_stratum for linux
there's still a need to support older systems.
But on future newer systems without libthread_db,
what would thread_stratum look like?]
  

On 10/24/2014 09:38 PM, Doug Evans wrote:
> Pedro Alves writes:
>  > > Not all targets use ptid.lwp.
>  > 
>  > All process_stratum targets do.
> 
> windows-nat.c doesn't
> (at least I remember seeing all calls to ptid_build there
> passing 0 for lwp).
> Could be missing something of course.

Eh, I was quite sure I once wrote a patch for that, and
I thought I had pushed it, but I can't find it now.

Ah, back in 2008 I went through all targets migrating
away from storing the thread in ptid_t.pid.

commit 2dc38344f418c9abe65755308ef6db836a61103a
Author:     Pedro Alves <palves@redhat.com>

        Use ptid_t.tid to store thread ids instead of ptid_t.pid.

        * win32-nat.c (win32_add_thread): Change thread argument type to

etc.  That made way for the multi-process support in the core
being able to rely on ptid_t.pid.

It was much later (years) that I realized that ptid.lwpid would
be a better field.

Anyway, should be a trivial change.  Nothing outside the target
should be looking at ptid.lwpid vs ptid.tid.  I fixed gnu-nat.c
and remote.c recently, but I now see that nto-procfs.c, remote-sim.c
and darwin-nat.c still haven't been given the treatment.
I got confused with wishful thinking.  :-P

> 
>  >     I believe that on the GDB side too, it's best that we standardize on
>  >     process_stratum targets using the ptid.lwp field to store thread ids
>  >     anyway.  The idea being leave the ptid.tid field free for any
>  >     thread_stratum target that might want to sit on top.
> 
> The language in the comment in ptid.h waffles a bit:
> 
>    process_stratum targets that handle threading themselves should
>    prefer using the ptid.lwp field, leaving the ptid.tid field for any
>    thread_stratum target that might want to sit on top.
> 
> Can we make this more of a rule than just a "should prefer"?
> [and fix targets to follow]

Sure, guess I didn't use a stronger word at the time because
I hadn't yet forgotten that the all-targets conversion wasn't
finished.  :-)

Thanks,
Pedro Alves
  

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.

-Sandra