[patch] Fix power7 ppc64 binaries loading

[patch] Fix power7 ppc64 binaries loading

[Jan Kratochvil]

Hi,

on the binary from:
./gas/as-new -mpower7 -o /tmp/power7c.o ./gas/testsuite/gas/ppc/power7.s
  Class:                             ELF64
  Machine:                           PowerPC64
Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 4] .PPC.EMB.apuinfo  PROGBITS         0000000000000000  000001e0
       0000000000000018  0000000000000000           0     0     1

FSF GDB HEAD prints:
$ ./gdb -q -nx -ex 'set pagination off' -ex 'x/100i 0' /tmp/power7.o 
</dev/null 
Architecture of file not recognized.
0x0: Cannot access memory at address 0x0
(gdb) quit

With the attached patch it prints an instruction dump.

It is a regression for power7 ppc64 binaries by:
http://sourceware.org/ml/gdb-patches/2002-08/threads.html#00629
http://sourceware.org/ml/gdb-cvs/2002-08/msg00263.html

> A powerpc executable doesn't have indications of which variant it is meant to
> run on.
There is the tdesc_* infrastructure for this purpose for remote targets, there
is a `info->disassembler_options = "any"' hack for local target by:
http://sourceware.org/ml/gdb-patches/2007-02/msg00000.html
http://sourceware.org/ml/gdb-cvs/2007-02/msg00061.html

Regression tested on powerpc64-unknown-linux-gnu (PPC970MP).


Thanks,
Jan

gdb/
2009-03-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix power7 ppc64 binaries loading.
	* rs6000-tdep.c (rs6000_gdbarch_init): Remove variable `sect'.  Remove
	the ppc32 e500 detection from `.PPC.EMB.apuinfo' section existence.

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 926d0b6..1c59a42 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3326,7 +3331,6 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   enum bfd_architecture arch;
   unsigned long mach;
   bfd abfd;
-  asection *sect;
   enum auto_boolean soft_float_flag = powerpc_soft_float_global;
   int soft_float;
   enum powerpc_vector_abi vector_abi = powerpc_vector_abi_global;
@@ -3375,26 +3379,6 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   arch = info.bfd_arch_info->arch;
   mach = info.bfd_arch_info->mach;
 
-  /* For e500 executables, the apuinfo section is of help here.  Such
-     section contains the identifier and revision number of each
-     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.  */
-
-  if (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);
-	}
-    }
-
   /* Find a default target description which describes our register
      layout, if we do not already have one.  */
   if (! tdesc_has_registers (tdesc))

Re: [patch] Fix power7 ppc64 binaries loading

[Daniel Jacobowitz]

On Mon, Mar 02, 2009 at 10:02:38PM +0100, Jan Kratochvil wrote:
> > A powerpc executable doesn't have indications of which variant it is meant to
> > run on.
> There is the tdesc_* infrastructure for this purpose for remote targets, there
> is a `info->disassembler_options = "any"' hack for local target by:
> http://sourceware.org/ml/gdb-patches/2007-02/msg00000.html
> http://sourceware.org/ml/gdb-cvs/2007-02/msg00061.html

Did you test this on an e500 binary?  "any" won't work, we should
probably parse the apuinfo dump instead of assuming it always means
e500.  SPE is incompatible with classic FP.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix power7 ppc64 binaries loading

[Jan Kratochvil]

On Mon, 02 Mar 2009 22:24:03 +0100, Daniel Jacobowitz wrote:
> Did you test this on an e500 binary?  "any" won't work, we should
> probably parse the apuinfo dump instead of assuming it always means
> e500.  SPE is incompatible with classic FP.

No e500 test by hand.  But GDB does not work out of the box even for power7
(and possibly others).

I agree it may be a regression for e500 binaries.  Although the second patch
makes it possible to handle all using `set powerpc disassembler-options'.
I agree apuinfo should be parsed.


Thanks,
Jan

Re: [patch] Fix power7 ppc64 binaries loading

[Thiago Jung Bauermann]

El lun, 02-03-2009 a las 22:33 +0100, Jan Kratochvil escribió:
> On Mon, 02 Mar 2009 22:24:03 +0100, Daniel Jacobowitz wrote:
> > Did you test this on an e500 binary?  "any" won't work, we should
> > probably parse the apuinfo dump instead of assuming it always means
> > e500.  SPE is incompatible with classic FP.
> 
> No e500 test by hand.  But GDB does not work out of the box even for power7
> (and possibly others).
> 
> I agree it may be a regression for e500 binaries.  Although the second patch
> makes it possible to handle all using `set powerpc disassembler-options'.
> I agree apuinfo should be parsed.

