Re: [BUG?] GCC 4.5.2 produces deprecated ARM relocation

[Matthew Gretton-Dann <matthew dot gretton-dann at arm dot com>]

Nick,

On 26/10/11 13:11, Nick Clifton wrote:
> Hi Richard,
>
> > > GCC is adding this suffix because the -fPIC flag enables NEED_PLT_RELOC
> > > (see gcc/config/arm/elf.h).  Why elf.h is doing this I am not sure -
> > > backwards compatibility maybe ?  Anyway it is GCC that needs fixing, not
> > > GAS.
>
> > I disagree, gas should just ignore the suffix when generating EABI
> > conforming objects.
>
> Well I based my conclusion upon this note in gas/doc/c-arm.texi:
>
>      For compatibility with older toolchains the assembler
>      also accepts @code{(PLT)} after branch targets.  This
>      will generate the deprecated @samp{R_ARM_PLT32} relocation.
>
> I do not mind enhancing the assembler however, so what do you think of
> the attached patch ?

Some comments below.

> Cheers
>     Nick
>
> gas/ChangeLog
> 2011-10-26  Nick Clifton<nickc@redhat.com>
>
> 	* config/tc-arm.c (md_begin): Remove ARM_PLT32 reloc associated
> 	with the (PLT) instruction suffix when operating in eabi mode.
> 	* doc/c-arm.texi (ARM_Relocations): Extend description of (PLT)
> 	suffix.
>
>
> arm.gas.plt32.patch
>
>
> Index: gas/config/tc-arm.c
> ===================================================================
> RCS file: /cvs/src/src/gas/config/tc-arm.c,v
> retrieving revision 1.502
> diff -u -3 -p -r1.502 tc-arm.c
> --- gas/config/tc-arm.c	18 Oct 2011 14:41:53 -0000	1.502
> +++ gas/config/tc-arm.c	26 Oct 2011 12:03:34 -0000
> @@ -32,7 +32,7 @@
>   #include "safe-ctype.h"
>   #include "subsegs.h"
>   #include "obstack.h"
> -
> +#include "libiberty.h"
>   #include "opcode/arm.h"
>
>   #ifdef OBJ_ELF
> @@ -22420,8 +22422,16 @@ md_begin (void)
>       hash_insert (arm_barrier_opt_hsh, barrier_opt_names[i].template_name,
>   		 (void *) (barrier_opt_names + i));
>   #ifdef OBJ_ELF
> -  for (i = 0; i<  sizeof (reloc_names) / sizeof (struct reloc_entry); i++)
> -    hash_insert (arm_reloc_hsh, reloc_names[i].name, (void *) (reloc_names + i));
> +  for (i = 0; i<  ARRAY_SIZE (reloc_names); i++)
> +    {
> +      struct reloc_entry * entry = reloc_names + i;
> +
> +      if (arm_is_eabi()&&  entry->reloc == BFD_RELOC_ARM_PLT32)
> +	/* This makes encode_branch() use the EABI versions of this relocation.  */
> +	entry->reloc = BFD_RELOC_UNUSED;
> +
> +      hash_insert (arm_reloc_hsh, entry->name, (void *) entry);
> +    }

While this is probably the simplest way to fix this in terms of code 
change I wonder whether it would be better to actually modify the 
encode_branch function?

By this I mean change the following:

> inst.reloc.type = inst.operands[0].imm == BFD_RELOC_ARM_PLT32
>   ? BFD_RELOC_ARM_PLT32
>   : thumb_mode ? BFD_RELOC_ARM_THM_TLS_CALL : BFD_RELOC_ARM_TLS_CALL;

so that when we are on an EABI target we set inst.reloc.type to 
default_reloc.

This also then allows us to ensure that the BFD_RELOC_ARM_* value in 
default_reloc really maps to R_ARM_CALL or R_ARM_JUMP24 as the 
documentation says :-).

I'm also concerned at else will happen if we stop marking these 
relocations as BFD_RELOC_ARM_PLT32.  For instance in arm_fix_adjustable 
there is code to not allow symbols to be discarded on GOT related relocs 
which depends on this relocation type.

>   #endif
>
>     set_constant_flonums ();
> Index: gas/doc/c-arm.texi
> ===================================================================
> RCS file: /cvs/src/src/gas/doc/c-arm.texi,v
> retrieving revision 1.85
> diff -u -3 -p -r1.85 c-arm.texi
> --- gas/doc/c-arm.texi	31 May 2011 14:12:54 -0000	1.85
> +++ gas/doc/c-arm.texi	26 Oct 2011 12:03:34 -0000
> @@ -493,8 +493,10 @@ and
>   @code{TPOFF}.
>
>   For compatibility with older toolchains the assembler also accepts
> -@code{(PLT)} after branch targets.  This will generate the deprecated
> -@samp{R_ARM_PLT32} relocation.
> +@code{(PLT)} after branch targets.  On legacy targets this will
> +generate the deprecated @samp{R_ARM_PLT32} relocation.  On EABI
> +targets it will encode either the @samp{R_ARM_CALL} or
> +@samp{R_ARM_JUMP24} relocation, as appropriate.
>
>   @cindex MOVW and MOVT relocations, ARM
>   Relocations for @samp{MOVW} and @samp{MOVT} instructions can be generated
> Index: gas/testsuite/gas/arm/pic.d
> ===================================================================
> RCS file: /cvs/src/src/gas/testsuite/gas/arm/pic.d,v
> retrieving revision 1.15
> diff -u -3 -p -r1.15 pic.d
> --- gas/testsuite/gas/arm/pic.d	7 Mar 2006 08:39:21 -0000	1.15
> +++ gas/testsuite/gas/arm/pic.d	26 Oct 2011 12:03:34 -0000
> @@ -13,7 +13,7 @@ Disassembly of section .text:
>   00+0<[^>]*>  eb...... 	bl	00+.<[^>]*>
>   			0: R_ARM_(PC24|CALL)	foo.*
>   00+4<[^>]*>  eb...... 	bl	0[0123456789abcdef]+<[^>]*>
> -			4: R_ARM_PLT32	foo
> +			4: R_ARM_(PLT32|CALL)	foo
>   	\.\.\.
>   			8: R_ARM_ABS32	sym
>   			c: R_ARM_GOT32	sym

Thanks,

Matt
--
Matthew Gretton-Dann
Principal Engineer, PD Software - Tools, ARM Ltd