[PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[James Clarke]

bfd/
 * elfxx-sparc.c (_bfd_sparc_elf_relocate_section): Don't convert
 R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64.

gold/
 * sparc.cc (Target_sparc::Scan::local): Don't convert R_SPARC_32 to
 R_SPARC_RELATIVE if class is ELFCLASS64.
 (Target_sparc::Scan::global): Likewise.
---
 bfd/elfxx-sparc.c |  3 ++-
 gold/sparc.cc     | 12 +++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index fc12805..db2d127 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -3481,7 +3481,8 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 		}
 	      else
 		{
-		  if (r_type == R_SPARC_32 || r_type == R_SPARC_64)
+		  if ((!ABI_64_P (output_bfd) && r_type == R_SPARC_32)
+		      || (ABI_64_P (output_bfd) && r_type == R_SPARC_64))
 		    {
 		      outrel.r_info = SPARC_ELF_R_INFO (htab, NULL,
 							0, R_SPARC_RELATIVE);
diff --git a/gold/sparc.cc b/gold/sparc.cc
index 10a5031..e5e0146 100644
--- a/gold/sparc.cc
+++ b/gold/sparc.cc
@@ -2292,7 +2292,9 @@ Target_sparc<size, big_endian>::Scan::local(
       // apply the link-time value, so we flag the location with
       // an R_SPARC_RELATIVE relocation so the dynamic loader can
       // relocate it easily.
-      if (parameters->options().output_is_position_independent())
+      if (parameters->options().output_is_position_independent()
+	  && ((size == 64 && r_type == elfcpp::R_SPARC_64)
+	      || (size == 32 && r_type == elfcpp::R_SPARC_32)))
 	{
 	  Reloc_section* rela_dyn = target->rela_dyn_section(layout);
 	  unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
@@ -2300,9 +2302,9 @@ Target_sparc<size, big_endian>::Scan::local(
 				       output_section, data_shndx,
 				       reloc.get_r_offset(),
 				       reloc.get_r_addend(), is_ifunc);
+	  break;
 	}
-      break;
-
+      /* Fall through */
     case elfcpp::R_SPARC_HIX22:
     case elfcpp::R_SPARC_LOX10:
     case elfcpp::R_SPARC_H34:
@@ -2766,8 +2768,8 @@ Target_sparc<size, big_endian>::Scan::global(
 						       reloc.get_r_offset(),
 						       reloc.get_r_addend());
 	      }
-	    else if ((r_type == elfcpp::R_SPARC_32
-		      || r_type == elfcpp::R_SPARC_64)
+	    else if (((size == 64 && r_type == elfcpp::R_SPARC_64)
+		      || (size == 32 && r_type == elfcpp::R_SPARC_32))
 		     && gsym->can_use_relative_reloc(false))
 	      {
 		Reloc_section* rela_dyn = target->rela_dyn_section(layout);
-- 
2.8.2

[PATCH 0/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[James Clarke]

Currently, ld (both bfd and gold) will convert an R_SPARC_32 relocation to be
an R_SPARC_RELATIVE one when creating an ELFCLASS64 file (and would convert
R_SPARC_64 to R_SPARC_RELATIVE for an ELFCLASS32 file if it were to be given
such a relocation). This is clearly incorrect based on the size and alignment
requirements for each. This patch ensures R_SPARC_XX is only converted to
R_SPARC_RELATIVE when creating an ELFCLASSXX file.

This seems to produce a regression in the test suite for -Bsymbolic-functions:

    /home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/../gas/as-new   -I/home/jrtc27/src/binutils/binutils-2.26/ld/testsuite/ld-elf   -o tmpdir/symbolic-func.o  /home/jrtc27/src/binutils/binutils-2.26/ld/testsuite/ld-elf/symbolic-fun
    c.s
    Executing on host: sh -c {/home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/../gas/as-new   -I/home/jrtc27/src/binutils/binutils-2.26/ld/testsuite/ld-elf   -o tmpdir/symbolic-func.o  /home/jrtc27/src/binutils/binutils-2.26/ld/tes
    tsuite/ld-elf/symbolic-func.s 2>&1}  /dev/null ld.tmp (timeout = 300)
    spawn [open ...]
    /home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/ld-new   -o tmpdir/symbolic-func.so -L/home/jrtc27/src/binutils/binutils-2.26/ld/testsuite/ld-elf -shared -Bsymbolic-functions tmpdir/symbolic-func.o 
    Executing on host: sh -c {/home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/ld-new   -o tmpdir/symbolic-func.so -L/home/jrtc27/src/binutils/binutils-2.26/ld/testsuite/ld-elf -shared -Bsymbolic-functions tmpdir/symbolic-func.o  2>
    &1}  /dev/null ld.tmp (timeout = 300)
    spawn [open ...]
    Executing on host: sh -c {/home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/../binutils/readelf -r --wide tmpdir/symbolic-func.so >dump.out 2>ld.stderr}  /dev/null  (timeout = 300)
    spawn [open ...]
    /home/jrtc27/src/binutils/binutils-2.26/builddir-single/ld/../binutils/readelf -r --wide tmpdir/symbolic-func.so
    regexp_diff match failure
    regexp "^0*[1-9a-f][0-9a-f]* +[^ ]+ +[^ ]+ +([0-9a-f]+( +\.text( \+ 0)?)?)?$"
    line   "0000000000100200  0000000100000003 R_SPARC_32             00000000000001f8 .text + 1f8"
    FAIL: -Bsymbolic-functions

Linking symbolic-func.so with system linker:

    Relocation section '.rela.dyn' at offset 0x1e0 contains 1 entries:
      Offset          Info           Type           Sym. Value    Sym. Name + Addend
    000000100200  000000000016 R_SPARC_RELATIVE                     1f8

Linking symbolic-func.so with new linker:

    Relocation section '.rela.dyn' at offset 0x1e0 contains 1 entries:
      Offset          Info           Type           Sym. Value    Sym. Name + Addend
    000000100200  000100000003 R_SPARC_32        00000000000001f8 .text + 1f8

I don't know what's supposed to happen here, but I would be very surprised if
the R_SPARC_32 is meant to be an R_SPARC_RELATIVE given the sizes and alignment
requirements.

Note: this bug appeared in 22b75d0ae688dadd85f4bf89bd14541f9c9c6f2c, which
created elfxx-sparc.c and unified the code in elf{32,64}-sparc.c.

James Clarke (1):
  Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

 bfd/elfxx-sparc.c |  3 ++-
 gold/sparc.cc     | 12 +++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.8.2

Re: [PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[Cary Coutant]

> gold/
>  * sparc.cc (Target_sparc::Scan::local): Don't convert R_SPARC_32 to
>  R_SPARC_RELATIVE if class is ELFCLASS64.
>  (Target_sparc::Scan::global): Likewise.

The gold part of this patch is OK.

(Sorry for the delay. The patch wasn't addressed to me, and it didn't
have [gold] in the subject, so I didn't see this until today.)

-cary

Re: [PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[Nick Clifton]

Hi James,

> bfd/
>  * elfxx-sparc.c (_bfd_sparc_elf_relocate_section): Don't convert
>  R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64.

This part is approved too.

The failure of the -Bsymbolic functions linker test looks, to me,
like a problem with the regexp.  Specifically I think that it may
need to accept tabs as well as spaces where there is whitespace
between parts of readelf's output.

Cheers
  Nick

Re: [PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[James Clarke]

Hi,
On Tue, May 24, 2016 at 20:26:58PM +0100, Cary Coutant wrote:
> > gold/
> >  * sparc.cc (Target_sparc::Scan::local): Don't convert R_SPARC_32 to
> >  R_SPARC_RELATIVE if class is ELFCLASS64.
> >  (Target_sparc::Scan::global): Likewise.
>
> The gold part of this patch is OK.

On Wed, May 25, 2016 at 03:40:32PM +0100, Nick Clifton wrote:
> > bfd/
> >  * elfxx-sparc.c (_bfd_sparc_elf_relocate_section): Don't convert
> >  R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64.
>
> This part is approved too.

Any progress on this? It would be good to get this fixed properly. I
know gold segfaults when building one particular package with the gold
part of the patch applied (although I haven't tried it without) but at
least getting just bfd fixed would be an improvement.

Regards,
James

Re: [PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[Nick Clifton]

Hi James,

> Any progress on this? It would be good to get this fixed properly. I
> know gold segfaults when building one particular package with the gold
> part of the patch applied (although I haven't tried it without) but at
> least getting just bfd fixed would be an improvement.

Sorry about that - the patch is now checked in.

Cheers
  Nick

Re: [PATCH 1/1] Don't convert R_SPARC_32 to R_SPARC_RELATIVE if class is ELFCLASS64

[John Paul Adrian Glaubitz]

On 06/28/2016 01:02 PM, Nick Clifton wrote:
>> Any progress on this? It would be good to get this fixed properly. I
>> know gold segfaults when building one particular package with the gold
>> part of the patch applied (although I haven't tried it without) but at
>> least getting just bfd fixed would be an improvement.
> 
> Sorry about that - the patch is now checked in.

Awesome, thank you very much!

@James: Now we just need to track down the Gold crashes.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913