Re: stabs at function entry point?

[Daniel Jacobowitz <drow at mvista dot com>]

On Sat, Mar 16, 2002 at 08:14:32PM -0500, Daniel Jacobowitz wrote:
> John reported a bug in addr2line with stabs debug info.
> 
> Right now, we initialize everything (file name, directory name, etc.)
> except for line number before we start the linear search.  But we've found
> which function it is in at this point, and if it is in the prologue
> we've just skipped past the only stab that will tell us the line
> number.  This patch simply picks up the line number from the N_FUN stab
> before we begin (or possible an N_SO in some circumstances, I think). 
> It looks correct to me, and causes no regressions in binutils, ld, or
> gas (although I get a flonums FAIL in gas/gasp right now).  GDB doesn't
> use this code at all, though perhaps it should :)

My only concern about this patch, by the way: GCC puts the line number
of the function in N_FUN stabs.  Other compilers do not; Sun documents
this field as (among other things) the size or type of the return
value.  So, if we have a compiler which neither puts an N_SLINE after
N_FUN or puts the line number in the N_FUN we would lose.  An example
is:
  cc: Sun WorkShop 6 update 1 C 5.2 2000/09/11

I wish there were a safe way to make use of the line number in the
N_FUN.  It's more accurate than using the first line of the function
containing an N_SLINE, or at least it feels like it should be.

Revised patch attached.  This works fairly well, because GCC 2.95 does
things like this:
main:
        pushl %ebp
        movl %esp,%ebp
        subl $24,%esp
.stabn 68,0,2,.LM1-main
.LM1:
.stabn 68,0,3,.LM2-main
.LM2:
.LBB2:
        movl $1,-4(%ebp)

i.e. the stab which probably ought to be before the prologue is in fact
after it.

> As for DWARF2 - we don't do this correctly there either.  For the first
> function in a compilation unit, we will return ??:0, because the first
> entry in the line table will be just after the prologue.  For the
> second and subsequent functions, we return the name of the correct
> function but the last line of the previous function.  You could argue
> this either way; it sounds to me as if GCC should emit line number
> information for the first PC of the function.  We can detect this and
> handle it correctly in binutils, however, and I believe we should
> (permissive in what we accept and all that).  If the line we found
> starts before the function we found, we could use the first line of the
> function we found.  It's a little hokey of a heuristic, but I can think
> of no case in which it would return less useful information than what
> we have, and I've got two trivial examples in which it is better.
> 
> 
> All the other debug formats seem to have the same "problem".  These are
> just the two I could easily test; if someone hollers I could try to fix
> dwarf1 too (though I'd rather just let it die...).  I don't grok ecoff
> well enough, and I'm keeping my hands out of pdp11.c on principal
> (;-)).  There's probably one or two more, but most of the others (like
> SOM) seem to be dummies.
> 
> Is the patch at the end of this message OK?  John, does the patch work
> for you?
> 
> On Wed, Mar 13, 2002 at 09:19:20PM -0500, Daniel Jacobowitz wrote:
> > On Thu, Mar 14, 2002 at 01:32:24AM +0000, John Levon wrote:
> > > On Wed, Mar 13, 2002 at 08:18:05PM -0500, Daniel Jacobowitz wrote:
> > > 
> > > > Have you looked at the find_nearest_line code for stabs?  It's in
> > > > bfd/syms.c.  It reads the line numbers from function stabs into the
> > > > line table.  The presence or absence of a matching line number marking
> > > > should be No Big Deal for this interface.
> > > 
> > > Browsing the code, it looks like if I look up the very start of the
> > > function, and there's no entry at the given vma (so that :
> > > 
> > >    1185           if (offset >= info->indextable[mid].val
> > >    1186               && offset < info->indextable[mid + 1].val)
> > > 
> > > would catch it), then the above binary search will find the previous
> > > stab entry. So for example the first function in a file will come out as
> > > :
> > > 
> > > file.c:0
> > > 
> > > since *pline never gets set properly.
> > > 
> > > And we've recently seen this problem with a user using 2.95.3.
> > > 
> > > So either our definitions of Big Deal differ, or I'm just wrong and 
> > > the problem we've been seeing is unrelated to what's being discussed
> > > here.
> > 
> > Both right again... the line information for the N_FUN stab is entered,
> > but then not used in the line number search.  I believe this is simply
> > a bug in BFD.  I'll test a patch later this week.
> > 
> > In other words, I don't believe this function needs the initial stab.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-03-16  Daniel Jacobowitz  <drow@mvista.com>

	* dwarf2.c (struct funcinfo): Move up.
	(lookup_address_in_function_table): New argument function_ptr.
	Set it.
	(lookup_address_in_line_table): New argument function.  If function
	is non-NULL, use it to handle ``addr'' before the first line note of
	the function.
	(comp_unit_find_nearest_line): Update and swap calls to
	lookup_address_in_function_table and lookup_address_in_line_table.
	* syms.c (_bfd_stab_section_find_nearest_line): Use the first
	N_SLINE encountered if we see an N_FUN before any N_SLINE.

