Re: [RFA/linespec] wrong line number in breakpoint location

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-01-28 23:45, Joel Brobecker wrote:
> Hi Simon,
> 
>> I started seeing a failure with this patch:
>> 
>> FAIL: gdb.base/break.exp: verify that they were cleared
>> 
>> Here is the test code:
>> 
>>  40 int
>>  41 main (int argc, char **argv, char **envp)
>>  42 {
>>  43     if (argc == 12345) {  /* an unlikely value < 2^16, in case 
>> uninited */ /* set breakpoint 6 here */
>>  44         fprintf (stderr, "usage:  factorial <number>\n");
>>  45         return 1;
>>  46     }
>>  47     printf ("%d\n", factorial (atoi ("6")));  /* set breakpoint 1 
>> here */
>>  48     /* set breakpoint 12 here */
>>  49     marker1 ();  /* set breakpoint 11 here */
>>  50     marker2 (43); /* set breakpoint 20 here */
>> 
>> What happens is that we build a binary with optimization, set a
>> breakpoint on line 47, and expect "info break" to show it at line 47.
>> In reality, everything about line 47 has been inlined and there's no
>> address associated to line 47.  The following location in that file
>> that has generated code associated to it is line 49, so that's where
>> the breakpoint is placed in reality.  With this patch, "info break"
>> therefore now shows line 49.
> 
> Looking at the assembly code between the two versions, the difference
> is that in the GCC 5.x case, the printf called gets inlined (!),
> whereas it does not when usin GCC 7.x, even on the same system.
> So, in the first case, the code generated for line 47 gets
> line numbers referencing either another file, or another function,
> which explains why we end up breaking on the next line of code,
> which is line 49.
> 
> With the more recent version of GCC, the call to printf is no longer
> inlined, and so we have some instructions "attached" to line 47,
> thanks to the call to "printf".
> 
>> This particular test isn't really about testing with optimized code,
>> it's about checking if we can clear breakpoint commands.  So we should
>> probably test that against a non-optimized binary.
> 
> That's true that it doesn't seem necessary to perform that check
> against the optimized version. On the other hand, we could keep
> the testcase as is, by simply extracting from the output of the
> "break" command which line we actually broke on, and then use that
> in the expected output.
> 
> WDYT?
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.base/break.exp: Save the location where the breakpoint
>         on break.c:47 was actually inserted when debugging the version
>         compiled at -O2 and use it in the expected output of the "info
>         break" test performed soon after.
> 
> tested on x86_64-linux, with two configurations:
>   - Ubuntu 16.04 with the system compiler (breakpoint lands on line 49)
>   - Ubuntu 16.04 with GCC 7.3.1 (breakpoint lands on line 47)

Hi Joel,

Thanks, this is fine with me.  Just a really small nit, I would suggest 
initializing the line_actual variable to 0 or -1 (an invalid line 
number) prior to calling gdb_test_multiple.  This way, if that test 
fails, line_actual will still be defined, and the expression that refers 
to it will generate a FAIL instead of an unreadable tcl backtrace.

Simon