Re: [PATCH] Add "thread-group id" and "id in thread-group" info to -thread-info output

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 17-01-19 09:41 AM, Simon Marchi wrote:
> The CLI has been showing inferior-qualified thread ids for a while, but
> front-ends have no way to do the same, as only the global thread ids are
> included in the MI output.  In front-ends in general, but mostly in
> those that include a gdb console, it would be interesting to show the
> same thread id in the GUI than what "info threads" displays.  The user
> would then see consistent thread ids across the two UIs.
> 
> This patch adds two extra fields to the -thread-info MI command output.
> In this snippet of prettified -thread-info output, the lines marked with
> a + are new:
> 
>     {
> +     tg-id = "2",
> +     id-in-tg = "1",
>       id = "3",
>       target-id = "Thread 0x2aaaaab04640 (LWP 3228)",
>       name = "mi-qualified-th",
>       ...
>     },
> 
> Here, the thread has the global id 3 (monotonously increasing across all
> inferiors) and the inferior-qualified id 2.1.  The global thread id is
> still the one used throughout the rest of MI.  The new fields are simply
> intended to be presented to the user.
> 
> We chose adding the new information as two separate fields rather than a
> single field with the value "2.1".  We think that it gives the front-end
> more flexibility about how it want to present the info.
> 
> Note thatt I renamed the existing CLI field "id-in-tg" to "qualified-id"
> and named one of the new MI field "id-in-tg", because I found that
> "id-in-tg" described better the Y in X.Y, and "qualified-id" described
> better the whole thing.  It's not a big deal, as the field name is never
> shown in the CLI.  We have to carefully choose the new names however,
> since they'll be exposed in the MI.
> 
> gdb/ChangeLog:
> 
> 	* thread.c (print_thread_info_1): Rename field id-in-tg to
> 	qualified-id.  Add new fields id-in-tg and tg-id when printing
> 	on MI.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* mi-qualified-thread-id.exp: New file.
> 	* mi-qualified-thread-id.c: New file.
> ---
>  gdb/testsuite/gdb.mi/mi-qualified-thread-id.c   | 57 +++++++++++++++++++
>  gdb/testsuite/gdb.mi/mi-qualified-thread-id.exp | 76 +++++++++++++++++++++++++
>  gdb/thread.c                                    | 11 +++-
>  3 files changed, 141 insertions(+), 3 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.mi/mi-qualified-thread-id.c
>  create mode 100644 gdb/testsuite/gdb.mi/mi-qualified-thread-id.exp
> 
> diff --git a/gdb/testsuite/gdb.mi/mi-qualified-thread-id.c b/gdb/testsuite/gdb.mi/mi-qualified-thread-id.c
> new file mode 100644
> index 0000000000..cb50e50c32
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-qualified-thread-id.c
> @@ -0,0 +1,57 @@
> +/* Copyright 2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +
> +static pthread_barrier_t barrier;
> +
> +static void *
> +thread_func (void *arg)
> +{
> +  /* Notify the main thread that this thread has started.  */
> +  pthread_barrier_wait (&barrier);
> +
> +  /* Wait until main gives us the signal that we can quit.  */
> +  pthread_barrier_wait (&barrier);
> +  return NULL;
> +}
> +
> +static void
> +thread_started ()
> +{
> +}
> +
> +int main ()
> +{
> +  pthread_t thread;
> +
> +  pthread_barrier_init (&barrier, NULL, 2);
> +
> +  pthread_create (&thread, NULL, thread_func, NULL);
> +
> +  /* Wait until the thread has started.  */
> +  pthread_barrier_wait (&barrier);
> +
> +  thread_started ();
> +
> +  /* Tell the thread that it can quit.  */
> +  pthread_barrier_wait (&barrier);
> +
> +  pthread_join (thread, NULL);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.mi/mi-qualified-thread-id.exp b/gdb/testsuite/gdb.mi/mi-qualified-thread-id.exp
> new file mode 100644
> index 0000000000..43c1409786
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-qualified-thread-id.exp
> @@ -0,0 +1,76 @@
> +# Copyright 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +standard_testfile
> +
> +if {[use_gdb_stub]} {
> +    # There is no point in testing inferior-qualified thread ids if we can't
> +    # have multiple inferiors.
> +    untested "multi-inferior not supported"
> +    return
> +}
> +
> +if  { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested ${testfile}
> +     return -1
> +}
> +
> +# Prepare the inferiors.  Start and run them until the "thread_started" function.
> +
> +proc setup { } {
> +    global binfile
> +    global srcdir
> +    global subdir
> +    global srcfile
> +
> +    gdb_exit
> +    mi_gdb_start
> +    mi_gdb_reinitialize_dir $srcdir/$subdir
> +    mi_gdb_load ${binfile}
> +
> +    mi_create_breakpoint "thread_started" "insert breakpoint at thread_started"
> +
> +    # Start inferior 1
> +    mi_run_cmd
> +    mi_expect_stop "breakpoint-hit" "thread_started" ".*" "${srcfile}" ".*" {"" "disp=\"keep\""} "inferior 1 stops"
> +
> +    # Add and start inferior 2
> +    mi_gdb_test "add-inferior -exec ${binfile}" ".*=thread-group-added,id=\"i2\".*" "add second inferior"
> +    mi_gdb_test "inferior 2" ".*" "switch to inferior 2"
> +    mi_run_cmd
> +    mi_expect_stop "breakpoint-hit" "thread_started" ".*" "${srcfile}" ".*" {"" "disp=\"keep\""} "inferior 2 stops"
> +}
> +
> +proc test_thread_info_qualified_thread_id { } {
> +    setup
> +
> +    set expected [join \
> +	{ "111\\^done,threads=.*" \
> +	  "tg-id=\"1\",id-in-tg=\"1\",id=\"1\".*" \
> +	  "tg-id=\"1\",id-in-tg=\"2\",id=\"2\".*" \
> +	  "tg-id=\"2\",id-in-tg=\"1\",id=\"3\".*" \
> +	  "tg-id=\"2\",id-in-tg=\"2\",id=\"4\".*" \
> +	} ""]
> +
> +    mi_gdb_test \
> +	"111-thread-info" \
> +	 $expected \
> +	"thread ids in -thread-info"
> +}
> +
> +test_thread_info_qualified_thread_id
> diff --git a/gdb/thread.c b/gdb/thread.c
> index e45b25750e..29cfa61e13 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1247,7 +1247,7 @@ print_thread_info_1 (struct ui_out *uiout, char *requested_threads,
>        uiout->table_header (1, ui_left, "current", "");
>  
>        if (!uiout->is_mi_like_p ())
> -	uiout->table_header (4, ui_left, "id-in-tg", "Id");
> +	uiout->table_header (4, ui_left, "qualified-id", "Id");
>        if (show_global_ids || uiout->is_mi_like_p ())
>  	uiout->table_header (4, ui_left, "id", "GId");
>        uiout->table_header (17, ui_left, "target-id", "Target Id");
> @@ -1282,8 +1282,13 @@ print_thread_info_1 (struct ui_out *uiout, char *requested_threads,
>  	    uiout->field_skip ("current");
>  	}
>  
> -      if (!uiout->is_mi_like_p ())
> -	uiout->field_string ("id-in-tg", print_thread_id (tp));
> +      if (uiout->is_mi_like_p ())
> +	{
> +	  uiout->field_int ("tg-id", tp->inf->num);
> +	  uiout->field_int ("id-in-tg", tp->per_inf_num);
> +	}
> +      else
> +	uiout->field_string ("qualified-id", print_thread_id (tp));
>  
>        if (show_global_ids || uiout->is_mi_like_p ())
>  	uiout->field_int ("id", tp->global_num);
> 

Ping.