Message ID | 20180617155221.kqev7e3bwxb5uxmq@localhost.localdomain |
---|---|
State | New, archived |
Headers |
Received: (qmail 88551 invoked by alias); 17 Jun 2018 15:55:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 88513 invoked by uid 89); 17 Jun 2018 15:55:23 -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=H*MI:localdomain, UD:gdb.ada, gdb.ada, gdbada 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; Sun, 17 Jun 2018 15:55:22 +0000 Received: from relay1.suse.de (charybdis-ext-too.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id EF536AC7D; Sun, 17 Jun 2018 15:55:18 +0000 (UTC) Date: Sun, 17 Jun 2018 17:55:17 +0200 From: Tom de Vries <tdevries@suse.de> To: gdb-patches@sourceware.org Cc: Joel Brobecker <brobecker@gnat.com> Subject: [PATCH][gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp Message-ID: <20180617155221.kqev7e3bwxb5uxmq@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: NeoMutt/20170912 (1.9.0) X-IsSubscribed: yes |
Commit Message
Tom de Vries
June 17, 2018, 3:55 p.m. UTC
Hi, 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 <MULTIPLE> 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 <b__read_small+4>: 0x22f8058b ... This patch updates the test to allow 5 breakpoint locations. Tested on the configurations mentioned above. OK for trunk? Thanks, - Tom [gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp 2018-06-17 Tom de Vries <tdevries@suse.de> * 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(-)
Comments
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 <MULTIPLE> > 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 <b__read_small+4>: 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 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. - Can you also include the platform itself on which you did the testing? - One spelling issue -- see below. > [gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp > > 2018-06-17 Tom de Vries <tdevries@suse.de> > > * 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,
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 <MULTIPLE> >> 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 <b__read_small+4>: 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 <tdevries@suse.de> >> >> * 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, >
> > - Can you also include the platform itself on which you did the > > testing? > > > > Done. > > > - One spelling issue -- see below. > > > > Fixed. > > Thanks for the review. You are welcome. In scanning quickly the commit, I noticed you said "x86_64" as the platform. No need to change anything now, but for your next submissions, it's better to include the OS as well. Typically, you'll see people say "tested on x86_64-linux" or "x86-windows", or "ppc-elf". Many times, the OS doesn't matter, but it's always good to have it, because the behavior does often depend on the OS. Thanks for the patch, though. GDB is the better for it, and this is what _really_ matters ;-).
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). 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