[GDB/MI Ada] New GDB/MI command: -ada-tasks-info

[GDB/MI Ada] New GDB/MI command: -ada-tasks-info

[Joel Brobecker]

Hello,

As mentioned earlier, I am resending this patch set, with corrected
email subjects. The purpose of this patchset is to add a new GDB/MI
command to get the list of Ada tasks. I mirrored that new command
after the -thread-info command, except that it returns a table
rather than a list (the latter is done for backwards compatibility
reasons, which do not apply to this new command).

I would appreciate some feedback, at least on the user-visible
part of it (the command syntax and output). So, I will wait for
comments before I commit anything.

The series was tested on x86_64-linux.

Thank you,
-- 
Joel

[RFA 1/3] [Ada] New GDB/MI command: -ada-tasks-info

[Joel Brobecker]

This patch introduces a new GDB/MI command: -ada-tasks-info,
which is meant to be the MI equivalent of the CLI `info tasks'
command.

This new command returns an array, with each row corresponding
to one task.  For now, the columns of the array corresponds to
the columns displayed in the CLI output.

gdb/ChangeLog:

        * ada-lang.h (struct inferior): Declare.
        (print_ada_task_info): Add declaration.
        * ada-tasks.c (print_ada_task_info): Make non-static.
        * mi/mi-cmds.c (mi_cmds): Add "ada-task-info".
        * mi/mi-cmds.h (mi_cmd_ada_task_info): Add declaration.
        * mi/mi-main.c: #include "ada-lang.h".
        (mi_cmd_list_features): Add "ada-task-info" to the list
        of supported features.
        (mi_cmd_ada_task_info): New function.

Tested on x86_64-linux.

Thanks,
-- 
Joel

---
 gdb/ada-lang.h   |    5 +++++
 gdb/ada-tasks.c  |    2 +-
 gdb/mi/mi-cmds.c |    1 +
 gdb/mi/mi-cmds.h |    1 +
 gdb/mi/mi-main.c |   13 +++++++++++++
 5 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index cea9804..d887ef3 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -22,6 +22,7 @@
 #define ADA_LANG_H 1
 
 struct frame_info;
+struct inferior;
 
 #include "value.h"
 #include "gdbtypes.h"
@@ -385,4 +386,8 @@ extern void iterate_over_live_ada_tasks
 
 extern int ada_build_task_list (void);
 
+extern void print_ada_task_info (struct ui_out *uiout,
+				 char *taskno_str,
+				 struct inferior *inf);
+
 #endif
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 6e2f2f5..7dff8f8 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -929,7 +929,7 @@ ada_build_task_list (void)
    interpreted as a task number, and the table will be limited to
    that task only.  */
 
-static void
+void
 print_ada_task_info (struct ui_out *uiout,
 		     char *arg_str,
 		     struct inferior *inf)
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 0b32db0..960d2dc 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -33,6 +33,7 @@ static void build_table (struct mi_cmd *commands);
 
 struct mi_cmd mi_cmds[] =
 {
+  { "ada-task-info", { NULL, 0 }, mi_cmd_ada_task_info },
   { "add-inferior", { NULL, 0 }, mi_cmd_add_inferior },
   { "break-after", { "ignore", 1 }, NULL },
   { "break-condition", { "cond", 1 }, NULL },
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 18b4ad7..67aae59 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -36,6 +36,7 @@ extern const char mi_all_values[];
 typedef void (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Function implementing each command */
+extern mi_cmd_argv_ftype mi_cmd_ada_task_info;
 extern mi_cmd_argv_ftype mi_cmd_add_inferior;
 extern mi_cmd_argv_ftype mi_cmd_break_insert;
 extern mi_cmd_argv_ftype mi_cmd_break_commands;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 59fa576..36c4fe6 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -52,6 +52,7 @@
 #include "osdata.h"
 #include "splay-tree.h"
 #include "tracepoint.h"
+#include "ada-lang.h"
 
 #include <ctype.h>
 #include <sys/time.h>
@@ -1720,6 +1721,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "thread-info");
       ui_out_field_string (uiout, NULL, "data-read-memory-bytes");
       ui_out_field_string (uiout, NULL, "breakpoint-notifications");
+      ui_out_field_string (uiout, NULL, "ada-task-info");
       
 #if HAVE_PYTHON
       ui_out_field_string (uiout, NULL, "python");
@@ -2503,3 +2505,14 @@ mi_cmd_trace_stop (char *command, char **argv, int argc)
   stop_tracing ();
   trace_status_mi (1);
 }
+
+/* Implement the "-ada-task-info" GDB/MI command.  */
+
+void
+mi_cmd_ada_task_info (char *command, char **argv, int argc)
+{
+  if (argc != 0 && argc != 1)
+    error (_("Invalid MI command"));
+
+  print_ada_task_info (current_uiout, argv[0], current_inferior ());
+}
-- 
1.7.1

Re: [RFA 1/3] [Ada] New GDB/MI command: -ada-tasks-info

[Vladimir Prus]

On Saturday, September 17, 2011 00:45:19 Joel Brobecker wrote:
> This patch introduces a new GDB/MI command: -ada-tasks-info,
> which is meant to be the MI equivalent of the CLI `info tasks'
> command.
> 
> This new command returns an array, with each row corresponding
> to one task.  For now, the columns of the array corresponds to
> the columns displayed in the CLI output.
> 
> gdb/ChangeLog:
> 
>         * ada-lang.h (struct inferior): Declare.
>         (print_ada_task_info): Add declaration.
>         * ada-tasks.c (print_ada_task_info): Make non-static.
>         * mi/mi-cmds.c (mi_cmds): Add "ada-task-info".
>         * mi/mi-cmds.h (mi_cmd_ada_task_info): Add declaration.
>         * mi/mi-main.c: #include "ada-lang.h".
>         (mi_cmd_list_features): Add "ada-task-info" to the list
>         of supported features.
>         (mi_cmd_ada_task_info): New function.

