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

[Egeyar Bagcioglu <egeyar dot bagcioglu at oracle dot com>]

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!

>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