From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 689 invoked by alias); 12 Nov 2008 21:23:34 -0000 Received: (qmail 585 invoked by uid 22791); 12 Nov 2008 21:23:31 -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, 12 Nov 2008 21:22:51 +0000 Received: (qmail 23523 invoked from network); 12 Nov 2008 21:22:48 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Nov 2008 21:22:48 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] Implement =thread-selected notification. Date: Thu, 13 Nov 2008 08:54:00 -0000 User-Agent: KMail/1.9.10 Cc: Vladimir Prus , gdb-patches@sources.redhat.com References: <200811122343.24950.vladimir@codesourcery.com> In-Reply-To: <200811122343.24950.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200811122122.54320.pedro@codesourcery.com> X-IsSubscribed: yes 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-11/txt/msg00257.txt.bz2 On Wednesday 12 November 2008 20:43:24, Vladimir Prus wrote: > > +const char * > +top_level_interpreter_name (void) > +{ > + gdb_assert (top_level_interpreter_ptr); > + return top_level_interpreter_ptr->name; > +} > + > /* This just adds the "interpreter-exec" command. */ > void > _initialize_interpreter (void) > diff --git a/gdb/interps.h b/gdb/interps.h > index 353ed0a..e6aa57a 100644 > --- a/gdb/interps.h > +++ b/gdb/interps.h > @@ -67,6 +67,7 @@ extern void current_interp_command_loop (void); > /* Returns opaque data associated with the top-level interpreter. */ > extern void *top_level_interpreter_data (void); > extern struct interp *top_level_interpreter (void); > +extern const char *top_level_interpreter_name (void); > > extern void clear_interpreter_hooks (void); > > diff --git a/gdb/mi/mi-common.h b/gdb/mi/mi-common.h > index e47afd1..8778e74 100644 > --- a/gdb/mi/mi-common.h > +++ b/gdb/mi/mi-common.h > @@ -41,4 +41,19 @@ enum async_reply_reason > > const char *async_reason_lookup (enum async_reply_reason reason); > > +struct mi_interp > +{ > + /* MI's output channels */ > + struct ui_file *out; > + struct ui_file *err; > + struct ui_file *log; > + struct ui_file *targ; > + struct ui_file *event_channel; > + > + /* This is the interpreter for the mi... */ > + struct interp *mi2_interp; > + struct interp *mi1_interp; > + struct interp *mi_interp; > +}; > + > #endif > diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c > index e05b1ad..5aa0e6c 100644 > --- a/gdb/mi/mi-interp.c > +++ b/gdb/mi/mi-interp.c > @@ -31,24 +31,10 @@ > #include "mi-cmds.h" > #include "mi-out.h" > #include "mi-console.h" > +#include "mi-common.h" > #include "observer.h" > #include "gdbthread.h" > > -struct mi_interp > -{ > - /* MI's output channels */ > - struct ui_file *out; > - struct ui_file *err; > - struct ui_file *log; > - struct ui_file *targ; > - struct ui_file *event_channel; > - > - /* This is the interpreter for the mi... */ > - struct interp *mi2_interp; > - struct interp *mi1_interp; > - struct interp *mi_interp; > -}; > - > /* These are the interpreter setup, etc. functions for the MI interpreter */ > static void mi_execute_command_wrapper (char *cmd); > static void mi_command_loop (int mi_version); > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index bd149ef..a205395 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -44,6 +44,7 @@ > #include "gdb.h" > #include "frame.h" > #include "mi-main.h" > +#include "mi-common.h" > #include "language.h" > #include "valprint.h" > #include "inferior.h" > @@ -1167,6 +1168,9 @@ mi_execute_command (char *cmd, int from_tty) > if (command != NULL) > { > struct gdb_exception result; > + ptid_t previous_ptid = inferior_ptid; > + int had_execution = target_has_execution; > + const char *top_name; > > if (do_timings) > { > @@ -1191,6 +1195,32 @@ mi_execute_command (char *cmd, int from_tty) > mi_out_rewind (uiout); > } > > + /* The notifications are only output when the top-level > + interpreter (specified on the command line) is MI. */ > + top_name = top_level_interpreter_name (); > + if (had_execution && target_has_execution > + && strstr (top_name, "mi") == top_name) Why not, ui_out_is_mi_like_p (interp_ui_out (top_level_interpreter ()))) instead of relying on the interpreter name? > + { > + struct mi_interp *mi = top_level_interpreter_data (); > + if (!ptid_equal (previous_ptid, null_ptid) > + && !ptid_equal (inferior_ptid, previous_ptid)) > + { Checking for target_has_execution means that these new notifications will not be emitted when using the "thread" command while debugging a multi-threaded core file. > + /* If there are no threads in thread table, we cannot report > + anything. */ > + if (strcmp (command->command, "thread-select") != 0 > + && thread_count () != 0) > + { > + struct thread_info *ti = find_thread_pid (inferior_ptid); > + gdb_assert (ti); These two lines can be replaced by 'struct thread_info *t = inferior_thread ()'. > + target_terminal_ours (); > + fprintf_unfiltered (mi->event_channel, > + "thread-selected,id=\"%d\"", > + ti->num); > + gdb_flush (mi->event_channel); > + } > + } > + } > + > mi_parse_free (command); > } > > diff --git a/gdb/testsuite/gdb.mi/mi-pthreads.exp b/gdb/testsuite/gdb.mi/mi-pthreads.exp > index dbb3ece..0ab5715 100644 > --- a/gdb/testsuite/gdb.mi/mi-pthreads.exp > +++ b/gdb/testsuite/gdb.mi/mi-pthreads.exp > @@ -55,6 +55,12 @@ proc check_mi_thread_command_set {} { > "\\^done,new-thread-id=\"$thread\",frame={.*}(,line=\"(-)?\[0-9\]+\",file=\".*\")?" \ > "check_mi_thread_command_set: -thread-select $thread" > } > + > + foreach thread $thread_list { > + mi_gdb_test "-interpreter-exec console \"thread $thread\"" \ > + ".*\\^done\r\n=thread-selected,id=\"$thread\"" \ > + "check =thread-selected: thread $thread" > + } > } > > # > diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > index 9ef9df9..f29ecd4 100644 > --- a/gdb/testsuite/lib/mi-support.exp > +++ b/gdb/testsuite/lib/mi-support.exp > @@ -816,7 +816,7 @@ proc mi_run_cmd {args} { > > send_gdb "220-exec-run $args\n" > gdb_expect { > - -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n|=thread-created,id=\"1\"\r\n)*${mi_gdb_prompt}" { > + -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n|=thread-created,id=\"1\"\r\n)*(=thread-selected,id=\".*\"\r\n)?${mi_gdb_prompt}" { ^ Could you please not use .* in the mi support files? We've recently been through removing those for non-stop, because they're racy --- they'll often times eat too much, and it's a good practice to not do it from the start. > } > timeout { > perror "Unable to start target" > @@ -1014,9 +1014,9 @@ proc mi_expect_stop { reason func args file line extra test } { > > set any "\[^\n\]*" > > - verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}\r\n$after_stopped$prompt_re" > + verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}\r\n$after_stopped(=thread-selected,id=\".*\"\r\n)?$prompt_re" > gdb_expect { > - -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re" { > + -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n(=thread-selected,id=\".*\"\r\n)?$prompt_re" { > pass "$test" > return $expect_out(2,string) > } > @@ -1433,7 +1433,7 @@ proc mi_send_resuming_command_raw {command test} { > > send_gdb "$command\n" > gdb_expect { > - -re "\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" { > + -re "\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n(=thread-selected,id=\".*\"\r\n)?${mi_gdb_prompt}" { ^ Same here. > # Note that lack of 'pass' call here -- this works around limitation > # in DejaGNU xfail mechanism. mi-until.exp has this: > # > @@ -1491,7 +1491,7 @@ proc mi_get_stop_line {test} { > } > > gdb_expect { > - -re ".*line=\"(\[0-9\]*)\".*\r\n$prompt_re" { > + -re ".*line=\"(\[0-9\]*)\".*\r\n(=thread-selected,id=\".*\"\r\n)?$prompt_re" { ^ Same here. > return $expect_out(1,string) > } > -re ".*$mi_gdb_prompt" { -- Pedro Alves From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 716 invoked by alias); 12 Nov 2008 21:23:34 -0000 Received: (qmail 582 invoked by uid 22791); 12 Nov 2008 21:23:31 -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, 12 Nov 2008 21:22:51 +0000 Received: (qmail 23523 invoked from network); 12 Nov 2008 21:22:48 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Nov 2008 21:22:48 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC] Implement =thread-selected notification. Date: Thu, 13 Nov 2008 11:26:00 -0000 User-Agent: KMail/1.9.10 Cc: Vladimir Prus , gdb-patches@sources.redhat.com References: <200811122343.24950.vladimir@codesourcery.com> In-Reply-To: <200811122343.24950.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-ID: <200811122122.54320.pedro@codesourcery.com> X-IsSubscribed: yes 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-11/txt/msg00258.txt.bz2 Message-ID: <20081113112600.ADYGI7HBaRmwIVsBCu4mCM-sCe2INQb43iSJYoknHDs@z> On Wednesday 12 November 2008 20:43:24, Vladimir Prus wrote: > > +const char * > +top_level_interpreter_name (void) > +{ > + gdb_assert (top_level_interpreter_ptr); > + return top_level_interpreter_ptr->name; > +} > + > /* This just adds the "interpreter-exec" command. */ > void > _initialize_interpreter (void) > diff --git a/gdb/interps.h b/gdb/interps.h > index 353ed0a..e6aa57a 100644 > --- a/gdb/interps.h > +++ b/gdb/interps.h > @@ -67,6 +67,7 @@ extern void current_interp_command_loop (void); > /* Returns opaque data associated with the top-level interpreter. */ > extern void *top_level_interpreter_data (void); > extern struct interp *top_level_interpreter (void); > +extern const char *top_level_interpreter_name (void); > > extern void clear_interpreter_hooks (void); > > diff --git a/gdb/mi/mi-common.h b/gdb/mi/mi-common.h > index e47afd1..8778e74 100644 > --- a/gdb/mi/mi-common.h > +++ b/gdb/mi/mi-common.h > @@ -41,4 +41,19 @@ enum async_reply_reason > > const char *async_reason_lookup (enum async_reply_reason reason); > > +struct mi_interp > +{ > + /* MI's output channels */ > + struct ui_file *out; > + struct ui_file *err; > + struct ui_file *log; > + struct ui_file *targ; > + struct ui_file *event_channel; > + > + /* This is the interpreter for the mi... */ > + struct interp *mi2_interp; > + struct interp *mi1_interp; > + struct interp *mi_interp; > +}; > + > #endif > diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c > index e05b1ad..5aa0e6c 100644 > --- a/gdb/mi/mi-interp.c > +++ b/gdb/mi/mi-interp.c > @@ -31,24 +31,10 @@ > #include "mi-cmds.h" > #include "mi-out.h" > #include "mi-console.h" > +#include "mi-common.h" > #include "observer.h" > #include "gdbthread.h" > > -struct mi_interp > -{ > - /* MI's output channels */ > - struct ui_file *out; > - struct ui_file *err; > - struct ui_file *log; > - struct ui_file *targ; > - struct ui_file *event_channel; > - > - /* This is the interpreter for the mi... */ > - struct interp *mi2_interp; > - struct interp *mi1_interp; > - struct interp *mi_interp; > -}; > - > /* These are the interpreter setup, etc. functions for the MI interpreter */ > static void mi_execute_command_wrapper (char *cmd); > static void mi_command_loop (int mi_version); > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index bd149ef..a205395 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -44,6 +44,7 @@ > #include "gdb.h" > #include "frame.h" > #include "mi-main.h" > +#include "mi-common.h" > #include "language.h" > #include "valprint.h" > #include "inferior.h" > @@ -1167,6 +1168,9 @@ mi_execute_command (char *cmd, int from_tty) > if (command != NULL) > { > struct gdb_exception result; > + ptid_t previous_ptid = inferior_ptid; > + int had_execution = target_has_execution; > + const char *top_name; > > if (do_timings) > { > @@ -1191,6 +1195,32 @@ mi_execute_command (char *cmd, int from_tty) > mi_out_rewind (uiout); > } > > + /* The notifications are only output when the top-level > + interpreter (specified on the command line) is MI. */ > + top_name = top_level_interpreter_name (); > + if (had_execution && target_has_execution > + && strstr (top_name, "mi") == top_name) Why not, ui_out_is_mi_like_p (interp_ui_out (top_level_interpreter ()))) instead of relying on the interpreter name? > + { > + struct mi_interp *mi = top_level_interpreter_data (); > + if (!ptid_equal (previous_ptid, null_ptid) > + && !ptid_equal (inferior_ptid, previous_ptid)) > + { Checking for target_has_execution means that these new notifications will not be emitted when using the "thread" command while debugging a multi-threaded core file. > + /* If there are no threads in thread table, we cannot report > + anything. */ > + if (strcmp (command->command, "thread-select") != 0 > + && thread_count () != 0) > + { > + struct thread_info *ti = find_thread_pid (inferior_ptid); > + gdb_assert (ti); These two lines can be replaced by 'struct thread_info *t = inferior_thread ()'. > + target_terminal_ours (); > + fprintf_unfiltered (mi->event_channel, > + "thread-selected,id=\"%d\"", > + ti->num); > + gdb_flush (mi->event_channel); > + } > + } > + } > + > mi_parse_free (command); > } > > diff --git a/gdb/testsuite/gdb.mi/mi-pthreads.exp b/gdb/testsuite/gdb.mi/mi-pthreads.exp > index dbb3ece..0ab5715 100644 > --- a/gdb/testsuite/gdb.mi/mi-pthreads.exp > +++ b/gdb/testsuite/gdb.mi/mi-pthreads.exp > @@ -55,6 +55,12 @@ proc check_mi_thread_command_set {} { > "\\^done,new-thread-id=\"$thread\",frame={.*}(,line=\"(-)?\[0-9\]+\",file=\".*\")?" \ > "check_mi_thread_command_set: -thread-select $thread" > } > + > + foreach thread $thread_list { > + mi_gdb_test "-interpreter-exec console \"thread $thread\"" \ > + ".*\\^done\r\n=thread-selected,id=\"$thread\"" \ > + "check =thread-selected: thread $thread" > + } > } > > # > diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > index 9ef9df9..f29ecd4 100644 > --- a/gdb/testsuite/lib/mi-support.exp > +++ b/gdb/testsuite/lib/mi-support.exp > @@ -816,7 +816,7 @@ proc mi_run_cmd {args} { > > send_gdb "220-exec-run $args\n" > gdb_expect { > - -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n|=thread-created,id=\"1\"\r\n)*${mi_gdb_prompt}" { > + -re "220\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n|=thread-created,id=\"1\"\r\n)*(=thread-selected,id=\".*\"\r\n)?${mi_gdb_prompt}" { ^ Could you please not use .* in the mi support files? We've recently been through removing those for non-stop, because they're racy --- they'll often times eat too much, and it's a good practice to not do it from the start. > } > timeout { > perror "Unable to start target" > @@ -1014,9 +1014,9 @@ proc mi_expect_stop { reason func args file line extra test } { > > set any "\[^\n\]*" > > - verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}\r\n$after_stopped$prompt_re" > + verbose -log "mi_expect_stop: expecting: \\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}\r\n$after_stopped(=thread-selected,id=\".*\"\r\n)?$prompt_re" > gdb_expect { > - -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re" { > + -re "\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",stopped-threads=$any,frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\"$any$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n(=thread-selected,id=\".*\"\r\n)?$prompt_re" { > pass "$test" > return $expect_out(2,string) > } > @@ -1433,7 +1433,7 @@ proc mi_send_resuming_command_raw {command test} { > > send_gdb "$command\n" > gdb_expect { > - -re "\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n${mi_gdb_prompt}" { > + -re "\\^running\r\n\\*running,thread-id=\"\[^\"\]+\"\r\n(=thread-selected,id=\".*\"\r\n)?${mi_gdb_prompt}" { ^ Same here. > # Note that lack of 'pass' call here -- this works around limitation > # in DejaGNU xfail mechanism. mi-until.exp has this: > # > @@ -1491,7 +1491,7 @@ proc mi_get_stop_line {test} { > } > > gdb_expect { > - -re ".*line=\"(\[0-9\]*)\".*\r\n$prompt_re" { > + -re ".*line=\"(\[0-9\]*)\".*\r\n(=thread-selected,id=\".*\"\r\n)?$prompt_re" { ^ Same here. > return $expect_out(1,string) > } > -re ".*$mi_gdb_prompt" { -- Pedro Alves