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

[Eli Zaretskii <eliz at gnu dot org>]

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