From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9524 invoked by alias); 10 May 2013 14:17:51 -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 9473 invoked by uid 89); 10 May 2013 14:17:50 -0000 X-Spam-SWARE-Status: No, score=-8.3 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_CP autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 10 May 2013 14:17:49 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4AEHkcU005512 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 10 May 2013 10:17:46 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r4AEHjkR021556; Fri, 10 May 2013 10:17:45 -0400 Message-ID: <518D0188.60003@redhat.com> Date: Fri, 10 May 2013 14:17:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: David Taylor CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix for QTro remote packet References: <10772.1367429886@usendtaylorx2l> <5182A112.3050303@redhat.com> In-Reply-To: <5182A112.3050303@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00386.txt.bz2 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 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 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 . */ + +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 . + +# 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)" + } + } + } + } +}