New mi commands for threads

New mi commands for threads

[Denis PILAT]

Hi everyone,


I'd like to propose an implementation of
-thread-info
-thread-list-all-threads

Before posting patches, I'd like to discuss a little bit.
First should I post 2 patches or can I post only one for both ?

About -thread-info:
------------------
The only way to get information on a thread is the "info thread" cli 
command, this one sometimes takes a while to execute since it gets a lot 
of stuff (we already discusses on performance problem before, I won't 
bother you with that today) . I propose an implementation of 
-thread-info which take an optional argument (the thread ID). If not 
given, we'll get information on the current thread, that could be useful 
as well. If argument if given ... I'm sure you've figured out.
Both of these patch aim at getting rid of CLI command usage into GUI 
client like Eclipse.

Here is an output example of what is implemented:
-thread-info 3
^done,thread-id="3",frame={level="0",addr="0x00aeaaec",func="nanosleep",args=[],from="/lib/tls/libc.so.6"},thread-extra-info="bla 
bla bla"

As you can see I display "extra information" as well. These are not 
given by -thread-list-ids, and it's a good idea since extra info 
retrieval could also take a while.


About -thread-list-all-threads:
-------------------------------
It will display the same for all threads, plus the number of thread and 
the current thread id.
Example:
-thread-list-all-threads
^done,thread-list={thread-id="6",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
bla",thread-id="5",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
bla",thread-id="4",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
bla",thread-id="3",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
bla",thread-id="2",frame={level="0",addr="0x00e80aec",func="nanosleep",args=[],from="/lib/tls/libc.so.6"},thread-extra-info="bla 
bla",thread-id="1",frame={level="0",func="main",args=[],file="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",fullname="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",line="87"},line="87",file="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",thread-extra-info="bla 
bla"},number-of-threads="6",current-thread-id="1"


Do you agree the output contains and format ?

Waiting for your feedback.
Denis

Re: New mi commands for threads

[Nick Roberts]

 > I'd like to propose an implementation of
 > -thread-info
 > -thread-list-all-threads
 > 
 > Before posting patches, I'd like to discuss a little bit.
 > First should I post 2 patches or can I post only one for both ?

I think it's best to keep the patches separate unless the code changes
overlap.

 > About -thread-list-all-threads:
 > -------------------------------
 > It will display the same for all threads, plus the number of thread and 
 > the current thread id.
 > Example:
 > -thread-list-all-threads
 > ^done,thread-list={thread-id="6",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
 > bla",thread-id="5",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
 > bla",thread-id="4",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
 > bla",thread-id="3",frame={level="0",addr="0x007b9939",func="__lll_mutex_lock_wait",args=[],from="/lib/tls/libpthread.so.0"},thread-extra-info="bla 
 > bla",thread-id="2",frame={level="0",addr="0x00e80aec",func="nanosleep",args=[],from="/lib/tls/libc.so.6"},thread-extra-info="bla 
 > bla",thread-id="1",frame={level="0",func="main",args=[],file="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",fullname="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",line="87"},line="87",file="/project/flexdbug/user/pd10/gdb/sts-gdb-repository/vendor/cvs/head/src/gdb/testsuite/gdb.mi/pthreads.c",thread-extra-info="bla 
 > bla"},number-of-threads="6",current-thread-id="1"

I would put number-of-threads="6",current-thread-id="1" at the start so that
they don't get lost behind the thread list (I know the output is only intended
for the front end but it might make debugging of GDB easier and costs nothing).

Lists use square brackets, information on each thread should be enclosed in
curly brackets:

threads=[{thread-id="6",frame={...},thread-extra-info="bla"},{thread-id="5",frame={...},thread-extra-info="bla"},...]

I guess your output has been manually generated as thread-id="1" seems to
duplicate some information.

I thought that the frame information was expensive to get and that it was
agreed you could get it, if needed, using other MI commands anyway.


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

Re: New mi commands for threads

[Denis PILAT]

Nick Roberts wrote:
> I think it's best to keep the patches separate unless the code changes
> overlap.
Ok I'll do that.
>
>
> I would put number-of-threads="6",current-thread-id="1" at the start so that
> they don't get lost behind the thread list (I know the output is only intended
> for the front end but it might make debugging of GDB easier and costs nothing).
>
> Lists use square brackets, information on each thread should be enclosed in
> curly brackets:
>
> threads=[{thread-id="6",frame={...},thread-extra-info="bla"},{thread-id="5",frame={...},thread-extra-info="bla"},...]
>
> I guess your output has been manually generated as thread-id="1" seems to
> duplicate some information.
This output is a side effect of the frame printing, the actual 
-thread-select command has the same. I'm going to have a look at how we 
can avoid it, but I think it will impact *-thread-select* as well.

