From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15583 invoked by alias); 2 Apr 2013 16:53:24 -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 15542 invoked by uid 89); 2 Apr 2013 16:53:17 -0000 X-Spam-SWARE-Status: No, score=-7.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS 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; Tue, 02 Apr 2013 16:53:15 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r32GrCjD025166 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 2 Apr 2013 12:53:12 -0400 Received: from host2.jankratochvil.net (ovpn-116-44.ams2.redhat.com [10.36.116.44]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r32Gr7de011848 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 2 Apr 2013 12:53:10 -0400 Date: Tue, 02 Apr 2013 17:45:00 -0000 From: Jan Kratochvil To: Aleksandar Ristovski Cc: "gdb-patches@sourceware.org" Subject: Re: [patch] validate binary before use Message-ID: <20130402165306.GA9479@host2.jankratochvil.net> References: <20121227211328.GA5739@host2.jankratochvil.net> <50DCBBD1.7000707@qnx.com> <5107F591.304@qnx.com> <20130131063518.GA3027@host2.jankratochvil.net> <510A7EB0.90702@qnx.com> <51278A2A.9000802@qnx.com> <512E42D1.3040101@qnx.com> <514C58B2.6090701@qnx.com> <20130328183727.GA14798@host2.jankratochvil.net> <515B0632.1040502@qnx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <515B0632.1040502@qnx.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2013-04/txt/msg00054.txt.bz2 Hi Aleksandar, just some obvious issues of the testsuite first: On Tue, 02 Apr 2013 18:24:18 +0200, Aleksandar Ristovski wrote: > >>+ send_gdb "set verbose 1\n" > > > >Never (only in some exceptional cases) use send_gdb, it creates races wrt > >syncing on end of the commands. Use gdb_test or gdb_test_no_output. > > [AR] I very much dislike using gdb_test unless I actually am doing a > test. Otherwise, we end up with testcases that tend to have 30-40 > passes but only 2-3 relevant. Thus, when these 2-3 relevant ones > start to FAIL it is easy to neglect that due to false cozy feeling > that, well, *most* are still passing. * Even a single PASS->FAIL can be a serious GDB regression. * There are still many racy testcases (with "random" results). * Therefore comparing any PASS/FAIL counts is irrelevant, only diff matters. Besides that send_gdb really does not work, it does not read the "(gdb) " response will confuse the later first test which does wait for a response. If you do not like trivial testcase names then just use: gdb_test_no_output "command" "" or gdb_test "command" "response" "" GDB testsuite handles testcase name "" by omitting it from the output. > >>+ send_gdb "tbreak ${srcfile}:${bp_location}\n" > > > >Do not use send_gdb and there is gdb_breakpoint function. > > [AR] I am not testing setting breakpoints. I do not want these to > show up as PASS-es. These passes are irrelevant. The assumption is > that breakpoints do work; there are other tests for breakponts. gdb_breakpoint does not produce any PASS message when it succeeds. But it will FAIL if a problem occured. > >>+ send_gdb "run\r\n" > > > >Use runto_main. And check its result code. > > [AR] The same. I am not testing run to main. I am testing this > particular feature. There are other tests that test runto_main. Again, successful runto_main does not produce any PASS message. > >>+ gdb_test "info sharedlibrary ${solibfile}" \ > >>+ ".*From.*To.*Syms.*Read.*Shared.*\r\n.*${symsloaded}.*" \ > > ^^ > >BTW leading .* is excessive, gdb_test regex does not have anchored its start. > > [AR] ok. > > > > > > >>+ "Symbols for ${solibfile} loaded: expected '${symsloaded}'" > > > >Protect ${symsloaded} by [string_to_regexp $string] as user > >may have regex-unsafe characters there. > > [AR] symsloaded is argument passed to solib_matching_test, and the > test is the only user. Ther eare no other users, and the string may > contain only 'Yes' or 'No'. OK, I did not notice, I agree string_to_regexp is not needed there. But when you expect only one shared library make the expectation explicit, both for a single line and for ${binlibfilebase}. gdb_test "info sharedlibrary ${solibfile}" \ "From\[^\r\n\]*To\[^\r\n\]*Syms\[^\r\n\]*Read\[^\r\n\]*Shared\[^\r\n\]*\r\n\[^\r\n\]*${symsloaded}\[^\r\n\]*[string_to_regexp ${binlibfilebase}]" \ (I did not test this regex.) I can very well imagine GDB could print >= 2 lines or a line without ${binlibfilebase} there which could make false PASS. Thanks, Jan