[RFA] Report the main thread.

[RFA] Report the main thread.

[Vladimir Prus]

Presently, on Linux, we do add the main thread to the thread list, but we
fail to report this new thread to outside world.  I think we should not
do any such special-casing -- if we want the =thread-created notification
to be usable as replacement for getting the thread, we better emit that
for all new thread.

This patch changes the behaviour. OK?

- Volodya

	* linux-nat.c (linux_nat_wait): Use add_thread, not
	add_thread_silent, when adding main thread.
---
 gdb/linux-nat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index cac4bb7..9471cd8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2453,7 +2453,7 @@ linux_nat_wait (ptid_t ptid, struct target_waitstatus *ourstatus)
       lp = add_lwp (inferior_ptid);
       lp->resumed = 1;
       /* Add the main thread to GDB's thread list.  */
-      add_thread_silent (lp->ptid);
+      add_thread (lp->ptid);
       set_running (lp->ptid, 1);
     }
 
-- 
1.5.3.5

Re: [RFA] Report the main thread.

[Mark Kettenis]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 26 Apr 2008 20:05:17 +0400
> 
> Presently, on Linux, we do add the main thread to the thread list, but we
> fail to report this new thread to outside world.  I think we should not
> do any such special-casing -- if we want the =thread-created notification
> to be usable as replacement for getting the thread, we better emit that
> for all new thread.

I disagree.  The "main" thread is not a new thread, but has been there
all along.  Users would be terribly confused you'd report some sort of
thread creation event here.

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Sat, Apr 26, 2008 at 06:17:09PM +0200, Mark Kettenis wrote:
> > Presently, on Linux, we do add the main thread to the thread list, but we
> > fail to report this new thread to outside world.  I think we should not
> > do any such special-casing -- if we want the =thread-created notification
> > to be usable as replacement for getting the thread, we better emit that
> > for all new thread.
> 
> I disagree.  The "main" thread is not a new thread, but has been there
> all along.  Users would be terribly confused you'd report some sort of
> thread creation event here.

Me too.  We used to report this thread - since we did not detect it
until later, when libthread_db initialized - and I deliberately left
it silent when I fixed that.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Vladimir Prus]

Mark Kettenis wrote:

>> From: Vladimir Prus <vladimir@codesourcery.com>
>> Date: Sat, 26 Apr 2008 20:05:17 +0400
>> 
>> Presently, on Linux, we do add the main thread to the thread list, but we
>> fail to report this new thread to outside world.  I think we should not
>> do any such special-casing -- if we want the =thread-created notification
>> to be usable as replacement for getting the thread, we better emit that
>> for all new thread.
> 
> I disagree.  The "main" thread is not a new thread, but has been there
> all along.  Users would be terribly confused you'd report some sort of
> thread creation event here.

Will some approach that only produce MI output be fine with you?

- Volodya

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
> Will some approach that only produce MI output be fine with you?

Perhaps that means any observer for a new MI thread should go in
add_thread_silent?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
>> Will some approach that only produce MI output be fine with you?
> 
> Perhaps that means any observer for a new MI thread should go in
> add_thread_silent?

Yes, moving the observer call to add_thread_silent is the most
direct approach to make MI work the way I want without disturbing CLI.

- Volodya

Re: [RFA] Report the main thread.

[Nick Roberts]

 > >> Will some approach that only produce MI output be fine with you?
 > > 
 > > Perhaps that means any observer for a new MI thread should go in
 > > add_thread_silent?
 > 
 > Yes, moving the observer call to add_thread_silent is the most
 > direct approach to make MI work the way I want without disturbing CLI.

I still don't see the point in reporting the main thread.  If there's more
than one thread the user can switch between them but what course of action
is available with just one thread?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Vladimir Prus]

Nick Roberts wrote:

>  > >> Will some approach that only produce MI output be fine with you?
>  > > 
>  > > Perhaps that means any observer for a new MI thread should go in
>  > > add_thread_silent?
>  > 
>  > Yes, moving the observer call to add_thread_silent is the most
>  > direct approach to make MI work the way I want without disturbing CLI.
> 
> I still don't see the point in reporting the main thread.  If there's more
> than one thread the user can switch between them but what course of action
> is available with just one thread?

What is the purpose of =thread-created? Presumably, this is to allow
the frontend to keep up-to-date list of threads without doing -thread-info.
For that to be possible, it's necessary that for every single thread in
the output of -thread-info, there's also =thread-created.