About brackets, I see how they were used in *-thread-list-ids*, 
according to your comment, this one should use square brackets as well, 
am I right ?

About position of "number-of-threads" and "current-thread-id", I prefer 
them in front as well, but that would be not consistent with 
-thread-list-ids. But do we care about consistency with this one that 
seems to be not consistent with the MI list format  ? 
>
> I thought that the frame information was expensive to get and that it was
> agreed you could get it, if needed, using other MI commands anyway.
Yes it is, but I want to get rid of CLI commands and for front ends that 
want to get all informations, they could use this command if they need 
extra information. But I can add an option to include or not these 
information.
For remote (or on board) debugging, our front ends will emit 
*-thread-list-ids* to get only the list, and then *-thread-info* to 
focus only on the current thread.

But for "native and efficient" debugging, I guess a call to 
*-thread-list-all-threads* will do the job.
That will give us:
-thread-list-ids                 : gives list of thread ids + number of 
threads
-thread-list-all-threads         : gives the above + stack frame 
information per thread + current thread
-thread-list-all-threads --extra : gives the above + extra information 
per thread.



Thanks a lot for your feedback Nick,
-- 
Denis Pilat

Re: New mi commands for threads

[Nick Roberts]

 > > Lists use square brackets, information on each thread should be enclosed in
 > > curly brackets:
 > >
 > > threads=[{thread-id="6",frame={...},thread-extra-info="bla"},{thread-id="5",frame={...},thread-extra-info="bla"},...]
 > >
 > > I guess your output has been manually generated as thread-id="1" seems to
 > > duplicate some information.
 > This output is a side effect of the frame printing, the actual 
 > -thread-select command has the same. I'm going to have a look at how we 
 > can avoid it, but I think it will impact *-thread-select* as well.

OK, I dodn't realise that.  Interestingly the example in the manual doesn't
show the duplicate information (or the fullname field, so it will have to be
updated).  It seems to due to the fact that do_captured_thread_select uses
SRC_AND_LOC, while the other MI commands use LOC_AND_ADDRESS.  I see that
normal_stop specifies LOC_AND_ADDRESS specifically for MI.  It looks a bit
of a hack but perhaps we should do the same for do_captured_thread_select:

	  /* For mi, have the same behavior every time we stop:
	     print everything but the source line. */
	  if (ui_out_is_mi_like_p (uiout))
	    source_flag = LOC_AND_ADDRESS;


 > About brackets, I see how they were used in *-thread-list-ids*, 
 > according to your comment, this one should use square brackets as well, 
 > am I right ?

Yes, I think it, should but we probably shouldn't change it until we bump the
MI level.  It's probably a legacy from mi0 when all brackets were curly ones.

 > About position of "number-of-threads" and "current-thread-id", I prefer 
 > them in front as well, but that would be not consistent with 
 > -thread-list-ids. But do we care about consistency with this one that 
 > seems to be not consistent with the MI list format  ? 

Well one lists threads and the other lists thread ids, so I don't see a
problem.

 > > I thought that the frame information was expensive to get and that it was
 > > agreed you could get it, if needed, using other MI commands anyway.
 > Yes it is, but I want to get rid of CLI commands and for front ends that 
 > want to get all informations, they could use this command if they need 
 > extra information. But I can add an option to include or not these 
 > information.
 > For remote (or on board) debugging, our front ends will emit 
 > *-thread-list-ids* to get only the list, and then *-thread-info* to 
 > focus only on the current thread.

OK, I see now.

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

Re: New mi commands for threads

[Denis PILAT]

Nick Roberts wrote:
>  > This output is a side effect of the frame printing, the actual 
>  > -thread-select command has the same. I'm going to have a look at how we 
>  > can avoid it, but I think it will impact *-thread-select* as well.
>
> OK, I dodn't realise that.  Interestingly the example in the manual doesn't
> show the duplicate information (or the fullname field, so it will have to be
> updated).  It seems to due to the fact that do_captured_thread_select uses
> SRC_AND_LOC, while the other MI commands use LOC_AND_ADDRESS.  I see that
> normal_stop specifies LOC_AND_ADDRESS specifically for MI.  It looks a bit
> of a hack but perhaps we should do the same for do_captured_thread_select:
>
> 	  /* For mi, have the same behavior every time we stop:
> 	     print everything but the source line. */
> 	  if (ui_out_is_mi_like_p (uiout))
> 	    source_flag = LOC_AND_ADDRESS;
>
>
>   
Right,
I'm going to propose a separate patch for that right now.

Then I'll do the same for -thread-info new command.

Denis