From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20513 invoked by alias); 22 Jun 2013 02:24:00 -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 20487 invoked by uid 89); 22 Jun 2013 02:23: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; Sat, 22 Jun 2013 02:23:53 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UqDUV-00068o-6V from Yao_Qi@mentor.com ; Fri, 21 Jun 2013 19:23:51 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 21 Jun 2013 19:23:51 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.2.247.3; Fri, 21 Jun 2013 19:23:50 -0700 Message-ID: <51C50AAC.70305@codesourcery.com> Date: Sat, 22 Jun 2013 11:23: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> In-Reply-To: <51C34B4A.7000606@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-SW-Source: 2013-06/txt/msg00628.txt.bz2 On 06/21/2013 02:34 AM, Pedro Alves wrote: >> The fix to this problem is to upload TSV in >> >extended_remote_create_inferior_1. > This fixes "run", but I imagine it leaves "attach" broken? > You are right, :(. I add a test that tsvs are uploaded after attach, to make sure this patch handles "attach" as well. >> >Another fix could be that move the code uploading TSVs earlier in >> >remote_start_remote before sending packet '?'. I don't choose this >> >approach because I'd like GDB to upload TSVs when inferior starts run, >> >instead of when GDB connects to the stub. > Why would you like that? (Not saying it might not make sense). > Both of them were right to me at that moment and I spent some time on choosing one of them. I thought it might be "better" to postpone the uploading from the point of connect to the point of run. In the updated patch, I move the code uploading tsvs earlier in remote_start_remote before handling the reply of '?'. >> > >> >The patch to gdb.trace/tsv.exp can expose this bug by checking 'info >> >tvariables' after connect to the remote. If only test part of this >> >patch is applied, we can get a fail, >> > >> > $ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver tsv.exp" >> > FAIL: gdb.trace/tsv.exp: check uploaded tsv (pattern 1) >> > >> >With the whole patch applied, the fail goes away. >> > >> >I notice that TSV "$trace_timestamp" is not a documented TSV, so stubs >> >other than GDBserver may not have it. If it is a concern, I am OK to >> >drop the tsv.exp part from this patch. > I recently added some code in gdb.trace/qtro.exp that probes > for GDBserver: > > # Check whether we're testing with our own GDBserver. > set is_gdbserver -1 > set test "probe for GDBserver" > gdb_test_multiple "monitor help" $test { > -re "The following monitor commands are supported.*debug-hw-points.*remote-debug.*GDBserver.*$gdb_prompt $" { > set is_gdbserver 1 > pass $test > } > -re "$gdb_prompt $" { > set is_gdbserver 0 > pass $test > } > } > if { $is_gdbserver == -1 } { > return -1 > } > > (and I have a TODO item about adding a "monitor version" > command, which would be better than "monitor help"). > > We could move that to a shared is_gdbserver procedure in lib/, > and then: > > if is_gdbserver then expect $trace_timestamp, otherwise, fail > else expect $trace_timestamp, but be graceful if it isn't supported. > > That's similar to what qtro.exp is doing. Thanks for the pointer. When I was moving this code, I had a question in my mind that how to test non-gdbserver stubs if they also have predefined tsvs. Some stubs don't have predefined tsvs, while some stubs have different ones. We can extend the testsuite to read predefined tsvs from the board info, and the different predefined tsvs can be set in different board files for different stubs. In the updated patch below, the following line is added in all gdbserver board files, set_board_info gdb,predefined_tsv "\\\$trace_timestamp" and the test in gdb.trace/tsv.exp checks the target_info of "gdb,predefined_tsv" accordingly. If only the testsuite part of this patch is applied, we can get some faills, make check RUNTESTFLAGS='--target_board=native-extended-gdbserver tsv.exp' FAIL: gdb.trace/tsv.exp: check uploaded tsv make check RUNTESTFLAGS='--target_board=unix ext-attach.exp' FAIL: gdb.server/ext-attach.exp: check uploaded tsv (pattern 1) With patch all applied, these fails go away. Regression tested x86_64-linux with board file unix, native-gdbserver and native-extended-gdbserver. This patch also fixes a fail with board native-extended-gdbserver, FAIL: gdb.trace/mi-tsv-changed.exp: create delete modify: tvariable $tvar3 modified -- Yao (齐尧) gdb: 2013-06-22 Yao Qi * remote.c (remote_start_remote): Move code to upload tsv earlier. gdb/testsuite: 2013-06-22 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 has predefined tsv. gdb/doc: 2013-06-22 Yao Qi * gdbint.texinfo (Testsuite): Document 'gdb,predefined_tsv'. --- gdb/doc/gdbint.texinfo | 3 +++ gdb/remote.c | 20 ++++++++++++-------- 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 | 17 +++++++++++++++++ 7 files changed, 51 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..ce60527 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3452,6 +3452,18 @@ 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 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. GDB has to upload them too when + connects to the remote stub. */ + 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 +3603,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..0660bb1 100644 --- a/gdb/testsuite/gdb.trace/tsv.exp +++ b/gdb/testsuite/gdb.trace/tsv.exp @@ -188,3 +188,20 @@ gdb_test_multiple "target ctf ${tracefile}.ctf" "" { check_tsv "ctf" } } + +# 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" +} -- 1.7.7.6