Of course, a frontend might decide to use -thread-info most of the time,
and use =thread-create to update the list only when the target is running,
in which case reporting the main thread might be less critical. But still,
it seems more consistent to report main thread, as long as -thread-info
includes it.

- Volodya

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Sun, Apr 27, 2008 at 10:23:03AM +1200, Nick Roberts wrote:
> I still don't see the point in reporting the main thread.  If there's more
> than one thread the user can switch between them but what course of action
> is available with just one thread?

The GUI might have a thread list, for instance.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > I still don't see the point in reporting the main thread.  If there's more
 > > than one thread the user can switch between them but what course of action
 > > is available with just one thread?
 > 
 > The GUI might have a thread list, for instance.

But my question was "what could a user do with it?".  AFAICS a thread list with
just one thread in it just looks like the output of the "frame" command.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > I still don't see the point in reporting the main thread.  If there's more
 > > than one thread the user can switch between them but what course of action
 > > is available with just one thread?
 > 
 > The GUI might have a thread list, for instance.

But my question was "what could a user do with it?".  AFAICS a thread list with
just one thread in it just looks like the output of the "frame" command.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Sun, Apr 27, 2008 at 10:56:33AM +1200, Nick Roberts wrote:
>  > > I still don't see the point in reporting the main thread.  If there's more
>  > > than one thread the user can switch between them but what course of action
>  > > is available with just one thread?
>  > 
>  > The GUI might have a thread list, for instance.
> 
> But my question was "what could a user do with it?".  AFAICS a thread list with
> just one thread in it just looks like the output of the "frame" command.

Well, for example, Eclipse's stack panel is organized as a multi-level
list.  It's got expanding items for each debug session, thread, and
stack frame.  So the main thread's backtrace is inside of an item
containing the thread ID.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Mark Kettenis]

> Date: Sun, 27 Apr 2008 09:52:46 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Sun, Apr 27, 2008 at 10:56:33AM +1200, Nick Roberts wrote:
> >  > > I still don't see the point in reporting the main thread.  If there's more
> >  > > than one thread the user can switch between them but what course of action
> >  > > is available with just one thread?
> >  > 
> >  > The GUI might have a thread list, for instance.
> > 
> > But my question was "what could a user do with it?".  AFAICS a thread list with
> > just one thread in it just looks like the output of the "frame" command.
> 
> Well, for example, Eclipse's stack panel is organized as a multi-level
> list.  It's got expanding items for each debug session, thread, and
> stack frame.  So the main thread's backtrace is inside of an item
> containing the thread ID.

So Eclipse assumes that all programs are threaded?  That's so ... Java.

Seriously though, in GDB we've always only populated the threads list
if a program actually has threads.  An MI client will have to deal
with that fact.  If it insists on providing a threaded view of the
world, it needs to fake up a main thread.  Since it already has to do
that for non-threaded programs, why would having a false thread create
event for the main thread help?

Re: [RFA] Report the main thread.

[Pedro Alves]

A Sunday 27 April 2008 15:22:59, Mark Kettenis wrote:

> Seriously though, in GDB we've always only populated the threads list
> if a program actually has threads.  

Would that be true if threads had been implemented in GDB from the
ground up, instead of being an afterthought?

> An MI client will have to deal 
> with that fact.  If it insists on providing a threaded view of the
> world, it needs to fake up a main thread.  Since it already has to do
> that for non-threaded programs, why would having a false thread create
> event for the main thread help?

If a command is defined to take --thread=$gdb_thread_id as a parameter,
it is awkward for the frontend to have to do something different
on single-threaded apps.  E.g., no --thread parameter means all
threads, or the main thread in ST apps?

A GDB thread id is also what can be made constant even though
the ptid of the main thread may change due to loading
thread support.

(My secret xmas wish was for GDB to consider threads a first
class citizen, and get rid of the infrun.c context switching.)

-- 
Pedro Alves

Re: [RFA] Report the main thread.

[Mark Kettenis]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sun, 27 Apr 2008 16:03:27 +0100
> 
> A Sunday 27 April 2008 15:22:59, Mark Kettenis wrote:
> 
> > Seriously though, in GDB we've always only populated the threads list
> > if a program actually has threads.  
> 
> Would that be true if threads had been implemented in GDB from the
> ground up, instead of being an afterthought?

