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

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

[Daniel Jacobowitz]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

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?

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

[-- Attachment #2: mips-use-pdr-section.patch --]
[-- Type: text/plain, Size: 6433 bytes --]

2002-06-11  Daniel Jacobowitz  <drow@mvista.com>

	* mips-tdep.c (PROC_SYMBOL): Add warning comment.
	(struct mips_objfile_private, compare_pdr_entries): New.
	(non_heuristic_proc_desc): Read the ".pdr" section if it
	is present.

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)
 #define _PROC_MAGIC_ 0x0F0F0F0F
 #define PROC_DESC_IS_DUMMY(proc) ((proc)->pdr.isym == _PROC_MAGIC_)
@@ -1932,6 +1934,30 @@ heuristic_proc_desc (CORE_ADDR start_pc,
   return &temp_proc_desc;
 }
 
+struct mips_objfile_private
+{
+  bfd_size_type size;
+  char *contents;
+};
+
+/* Global used to communicate between non_heuristic_proc_desc and
+   compare_pdr_entries.  */
+static bfd *the_bfd;
+
+static int
+compare_pdr_entries (const void *a, const void *b)
+{
+  CORE_ADDR lhs = bfd_get_32 (the_bfd, (bfd_byte *) a);
+  CORE_ADDR rhs = bfd_get_32 (the_bfd, (bfd_byte *) b);
+
+  if (lhs < rhs)
+    return -1;
+  else if (lhs == rhs)
+    return 0;
+  else
+    return 1;
+}
+
 static mips_extra_func_info_t
 non_heuristic_proc_desc (CORE_ADDR pc, CORE_ADDR *addrptr)
 {
@@ -1939,22 +1965,143 @@ non_heuristic_proc_desc (CORE_ADDR pc, C
   mips_extra_func_info_t proc_desc;
   struct block *b = block_for_pc (pc);
   struct symbol *sym;
+  struct obj_section *sec;
+  struct mips_objfile_private *priv;
+
+  if (PC_IN_CALL_DUMMY (pc, 0, 0))
+    return NULL;
 
   find_pc_partial_function (pc, NULL, &startaddr, NULL);
   if (addrptr)
     *addrptr = startaddr;
-  if (b == NULL || PC_IN_CALL_DUMMY (pc, 0, 0))
-    sym = NULL;
-  else
+
+  priv = NULL;
+
+  sec = find_pc_section (pc);
+  if (sec != NULL)
     {
-      if (startaddr > BLOCK_START (b))
-	/* This is the "pathological" case referred to in a comment in
-	   print_frame_info.  It might be better to move this check into
-	   symbol reading.  */
-	sym = NULL;
-      else
-	sym = lookup_symbol (MIPS_EFI_SYMBOL_NAME, b, LABEL_NAMESPACE, 0, NULL);
+      priv = (struct mips_objfile_private *) sec->objfile->obj_private;
+
+      /* 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;
+      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))
+	{
+	  priv = obstack_alloc (& sec->objfile->psymbol_obstack,
+				sizeof (struct mips_objfile_private));
+	  priv->size = 0;
+	  sec->objfile->obj_private = priv;
+	}
+      else if (priv == NULL)
+	{
+	  asection *bfdsec;
+
+	  priv = obstack_alloc (& sec->objfile->psymbol_obstack,
+				sizeof (struct mips_objfile_private));
+
+	  bfdsec = bfd_get_section_by_name (sec->objfile->obfd, ".pdr");
+	  if (bfdsec != NULL)
+	    {
+	      priv->size = bfd_section_size (sec->objfile->obfd, bfdsec);
+	      priv->contents = obstack_alloc (& sec->objfile->psymbol_obstack,
+					      priv->size);
+	      bfd_get_section_contents (sec->objfile->obfd, bfdsec,
+					priv->contents, 0, priv->size);
+
+	      /* 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.  */
+	      qsort (priv->contents, priv->size / 32, 32, compare_pdr_entries);
+	    }
+	  else
+	    priv->size = 0;
+
+	  sec->objfile->obj_private = priv;
+	}
+
+      if (priv->size != 0)
+	{
+	  int low, mid, high;
+	  char *ptr;
+
+	  low = 0;
+	  high = priv->size / 32;
+
+	  do
+	    {
+	      CORE_ADDR pdr_pc;
+
+	      mid = (low + high) / 2;
+
+	      ptr = priv->contents + mid * 32;
+	      pdr_pc = bfd_get_signed_32 (sec->objfile->obfd, ptr);
+	      pdr_pc += ANOFFSET (sec->objfile->section_offsets,
+				  SECT_OFF_TEXT (sec->objfile));
+	      if (pdr_pc == startaddr)
+		break;
+	      if (pdr_pc > startaddr)
+		high = mid;
+	      else
+		low = mid + 1;
+	    }
+	  while (low != high);
+
+	  if (low != high)
+	    {
+	      struct symbol *sym = find_pc_function (pc);
+
+	      /* Fill in what we need of the proc_desc.  */
+	      proc_desc = (mips_extra_func_info_t)
+		obstack_alloc (&sec->objfile->psymbol_obstack,
+			       sizeof (struct mips_extra_func_info));
+	      PROC_LOW_ADDR (proc_desc) = startaddr;
+
+	      /* Only used for dummy frames.  */
+	      PROC_HIGH_ADDR (proc_desc) = 0;
+
+	      PROC_FRAME_OFFSET (proc_desc)
+		= bfd_get_32 (sec->objfile->obfd, ptr + 20);
+	      PROC_FRAME_REG (proc_desc) = bfd_get_32 (sec->objfile->obfd,
+						       ptr + 24);
+	      PROC_FRAME_ADJUST (proc_desc) = 0;
+	      PROC_REG_MASK (proc_desc) = bfd_get_32 (sec->objfile->obfd,
+						      ptr + 4);
+	      PROC_FREG_MASK (proc_desc) = bfd_get_32 (sec->objfile->obfd,
+						       ptr + 12);
+	      PROC_REG_OFFSET (proc_desc) = bfd_get_32 (sec->objfile->obfd,
+							ptr + 8);
+	      PROC_FREG_OFFSET (proc_desc)
+		= bfd_get_32 (sec->objfile->obfd, ptr + 16);
+	      PROC_PC_REG (proc_desc) = bfd_get_32 (sec->objfile->obfd,
+						    ptr + 28);
+	      proc_desc->pdr.isym = (long) sym;
+
+	      return proc_desc;
+	    }
+	}
     }
