From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16538 invoked by alias); 2 Feb 2013 08:29:25 -0000 Received: (qmail 16525 invoked by uid 22791); 2 Feb 2013 08:29:23 -0000 X-SWARE-Spam-Status: No, hits=-4.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 02 Feb 2013 08:29:18 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1U1YTM-0005Bq-Ks from Yao_Qi@mentor.com ; Sat, 02 Feb 2013 00:29:16 -0800 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Sat, 2 Feb 2013 00:29:16 -0800 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Sat, 2 Feb 2013 00:29:14 -0800 Message-ID: <510CCE26.50600@codesourcery.com> Date: Sat, 02 Feb 2013 08:29:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Pedro Alves CC: Subject: Re: [PATCH] New MI notification "=tsv-modified" References: <1359560580-1970-1-git-send-email-yao@codesourcery.com> <510C27CE.3090102@redhat.com> In-Reply-To: <510C27CE.3090102@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit 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: 2013-02/txt/msg00042.txt.bz2 On 02/02/2013 04:38 AM, Pedro Alves wrote: > Should we emit a notification when GDB detects the target > changes the variable too? > Yes, we should, but I am still evaluating the pros vs. cons. of using async remote notification on tsv changes in the target or let GDB to check the tsv changes. I need more time to get the answer, but in parallel, I'd like the "=tsv-modified" notification goes in to notify MI front-end that a tsv is modified by command. >> >2013-01-30 Yao Qi >> > >> > * mi/mi-interp.c (mi_tsv_modified): Declare. >> > (mi_interpreter_init): Call observer_attach_tsv_modified. >> > (mi_tsv_modified): New. >> > * tracepoint.c (trace_variable_command): Call >> > observer_notify_tsv_modified if the initial value of tsv is >> > changed. >> > >> > * NEWS: Mention the new MI notification. > Please spell out the notification. Nothing else in the GDB > ChangeLog entry indicates what is "the new". > because "what is the new" is added into the changelog entry in gdb/doc/ChangeLog, don't have to replicate it again in gdb/ChangeLog to mention it again for NEWS. >> > >> >gdb/testsuite: >> > >> >2013-01-30 Yao Qi >> > >> > * gdb.trace/mi-tsv-changed.exp (test_create_delete_tsv): Rename >> > to ... >> > (test_create_delete_modify_tsv): New. > It's a rename, or a new function? > It is a rename, from test_create_delete_tsv to test_create_delete_modify_tsv, which is new (in terms of name, at least). >> >@@ -37,6 +37,9 @@ proc test_create_delete_tsv { } {with_test_prefix "create delete" { >> > mi_gdb_test "tvariable \$tvar1" \ >> > ".*=tsv-created,name=\"tvar1\",value=\"0\"\\\\n.*\\^done" \ >> > "tvariable \$tvar1" >> >+ mi_gdb_test "tvariable \$tvar1 = 1" \ >> >+ ".*=tsv-modified,name=\"tvar1\",value=\"1\"\\\\n.*\\^done" \ >> >+ "tvariable \$tvar1 modified" > It would be thorough to test that another "tvariable \$tvar1 = 1" > _doesn't_ emit a modified notification. > Added. >> > mi_gdb_test "tvariable \$tvar2 = 45" \ >> > ".*=tsv-created,name=\"tvar2\",value=\"45\"\\\\n.*\\^done" \ >> > "tvariable \$tvar2" >> >@@ -81,6 +84,10 @@ proc test_upload_tsv { } { with_test_prefix "upload" { >> > gdb_test "tvariable \$tvar2 = 45" \ >> > "Trace state variable \\\$tvar2 created, with initial value 45." \ >> > "Create a trace state variable with initial value" >> >+ >> >+ gdb_test "tvariable \$tvar3 = 1" \ >> >+ "Trace state variable \\\$tvar3 created, with initial value 1." \ >> >+ "Create a trace state variable \$tvar3 with initial value 1" > I couldn't figure out what motivated this addition. It was used to test "=tsv-modified" happened when GDB reconnects to the stub. This chunk should be removed. Here is the new version below, to address your comments. I don't insist on my original changelog entry, as either is OK to me. -- Yao (齐尧) gdb/doc: 2013-02-02 Yao Qi * gdb.texinfo (GDB/MI Async Records): Document new MI notification "=tsv-modified". * observer.texi (GDB Observers): New observer tsv_modified. gdb: 2013-02-02 Yao Qi * mi/mi-interp.c (mi_tsv_modified): Declare. (mi_interpreter_init): Call observer_attach_tsv_modified. (mi_tsv_modified): New. * tracepoint.c (trace_variable_command): Call observer_notify_tsv_modified if the initial value of tsv is changed. * NEWS: Mention the new MI notification "=tsv-modified". gdb/testsuite: 2013-02-02 Yao Qi * gdb.trace/mi-tsv-changed.exp (test_create_delete_tsv): Rename to ... (test_create_delete_modify_tsv): ... here. New test on modifying the initial value of a tsv. --- gdb/NEWS | 5 +++-- gdb/doc/gdb.texinfo | 4 ++++ gdb/doc/observer.texi | 5 +++++ gdb/mi/mi-interp.c | 18 ++++++++++++++++++ gdb/testsuite/gdb.trace/mi-tsv-changed.exp | 15 ++++++++++++--- gdb/tracepoint.c | 6 +++++- 6 files changed, 47 insertions(+), 6 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 539ceb9..0e01deb 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -121,8 +121,9 @@ show print type typedefs "=cmd-param-changed". ** Trace frame changes caused by command "tfind" are now notified using new async record "=traceframe-changed". - ** The creation and deletion of trace state variables are now notified - using new async records "=tsv-created" and "=tsv-deleted". + ** The creation, deletion and modification of trace state variables + are now notified using new async records "=tsv-created", + "=tsv-deleted" and "=tsv-modified". ** The start and stop of process record are now notified using new async record "=record-started" and "=record-stopped". ** Memory changes are now notified using new async record diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 4b51228..b6b0439 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -27954,6 +27954,10 @@ value @var{value}. Reports that the trace state variable @var{name} is deleted or all trace state variables are deleted. +@item =tsv-modified,name=@var{name},value=@var{value} +Reports that the trace state variable @var{name} is modified with +value @var{value}. + @item =breakpoint-created,bkpt=@{...@} @itemx =breakpoint-modified,bkpt=@{...@} @itemx =breakpoint-deleted,id=@var{number} diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi index 87c08e1..11fdc87 100644 --- a/gdb/doc/observer.texi +++ b/gdb/doc/observer.texi @@ -246,6 +246,11 @@ The trace state variable @var{name} is deleted. If @var{name} is @code{NULL}, all trace state variables are deleted. @end deftypefun +@deftypefun void tsv_modified (const char *@var{name}, LONGEST @var{value}) +The trace state value @var{name} is modified with value +@var{value}. +@end deftypefun + @deftypefun void test_notification (int @var{somearg}) This observer is used for internal testing. Do not use. See testsuite/gdb.gdb/observer.exp. diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 6617647..c8ec3f1 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -73,6 +73,7 @@ static void mi_about_to_proceed (void); static void mi_traceframe_changed (int tfnum, int tpnum); static void mi_tsv_created (const char *name, LONGEST value); static void mi_tsv_deleted (const char *name); +static void mi_tsv_modified (const char *name, LONGEST value); static void mi_breakpoint_created (struct breakpoint *b); static void mi_breakpoint_deleted (struct breakpoint *b); static void mi_breakpoint_modified (struct breakpoint *b); @@ -137,6 +138,7 @@ mi_interpreter_init (struct interp *interp, int top_level) observer_attach_traceframe_changed (mi_traceframe_changed); observer_attach_tsv_created (mi_tsv_created); observer_attach_tsv_deleted (mi_tsv_deleted); + observer_attach_tsv_modified (mi_tsv_modified); observer_attach_breakpoint_created (mi_breakpoint_created); observer_attach_breakpoint_deleted (mi_breakpoint_deleted); observer_attach_breakpoint_modified (mi_breakpoint_modified); @@ -594,6 +596,22 @@ mi_tsv_deleted (const char *name) gdb_flush (mi->event_channel); } +/* Emit notification on modifying a trace state variable. */ + +static void +mi_tsv_modified (const char *name, LONGEST value) +{ + struct mi_interp *mi = top_level_interpreter_data (); + + target_terminal_ours (); + + fprintf_unfiltered (mi->event_channel, "tsv-modified," + "name=\"%s\",value=\"%s\"\n", + name, plongest (value)); + + gdb_flush (mi->event_channel); +} + /* Emit notification about a created breakpoint. */ static void diff --git a/gdb/testsuite/gdb.trace/mi-tsv-changed.exp b/gdb/testsuite/gdb.trace/mi-tsv-changed.exp index 4e7d5a4..9ab15ce 100644 --- a/gdb/testsuite/gdb.trace/mi-tsv-changed.exp +++ b/gdb/testsuite/gdb.trace/mi-tsv-changed.exp @@ -23,9 +23,9 @@ if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \ return -1 } -# Test notifications on creating and deleting TSV. +# Test notifications on creating, deleting and modifying TSV. -proc test_create_delete_tsv { } {with_test_prefix "create delete" { +proc test_create_delete_modify_tsv { } {with_test_prefix "create delete modify" { global binfile global decimal @@ -37,6 +37,14 @@ proc test_create_delete_tsv { } {with_test_prefix "create delete" { mi_gdb_test "tvariable \$tvar1" \ ".*=tsv-created,name=\"tvar1\",value=\"0\"\\\\n.*\\^done" \ "tvariable \$tvar1" + mi_gdb_test "tvariable \$tvar1 = 1" \ + ".*=tsv-modified,name=\"tvar1\",value=\"1\"\\\\n.*\\^done" \ + "tvariable \$tvar1 modified" + # No "=tsv-modified" notification is emitted, because the initial + # value is not changed. + mi_gdb_test "tvariable \$tvar1 = 1" \ + ".*\\\$tvar1 = 1\\\\n\"\r\n~\"Trace state .*\\\\n.*\\^done" \ + "tvariable \$tvar1 modified without notification" mi_gdb_test "tvariable \$tvar2 = 45" \ ".*=tsv-created,name=\"tvar2\",value=\"45\"\\\\n.*\\^done" \ "tvariable \$tvar2" @@ -81,6 +89,7 @@ proc test_upload_tsv { } { with_test_prefix "upload" { gdb_test "tvariable \$tvar2 = 45" \ "Trace state variable \\\$tvar2 created, with initial value 45." \ "Create a trace state variable with initial value" + # Define a tracepoint otherwise tracing cannot be started. gdb_test "trace main" "Tracepoint $decimal at .*" gdb_test_no_output "tstart" "start trace experiment" @@ -148,7 +157,7 @@ proc test_upload_tsv { } { with_test_prefix "upload" { set gdbserver_reconnect_p 0 }} - test_create_delete_tsv + test_create_delete_modify_tsv # Test target supports tracepoints or not. diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index be45cb4..8345ac6 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -408,7 +408,11 @@ trace_variable_command (char *args, int from_tty) tsv = find_trace_state_variable (internalvar_name (intvar)); if (tsv) { - tsv->initial_value = initval; + if (tsv->initial_value != initval) + { + tsv->initial_value = initval; + observer_notify_tsv_modified (tsv->name, initval); + } printf_filtered (_("Trace state variable $%s " "now has initial value %s.\n"), tsv->name, plongest (tsv->initial_value)); -- 1.7.7.6