This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Implement -thread-info.


On Tuesday 19 February 2008 00:44:33 Nick Roberts wrote:
>  > > mi_info_threads is nearly the same function as thread_command.  
>  > 
>  > You probably meant info_threads_command?
> 
> Yes.
> 
>  > > Can this be 
>  > > used in a dual way just as -break-list uses breakpoint_1?
>  > 
>  > I don't know -- I actually have no idea what magic makes breakpoint_1 to
>  > work both for MI and CLI. Do you happen to know?
> 
> It just seems to be a kind of overloading where the output format of
> functions like ui_out_field_string depends on the interpreter used (value
> of uiout).  

Okay, I've tried to do that. Now, I'm using ui_out_text to ensure empty
spaces between fields. breakpoint.c uses ui_out_table_begin_end, but
I don't think the formatting that one produces is good here.

> In cases where only one interpreter outputs anything 
> ui_out_is_mi_like_p is used.
> 
>  > > Also Denis Pilat has already proposed a patch for -thread-info:
>  > > http://sourceware.org/ml/gdb-patches/2007-03/msg00167.html
>  > 
>  > I did not notice that.
>  > 
>  > > 
>  > > How does your patch compare?  (assuming the problem of return type is solved
>  > > as has been done for thread_select).
>  > 
>  > It appears that my patch:
>  > 
>  > 1. Does not bother with making non-throwing function, as MI top-level can
>  > handle exceptions.
>  > 
>  > 2. Allows to print information about all threads.
> 
> I see.  Denis also proposed something for -thread-list-all-threads
> (http://sourceware.org/ml/gdb-patches/2007-03/msg00144.html)

I think having one command is a bit better.

> Do you not want to print the frame level?  

Probably not; this command is useful right after stop, when the level is always 0.

> Also do you want to 
> catch errors in print_stack_frame?

Yes, why not? A typical error is when we have a char* parameter pointing to
something not very accessible, in which case I'd prefer print_stack_frame
to catch this error, as opposed to -thread-info outright dying. 


> Rather than: 
> 
> +      print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
> 
> Would it be better to use:
> 
> +      print_frame_info (get_selected_frame (NULL), 1, LOC_AND_ADDRESS, 0);
> 
> ?
> 
> Note that with MI (following the thread with Denis) that the address gets
> printed anyway when print_stack_frame is used:
> 
>   args.print_what = ui_out_is_mi_like_p (uiout) ? LOC_AND_ADDRESS : print_what;

Then, using LOC_AND_ADDRESS in print_stack_frame invocation will only
be more clear, without affecting behaviour? But I guess it's better, indeed.

> 
>  > I don't know what problem of return type you refer to -- can you clarify?
> 
> That, in Denis' patch, gdb_thread_info returns type enum gdb_rc but
> catch_exceptions_with_msg returns type int.  (just like gdb_breakpoint and
> break_command_really did for a while).

This problem does not exist in my patch, as catch_exceptions_with_msg is
just not used.

I attach the revised version of the patch, including docs. Eli, is the
doc part OK? I intend to commit the code part in a week unless there are
objections.

- Volodya

	[gdb]
	Implement -thread-info.
        * gdb.h (mi_info_threads): Declare.

        * thread.c (mi_info_threads): New, extracted
        from info_threads_command and adjusted to
        work for CLI and MI.
        (info_threads_command): Use mi_info_threads.

        * mi/mi-cmds.c (mi_cmds): Specify a handler
        for -thread-info.
        * mi/mi-cmds.h (mi_cmd_thread_info): Declare.
        * mi/mi-main.c (mi_cmd_thread_info): New.
        (mi_cmd_list_features): Include 'thread-info'.

        [gdb/doc]
        * gdb.texinfo (Thread Commands): Document
        -thread-info.  Remove -thread-list-all-threads.
From b194235c305682ab6e4176df70dc52533e8b2e07 Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Sun, 17 Feb 2008 18:16:02 +0300
Subject: [RFA] Implement -thread-info.
To: gdb-patches@sources.redhat.com
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

	* gdb.h (mi_info_threads): Declare.

	* thread.c (mi_info_threads): New, extracted
	from info_threads_command and adjusted to
	work for CLI and MI.
	(info_threads_command): Use mi_info_threads.

	* mi/mi-cmds.c (mi_cmds): Specify a handler
	for -thread-info.
	* mi/mi-cmds.h (mi_cmd_thread_info): Declare.
	* mi/mi-main.c (mi_cmd_thread_info): New.
	(mi_cmd_list_features): Include 'thread-info'.

	doc/
	* gdb.texinfo (Thread Commands): Document
	-thread-info.  Remove -thread-list-all-threads.
---
 gdb/doc/gdb.texinfo |   35 ++++++++++++-------------
 gdb/gdb.h           |    2 +
 gdb/mi/mi-cmds.c    |    3 +-
 gdb/mi/mi-cmds.h    |    1 +
 gdb/mi/mi-main.c    |   19 +++++++++++++
 gdb/thread.c        |   71 ++++++++++++++++++++++++++++++++++++++++-----------
 6 files changed, 96 insertions(+), 35 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8754350..66480d8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18854,34 +18854,33 @@ The corresponding @value{GDBN} command is @samp{pwd}.
 @subsubheading Synopsis
 
 @smallexample
- -thread-info
+ -thread-info [ @var{thread-id} ]
 @end smallexample
 
+Reports the information about either a specific thread, if 
+the @var{thread-id} parameter is present, or about all
+threads.  When printing information about all threads,
+also reports the current thread.
+
 @subsubheading @value{GDBN} Command
 
-No equivalent.
+The @samp{info thread} command prints the same information
+about all threads.
 
 @subsubheading Example
-N.A.
-
-
-@subheading The @code{-thread-list-all-threads} Command
-@findex -thread-list-all-threads
-
-@subsubheading Synopsis
 
 @smallexample
- -thread-list-all-threads
+-thread-info
+^done,threads=[
+@{id="2",target-id="Thread 0xb7e14b90 (LWP 21257)",
+   frame=@{addr="0xffffe410",func="__kernel_vsyscall",args=[]@},
+@{id="1",target-id="Thread 0xb7e156b0 (LWP 21254)",
+   frame=@{addr="0x0804891f",func="foo",args=[@{name="i",value="10"@}],
+           file="/tmp/a.c",fullname="/tmp/a.c",line="158"@}@}],
+current-thread-id="1"
+(gdb)
 @end smallexample
 
-@subsubheading @value{GDBN} Command
-
-The equivalent @value{GDBN} command is @samp{info threads}.
-
-@subsubheading Example
-N.A.
-
-
 @subheading The @code{-thread-list-ids} Command
 @findex -thread-list-ids
 
diff --git a/gdb/gdb.h b/gdb/gdb.h
index fcd3e3b..13523fb 100644
--- a/gdb/gdb.h
+++ b/gdb/gdb.h
@@ -55,4 +55,6 @@ enum gdb_rc gdb_thread_select (struct ui_out *uiout, char *tidstr,
 enum gdb_rc gdb_list_thread_ids (struct ui_out *uiout,
 				 char **error_message);
 
+extern void mi_info_threads (struct ui_out *uiout, int thread);
+
 #endif
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index c651694..89c6376 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -130,8 +130,7 @@ struct mi_cmd mi_cmds[] =
   { "target-list-current-targets", { NULL, 0 }, NULL, NULL },
   { "target-list-parameters", { NULL, 0 }, NULL, NULL },
   { "target-select", { NULL, 0 }, mi_cmd_target_select},
-  { "thread-info", { NULL, 0 }, NULL, NULL },
-  { "thread-list-all-threads", { NULL, 0 }, NULL, NULL },
+  { "thread-info", { NULL, 0 }, NULL, mi_cmd_thread_info },
   { "thread-list-ids", { NULL, 0 }, 0, mi_cmd_thread_list_ids},
   { "thread-select", { NULL, 0 }, 0, mi_cmd_thread_select},
   { "trace-actions", { NULL, 0 }, NULL, NULL },
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 2d0393b..6a033e5 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -105,6 +105,7 @@ extern mi_cmd_argv_ftype mi_cmd_target_file_get;
 extern mi_cmd_argv_ftype mi_cmd_target_file_put;
 extern mi_cmd_argv_ftype mi_cmd_target_file_delete;
 extern mi_cmd_args_ftype mi_cmd_target_select;
+extern mi_cmd_argv_ftype mi_cmd_thread_info;
 extern mi_cmd_argv_ftype mi_cmd_thread_list_ids;
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
 extern mi_cmd_argv_ftype mi_cmd_var_assign;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 41e12d2..5d89847 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -277,6 +277,24 @@ mi_cmd_thread_list_ids (char *command, char **argv, int argc)
 }
 
 enum mi_cmd_result
+mi_cmd_thread_info (char *command, char **argv, int argc)
+{
+  int thread = -1;
+  
+  if (argc != 0 && argc != 1)
+    {
+      mi_error_message = xstrprintf ("Invalid MI command");
+      return MI_CMD_ERROR;
+    }
+
+  if (argc == 1)
+    thread = atoi (argv[0]);
+
+  mi_info_threads (uiout, thread);
+  return MI_CMD_DONE;
+}
+
+enum mi_cmd_result
 mi_cmd_data_list_register_names (char *command, char **argv, int argc)
 {
   int regnum, numregs;
@@ -1055,6 +1073,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
 
       ui_out_field_string (uiout, NULL, "frozen-varobjs");
       ui_out_field_string (uiout, NULL, "pending-breakpoints");
+      ui_out_field_string (uiout, NULL, "thread-info");
       
       do_cleanups (cleanup);
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 3a39703..1f4b099 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -416,15 +416,14 @@ prune_threads (void)
     }
 }
 
-/* Print information about currently known threads 
-
- * Note: this has the drawback that it _really_ switches
- *       threads, which frees the frame cache.  A no-side
- *       effects info-threads command would be nicer.
- */
-
-static void
-info_threads_command (char *arg, int from_tty)
+/* Prints the list of threads and their details on UIOUT.
+   This is a version of 'info_thread_command' suitable for
+   use from MI.  
+   If REQESTED_THREAD is not -1, it's the GDB id of the thread
+   that should be printed.  Otherwise, all threads are
+   printed.  */
+void
+mi_info_threads (struct ui_out *uiout, int requested_thread)
 {
   struct thread_info *tp;
   ptid_t current_ptid;
@@ -432,45 +431,87 @@ info_threads_command (char *arg, int from_tty)
   struct cleanup *old_chain;
   struct frame_id saved_frame_id;
   char *extra_info;
+  int current_thread = -1;
 
   /* Backup current thread and selected frame.  */
   saved_frame_id = get_frame_id (get_selected_frame (NULL));
   old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
 
+  old_chain = make_cleanup_ui_out_list_begin_end (uiout, "threads");
+
   prune_threads ();
   target_find_new_threads ();
   current_ptid = inferior_ptid;
   for (tp = thread_list; tp; tp = tp->next)
     {
+      struct cleanup *chain2;
+
+      if (requested_thread != -1 && tp->num != requested_thread)
+	continue;
+
+      chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+
       if (ptid_equal (tp->ptid, current_ptid))
-	printf_filtered ("* ");
+	{
+	  current_thread = tp->num;
+	  ui_out_text (uiout, "* ");
+	}
       else
-	printf_filtered ("  ");
+	ui_out_text (uiout, "  ");
 
-      printf_filtered ("%d %s", tp->num, target_tid_to_str (tp->ptid));
+      ui_out_field_int (uiout, "id", tp->num);
+      ui_out_text (uiout, " ");
+      ui_out_field_string (uiout, "target-id", target_tid_to_str (tp->ptid));
 
       extra_info = target_extra_thread_info (tp);
       if (extra_info)
-	printf_filtered (" (%s)", extra_info);
-      puts_filtered ("  ");
+	ui_out_field_fmt (uiout, "details", " (%s)", extra_info);
+      ui_out_text (uiout, "  ");
       /* That switch put us at the top of the stack (leaf frame).  */
       switch_to_thread (tp->ptid);
       print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
+
+      do_cleanups (chain2);
     }
 
   /* Restores the current thread and the frame selected before
      the "info threads" command.  */
   do_cleanups (old_chain);
 
+  if (requested_thread == -1)
+    {
+      gdb_assert (current_thread != -1);
+      if (ui_out_is_mi_like_p (uiout))
+	ui_out_field_int (uiout, "current-thread-id", current_thread);
+    }
+
   /*  If case we were not able to find the original frame, print the
       new selected frame.  */
   if (frame_find_by_id (saved_frame_id) == NULL)
     {
       warning (_("Couldn't restore frame in current thread, at frame 0"));
-      print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
+      /* For MI, we should probably have a notification about
+	 current frame change.  But this error is not very likely, so
+	 don't bother for now.  */
+      if (!ui_out_is_mi_like_p (uiout))
+	print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
     }
 }
 
+
+/* Print information about currently known threads 
+
+ * Note: this has the drawback that it _really_ switches
+ *       threads, which frees the frame cache.  A no-side
+ *       effects info-threads command would be nicer.
+ */
+
+static void
+info_threads_command (char *arg, int from_tty)
+{
+  mi_info_threads (uiout, -1);
+}
+
 /* Switch from one thread to another. */
 
 void
-- 
1.5.3.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]