public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
@ 2014-12-08 15:30 Manuel Lauss
  2014-12-08 15:33 ` Andrew Bennett
  0 siblings, 1 reply; 6+ messages in thread
From: Manuel Lauss @ 2014-12-08 15:30 UTC (permalink / raw)
  To: Alan Modra, binutils; +Cc: Andrew Bennett

Hi Alan,

your patch to binutils/ld "Sort relocs output by ld -r" causes a lot of
the following warnings when I cross-compile the linux kernel for
little-endian mips32 on an x64 host:

mipsel-softfloat-linux-gnu-ld: init/mounts.o: Can't find matching LO16
reloc against `$LC2' for R_MIPS_HI16 at 0x4e8 in section `.text'

If I revert that patch the kernel build succeeds without warnings and boots.
(I wrongly blamed a patch by Andrew Bennett before which turns out
 to be innocent).

Thanks!
      Manuel Lauss

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

* RE: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
  2014-12-08 15:30 binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux Manuel Lauss
@ 2014-12-08 15:33 ` Andrew Bennett
  2014-12-09  1:21   ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Bennett @ 2014-12-08 15:33 UTC (permalink / raw)
  To: Manuel Lauss, Alan Modra, binutils

> your patch to binutils/ld "Sort relocs output by ld -r" causes a lot of
> the following warnings when I cross-compile the linux kernel for
> little-endian mips32 on an x64 host:
> 
> mipsel-softfloat-linux-gnu-ld: init/mounts.o: Can't find matching LO16
> reloc against `$LC2' for R_MIPS_HI16 at 0x4e8 in section `.text'
> 
> If I revert that patch the kernel build succeeds without warnings and boots.
> (I wrongly blamed a patch by Andrew Bennett before which turns out
>  to be innocent).

It is probably best to explain why this causes the issue, so I have explained 
it in more detail below:

The MIPS HI16/LO16 relocations must occur in pairs.  This means the LO16 
relocation must occur directly after the HI16 relocation in the relocation table.  

If we look at an example that was found in the Linux build.  The original
elf file had the relocations in the following order.

00000514  00005f05 R_MIPS_HI16       00000000   block_class
00000458  00005f05 R_MIPS_HI16       00000000   block_class
00000460  00005f06 R_MIPS_LO16       00000000   block_class

But these became the following when -r is used:

00000458  00005705 R_MIPS_HI16       00000000   block_class
0000045c  00000305 R_MIPS_HI16       00000000   .text
00000460  00005706 R_MIPS_LO16       00000000   block_class
...
00000514  00005705 R_MIPS_HI16       00000000   block_class


Therefore when the HI16/LO16 relocation checking is done for the block_class
symbol it will be unable to find the corresponding LO16 relocation in the
second instance, which is why Manuel was getting error messages about
missing relocations.


Regards,



Andrew

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

* Re: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
  2014-12-08 15:33 ` Andrew Bennett
@ 2014-12-09  1:21   ` Alan Modra
  2014-12-09  7:29     ` Manuel Lauss
  2014-12-09 14:30     ` Andrew Bennett
  0 siblings, 2 replies; 6+ messages in thread
From: Alan Modra @ 2014-12-09  1:21 UTC (permalink / raw)
  To: Andrew Bennett; +Cc: Manuel Lauss, binutils

On Mon, Dec 08, 2014 at 03:33:45PM +0000, Andrew Bennett wrote:
> > your patch to binutils/ld "Sort relocs output by ld -r" causes a lot of
> > the following warnings when I cross-compile the linux kernel for
> > little-endian mips32 on an x64 host:
> > 
> > mipsel-softfloat-linux-gnu-ld: init/mounts.o: Can't find matching LO16
> > reloc against `$LC2' for R_MIPS_HI16 at 0x4e8 in section `.text'
> > 
> > If I revert that patch the kernel build succeeds without warnings and boots.
> > (I wrongly blamed a patch by Andrew Bennett before which turns out
> >  to be innocent).
> 
> It is probably best to explain why this causes the issue, so I have explained 
> it in more detail below:
> 
> The MIPS HI16/LO16 relocations must occur in pairs.  This means the LO16 
> relocation must occur directly after the HI16 relocation in the relocation table.  
> 
> If we look at an example that was found in the Linux build.  The original
> elf file had the relocations in the following order.
> 
> 00000514  00005f05 R_MIPS_HI16       00000000   block_class
> 00000458  00005f05 R_MIPS_HI16       00000000   block_class
> 00000460  00005f06 R_MIPS_LO16       00000000   block_class
> 
> But these became the following when -r is used:
> 
> 00000458  00005705 R_MIPS_HI16       00000000   block_class
> 0000045c  00000305 R_MIPS_HI16       00000000   .text
> 00000460  00005706 R_MIPS_LO16       00000000   block_class
> ...
> 00000514  00005705 R_MIPS_HI16       00000000   block_class
> 
> 
> Therefore when the HI16/LO16 relocation checking is done for the block_class
> symbol it will be unable to find the corresponding LO16 relocation in the
> second instance, which is why Manuel was getting error messages about
> missing relocations.

