Re: [RFA] Fix "break foo" when `foo's prologue ends before line table

Eli Zaretskii
2009-05-23T10:20:35Z

> Date: Wed, 20 May 2009 14:16:31 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Daniel Jacobowitz <drow@false.org>, gdb-patches@sourceware.org
>
> > 2009-05-09 Eli Zaretskii <eliz@gnu.org>
> >
> > * symtab.c (skip_prologue_using_lineinfo): New function.
> > (find_function_start_sal): Use it to get to the first line of
> > function's body that has an entry in the lineinfo table.
>
> The change looks OK to me, overall. I do have a couple of little
> comments, but do feel free to check it in once the comments are
> addressed.

Thanks. The patch I actually committed is reproduced below.

> > + /* Get the range for the function's PC values, or give up if we
> > + cannot, for some reason. */
> > + if (!find_pc_partial_function (func_addr, NULL, &func_start, &func_end))
> > + return func_addr;
> > + l = LINETABLE (symtab);
> > + if (l == NULL)
> > + return func_addr;
>
> Perhaps you want to check for this before doing the lookup above,
> since you have access to the linetable directly. Although, in
> practice, we know it should never fail, since the func_addr comes
> from a symbol. So we should always find the associated partial
> symbol.

I moved the test of l == NULL before the call to
find_pc_partial_function.

As for always finding the partial symbol: a single test instruction is
not expensive enough, IMO, to justify giving up defensive code, even
if it should always succeed. So I left it in place.

> > + /* Linetable entries are ordered by PC values, see the commentary in
> ^^^^^^ comment

Hmm? are you saying that "commentary" is incorrect English? If so, I
beg to differ. ;-)

> > + symtab.h where `struct linetable' is defined. Thus, the first
> > + entry whose PC is in the range [FUNC_START..FUNC_END] is the
> ^^^
> You probably want to say [FUNC_START..FUNC_END[, as FUNC_END should
> be excluded from the function range. And therefore:
>
> > + the table. See the commentary on symtab.h before the
> ^^^^^^^^ comment
> > + definition of struct linetable. */
> > + if (item->line > 0 && func_start <= item->pc && item->pc <= func_end)
> ^^
> I believe that the second comparison should be a strict compare:

Right, fixed.

> That's just a suggestion, but I wouldn't mind if we provided a little
> extra explainations about the situation where we have encountered this.

Is the comment I added before the call to skip_prologue_using_lineinfo
good enough?

2009-05-23 Eli Zaretskii <eliz@gnu.org>

* symtab.c (skip_prologue_using_lineinfo): New function.
(find_function_start_sal): Use it to get to the first line of
function's body that has an entry in the lineinfo table.

--- symtab.c~0 2009-04-01 02:21:07.000000000 +0300
+++ symtab.c 2009-05-23 12:52:12.609375000 +0300
@@ -2599,6 +2599,47 @@ find_function_start_pc (struct gdbarch *
return pc;
}

+/* Given a function start address FUNC_ADDR and SYMTAB, find the first
+ address for that function that has an entry in SYMTAB's line info
+ table. If such an entry cannot be found, return FUNC_ADDR
+ unaltered. */
+CORE_ADDR
+skip_prologue_using_lineinfo (CORE_ADDR func_addr, struct symtab *symtab)
+{
+ CORE_ADDR func_start, func_end;
+ struct linetable *l;
+ int ind, i, len;
+ int best_lineno = 0;
+ CORE_ADDR best_pc = func_addr;
+
+ /* Give up if this symbol has no lineinfo table. */
+ l = LINETABLE (symtab);
+ if (l == NULL)
+ return func_addr;
+
+ /* Get the range for the function's PC values, or give up if we
+ cannot, for some reason. */
+ if (!find_pc_partial_function (func_addr, NULL, &func_start, &func_end))
+ return func_addr;
+
+ /* Linetable entries are ordered by PC values, see the commentary in
+ symtab.h where `struct linetable' is defined. Thus, the first
+ entry whose PC is in the range [FUNC_START..FUNC_END[ is the
+ address we are looking for. */
+ for (i = 0; i < l->nitems; i++)
+ {
+ struct linetable_entry *item = &(l->item[i]);
+
+ /* Don't use line numbers of zero, they mark special entries in
+ the table. See the commentary on symtab.h before the
+ definition of struct linetable. */
+ if (item->line > 0 && func_start <= item->pc && item->pc < func_end)
+ return item->pc;
+ }
+
+ return func_addr;
+}
+
/* Given a function symbol SYM, find the symtab and line for the start
of the function.
If the argument FUNFIRSTLINE is nonzero, we want the first line
@@ -2649,6 +2690,21 @@ find_function_start_sal (struct symbol *
sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
}

+ /* If we still don't have a valid source line, try to find the first
+ PC in the lineinfo table that belongs to the same function. This
+ happens with COFF debug info, which does not seem to have an
+ entry in lineinfo table for the code after the prologue which has
+ no direct relation to source. For example, this was found to be
+ the case with the DJGPP target using "gcc -gcoff" when the
+ compiler inserted code after the prologue to make sure the stack
+ is aligned. */
+ if (funfirstline && sal.symtab == NULL)
+ {
+ pc = skip_prologue_using_lineinfo (pc, SYMBOL_SYMTAB (sym));
+ /* Recalculate the line number. */
+ sal = find_pc_sect_line (pc, SYMBOL_OBJ_SECTION (sym), 0);
+ }
+
sal.pc = pc;

return sal;