Does apuinfo get generated for every binary in every platform? Why
wasn't this an issue before? It just started being generated recently?

In any case, if it tells which processor model the binary needs, then it
could be used as method for an "auto" setting in disassembler-options.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [patch] Fix power7 ppc64 binaries loading

[Daniel Jacobowitz]

On Tue, Mar 03, 2009 at 05:24:41PM -0300, Thiago Jung Bauermann wrote:
> Does apuinfo get generated for every binary in every platform? Why
> wasn't this an issue before? It just started being generated recently?

I assume it is only generated for specific ISA extensions - and that
both POWER7 and e500 have them.  GAS says:

  if (ppc_cpu & (PPC_OPCODE_SPE
               | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
               | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
               | PPC_OPCODE_RFMCI))

I don't know what all of those mean.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix power7 ppc64 binaries loading

[Peter Bergner]

On Tue, 2009-03-03 at 15:30 -0500, Daniel Jacobowitz wrote:
> On Tue, Mar 03, 2009 at 05:24:41PM -0300, Thiago Jung Bauermann wrote:
> > Does apuinfo get generated for every binary in every platform? Why
> > wasn't this an issue before? It just started being generated recently?
> 
> I assume it is only generated for specific ISA extensions - and that
> both POWER7 and e500 have them.  GAS says:
> 
>   if (ppc_cpu & (PPC_OPCODE_SPE
>                | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
>                | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
>                | PPC_OPCODE_RFMCI))

With Power ISA 2.06 which was just recently announced on power.org, the
isel instruction has now moved from an optional instruction to a required
instruction.  The POWER7 processor is ISA 2.06 compliant, so it has the
PPC_OPCODE_ISEL flag set which triggers this...as will all future Power
processors.

That said, I have no idea what the apuinfo section is being used for.

Peter

Re: [patch] Fix power7 ppc64 binaries loading

[Alan Modra]

On Tue, Mar 03, 2009 at 02:54:59PM -0600, Peter Bergner wrote:
> On Tue, 2009-03-03 at 15:30 -0500, Daniel Jacobowitz wrote:
> > On Tue, Mar 03, 2009 at 05:24:41PM -0300, Thiago Jung Bauermann wrote:
> > > Does apuinfo get generated for every binary in every platform? Why
> > > wasn't this an issue before? It just started being generated recently?
> > 
> > I assume it is only generated for specific ISA extensions - and that
> > both POWER7 and e500 have them.  GAS says:
> > 
> >   if (ppc_cpu & (PPC_OPCODE_SPE
> >                | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
> >                | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
> >                | PPC_OPCODE_RFMCI))
> 
> With Power ISA 2.06 which was just recently announced on power.org, the
> isel instruction has now moved from an optional instruction to a required
> instruction.  The POWER7 processor is ISA 2.06 compliant, so it has the
> PPC_OPCODE_ISEL flag set which triggers this...as will all future Power
> processors.
> 
> That said, I have no idea what the apuinfo section is being used for.

Google for E500 ABI.  It should only be enabled when you are
assembling for e500.  I've committed the following to mainline
and 2.19 branch.

       * config/tc-ppc.c (md_assemble): APUinfo only for e500.

Index: gas/config/tc-ppc.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ppc.c,v
retrieving revision 1.147
diff -u -p -r1.147 tc-ppc.c
--- gas/config/tc-ppc.c	26 Feb 2009 22:07:32 -0000	1.147
+++ gas/config/tc-ppc.c	3 Mar 2009 22:54:35 -0000
@@ -2965,10 +3038,7 @@ md_assemble (char *str)
 
 #ifdef OBJ_ELF
   /* Do we need/want a APUinfo section? */