Blah.  I sorted external relocs rather than internal just for mips,
otherwise a non-stable qsort might reorder within the abominable
3-for-1 internal relocs.  I didn't remember that mips deliberately
puts relocs out of r_offset order as per the mips ABI.

The following ought to fix it.  Please review the mips changes and let
me know if they are good.

commit 5923dea21d68cb33236f3430cc2944b6a1cdc15f
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Dec 9 10:19:27 2014 +1030

    Don't sort ld -r relocs for mips
    
    HI16/LO16 are deliberately put adjacent, which might mean relocs are
    then not sorted by r_offset.  See tc-mips.c:mips_frob_file.  Don't undo
    the HI16/LO16 sorting.
    
    	PR 17666
    	* elf-bfd.h (struct elf_backend_data): Add sort_relocs_p.
    	* elflink.c (elf_link_adjust_relocs): Conditionally sort.
    	(bfd_elf_final_link): Control sorting of relocs.
    	* elfxx-mips.c (_bfd_mips_elf_sort_relocs_p): New function.
    	* elfxx-mips.h (_bfd_mips_elf_sort_relocs_p): Declare.
    	* elf32-mips.c (elf_backend_sort_relocs_p): Define.
    	* elf64-mips.c (elf_backend_sort_relocs_p): Define.

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 3e54ab1..514fdcd 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1109,6 +1109,11 @@ struct elf_backend_data
   unsigned int (*elf_backend_count_relocs)
     (struct bfd_link_info *, asection *);
 
+  /* Say whether to sort relocs output by ld -r and ld --emit-relocs,
+     by r_offset.  If NULL, default to true.  */
+  bfd_boolean (*sort_relocs_p)
+    (asection *);
+
   /* This function, if defined, is called when an NT_PRSTATUS note is found
      in a core file.  */
   bfd_boolean (*elf_backend_grok_prstatus)
