[PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[Egeyar Bagcioglu]

Prevent sparc backend making symbols dynamic unless they are undefined
weak. By doing so, stop dynamic linker confusing these symbols with
library symbols.

bfd/ChangeLog:

2017-05-19  Egeyar Bagcioglu <egeyar.bagcioglu@oracle.com>

        * elfxx-sparc.c (allocate_dynrelocs): If a symbol isn't undefined
        weak symbol, don't make it dynamic.
---
 bfd/elfxx-sparc.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index a9362a3..79746b7 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -2310,7 +2310,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	 Undefined weak syms won't yet be marked as dynamic.  */
       if (h->dynindx == -1
 	  && !h->forced_local
-          && !resolved_to_zero)
+          && !resolved_to_zero
+          && h->root.type == bfd_link_hash_undefweak)
 	{
 	  if (! bfd_elf_link_record_dynamic_symbol (info, h))
 	    return FALSE;
@@ -2422,7 +2423,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	 Undefined weak syms won't yet be marked as dynamic.  */
       if (h->dynindx == -1
 	  && !h->forced_local
-          && !resolved_to_zero)
+          && !resolved_to_zero
+          && h->root.type == bfd_link_hash_undefweak)
 	{
 	  if (! bfd_elf_link_record_dynamic_symbol (info, h))
 	    return FALSE;
@@ -2564,7 +2566,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	     Undefined weak syms won't yet be marked as dynamic.  */
 	  if (h->dynindx == -1
 	      && !h->forced_local
-              && !resolved_to_zero)
+              && !resolved_to_zero
+              && h->root.type == bfd_link_hash_undefweak)
 	    {
 	      if (! bfd_elf_link_record_dynamic_symbol (info, h))
 		return FALSE;
-- 
1.7.1

Re: [PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[Jose E. Marchesi]

Hi Egeyar.

    Prevent sparc backend making symbols dynamic unless they are undefined
    weak. By doing so, stop dynamic linker confusing these symbols with
    library symbols.

Your patch seems to fix all the currently failing tests of the LD
testsuite in sparc64:

FAIL: Run indirect5 3
FAIL: Run indirect5 4
FAIL: Run indirect6 3
FAIL: Run indirect6 4
FAIL: indirect5c dynsym
FAIL: indirect5d dynsym
FAIL: indirect6c dynsym
FAIL: indirect6d dynsym
FAIL: Run pr2404 with PIE
FAIL: Run pr2404 with PIE (-z now)

These all pass now.  Which is great! :)

However, it also triggers a regression:

gcc -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
-L/usr/local/sparc64-unknown-linux-gnu/lib64 \
-L/usr/local/lib64 -L/lib64 -L/usr/lib64
-L/usr/local/sparc64-unknown-linux-gnu/lib -L/usr/local/lib -L/lib\
 -L/usr/lib  -o tmpdir/ifunc-mainpn
 -L/home/jemarch/couts2/binutils-gdb/ld/testsuite/ld-ifunc -pie -Wl,-z,\
 now -Wl,--no-as-needed tmpdir/libifunc-libn.so tmpdir/ifunc-main.o
 Executing on host: sh -c {gcc
 -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
 -L/usr/local/sparc6\
 4-unknown-linux-gnu/lib64 -L/usr/local/lib64 -L/lib64 -L/usr/lib64
 -L/usr/local/sparc64-unknown-linux-gnu/l\
 ib -L/usr/local/lib -L/lib -L/usr/lib  -o tmpdir/ifunc-mainpn
 -L/home/jemarch/couts2/binutils-gdb/ld/tests\
 uite/ld-ifunc -pie -Wl,-z,now -Wl,--no-as-needed
 tmpdir/libifunc-libn.so tmpdir/ifunc-main.o 2>&1}  /dev/nu\
 ll ld.tmp (timeout = 300)
 spawn [open ...]^M
 Running: tmpdir/ifunc-mainpn > tmpdir/ifunc-mainpn.out
 child killed: segmentation violation
FAIL: Run ifunc-main with PIE (-z now)

Could you please take a look?
Thanks!

Re: [PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[Egeyar Bagcioglu]

On 06/06/2017 07:30 AM, jose.marchesi@oracle.com wrote:
> Hi Egeyar.
>
>      Prevent sparc backend making symbols dynamic unless they are undefined
>      weak. By doing so, stop dynamic linker confusing these symbols with
>      library symbols.
>
> Your patch seems to fix all the currently failing tests of the LD
> testsuite in sparc64:
>
> FAIL: Run indirect5 3
> FAIL: Run indirect5 4
> FAIL: Run indirect6 3
> FAIL: Run indirect6 4
> FAIL: indirect5c dynsym
> FAIL: indirect5d dynsym
> FAIL: indirect6c dynsym
> FAIL: indirect6d dynsym
> FAIL: Run pr2404 with PIE
> FAIL: Run pr2404 with PIE (-z now)
>
> These all pass now.  Which is great! :)
>
> However, it also triggers a regression:
>
> gcc -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
> -L/usr/local/sparc64-unknown-linux-gnu/lib64 \
> -L/usr/local/lib64 -L/lib64 -L/usr/lib64
> -L/usr/local/sparc64-unknown-linux-gnu/lib -L/usr/local/lib -L/lib\
>   -L/usr/lib  -o tmpdir/ifunc-mainpn
>   -L/home/jemarch/couts2/binutils-gdb/ld/testsuite/ld-ifunc -pie -Wl,-z,\
>   now -Wl,--no-as-needed tmpdir/libifunc-libn.so tmpdir/ifunc-main.o
>   Executing on host: sh -c {gcc
>   -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
>   -L/usr/local/sparc6\
>   4-unknown-linux-gnu/lib64 -L/usr/local/lib64 -L/lib64 -L/usr/lib64
>   -L/usr/local/sparc64-unknown-linux-gnu/l\
>   ib -L/usr/local/lib -L/lib -L/usr/lib  -o tmpdir/ifunc-mainpn
>   -L/home/jemarch/couts2/binutils-gdb/ld/tests\
>   uite/ld-ifunc -pie -Wl,-z,now -Wl,--no-as-needed
>   tmpdir/libifunc-libn.so tmpdir/ifunc-main.o 2>&1}  /dev/nu\
>   ll ld.tmp (timeout = 300)
>   spawn [open ...]^M
>   Running: tmpdir/ifunc-mainpn > tmpdir/ifunc-mainpn.out
>   child killed: segmentation violation
> FAIL: Run ifunc-main with PIE (-z now)
>
> Could you please take a look?
> Thanks!

Hi Jose,

Thank you for the review and bringing the issue to my attention! I am 
looking into it.

Egeyar

Re: [PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[Egeyar Bagcioglu]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

On 06/07/2017 09:05 PM, Egeyar Bagcioglu wrote:
> On 06/06/2017 07:30 AM, jose.marchesi@oracle.com wrote:
>> Hi Egeyar.
>>
>>      Prevent sparc backend making symbols dynamic unless they are 
>> undefined
>>      weak. By doing so, stop dynamic linker confusing these symbols with
>>      library symbols.
>>
>> Your patch seems to fix all the currently failing tests of the LD
>> testsuite in sparc64:
>>
>> FAIL: Run indirect5 3
>> FAIL: Run indirect5 4
>> FAIL: Run indirect6 3
>> FAIL: Run indirect6 4
>> FAIL: indirect5c dynsym
>> FAIL: indirect5d dynsym
>> FAIL: indirect6c dynsym
>> FAIL: indirect6d dynsym
>> FAIL: Run pr2404 with PIE
>> FAIL: Run pr2404 with PIE (-z now)
>>
>> These all pass now.  Which is great! :)
>>
>> However, it also triggers a regression:
>>
>> gcc -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
>> -L/usr/local/sparc64-unknown-linux-gnu/lib64 \
>> -L/usr/local/lib64 -L/lib64 -L/usr/lib64
>> -L/usr/local/sparc64-unknown-linux-gnu/lib -L/usr/local/lib -L/lib\
>>   -L/usr/lib  -o tmpdir/ifunc-mainpn
>>   -L/home/jemarch/couts2/binutils-gdb/ld/testsuite/ld-ifunc -pie 
>> -Wl,-z,\
>>   now -Wl,--no-as-needed tmpdir/libifunc-libn.so tmpdir/ifunc-main.o
>>   Executing on host: sh -c {gcc
>>   -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
>>   -L/usr/local/sparc6\
>>   4-unknown-linux-gnu/lib64 -L/usr/local/lib64 -L/lib64 -L/usr/lib64
>>   -L/usr/local/sparc64-unknown-linux-gnu/l\
>>   ib -L/usr/local/lib -L/lib -L/usr/lib  -o tmpdir/ifunc-mainpn
>>   -L/home/jemarch/couts2/binutils-gdb/ld/tests\
>>   uite/ld-ifunc -pie -Wl,-z,now -Wl,--no-as-needed
>>   tmpdir/libifunc-libn.so tmpdir/ifunc-main.o 2>&1} /dev/nu\
>>   ll ld.tmp (timeout = 300)
>>   spawn [open ...]^M
>>   Running: tmpdir/ifunc-mainpn > tmpdir/ifunc-mainpn.out
>>   child killed: segmentation violation
>> FAIL: Run ifunc-main with PIE (-z now)
>>
>> Could you please take a look?
>> Thanks!
>
> Hi Jose,
>
> Thank you for the review and bringing the issue to my attention! I am 
> looking into it.
>
> Egeyar

The patch is updated and attached. It fixes ld's test cases that fail on 
sparc. No regressions are revealed by made check. Please take a look.
Thanks!

[-- Attachment #2: 0001-sparc-Prevent-all-but-undef-weak-syms-from-becoming-.patch --]
[-- Type: text/x-patch, Size: 5456 bytes --]

From 119970152ea0f18c08fe7985ff637607cb17a2c8 Mon Sep 17 00:00:00 2001
From: Egeyar Bagcioglu <egeyar.bagcioglu@oracle.com>
Date: Mon, 26 Jun 2017 18:56:43 -0700
Subject: [PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

Prevent sparc backend making symbols dynamic unless they are undefined weak. Use R_SPARC_RELATIVE for the symbols which are not dynamic in PIC.

This patch is tested on sparc64-unknown-linux-gnu, no regressions are found.

bfd/ChangeLog:

2017-06-27  Egeyar Bagcioglu  <egeyar.bagcioglu@oracle.com>

	* elfxx-sparc.c (allocate_dynrelocs): Don't make a symbol dynamic
	unless it is undefined weak.
	* elfxx-sparc.c (_bfd_sparc_elf_relocate_section): Set the flag
	relative_reloc to direct non-dynamic symbols to R_SPARC_RELATIVE
	relocation.
	* elfxx-sparc.c (_bfd_sparc_elf_finish_dynamic_symbol): If symbol
        is not dynamic in PIC, abort.
---
 bfd/elfxx-sparc.c |   67 +++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index a9362a3..1fd2141 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -2310,7 +2310,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	 Undefined weak syms won't yet be marked as dynamic.  */
       if (h->dynindx == -1
 	  && !h->forced_local
-          && !resolved_to_zero)
+          && !resolved_to_zero
+          && h->root.type == bfd_link_hash_undefweak)
 	{
 	  if (! bfd_elf_link_record_dynamic_symbol (info, h))
 	    return FALSE;
@@ -2422,7 +2423,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	 Undefined weak syms won't yet be marked as dynamic.  */
       if (h->dynindx == -1
 	  && !h->forced_local
-          && !resolved_to_zero)
+          && !resolved_to_zero
+          && h->root.type == bfd_link_hash_undefweak)
 	{
 	  if (! bfd_elf_link_record_dynamic_symbol (info, h))
 	    return FALSE;
@@ -2564,7 +2566,8 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
 	     Undefined weak syms won't yet be marked as dynamic.  */
 	  if (h->dynindx == -1
 	      && !h->forced_local
-              && !resolved_to_zero)
+              && !resolved_to_zero
+              && h->root.type == bfd_link_hash_undefweak)
 	    {
 	      if (! bfd_elf_link_record_dynamic_symbol (info, h))
 		return FALSE;
@@ -3125,6 +3128,7 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
       bfd_boolean is_plt = FALSE;
       bfd_boolean unresolved_reloc;
       bfd_boolean resolved_to_zero;
+      bfd_boolean relative_reloc;
 
       r_type = SPARC_ELF_R_TYPE (rel->r_info);
       if (r_type == R_SPARC_GNU_VTINHERIT
@@ -3349,6 +3353,7 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 	  if (htab->elf.sgot == NULL)
 	    abort ();
 
+	  relative_reloc = FALSE;
 	  if (h != NULL)
 	    {
 	      bfd_boolean dyn;
@@ -3382,6 +3387,16 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 		      SPARC_ELF_PUT_WORD (htab, output_bfd, relocation,
 					  htab->elf.sgot->contents + off);
 		      h->got.offset |= 1;
+
+		      if (h->dynindx == -1
+			  && !h->forced_local
+			  && h->root.type != bfd_link_hash_undefweak
+			  && bfd_link_pic (info))
+			{
+			  /* If this symbol isn't dynamic in PIC
+			     generate R_SPARC_RELATIVE here.  */
+			  relative_reloc = TRUE;
+			}
 		    }
 		}
 	      else
@@ -3401,25 +3416,9 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 		off &= ~1;
 	      else
 		{
-
 		  if (bfd_link_pic (info))
 		    {
-		      asection *s;
-		      Elf_Internal_Rela outrel;
-
-		      /* We need to generate a R_SPARC_RELATIVE reloc
-			 for the dynamic linker.  */
-		      s = htab->elf.srelgot;
-		      BFD_ASSERT (s != NULL);
-
-		      outrel.r_offset = (htab->elf.sgot->output_section->vma
-					 + htab->elf.sgot->output_offset
-					 + off);
-		      outrel.r_info = SPARC_ELF_R_INFO (htab, NULL,
-							0, R_SPARC_RELATIVE);
-		      outrel.r_addend = relocation;
-		      relocation = 0;
-		      sparc_elf_append_rela (output_bfd, s, &outrel);
+		      relative_reloc = TRUE;
 		    }
 
 		  SPARC_ELF_PUT_WORD (htab, output_bfd, relocation,
@@ -3427,6 +3426,27 @@ _bfd_sparc_elf_relocate_section (bfd *output_bfd,
 		  local_got_offsets[r_symndx] |= 1;
 		}
 	    }
+
+	  if (relative_reloc)
+	    {
+	      asection *s;
+	      Elf_Internal_Rela outrel;
+
+	      /* We need to generate a R_SPARC_RELATIVE reloc
+		 for the dynamic linker.  */
+	      s = htab->elf.srelgot;
+	      BFD_ASSERT (s != NULL);
+
+	      outrel.r_offset = (htab->elf.sgot->output_section->vma
+				 + htab->elf.sgot->output_offset
+				 + off);
+	      outrel.r_info = SPARC_ELF_R_INFO (htab, NULL,
+						0, R_SPARC_RELATIVE);
+	      outrel.r_addend = relocation;
+	      relocation = 0;
+	      sparc_elf_append_rela (output_bfd, s, &outrel);
+	    }
+
 	  relocation = htab->elf.sgot->output_offset + off - got_base;
 	  break;
 
@@ -4482,6 +4502,13 @@ _bfd_sparc_elf_finish_dynamic_symbol (bfd *output_bfd,
 
   eh = (struct _bfd_sparc_elf_link_hash_entry *) h;
 
+  /* Abort if the symbol is not dynamic in PIC */
+  if (h->dynindx == -1
+      && !h->forced_local
+      && h->root.type != bfd_link_hash_undefweak
+      && bfd_link_pic (info))
+    abort();
+
   /* We keep PLT/GOT entries without dynamic PLT/GOT relocations for
      resolved undefined weak symbols in executable so that their
      references have value 0 at run-time.  */
-- 
1.7.1

Re: [PATCH] sparc: Prevent all but undef weak syms from becoming dynamic.

[Jose E. Marchesi]

    >>      Prevent sparc backend making symbols dynamic unless they are
    >> undefined
    >>      weak. By doing so, stop dynamic linker confusing these symbols with
    >>      library symbols.
    >>
    >> Your patch seems to fix all the currently failing tests of the LD
    >> testsuite in sparc64:
    >>
    >> FAIL: Run indirect5 3
    >> FAIL: Run indirect5 4
    >> FAIL: Run indirect6 3
    >> FAIL: Run indirect6 4
    >> FAIL: indirect5c dynsym
    >> FAIL: indirect5d dynsym
    >> FAIL: indirect6c dynsym
    >> FAIL: indirect6d dynsym
    >> FAIL: Run pr2404 with PIE
    >> FAIL: Run pr2404 with PIE (-z now)
    >>
    >> These all pass now.  Which is great! :)
    >>
    >> However, it also triggers a regression:
    >>
    >> gcc -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
    >> -L/usr/local/sparc64-unknown-linux-gnu/lib64 \
    >> -L/usr/local/lib64 -L/lib64 -L/usr/lib64
    >> -L/usr/local/sparc64-unknown-linux-gnu/lib -L/usr/local/lib -L/lib\
    >>   -L/usr/lib  -o tmpdir/ifunc-mainpn
    >>   -L/home/jemarch/couts2/binutils-gdb/ld/testsuite/ld-ifunc -pie
    >> -Wl,-z,\
    >>   now -Wl,--no-as-needed tmpdir/libifunc-libn.so tmpdir/ifunc-main.o
    >>   Executing on host: sh -c {gcc
    >>   -B/home/jemarch/couts2/binutils-gdb/build/ld/tmpdir/ld/
    >>   -L/usr/local/sparc6\
    >>   4-unknown-linux-gnu/lib64 -L/usr/local/lib64 -L/lib64 -L/usr/lib64
    >>   -L/usr/local/sparc64-unknown-linux-gnu/l\
    >>   ib -L/usr/local/lib -L/lib -L/usr/lib  -o tmpdir/ifunc-mainpn
    >>   -L/home/jemarch/couts2/binutils-gdb/ld/tests\
    >>   uite/ld-ifunc -pie -Wl,-z,now -Wl,--no-as-needed
    >>   tmpdir/libifunc-libn.so tmpdir/ifunc-main.o 2>&1} /dev/nu\
    >>   ll ld.tmp (timeout = 300)
    >>   spawn [open ...]^M
    >>   Running: tmpdir/ifunc-mainpn > tmpdir/ifunc-mainpn.out
    >>   child killed: segmentation violation
    >> FAIL: Run ifunc-main with PIE (-z now)
    >>
    >> Could you please take a look?
    >> Thanks!
    >
    > Hi Jose,
    >
    > Thank you for the review and bringing the issue to my attention! I
    > am looking into it.
    >
    > Egeyar
    
    The patch is updated and attached. It fixes ld's test cases that fail
    on sparc. No regressions are revealed by made check. Please take a
    look.


Applied.
Thank you very mucho!