[SPARC] Fix PR ld/22972

Message ID 7015683.x2jCpxZVUG@polaris
State New
Headers show
Series
  • [SPARC] Fix PR ld/22972
Related show

Commit Message

Eric Botcazou March 22, 2018, 4:34 p.m.
Hi,

this is a regression present on mainline and 2.30 branch for the corner case 
of a hidden symbol in a PIC/PIE binary which is subject to both a new-style 
GOTDATA relocation and an old-style GOT relocation.  In this case, depending 
on the link order, the R_SPARC_RELATIVE dynamic relocation for the GOT slot 
needed because of the old-style relocation can be replaced with R_SPARC_NONE 
coming from the GOTDATA relocation.

The attached patch simply records whether an old-style GOT relocation is seen 
for a symbol and prevents the R_SPARC_NONE from being generated in this case.

Tested on SPARC64/Linux with a GCC bootstrap, OK for mainline and 2.30 branch?


2018-03-22  Eric Botcazou  <ebotcazou@adacore.com>

	PR ld/22972
bfd/
	* elfxx-sparc.c (struct _bfd_sparc_elf_link_hash_entry): Add new flag
	has_old_style_got_reloc.
	(_bfd_sparc_elf_check_relocs) <GOT relocations>: Set it for old-style
	relocations.  Fix a couple of long lines.
	(_bfd_sparc_elf_relocate_section) <R_SPARC_GOTDATA_OP>: Do not generate
	a R_SPARC_NONE for the GOT slot if the symbol is also subject to old-style
 	GOT relocations.

ld/
	* testsuite/ld-sparc/sparc.exp: Add test for mixed GOTDATA/GOT relocations.
	* testsuite/ld-sparc/gotop-hidden.c: New file.
	* testsuite/ld-sparc/got-hidden32.s: Likewise.
	* testsuite/ld-sparc/got-hidden64.s: Likewise.
	* testsuite/ld-sparc/pass.out: Likewise.

-- 
Eric Botcazou

Comments

Jose E. Marchesi March 23, 2018, 1:20 p.m. | #1
Hi Eric.
    
    this is a regression present on mainline and 2.30 branch for the corner case 
    of a hidden symbol in a PIC/PIE binary which is subject to both a new-style 
    GOTDATA relocation and an old-style GOT relocation.  In this case, depending 
    on the link order, the R_SPARC_RELATIVE dynamic relocation for the GOT slot 
    needed because of the old-style relocation can be replaced with R_SPARC_NONE 
    coming from the GOTDATA relocation.
    
    The attached patch simply records whether an old-style GOT relocation is seen 
    for a symbol and prevents the R_SPARC_NONE from being generated in this case.
    
    Tested on SPARC64/Linux with a GCC bootstrap, OK for mainline and 2.30 branch?

LGTM
    
    2018-03-22  Eric Botcazou  <ebotcazou@adacore.com>
    
    	PR ld/22972
    bfd/
    	* elfxx-sparc.c (struct _bfd_sparc_elf_link_hash_entry): Add new flag
    	has_old_style_got_reloc.
    	(_bfd_sparc_elf_check_relocs) <GOT relocations>: Set it for old-style
    	relocations.  Fix a couple of long lines.
    	(_bfd_sparc_elf_relocate_section) <R_SPARC_GOTDATA_OP>: Do not generate
    	a R_SPARC_NONE for the GOT slot if the symbol is also subject to old-style
     	GOT relocations.
    
    ld/
    	* testsuite/ld-sparc/sparc.exp: Add test for mixed GOTDATA/GOT relocations.
    	* testsuite/ld-sparc/gotop-hidden.c: New file.
    	* testsuite/ld-sparc/got-hidden32.s: Likewise.
    	* testsuite/ld-sparc/got-hidden64.s: Likewise.
    	* testsuite/ld-sparc/pass.out: Likewise.

Patch

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index 849182fc8c..81812afc5a 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -701,9 +701,12 @@  struct _bfd_sparc_elf_link_hash_entry
 #define GOT_TLS_IE      3
   unsigned char tls_type;
 
-    /* Symbol has GOT or PLT relocations.  */
+  /* Symbol has GOT or PLT relocations.  */
   unsigned int has_got_reloc : 1;
 
+  /* Symbol has old-style, non-relaxable GOT relocations.  */
+  unsigned int has_old_style_got_reloc : 1;
+
   /* Symbol has non-GOT/non-PLT relocations in text sections.  */
   unsigned int has_non_got_reloc : 1;
 
@@ -1569,11 +1572,12 @@  _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 		    && r_type != R_SPARC_GOTDATA_OP_LOX10)
 		  local_got_refcounts[r_symndx] += 1;
 
-		old_tls_type = _bfd_sparc_elf_local_got_tls_type (abfd) [r_symndx];
+		old_tls_type
+		  = _bfd_sparc_elf_local_got_tls_type (abfd) [r_symndx];
 	      }
 
