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.

5 files changed, 221 insertions, 0 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f7b27f5..26f2bbc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+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.
+
 2013-05-10  Joel Brobecker  <brobecker@adacore.com>
 
 	* solib-aix.c (solib_aix_relocate_section_addresses):
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/ChangeLog b/gdb/testsuite/ChangeLog
index c6351fc..4289819 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2013-05-10  Pedro Alves  <palves@redhat.com>
+
+	PR remote/15455
+
+	* gdb.trace/qtro.c: New file.
+	* gdb.trace/qtro.exp: New file.
+
 2013-05-10  Phil Muldoon  <pmuldoon@redhat.com>
 
 	* gdb.python/py-framefilter.py: New File.
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)"
+		}
+	    }
+	}
+    }
+}