Hard to tell.  But things like the application thread ID are simply
not defined for programs that are not linked with a threads library.

Not showing anything in response to the "info threads" command for
non-threaded applications has been quite a conscious decision.  We
don't want to confront users with the complication of threads if they
don't need it.

We do populate the threads lists for multi-threaded applications that
have only one thread running.

> > An MI client will have to deal 
> > with that fact.  If it insists on providing a threaded view of the
> > world, it needs to fake up a main thread.  Since it already has to do
> > that for non-threaded programs, why would having a false thread create
> > event for the main thread help?
> 
> If a command is defined to take --thread=$gdb_thread_id as a parameter,
> it is awkward for the frontend to have to do something different
> on single-threaded apps.  E.g., no --thread parameter means all
> threads, or the main thread in ST apps?

Well, there is no main thread in a non-threaded application; it isn't
threaded, you can't ask the program for some sort of thread ID.

Specifying a --thread parameter for such applications doesn't make
sense, because issuing such a command means that the front-end (and
probably the user) thinks the application is multi-threaded when it
really isn't.  I would consider it completely reasonable for gdb to
warn about this.

> A GDB thread id is also what can be made constant even though
> the ptid of the main thread may change due to loading
> thread support.

Well, the GDB thread IDs depend on the order in which the threads are
created.  I suppose GDB will always see the main thread of a threaded
applicatication first if you start it from GDB, but what if you attach
GDB to an existing process?  The main thread at that point might not
even exist anymore.

Re: [RFA] Report the main thread.

[Pedro Alves]

A Sunday 27 April 2008 16:47:42, Mark Kettenis wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Sun, 27 Apr 2008 16:03:27 +0100
> >
> > A Sunday 27 April 2008 15:22:59, Mark Kettenis wrote:
> > > Seriously though, in GDB we've always only populated the threads list
> > > if a program actually has threads.
> >
> > Would that be true if threads had been implemented in GDB from the
> > ground up, instead of being an afterthought?
>
> Hard to tell.  But things like the application thread ID are simply
> not defined for programs that are not linked with a threads library.
>

True, but we're talking about a GDB thread id, which is made by GDB, not
by the thread library.  It is on the same arbitrary level as the
thread library coming up with a thread id for the "main thread" in
a MT app.

> Not showing anything in response to the "info threads" command for
> non-threaded applications has been quite a conscious decision.  We
> don't want to confront users with the complication of threads if they
> don't need it.
>

I don't know what complication is this.

> We do populate the threads lists for multi-threaded applications that
> have only one thread running.
>

Exactly, and that is confusing, and unconsistent.  The main thread is
not pthread_create'd (or equivalent), yet it is listed as a thread.

> > > An MI client will have to deal
> > > with that fact.  If it insists on providing a threaded view of the
> > > world, it needs to fake up a main thread.  Since it already has to do
> > > that for non-threaded programs, why would having a false thread create
> > > event for the main thread help?
> >
> > If a command is defined to take --thread=$gdb_thread_id as a parameter,
> > it is awkward for the frontend to have to do something different
> > on single-threaded apps.  E.g., no --thread parameter means all
> > threads, or the main thread in ST apps?
>
> Well, there is no main thread in a non-threaded application; it isn't
> threaded, you can't ask the program for some sort of thread ID.
>

I can ask GDB because GDB is providing an abstraction.

> Specifying a --thread parameter for such applications doesn't make
> sense, because issuing such a command means that the front-end (and
> probably the user) thinks the application is multi-threaded when it
> really isn't.  I would consider it completely reasonable for gdb to
> warn about this.
>
> > A GDB thread id is also what can be made constant even though
> > the ptid of the main thread may change due to loading
> > thread support.
>

> Well, the GDB thread IDs depend on the order in which the threads are
> created.  

That is new to me, and seems to be depending on a thread library
implementation detail.  What if the thread library doesn't report
the main thread first when GDB queries the thread list?

> I suppose GDB will always see the main thread of a threaded 
> applicatication first if you start it from GDB, but what if you attach
> GDB to an existing process?  The main thread at that point might not
> even exist anymore.

When we attach to a ST app, we'll add the thread that
had the stop event to the thread list -- that will be always the
"main thread" by definition.  If we're attaching to a MT app, the
thread that got the event will still exist, of course, otherwise
it wouldn't report the event, so we can add it.  Then we query the
thread library for the rest of the threads -- we never add a ptid
to the thread list if it is already listed.  If the main thread is
gone, then it  won't be the thread that reports the SIGSTOP, and
it won't the returned by the thread library.

