Re: RFA/mips: Use ".pdr" sections generated by GAS

[Daniel Jacobowitz <drow@mvista.com>]

On Wed, Jun 12, 2002 at 07:04:54PM -0400, Andrew Cagney wrote:
> >ECOFF debug information (mdebug) included the concept of a PDR.  MIPS uses
> >these for unwinding through functions, among other things.  Right now we 
> >can
> >get them from mdebug or from code inspection.  But current GNU binutils
> >emits ".pdr" sections instead of using the old ".mdebug" style debug info.
> >
> >This patch lets us read and handle the .pdr sections.  It's only for
> >32-bit, because for 64-bit gas currently emits only half the address.  That
> >I'll address later.  It changes a testsuite run from 5591 calls to
> >heuristic_proc_desc to 1787 calls.  Most or all of those are because we
> >build a heuristic procedure description if we find that we are still in the
> >prologue of a function.  The new ".pdr" method finds a PDR successfully for
> >nearly all calls to non_heuristic_proc_desc; the others are probably 
> >either a
> >lingering library on my system built by an older assembler, or signal/solib
> >trampolines.  Only 29 misses total.
> >
> >There's an increase in memory usage per-objfile, but no leaks, and the
> >section is generally not large; for all of GDB it is only 78K.
> >
> >Andrew, does this look OK to you?
> 
> Given my knowedge of PDR is limited, I 'll take your word on this.

Great.  Committed with comment tweaks.

> >Index: mips-tdep.c
> >===================================================================
> >RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> >retrieving revision 1.75
> >diff -u -p -u -r1.75 mips-tdep.c
> >--- mips-tdep.c	9 Jun 2002 19:36:15 -0000	1.75
> >+++ mips-tdep.c	11 Jun 2002 16:54:51 -0000
> >@@ -425,6 +425,8 @@ static unsigned int heuristic_fence_post
> > #define PROC_REG_OFFSET(proc) ((proc)->pdr.regoffset)
> > #define PROC_FREG_OFFSET(proc) ((proc)->pdr.fregoffset)
> > #define PROC_PC_REG(proc) ((proc)->pdr.pcreg)
> >+/* FIXME drow/2002-06-10: If a pointer on the host is bigger than a long,
> >+   this will corrupt pdr.iline.  Fortunately we don't use it.  */
> > #define PROC_SYMBOL(proc) (*(struct symbol**)&(proc)->pdr.isym)
> 
> Can we simply wack these macros?

Sure.  That's another patch for another day, though.

> >+      /* Search the ".pdr" section generated by GAS.  This includes most 
> >of
> >+	 the information normally found in ECOFF PDRs.
> >+
> >+	 Right now GAS only outputs the address as a four-byte sequence.  
> >This
> >+	 means that we should not bother with this method on 64-bit targets
> >+	 until that is fixed.  */
> >+
> >+      the_bfd = sec->objfile->obfd;
> 
> I suspect the use of ``the_bfd'' global at this point was unintentional vis:
> 
> >+      if (priv == NULL
> >+	  && (the_bfd->format == bfd_object
> >+	      && bfd_get_flavour (the_bfd) == bfd_target_elf_flavour
> >+	      && elf_elfheader (the_bfd)->e_ident[EI_CLASS] == ELFCLASS64))
> >+	{

Nope, perfectly intentional.  It has a shorter name and its value at
that point is quite clear.  I set it to NULL at the bottom of the if
statement as you suggested.

> Suggest putting the bulk of the comment explaing the ``64 bit problem'' 
> here where it is having an effect.
> 
> >+	      /* In general, the .pdr section is sorted.  However, in the
> >+		 presence of multiple code sections (and other corner cases)
> >+		 it can become unsorted.  Sort it so that we can use a faster
> >+		 binary search.  */

I've always been a fan of commenting above conditions rather than
below, but sure.

> >+	      qsort (priv->contents, priv->size / 32, 32, 
> >compare_pdr_entries);
> 
> compare_pdr_entries_abfd = NULL;
> 
> (I think I'll post a patch adding a gdb_qsort() function that takes a 
> context parameter).

Good idea.

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