Re: [PATCH] tile: Remove the __tls_get_addr check from gc_mark_hook

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Oct 17, 2017 at 4:19 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Oct 17, 2017 at 02:27:36PM -0700, H.J. Lu wrote:
>> Reference to __tls_get_addr is created by check_relocs.  Since check_relocs
>> is called after garbage collection now, the __tls_get_addr check should be
>> removed from gc_mark_hook.
>
> What happens if the only reference to __tls_get_addr is an implicit
> one via TLS_GD_CALL?  Imagine a static --gc-sections link with
> __tls_get_addr in its own section.  Shouldn't you instead be creating
> the __tls_get_addr reference in gcc_mark_hook?
>

Here is the updated patch.  Tested with glibc build-many-glibcs.py.
OK for master?


-- 
H.J.

From 0ccde551af515ceaab49f4423c0de168dd67676f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 17 Oct 2017 13:07:50 -0700
Subject: [PATCH] tile: Mark __tls_get_addr in gc_mark_hook

TLS_GD_CALL relocations implicitly reference __tls_get_addr.  Since
elf_gc_mark_hook is called before check_relocs now, we need to call
_bfd_generic_link_add_one_symbol to mark __tls_get_addr for garbage
collection.

	* elf32-tilepro.c (tilepro_elf_gc_mark_hook): Call
	_bfd_generic_link_add_one_symbol to mark __tls_get_addr.
	* elfxx-tilegx.c (tilegx_elf_gc_mark_hook): Likewise.
---
 bfd/elf32-tilepro.c | 20 +++++++++++++-------
 bfd/elfxx-tilegx.c  | 20 +++++++++++++-------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/bfd/elf32-tilepro.c b/bfd/elf32-tilepro.c
index 7ab60bbaca..d9e282bc72 100644
--- a/bfd/elf32-tilepro.c
+++ b/bfd/elf32-tilepro.c
@@ -1864,16 +1864,22 @@ tilepro_elf_gc_mark_hook (asection *sec,
      dealing with TLS optimization, ought to be !bfd_link_executable (info).  */
   if (bfd_link_pic (info))
     {
+      struct bfd_link_hash_entry *bh;
+
       switch (ELF32_R_TYPE (rel->r_info))
 	{
 	case R_TILEPRO_TLS_GD_CALL:
-	  /* This reloc implicitly references __tls_get_addr.  We know
-	     another reloc will reference the same symbol as the one
-	     on this reloc, so the real symbol and section will be
-	     gc marked when processing the other reloc.  That lets
-	     us handle __tls_get_addr here.  */
-	  h = elf_link_hash_lookup (elf_hash_table (info), "__tls_get_addr",
-				    FALSE, FALSE, TRUE);
+	  /* This reloc implicitly references __tls_get_addr.  This
+	     function is called before check_relocs.  Mark it for
+	     garbage collection here.  */
+	  bh = NULL;
+	  if (! _bfd_generic_link_add_one_symbol (info, sec->owner,
+						  "__tls_get_addr", 0,
+						  bfd_und_section_ptr,
+						  0, NULL, FALSE,
+						  FALSE, &bh))
+	    return NULL;
+	  h = (struct elf_link_hash_entry *) bh;
 	  BFD_ASSERT (h != NULL);
 	  h->mark = 1;
 	  if (h->u.weakdef != NULL)
diff --git a/bfd/elfxx-tilegx.c b/bfd/elfxx-tilegx.c
index c9e860a70c..5ae0c53ea4 100644
--- a/bfd/elfxx-tilegx.c
+++ b/bfd/elfxx-tilegx.c
@@ -2103,16 +2103,22 @@ tilegx_elf_gc_mark_hook (asection *sec,
      dealing with TLS optimization, ought to be !bfd_link_executable (info).  */
   if (bfd_link_pic (info))
     {
+      struct bfd_link_hash_entry *bh;
+
       switch (TILEGX_ELF_R_TYPE (rel->r_info))
 	{
 	case R_TILEGX_TLS_GD_CALL:
-	  /* This reloc implicitly references __tls_get_addr.  We know
-	     another reloc will reference the same symbol as the one
-	     on this reloc, so the real symbol and section will be
-	     gc marked when processing the other reloc.  That lets
-	     us handle __tls_get_addr here.  */
-	  h = elf_link_hash_lookup (elf_hash_table (info), "__tls_get_addr",
-				    FALSE, FALSE, TRUE);
+	  /* This reloc implicitly references __tls_get_addr.  This
+	     function is called before check_relocs.  Mark it for
+	     garbage collection here.  */
+	  bh = NULL;
+	  if (! _bfd_generic_link_add_one_symbol (info, sec->owner,
+						  "__tls_get_addr", 0,
+						  bfd_und_section_ptr,
+						  0, NULL, FALSE,
+						  FALSE, &bh))
+	    return NULL;
+	  h = (struct elf_link_hash_entry *) bh;
 	  BFD_ASSERT (h != NULL);
 	  h->mark = 1;
 	  if (h->u.weakdef != NULL)
-- 
2.13.6