[PATCH] fix debugging code compiled for newer PPC BookE processors

[PATCH] fix debugging code compiled for newer PPC BookE processors

[Nathan Froyd]

rs6000_gdbarch_init contains a bit of a hack in that it assumes that the
mere presence of a .PPC.EMB.apuinfo section indicates that an E500
binary is being debugged.  The .PPC.EMB.apuinfo section is actually
quite a bit more general and can appear in object files that have
nothing to do with E500: simply compiling code for (GCC options)
-mcpu=e500mc or -mcpu=e500mc64 will produce an object file containing an
.PPC.EMB.apuinfo section.

The naive handling of .PPC.EMB.apuinfo sections can cause problems
particularly for -mcpu=e500mc64 code, as bfd will indicate that there's
no 64-bit E500 architecture available, leading to internal errors down
the road.

The patch below addresses this by attempting to be smarter about whether
we might be debugging an E500 binary.  It checks the value of
Tag_GNU_Power_ABI_Vector from the .gnu_attributes section and also
attempts to parse any .PPC.EMB.apuinfo section, looking for
E500-specific markers.  If either of these tests succeed, then we are
debugging an E500 binary.

Tested with cross to powerpc-linux-gnu.  OK to commit?

-Nathan

	* rs6000-tdep.c (bfd_uses_spe_extensions): New function.
	(rs6000_gdbarch_init): Call it.
---
 gdb/rs6000-tdep.c |  135 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 124 insertions(+), 11 deletions(-)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index ef049d9..caf9d6b 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3350,6 +3350,123 @@ ppc_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
 }
 
 
+/* Return true if a .gnu_attributes section exists in BFD and it
+   indicates we are using SPE extensions OR if a .PPC.EMB.apuinfo
+   section exists in BFD and it indicates that SPE extensions are in
+   use.  Check the .gnu.attributes section first, as the binary might be
+   compiled for SPE, but not actually using SPE instructions.  */
+
+static int
+bfd_uses_spe_extensions (bfd *abfd)
+{
+  asection *sect;
+  gdb_byte *contents = NULL;
+  bfd_size_type size;
+  gdb_byte *ptr;
+  int success = 0;
+  int vector_abi;
+
+  if (!abfd)
+    return 0;
+
+  /* Using Tag_GNU_Power_ABI_Vector here is a bit of a hack, as the user
+     could be using the SPE vector abi without actually using any spe
+     bits whatsoever.  But it's close enough for now.  */
+  vector_abi = bfd_elf_get_obj_attr_int (abfd, OBJ_ATTR_GNU,
+					 Tag_GNU_Power_ABI_Vector);
+  if (vector_abi == 3)
+    return 1;
+
+  sect = bfd_get_section_by_name (abfd, ".PPC.EMB.apuinfo");
+  if (!sect)
+    return 0;
+
+  size = bfd_get_section_size (sect);
+  contents = xmalloc (size);
+  if (!bfd_get_section_contents (abfd, sect, contents, 0, size))
+    {
+      xfree (contents);
+      return 0;
+    }
+
+  /* Parse the .PPC.EMB.apuinfo section.  The layout is as follows:
+
+     struct {
+       uint32 name_len;
+       uint32 data_len;
+       uint32 type;
+       char name[name_len rounded up to 4-byte alignment];
+       char data[data_len];
+     };
+
+     Technically, there's only supposed to be one such structure in a
+     given apuinfo section, but the linker is not always vigilant about
+     merging apuinfo sections from input files.  Just go ahead and parse
+     them all, exiting early when we discover the binary uses SPE
+     insns.
+
+     It's not specified in what endianness the information in this
+     section is stored.  Assume that it's the endianness of the BFD.  */
+  ptr = contents;
+  while (1)
+    {
+      unsigned int name_len;
+      unsigned int data_len;
+      unsigned int type;
+
+      /* If we can't read the first three fields, we're done.  */
+      if (size < 12)
+	break;
+
+      name_len = bfd_get_32 (abfd, ptr);
+      name_len = (name_len + 3) & ~3U; /* Round to 4 bytes.  */
+      data_len = bfd_get_32 (abfd, ptr + 4);
+      type = bfd_get_32 (abfd, ptr + 8);
+      ptr += 12;
+
+      /* The name must be "APUinfo\0".  */
+      if (name_len != 8
+	  && strcmp ((const char *) ptr, "APUinfo") != 0)
+	break;
+      ptr += name_len;
+
+      /* The type must be 2.  */
+      if (type != 2)
+	break;
+
+      /* The data is stored as a series of uint32.  The upper half of
+	 each uint32 indicates the particular APU used and the lower
+	 half indicates the revision of that APU.  We just care about
+	 the upper half.  */
+
+      /* Not 4-byte quantities.  */
+      if (data_len & 3U)
+	break;
+
+      while (data_len)
+	{
+	  unsigned int apuinfo = bfd_get_32 (abfd, ptr);
+	  unsigned int apu = apuinfo >> 16;
+	  ptr += 4;
+	  data_len -= 4;
+
+	  /* The SPE APU is 0x100; the SPEFP APU is 0x101.  Accept
+	     either.  */
+	  if (apu == 0x100 || apu == 0x101)
+	    {
+	      success = 1;
+	      data_len = 0;
+	    }
+	}
+
+      if (success)
+	break;
+    }
+
+  xfree (contents);
+  return success;
+}
+
 /* Initialize the current architecture based on INFO.  If possible, re-use an
    architecture from ARCHES, which is a list of architectures already created
    during this debugging session.
@@ -3430,19 +3547,15 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
      Application-specific Processing Unit that is present on the
      chip.  The content of the section is determined by the assembler
      which looks at each instruction and determines which unit (and
-     which version of it) can execute it. In our case we just look for
-     the existance of the section.  */
+     which version of it) can execute it.  Grovel through the section
+     looking for relevant e500 APUs.  */
 
