Re: binutils/ld: "Sort relocs output by ld -r" breaks mips cross compilation of linux

[Alan Modra <amodra at gmail dot com>]

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