Re: [RFA] skip_prologue_sal and sal expansion

[Doug Evans <dje at google dot com>]

On Tue, Jun 2, 2009 at 9:21 AM, Jerome Guitton <guitton@adacore.com> wrote:
>
> A couple of cleanups in breakpoint.c. Let me give some background
> first; consider the following program:
>
> int counter = 42;
>
> inline void
> callee ()
> {
> ?counter = 0; /* set breakpoint in an inlined function. ?*/
> }
>
> void
> caller ()
> {
> ?callee ();
> }
>
> int
> main ()
> {
> ?caller ();
> ?callee ();
> ?return counter;
> }
>
>
>
> When callee is inlined, we have three occurence for the line
> "counter = 0;": inlined in caller, inlined in main, and not inlined.
> When a breakpoint is set on this line, GDB sets a breakpoint on 3
> locations.
>
> (gdb) l p.c:6
> 1 ? ? ? int counter = 42;
> 2
> 3 ? ? ? inline void
> 4 ? ? ? callee ()
> 5 ? ? ? {
> 6 ? ? ? ? counter = 0;
> 7 ? ? ? }
> 8
> 9 ? ? ? void
> 10 ? ? ?caller ()
> (gdb) b 6
> Breakpoint 1 at 0x1800074: file p.c, line 6. (3 locations)
>
>
> I have recently hit a bug in an assembler which was optimizing out the
> prologue line info; it was making GDB think that the line
> "counter = 0;" was a part of callee's prologue. And this pointed me to
> something strange in GDB.
>
> After having used this bogus assembler to generate my program, if I try
> to set a breakpoint at line "counter = 0;", I end up with only one
> occurence instead of three:
>
> (gdb) b 6
> Breakpoint 1 at 0x1800074: file p.c, line 6.
>
> The problem was in skip_prologue_sal defined in breakpoint.c. When it
> actually skips a prologue, it does not assure that the other sal's
> fields (explicit_pc and explicit_line) are left unchanged. In my case,
> it was accidently changing explicit_line from 1 to 0. This change
> disabled the line sal expansion, and in consequence we ended up with
> the breakpoint set in only one location. I think that it's a bug in
> skip_prologue_sal, this function should not change mess with these
> fields.
>
> Now, if I change skip_prologue_sal to copy explicit_line and
> explicit_pc, the line expansion is done; but we should make sure that
> prologue is skipped similarly, otherwise we get an assertion failure
> when the address returned by resolve_sal_pc cannot be found after
> line sal expansion:
>
> (gdb) break p.c:6
> ../../src/gdb/breakpoint.c:5113: internal-error: expand_line_sal_maybe:
> Assertion `found' failed.
>
>
> Patch attached, tested on x86-linux. OK to apply?
>
>
> 2009-06-02 ?Jerome Guitton ?<guitton@adacore.com>
>
> ? ? ? ?* breakpoint.c (expand_line_sal_maybe): When explicit_line,
> ? ? ? ?skip prologue on each sals.
> ? ? ? ?(skip_prologue_sal): Return explicit_line and explicit_pc
> ? ? ? ?unmodified.
>

Sounds reasonable to me (fwiw).  Still need to wait for an official
maintainer's comments.