This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch bfd/mach-o] some tweaks to bfd_mach_o_canonicalize_one_reloc


On Feb 23, 2012, at 3:29 PM, Iain Sandoe wrote:

> 
> On 23 Feb 2012, at 09:11, Tristan Gingold wrote:
>>> ... or a set of macros to do the same.
>> Yes, a set of macros should do it.
> 
> well, doing it completely in macros was looking unwieldy ..
> so I've made two small routines and macro-ized the values (and shifted all this to external.h).
> 
> .. I've done the output side too - but I don't have enough of the PPC port working to test that (output) much.
> .. for [BE] input it's OK and the x86 port has unchanged test results [in & out].
> 
> so, how about this?

Yes, looks good.  One minor nit, see below.

I think the usual name in BFD for encode/decode is swap in/swap out.  Feel free to rename.

Ok for commit,
thank you for the work.

Tristan

> Iain
> 
> BFD:
> 
> 	* mach-o.c (bfd_mach_o_decode_non_scattered_reloc): New.
> 	(bfd_mach_o_canonicalize_one_reloc):  Swap non-scattered reloc
> 	bit-fields when target and host differ in endian-ness.  When
> 	PAIRs are non-scattered	find the 'symbol' from the preceding
> 	reloc.  Add FIXME re. reloc symbols on section boundaries.
> 	(bfd_mach_o_encode_non_scattered_reloc): New.
> 	(bfd_mach_o_write_relocs): Use bfd_mach_o_encode_non_scattered_reloc.
> 
> include/mach-o:
> 
> 	* external.h: Add comments about relocations fields.  Add macros
> 	for non-scattered relocations.  Move scattered relocation macros to here.
> 	* reloc.h: Remove macros related to external representation of reloc fields.
> 
> Index: bfd/mach-o.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/mach-o.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 mach-o.c
> --- bfd/mach-o.c	10 Feb 2012 11:24:44 -0000	1.101
> +++ bfd/mach-o.c	23 Feb 2012 14:14:34 -0000
> @@ -975,6 +975,35 @@ bfd_mach_o_get_reloc_upper_bound (bfd *a
>   return (asect->reloc_count + 1) * sizeof (arelent *);
> }
> 
> +/* In addition to the need to byte-swap the symbol number, the bit positions
> +   of the fields in the relocation information vary per target endian-ness.  */
> +
> +static void
> +bfd_mach_o_decode_non_scattered_reloc (bfd *abfd, bfd_mach_o_reloc_info *rel,
> +				       unsigned char *fields)
> +{
> +  unsigned char info = fields[3];
> +
> +  if (bfd_big_endian (abfd))
> +    {
> +      rel->r_value = (fields[0] << 16) | (fields[1] << 8) | fields[2];
> +      rel->r_type = (info >> BFD_MACH_O_BE_TYPE_SHIFT) & BFD_MACH_O_TYPE_MASK;
> +      rel->r_pcrel = (info & BFD_MACH_O_BE_PCREL) ? 1 : 0;
> +      rel->r_length = (info >> BFD_MACH_O_BE_LENGTH_SHIFT)
> +		      & BFD_MACH_O_LENGTH_MASK;
> +      rel->r_extern = (info & BFD_MACH_O_BE_EXTERN) ? 1 : 0;
> +    }
> +  else
> +    {
> +      rel->r_value = (fields[2] << 16) | (fields[1] << 8) | fields[0];
> +      rel->r_type = (info >> BFD_MACH_O_LE_TYPE_SHIFT) & BFD_MACH_O_TYPE_MASK;
> +      rel->r_pcrel = (info & BFD_MACH_O_LE_PCREL) ? 1 : 0;
> +      rel->r_length = (info >> BFD_MACH_O_LE_LENGTH_SHIFT)
> +		      & BFD_MACH_O_LENGTH_MASK;
> +      rel->r_extern = (info & BFD_MACH_O_LE_EXTERN) ? 1 : 0;
> +    }
> +}
> +
> static int
> bfd_mach_o_canonicalize_one_reloc (bfd *abfd,
>                                    struct mach_o_reloc_info_external *raw,
> @@ -984,20 +1013,28 @@ bfd_mach_o_canonicalize_one_reloc (bfd *
>   bfd_mach_o_backend_data *bed = bfd_mach_o_get_backend_data (abfd);
>   bfd_mach_o_reloc_info reloc;
>   bfd_vma addr;
> -  bfd_vma symnum;
>   asymbol **sym;
> 
>   addr = bfd_get_32 (abfd, raw->r_address);
> -  symnum = bfd_get_32 (abfd, raw->r_symbolnum);
> +  res->sym_ptr_ptr = NULL;
> +  res->addend = 0;
> 
>   if (addr & BFD_MACH_O_SR_SCATTERED)
>     {
>       unsigned int j;
> +      bfd_vma symnum = bfd_get_32 (abfd, raw->r_symbolnum);
> 
> -      /* Scattered relocation.
> -         Extract section and offset from r_value.  */
> -      res->sym_ptr_ptr = NULL;
> -      res->addend = 0;
> +      /* Scattered relocation, can't be extern. */
> +      reloc.r_scattered = 1;
> +      reloc.r_extern = 0;
> +
> +      /*   Extract section and offset from r_value (symnum).  */
> +      reloc.r_value = symnum;
> +      /* FIXME: This breaks when a symbol in a reloc exactly follows the
> +	 end of the data for the section (e.g. in a calculation of section
> +	 data length).  At present, the symbol will end up associated with
> +	 the following section or, if it falls within alignment padding, as
> +	 null - which will assert later.  */
>       for (j = 0; j < mdata->nsects; j++)
>         {
>           bfd_mach_o_section *sect = mdata->sections[j];
> @@ -1008,42 +1045,62 @@ bfd_mach_o_canonicalize_one_reloc (bfd *
>               break;
>             }
>         }
> -      res->address = BFD_MACH_O_GET_SR_ADDRESS (addr);
> +
> +      /* Extract the info and address fields from r_address.  */
>       reloc.r_type = BFD_MACH_O_GET_SR_TYPE (addr);
>       reloc.r_length = BFD_MACH_O_GET_SR_LENGTH (addr);
>       reloc.r_pcrel = addr & BFD_MACH_O_SR_PCREL;
> -      reloc.r_scattered = 1;
> +      reloc.r_address = BFD_MACH_O_GET_SR_TYPE (addr);
> +      res->address = BFD_MACH_O_GET_SR_ADDRESS (addr);
>     }
>   else
>     {
> -      unsigned int num = BFD_MACH_O_GET_R_SYMBOLNUM (symnum);
> -      res->addend = 0;
> -      res->address = addr;
> -      if (symnum & BFD_MACH_O_R_EXTERN)
> -        {
> +      unsigned int num;
> +
> +      /* Non-scattered relocation.  */
> +      reloc.r_scattered = 0;
> +
> +      /* The value and info fields have to be extracted dependent on target
> +         endian-ness.  */
> +      bfd_mach_o_decode_non_scattered_reloc (abfd, &reloc, raw->r_symbolnum);
> +      num = reloc.r_value;
> +
> +      if (reloc.r_extern)
>           sym = syms + num;
> -          reloc.r_extern = 1;
> -        }
> -      else
> +      else if (reloc.r_scattered
> +	       || (reloc.r_type != BFD_MACH_O_GENERIC_RELOC_PAIR))
>         {
>           BFD_ASSERT (num != 0);
>           BFD_ASSERT (num <= mdata->nsects);
>           sym = mdata->sections[num - 1]->bfdsection->symbol_ptr_ptr;
>           /* For a symbol defined in section S, the addend (stored in the
>              binary) contains the address of the section.  To comply with
> -             bfd conventio, substract the section address.
> +             bfd convention, subtract the section address.
>              Use the address from the header, so that the user can modify
>              the vma of the section.  */
>           res->addend = -mdata->sections[num - 1]->addr;
> -          reloc.r_extern = 0;
> +        }
> +      else /* ... The 'symnum' in a non-scattered PAIR will be 0x00ffffff.  */
> +        {
> +          /* Pairs for PPC LO/HI/HA are not scattered, but contain the offset
> +             in the lower 16bits of the address value.  So we have to find the
> +             'symbol' from the preceding reloc.  We do this even thoough the
> +             section symbol is probably not needed here, because NULL symbol
> +             values cause an assert in generic BFD code.  */
> +          sym = (res-1)->sym_ptr_ptr;

res - 1.

>         }
>       res->sym_ptr_ptr = sym;
> -      reloc.r_type = BFD_MACH_O_GET_R_TYPE (symnum);
> -      reloc.r_length = BFD_MACH_O_GET_R_LENGTH (symnum);
> -      reloc.r_pcrel = (symnum & BFD_MACH_O_R_PCREL) ? 1 : 0;
> -      reloc.r_scattered = 0;
> +
> +      /* The 'address' is just r_address.
> +         ??? maybe this should be masked with  0xffffff for safety.  */
> +      res->address = addr;
> +      reloc.r_address = addr;
>     }
> 
> +  /* We have set up a reloc with all the information present, so the swapper can
> +     modify address, value and addend fields, if necessary, to convey information
> +     in the generic BFD reloc that is mach-o specific.  */
> +
>   if (!(*bed->_bfd_mach_o_swap_reloc_in)(res, &reloc))
>     return -1;
>   return 0;
> @@ -1182,6 +1239,41 @@ bfd_mach_o_canonicalize_dynamic_reloc (b
>   return i;
> }
> 
> +/* In addition to the need to byte-swap the symbol number, the bit positions
> +   of the fields in the relocation information vary per target endian-ness.  */
> +
> +static void
> +bfd_mach_o_encode_non_scattered_reloc (bfd *abfd, unsigned char *fields,
> +				       bfd_mach_o_reloc_info *rel)
> +{
> +  unsigned char info = 0;
> +
> +  BFD_ASSERT (rel->r_type <= 15);
> +  BFD_ASSERT (rel->r_length <= 3);
> +
> +  if (bfd_big_endian (abfd))
> +    {
> +      fields[0] = (rel->r_value >> 16) & 0xff;
> +      fields[1] = (rel->r_value >> 8) & 0xff;
> +      fields[2] = rel->r_value & 0xff;
> +      info |= rel->r_type << BFD_MACH_O_BE_TYPE_SHIFT;
> +      info |= rel->r_pcrel ? BFD_MACH_O_BE_PCREL : 0;
> +      info |= rel->r_length << BFD_MACH_O_BE_LENGTH_SHIFT;
> +      info |= rel->r_extern ? BFD_MACH_O_BE_EXTERN : 0;
> +    }
> +  else
> +    {
> +      fields[2] = (rel->r_value >> 16) & 0xff;
> +      fields[1] = (rel->r_value >> 8) & 0xff;
> +      fields[0] = rel->r_value & 0xff;
> +      info |= rel->r_type << BFD_MACH_O_LE_TYPE_SHIFT;
> +      info |= rel->r_pcrel ? BFD_MACH_O_LE_PCREL : 0;
> +      info |= rel->r_length << BFD_MACH_O_LE_LENGTH_SHIFT;
> +      info |= rel->r_extern ? BFD_MACH_O_LE_EXTERN : 0;
> +    }
> +  fields[3] = info;
> +}
> +
> static bfd_boolean
> bfd_mach_o_write_relocs (bfd *abfd, bfd_mach_o_section *section)
> {
> @@ -1228,15 +1320,8 @@ bfd_mach_o_write_relocs (bfd *abfd, bfd_
>         }
>       else
>         {
> -          unsigned long v;
> -
>           bfd_put_32 (abfd, pinfo->r_address, raw.r_address);
> -          v = BFD_MACH_O_SET_R_SYMBOLNUM (pinfo->r_value)
> -            | (pinfo->r_pcrel ? BFD_MACH_O_R_PCREL : 0)
> -            | BFD_MACH_O_SET_R_LENGTH (pinfo->r_length)
> -            | (pinfo->r_extern ? BFD_MACH_O_R_EXTERN : 0)
> -            | BFD_MACH_O_SET_R_TYPE (pinfo->r_type);
> -          bfd_put_32 (abfd, v, raw.r_symbolnum);
> +          bfd_mach_o_encode_non_scattered_reloc (abfd, raw.r_symbolnum, pinfo);
>         }
> 
>       if (bfd_bwrite (&raw, BFD_MACH_O_RELENT_SIZE, abfd)
> Index: include/mach-o/external.h
> ===================================================================
> RCS file: /cvs/src/src/include/mach-o/external.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 external.h
> --- include/mach-o/external.h	4 Jan 2012 10:37:36 -0000	1.4
> +++ include/mach-o/external.h	23 Feb 2012 14:14:34 -0000
> @@ -116,6 +116,45 @@ struct mach_o_reloc_info_external
> };
> #define BFD_MACH_O_RELENT_SIZE 8
> 
> +/* Relocations are based on 'address' being a section offset and an assumption
> +   that sections are never more than 2^24-1 bytes in size.  Relocation data
> +   also carry information on type/size/PC-relative/extern and whether scattered
> +   or not [stored in the MSB of the r_address].  */
> +
> +#define BFD_MACH_O_SR_SCATTERED		0x80000000
> +
> +/* For a non-scattered reloc, the relocation info is found in r_symbolnum.
> +   Bytes 1 to 3 contain the symbol number (0xffffff, in a non-scattered PAIR).
> +   Byte 4 contains the relocation info - but with differing bit-positions
> +   dependent on target endian-ness - as below.  */
> +
> +#define BFD_MACH_O_LE_PCREL		0x01
> +#define BFD_MACH_O_LE_LENGTH_SHIFT	1
> +#define BFD_MACH_O_LE_EXTERN		0x08
> +#define BFD_MACH_O_LE_TYPE_SHIFT	4
> +
> +#define BFD_MACH_O_BE_PCREL		0x80
> +#define BFD_MACH_O_BE_LENGTH_SHIFT	6
> +#define BFD_MACH_O_BE_EXTERN		0x10
> +#define BFD_MACH_O_BE_TYPE_SHIFT	0
> +
> +/* The field sizes are the same for both BE and LE.  */
> +#define BFD_MACH_O_LENGTH_MASK		0x03
> +#define BFD_MACH_O_TYPE_MASK		0x0f
> +
> +/* For a scattered reloc entry the info is contained in r_address.  There
> +   is no need to discriminate on target endian-ness, since the design was
> +   arranged to produce the same layout on both.  Scattered relocations are
> +   only used for local items, therefore there is no 'extern' field.  */
> +
> +#define BFD_MACH_O_SR_PCREL		0x40000000
> +#define BFD_MACH_O_GET_SR_LENGTH(s)	(((s) >> 28) & 0x3)
> +#define BFD_MACH_O_GET_SR_TYPE(s)	(((s) >> 24) & 0x0f)
> +#define BFD_MACH_O_GET_SR_ADDRESS(s)	((s) & 0x00ffffff)
> +#define BFD_MACH_O_SET_SR_LENGTH(l)	(((l) & 0x3) << 28)
> +#define BFD_MACH_O_SET_SR_TYPE(t)	(((t) & 0xf) << 24)
> +#define BFD_MACH_O_SET_SR_ADDRESS(s)	((s) & 0x00ffffff)
> +
> struct mach_o_symtab_command_external
> {
>   unsigned char symoff[4];	/* File offset of the symbol table.  */
> Index: include/mach-o/reloc.h
> ===================================================================
> RCS file: /cvs/src/src/include/mach-o/reloc.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 reloc.h
> --- include/mach-o/reloc.h	8 Aug 2011 08:59:33 -0000	1.1
> +++ include/mach-o/reloc.h	23 Feb 2012 14:14:34 -0000
> @@ -1,5 +1,5 @@
> /* Mach-O support for BFD.
> -   Copyright 2011
> +   Copyright 2011, 2012
>    Free Software Foundation, Inc.
> 
>    This file is part of BFD, the Binary File Descriptor library.
> @@ -22,26 +22,6 @@
> #ifndef _MACH_O_RELOC_H
> #define _MACH_O_RELOC_H
> 
> -/* Fields for a normal (non-scattered) entry.  */
> -#define BFD_MACH_O_R_PCREL		0x01000000
> -#define BFD_MACH_O_GET_R_LENGTH(s)	(((s) >> 25) & 0x3)
> -#define BFD_MACH_O_R_EXTERN		0x08000000
> -#define BFD_MACH_O_GET_R_TYPE(s)	(((s) >> 28) & 0x0f)
> -#define BFD_MACH_O_GET_R_SYMBOLNUM(s)	((s) & 0x00ffffff)
> -#define BFD_MACH_O_SET_R_LENGTH(l)	(((l) & 0x3) << 25)
> -#define BFD_MACH_O_SET_R_TYPE(t)	(((t) & 0xf) << 28)
> -#define BFD_MACH_O_SET_R_SYMBOLNUM(s)	((s) & 0x00ffffff)
> -
> -/* Fields for a scattered entry.  */
> -#define BFD_MACH_O_SR_SCATTERED		0x80000000
> -#define BFD_MACH_O_SR_PCREL		0x40000000
> -#define BFD_MACH_O_GET_SR_LENGTH(s)	(((s) >> 28) & 0x3)
> -#define BFD_MACH_O_GET_SR_TYPE(s)	(((s) >> 24) & 0x0f)
> -#define BFD_MACH_O_GET_SR_ADDRESS(s)	((s) & 0x00ffffff)
> -#define BFD_MACH_O_SET_SR_LENGTH(l)	(((l) & 0x3) << 28)
> -#define BFD_MACH_O_SET_SR_TYPE(t)	(((t) & 0xf) << 24)
> -#define BFD_MACH_O_SET_SR_ADDRESS(s)	((s) & 0x00ffffff)
> -
> /* Generic relocation types (used by i386).  */
> #define BFD_MACH_O_GENERIC_RELOC_VANILLA 	0
> #define BFD_MACH_O_GENERIC_RELOC_PAIR	 	1
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]