This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Upload TSVs in extend-remote mode
- From: Yao Qi <yao at codesourcery dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Tue, 25 Jun 2013 21:02:15 +0800
- Subject: Re: [PATCH] Upload TSVs in extend-remote mode
- References: <1371636174-9822-1-git-send-email-yao at codesourcery dot com> <51C34B4A dot 7000606 at redhat dot com> <51C50AAC dot 70305 at codesourcery dot com> <51C864CD dot 5080008 at redhat dot com>
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