-  if (ppc_cpu & (PPC_OPCODE_SPE
-   	       | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
-	       | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
-	       | PPC_OPCODE_RFMCI))
+  if ((ppc_cpu & PPC_OPCODE_E500MC) != 0)
     {
       /* These are all version "1".  */
       if (opcode->flags & PPC_OPCODE_SPE)


-- 
Alan Modra
Australia Development Lab, IBM

Re: [patch] Fix power7 ppc64 binaries loading

[Joseph S. Myers]

On Wed, 4 Mar 2009, Alan Modra wrote:

> Google for E500 ABI.  It should only be enabled when you are
> assembling for e500.  I've committed the following to mainline
> and 2.19 branch.
> 
>        * config/tc-ppc.c (md_assemble): APUinfo only for e500.

> -  if (ppc_cpu & (PPC_OPCODE_SPE
> -   	       | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
> -	       | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
> -	       | PPC_OPCODE_RFMCI))
> +  if ((ppc_cpu & PPC_OPCODE_E500MC) != 0)

Most e500 are not e500mc (indeed, e500mc is mostly like classic PowerPC at 
the userspace level, not like e500).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [patch] Fix power7 ppc64 binaries loading

[Daniel Jacobowitz]

On Tue, Mar 03, 2009 at 11:30:27PM +0000, Joseph S. Myers wrote:
> On Wed, 4 Mar 2009, Alan Modra wrote:
> 
> > Google for E500 ABI.  It should only be enabled when you are
> > assembling for e500.  I've committed the following to mainline
> > and 2.19 branch.
> > 
> >        * config/tc-ppc.c (md_assemble): APUinfo only for e500.
> 
> > -  if (ppc_cpu & (PPC_OPCODE_SPE
> > -   	       | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
> > -	       | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
> > -	       | PPC_OPCODE_RFMCI))
> > +  if ((ppc_cpu & PPC_OPCODE_E500MC) != 0)
> 
> Most e500 are not e500mc (indeed, e500mc is mostly like classic PowerPC at 
> the userspace level, not like e500).

The check is right - I had to go look in the assembler option parsing
to figure it out though, so might deserve a comment.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix power7 ppc64 binaries loading

[Alan Modra]

On Tue, Mar 03, 2009 at 11:30:27PM +0000, Joseph S. Myers wrote:
> On Wed, 4 Mar 2009, Alan Modra wrote:
> 
> > Google for E500 ABI.  It should only be enabled when you are
> > assembling for e500.  I've committed the following to mainline
> > and 2.19 branch.
> > 
> >        * config/tc-ppc.c (md_assemble): APUinfo only for e500.
> 
> > -  if (ppc_cpu & (PPC_OPCODE_SPE
> > -   	       | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
> > -	       | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
> > -	       | PPC_OPCODE_RFMCI))
> > +  if ((ppc_cpu & PPC_OPCODE_E500MC) != 0)
> 
> Most e500 are not e500mc (indeed, e500mc is mostly like classic PowerPC at 
> the userspace level, not like e500).

The effect of the new test is to generate .PPC.EMB.apuinfo when
gas is given any of -me500, -me500x2 or -me500mc options.  Is that
not correct?  I freely admit to not being 100% certain.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [patch] Fix power7 ppc64 binaries loading

[Joseph S. Myers]

On Wed, 4 Mar 2009, Alan Modra wrote:

> On Tue, Mar 03, 2009 at 11:30:27PM +0000, Joseph S. Myers wrote:
> > On Wed, 4 Mar 2009, Alan Modra wrote:
> > 
> > > Google for E500 ABI.  It should only be enabled when you are
> > > assembling for e500.  I've committed the following to mainline
> > > and 2.19 branch.
> > > 
> > >        * config/tc-ppc.c (md_assemble): APUinfo only for e500.
> > 
> > > -  if (ppc_cpu & (PPC_OPCODE_SPE
> > > -   	       | PPC_OPCODE_ISEL | PPC_OPCODE_EFS
> > > -	       | PPC_OPCODE_BRLOCK | PPC_OPCODE_PMR | PPC_OPCODE_CACHELCK
> > > -	       | PPC_OPCODE_RFMCI))
> > > +  if ((ppc_cpu & PPC_OPCODE_E500MC) != 0)
> > 
> > Most e500 are not e500mc (indeed, e500mc is mostly like classic PowerPC at 
> > the userspace level, not like e500).
> 
> The effect of the new test is to generate .PPC.EMB.apuinfo when
> gas is given any of -me500, -me500x2 or -me500mc options.  Is that
> not correct?  I freely admit to not being 100% certain.

That seems correct; it just seems odd for PPC_OPCODE_E500MC to apply also 
to the other e500 processors.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [patch] Fix power7 ppc64 binaries loading

[Thiago Jung Bauermann]

El mié, 04-03-2009 a las 09:46 +1030, Alan Modra escribió:
>        * config/tc-ppc.c (md_assemble): APUinfo only for e500.

If I'm following things correctly, we still want to reorder power2 and
power7 instructions so that -Many works.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [patch] Fix power7 ppc64 binaries loading

[Alan Modra]

On Tue, Mar 03, 2009 at 08:49:12PM -0300, Thiago Jung Bauermann wrote:
> El mié, 04-03-2009 a las 09:46 +1030, Alan Modra escribió:
> >        * config/tc-ppc.c (md_assemble): APUinfo only for e500.
> 
> If I'm following things correctly, we still want to reorder power2 and
> power7 instructions so that -Many works.

Yes.  I OK'd Peter's patch but trimmed the cc list.  Sorry, it appears
I misjudged the level of interest.  :-)

-- 
Alan Modra
Australia Development Lab, IBM