From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22608 invoked by alias); 20 Feb 2008 11:28:12 -0000 Received: (qmail 22596 invoked by uid 22791); 20 Feb 2008 11:28:09 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 20 Feb 2008 11:27:41 +0000 Received: (qmail 21212 invoked from network); 20 Feb 2008 11:27:37 -0000 Received: from unknown (HELO localhost) (vladimir@127.0.0.2) by mail.codesourcery.com with ESMTPA; 20 Feb 2008 11:27:37 -0000 From: Vladimir Prus To: Nick Roberts Subject: Re: [RFA] Implement -thread-info. Date: Wed, 20 Feb 2008 11:28:00 -0000 User-Agent: KMail/1.9.6 (enterprise 0.20070907.709405) Cc: gdb-patches@sources.redhat.com, Eli Zaretskii References: <200802171833.26673.vladimir@codesourcery.com> <200802181038.04497.vladimir@codesourcery.com> <18361.64577.771676.184618@kahikatea.snap.net.nz> In-Reply-To: <18361.64577.771676.184618@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_o6AvHzXEzctz808" Message-Id: <200802201427.36288.vladimir@codesourcery.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-02/txt/msg00324.txt.bz2 --Boundary-00=_o6AvHzXEzctz808 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 3712 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. --Boundary-00=_o6AvHzXEzctz808 Content-Type: text/x-diff; charset="iso-8859-1"; name="0001-Implement-thread-info.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-Implement-thread-info.patch" Content-length: 9159 =46rom b194235c305682ab6e4176df70dc52533e8b2e07 Mon Sep 17 00:00:00 2001 From: Vladimir Prus 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{p= wd}. @subsubheading Synopsis =20 @smallexample - -thread-info + -thread-info [ @var{thread-id} ] @end smallexample =20 +Reports the information about either a specific thread, if=20 +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 =20 -No equivalent. +The @samp{info thread} command prints the same information +about all threads. =20 @subsubheading Example -N.A. - - -@subheading The @code{-thread-list-all-threads} Command -@findex -thread-list-all-threads - -@subsubheading Synopsis =20 @smallexample - -thread-list-all-threads +-thread-info +^done,threads=3D[ +@{id=3D"2",target-id=3D"Thread 0xb7e14b90 (LWP 21257)", + frame=3D@{addr=3D"0xffffe410",func=3D"__kernel_vsyscall",args=3D[]@}, +@{id=3D"1",target-id=3D"Thread 0xb7e156b0 (LWP 21254)", + frame=3D@{addr=3D"0x0804891f",func=3D"foo",args=3D[@{name=3D"i",value= =3D"10"@}], + file=3D"/tmp/a.c",fullname=3D"/tmp/a.c",line=3D"158"@}@}], +current-thread-id=3D"1" +(gdb) @end smallexample =20 -@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 =20 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); =20 +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[] =3D { "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, in= t argc) } =20 enum mi_cmd_result +mi_cmd_thread_info (char *command, char **argv, int argc) +{ + int thread =3D -1; +=20=20 + if (argc !=3D 0 && argc !=3D 1) + { + mi_error_message =3D xstrprintf ("Invalid MI command"); + return MI_CMD_ERROR; + } + + if (argc =3D=3D 1) + thread =3D 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) =20 ui_out_field_string (uiout, NULL, "frozen-varobjs"); ui_out_field_string (uiout, NULL, "pending-breakpoints"); + ui_out_field_string (uiout, NULL, "thread-info"); =20=20=20=20=20=20=20 do_cleanups (cleanup); =20 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) } } =20 -/* Print information about currently known threads=20 - - * 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.=20=20 + 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 =3D -1; =20 /* Backup current thread and selected frame. */ saved_frame_id =3D get_frame_id (get_selected_frame (NULL)); old_chain =3D make_cleanup_restore_current_thread (inferior_ptid, saved_= frame_id); =20 + old_chain =3D make_cleanup_ui_out_list_begin_end (uiout, "threads"); + prune_threads (); target_find_new_threads (); current_ptid =3D inferior_ptid; for (tp =3D thread_list; tp; tp =3D tp->next) { + struct cleanup *chain2; + + if (requested_thread !=3D -1 && tp->num !=3D requested_thread) + continue; + + chain2 =3D make_cleanup_ui_out_tuple_begin_end (uiout, NULL); + if (ptid_equal (tp->ptid, current_ptid)) - printf_filtered ("* "); + { + current_thread =3D tp->num; + ui_out_text (uiout, "* "); + } else - printf_filtered (" "); + ui_out_text (uiout, " "); =20 - 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= )); =20 extra_info =3D 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); } =20 /* Restores the current thread and the frame selected before the "info threads" command. */ do_cleanups (old_chain); =20 + if (requested_thread =3D=3D -1) + { + gdb_assert (current_thread !=3D -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) =3D=3D 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); } } =20 + +/* Print information about currently known threads=20 + + * 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. */ =20 void --=20 1.5.3.5 --Boundary-00=_o6AvHzXEzctz808--