-	    /* If a TLS symbol is accessed using IE at least once, there is no point
-	       in using the dynamic model for it.  */
+	    /* If a TLS symbol is accessed using IE at least once, there is no
+	       point in using the dynamic model for it.  */
 	    if (old_tls_type != tls_type)
 	      {
 		if (old_tls_type == GOT_UNKNOWN)
@@ -1603,7 +1607,13 @@  _bfd_sparc_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	    return FALSE;
 
 	  if (eh != NULL)
-	    eh->has_got_reloc = 1;
+	    {
+	      eh->has_got_reloc = 1;
+	      if (r_type == R_SPARC_GOT10
+		  || r_type == R_SPARC_GOT13
+		  || r_type == R_SPARC_GOT22)
+		eh->has_old_style_got_reloc = 1;
+	    }
 	  break;
 
 	case R_SPARC_TLS_GD_CALL:
@@ -3138,12 +3148,14 @@  _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 	      bfd_put_32 (output_bfd, relocation, contents + rel->r_offset);
 
 	      /* If the symbol is global but not dynamic, an .rela.* slot has
-		 been allocated for it in the GOT so output R_SPARC_NONE here.
-		 See also the handling of other GOT relocations just below.  */
+		 been allocated for it in the GOT so output R_SPARC_NONE here,
+		 if it isn't also subject to another, old-style GOT relocation.
+		 See also the handling of these GOT relocations just below.  */
 	      if (h != NULL
 		  && h->dynindx == -1
 		  && !h->forced_local
 		  && h->root.type != bfd_link_hash_undefweak
+		  && !eh->has_old_style_got_reloc
 		  && (h->got.offset & 1) == 0
 		  && bfd_link_pic (info))
 		{
diff --git a/ld/testsuite/ld-sparc/got-hidden32.s b/ld/testsuite/ld-sparc/got-hidden32.s
new file mode 100644
index 0000000000..cd1ecf2d94
--- /dev/null
+++ b/ld/testsuite/ld-sparc/got-hidden32.s
@@ -0,0 +1,18 @@ 
+	.text
+.LLGETPC0:
+	retl
+	 add	%o7, %l7, %l7
+	.global foo
+	.type foo, #function
+	.proc   04
+foo:
+	save    %sp, -104, %sp
+	sethi	%hi(_GLOBAL_OFFSET_TABLE_-4), %l7
+	call	.LLGETPC0
+	 add	%l7, %lo(_GLOBAL_OFFSET_TABLE_+4), %l7
+	sethi 	%hi(var), %g1
+	or 	%g1, %lo(var), %g1
+	ld 	[%l7+%g1], %g1
+	ld 	[%g1], %i0
+	ret
+	 restore
diff --git a/ld/testsuite/ld-sparc/got-hidden64.s b/ld/testsuite/ld-sparc/got-hidden64.s
new file mode 100644
index 0000000000..50e75ca452
--- /dev/null
+++ b/ld/testsuite/ld-sparc/got-hidden64.s
@@ -0,0 +1,18 @@ 
+	.text
+.LLGETPC0:
+	retl
+	 add	%o7, %l7, %l7
+	.global foo
+	.type foo, #function
+	.proc   04
+foo:
+	save    %sp, -160, %sp
+	sethi	%hi(_GLOBAL_OFFSET_TABLE_-4), %l7
+	call	.LLGETPC0
+	 add	%l7, %lo(_GLOBAL_OFFSET_TABLE_+4), %l7
+	sethi 	%hi(var), %g1
+	or 	%g1, %lo(var), %g1
+	ldx 	[%l7+%g1], %g1
+	ld 	[%g1], %i0
+	ret
+	 restore
diff --git a/ld/testsuite/ld-sparc/gotop-hidden.c b/ld/testsuite/ld-sparc/gotop-hidden.c
new file mode 100644
index 0000000000..d769c6ddc6
--- /dev/null
+++ b/ld/testsuite/ld-sparc/gotop-hidden.c
@@ -0,0 +1,13 @@ 
+#include <stdio.h>
+
+extern unsigned int foo (void);
+
+__attribute__((visibility("hidden"))) unsigned int var = 0xdeadbeef;
+
+int main (void)
+{
+  if (var == foo ())
+    puts ("PASS");
+
+  return 0;
+}
diff --git a/ld/testsuite/ld-sparc/pass.out b/ld/testsuite/ld-sparc/pass.out
new file mode 100644
index 0000000000..7ef22e9a43
--- /dev/null
+++ b/ld/testsuite/ld-sparc/pass.out
@@ -0,0 +1 @@ 
+PASS
diff --git a/ld/testsuite/ld-sparc/sparc.exp b/ld/testsuite/ld-sparc/sparc.exp
index e8aa0c284c..41aa2e68b2 100644
--- a/ld/testsuite/ld-sparc/sparc.exp
+++ b/ld/testsuite/ld-sparc/sparc.exp
@@ -146,8 +146,37 @@  set sparc64tests {
 if { ![istarget "sparc64-*-elf*"] } {
     run_ld_link_tests $sparctests
 }
+
 if { !([istarget "sparc-*-elf*"]
        || [istarget "sparc-sun-solaris2.5*"]
        || [istarget "sparc-sun-solaris2.6"]) } {
     run_ld_link_tests $sparc64tests
 }
+
+if { [istarget "sparc*-*-linux*"] && [isnative] } {
+    run_ld_link_exec_tests [list \
+	[list \
+	    "32-bit: mixed GOTDATA/GOT relocations" \
+	    "-pie -m32" \
+	    "" \
+	    { gotop-hidden.c got-hidden32.s } \
+	    "gotop-hidden32" \
+	    "pass.out" \
+	    "-fPIE -m32" \
+	] \
+    ]
+}
+
+if { [istarget "sparc64-*-linux*"] && [isnative] } {
+    run_ld_link_exec_tests [list \
+	[list \
+	    "64-bit: mixed GOTDATA/GOT relocations" \
+	    "-pie -m64" \
+	    "" \
+	    { gotop-hidden.c got-hidden64.s } \
+	    "gotop-hidden64" \
+	    "pass.out" \
+	    "-fPIE -m64" \
+	] \
+    ]
+}