-- 
Pedro Alves

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Sun, Apr 27, 2008 at 04:22:59PM +0200, Mark Kettenis wrote:
> Seriously though, in GDB we've always only populated the threads list
> if a program actually has threads.  An MI client will have to deal
> with that fact.  If it insists on providing a threaded view of the
> world, it needs to fake up a main thread.  Since it already has to do
> that for non-threaded programs, why would having a false thread create
> event for the main thread help?

Personally I think that's a mistake.  And if we don't want to change
the remaining backends - some of them already populated the first
thread, IIRC, and Linux now does - then maybe we should change MI
to fake up the thread.  It seems like a silly thing to make each
client do individually.

The "always a frame" work has made parts of GDB much clearer; I think
"always a thread" would too.  The big win would be exactly what Pedro
mentioned - it would simplify execution control.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Vladimir Prus]

Vladimir Prus wrote:

> Daniel Jacobowitz wrote:
> 
>> On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
>>> Will some approach that only produce MI output be fine with you?
>> 
>> Perhaps that means any observer for a new MI thread should go in
>> add_thread_silent?
> 
> Yes, moving the observer call to add_thread_silent is the most
> direct approach to make MI work the way I want without disturbing CLI.

Here's a patch to that effect. OK?

        * thread.c (add_thread): Move observer call to ...
        (add_thread_silent): ... here.
---
 gdb/thread.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index 46e6ba7..d3cfb32 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -124,6 +124,9 @@ add_thread_silent (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+
+  observer_notify_new_thread (tp);
+
   return tp;
 }
 
@@ -136,8 +139,6 @@ add_thread_with_info (ptid_t ptid, struct private_thread_info *private)
 
   if (print_thread_events)
     printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
-
-  observer_notify_new_thread (result);
   
   return result;
 }

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Wed, Apr 30, 2008 at 11:54:01AM +0400, Vladimir Prus wrote:
> Here's a patch to that effect. OK?
> 
>         * thread.c (add_thread): Move observer call to ...
>         (add_thread_silent): ... here.

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Nick Roberts]

> Vladimir Prus wrote:

>> Daniel Jacobowitz wrote:
>> 
>>> On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
>>>> Will some approach that only produce MI output be fine with you?
>>> 
>>> Perhaps that means any observer for a new MI thread should go in
>>> add_thread_silent?
>> 
>> Yes, moving the observer call to add_thread_silent is the most
>> direct approach to make MI work the way I want without disturbing CLI.

> Here's a patch to that effect. OK?

>        * thread.c (add_thread): Move observer call to ...
>        (add_thread_silent): ... here.
> ---
> gdb/thread.c |    5 +++--
> > 1 files changed, 3 insertions(+), 2 deletions(-)

> diff --git a/gdb/thread.c b/gdb/thread.c
> index 46e6ba7..d3cfb32 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -124,6 +124,9 @@ add_thread_silent (ptid_t ptid)
>    tp->num = ++highest_thread_num;
>    tp->next = thread_list;
>    thread_list = tp;
> +
> +  observer_notify_new_thread (tp);
> +
>    return tp;
>  }


Now that Joel has requested that I use observer_attach_new_thread for
annotations this approach doesn't only produce MI output so it needs to
be conditioned on MI:

@@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+
+  if (ui_out_is_mi_like_p (uiout))
+    observer_notify_new_thread (tp);
+
   return tp;
 }

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Vladimir Prus]

Nick Roberts wrote:

> 
>> Vladimir Prus wrote:
> 
>>> Daniel Jacobowitz wrote:
>>> 
>>>> On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
>>>>> Will some approach that only produce MI output be fine with you?
>>>> 
>>>> Perhaps that means any observer for a new MI thread should go in
>>>> add_thread_silent?
>>> 
>>> Yes, moving the observer call to add_thread_silent is the most
>>> direct approach to make MI work the way I want without disturbing CLI.
> 
>> Here's a patch to that effect. OK?
> 
>>        * thread.c (add_thread): Move observer call to ...
>>        (add_thread_silent): ... here.
>> ---
>> gdb/thread.c |    5 +++--
>> > 1 files changed, 3 insertions(+), 2 deletions(-)
> 
>> diff --git a/gdb/thread.c b/gdb/thread.c
>> index 46e6ba7..d3cfb32 100644
>> --- a/gdb/thread.c
>> +++ b/gdb/thread.c
>> @@ -124,6 +124,9 @@ add_thread_silent (ptid_t ptid)
>>    tp->num = ++highest_thread_num;
>>    tp->next = thread_list;
>>    thread_list = tp;
>> +
>> +  observer_notify_new_thread (tp);
>> +
>>    return tp;
>>  }
> 
> 
> Now that Joel has requested that I use observer_attach_new_thread for
> annotations this approach doesn't only produce MI output so it needs to
> be conditioned on MI:
> 
> @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
>    tp->num = ++highest_thread_num;
>    tp->next = thread_list;
>    thread_list = tp;
> +
> +  if (ui_out_is_mi_like_p (uiout))
> +    observer_notify_new_thread (tp);

This patch, IIUC, will cause the new_thread observer to be notified only
in MI mode. Then, no observer except for MI one will ever be called?
Then, why those non-MI observers exist?

Or to put in other way, I think this patch will make you annotation
observers to be never called. Is this what you want?

- Volodya

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > Now that Joel has requested that I use observer_attach_new_thread for
 > > annotations this approach doesn't only produce MI output so it needs to
 > > be conditioned on MI:
 > > 
 > > @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
 > >    tp->num = ++highest_thread_num;
 > >    tp->next = thread_list;
 > >    thread_list = tp;
 > > +
 > > +  if (ui_out_is_mi_like_p (uiout))
 > > +    observer_notify_new_thread (tp);
 > 
 > This patch, IIUC, will cause the new_thread observer to be notified only
 > in MI mode. Then, no observer except for MI one will ever be called?
 > Then, why those non-MI observers exist?
 > 
 > Or to put in other way, I think this patch will make you annotation
 > observers to be never called. Is this what you want?

Yes, you're right.  I didn't notice that the observer in add_thread_with_info
was removed at the same time this one was added.

I just want threads other than the main thread to be reported, as used to
be the case.  That way I can create a threads buffer and display the thread
number when there is more than one thread.  Putting observer_notify_new_thread
back in add_thread_with_info like this would work:

  if (!ui_out_is_mi_like_p (uiout))
    observer_notify_new_thread (tp);

but it might be cleaner if I applied my original patch.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Vladimir Prus]

Nick Roberts wrote:

>  > > Now that Joel has requested that I use observer_attach_new_thread for
>  > > annotations this approach doesn't only produce MI output so it needs to
>  > > be conditioned on MI:
>  > > 
>  > > @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
>  > >    tp->num = ++highest_thread_num;
>  > >    tp->next = thread_list;
>  > >    thread_list = tp;
>  > > +
>  > > +  if (ui_out_is_mi_like_p (uiout))
>  > > +    observer_notify_new_thread (tp);
>  > 
>  > This patch, IIUC, will cause the new_thread observer to be notified only
>  > in MI mode. Then, no observer except for MI one will ever be called?
>  > Then, why those non-MI observers exist?
>  > 
>  > Or to put in other way, I think this patch will make you annotation
>  > observers to be never called. Is this what you want?
> 
> Yes, you're right.  I didn't notice that the observer in add_thread_with_info
> was removed at the same time this one was added.
> 
> I just want threads other than the main thread to be reported, as used to
> be the case.  

Assuming that makes sense for annotations, I think your observer is in position
to decide which threads to report, and which not?

> That way I can create a threads buffer and display the thread 
> number when there is more than one thread.  

Cannot you just suppress display of thread list if there's just one
thread reported, on UI side?

- Volodya

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > Yes, you're right.  I didn't notice that the observer in
 > > add_thread_with_info was removed at the same time this one was added.
 > > 
 > > I just want threads other than the main thread to be reported, as used to
 > > be the case.  
 > 
 > Assuming that makes sense for annotations, I think your observer is in
 > position to decide which threads to report, and which not?

Currently the new_thread observer only declares struct thread_info and not it's
members so I can't identify the thread number in annotate.c

 > > That way I can create a threads buffer and display the thread 
 > > number when there is more than one thread.  
 > 
 > Cannot you just suppress display of thread list if there's just one
 > thread reported, on UI side?