-  if (info.abfd)
+  if (bfd_uses_spe_extensions (info.abfd))
     {
-      sect = bfd_get_section_by_name (info.abfd, ".PPC.EMB.apuinfo");
-      if (sect)
-	{
-	  arch = info.bfd_arch_info->arch;
-	  mach = bfd_mach_ppc_e500;
-	  bfd_default_set_arch_mach (&abfd, arch, mach);
-	  info.bfd_arch_info = bfd_get_arch_info (&abfd);
-	}
+      arch = info.bfd_arch_info->arch;
+      mach = bfd_mach_ppc_e500;
+      bfd_default_set_arch_mach (&abfd, arch, mach);
+      info.bfd_arch_info = bfd_get_arch_info (&abfd);
     }
 
   /* Find a default target description which describes our register
-- 
1.6.3.2

Re: [PATCH] fix debugging code compiled for newer PPC BookE processors

[Joel Brobecker]

> -Nathan
> 
> 	* rs6000-tdep.c (bfd_uses_spe_extensions): New function.
> 	(rs6000_gdbarch_init): Call it.

This looks reasonable to me.  Just one question, but please go ahead
and commit if you think the code is right.


> +      /* The type must be 2.  */
> +      if (type != 2)
> +	break;

So, the type must always be 2 and I am to understand that if type is not
2, then the data is screwed, hence the early exit? What's the purpose of
that field, if it is always the same value?

-- 
Joel

Re: [PATCH] fix debugging code compiled for newer PPC BookE processors

[Nathan Froyd]

On Tue, Nov 02, 2010 at 11:23:26AM -0700, Joel Brobecker wrote:
> > +      /* The type must be 2.  */
> > +      if (type != 2)
> > +	break;
> 
> So, the type must always be 2 and I am to understand that if type is not
> 2, then the data is screwed, hence the early exit? What's the purpose of
> that field, if it is always the same value?