+
+  if (b == NULL)
+    return NULL;
+
+  if (startaddr > BLOCK_START (b))
+    {
+      /* This is the "pathological" case referred to in a comment in
+	 print_frame_info.  It might be better to move this check into
+	 symbol reading.  */
+      return NULL;
+    }
+
+  sym = lookup_symbol (MIPS_EFI_SYMBOL_NAME, b, LABEL_NAMESPACE, 0, NULL);
 
   /* If we never found a PDR for this function in symbol reading, then
      examine prologues to find the information.  */

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

[Andrew Cagney]

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

Some suggested tweeks do follow,

enjoy,
Andrew
> 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?

>  #define _PROC_MAGIC_ 0x0F0F0F0F
>  #define PROC_DESC_IS_DUMMY(proc) ((proc)->pdr.isym == _PROC_MAGIC_)
> @@ -1932,6 +1934,30 @@ heuristic_proc_desc (CORE_ADDR start_pc,
>    return &temp_proc_desc;
>  }
>  
> +struct mips_objfile_private
> +{
> +  bfd_size_type size;
> +  char *contents;
> +};
> +
> +/* Global used to communicate between non_heuristic_proc_desc and
> +   compare_pdr_entries.  */

.... when doing a qsort.

> +static bfd *the_bfd;

Suggest ``compare_pdr_entries_abfd;''.

> +      /* 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))
> +	{

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.  */

compare_pdr_entries_abfd = sec->objfile->obfd;

> +	      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).

Andrew

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

[Daniel Jacobowitz]

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

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

[Paul Hilfinger]

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

According to its documentation, exc_unwind on IRIX 6.5 currently uses
.debug_frame information for unwinding purposes (and its native
assembler now does not produce PDR sections there).  I am not familiar
with the set of all MIPS platforms, so I am moved to ask whether it
would (1) be a good idea if or (2) be unnecessary for GDB to do the
same thing.  

Paul

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

[Daniel Jacobowitz]

On Thu, Jun 13, 2002 at 10:01:42AM -0700, Paul Hilfinger 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.
> 
> According to its documentation, exc_unwind on IRIX 6.5 currently uses
> .debug_frame information for unwinding purposes (and its native
> assembler now does not produce PDR sections there).  I am not familiar
> with the set of all MIPS platforms, so I am moved to ask whether it
> would (1) be a good idea if or (2) be unnecessary for GDB to do the
> same thing.  

Did the IRIX 6.5 assembler ever produce PDR sections?  I was unaware of
this.

Yes, we should use .debug_frame (when it is available).  Dan Berlin
mentioned this a week or so ago on gcc@ also.  Most of the code is
there, it just needs to be hooked in.

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

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

[Paul Hilfinger]

 > > According to its documentation, exc_unwind on IRIX 6.5 currently uses
 > > .debug_frame information for unwinding purposes (and its native
 > > assembler now does not produce PDR sections there).  I am not familiar
 > > with the set of all MIPS platforms, so I am moved to ask whether it
 > > would (1) be a good idea if or (2) be unnecessary for GDB to do the
 > > same thing.  
 > 
 > Did the IRIX 6.5 assembler ever produce PDR sections?  I was unaware of
 > this.

Sorry for the confusing wording.  I should have said simply that the
IRIX assembler no longer produces PDR information.  (To be honest, I
really haven't followed the evolution of this software enough to know
when last the SGI tool chain DID produce this information, but am
piecing together scraps of historical data by reading out-of-date
comments on current #include files and historical hints in current
man pages.)

Paul

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

[Daniel Jacobowitz]

On Thu, Jun 13, 2002 at 10:49:07AM -0700, Paul Hilfinger wrote:
> 
>  > > According to its documentation, exc_unwind on IRIX 6.5 currently uses
>  > > .debug_frame information for unwinding purposes (and its native
>  > > assembler now does not produce PDR sections there).  I am not familiar
>  > > with the set of all MIPS platforms, so I am moved to ask whether it
>  > > would (1) be a good idea if or (2) be unnecessary for GDB to do the
>  > > same thing.  
>  > 
>  > Did the IRIX 6.5 assembler ever produce PDR sections?  I was unaware of
>  > this.
> 
> Sorry for the confusing wording.  I should have said simply that the
> IRIX assembler no longer produces PDR information.  (To be honest, I
> really haven't followed the evolution of this software enough to know
> when last the SGI tool chain DID produce this information, but am
> piecing together scraps of historical data by reading out-of-date
> comments on current #include files and historical hints in current
> man pages.)

Let me clarify my response too, then... did any IRIX assembler ever
produce PDR sections?  I've been unable to find any evidence of this,
but I haven't spent great effort searching.

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