[PATCH] Upload TSVs in extend-remote mode

[PATCH] Upload TSVs in extend-remote mode

[Yao Qi]

Hi,
In extended-remote, when GDB connects the target, but target is not
running, the TSVs are not uploaded (because remote_start_remote
returns earlier when the reply to packet '?' is either 'X' or 'W').

However, GDBserver has some builtin or predefined TSVs to upload, such
as $trace_timestamp.  This bug causes $trace_timestamp is never uploaded.

The fix to this problem is to upload TSV in
extended_remote_create_inferior_1.

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.

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.

Regression tested on x86_64-linux with board file
native-extended-gdbserver and native-gdbserver.  Is it OK?

gdb:

2013-06-19  Yao Qi  <yao@codesourcery.com>

	* remote.c (extended_remote_create_inferior_1): Upload TSVs from
	the remote stub.

gdb/testsuite:

2013-06-19  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/tsv.exp: Test TSV is uploaded.
---
 gdb/remote.c                    |    9 +++++++++
 gdb/testsuite/gdb.trace/tsv.exp |    6 ++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index be9186b..9f38d8d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8076,6 +8076,15 @@ extended_remote_create_inferior_1 (char *exec_file, char *args,
 
   /* Get updated offsets, if the stub uses qOffsets.  */
   get_offsets ();
+
+  /* Upload the TSVs from 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);
+    }
 }
 
 static void
diff --git a/gdb/testsuite/gdb.trace/tsv.exp b/gdb/testsuite/gdb.trace/tsv.exp
index 4177d13..302bcef 100644
--- a/gdb/testsuite/gdb.trace/tsv.exp
+++ b/gdb/testsuite/gdb.trace/tsv.exp
@@ -118,6 +118,12 @@ if { $trcpt1 <= 0 } then {
     return
 }
 
+# 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 "tvariable \$tvar5 = 15" \
   "Trace state variable \\\$tvar5 created, with initial value 15." \
   "Create a trace state variable tvar5"
-- 
1.7.7.6

Re: [PATCH] Upload TSVs in extend-remote mode

[Pedro Alves]

On 06/19/2013 11:02 AM, Yao Qi wrote:
> Hi,
> In extended-remote, when GDB connects the target, but target is not
> running, the TSVs are not uploaded (because remote_start_remote
> returns earlier when the reply to packet '?' is either 'X' or 'W').
> 
> However, GDBserver has some builtin or predefined TSVs to upload, such
> as $trace_timestamp.  This bug causes $trace_timestamp is never uploaded.
> 
> 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?

> 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).

> 
> 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.

-- 
Pedro Alves

Re: [PATCH] Upload TSVs in extend-remote mode

[Yao Qi]

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  <yao@codesourcery.com>

        * remote.c (remote_start_remote): Move code to upload tsv
        earlier.

gdb/testsuite:

2013-06-22  Yao Qi  <yao@codesourcery.com>

	* 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  <yao@codesourcery.com>

	* 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

Re: [PATCH] Upload TSVs in extend-remote mode

[Pedro Alves]

On 06/22/2013 03:23 AM, Yao Qi wrote:
> 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.

Thanks.

>
>>>> 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 '?'.

Yeah, I agree the choice isn't obvious, and I wasn't objecting to
your choice.  For now, I was just looking at recording the reasons
for our preferences.  I suspect we'll need to revisit this once we have
multi-inferior-aware tracing, or when gdb supports multi-target.

> 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,

Good idea.

>
> +  /* 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.

> +  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
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.

WDYT?

The patch is okay with or without that change.

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.

-- 
Pedro Alves

Re: [PATCH] Upload TSVs in extend-remote mode

[Yao Qi]

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  <yao@codesourcery.com>

        * remote.c (remote_start_remote): Move code to upload tsv
        earlier.

gdb/testsuite:

2013-06-25  Yao Qi  <yao@codesourcery.com>

	* 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  <yao@codesourcery.com>

	* 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

Re: [PATCH] Upload TSVs in extend-remote mode

[Pedro Alves]

On 06/25/2013 02:02 PM, Yao Qi wrote:
> On 06/24/2013 11:25 PM, Pedro Alves wrote:
>> 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.

Well, it'd be the board writer's responsibility to only do
that if he wanted to be lax, but I don't think this is that
important.  Fine with me as is.

Thanks!

-- 
Pedro Alves