From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17592 invoked by alias); 25 Jun 2013 13:02:55 -0000 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 Received: (qmail 17577 invoked by uid 89); 25 Jun 2013 13:02:54 -0000 X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 25 Jun 2013 13:02:53 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UrStX-0000a0-Fm from Yao_Qi@mentor.com ; Tue, 25 Jun 2013 06:02:51 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 25 Jun 2013 06:02:51 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.2.247.3; Tue, 25 Jun 2013 06:02:34 -0700 Message-ID: <51C994D7.40603@codesourcery.com> Date: Tue, 25 Jun 2013 13:57:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Pedro Alves CC: Subject: Re: [PATCH] Upload TSVs in extend-remote mode References: <1371636174-9822-1-git-send-email-yao@codesourcery.com> <51C34B4A.7000606@redhat.com> <51C50AAC.70305@codesourcery.com> <51C864CD.5080008@redhat.com> In-Reply-To: <51C864CD.5080008@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-SW-Source: 2013-06/txt/msg00696.txt.bz2 On 06/24/2013 11:25 PM, Pedro Alves wrote: >> + /* Upload TSVs regardless the target is running or not. The remote > ... regardless of whether the ... > >> >+ stub, such as GDBserver, may have some predefined or builtin TSVs, >> >+ even if the target is not running. GDB has to upload them too when >> >+ connects to the remote stub. */ > "when it connects", "when connecting", or better "GDB has to upload them > too in that case". > > But I'd drop that last sentence entirely. Fixed by removing the last sentence. > >> >+ if (remote_get_trace_status (current_trace_status ()) != -1) >> >+ { >> >+ struct uploaded_tsv *uploaded_tsvs = NULL; >> >+ >> >+ remote_upload_trace_state_variables (&uploaded_tsvs); >> >+ merge_uploaded_trace_state_variables (&uploaded_tsvs); >> >+ } >> >+ >> >+# If there are predefined TSVs, test these predefined TSVs are correctly >> >+# uploaded. >> >+if [target_info exists gdb,predefined_tsv] { >> >+ set tsv [target_info gdb,predefined_tsv] >> >+ >> >+ # Restart. >> >+ clean_restart ${binfile} >> >+ >> >+ if ![runto_main] then { >> >+ fail "Can't run to main" >> >+ return >> >+ } >> >+ >> >+ # Test predefined TSVs are uploaded. >> >+ gdb_test "info tvariables" ".*${tsv}.*" "check uploaded tsv" >> >+} > I think there could be an else branch here, that did "info tvariables" > and made sure nothing comes out. Whoever tests against a target that > has builtin tsvs that is not GDBserver, would then notice the FAIL and I agree. I add a test in else branch to make sure no tsvs are uploaded. It reports a fail if I hack GDBserver to remove $trace_timestamp, which is expected. > update the board file. I'd suggest in the same vein, making ".*${tsv}.*" > less lax. Otherwise, I'm afraid this is the sort of board setting that > will just go silently unnoticed. I think about making the regexp less lax, but run out of ideas. Users can set a regexp in gdb,predefined_tsv if their targets has multiple predfined tsvs, for example, there are two predefined tsvs "$foo" and "$bar", and this line can be written in the board file, set_board_info gdb,predefined_tsv "\\\$foo.*\\\$bar" so it is hard to make ".*${tsv}.*" more strict, afaik. > > The patch is okay with or without that change. > Patch below is committed, and we can work on the follow-up to refine the pattern to match. > In the future, we should be thinking of coming up with a new > boards/gdbserver.exp file with these GDBserver specific > settings that native-gdbserver.exp, native-extended-gdbserver.exp, > etc. would source, instead of duplicating these bits across > several boards. Right, that is what I thought when I added a line across three board files. Will post a patch for it. -- Yao (齐尧) gdb: 2013-06-25 Yao Qi * remote.c (remote_start_remote): Move code to upload tsv earlier. gdb/testsuite: 2013-06-25 Yao Qi * boards/native-extended-gdbserver.exp: Set board_info 'gdb,predefined_tsv'. * boards/native-gdbserver.exp: Likewise. * boards/native-stdio-gdbserver.exp: Likewise. * gdb.server/ext-attach.exp: Load trace-support.exp. Check uploaded TSVs if target supports tracing. * gdb.trace/tsv.exp: Check uploaded TSVs if target supports tracing and target has predefined tsv. gdb/doc: 2013-06-25 Yao Qi * gdbint.texinfo (Testsuite): Document 'gdb,predefined_tsv'. --- gdb/doc/gdbint.texinfo | 3 ++ gdb/remote.c | 19 ++++++++++------- gdb/testsuite/boards/native-extended-gdbserver.exp | 3 ++ gdb/testsuite/boards/native-gdbserver.exp | 3 ++ gdb/testsuite/boards/native-stdio-gdbserver.exp | 3 ++ gdb/testsuite/gdb.server/ext-attach.exp | 10 +++++++++ gdb/testsuite/gdb.trace/tsv.exp | 22 ++++++++++++++++++++ 7 files changed, 55 insertions(+), 8 deletions(-) diff --git a/gdb/doc/gdbint.texinfo b/gdb/doc/gdbint.texinfo index 7f1f49f..e7caabe 100644 --- a/gdb/doc/gdbint.texinfo +++ b/gdb/doc/gdbint.texinfo @@ -7983,6 +7983,9 @@ The board does not support type @code{long long}. @c NEED DOCUMENT. @item use_gdb_stub The tests are running with gdb stub. +@item gdb,predefined_tsv +The predefined trace state variables the board has. + @end table @node Hints diff --git a/gdb/remote.c b/gdb/remote.c index be9186b..6354fff 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3452,6 +3452,17 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p) error (_("Remote refused setting all-stop mode with: %s"), rs->buf); } + /* Upload TSVs regardless of whether the target is running or not. The + remote stub, such as GDBserver, may have some predefined or builtin + TSVs, even if the target is not running. */ + if (remote_get_trace_status (current_trace_status ()) != -1) + { + struct uploaded_tsv *uploaded_tsvs = NULL; + + remote_upload_trace_state_variables (&uploaded_tsvs); + merge_uploaded_trace_state_variables (&uploaded_tsvs); + } + /* Check whether the target is running now. */ putpkt ("?"); getpkt (&rs->buf, &rs->buf_size, 0); @@ -3591,18 +3602,10 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p) if (remote_get_trace_status (current_trace_status ()) != -1) { struct uploaded_tp *uploaded_tps = NULL; - struct uploaded_tsv *uploaded_tsvs = NULL; if (current_trace_status ()->running) printf_filtered (_("Trace is already running on the target.\n")); - /* Get trace state variables first, they may be checked when - parsing uploaded commands. */ - - remote_upload_trace_state_variables (&uploaded_tsvs); - - merge_uploaded_trace_state_variables (&uploaded_tsvs); - remote_upload_tracepoints (&uploaded_tps); merge_uploaded_tracepoints (&uploaded_tps); diff --git a/gdb/testsuite/boards/native-extended-gdbserver.exp b/gdb/testsuite/boards/native-extended-gdbserver.exp index bf363c7..e9b2998 100644 --- a/gdb/testsuite/boards/native-extended-gdbserver.exp +++ b/gdb/testsuite/boards/native-extended-gdbserver.exp @@ -148,3 +148,6 @@ proc ${board}_file { dest op args } { } return [eval [list standard_file $dest $op] $args] } + +# The predefined TSVs in GDBserver. +set_board_info gdb,predefined_tsv "\\\$trace_timestamp" diff --git a/gdb/testsuite/boards/native-gdbserver.exp b/gdb/testsuite/boards/native-gdbserver.exp index 8034a48..f32a37e 100644 --- a/gdb/testsuite/boards/native-gdbserver.exp +++ b/gdb/testsuite/boards/native-gdbserver.exp @@ -90,3 +90,6 @@ proc ${board}_file { dest op args } { } return [eval [list standard_file $dest $op] $args] } + +# The predefined TSVs in GDBserver. +set_board_info gdb,predefined_tsv "\\\$trace_timestamp" diff --git a/gdb/testsuite/boards/native-stdio-gdbserver.exp b/gdb/testsuite/boards/native-stdio-gdbserver.exp index 7e74970..d4983a6 100644 --- a/gdb/testsuite/boards/native-stdio-gdbserver.exp +++ b/gdb/testsuite/boards/native-stdio-gdbserver.exp @@ -152,3 +152,6 @@ proc ${board}_file { dest op args } { } return [eval [list standard_file $dest $op] $args] } + +# The predefined TSVs in GDBserver. +set_board_info gdb,predefined_tsv "\\\$trace_timestamp" diff --git a/gdb/testsuite/gdb.server/ext-attach.exp b/gdb/testsuite/gdb.server/ext-attach.exp index 1a2b539..61f93c1 100644 --- a/gdb/testsuite/gdb.server/ext-attach.exp +++ b/gdb/testsuite/gdb.server/ext-attach.exp @@ -18,6 +18,7 @@ # Test attaching to already-running programs using extended-remote. load_lib gdbserver-support.exp +load_lib trace-support.exp standard_testfile @@ -56,6 +57,15 @@ if { [istarget "*-*-cygwin*"] } { gdb_test "attach $testpid" \ "Attaching to program: .*, process $testpid.*(in|at).*" \ "attach to remote program 1" + +if { [gdb_target_supports_trace] } then { + # Test predefined TSVs are uploaded. + gdb_test_sequence "info tvariables" "check uploaded tsv" { + "\[\r\n\]+Name\[\t \]+Initial\[\t \]+Current" + "\[\r\n\]+\\\$trace_timestamp 0" + } +} + gdb_test "backtrace" ".*main.*" "backtrace 1" gdb_test "detach" "Detaching from program.*process.*" diff --git a/gdb/testsuite/gdb.trace/tsv.exp b/gdb/testsuite/gdb.trace/tsv.exp index 4177d13..cd0b36b 100644 --- a/gdb/testsuite/gdb.trace/tsv.exp +++ b/gdb/testsuite/gdb.trace/tsv.exp @@ -188,3 +188,25 @@ gdb_test_multiple "target ctf ${tracefile}.ctf" "" { check_tsv "ctf" } } + +# Restart. +clean_restart ${binfile} + +if ![runto_main] then { + fail "Can't run to main" + return +} + +# If there are predefined TSVs, test these predefined TSVs are correctly +# uploaded. +if [target_info exists gdb,predefined_tsv] { + set tsv [target_info gdb,predefined_tsv] + + # Test predefined TSVs are uploaded. + gdb_test "info tvariables" ".*${tsv}.*" "predefined tsvs are uploaded" +} else { + # Otherwise (the predefined TSVs are not defined in the board file), + # test there is no TSVs in GDB. + gdb_test "info tvariables" "No trace state variables\." \ + "no predefined tsvs" +} -- 1.7.7.6