Hi Joel,

I don't have any questions about the implementation, but I do wonder
whether reusing "info os xxx" mechanism might be better here, e.g.:

	info os ada-tasks

and a matching MI command. Oops, it appears we never submitted the
matching MI command, but that's a one line patch anyway, below.

I don't have the slighest clue whether "info os" mechanism is a 
good match here; I'm speaking purely from extensible interface
point of view.

Thanks,


-- 
Vladimir Prus
CodeSourcery / Mentor Graphics
+7 (812) 677-68-40


Index: gdb/mi/mi-cmds.c
===================================================================
--- gdb/mi/mi-cmds.c    (revision 305439)
+++ gdb/mi/mi-cmds.c    (revision 305440)
@@ -84,6 +84,7 @@
   { "gdb-version", { "show version", 0 }, 0 },
   { "inferior-tty-set", { NULL, 0 }, mi_cmd_inferior_tty_set},
   { "inferior-tty-show", { NULL, 0 }, mi_cmd_inferior_tty_show},
+  { "info-os", { "info os", 1 }, NULL },
   { "interpreter-exec", { NULL, 0 }, mi_cmd_interpreter_exec},
   { "list-features", { NULL, 0 }, mi_cmd_list_features},
   { "list-target-features", { NULL, 0 }, mi_cmd_list_target_features},

Re: [RFA 1/3] [Ada] New GDB/MI command: -ada-tasks-info

[Joel Brobecker]

> I don't have any questions about the implementation, but I do wonder
> whether reusing "info os xxx" mechanism might be better here, e.g.:
> 
> 	info os ada-tasks

