From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21854 invoked by alias); 17 Dec 2012 20:26:06 -0000 Received: (qmail 21843 invoked by uid 22791); 17 Dec 2012 20:26:05 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Dec 2012 20:25:52 +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 qBHKPop3022920 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 17 Dec 2012 15:25:50 -0500 Received: from host2.jankratochvil.net (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qBHKPibJ013920 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 17 Dec 2012 15:25:47 -0500 Date: Mon, 17 Dec 2012 20:26:00 -0000 From: Jan Kratochvil To: markus.t.metzger@intel.com Cc: palves@redhat.com, tromey@redhat.com, kettenis@gnu.org, gdb-patches@sourceware.org, markus.t.metzger@gmail.com Subject: Re: [patch v6 10/12] test, btrace: add branch tracing tests Message-ID: <20121217202543.GI14232@host2.jankratochvil.net> References: <1355760101-26237-1-git-send-email-markus.t.metzger@intel.com> <1355760101-26237-11-git-send-email-markus.t.metzger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1355760101-26237-11-git-send-email-markus.t.metzger@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 X-SW-Source: 2012-12/txt/msg00598.txt.bz2 On Mon, 17 Dec 2012 17:01:39 +0100, markus.t.metzger@intel.com wrote: [...] > --- /dev/null > +++ b/gdb/testsuite/gdb.btrace/enable.exp > @@ -0,0 +1,89 @@ [...] > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } { All these tests could be limited to "XXX-*-linux*" I think, couldn't they? > + verbose "Tests ignored for all but x86 based targets." > + return > +} [...] > +# we don't have trace, yet > +gdb_test "btrace" "No thread\." "btrace enable 2.2" This way it is equivalent to "No thread\." as \. is no special Tcl sequence so it equals to '.'. You wanted "No thread\\." so that backslash gets parsed by the regex parser and not the Tcl parser. Alternatively one can use {No thread\.} which skips the Tcl unbackslashing (but then some characters like \n or \r or $variables are not available). It is more times here. > +gdb_test "btrace /m" "No thread\." "btrace enable 2.3" > +gdb_test "btrace list /fal" "No thread\." "btrace enable 2.4" > +gdb_test "btrace list /t" "No thread\." "btrace enable 2.5" > + > +set testfile "x86-list" Please do not override $testfile to non-basename of the .exp file, see below for standard_testfile recommendation instead. > + > +# start a debuggee program > +if { [set binfile [btrace_assemble ${testfile}]] == "" } { > + untested ${testfile}.exp > + return -1 > +} > + > +gdb_reinitialize_dir $srcdir/$subdir > +gdb_load $binfile Why not clean_restart? > +# automatic enabling is off. > +if ![runto test_1] { > + fail "runto test function, 1" > + return -1 > +} [...] > --- /dev/null > +++ b/gdb/testsuite/gdb.btrace/list_function.c > @@ -0,0 +1,31 @@ [...] > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +extern int inc (int); > +extern int dec (int); > + > +extern int main (void) I find "extern" excessive/unusual during function definiton. > +{ > + int x = 0; > + > + x = inc (x); > + x = dec (x); > + > + return x; /* bp.1 */ > +} > diff --git a/gdb/testsuite/gdb.btrace/list_function.exp b/gdb/testsuite/gdb.btrace/list_function.exp > new file mode 100644 > index 0000000..180273f > --- /dev/null > +++ b/gdb/testsuite/gdb.btrace/list_function.exp > @@ -0,0 +1,50 @@ [...] > +load_lib btrace.exp > + > +set testfile "list_function" > +set sources [list $testfile.c inc.c dec.c] Use standard_testfile instead of these two sets. > + > +# check for btrace support > +if { [skip_btrace_tests] } { return -1 } [...] > --- /dev/null > +++ b/gdb/testsuite/lib/btrace.exp > @@ -0,0 +1,78 @@ [...] > +proc btrace_assemble { testfile } { > + global srcdir > + global objdir > + global subdir > + > + set srcfile ${srcdir}/${subdir}/${testfile}.S > + set objfile ${objdir}/${subdir}/${testfile}.o > + set binfile ${objdir}/${subdir}/${testfile}.x > + > + if {[target_assemble ${srcfile} ${objfile} ""] != ""} { return "" } > + > + if {[target_link ${objfile} ${binfile} "-Ttext 0x400100"] != ""} { return "" } Why is here -Ttext? It requires at least a comment but I would prefer to remove it. Can't you just use additional_flags=-nostdlib for the build? And can't you just not use -nostdlib, use standard system crt1 and just use runto_main and no longer define "_start" in your .S file? This btrace_assemble I find overengineered a bit, there should be standard_testfile called in each *.exp file which will set srcfile/binfile. > + > + return ${binfile} > +} > + > +proc skip_btrace_tests {} { > + global gdb_prompt > + > + set testfile "x86-list" Please do not override $testfile. $testfile is set by stadnard_testfile and it should always match the gdb.btrace/TESTFILE.exp name TESTFILE. > + set skip 0 > + > + if { [set binfile [btrace_assemble ${testfile}]] == "" } { > + return 1 > + } > + > + gdb_exit > + gdb_start > + gdb_load $binfile > + > + runto main You could use clean_restart, couldn't you? > + > + gdb_test_multiple "btrace enable" "check btrace support" { > + -re "You can't do that when your target is.*\r\n$gdb_prompt $" { > + xfail "check btrace support" > + set skip 1 > + } > + -re "Target does not support branch tracing.*\r\n$gdb_prompt $" { > + xfail "check btrace support" > + set skip 1 > + } > + -re "Could not enable branch tracing.*\r\n$gdb_prompt $" { > + xfail "check btrace support" > + set skip 1 > + } > + -re "$gdb_prompt $" { > + pass "check btrace support" > + } Tabs vs. spaces are wrong here. Tab is always 8 characters. > + } > + gdb_exit > + remote_file build delete $testfile > + > + return $skip > +} > + > +proc btrace_reset_trace {} { > + gdb_test_no_output "btr disable" > + gdb_test_no_output "btr enable" > + > + gdb_test "btr list" "No trace." "reset btrace" > +} > -- > 1.7.6.5 Thanks, Jan