From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12075 invoked by alias); 8 Dec 2011 15:33:43 -0000 Received: (qmail 12062 invoked by uid 22791); 8 Dec 2011 15:33:41 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 08 Dec 2011 15:33:27 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1RYfyP-0006El-Vp from Maciej_Rozycki@mentor.com ; Thu, 08 Dec 2011 07:33:26 -0800 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 8 Dec 2011 07:33:25 -0800 Received: from [172.30.5.182] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Thu, 8 Dec 2011 15:33:24 +0000 Date: Thu, 08 Dec 2011 15:33:00 -0000 From: "Maciej W. Rozycki" To: CC: Doug Evans , Joel Brobecker Subject: Re: [PATCH] testsuite: Add (extensive) hardware breakpoint testing In-Reply-To: Message-ID: References: User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: 8BIT 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: 2011-12/txt/msg00268.txt.bz2 On Fri, 11 Nov 2011, Maciej W. Rozycki wrote: > On Fri, 11 Nov 2011, Doug Evans wrote: > > > {i386,amd64}-linux are important enough targets that I think this > > should be fixed for 7.4. > > OK, so we've got this part sorted out -- and I verified the two failures > observed are also present with break.exp, so they're a generic > x86_64-linux-gnu problem and nothing wrong with this test. > > > One high level comment: > > There's a lot of basic linespec testing that would be nice to make table driven. > > E.g., {hbreak,thbreak} x {function, file:lineno, ...}, followed by > > delete_breakpoints. > > [I might not do this when testing running to the breakpoints, dunno if > > that is easily made data-driven, though that would be nice too.] > > If we're going to do (semi-)exhaustive linespec testing, I'd rather > > have a table than a lot of repetitive code. > > > > The same applies to break,tbreak. > > They too could be table driven. > > > > OTOH, maybe it's better to do simple linespec testing separately. > > There I would move {break,tbreak,hbreak,thbreak} x {function, > > file:lineno, ...} into its own file, > > have a table to drive it, and do a lot of testing with less code. > > It'd be more easily extensible too. > > > > I wouldn't make this a requirement, just mentioning it. > > I agree lots of older test cases ask for cleaning up -- I refrained from > diverging from break.exp too much, although I did some obvious tidy-ups, > like removing the remaining send_gdb/gdb_expect pieces in favour to > gdb_test/gdb_test_multiple. Interestingly enough someone did that to > break.exp too at one point, but didn't catch all the places. I think > it'll make sense to complete it using hbreak2.exp as a reference for a > change. I can't commit to doing it now, but I'll try as my time permits. > > > > @@ -0,0 +1,629 @@ > > > +#   Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999, > > > +#   2000, 2002, 2003, 2007, 2008, 2009, 2010, 2011 > > > +#   Free Software Foundation, Inc. > > > > This is a new file. I'm not sure what the rule is, but I think this > > should just say 2011. > > I wasn't sure -- the contents of the file certainly are not new, as this > is break.exp taken as a whole and then edited. Not even quite heavily -- > you can run a diff or compare the files side by side and they are still > pretty close to each other. I have removed the older years now; they can > be still added back if needed. > > > > +# FIXME: The rest of this test doesn't work with anything that can't > > > +# handle arguments. > > > +# Huh?  There doesn't *appear* to be anything that passes arguments > > > +# below. > > > > I think I'd just remove the FIXME and Huh? comments. > > And then ideally remove the mips-idt-* test. > > Again, this was taken from break.exp verbatim. I have removed it now as > it seems to make little sense indeed, but in this case I think the former > should be updated accordingly. > > > > +    if [istarget "mips*tx39-*"] { > > > +       set timeout 60 > > > > I would do: > > > > set oldtimeout $timeout > > if [istarget ...] { > > set timeout 60 > > } > > > > and then restore timeout at the end of the function. > > Hmm, the TX39 is an R3000 clone and probably does not have hardware > breakpoints in the first place. ;) So this is another break.exp > compatibility place -- I'm leaving it in place for this reason. > > As to your observation -- does it really matter? The timeout variable is > local to this function (accessed with upvar from gdb_test), so it's wiped > away at the point the procedure returns. I'm therefore leaving it as it > is unless you have further notes. > > > > +# Reset the default arguments for VxWorks > > > +if [istarget "*-*-vxworks*"] { > > > +    set timeout 10 > > > +    verbose "Timeout is now $timeout seconds" 2 > > > > Timeout restoration is no longer needed at the end of a file. > > I would delete these lines. > > > > > +    gdb_test_no_output "set args main" > > > +} > > Hmm, this actually looks like a blind copy&paste from gdb.base/a2-run.exp > with the timeout bit added later on. I have therefore removed the whole > conditional -- it's not like we change the arguments anywhere here. > > Here's an updated version -- any further comments? The update below, taken from break.exp, is required to fix: (gdb) hbreak 999 No line 999 in the current file. Make hw breakpoint pending on future shared library load? (y or [n]) n (gdb) FAIL: gdb.base/hbreak2.exp: hardware break on non-existent source line (got interactive prompt) on some platforms. I have integrated it into my patch now. Any progress with this test case otherwise? Maciej Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.base/hbreak2.exp =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.base/hbreak2.exp 2011-12-08 15:28:05.585562274 +0000 +++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.base/hbreak2.exp 2011-12-08 15:04:31.305404817 +0000 @@ -304,6 +304,7 @@ if ![runto_main] then { fail "break test # Verify that GDB responds gracefully when asked to set a breakpoint # on a nonexistent source line. # +gdb_test_no_output "set breakpoint pending off" gdb_test "hbreak 999" \ "No line 999 in file .*" \ "hardware break on non-existent source line"