Re: [PATCH][gdb/testsuite/ada] Fix number-of-bp test in bp_inlined_func.exp

[Tom de Vries <tdevries@suse.de>]

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,
>