Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address

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

On 8/28/20 5:14 PM, Pedro Alves wrote:
> On 8/28/20 2:53 PM, Tom de Vries wrote:
> 
>> I see what you mean, but let's try this counter-example:
>> ...
>>  cat -n test.c
>>      1  int
>>      2  main (void)
>>      3  {
>>      4    goto L2;
>>      5
>>      6   L3:
>>      7    return 0;
>>      8
>>      9   L1:
>>     10    (void)0;
>>     11    return 1;
>>     12
>>     13   L2:
>>     14    goto L3;
>>     15  }
>>     16
>> ...
>> compiled like this:
>> ...
>> $ gcc test.c -g
>> ...
>>
>> With the patch, we're not able to set a breakpoint at L1, and setting
>> the breakpoint at the corresponding line, line 9:
>> ...
>> $ gdb a.out
>> Reading symbols from a.out...
>> (gdb) b main:L1
>> Location main:L1 not available
>> (gdb) b 9
>> Breakpoint 1 at 0x40049c: file test.c, line 14.
>> (gdb)
>> ...
>> yields a breakpoint at line 14, a piece of code that's not reachable
>> from L1.
>>
>> To me, label L1 and line 14 are unrelated enough to convince me to not
>> do this automatically.
> 
> Well, OK.  All the code after line 7 is unreacheable.
> 

Um, not sure what you mean here, line 13/14 is reachable.

> Then using that rationale, why don't we reject a breakpoint at line 9 too?

Well, I did wonder if we can somehow deduce something 'irregular' from
the line table in this case and reject the breakpoint at line 9, while
having the regular and helpful:
...
3 int main (void) {
4
5   i = 3;
(gdb) b 4
...
still work and set a breakpoint in line 5, but I didn't manage to come
up with something.

So I think this is something we have to live with because the generic
case is useful.

> Line 14 is not reacheable from line 9 either.  Can you justify the
> difference in a small sentence/paragraph?
> 

Let me try.

My argument is basically is that the behaviour "this line doesn't work,
let me try a different line" is simple enough for the user to follow
(although arguably, we could be more verbose about it).

The behaviour "this label doesn't work, let me try the corresponding
line, hey the line doesn't work, let me try a different line" is too
elaborate IMO.

The behaviour "this label doesn't work, let me try the corresponding
line, if the line works" sounds still ok to me, but I don't think it
will ever be triggered.  In this situation, I expect the compiler to be
able to generate a label with address itself.

> It would be perhaps nice if "break" accepted an option to prevent the
> move-to-following-insn -- for all kinds of breakpoints -- like lldb's
> "break set -m" option:
> 
>        -m <boolean> ( --move-to-nearest-code <boolean> )
>             Move breakpoints to nearest code. If not set the target.move-to-nearest-codesetting is used.
> 

Oh, that's interesting.  In general I'm in favor of putting things which
may help the user in certain situations but can backfire in others under
user control.

> BTW, I noticed that, setting the breakpoint at the label _after_ running
> to main "works":
> 
>  Reading symbols from testsuite/outputs/gdb.base/label-without-address/label-without-address...
>  (gdb) b main:L1
>  Location main:L1 not available
>  (gdb) start
>  Temporary breakpoint 1 at 0x1131: file testsuite/gdb.base/label-without-address.c, line 21.
>  Starting program: testsuite/outputs/gdb.base/label-without-address/label-without-address 
> 
>  Temporary breakpoint 1, main () at testsuite/gdb.base/label-without-address.c:21
>  21        return 0;
>  (gdb) b main:L1
>  Breakpoint 2 at 0x555555554000: file testsuite/gdb.base/label-without-address.c, line 22.
>  (gdb) info breakpoints 
>  Num     Type           Disp Enb Address            What
>  2       breakpoint     keep y   0x0000555555554000 main:L1
> 
> I see the same with your more complicated example.  What's going on here?
> 

This is what you get when you're generate a PIE.  It looks like it
works, but it doesn't, that breakpoint address is incorrect, you're
running into PR26546, for which I submitted a patch.

Thanks,
- Tom