I don't know exactly what the purpose is.  The E500 ABI User's Guide
says simply:

  For the .PPC.EMB.apuinfo section, the name shall be “APUinfo\0”, the
  type shall be 2 (as type 1 is already reserved for ELF_NOTE_ABI), and
  the data shall contain a series of words containing APU information,
  one per word. The APU information contains two unsigned halfwords: the
  upper half contains the unique APU identifier, and the lower half
  contains the revision of that APU.

So, there's the rationale.

-Nathan

Re: [PATCH] fix debugging code compiled for newer PPC BookE processors

[Joel Brobecker]

> I don't know exactly what the purpose is.  The E500 ABI User's Guide
> says simply:
> 
>   For the .PPC.EMB.apuinfo section, the name shall be ???APUinfo\0???, the
>   type shall be 2 (as type 1 is already reserved for ELF_NOTE_ABI), and
>   the data shall contain a series of words containing APU information,
>   one per word. The APU information contains two unsigned halfwords: the
>   upper half contains the unique APU identi???er, and the lower half
>   contains the revision of that APU.

Interesting. Perhaps the purpose is for cases where all those separate
sections can no longer be separate, but merged somehow somewhere.
In that case, the type becomes important whereas, right now, it's only
redundant with the section name...

In any case, the patch looked fine to me.

-- 
Joel

[commit] Build failure on ppc-aix (bfd_elf_get_obj_attr_int is undefined)

[Joel Brobecker]

Hello,

The following change introduced an unconditional use of a bfd/ELF routine:

        * rs6000-tdep.c (bfd_uses_spe_extensions): New function.
        (rs6000_gdbarch_init): Call it.

However, bfd_uses_spe_extensions should only be used when BFD has been built
with ELF support.  The typical way of checking that in GDB is to use
the HAVE_ELF macro.

I think that the attached patch handles things the proper way:
  - If ELF is supported, then using the attribute;
  - Otherwise, just skip that test, and use the other methods.

Nathan, if you could test this patch on your end of things, to make sure
I didn't break anything, that'd be great.

gdb/ChangeLog:

        * rs6000-tdep.c (bfd_uses_spe_extensions): Use bfd_elf_get_obj_attr_int
        only if HAVE_ELF is defined.

In the meantime, I have checked this patch in, since if fixes a build
failure.  Tested on ppc-aix as well as x86_64-linux (kind of useless,
but since I did include that patch in the batch of testing...).

---
 gdb/ChangeLog     |    5 +++++
 gdb/rs6000-tdep.c |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 03bf65d..869b8d7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2010-11-24  Joel Brobecker  <brobecker@adacore.com>
+
+	* rs6000-tdep.c (bfd_uses_spe_extensions): Use bfd_elf_get_obj_attr_int
+	only if HAVE_ELF is defined.
+
 2010-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Code cleanup.
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 53c3f4c..81a99b6 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3376,6 +3376,7 @@ bfd_uses_spe_extensions (bfd *abfd)
   if (!abfd)
     return 0;
 
+#ifdef HAVE_ELF
   /* Using Tag_GNU_Power_ABI_Vector here is a bit of a hack, as the user
      could be using the SPE vector abi without actually using any spe
      bits whatsoever.  But it's close enough for now.  */
@@ -3383,6 +3384,7 @@ bfd_uses_spe_extensions (bfd *abfd)
 					 Tag_GNU_Power_ABI_Vector);
   if (vector_abi == 3)
     return 1;
+#endif
 
   sect = bfd_get_section_by_name (abfd, ".PPC.EMB.apuinfo");
   if (!sect)
-- 
1.7.1

Re: [commit] Build failure on ppc-aix (bfd_elf_get_obj_attr_int is undefined)

[Nathan Froyd]

On Wed, Nov 24, 2010 at 04:28:59PM -0800, Joel Brobecker wrote:
> Nathan, if you could test this patch on your end of things, to make sure
> I didn't break anything, that'd be great.

Looks good to me.  Sorry about breaking things; thanks for cleaning up
after me.

-Nathan