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 22, 2012, at 5:35 PM, Iain Sandoe wrote:

> 
> I tried looking at a ppc object with objdump, and got a seg-fault - which was eventually traced to an icky situation that we need to swap the bit-fields in non-scattered relocs.  This is (kinda) documented in the system /usr/include/mach-o/reloc.h (it's documented by the absence of an endian-dependent version of the non-scattered reloc, where the comments indicate that such is needed for the scattered one).
> 
> This testing also revealed that the PAIR does not have to be scattered (and that the sym value needs to be found differently for such).
> 
> [that's what this patch fixes - at least for the input side]

Ian,

I think we should really avoid to have structures with bit fields in mach-o/external.h, simply because the representation is not well defined.

Fortunately, these relocs shouldn't be hard to be defined using unsigned char, because the bounds are byte aligned.
So I'd suggest to manually extract r_type,r_extern,r_length and r_pcrel according to header_byteorder field of the target.

Thank you for fixing this issue, I was not sure about the endiannes when I wrote this code and I don't have access anymore to a ppc darwin system.

> ---- NOTE:
> 
> There is also a lurking problem (not fixed by this patch, but FIXME'd) that reloc symbols that appear right at the end of the section data e.g.
> 
> 	.text
> Lstart:
> 	some stuff
> Lend:
> 	.a_different_section ...
> 
> Lend would (if emitted as part of a reloc) end up associated with "a_different_section" or nul if "a_different_section" required some alignment padding.  IIRC, the vendor's tool-chain used to have this problem... I think we'll need to fix it by doing the canonicalize operations per section, but not looked hard yet.

This part is good!

Tristan.

> 
> ----
> 
> so patch to fix endian-ness of bit-fields in bfd_mach_o_canonicalize_one_reloc non-scattered
> OK?
> Iain
> 
> include:
> 
> 	* mach-o/external.h (bfd_mach_o_reg_reloc_swap): New typedef.
> 
> bfd:
> 	* mach-o.c (swap_relocs): New var.
> 	(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.c              |   58 +++++++++++++++++++++++++++++++++++---------
> include/mach-o/external.h |   27 +++++++++++++++++++++
> 2 files changed, 73 insertions(+), 12 deletions(-)
> 
> diff --git a/bfd/mach-o.c b/bfd/mach-o.c
> index ff84de0..4e86cc9 100644
> --- a/bfd/mach-o.c
> +++ b/bfd/mach-o.c
> @@ -975,6 +975,8 @@ bfd_mach_o_get_reloc_upper_bound (bfd *abfd ATTRIBUTE_UNUSED,
>   return (asect->reloc_count + 1) * sizeof (arelent *);
> }
> 
> +static bfd_boolean swap_relocs = FALSE;
> +
> static int
> bfd_mach_o_canonicalize_one_reloc (bfd *abfd,
>                                    struct mach_o_reloc_info_external *raw,
> @@ -998,6 +1000,11 @@ bfd_mach_o_canonicalize_one_reloc (bfd *abfd,
>          Extract section and offset from r_value.  */
>       res->sym_ptr_ptr = NULL;
>       res->addend = 0;
> +      /* 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];
> @@ -1016,32 +1023,55 @@ bfd_mach_o_canonicalize_one_reloc (bfd *abfd,
>     }
>   else
>     {
> -      unsigned int num = BFD_MACH_O_GET_R_SYMBOLNUM (symnum);
> +      bfd_mach_o_reg_reloc_swap s;
> +      unsigned int num;
> +
>       res->addend = 0;
>       res->address = addr;
> -      if (symnum & BFD_MACH_O_R_EXTERN)
> -        {
> -          sym = syms + num;
> -          reloc.r_extern = 1;
> -        }
> +      reloc.r_scattered = 0;
> +      /* For the 'regular' reloc we need to account for the endian-ness of
> +	 the bitfields.  */
> +      s.access = symnum;
> +      if (swap_relocs)
> +	{
> +	  reloc.r_type = s.rev.r_type;
> +	  reloc.r_extern = s.rev.r_extern;
> +	  reloc.r_length = s.rev.r_length;
> +	  reloc.r_pcrel = s.rev.r_pcrel;
> +	  num = s.rev.r_symbolnum;
> +	}
>       else
> +	{
> +	  num = s.fwd.r_symbolnum;
> +	  reloc.r_pcrel = s.fwd.r_pcrel;
> +	  reloc.r_length = s.fwd.r_length;
> +	  reloc.r_extern = s.fwd.r_extern;
> +	  reloc.r_type = s.fwd.r_type;
> +	}
> +
> +      if (reloc.r_extern)
> +        sym = syms + num;
> +      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 appears to be 0x00ffffff ('-1' 24 bits) .  */
> +        {
> +          /* 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.  */
> +          sym = (res-1)->sym_ptr_ptr;
>         }
>       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;
>     }
> 
>   if (!(*bed->_bfd_mach_o_swap_reloc_in)(res, &reloc))
> @@ -1058,6 +1088,10 @@ bfd_mach_o_canonicalize_relocs (bfd *abfd, unsigned long filepos,
>   struct mach_o_reloc_info_external *native_relocs;
>   bfd_size_type native_size;
> 
> +  i = 1;
> +  if (bfd_get_32 (abfd, &i) != 1)
> +    swap_relocs = TRUE;
> +
>   /* Allocate and read relocs.  */
>   native_size = count * BFD_MACH_O_RELENT_SIZE;
>   native_relocs =
> diff --git a/include/mach-o/external.h b/include/mach-o/external.h
> index ad419ef..2396073 100644
> --- a/include/mach-o/external.h
> +++ b/include/mach-o/external.h
> @@ -284,4 +284,31 @@ struct mach_o_fat_arch_external
>   unsigned char align[4];	/* Power of 2.  */
> };
> 
> +/* For non-scattered relocations, we have to account for the bit-endianess
> +   of the fields within the reloc.  So these need to be swapped in/out when
> +   the endianess of the host differs from the target.  */
> +
> +typedef union bfd_mach_o_reg_reloc_swap
> +{
> +  unsigned int access;
> +  struct
> +    {
> +      unsigned int r_symbolnum:24,
> +		   r_pcrel:1,
> +		   r_length:2,
> +		   r_extern:1,
> +		   r_type:4;
> +    } fwd;
> +  struct
> +    {
> +      unsigned int r_type:4,
> +		   r_extern:1,
> +		   r_length:2,
> +		   r_pcrel:1,
> +		   r_symbolnum:24;
> +    } rev;
> +}
> +bfd_mach_o_reg_reloc_swap;
> +
> +
> #endif /* _MACH_O_EXTERNAL_H */
> 


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