Index: bfd/syms.c
===================================================================
RCS file: /cvs/src/src/bfd/syms.c,v
retrieving revision 1.18
diff -u -p -r1.18 syms.c
--- syms.c	2002/02/19 16:33:04	1.18
+++ syms.c	2002/03/17 03:46:47
@@ -1238,9 +1238,11 @@ _bfd_stab_section_find_nearest_line (abf
 
   for (; stab < (indexentry+1)->stab; stab += STABSIZE)
     {
-      boolean done;
+      boolean done, saw_line, saw_func;
       bfd_vma val;
 
+      saw_line = false;
+      saw_func = false;
       done = false;
 
       switch (stab[TYPEOFF])
@@ -1261,7 +1263,9 @@ _bfd_stab_section_find_nearest_line (abf
 	  /* A line number.  The value is relative to the start of the
              current function.  */
 	  val = indexentry->val + bfd_get_32 (abfd, stab + VALOFF);
-	  if (val <= offset)
+	  /* If this line starts before our desired offset, or if it's
+	     the first line we've been able to find, use it.  */
+	  if (!saw_line || val <= offset)
 	    {
 	      *pline = bfd_get_16 (abfd, stab + DESCOFF);
 
@@ -1274,11 +1278,14 @@ _bfd_stab_section_find_nearest_line (abf
 	    }
 	  if (val > offset)
 	    done = true;
+	  saw_line = true;
 	  break;
 
 	case N_FUN:
 	case N_SO:
-	  done = true;
+	  if (saw_func || saw_line)
+	    done = true;
+	  saw_func = true;
 	  break;
 	}
 
Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.28
diff -u -p -r1.28 dwarf2.c
--- dwarf2.c	2002/01/30 10:28:47	1.28
+++ dwarf2.c	2002/03/17 03:46:48
@@ -240,9 +240,10 @@ static void arange_add PARAMS ((struct c
 static struct line_info_table *decode_line_info
   PARAMS ((struct comp_unit *, struct dwarf2_debug *));
 static boolean lookup_address_in_line_info_table
-  PARAMS ((struct line_info_table *, bfd_vma, const char **, unsigned int *));
+  PARAMS ((struct line_info_table *, bfd_vma, struct funcinfo *,
+	   const char **, unsigned int *));
 static boolean lookup_address_in_function_table
-  PARAMS ((struct funcinfo *, bfd_vma, const char **));
+  PARAMS ((struct funcinfo *, bfd_vma, struct funcinfo **, const char **));
 static boolean scan_unit_for_functions PARAMS ((struct comp_unit *));
 static bfd_vma find_rela_addend
   PARAMS ((bfd *, asection *, bfd_size_type, asymbol**));
@@ -808,6 +809,14 @@ struct line_info_table
   struct line_info* last_line;
 };
 
+struct funcinfo
+{
+  struct funcinfo *prev_func;
+  char* name;
+  bfd_vma low;
+  bfd_vma high;
+};
+
 static void
 add_line_info (table, address, filename, line, column, end_sequence)
      struct line_info_table* table;
@@ -1215,10 +1224,12 @@ decode_line_info (unit, stash)
 static boolean
 lookup_address_in_line_info_table (table,
 				   addr,
+				   function,
 				   filename_ptr,
 				   linenumber_ptr)
      struct line_info_table* table;
      bfd_vma addr;
+     struct funcinfo *function;
      const char **filename_ptr;
      unsigned int *linenumber_ptr;
 {
@@ -1235,35 +1246,52 @@ lookup_address_in_line_info_table (table
       if (!each_line->end_sequence
 	  && addr >= each_line->address && addr < next_line->address)
 	{
-	  *filename_ptr = each_line->filename;
-	  *linenumber_ptr = each_line->line;
+	  /* If this line appears to span functions, and addr is in the
+	     later function, return the first line of that function instead
+	     of the last line of the earlier one.  */
+	  if (function != NULL
+	      && each_line->address < function->low
+	      && next_line->address > function->low)
+	    {
+	      *filename_ptr = next_line->filename;
+	      *linenumber_ptr = next_line->line;
+	    }
+	  else
+	    {
+	      *filename_ptr = each_line->filename;
+	      *linenumber_ptr = each_line->line;
+	    }
 	  return true;
 	}
       next_line = each_line;
       each_line = each_line->prev_line;
     }
 
+  /* At this point each_line is NULL but next_line is not.  If we found the
+     containing function in this compilation unit, return the first line we
+     have a number for.  */
+  if (function != NULL)
+    {
+      *filename_ptr = next_line->filename;
+      *linenumber_ptr = next_line->line;
+      return true;
+    }
+
   return false;
 }
 
 /* Function table functions.  */
 
-struct funcinfo
-{
-  struct funcinfo *prev_func;
-  char* name;
-  bfd_vma low;
-  bfd_vma high;
-};
-
 /* If ADDR is within TABLE, set FUNCTIONNAME_PTR, and return true.  */
 
 static boolean
 lookup_address_in_function_table (table,
 				  addr,
+				  function_ptr,
 				  functionname_ptr)
      struct funcinfo* table;
      bfd_vma addr;
+     struct funcinfo** function_ptr;
      const char **functionname_ptr;
 {
   struct funcinfo* each_func;
@@ -1275,6 +1303,7 @@ lookup_address_in_function_table (table,
       if (addr >= each_func->low && addr < each_func->high)
 	{
 	  *functionname_ptr = each_func->name;
+	  *function_ptr = each_func;
 	  return true;
 	}
     }
@@ -1636,6 +1665,7 @@ comp_unit_find_nearest_line (unit, addr,
 {
   boolean line_p;
   boolean func_p;
+  struct funcinfo *function;
 
   if (unit->error)
     return false;
@@ -1664,13 +1694,16 @@ comp_unit_find_nearest_line (unit, addr,
 	}
     }
 
+  function = NULL;
+  func_p = lookup_address_in_function_table (unit->function_table,
+					     addr,
+					     &function,
+					     functionname_ptr);
   line_p = lookup_address_in_line_info_table (unit->line_table,
 					      addr,
+					      function,
 					      filename_ptr,
 					      linenumber_ptr);
-  func_p = lookup_address_in_function_table (unit->function_table,
-					     addr,
-					     functionname_ptr);
   return line_p || func_p;
 }