I don't think it makes sense in this case. I could see system-wide
queries, or os-specific commands to use that prefix.  But I don't see
how the list of Ada tasks would fit in the `info os' prefix. Ada tasks
and threads are very similar, so I would treat them the same.

-- 
Joel

[PATCH 3/3] [Ada/doco] Document the new -ada-task-info GDB/MI command.

[Joel Brobecker]

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Ada Tasking Commands): New node.
        (GDB/MI Miscellaneous Commands): Add `ada-task-info'
        as possible feature returned by the `-list-features'
        command.

Tested on x86_64-linux.  OK to commit?

Thanks,
-- 
Joel

---
 gdb/doc/gdb.texinfo |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 051377d..cd35f2b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24765,6 +24765,7 @@ may repeat one or more times.
 * GDB/MI Breakpoint Commands::
 * GDB/MI Program Context::
 * GDB/MI Thread Commands::
+* GDB/MI Ada Tasking Commands::
 * GDB/MI Program Execution::
 * GDB/MI Stack Manipulation::
 * GDB/MI Variable Objects::
@@ -26641,6 +26642,82 @@ args=[@{name="format",value="0x8048e9c \"%*s%c %d %c\\n\""@},
 @end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+@node GDB/MI Ada Tasking Commands
+@section @sc{gdb/mi} Ada Tasking Commands
+
+@subheading The @code{-ada-task-info} Command
+@findex -ada-task-info
+
+@subsubheading Synopsis
+
+@smallexample
+ -ada-task-info [ @var{task-id} ]
+@end smallexample
+
+Reports information about either a specific Ada task, if the
+@var{task-id} parameter is present, or about all Ada tasks.
+
+@subsubheading @value{GDBN} Command
+
+The @samp{info tasks} command prints the same information
+about all Ada tasks.
+
+@subsubheading Result
+
+The result is a table of Ada tasks.  The following columns are
+defined for each Ada task:
+
+@table @samp
+@item current
+This field exists only for the current thread.  It has the value @samp{*}.
+
+@item id
+The identifier that @value{GDBN} uses to refer to the Ada task.
+
+@item task-id
+The identifier that the target uses to refer to the Ada task.
+
+@item thread-id
+The identifier of the thread corresponding to the Ada task.
+
+This field should always exist, as Ada tasks are always implemented
+on top of a thread.  But if @value{GDBN} cannot find this corresponding
+thread for any reason, the field is omitted.
+
+@item parent-id
+This field exists only when the task was created by another task.
+In this case, it provides the ID of the parent task.
+
+@item priority
+The base priority of the task.
+
+@item state
+The current state of the task.
+
+@item name
+The name of the task.
+
+@end table
+
+@subsubheading Example
+
+@smallexample
+-thread-info
+^done,tasks=@{nr_rows="3",nr_cols="8",
+hdr=[@{width="1",alignment="-1",col_name="current",colhdr=""@},
+@{width="3",alignment="1",col_name="id",colhdr="ID"@},
+@{width="9",alignment="1",col_name="task-id",colhdr="TID"@},
+@{width="4",alignment="1",col_name="thread-id",colhdr=""@},
+@{width="4",alignment="1",col_name="parent-id",colhdr="P-ID"@},
+@{width="3",alignment="1",col_name="priority",colhdr="Pri"@},
+@{width="22",alignment="-1",col_name="state",colhdr="State"@},
+@{width="1",alignment="2",col_name="name",colhdr="Name"@}],
+body=[@{current="*",id="1",task-id="   644010",thread-id="1",priority="48",
+state="Child Termination Wait",name="main_task"@}]@}
+(gdb)
+@end smallexample
+
+@c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Program Execution
 @section @sc{gdb/mi} Program Execution
 
@@ -30300,7 +30377,8 @@ Indicates presense of the @code{-data-read-memory-bytes} and the
 @item breakpoint-notifications
 Indicates that changes to breakpoints and breakpoints created via the
 CLI will be announced via async records.
-
+@item ada-task-info
+Indicates the presence of the @code{-ada-task-info} command.
 @end table
 
 @subheading The @code{-list-target-features} Command
-- 
1.7.1

Re: [PATCH 3/3] [Ada/doco] Document the new -ada-task-info GDB/MI command.

[Eli Zaretskii]

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: vladimir@codesourcery.com,	Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 16 Sep 2011 13:45:21 -0700
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (GDB/MI Ada Tasking Commands): New node.
>         (GDB/MI Miscellaneous Commands): Add `ada-task-info'
>         as possible feature returned by the `-list-features'
>         command.

I sent my comments in response to the previous message with similar
content.

[PATCH 2/3] [Ada] Add field "thread-id" in -ada-task-info output

[Joel Brobecker]

This patch enhances the -ada-task-info command, adding a new
field called "thread-id" in the command output.  This field
provides the ID of the task's corresponding thread (if that
information could not be determined, the field is omitted).

This field is useful in the context of GDB/MI users, because
it allows a quick translation from Ada task to thread, and
thus easy use of all the commands that take a --thread argument.

gdb/ChangeLog:

        * ada-tasks.c (print_ada_task_info): Add "thread-id" field
        in output of -ada-task-info GDB/MI command.

Tested on x86_64-linux.

Thanks,
-- 
Joel

---
 gdb/ada-tasks.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 7dff8f8..2673fed 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -938,6 +938,7 @@ print_ada_task_info (struct ui_out *uiout,
   int taskno, nb_tasks;
   int taskno_arg = 0;
   struct cleanup *old_chain;
+  int nb_columns;
 
   if (ada_build_task_list () == 0)
     {
@@ -949,14 +950,29 @@ print_ada_task_info (struct ui_out *uiout,
   if (arg_str != NULL && arg_str[0] != '\0')
     taskno_arg = value_as_long (parse_and_eval (arg_str));
 
+  if (ui_out_is_mi_like_p (uiout))
+    /* In GDB/MI mode, we want to provide the thread ID corresponding
+       to each task.  This allows clients to quickly find the thread
+       associated to any task, which is helpful for commands that
+       take a --thread argument.  However, in order to be able to
+       provide that thread ID, the thread list must be up to date
+       first.  */
+    target_find_new_threads ();
+
   data = get_ada_tasks_inferior_data (inf);
   nb_tasks = VEC_length (ada_task_info_s, data->task_list);
 
-  old_chain = make_cleanup_ui_out_table_begin_end (uiout, 7, nb_tasks,
-                                                   "tasks");
+  nb_columns = ui_out_is_mi_like_p (uiout) ? 8 : 7;
+  old_chain = make_cleanup_ui_out_table_begin_end (uiout, nb_columns,
+						   nb_tasks, "tasks");
   ui_out_table_header (uiout, 1, ui_left, "current", "");
   ui_out_table_header (uiout, 3, ui_right, "id", "ID");
   ui_out_table_header (uiout, 9, ui_right, "task-id", "TID");
+  /* The following column is provided in GDB/MI mode only because
+     it is only really useful in that mode, and also because it
+     allows us to keep the CLI output shorter and more compact.  */
+  if (ui_out_is_mi_like_p (uiout))
+    ui_out_table_header (uiout, 4, ui_right, "thread-id", "");
   ui_out_table_header (uiout, 4, ui_right, "parent-id", "P-ID");
   ui_out_table_header (uiout, 3, ui_right, "priority", "Pri");
   ui_out_table_header (uiout, 22, ui_left, "state", "State");
@@ -996,6 +1012,19 @@ print_ada_task_info (struct ui_out *uiout,
       /* Print the Task ID.  */
       ui_out_field_fmt (uiout, "task-id", "%9lx", (long) task_info->task_id);
 
+      /* Print the associated Thread ID.  */
+      if (ui_out_is_mi_like_p (uiout))
+        {
+	  const int thread_id = pid_to_thread_id (task_info->ptid);
+
+	  if (thread_id != 0)
+	    ui_out_field_int (uiout, "thread-id", thread_id);
+	  else
+	    /* This should never happen unless there is a bug somewhere,
+	       but be resilient when that happens.  */
+	    ui_out_field_skip (uiout, "thread-id");
+	}
+
       /* Print the ID of the parent task.  */
       parent_id = get_task_number_from_id (task_info->parent, inf);
       if (parent_id)
-- 
1.7.1

Re: [GDB/MI Ada] New GDB/MI command: -ada-tasks-info

[Joel Brobecker]

> The purpose of this patchset is to add a new GDB/MI
> command to get the list of Ada tasks. I mirrored that new command
> after the -thread-info command, except that it returns a table
> rather than a list (the latter is done for backwards compatibility
> reasons, which do not apply to this new command).

FYI: The whole patch set has now been checked in.

Thanks again to everyone who gave their feedback.
-- 
Joel

Re: [RFA 1/3] [Ada] New GDB/MI command: -ada-tasks-info

[Prus, Vladimir]

OK. In that case, I have no more comments. 

Joel Brobecker <brobecker@adacore.com> wrote:

> I don't have any questions about the implementation, but I do wonder
> whether reusing "info os xxx" mechanism might be better here, e.g.:
> 
> 	info os ada-tasks

I don't think it makes sense in this case. I could see system-wide
queries, or os-specific commands to use that prefix.  But I don't see
how the list of Ada tasks would fit in the `info os' prefix. Ada tasks
and threads are very similar, so I would treat them the same.

-- 
Joel