Yes, I could count the new-thread annotations and and only trigger things when
there is more than one thread.  It's simpler if I don't have to and I'm just
baffled why I need to go through these hoops for a small stop gap change that
no-one else is interested in using.  The only requirement I can see on such a
patch is that it doesn't affect anybody else's use of Gdb, and I think my
original patch filled that requirement.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Vladimir Prus]

On Sunday 11 May 2008 18:09:30 Nick Roberts wrote:
>  > > Yes, you're right.  I didn't notice that the observer in
>  > > add_thread_with_info was removed at the same time this one was added.
>  > > 
>  > > I just want threads other than the main thread to be reported, as used to
>  > > be the case.  
>  > 
>  > Assuming that makes sense for annotations, I think your observer is in
>  > position to decide which threads to report, and which not?
> 
> Currently the new_thread observer only declares struct thread_info

What do you mean? The observer does not declare struct thread_info, it declares
a parameter having type 'struct thread_info'. 

> and not it's 
> members so I can't identify the thread number in annotate.c

struct thread_info is defined in gdbthread.h. What prevents you from including
that header?

>  > > That way I can create a threads buffer and display the thread 
>  > > number when there is more than one thread.  
>  > 
>  > Cannot you just suppress display of thread list if there's just one
>  > thread reported, on UI side?
> 
> Yes, I could count the new-thread annotations and and only trigger things when
> there is more than one thread.  It's simpler if I don't have to and I'm just
> baffled why I need to go through these hoops for a small stop gap change that
> no-one else is interested in using.  The only requirement I can see on such a
> patch is that it doesn't affect anybody else's use of Gdb, and I think my
> original patch filled that requirement.

I have no opinion on this point (and I was not involved in the discussion of 
your original patch).

- Volodya

 

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > Currently the new_thread observer only declares struct thread_info
 > 
 > What do you mean? The observer does not declare struct thread_info, it
 > declares a parameter having type 'struct thread_info'.
 
struct thread_info is declared in observer.h (through observer.sh) which
is included in annotate.c

 > > and not it's 
 > > members so I can't identify the thread number in annotate.c
 > 
 > struct thread_info is defined in gdbthread.h. What prevents you from
 > including that header?

I think the idea is not to expose the internals to other parts of the code.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Vladimir Prus]

On Sunday 11 May 2008 18:40:11 Nick Roberts wrote:
>  > > Currently the new_thread observer only declares struct thread_info
>  > 
>  > What do you mean? The observer does not declare struct thread_info, it
>  > declares a parameter having type 'struct thread_info'.
>  
> struct thread_info is declared in observer.h (through observer.sh) which
> is included in annotate.c

Well, that's just forward/incomplete declaration, which presumably is used
(as opposed to including all the necessary headers), just to reduce compile
time. Modules that register observers need to #include necessary headers
to use the parameters.

>  > > and not it's 
>  > > members so I can't identify the thread number in annotate.c
>  > 
>  > struct thread_info is defined in gdbthread.h. What prevents you from
>  > including that header?
> 
> I think the idea is not to expose the internals to other parts of the code.

There's nothing in gdbthread.h to indicate that, I think.

- Volodya

Re: [RFA] Report the main thread.

[Nick Roberts]

 > >  > struct thread_info is defined in gdbthread.h. What prevents you from
 > >  > including that header?
 > > 
 > > I think the idea is not to expose the internals to other parts of the code.
 > 
 > There's nothing in gdbthread.h to indicate that, I think.

It's not specific to gdbthread.h but a general principle.  Look at varobj.h.
It just declares struct varobj (without it's members) which means that all
the setters and getters (varobj_set/get_*) need to be defined in varobj.c
for mi-cmd-var.c to access the members.  I guess it's a poor man's data
encapsulation.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Michael Snyder]

On Mon, 2008-05-12 at 02:09 +1200, Nick Roberts wrote:

> Yes, I could count the new-thread annotations and and only trigger things when
> there is more than one thread.  It's simpler if I don't have to and I'm just
> baffled why I need to go through these hoops for a small stop gap change that
> no-one else is interested in using.  The only requirement I can see on such a
> patch is that it doesn't affect anybody else's use of Gdb, and I think my
> original patch filled that requirement.