diff --git a/bfd/elf32-mips.c b/bfd/elf32-mips.c
index 4cfef7a..78ae1dd 100644
--- a/bfd/elf32-mips.c
+++ b/bfd/elf32-mips.c
@@ -2473,6 +2473,8 @@ static const struct ecoff_debug_swap mips_elf32_ecoff_debug_swap = {
 #define elf_backend_write_section	_bfd_mips_elf_write_section
 #define elf_backend_mips_irix_compat	elf32_mips_irix_compat
 #define elf_backend_mips_rtype_to_howto	mips_elf32_rtype_to_howto
+#define elf_backend_sort_relocs_p	_bfd_mips_elf_sort_relocs_p
+
 #define bfd_elf32_bfd_is_local_label_name \
 					mips_elf_is_local_label_name
 #define bfd_elf32_bfd_is_target_special_symbol \
diff --git a/bfd/elf64-mips.c b/bfd/elf64-mips.c
index 2f323c6..eb7e1fb 100644
--- a/bfd/elf64-mips.c
+++ b/bfd/elf64-mips.c
@@ -4444,6 +4444,7 @@ const struct elf_size_info mips_elf64_size_info =
 #define elf_backend_sign_extend_vma	TRUE
 
 #define elf_backend_write_section	_bfd_mips_elf_write_section
+#define elf_backend_sort_relocs_p	_bfd_mips_elf_sort_relocs_p
 
 /* We don't set bfd_elf64_bfd_is_local_label_name because the 32-bit
    MIPS-specific function only applies to IRIX5, which had no 64-bit
diff --git a/bfd/elflink.c b/bfd/elflink.c
index e768634..b023ec5 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -8097,7 +8097,8 @@ cmp_ext64b_r_offset (const void *p, const void *q)
 
 static void
 elf_link_adjust_relocs (bfd *abfd,
-			struct bfd_elf_section_reloc_data *reldata)
+			struct bfd_elf_section_reloc_data *reldata,
+			bfd_boolean sort)
 {
   unsigned int i;
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
@@ -8108,7 +8109,6 @@ elf_link_adjust_relocs (bfd *abfd,
   int r_sym_shift;
   unsigned int count = reldata->count;
   struct elf_link_hash_entry **rel_hash = reldata->hashes;
-  int (*compare) (const void *, const void *);
 
   if (reldata->hdr->sh_entsize == bed->s->sizeof_rel)
     {
@@ -8155,29 +8155,34 @@ elf_link_adjust_relocs (bfd *abfd,
       (*swap_out) (abfd, irela, erela);
     }
 
-  if (bed->s->arch_size == 32)
+  if (sort)
     {
-      if (abfd->xvec->header_byteorder == BFD_ENDIAN_LITTLE)
-	compare = cmp_ext32l_r_offset;
-      else if (abfd->xvec->header_byteorder == BFD_ENDIAN_BIG)
-	compare = cmp_ext32b_r_offset;
+      int (*compare) (const void *, const void *);
+
+      if (bed->s->arch_size == 32)
+	{
+	  if (abfd->xvec->header_byteorder == BFD_ENDIAN_LITTLE)
+	    compare = cmp_ext32l_r_offset;
+	  else if (abfd->xvec->header_byteorder == BFD_ENDIAN_BIG)
+	    compare = cmp_ext32b_r_offset;
+	  else
+	    abort ();
+	}
       else
-	abort ();
-    }
-  else
-    {
+	{
 #ifdef BFD_HOST_64_BIT
-      if (abfd->xvec->header_byteorder == BFD_ENDIAN_LITTLE)
-	compare = cmp_ext64l_r_offset;
-      else if (abfd->xvec->header_byteorder == BFD_ENDIAN_BIG)
-	compare = cmp_ext64b_r_offset;
-      else
+	  if (abfd->xvec->header_byteorder == BFD_ENDIAN_LITTLE)
+	    compare = cmp_ext64l_r_offset;
+	  else if (abfd->xvec->header_byteorder == BFD_ENDIAN_BIG)
+	    compare = cmp_ext64b_r_offset;
+	  else
 #endif
-	abort ();
+	    abort ();
+	}
+      qsort (reldata->hdr->contents, count, reldata->hdr->sh_entsize, compare);
+      free (reldata->hashes);
+      reldata->hashes = NULL;
     }
-  qsort (reldata->hdr->contents, count, reldata->hdr->sh_entsize, compare);
-  free (reldata->hashes);
-  reldata->hashes = NULL;
 }
 
 struct elf_link_sort_rela
@@ -11325,13 +11330,15 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
   for (o = abfd->sections; o != NULL; o = o->next)
     {
       struct bfd_elf_section_data *esdo = elf_section_data (o);
+      bfd_boolean sort;
       if ((o->flags & SEC_RELOC) == 0)
 	continue;
 
+      sort = bed->sort_relocs_p == NULL || (*bed->sort_relocs_p) (o);
       if (esdo->rel.hdr != NULL)
-	elf_link_adjust_relocs (abfd, &esdo->rel);
+	elf_link_adjust_relocs (abfd, &esdo->rel, sort);
       if (esdo->rela.hdr != NULL)
-	elf_link_adjust_relocs (abfd, &esdo->rela);
+	elf_link_adjust_relocs (abfd, &esdo->rela, sort);
 
       /* Set the reloc_count field to 0 to prevent write_relocs from
 	 trying to swap the relocs out itself.  */
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index 4cf4ac0..8112849 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -11912,6 +11912,18 @@ mips_set_isa_flags (bfd *abfd)
 }
 
 
+/* Whether to sort relocs output by ld -r or ld --emit-relocs, by r_offset.
+   Don't do so for code sections.  We want to keep ordering of HI16/LO16
+   as is.  On the other hand, elf-eh-frame.c processing requires .eh_frame
+   relocs to be sorted.  */
+
+bfd_boolean
+_bfd_mips_elf_sort_relocs_p (asection *sec)
+{
+  return (sec->flags & SEC_CODE) == 0;
+}
+
+
 /* The final processing done just before writing out a MIPS ELF object
    file.  This gets the MIPS architecture right based on the machine
    number.  This is used by both the 32-bit and the 64-bit ABI.  */
diff --git a/bfd/elfxx-mips.h b/bfd/elfxx-mips.h
index 8f5c53e..0605447 100644
--- a/bfd/elfxx-mips.h
+++ b/bfd/elfxx-mips.h
@@ -67,6 +67,8 @@ extern bfd_boolean _bfd_mips_vxworks_finish_dynamic_symbol
    Elf_Internal_Sym *);
 extern bfd_boolean _bfd_mips_elf_finish_dynamic_sections
   (bfd *, struct bfd_link_info *);
+extern bfd_boolean _bfd_mips_elf_sort_relocs_p
+  (asection *);
 extern void _bfd_mips_elf_final_write_processing
   (bfd *, bfd_boolean);
 extern int _bfd_mips_elf_additional_program_headers
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 9bf4b18..83a3caf 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -535,6 +535,9 @@
 #ifndef elf_backend_count_relocs
 #define elf_backend_count_relocs		NULL
 #endif
