From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123120 invoked by alias); 18 Jun 2018 07:37:00 -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 123095 invoked by uid 89); 18 Jun 2018 07:36:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=speaker, rationale, Hx-languages-length:3236, precommit X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Jun 2018 07:36:57 +0000 Received: from relay2.suse.de (charybdis-ext-too.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 33143AE12; Mon, 18 Jun 2018 07:36:55 +0000 (UTC) Subject: Re: [PATCH][gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp To: Joel Brobecker Cc: gdb-patches@sourceware.org References: <20180617155221.kqev7e3bwxb5uxmq@localhost.localdomain> <20180618001158.GA2447@adacore.com> From: Tom de Vries Message-ID: <68c97f70-34bc-1c6c-6241-ff98282173f3@suse.de> Date: Mon, 18 Jun 2018 07:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180618001158.GA2447@adacore.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00438.txt.bz2 On 06/18/2018 02:11 AM, Joel Brobecker wrote: > Hello, > >> Atm bp_inlined_func.exp passes for a combined current gcc and gdb-binutils >> repos build but fails for a build with system gcc (7.3.1) and ld (2.29.1). >> >> It checks for 4 breakpoints on read_small: >> ... >> gdb_test "break read_small" \ >> "Breakpoint $decimal at $hex: read_small\\. \\(4 locations\\)" \ >> "set breakpoint at read_small" >> ... >> and fails because it gets 5 breakpoint locations instead: >> ... >> (gdb) break read_small >> Breakpoint 2 at 0x401f9a: read_small. (5 locations) >> (gdb) FAIL: gdb.ada/bp_inlined_func.exp: set breakpoint at read_small >> ... >> >> The 4 expected breakpoint locations are inlined versions of read_small, and >> the 5th breakpoint location has this address: >> ... >> (gdb) info breakpoint >> Num Type Disp Enb Address What >> 1 breakpoint keep y >> 1.1 y 0x0000000000401f9a in b.read_small >> at bp_inlined_func/b.adb:20 >> ... >> which is the read_small function itself: >> ... >> (gdb) x 0x0000000000401f9a >> 0x401f9a : 0x22f8058b >> ... >> >> This patch updates the test to allow 5 breakpoint locations. >> >> Tested on the configurations mentioned above. >> >> OK for trunk? > > OK with me, with a few comments: > > - Can you include the description you provided above as part of > the commit's revision log? I had to do this manually until now, and forgot it one time, but I've now modified my precommit and submission scripts to handle the rationale as part of the commit log. Indeed the submission email draft was generated from the commit log containing the rationale. > I would avoid "Atm" and spell it out > as well. This is a bit of a nitpicking, but it's an easy thing > to be doing and can help readers less familiar with English. > I'm not a native speaker either, so nitpicks on language issues are welcome ;) > - Can you also include the platform itself on which you did the > testing? > Done. > - One spelling issue -- see below. > Fixed. Thanks for the review. - Tom >> [gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp >> >> 2018-06-17 Tom de Vries >> >> * gdb.ada/bp_inlined_func.exp: Allow 5 breakpoint locations. >> >> --- >> gdb/testsuite/gdb.ada/bp_inlined_func.exp | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func.exp b/gdb/testsuite/gdb.ada/bp_inlined_func.exp >> index 0f615f5d9b..79f9697124 100644 >> --- a/gdb/testsuite/gdb.ada/bp_inlined_func.exp >> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func.exp >> @@ -29,10 +29,10 @@ if ![runto_main] then { >> } >> >> # Check that inserting breakpoint on read_small inlined function inserts >> -# 4 breakpoints. >> +# 4 breakpoints (or posibbly 5, including the read_small function itself). > > "posibbly" -> "possibly". > >> gdb_test "break read_small" \ >> - "Breakpoint $decimal at $hex: read_small\\. \\(4 locations\\)" \ >> + "Breakpoint $decimal at $hex: read_small\\. \\(\[45\] locations\\)" \ >> "set breakpoint at read_small" >> >> # We do not verify each breakpoint info, but use continue commands instead > > Thank you, >