We used to have to do the same thing for CLI.
On Linux and Solaris (at least), as soon as you link
with the thread library, you would always get a main
thread (even if you hadn't created any subsidiary threads).
So we would suppress the "info threads" display unless
there was more than one thread.

Or so I seem to recall...

Re: [RFA] Report the main thread.

[Joel Brobecker]

> > I just want threads other than the main thread to be reported, as
> > used to be the case.  
[...]
> > That way I can create a threads buffer and display the thread 
> > number when there is more than one thread.  
> 
> Cannot you just suppress display of thread list if there's just one
> thread reported, on UI side?

I think it is important to be consistent between interpreters.
If we have the annotation for MI, we should have it in the annotate
mode as well.

There has been a discussion earlier about the merits and drawbacks
of adding the main "thread" to the list of threads when debugging
an application that doesn't use threads.  We only considered the
perspective of the user, but there is indeed the case of front-ends.
Assuming that the decision still stands, I think it is reasonable
to ask the front-ends to check that there is more than one thread
before going into "thread" mode.  I understand that this is making
things a little more complicated, but it shouldn't be that hard.

On the other hand, if there is a way to detect that the new "thread"
is from an unthreaded program, we could decide to not emit the
notification. But I would like things to be consistent - it should
be the same for both CLI, MI and annotations.

-- 
Joel

Re: [RFA] Report the main thread.

[Vladimir Prus]

Joel Brobecker wrote:

>> > I just want threads other than the main thread to be reported, as
>> > used to be the case.
> [...]
>> > That way I can create a threads buffer and display the thread
>> > number when there is more than one thread.
>> 
>> Cannot you just suppress display of thread list if there's just one
>> thread reported, on UI side?
> 
> I think it is important to be consistent between interpreters.
> If we have the annotation for MI, we should have it in the annotate
> mode as well.
> 
> There has been a discussion earlier about the merits and drawbacks
> of adding the main "thread" to the list of threads when debugging
> an application that doesn't use threads.  We only considered the
> perspective of the user, but there is indeed the case of front-ends.
> Assuming that the decision still stands, I think it is reasonable
> to ask the front-ends to check that there is more than one thread
> before going into "thread" mode.  I understand that this is making
> things a little more complicated, but it shouldn't be that hard.
> 
> On the other hand, if there is a way to detect that the new "thread"
> is from an unthreaded program, we could decide to not emit the
> notification. 

I think we're just trading different complexities here. Using "single-threaded 
program has a single thread" notion simplifies for frontend the task of
building the the internal representation of program. On the other hand, if
the frontend wishes to display just frames, and no any thread items, for
the case where the program has just a single thread, it requires some extra
logic -- presumably, just using different object as the root of some treeview
widget. 

And in my opinion, helping build accurate representation of a program
is what MI is about. Representation issues is what GUI toolkits are good
about.

> But I would like things to be consistent - it should 
> be the same for both CLI, MI and annotations.

Should CLI and MI be 100% consistent? They have different target audience.

- Volodya

Re: [RFA] Report the main thread.

[Nick Roberts]

 > There has been a discussion earlier about the merits and drawbacks
 > of adding the main "thread" to the list of threads when debugging
 > an application that doesn't use threads.  We only considered the
 > perspective of the user, but there is indeed the case of front-ends.
 > Assuming that the decision still stands, I think it is reasonable
 > to ask the front-ends to check that there is more than one thread
 > before going into "thread" mode.  I understand that this is making
 > things a little more complicated, but it shouldn't be that hard.
 > 
 > On the other hand, if there is a way to detect that the new "thread"
 > is from an unthreaded program, we could decide to not emit the
 > notification. But I would like things to be consistent - it should
 > be the same for both CLI, MI and annotations.

CLI and MI are both interpreters.  Annotations are just a markup of CLI
output, so I don't think they should be considered equally.

I don't see the need to report the main thread but if Pedro and Vladimir do,
then I'm happy to live with that.  If I propose an annotation that doesn't
report the main thread, then since they won't be using it, I would expect that
they won't object.  Furthermore, since annotations would never be used with MI
and they are destined for removal anyway, trying to make them consistent seems
a pointless exercise.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Mon, May 12, 2008 at 10:02:27AM +1200, Nick Roberts wrote:
> I don't see the need to report the main thread but if Pedro and Vladimir do,
> then I'm happy to live with that.  If I propose an annotation that doesn't
> report the main thread, then since they won't be using it, I would expect that
> they won't object.

So, could you do so in annotation-specific code?  The observer is not
annotation-specific or MI-specific; it may be used for more things in
the future.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Nick Roberts]

 > > I don't see the need to report the main thread but if Pedro and Vladimir
 > > do, then I'm happy to live with that.  If I propose an annotation that
 > > doesn't report the main thread, then since they won't be using it, I would
 > > expect that they won't object.
 > 
 > So, could you do so in annotation-specific code?  The observer is not
 > annotation-specific or MI-specific; it may be used for more things in
 > the future.