+#ifndef elf_backend_sort_relocs_p
+#define elf_backend_sort_relocs_p		NULL
+#endif
 #ifndef elf_backend_grok_prstatus
 #define elf_backend_grok_prstatus		NULL
 #endif
@@ -737,6 +740,7 @@ static struct elf_backend_data elfNN_bed =
   elf_backend_ignore_undef_symbol,
   elf_backend_emit_relocs,
   elf_backend_count_relocs,
+  elf_backend_sort_relocs_p,
   elf_backend_grok_prstatus,
   elf_backend_grok_psinfo,
   elf_backend_write_core_note,

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
  2014-12-09  1:21   ` Alan Modra
@ 2014-12-09  7:29     ` Manuel Lauss
  2014-12-09 14:30     ` Andrew Bennett
  1 sibling, 0 replies; 6+ messages in thread
From: Manuel Lauss @ 2014-12-09  7:29 UTC (permalink / raw)
  To: Andrew Bennett, binutils, Alan Modra

On Tue, Dec 9, 2014 at 2:21 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Dec 08, 2014 at 03:33:45PM +0000, Andrew Bennett wrote:
>> > your patch to binutils/ld "Sort relocs output by ld -r" causes a lot of
>> > the following warnings when I cross-compile the linux kernel for
>> > little-endian mips32 on an x64 host:
>> >
>> > mipsel-softfloat-linux-gnu-ld: init/mounts.o: Can't find matching LO16
>> > reloc against `$LC2' for R_MIPS_HI16 at 0x4e8 in section `.text'
>> >
>> > If I revert that patch the kernel build succeeds without warnings and boots.
>> > (I wrongly blamed a patch by Andrew Bennett before which turns out
>> >  to be innocent).
>>
>> It is probably best to explain why this causes the issue, so I have explained
>> it in more detail below:
>>
>> The MIPS HI16/LO16 relocations must occur in pairs.  This means the LO16
>> relocation must occur directly after the HI16 relocation in the relocation table.
>>
>> If we look at an example that was found in the Linux build.  The original
>> elf file had the relocations in the following order.
>>
>> 00000514  00005f05 R_MIPS_HI16       00000000   block_class
>> 00000458  00005f05 R_MIPS_HI16       00000000   block_class
>> 00000460  00005f06 R_MIPS_LO16       00000000   block_class
>>
>> But these became the following when -r is used:
>>
>> 00000458  00005705 R_MIPS_HI16       00000000   block_class
>> 0000045c  00000305 R_MIPS_HI16       00000000   .text
>> 00000460  00005706 R_MIPS_LO16       00000000   block_class
>> ...
>> 00000514  00005705 R_MIPS_HI16       00000000   block_class
>>
>>
>> Therefore when the HI16/LO16 relocation checking is done for the block_class
>> symbol it will be unable to find the corresponding LO16 relocation in the
>> second instance, which is why Manuel was getting error messages about
>> missing relocations.
>
> Blah.  I sorted external relocs rather than internal just for mips,
> otherwise a non-stable qsort might reorder within the abominable
> 3-for-1 internal relocs.  I didn't remember that mips deliberately
> puts relocs out of r_offset order as per the mips ABI.
>
> The following ought to fix it.  Please review the mips changes and let
> me know if they are good.

This patch works fine, kernel builds and boots.

Thank you!
      Manuel Lauss

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

* RE: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
  2014-12-09  1:21   ` Alan Modra
  2014-12-09  7:29     ` Manuel Lauss
@ 2014-12-09 14:30     ` Andrew Bennett
  2014-12-10  0:03       ` Alan Modra
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew Bennett @ 2014-12-09 14:30 UTC (permalink / raw)
  To: Alan Modra; +Cc: Manuel Lauss, binutils

> The following ought to fix it.  Please review the mips changes and let
> me know if they are good.

I am happy with this fix.

Regards,


Andrew

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

* Re: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux
  2014-12-09 14:30     ` Andrew Bennett
@ 2014-12-10  0:03       ` Alan Modra
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Modra @ 2014-12-10  0:03 UTC (permalink / raw)
  To: Andrew Bennett; +Cc: Manuel Lauss, binutils

On Tue, Dec 09, 2014 at 02:30:40PM +0000, Andrew Bennett wrote:
> > The following ought to fix it.  Please review the mips changes and let
> > me know if they are good.
> 
> I am happy with this fix.

Committed.

-- 
Alan Modra
Australia Development Lab, IBM

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

end of thread, other threads:[~2014-12-10  0:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-08 15:30 binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux Manuel Lauss
2014-12-08 15:33 ` Andrew Bennett
2014-12-09  1:21   ` Alan Modra
2014-12-09  7:29     ` Manuel Lauss
2014-12-09 14:30     ` Andrew Bennett
2014-12-10  0:03       ` Alan Modra

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