public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
@ 2012-02-22 16:35 Iain Sandoe
  2012-02-23  8:28 ` Tristan Gingold
  0 siblings, 1 reply; 7+ messages in thread
From: Iain Sandoe @ 2012-02-22 16:35 UTC (permalink / raw)
  To: binutils Development; +Cc: Tristan Gingold


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]

---- 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.

----

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 */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-22 16:35 [Patch bfd/mach-o] some tweaks to bfd_mach_o_canonicalize_one_reloc Iain Sandoe
@ 2012-02-23  8:28 ` Tristan Gingold
  2012-02-23  9:00   ` Iain Sandoe
  0 siblings, 1 reply; 7+ messages in thread
From: Tristan Gingold @ 2012-02-23  8:28 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: binutils Development


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 */
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-23  8:28 ` Tristan Gingold
@ 2012-02-23  9:00   ` Iain Sandoe
  2012-02-23  9:11     ` Tristan Gingold
  0 siblings, 1 reply; 7+ messages in thread
From: Iain Sandoe @ 2012-02-23  9:00 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: binutils Development

Hi Tristan,
On 23 Feb 2012, at 08:28, Tristan Gingold wrote:

> 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.

It's not necessary to have a PPC system (the problem I saw was trying  
to objdump PPC code on an x86 system) - it will affect any case where  
the endian-ness is different between host & target.

I was using:
echo " .long _foo " | as -arch ppc -o t.o
as a test-case ;)

[I see the problem on x86 because I have a partial port to PPC  
enabled, which means that objdump is now able to list PPC relocs - I  
can send you a patch, off-list if you like (it's not ready to post  
yet)].

I'll look at moving the representation to bytes - getting rid of the  
r_symbolnum field would probably be quite easy.

.. but it's bit-field layout within bytes that's the issue, so I think  
we still would need:

union {
   unsigned char access;
   struct
    {
	unsigned char r_pcrel:1, r_length:2, r_extern:1,  r_type:4;
    } rev;
    {
	unsigned char r_type:4, r_extern:1, r_length:2, r_pcrel:1;
    } fwd;
} bfd_mach_o_reloc_field_swap;

... or a set of macros to do the same.
* this has to be activated by TARGET-endian-ness != HOST-endian-ness
... it's not enough to look at the target.

... anyway it will have to go on the TODO ;) ... other things to  
attack first.
cheers
Iain




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-23  9:00   ` Iain Sandoe
@ 2012-02-23  9:11     ` Tristan Gingold
  2012-02-23 14:30       ` Iain Sandoe
  0 siblings, 1 reply; 7+ messages in thread
From: Tristan Gingold @ 2012-02-23  9:11 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: binutils Development


On Feb 23, 2012, at 9:59 AM, Iain Sandoe wrote:

> Hi Tristan,
> On 23 Feb 2012, at 08:28, Tristan Gingold wrote:
> 
>> 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.
> 
> It's not necessary to have a PPC system (the problem I saw was trying to objdump PPC code on an x86 system) - it will affect any case where the endian-ness is different between host & target.
> 
> I was using:
> echo " .long _foo " | as -arch ppc -o t.o
> as a test-case ;)

Ah ok.

> [I see the problem on x86 because I have a partial port to PPC enabled, which means that objdump is now able to list PPC relocs - I can send you a patch, off-list if you like (it's not ready to post yet)].
> 
> I'll look at moving the representation to bytes - getting rid of the r_symbolnum field would probably be quite easy.
> 
> .. but it's bit-field layout within bytes that's the issue, so I think we still would need:
> 
> union {
>  unsigned char access;
>  struct
>   {
> 	unsigned char r_pcrel:1, r_length:2, r_extern:1,  r_type:4;
>   } rev;
>   {
> 	unsigned char r_type:4, r_extern:1, r_length:2, r_pcrel:1;
>   } fwd;
> } bfd_mach_o_reloc_field_swap;
> 
> ... or a set of macros to do the same.

Yes, a set of macros should do it.

> * this has to be activated by TARGET-endian-ness != HOST-endian-ness
> ... it's not enough to look at the target.

I don't think so.  If you do byte access (like we should do as soon as we access to 'external' structures), the host doesn't matter.

> ... anyway it will have to go on the TODO ;) ... other things to attack first.

Up to you!

Cheers,
Tristan.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-23  9:11     ` Tristan Gingold
@ 2012-02-23 14:30       ` Iain Sandoe
  2012-02-23 14:43         ` Tristan Gingold
  0 siblings, 1 reply; 7+ messages in thread
From: Iain Sandoe @ 2012-02-23 14:30 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: binutils Development


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?
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->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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-23 14:30       ` Iain Sandoe
@ 2012-02-23 14:43         ` Tristan Gingold
  2012-02-23 16:47           ` Iain Sandoe
  0 siblings, 1 reply; 7+ messages in thread
From: Tristan Gingold @ 2012-02-23 14:43 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: binutils Development


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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch bfd/mach-o] some tweaks to  bfd_mach_o_canonicalize_one_reloc
  2012-02-23 14:43         ` Tristan Gingold
@ 2012-02-23 16:47           ` Iain Sandoe
  0 siblings, 0 replies; 7+ messages in thread
From: Iain Sandoe @ 2012-02-23 16:47 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: binutils Development


On 23 Feb 2012, at 14:43, Tristan Gingold wrote:
>> so, how about this?
>
> Yes, looks good.  One minor nit, see below.
Thanks, fixed.
> I think the usual name in BFD for encode/decode is swap in/swap  
> out.  Feel free to rename.
changed the names and applied
Iain

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-02-23 16:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-22 16:35 [Patch bfd/mach-o] some tweaks to bfd_mach_o_canonicalize_one_reloc Iain Sandoe
2012-02-23  8:28 ` Tristan Gingold
2012-02-23  9:00   ` Iain Sandoe
2012-02-23  9:11     ` Tristan Gingold
2012-02-23 14:30       ` Iain Sandoe
2012-02-23 14:43         ` Tristan Gingold
2012-02-23 16:47           ` Iain Sandoe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).