* Re: [PATCH] Fix for QTro remote packet
2013-05-02 17:23 ` Pedro Alves
@ 2013-05-10 14:17 ` Pedro Alves
2013-05-11 10:38 ` Joel Brobecker
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2013-05-10 14:17 UTC (permalink / raw)
To: David Taylor; +Cc: gdb-patches
I've added a new test case, and applied this, mainline and 7.6.
The tricky part with the test is that if
qXfer:traceframe-info:read is supported, QTro doesn't have
to be. That means that if the target does support
qXfer:traceframe-info:read, and we disable qXfer:traceframe-info:read
for testing, we can't assume and FAIL if accessing read-only memory
fails. However, since we know GDBserver supports both packets, we
can make the test detect it's running against GDBserver, and issue
a FAIL then, otherwise, issue just UNSUPPORTED. This should be enough
for catching blatant QTro regressions such as PR15455, as GDBserver
is regularly tested.
As for the 7.6.1 release notes in the wiki, I've went ahead and
modelled from Joel's entry, and added:
"PR remote/15455 (QTro remote packet broken)"
I think there's no way to get that wrong if Joel's entry is
good too. :-)
Below's what I checked in.
Thanks.
---
Subject:PR remote/15455 - QTro remote packet broken
In the function remote_trace_set_readonly_regions in gdb/remote.c, the
local variable 'offset' does not account for "QTro" at the start of
the packet with the result that if there are any read-only regions,
the packet is sent -- but without the "QTro" -- causing the remote
stub to report that the packet is unsupported:
Sending packet: $:0000000000400200,(...),00000000004560a4#ab...Packet received:
vs the expected:
Sending packet: $QTro:0000000000400200,(...),00000000004560a4#31...Packet received: OK
We don't see the problem when testing with GDBserver, as that supports
qXfer:trace-frame-info:read, meaning GDBserver never needs to read
from the read-only sections directly itself. This commit adds a test
that explicitly disables qXfer:trace-frame-info:read.
gdb/
2013-05-10 David Taylor <dtaylor@emc.com>
PR remote/15455
* remote.c (remote_trace_set_readonly_regions): Do not overwrite
"QTro" at start of packet.
gdb/testsuite/
2013-05-10 Pedro Alves <palves@redhat.com>
PR remote/15455
* gdb.trace/qtro.c: New file.
* gdb.trace/qtro.exp: New file.
---
gdb/remote.c | 1
gdb/testsuite/gdb.trace/qtro.c | 33 +++++++
gdb/testsuite/gdb.trace/qtro.exp | 173 ++++++++++++++++++++++++++++++++++++++
3 files changed, 207 insertions(+)
create mode 100644 gdb/testsuite/gdb.trace/qtro.c
create mode 100644 gdb/testsuite/gdb.trace/qtro.exp
diff --git a/gdb/remote.c b/gdb/remote.c
index e1c63ad..b7a7cf6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10667,6 +10667,7 @@ remote_trace_set_readonly_regions (void)
return; /* No information to give. */
strcpy (target_buf, "QTro");
+ offset = strlen (target_buf);
for (s = exec_bfd->sections; s; s = s->next)
{
char tmp1[40], tmp2[40];
diff --git a/gdb/testsuite/gdb.trace/qtro.c b/gdb/testsuite/gdb.trace/qtro.c
new file mode 100644
index 0000000..c32cebf
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/qtro.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+void
+subr (int parm)
+{
+}
+
+void
+end (void)
+{
+}
+
+int
+main ()
+{
+ subr (1);
+ end ();
+}
diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
new file mode 100644
index 0000000..f07c954
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -0,0 +1,173 @@
+# Copyright 1998-2013 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# This test helps making sure QTro support doesn't regress. If the
+# stub supports the newer qXfer:traceframe-info:read, then the QTro
+# paths in the stub are never exercised. PR remote/15455 is an
+# example of a regression that unfortunately went unnoticed for long.
+
+load_lib trace-support.exp
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+ return -1
+}
+clean_restart $testfile
+
+if ![runto_main] {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+# Check whether we're testing with the remote or extended-remote
+# targets, and whether the target supports tracepoints.
+
+proc gdb_is_target_remote { } {
+ global gdb_prompt
+
+ set test "probe for target remote"
+ gdb_test_multiple "maint print target-stack" $test {
+ -re ".*emote serial target in gdb-specific protocol.*$gdb_prompt $" {
+ pass $test
+ return 1
+ }
+ -re "$gdb_prompt $" {
+ pass $test
+ }
+ }
+ return 0
+}
+
+if ![gdb_is_target_remote] {
+ return -1
+}
+
+if ![gdb_target_supports_trace] {
+ unsupported "Current target does not support trace"
+ return -1;
+}
+
+# Run a trace session, stop it, and then inspect the resulting trace
+# frame (IOW, returns while tfind mode is active).
+proc prepare_for_trace_disassembly { } {
+ global gdb_prompt
+ gdb_breakpoint "end"
+
+ gdb_test "trace subr" "Tracepoint .*" \
+ "tracepoint at subr"
+
+ gdb_trace_setactions "define action" \
+ "" \
+ "collect parm" "^$"
+
+ gdb_test_no_output "tstart"
+
+ gdb_test "continue" ".*Breakpoint \[0-9\]+, end .*" \
+ "advance through tracing"
+
+ gdb_test "tstatus" ".*Collected 1 trace frame.*" \
+ "collected 1 trace frame"
+
+ gdb_test_no_output "tstop"
+
+ gdb_tfind_test "tfind start" "start" "0"
+}
+
+clean_restart $testfile
+runto_main
+
+# Trace once, issuing a tstatus, so that GDB tries
+# qXfer:trace-frame-info:read.
+prepare_for_trace_disassembly
+
+# Now check whether the packet is supported.
+set traceframe_info_supported -1
+set test "probe for traceframe-info support"
+gdb_test_multiple "show remote traceframe-info-packet" $test {
+ -re ".*Support for .* is auto-detected, currently (\[a-z\]*).*$gdb_prompt $" {
+ set status $expect_out(1,string)
+
+ if { $status == "enabled" } {
+ set traceframe_info_supported 1
+ } else {
+ set traceframe_info_supported 0
+ }
+
+ pass $test
+ }
+}
+if { $traceframe_info_supported == -1 } {
+ return -1
+}
+
+# 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
+}
+
+# Now disassemble (IOW, read from read-only memory) while inspecting a
+# trace frame, twice. Once with qXfer:traceframe-info:read left to
+# auto, and once with it disabled, exercising the QTro fallback path
+# in the stub side.
+foreach tfinfo { auto off } {
+ with_test_prefix "qXfer:traceframe-info:read $tfinfo" {
+
+ clean_restart $testfile
+ runto_main
+ gdb_test_no_output "set remote traceframe-info-packet $tfinfo"
+
+ prepare_for_trace_disassembly
+
+ set test "trace disassembly"
+ gdb_test_multiple "disassemble subr" $test {
+ -re "<(\.\[0-9\]+|)>:.*End of assembler dump.*$gdb_prompt $" {
+ pass $test
+ }
+ -re "Cannot access memory.*$gdb_prompt $" {
+ if { $traceframe_info_supported == 0 } {
+ # If qXfer:traceframe-info:read is not supported,
+ # then there should be QTro support.
+ fail $test
+ } elseif { $tfinfo == off && $is_gdbserver == 1 } {
+ # We we're testing with GDBserver, we know both
+ # qXfer:traceframe-info:read and QTro are
+ # supported (although supporting the former only
+ # would be sufficient), so issue a FAIL instead of
+ # UNSUPPORTED, giving us better visibility of QTro
+ # regressions.
+ fail $test
+ } else {
+ # Otherwise, qXfer:traceframe-info:read is
+ # supported, making QTro optional, so this isn't
+ # really a failure.
+ unsupported "$test (no QTro support)"
+ }
+ }
+ }
+ }
+}
^ permalink raw reply [flat|nested] 4+ messages in thread