That's what my original patch did but Joel asked me to use the observer.
To be fair, that was before GDB reported the main thread.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Report the main thread.

[Daniel Jacobowitz]

On Mon, May 12, 2008 at 10:23:21AM +1200, Nick Roberts wrote:
> That's what my original patch did but Joel asked me to use the observer.
> To be fair, that was before GDB reported the main thread.

Yes, my comment was about the patch you posted this weekend that moved
the observer call.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Report the main thread.

[Vladimir Prus]

Nick Roberts wrote:

> 
>> Vladimir Prus wrote:
> 
>>> Daniel Jacobowitz wrote:
>>> 
>>>> On Sat, Apr 26, 2008 at 08:44:23PM +0400, Vladimir Prus wrote:
>>>>> Will some approach that only produce MI output be fine with you?
>>>> 
>>>> Perhaps that means any observer for a new MI thread should go in
>>>> add_thread_silent?
>>> 
>>> Yes, moving the observer call to add_thread_silent is the most
>>> direct approach to make MI work the way I want without disturbing CLI.
> 
>> Here's a patch to that effect. OK?
> 
>>        * thread.c (add_thread): Move observer call to ...
>>        (add_thread_silent): ... here.
>> ---
>> gdb/thread.c |    5 +++--
>> > 1 files changed, 3 insertions(+), 2 deletions(-)
> 
>> diff --git a/gdb/thread.c b/gdb/thread.c
>> index 46e6ba7..d3cfb32 100644
>> --- a/gdb/thread.c
>> +++ b/gdb/thread.c
>> @@ -124,6 +124,9 @@ add_thread_silent (ptid_t ptid)
>>    tp->num = ++highest_thread_num;
>>    tp->next = thread_list;
>>    thread_list = tp;
>> +
>> +  observer_notify_new_thread (tp);
>> +
>>    return tp;
>>  }
> 
> 
> Now that Joel has requested that I use observer_attach_new_thread for
> annotations this approach doesn't only produce MI output so it needs to
> be conditioned on MI:
> 
> @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
>    tp->num = ++highest_thread_num;
>    tp->next = thread_list;
>    thread_list = tp;
> +
> +  if (ui_out_is_mi_like_p (uiout))
> +    observer_notify_new_thread (tp);

This patch, IIUC, will cause the new_thread observer to be notified only
in MI mode. Then, no observer except for MI one will ever be called?
Then, why those non-MI observers exist?

Or to put in other way, I think this patch will make you annotation
observers to be never called. Is this what you want?

- Volodya

Re: [RFA] Report the main thread.

[Joel Brobecker]

> Now that Joel has requested that I use observer_attach_new_thread for
> annotations this approach doesn't only produce MI output so it needs to
> be conditioned on MI:
> 
> @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
>    tp->num = ++highest_thread_num;
>    tp->next = thread_list;
>    thread_list = tp;
> +
> +  if (ui_out_is_mi_like_p (uiout))
> +    observer_notify_new_thread (tp);
> +
>    return tp;
>  }

I am afraid this is not right. Could you elaborate more on what
the problem is?

-- 
Joel

Re: [RFA] Report the main thread.

[Nick Roberts]

Joel Brobecker writes:
 > > Now that Joel has requested that I use observer_attach_new_thread for
 > > annotations this approach doesn't only produce MI output so it needs to
 > > be conditioned on MI:
 > > 
 > > @@ -124,6 +124,10 @@ add_thread_silent (ptid_t ptid)
 > >    tp->num = ++highest_thread_num;
 > >    tp->next = thread_list;
 > >    thread_list = tp;
 > > +
 > > +  if (ui_out_is_mi_like_p (uiout))
 > > +    observer_notify_new_thread (tp);
 > > +
 > >    return tp;
 > >  }
 > 
 > I am afraid this is not right. Could you elaborate more on what
 > the problem is?

The problem is that Vladimir's patch means that the main thread is now
reported, not only for MI, but whenever the new_thread observer is registered.

-- 
Nick                                           http://www.inet.net.nz/~nickrob