public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add a record_link_assignments hook to ldemul
@ 2016-04-28 21:01 H.J. Lu
  2016-04-28 23:38 ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2016-04-28 21:01 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

On Thu, Apr 28, 2016 at 10:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 6:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Apr 28, 2016 at 6:09 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, Apr 28, 2016 at 6:07 AM, Alan Modra <amodra@gmail.com> wrote:
>>>> On Thu, Apr 28, 2016 at 05:49:57AM -0700, H.J. Lu wrote:
>>>>> On Wed, Apr 27, 2016 at 6:11 PM, Alan Modra <amodra@gmail.com> wrote:
>>>>> > On Wed, Apr 27, 2016 at 03:24:48PM -0700, H.J. Lu wrote:
>>>>> >> bfd_elf_record_link_assignment
>>>>> >> is called after check_relocs.  bfd_elf_record_link_assignment sets non_elf,
>>>>> >> def_regular and  forced_local.   For PROVIDE, it also updates root.type. They
>>>>> >> are needed in reloc_checks.
>>>>> >
>>>>> > My guess is that symbol twiddling done in before_allocation should be
>>>>> > moved to a new ldemul hook called at the start of lang_do_assignments.
>>>>> > The idea being to stabilize symbols earlier.
>>>>> >
>>>>> > The hook would twiddle __ehdr_start and call find_statement_assignment
>>>>> > when lang_mark_phase_enum.  Reversing the __ehdr_start twiddle stays
>>>>> > in before_allocation.
>>>>> >
>>>>>
>>>>> I tried this.  But it doesn't work with __start/__stop symbols.  I
>>>>> need to know if they are defined and referenced local in check_relocs.
>>>> [snip]
>>>>
>>>>> --- a/ld/ldlang.c
>>>>> +++ b/ld/ldlang.c
>>>>> @@ -6930,6 +6930,8 @@ lang_process (void)
>>>>>       collection in order to make sure that all symbol aliases are resolved.  */
>>>>>    lang_do_assignments (lang_mark_phase_enum);
>>>>>
>>>>> +  ldemul_record_link_assignments (lang_mark_phase_enum);
>>>>> +
>>>>>    lang_do_memory_regions();
>>>>>    expld.phase = lang_first_phase_enum;
>>>>
>>>> You'll need to run ldemul_record_link_assignments before
>>>> lang_do_assignments if you want provided symbols to be defined.
>>>
>>> I got many more failures when I did that since many assignments
>>> haven't been processed yet.
>>>
>>>> I suggest renaming to ldemul_do_assignments and putting
>>>>   ldemul_do_assignments (phase);
>>>> inside lang_do_assignments just before the call to
>>>> lang_do_assignments_1.
>>>>
>>
>> With this patch, I only got 2 failures on x86-64.
>>
>
> This one works for both i386 and x86-64.
>

This is the complete patch.  OK for master?


-- 
H.J.

[-- Attachment #2: 0001-Add-a-record_link_assignments-hook-to-ldemul.patch --]
[-- Type: text/x-patch, Size: 22676 bytes --]

From e34bcbafec5ec5181440d4abe9a4f43ca5eb50c8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 28 Apr 2016 05:25:17 -0700
Subject: [PATCH] Add a record_link_assignments hook to ldemul

When ELF relocation check is moved after lang_gc_sections, we should
call bfd_elf_record_link_assignment before ELF relocation check so that
linker defined symbols will be processed earlier to stabilize symbols.
We have to do it after lang_do_assignments to make sure that assignments
in linker scripts are processed first before updating fields in
elf_link_hash_entry.

bfd/

	* elf-bfd.h (_bfd_elf_record_start_stop): New prototype.
	* elflink.c (_bfd_elf_is_start_stop): Updated to check symbols
	currently defined in dynamic objects.
	(elf_link_record_start_stop): New function.
	(_bfd_elf_record_start_stop): Likewise.

ld/

	* ldemul.c (ldemul_record_link_assignments): New function.
	* ldemul.h (ldemul_record_link_assignments): New prototype.
	(ld_emulation_xfer_struct): Add record_link_assignments.
	* ldlang.c (lang_process): Call ldemul_record_link_assignments
	after lang_do_assignments.
	* emultempl/aix.em (ld_${EMULATION_NAME}_emulation): Initialize
	the record_link_assignments field to NULL.
	* emultempl/armcoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/beos.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/generic.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/gld960.em (ld_gld960_emulation): Likewise.
	* emultempl/gld960c.em (ld_gld960coff_emulation): Likewise.
	* emultempl/linux.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/lnk960.em (ld_lnk960_emulation): Likewise.
	* emultempl/m68kcoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/msp430.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/pe.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/pep.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/sunos.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/ticoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/vanilla.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/elf32.em (ehdr_start): New variable.
	(ehdr_start_save): Likewise.
	(gld${EMULATION_NAME}_record_link_assignments): New function.
	(gld${EMULATION_NAME}_restore_ehdr_start): Likewise.
	(ehdr_start_empty): Removed.
	(gld${EMULATION_NAME}_before_allocation): Don't adjust
	__ehdr_start here.  Call gld${EMULATION_NAME}_restore_ehdr_start
	at the end.
	(ld_${EMULATION_NAME}_emulation): Initialize
	the record_link_assignments field with
	gld${EMULATION_NAME}_record_link_assignments by default.
---
 bfd/elf-bfd.h            |   3 +
 bfd/elflink.c            |  87 ++++++++++++++++++++++-------
 ld/emultempl/aix.em      |   3 +-
 ld/emultempl/armcoff.em  |   3 +-
 ld/emultempl/beos.em     |   3 +-
 ld/emultempl/elf32.em    | 139 +++++++++++++++++++++++++++--------------------
 ld/emultempl/generic.em  |   3 +-
 ld/emultempl/gld960.em   |   3 +-
 ld/emultempl/gld960c.em  |   3 +-
 ld/emultempl/linux.em    |   3 +-
 ld/emultempl/lnk960.em   |   3 +-
 ld/emultempl/m68kcoff.em |   3 +-
 ld/emultempl/msp430.em   |   3 +-
 ld/emultempl/pe.em       |   3 +-
 ld/emultempl/pep.em      |   3 +-
 ld/emultempl/sunos.em    |   3 +-
 ld/emultempl/ticoff.em   |   3 +-
 ld/emultempl/vanilla.em  |   3 +-
 ld/ldemul.c              |   7 +++
 ld/ldemul.h              |   6 ++
 ld/ldlang.c              |   3 +
 21 files changed, 196 insertions(+), 94 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 6c05b55..49dc70b 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2339,6 +2339,9 @@ extern bfd_boolean bfd_elf_gc_common_final_link
 extern asection *_bfd_elf_is_start_stop
   (const struct bfd_link_info *, struct elf_link_hash_entry *);
 
+extern void _bfd_elf_record_start_stop
+  (struct bfd_link_info *);
+
 extern bfd_boolean bfd_elf_reloc_symbol_deleted_p
   (bfd_vma, void *);
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index b6ff6b6..9395052 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -12230,8 +12230,9 @@ _bfd_elf_gc_mark_hook (asection *sec,
   return NULL;
 }
 
-/* For undefined __start_<name> and __stop_<name> symbols, return the
-   first input section matching <name>.  Return NULL otherwise.  */
+/* For __start_<name> and __stop_<name> symbols, return the first
+   input section matching <name> in a regular object.  Return NULL
+   otherwise.  */
 
 asection *
 _bfd_elf_is_start_stop (const struct bfd_link_info *info,
@@ -12239,17 +12240,28 @@ _bfd_elf_is_start_stop (const struct bfd_link_info *info,
 {
   asection *s;
   const char *sec_name;
+  bfd_boolean undefined = (h->root.type == bfd_link_hash_undefined
+			   || h->root.type == bfd_link_hash_undefweak);
 
-  if (h->root.type != bfd_link_hash_undefined
-      && h->root.type != bfd_link_hash_undefweak)
-    return NULL;
-
-  s = h->root.u.undef.section;
-  if (s != NULL)
+  if (undefined)
+    {
+      s = h->root.u.undef.section;
+      if (s != NULL)
+	{
+	  if (s == (asection *) 0 - 1)
+	    return NULL;
+	  return s;
+	}
+    }
+  else
     {
-      if (s == (asection *) 0 - 1)
+      /* Symbol is defined.  Check if it is also defined in a regular
+	 input file only when it is currently defined in a dynamic
+	 object, since otherwise, it can't be a __start_<name> nor
+	 __stop_<name> symbol.  */
+      if (!h->def_dynamic)
 	return NULL;
-      return s;
+      s = NULL;
     }
 
   sec_name = NULL;
@@ -12262,18 +12274,21 @@ _bfd_elf_is_start_stop (const struct bfd_link_info *info,
     {
       bfd *i;
 
+      /* Only check regular input files.  */
       for (i = info->input_bfds; i != NULL; i = i->link.next)
-	{
-	  s = bfd_get_section_by_name (i, sec_name);
-	  if (s != NULL)
-	    {
-	      h->root.u.undef.section = s;
-	      break;
-	    }
-	}
+	if ((i->flags
+	     & (DYNAMIC | BFD_LINKER_CREATED | BFD_PLUGIN)) == 0)
+	  {
+	    s = bfd_get_section_by_name (i, sec_name);
+	    if (s != NULL)
+	      {
+		h->root.u.undef.section = s;
+		break;
+	      }
+	  }
     }
 
-  if (s == NULL)
+  if (undefined && s == NULL)
     h->root.u.undef.section = (asection *) 0 - 1;
 
   return s;
@@ -13791,3 +13806,37 @@ elf_append_rel (bfd *abfd, asection *s, Elf_Internal_Rela *rel)
   BFD_ASSERT (loc + bed->s->sizeof_rel <= s->contents + s->size);
   bed->s->swap_reloc_out (abfd, rel, loc);
 }
+
+/* Record __start_<name> and __stop_<name> symbols referenced by
+   regular objects.  This is called via elf_link_hash_traverse.  */
+
+static bfd_boolean
+elf_link_record_start_stop (struct elf_link_hash_entry *h, void *data)
+{
+  if (!h->def_regular && h->ref_regular)
+    {
+      asection *sec
+	= _bfd_elf_is_start_stop ((struct bfd_link_info *) data, h);
+      if (sec != NULL)
+	{
+	  h->def_regular = 1;
+	  h->type = STT_OBJECT;
+	  /* If it is currently defined by a dynamic object, but not
+	     by a regular object, then mark it as undefined so that
+	     the generic linker will force the correct value.  */
+	  if (h->def_dynamic)
+	    h->root.type = bfd_link_hash_undefined;
+	}
+    }
+  return TRUE;
+}
+
+/* Record all __start_<name> and __stop_<name> symbols referenced by
+   regular objects.  */
+
+void
+_bfd_elf_record_start_stop (struct bfd_link_info * info)
+{
+  elf_link_hash_traverse (elf_hash_table (info),
+			  elf_link_record_start_stop, info);
+}
diff --git a/ld/emultempl/aix.em b/ld/emultempl/aix.em
index b9cab4e..b625c44 100644
--- a/ld/emultempl/aix.em
+++ b/ld/emultempl/aix.em
@@ -1554,6 +1554,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation = {
   NULL,				/* recognized_file */
   NULL,				/* find potential_libraries */
   NULL,				/* new_vers_pattern */
-  NULL				/* extra_map_file_text */
+  NULL,				/* extra_map_file_text */
+  NULL				/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/armcoff.em b/ld/emultempl/armcoff.em
index 387a1f7..adfaa1e 100644
--- a/ld/emultempl/armcoff.em
+++ b/ld/emultempl/armcoff.em
@@ -280,6 +280,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/beos.em b/ld/emultempl/beos.em
index 6430102..8f861a8 100644
--- a/ld/emultempl/beos.em
+++ b/ld/emultempl/beos.em
@@ -778,6 +778,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 4f5d1a4..bb73dff 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1330,6 +1330,7 @@ fragment <<EOF
 EOF
 fi
 
+if test x"$LDEMUL_RECORD_LINK_ASSIGNMENTS" != xgld"$EMULATION_NAME"_record_link_assignments; then
 fragment <<EOF
 
 /* Look through an expression for an assignment statement.  */
@@ -1398,7 +1399,81 @@ gld${EMULATION_NAME}_find_statement_assignment (lang_statement_union_type *s)
     gld${EMULATION_NAME}_find_exp_assignment (s->assignment_statement.exp);
 }
 
+static struct elf_link_hash_entry *ehdr_start;
+static struct bfd_link_hash_entry ehdr_start_save;
+
+static void
+gld${EMULATION_NAME}_record_link_assignments (lang_phase_type phase)
+{
+  if (phase == lang_mark_phase_enum
+      && is_elf_hash_table (link_info.hash))
+    {
+      /* Make __ehdr_start hidden if it has been referenced, to
+	 prevent the symbol from being dynamic.  */
+      if (!bfd_link_relocatable (&link_info))
+	{
+	  struct elf_link_hash_entry *h
+	    = elf_link_hash_lookup (elf_hash_table (&link_info),
+				    "__ehdr_start", FALSE, FALSE, TRUE);
+
+	  /* Only adjust the export class if the symbol was referenced
+	     and not defined, otherwise leave it alone.  */
+	  if (h != NULL
+	      && (h->root.type == bfd_link_hash_new
+		  || h->root.type == bfd_link_hash_undefined
+		  || h->root.type == bfd_link_hash_undefweak
+		  || h->root.type == bfd_link_hash_common))
+	    {
+	      _bfd_elf_link_hash_hide_symbol (&link_info, h, TRUE);
+	      if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
+		h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
+	      /* Don't leave the symbol undefined.  Undefined hidden
+		 symbols typically won't have dynamic relocations, but
+		 we most likely will need dynamic relocations for
+		 __ehdr_start if we are building a PIE or shared
+		 library.  */
+	      ehdr_start = h;
+	      ehdr_start_save = h->root;
+	      h->root.type = bfd_link_hash_defined;
+	      h->root.u.def.section = bfd_abs_section_ptr;
+	      h->root.u.def.value = 0;
+	    }
+	}
+
+      /* If we are going to make any variable assignments, we need to
+	 let the ELF backend know about them in case the variables are
+	 referred to by dynamic objects.  */
+      lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
+
+      /* Also let the ELF backend know about __start_<name> and
+	 __stop_<name> symbols.  */
+      _bfd_elf_record_start_stop (&link_info);
+    }
+}
+
+static void
+gld${EMULATION_NAME}_restore_ehdr_start (void)
+{
+  if (ehdr_start != NULL)
+    {
+      /* If we twiddled __ehdr_start to defined earlier, put it back
+	 as it was.  */
+      ehdr_start->root.type = ehdr_start_save.type;
+      ehdr_start->root.u = ehdr_start_save.u;
+    }
+}
+
 EOF
+else
+fragment <<EOF
+
+static
+gld${EMULATION_NAME}_restore_ehdr_start (void)
+{
+}
+
+EOF
+fi
 
 if test x"$LDEMUL_BEFORE_ALLOCATION" != xgld"$EMULATION_NAME"_before_allocation; then
   if test x"${ELF_INTERPRETER_NAME+set}" = xset; then
@@ -1455,11 +1530,6 @@ gld${EMULATION_NAME}_append_to_separated_string (char **to, char *op_arg)
     }
 }
 
-#if defined(__GNUC__) && GCC_VERSION < 4006
-  /* Work around a GCC uninitialized warning bug fixed in GCC 4.6.  */
-static struct bfd_link_hash_entry ehdr_start_empty;
-#endif
-
 /* This is called after the sections have been attached to output
    sections, but before any sizes or addresses have been set.  */
 
@@ -1469,55 +1539,9 @@ gld${EMULATION_NAME}_before_allocation (void)
   const char *rpath;
   asection *sinterp;
   bfd *abfd;
-  struct elf_link_hash_entry *ehdr_start = NULL;
-#if defined(__GNUC__) && GCC_VERSION < 4006
-  /* Work around a GCC uninitialized warning bug fixed in GCC 4.6.  */
-  struct bfd_link_hash_entry ehdr_start_save = ehdr_start_empty;
-#else
-  struct bfd_link_hash_entry ehdr_start_save;
-#endif
 
   if (is_elf_hash_table (link_info.hash))
-    {
-      _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
-
-      /* Make __ehdr_start hidden if it has been referenced, to
-	 prevent the symbol from being dynamic.  */
-      if (!bfd_link_relocatable (&link_info))
-       {
-         struct elf_link_hash_entry *h
-           = elf_link_hash_lookup (elf_hash_table (&link_info), "__ehdr_start",
-                                   FALSE, FALSE, TRUE);
-
-         /* Only adjust the export class if the symbol was referenced
-            and not defined, otherwise leave it alone.  */
-         if (h != NULL
-             && (h->root.type == bfd_link_hash_new
-                 || h->root.type == bfd_link_hash_undefined
-                 || h->root.type == bfd_link_hash_undefweak
-                 || h->root.type == bfd_link_hash_common))
-           {
-             _bfd_elf_link_hash_hide_symbol (&link_info, h, TRUE);
-             if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
-               h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
-	     /* Don't leave the symbol undefined.  Undefined hidden
-		symbols typically won't have dynamic relocations, but
-		we most likely will need dynamic relocations for
-		__ehdr_start if we are building a PIE or shared
-		library.  */
-	     ehdr_start = h;
-	     ehdr_start_save = h->root;
-	     h->root.type = bfd_link_hash_defined;
-	     h->root.u.def.section = bfd_abs_section_ptr;
-	     h->root.u.def.value = 0;
-           }
-       }
-
-      /* If we are going to make any variable assignments, we need to
-	 let the ELF backend know about them in case the variables are
-	 referred to by dynamic objects.  */
-      lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
-    }
+    _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
 
   /* Let the ELF backend work out the sizes of any sections required
      by dynamic linking.  */
@@ -1627,13 +1651,7 @@ ${ELF_INTERPRETER_SET_DEFAULT}
   if (!bfd_elf_size_dynsym_hash_dynstr (link_info.output_bfd, &link_info))
     einfo ("%P%F: failed to set dynamic section sizes: %E\n");
 
-  if (ehdr_start != NULL)
-    {
-      /* If we twiddled __ehdr_start to defined earlier, put it back
-	 as it was.  */
-      ehdr_start->root.type = ehdr_start_save.type;
-      ehdr_start->root.u = ehdr_start_save.u;
-    }
+  gld${EMULATION_NAME}_restore_ehdr_start ();
 }
 
 EOF
@@ -2527,6 +2545,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-gld${EMULATION_NAME}_load_symbols},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-gld${EMULATION_NAME}_record_link_assignments},
 };
 EOF
diff --git a/ld/emultempl/generic.em b/ld/emultempl/generic.em
index 7924cdf..cfe2b94 100644
--- a/ld/emultempl/generic.em
+++ b/ld/emultempl/generic.em
@@ -156,6 +156,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-NULL},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-NULL}
 };
 EOF
diff --git a/ld/emultempl/gld960.em b/ld/emultempl/gld960.em
index c4c9c55..3b22d1b 100644
--- a/ld/emultempl/gld960.em
+++ b/ld/emultempl/gld960.em
@@ -149,6 +149,7 @@ struct ld_emulation_xfer_struct ld_gld960_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/gld960c.em b/ld/emultempl/gld960c.em
index 6b80be2..09b09e9 100644
--- a/ld/emultempl/gld960c.em
+++ b/ld/emultempl/gld960c.em
@@ -162,6 +162,7 @@ struct ld_emulation_xfer_struct ld_gld960coff_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/linux.em b/ld/emultempl/linux.em
index c28e978..cc68c23 100644
--- a/ld/emultempl/linux.em
+++ b/ld/emultempl/linux.em
@@ -206,6 +206,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/lnk960.em b/ld/emultempl/lnk960.em
index 4a2bd72..ea4e133 100644
--- a/ld/emultempl/lnk960.em
+++ b/ld/emultempl/lnk960.em
@@ -343,6 +343,7 @@ struct ld_emulation_xfer_struct ld_lnk960_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL, /* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/m68kcoff.em b/ld/emultempl/m68kcoff.em
index 594cd56..dd4af3a 100644
--- a/ld/emultempl/m68kcoff.em
+++ b/ld/emultempl/m68kcoff.em
@@ -240,6 +240,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index 22e7c42..41d5a48 100644
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -295,7 +295,8 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-NULL},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-NULL}
 };
 EOF
 # \f
diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index c13fa4d..9d63eac 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -2483,6 +2483,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   gld_${EMULATION_NAME}_recognized_file,
   gld_${EMULATION_NAME}_find_potential_libraries,
   NULL,	/* new_vers_pattern.  */
-  NULL	/* extra_map_file_text.  */
+  NULL,	/* extra_map_file_text.  */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ab7c473..62cfb32 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -2257,6 +2257,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   gld_${EMULATION_NAME}_recognized_file,
   gld_${EMULATION_NAME}_find_potential_libraries,
   NULL,	/* new_vers_pattern.  */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/sunos.em b/ld/emultempl/sunos.em
index 8be8669..3f6bb23 100644
--- a/ld/emultempl/sunos.em
+++ b/ld/emultempl/sunos.em
@@ -1036,6 +1036,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/ticoff.em b/ld/emultempl/ticoff.em
index 9b5495e..b596b8f 100644
--- a/ld/emultempl/ticoff.em
+++ b/ld/emultempl/ticoff.em
@@ -181,6 +181,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL, /* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL  /* extra_map_file_text */
+  NULL, /* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/vanilla.em b/ld/emultempl/vanilla.em
index d627dfc..e80189c 100644
--- a/ld/emultempl/vanilla.em
+++ b/ld/emultempl/vanilla.em
@@ -82,6 +82,7 @@ struct ld_emulation_xfer_struct ld_vanilla_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/ldemul.c b/ld/ldemul.c
index 841a14d..c1a75f1 100644
--- a/ld/ldemul.c
+++ b/ld/ldemul.c
@@ -355,3 +355,10 @@ ldemul_extra_map_file_text (bfd *abfd, struct bfd_link_info *info, FILE *mapf)
   if (ld_emulation->extra_map_file_text)
     ld_emulation->extra_map_file_text (abfd, info, mapf);
 }
+
+void
+ldemul_record_link_assignments (lang_phase_type phase)
+{
+  if (ld_emulation->record_link_assignments)
+    ld_emulation->record_link_assignments (phase);
+}
diff --git a/ld/ldemul.h b/ld/ldemul.h
index 937b1c9..7f823d7 100644
--- a/ld/ldemul.h
+++ b/ld/ldemul.h
@@ -96,6 +96,8 @@ extern struct bfd_elf_version_expr *ldemul_new_vers_pattern
   (struct bfd_elf_version_expr *);
 extern void ldemul_extra_map_file_text
   (bfd *, struct bfd_link_info *, FILE *);
+extern void ldemul_record_link_assignments
+  (lang_phase_type);
 
 typedef struct ld_emulation_xfer_struct {
   /* Run before parsing the command line and script file.
@@ -201,6 +203,10 @@ typedef struct ld_emulation_xfer_struct {
   void (*extra_map_file_text)
     (bfd *, struct bfd_link_info *, FILE *);
 
+  /* Called to record link assignments.  */
+  void (*record_link_assignments)
+    (lang_phase_type);
+
 } ld_emulation_xfer_type;
 
 typedef enum {
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 96947da..881d0a3 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6930,6 +6930,9 @@ lang_process (void)
      collection in order to make sure that all symbol aliases are resolved.  */
   lang_do_assignments (lang_mark_phase_enum);
 
+  /* Record symbols created by linker and linker scripts.  */
+  ldemul_record_link_assignments (lang_mark_phase_enum);
+
   lang_do_memory_regions();
   expld.phase = lang_first_phase_enum;
 
-- 
2.5.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add a record_link_assignments hook to ldemul
  2016-04-28 21:01 [PATCH] Add a record_link_assignments hook to ldemul H.J. Lu
@ 2016-04-28 23:38 ` Alan Modra
  2016-04-29  1:00   ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2016-04-28 23:38 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
> +      /* Symbol is defined.  Check if it is also defined in a regular
> +	 input file only when it is currently defined in a dynamic
> +	 object, since otherwise, it can't be a __start_<name> nor
> +	 __stop_<name> symbol.  */
> +      if (!h->def_dynamic)
>  	return NULL;
[snip]
> +	    if (s != NULL)
> +	      {
> +		h->root.u.undef.section = s;
> +		break;
> +	      }

You can't set u.undef here on a defined symbol.  That's just too ugly,
even if you later set it to undefined.  Better to force it
bfd_link_hash_undefined here.

This is getting quite messy, and I'm wondering if we even need
_bfd_elf_is_start_stop, except for gc-sections code.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add a record_link_assignments hook to ldemul
  2016-04-28 23:38 ` Alan Modra
@ 2016-04-29  1:00   ` H.J. Lu
  2016-04-29  6:35     ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2016-04-29  1:00 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

On Thu, Apr 28, 2016 at 4:38 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
>> +      /* Symbol is defined.  Check if it is also defined in a regular
>> +      input file only when it is currently defined in a dynamic
>> +      object, since otherwise, it can't be a __start_<name> nor
>> +      __stop_<name> symbol.  */
>> +      if (!h->def_dynamic)
>>       return NULL;
> [snip]
>> +         if (s != NULL)
>> +           {
>> +             h->root.u.undef.section = s;
>> +             break;
>> +           }
>
> You can't set u.undef here on a defined symbol.  That's just too ugly,
> even if you later set it to undefined.  Better to force it
> bfd_link_hash_undefined here.

Like this?

> This is getting quite messy, and I'm wondering if we even need
> _bfd_elf_is_start_stop, except for gc-sections code.
>

-- 
H.J.

[-- Attachment #2: 0001-Add-a-record_link_assignments-hook-to-ldemul.patch --]
[-- Type: text/x-patch, Size: 22801 bytes --]

From 8b85a754a0b5e2738c06ec9ecd87bc079751ecec Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 28 Apr 2016 05:25:17 -0700
Subject: [PATCH] Add a record_link_assignments hook to ldemul

When ELF relocation check is moved after lang_gc_sections, we should
call bfd_elf_record_link_assignment before ELF relocation check so that
linker defined symbols will be processed earlier to stabilize symbols.
We have to do it after lang_do_assignments to make sure that assignments
in linker scripts are processed first before updating fields in
elf_link_hash_entry.

bfd/

	* elf-bfd.h (_bfd_elf_record_start_stop): New prototype.
	* elflink.c (_bfd_elf_is_start_stop): Updated to check symbols
	currently defined in dynamic objects.  If a symbol isn't defined
	in a regular object, force it to bfd_link_hash_undefined.
	(elf_link_record_start_stop): New function.
	(_bfd_elf_record_start_stop): Likewise.

ld/

	* ldemul.c (ldemul_record_link_assignments): New function.
	* ldemul.h (ldemul_record_link_assignments): New prototype.
	(ld_emulation_xfer_struct): Add record_link_assignments.
	* ldlang.c (lang_process): Call ldemul_record_link_assignments
	after lang_do_assignments.
	* emultempl/aix.em (ld_${EMULATION_NAME}_emulation): Initialize
	the record_link_assignments field to NULL.
	* emultempl/armcoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/beos.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/generic.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/gld960.em (ld_gld960_emulation): Likewise.
	* emultempl/gld960c.em (ld_gld960coff_emulation): Likewise.
	* emultempl/linux.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/lnk960.em (ld_lnk960_emulation): Likewise.
	* emultempl/m68kcoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/msp430.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/pe.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/pep.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/sunos.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/ticoff.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/vanilla.em (ld_${EMULATION_NAME}_emulation): Likewise.
	* emultempl/elf32.em (ehdr_start): New variable.
	(ehdr_start_save): Likewise.
	(gld${EMULATION_NAME}_record_link_assignments): New function.
	(gld${EMULATION_NAME}_restore_ehdr_start): Likewise.
	(ehdr_start_empty): Removed.
	(gld${EMULATION_NAME}_before_allocation): Don't adjust
	__ehdr_start here.  Call gld${EMULATION_NAME}_restore_ehdr_start
	at the end.
	(ld_${EMULATION_NAME}_emulation): Initialize
	the record_link_assignments field with
	gld${EMULATION_NAME}_record_link_assignments by default.
---
 bfd/elf-bfd.h            |   3 +
 bfd/elflink.c            |  89 +++++++++++++++++++++++-------
 ld/emultempl/aix.em      |   3 +-
 ld/emultempl/armcoff.em  |   3 +-
 ld/emultempl/beos.em     |   3 +-
 ld/emultempl/elf32.em    | 139 +++++++++++++++++++++++++++--------------------
 ld/emultempl/generic.em  |   3 +-
 ld/emultempl/gld960.em   |   3 +-
 ld/emultempl/gld960c.em  |   3 +-
 ld/emultempl/linux.em    |   3 +-
 ld/emultempl/lnk960.em   |   3 +-
 ld/emultempl/m68kcoff.em |   3 +-
 ld/emultempl/msp430.em   |   3 +-
 ld/emultempl/pe.em       |   3 +-
 ld/emultempl/pep.em      |   3 +-
 ld/emultempl/sunos.em    |   3 +-
 ld/emultempl/ticoff.em   |   3 +-
 ld/emultempl/vanilla.em  |   3 +-
 ld/ldemul.c              |   7 +++
 ld/ldemul.h              |   6 ++
 ld/ldlang.c              |   3 +
 21 files changed, 198 insertions(+), 94 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 6c05b55..49dc70b 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2339,6 +2339,9 @@ extern bfd_boolean bfd_elf_gc_common_final_link
 extern asection *_bfd_elf_is_start_stop
   (const struct bfd_link_info *, struct elf_link_hash_entry *);
 
+extern void _bfd_elf_record_start_stop
+  (struct bfd_link_info *);
+
 extern bfd_boolean bfd_elf_reloc_symbol_deleted_p
   (bfd_vma, void *);
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index b6ff6b6..021db68 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -12230,8 +12230,9 @@ _bfd_elf_gc_mark_hook (asection *sec,
   return NULL;
 }
 
-/* For undefined __start_<name> and __stop_<name> symbols, return the
-   first input section matching <name>.  Return NULL otherwise.  */
+/* For __start_<name> and __stop_<name> symbols, return the first
+   input section matching <name> in a regular object.  Return NULL
+   otherwise.  */
 
 asection *
 _bfd_elf_is_start_stop (const struct bfd_link_info *info,
@@ -12239,17 +12240,28 @@ _bfd_elf_is_start_stop (const struct bfd_link_info *info,
 {
   asection *s;
   const char *sec_name;
+  bfd_boolean undefined = (h->root.type == bfd_link_hash_undefined
+			   || h->root.type == bfd_link_hash_undefweak);
 
-  if (h->root.type != bfd_link_hash_undefined
-      && h->root.type != bfd_link_hash_undefweak)
-    return NULL;
-
-  s = h->root.u.undef.section;
-  if (s != NULL)
+  if (undefined)
+    {
+      s = h->root.u.undef.section;
+      if (s != NULL)
+	{
+	  if (s == (asection *) 0 - 1)
+	    return NULL;
+	  return s;
+	}
+    }
+  else
     {
-      if (s == (asection *) 0 - 1)
+      /* Symbol is defined.  Check if it is also defined in a regular
+	 input file only when it is currently defined in a dynamic
+	 object, since otherwise, it can't be a __start_<name> nor
+	 __stop_<name> symbol.  */
+      if (!h->def_dynamic)
 	return NULL;
-      return s;
+      s = NULL;
     }
 
   sec_name = NULL;
@@ -12262,18 +12274,30 @@ _bfd_elf_is_start_stop (const struct bfd_link_info *info,
     {
       bfd *i;
 
+      /* Only check regular input files.  */
       for (i = info->input_bfds; i != NULL; i = i->link.next)
-	{
-	  s = bfd_get_section_by_name (i, sec_name);
-	  if (s != NULL)
-	    {
-	      h->root.u.undef.section = s;
-	      break;
-	    }
-	}
+	if ((i->flags
+	     & (DYNAMIC | BFD_LINKER_CREATED | BFD_PLUGIN)) == 0)
+	  {
+	    s = bfd_get_section_by_name (i, sec_name);
+	    if (s != NULL)
+	      {
+		if (undefined)
+		  h->root.u.undef.section = s;
+		else if (!h->def_regular)
+		  {
+		    /* If it is currently defined by a dynamic object,
+		       but not by a regular object, then mark it as
+		       undefined so that the generic linker will force
+		       the correct value.  */
+		    h->root.type = bfd_link_hash_undefined;
+		  }
+		break;
+	      }
+	  }
     }
 
-  if (s == NULL)
+  if (undefined && s == NULL)
     h->root.u.undef.section = (asection *) 0 - 1;
 
   return s;
@@ -13791,3 +13815,30 @@ elf_append_rel (bfd *abfd, asection *s, Elf_Internal_Rela *rel)
   BFD_ASSERT (loc + bed->s->sizeof_rel <= s->contents + s->size);
   bed->s->swap_reloc_out (abfd, rel, loc);
 }
+
+/* Record __start_<name> and __stop_<name> symbols referenced by
+   regular objects.  This is called via elf_link_hash_traverse.  */
+
+static bfd_boolean
+elf_link_record_start_stop (struct elf_link_hash_entry *h, void *data)
+{
+  if (!h->def_regular
+      && h->ref_regular
+      && (_bfd_elf_is_start_stop ((struct bfd_link_info *) data, h)
+	  != NULL))
+    {
+      h->def_regular = 1;
+      h->type = STT_OBJECT;
+    }
+  return TRUE;
+}
+
+/* Record all __start_<name> and __stop_<name> symbols referenced by
+   regular objects.  */
+
+void
+_bfd_elf_record_start_stop (struct bfd_link_info * info)
+{
+  elf_link_hash_traverse (elf_hash_table (info),
+			  elf_link_record_start_stop, info);
+}
diff --git a/ld/emultempl/aix.em b/ld/emultempl/aix.em
index b9cab4e..b625c44 100644
--- a/ld/emultempl/aix.em
+++ b/ld/emultempl/aix.em
@@ -1554,6 +1554,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation = {
   NULL,				/* recognized_file */
   NULL,				/* find potential_libraries */
   NULL,				/* new_vers_pattern */
-  NULL				/* extra_map_file_text */
+  NULL,				/* extra_map_file_text */
+  NULL				/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/armcoff.em b/ld/emultempl/armcoff.em
index 387a1f7..adfaa1e 100644
--- a/ld/emultempl/armcoff.em
+++ b/ld/emultempl/armcoff.em
@@ -280,6 +280,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/beos.em b/ld/emultempl/beos.em
index 6430102..8f861a8 100644
--- a/ld/emultempl/beos.em
+++ b/ld/emultempl/beos.em
@@ -778,6 +778,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 4f5d1a4..bb73dff 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1330,6 +1330,7 @@ fragment <<EOF
 EOF
 fi
 
+if test x"$LDEMUL_RECORD_LINK_ASSIGNMENTS" != xgld"$EMULATION_NAME"_record_link_assignments; then
 fragment <<EOF
 
 /* Look through an expression for an assignment statement.  */
@@ -1398,7 +1399,81 @@ gld${EMULATION_NAME}_find_statement_assignment (lang_statement_union_type *s)
     gld${EMULATION_NAME}_find_exp_assignment (s->assignment_statement.exp);
 }
 
+static struct elf_link_hash_entry *ehdr_start;
+static struct bfd_link_hash_entry ehdr_start_save;
+
+static void
+gld${EMULATION_NAME}_record_link_assignments (lang_phase_type phase)
+{
+  if (phase == lang_mark_phase_enum
+      && is_elf_hash_table (link_info.hash))
+    {
+      /* Make __ehdr_start hidden if it has been referenced, to
+	 prevent the symbol from being dynamic.  */
+      if (!bfd_link_relocatable (&link_info))
+	{
+	  struct elf_link_hash_entry *h
+	    = elf_link_hash_lookup (elf_hash_table (&link_info),
+				    "__ehdr_start", FALSE, FALSE, TRUE);
+
+	  /* Only adjust the export class if the symbol was referenced
+	     and not defined, otherwise leave it alone.  */
+	  if (h != NULL
+	      && (h->root.type == bfd_link_hash_new
+		  || h->root.type == bfd_link_hash_undefined
+		  || h->root.type == bfd_link_hash_undefweak
+		  || h->root.type == bfd_link_hash_common))
+	    {
+	      _bfd_elf_link_hash_hide_symbol (&link_info, h, TRUE);
+	      if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
+		h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
+	      /* Don't leave the symbol undefined.  Undefined hidden
+		 symbols typically won't have dynamic relocations, but
+		 we most likely will need dynamic relocations for
+		 __ehdr_start if we are building a PIE or shared
+		 library.  */
+	      ehdr_start = h;
+	      ehdr_start_save = h->root;
+	      h->root.type = bfd_link_hash_defined;
+	      h->root.u.def.section = bfd_abs_section_ptr;
+	      h->root.u.def.value = 0;
+	    }
+	}
+
+      /* If we are going to make any variable assignments, we need to
+	 let the ELF backend know about them in case the variables are
+	 referred to by dynamic objects.  */
+      lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
+
+      /* Also let the ELF backend know about __start_<name> and
+	 __stop_<name> symbols.  */
+      _bfd_elf_record_start_stop (&link_info);
+    }
+}
+
+static void
+gld${EMULATION_NAME}_restore_ehdr_start (void)
+{
+  if (ehdr_start != NULL)
+    {
+      /* If we twiddled __ehdr_start to defined earlier, put it back
+	 as it was.  */
+      ehdr_start->root.type = ehdr_start_save.type;
+      ehdr_start->root.u = ehdr_start_save.u;
+    }
+}
+
 EOF
+else
+fragment <<EOF
+
+static
+gld${EMULATION_NAME}_restore_ehdr_start (void)
+{
+}
+
+EOF
+fi
 
 if test x"$LDEMUL_BEFORE_ALLOCATION" != xgld"$EMULATION_NAME"_before_allocation; then
   if test x"${ELF_INTERPRETER_NAME+set}" = xset; then
@@ -1455,11 +1530,6 @@ gld${EMULATION_NAME}_append_to_separated_string (char **to, char *op_arg)
     }
 }
 
-#if defined(__GNUC__) && GCC_VERSION < 4006
-  /* Work around a GCC uninitialized warning bug fixed in GCC 4.6.  */
-static struct bfd_link_hash_entry ehdr_start_empty;
-#endif
-
 /* This is called after the sections have been attached to output
    sections, but before any sizes or addresses have been set.  */
 
@@ -1469,55 +1539,9 @@ gld${EMULATION_NAME}_before_allocation (void)
   const char *rpath;
   asection *sinterp;
   bfd *abfd;
-  struct elf_link_hash_entry *ehdr_start = NULL;
-#if defined(__GNUC__) && GCC_VERSION < 4006
-  /* Work around a GCC uninitialized warning bug fixed in GCC 4.6.  */
-  struct bfd_link_hash_entry ehdr_start_save = ehdr_start_empty;
-#else
-  struct bfd_link_hash_entry ehdr_start_save;
-#endif
 
   if (is_elf_hash_table (link_info.hash))
-    {
-      _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
-
-      /* Make __ehdr_start hidden if it has been referenced, to
-	 prevent the symbol from being dynamic.  */
-      if (!bfd_link_relocatable (&link_info))
-       {
-         struct elf_link_hash_entry *h
-           = elf_link_hash_lookup (elf_hash_table (&link_info), "__ehdr_start",
-                                   FALSE, FALSE, TRUE);
-
-         /* Only adjust the export class if the symbol was referenced
-            and not defined, otherwise leave it alone.  */
-         if (h != NULL
-             && (h->root.type == bfd_link_hash_new
-                 || h->root.type == bfd_link_hash_undefined
-                 || h->root.type == bfd_link_hash_undefweak
-                 || h->root.type == bfd_link_hash_common))
-           {
-             _bfd_elf_link_hash_hide_symbol (&link_info, h, TRUE);
-             if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
-               h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
-	     /* Don't leave the symbol undefined.  Undefined hidden
-		symbols typically won't have dynamic relocations, but
-		we most likely will need dynamic relocations for
-		__ehdr_start if we are building a PIE or shared
-		library.  */
-	     ehdr_start = h;
-	     ehdr_start_save = h->root;
-	     h->root.type = bfd_link_hash_defined;
-	     h->root.u.def.section = bfd_abs_section_ptr;
-	     h->root.u.def.value = 0;
-           }
-       }
-
-      /* If we are going to make any variable assignments, we need to
-	 let the ELF backend know about them in case the variables are
-	 referred to by dynamic objects.  */
-      lang_for_each_statement (gld${EMULATION_NAME}_find_statement_assignment);
-    }
+    _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
 
   /* Let the ELF backend work out the sizes of any sections required
      by dynamic linking.  */
@@ -1627,13 +1651,7 @@ ${ELF_INTERPRETER_SET_DEFAULT}
   if (!bfd_elf_size_dynsym_hash_dynstr (link_info.output_bfd, &link_info))
     einfo ("%P%F: failed to set dynamic section sizes: %E\n");
 
-  if (ehdr_start != NULL)
-    {
-      /* If we twiddled __ehdr_start to defined earlier, put it back
-	 as it was.  */
-      ehdr_start->root.type = ehdr_start_save.type;
-      ehdr_start->root.u = ehdr_start_save.u;
-    }
+  gld${EMULATION_NAME}_restore_ehdr_start ();
 }
 
 EOF
@@ -2527,6 +2545,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-gld${EMULATION_NAME}_load_symbols},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-gld${EMULATION_NAME}_record_link_assignments},
 };
 EOF
diff --git a/ld/emultempl/generic.em b/ld/emultempl/generic.em
index 7924cdf..cfe2b94 100644
--- a/ld/emultempl/generic.em
+++ b/ld/emultempl/generic.em
@@ -156,6 +156,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-NULL},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-NULL}
 };
 EOF
diff --git a/ld/emultempl/gld960.em b/ld/emultempl/gld960.em
index c4c9c55..3b22d1b 100644
--- a/ld/emultempl/gld960.em
+++ b/ld/emultempl/gld960.em
@@ -149,6 +149,7 @@ struct ld_emulation_xfer_struct ld_gld960_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/gld960c.em b/ld/emultempl/gld960c.em
index 6b80be2..09b09e9 100644
--- a/ld/emultempl/gld960c.em
+++ b/ld/emultempl/gld960c.em
@@ -162,6 +162,7 @@ struct ld_emulation_xfer_struct ld_gld960coff_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/linux.em b/ld/emultempl/linux.em
index c28e978..cc68c23 100644
--- a/ld/emultempl/linux.em
+++ b/ld/emultempl/linux.em
@@ -206,6 +206,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/lnk960.em b/ld/emultempl/lnk960.em
index 4a2bd72..ea4e133 100644
--- a/ld/emultempl/lnk960.em
+++ b/ld/emultempl/lnk960.em
@@ -343,6 +343,7 @@ struct ld_emulation_xfer_struct ld_lnk960_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL, /* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/m68kcoff.em b/ld/emultempl/m68kcoff.em
index 594cd56..dd4af3a 100644
--- a/ld/emultempl/m68kcoff.em
+++ b/ld/emultempl/m68kcoff.em
@@ -240,6 +240,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/msp430.em b/ld/emultempl/msp430.em
index 22e7c42..41d5a48 100644
--- a/ld/emultempl/msp430.em
+++ b/ld/emultempl/msp430.em
@@ -295,7 +295,8 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_RECOGNIZED_FILE-NULL},
   ${LDEMUL_FIND_POTENTIAL_LIBRARIES-NULL},
   ${LDEMUL_NEW_VERS_PATTERN-NULL},
-  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL}
+  ${LDEMUL_EXTRA_MAP_FILE_TEXT-NULL},
+  ${LDEMUL_RECORD_LINK_ASSIGNMENTS-NULL}
 };
 EOF
 # \f
diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index c13fa4d..9d63eac 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -2483,6 +2483,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   gld_${EMULATION_NAME}_recognized_file,
   gld_${EMULATION_NAME}_find_potential_libraries,
   NULL,	/* new_vers_pattern.  */
-  NULL	/* extra_map_file_text.  */
+  NULL,	/* extra_map_file_text.  */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ab7c473..62cfb32 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -2257,6 +2257,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   gld_${EMULATION_NAME}_recognized_file,
   gld_${EMULATION_NAME}_find_potential_libraries,
   NULL,	/* new_vers_pattern.  */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/sunos.em b/ld/emultempl/sunos.em
index 8be8669..3f6bb23 100644
--- a/ld/emultempl/sunos.em
+++ b/ld/emultempl/sunos.em
@@ -1036,6 +1036,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/ticoff.em b/ld/emultempl/ticoff.em
index 9b5495e..b596b8f 100644
--- a/ld/emultempl/ticoff.em
+++ b/ld/emultempl/ticoff.em
@@ -181,6 +181,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   NULL, /* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL  /* extra_map_file_text */
+  NULL, /* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/emultempl/vanilla.em b/ld/emultempl/vanilla.em
index d627dfc..e80189c 100644
--- a/ld/emultempl/vanilla.em
+++ b/ld/emultempl/vanilla.em
@@ -82,6 +82,7 @@ struct ld_emulation_xfer_struct ld_vanilla_emulation =
   NULL,	/* recognized file */
   NULL,	/* find_potential_libraries */
   NULL,	/* new_vers_pattern */
-  NULL	/* extra_map_file_text */
+  NULL,	/* extra_map_file_text */
+  NULL	/* record_link_assignments */
 };
 EOF
diff --git a/ld/ldemul.c b/ld/ldemul.c
index 841a14d..c1a75f1 100644
--- a/ld/ldemul.c
+++ b/ld/ldemul.c
@@ -355,3 +355,10 @@ ldemul_extra_map_file_text (bfd *abfd, struct bfd_link_info *info, FILE *mapf)
   if (ld_emulation->extra_map_file_text)
     ld_emulation->extra_map_file_text (abfd, info, mapf);
 }
+
+void
+ldemul_record_link_assignments (lang_phase_type phase)
+{
+  if (ld_emulation->record_link_assignments)
+    ld_emulation->record_link_assignments (phase);
+}
diff --git a/ld/ldemul.h b/ld/ldemul.h
index 937b1c9..7f823d7 100644
--- a/ld/ldemul.h
+++ b/ld/ldemul.h
@@ -96,6 +96,8 @@ extern struct bfd_elf_version_expr *ldemul_new_vers_pattern
   (struct bfd_elf_version_expr *);
 extern void ldemul_extra_map_file_text
   (bfd *, struct bfd_link_info *, FILE *);
+extern void ldemul_record_link_assignments
+  (lang_phase_type);
 
 typedef struct ld_emulation_xfer_struct {
   /* Run before parsing the command line and script file.
@@ -201,6 +203,10 @@ typedef struct ld_emulation_xfer_struct {
   void (*extra_map_file_text)
     (bfd *, struct bfd_link_info *, FILE *);
 
+  /* Called to record link assignments.  */
+  void (*record_link_assignments)
+    (lang_phase_type);
+
 } ld_emulation_xfer_type;
 
 typedef enum {
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 96947da..881d0a3 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6930,6 +6930,9 @@ lang_process (void)
      collection in order to make sure that all symbol aliases are resolved.  */
   lang_do_assignments (lang_mark_phase_enum);
 
+  /* Record symbols created by linker and linker scripts.  */
+  ldemul_record_link_assignments (lang_mark_phase_enum);
+
   lang_do_memory_regions();
   expld.phase = lang_first_phase_enum;
 
-- 
2.5.5


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add a record_link_assignments hook to ldemul
  2016-04-29  1:00   ` H.J. Lu
@ 2016-04-29  6:35     ` Alan Modra
  2016-04-29 12:30       ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2016-04-29  6:35 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Thu, Apr 28, 2016 at 05:59:55PM -0700, H.J. Lu wrote:
> On Thu, Apr 28, 2016 at 4:38 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
> >> +      /* Symbol is defined.  Check if it is also defined in a regular
> >> +      input file only when it is currently defined in a dynamic
> >> +      object, since otherwise, it can't be a __start_<name> nor
> >> +      __stop_<name> symbol.  */
> >> +      if (!h->def_dynamic)
> >>       return NULL;
> > [snip]
> >> +         if (s != NULL)
> >> +           {
> >> +             h->root.u.undef.section = s;
> >> +             break;
> >> +           }
> >
> > You can't set u.undef here on a defined symbol.  That's just too ugly,
> > even if you later set it to undefined.  Better to force it
> > bfd_link_hash_undefined here.
> 
> Like this?

No, I meant that if you want to make use of the u.undef.section cache,
then set it to undefined first.  If you don't intend to set
u.undef.section then set to undefined later.

> > This is getting quite messy, and I'm wondering if we even need
> > _bfd_elf_is_start_stop, except for gc-sections code.

Note that when I wrote _bfd_elf_is_start_stop I thought it might come
in useful in check_relocs, to prevent __start_* and __stop_* from
being seen as dynamic.  Now that you're running both
find_statement_assignment and lang_do_assignments before check_relocs,
doing anything special with _bfd_elf_is_start_stop should not be
necessary.

find_statement_assignment has given the bfd backend a look at symbols,
and you've processed the PROVIDE (__start_sec = .) linker script
statements in lang_do_assignments.  So it ought to be possible to make
__start_sec a def_regular normal symbol by that point.  As I said
before, I'd expect that to work better if find_statement_assignment
ran before lang_do_assignments.  That allows
bfd_elf_record_link_assignment to force def_dynamic symbols to
bfd_link_hash_undefined so that ldexp.c code handling PROVIDE doesn't
need to know ELF specific details.

However, you said that doing it that way didn't work.  What didn't
work, and what needs to change in bfd_elf_record_link_assignment to
make it work?

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add a record_link_assignments hook to ldemul
  2016-04-29  6:35     ` Alan Modra
@ 2016-04-29 12:30       ` H.J. Lu
  2016-04-29 13:35         ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2016-04-29 12:30 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Thu, Apr 28, 2016 at 11:35 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 05:59:55PM -0700, H.J. Lu wrote:
>> On Thu, Apr 28, 2016 at 4:38 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
>> >> +      /* Symbol is defined.  Check if it is also defined in a regular
>> >> +      input file only when it is currently defined in a dynamic
>> >> +      object, since otherwise, it can't be a __start_<name> nor
>> >> +      __stop_<name> symbol.  */
>> >> +      if (!h->def_dynamic)
>> >>       return NULL;
>> > [snip]
>> >> +         if (s != NULL)
>> >> +           {
>> >> +             h->root.u.undef.section = s;
>> >> +             break;
>> >> +           }
>> >
>> > You can't set u.undef here on a defined symbol.  That's just too ugly,
>> > even if you later set it to undefined.  Better to force it
>> > bfd_link_hash_undefined here.
>>
>> Like this?
>
> No, I meant that if you want to make use of the u.undef.section cache,
> then set it to undefined first.  If you don't intend to set
> u.undef.section then set to undefined later.
>
>> > This is getting quite messy, and I'm wondering if we even need
>> > _bfd_elf_is_start_stop, except for gc-sections code.
>
> Note that when I wrote _bfd_elf_is_start_stop I thought it might come
> in useful in check_relocs, to prevent __start_* and __stop_* from
> being seen as dynamic.  Now that you're running both
> find_statement_assignment and lang_do_assignments before check_relocs,
> doing anything special with _bfd_elf_is_start_stop should not be
> necessary.

 __start_* and __stop_* aren't handled properly with shared library:

https://sourceware.org/bugzilla/show_bug.cgi?id=20022

_bfd_elf_is_start_stop needs to check symbols defined in a
shared library.

> find_statement_assignment has given the bfd backend a look at symbols,
> and you've processed the PROVIDE (__start_sec = .) linker script
> statements in lang_do_assignments.  So it ought to be possible to make
> __start_sec a def_regular normal symbol by that point.  As I said
> before, I'd expect that to work better if find_statement_assignment
> ran before lang_do_assignments.  That allows
> bfd_elf_record_link_assignment to force def_dynamic symbols to
> bfd_link_hash_undefined so that ldexp.c code handling PROVIDE doesn't
> need to know ELF specific details.
>
> However, you said that doing it that way didn't work.  What didn't
> work, and what needs to change in bfd_elf_record_link_assignment to
> make it work?

The failed testcase is ld-elf/ehdr_start-userdef.  We have

         /* Only adjust the export class if the symbol was referenced
             and not defined, otherwise leave it alone.  */
          if (h != NULL
              && (h->root.type == bfd_link_hash_new
                  || h->root.type == bfd_link_hash_undefined
                  || h->root.type == bfd_link_hash_undefweak
                  || h->root.type == bfd_link_hash_common))
            {

Since this is run before  lang_do_assignments, we don't see

__ehdr_start = 0x12345678;

in linker script and get

(gdb) p *h
$2 = {root = {root = {next = 0x0, string = 0x859bad "__ehdr_start",
      hash = 78192523}, type = bfd_link_hash_undefined, non_ir_ref = 0,
    linker_def = 0, u = {undef = {next = 0x0, abfd = 0x857100, section = 0x0},
      def = {next = 0x0, section = 0x857100, value = 0}, i = {next = 0x0,
        link = 0x857100, warning = 0x0}, c = {next = 0x0, p = 0x857100,
        size = 0}}}, indx = -1, dynindx = -1, got = {refcount = 0, offset = 0,
    glist = 0x0, plist = 0x0}, plt = {refcount = 0, offset = 0, glist = 0x0,
    plist = 0x0}, size = 0, type = 0, other = 0, target_internal = 0,
  ref_regular = 1, def_regular = 0, ref_dynamic = 0, def_dynamic = 0,
  ref_regular_nonweak = 1, dynamic_adjusted = 0, needs_copy = 0,
  needs_plt = 0, non_elf = 0, versioned = unversioned, forced_local = 0,
  dynamic = 0, mark = 0, non_got_ref = 0, dynamic_def = 0,
  ref_dynamic_nonweak = 0, pointer_equality_needed = 0, unique_global = 0,
  protected_def = 0, dynstr_index = 0, u = {weakdef = 0x0,
    elf_hash_value = 0}, verinfo = {verdef = 0x0, vertree = 0x0}, vtable = 0x0}

We get

     3: 0000000012345678     0 NOTYPE  LOCAL  DEFAULT  ABS __ehdr_start


instead of

     3: 0000000012345678     0 NOTYPE  GLOBAL  DEFAULT  ABS __ehdr_start

in output.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add a record_link_assignments hook to ldemul
  2016-04-29 12:30       ` H.J. Lu
@ 2016-04-29 13:35         ` H.J. Lu
  0 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2016-04-29 13:35 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Fri, Apr 29, 2016 at 5:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Apr 28, 2016 at 11:35 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Thu, Apr 28, 2016 at 05:59:55PM -0700, H.J. Lu wrote:
>>> On Thu, Apr 28, 2016 at 4:38 PM, Alan Modra <amodra@gmail.com> wrote:
>>> > On Thu, Apr 28, 2016 at 02:01:47PM -0700, H.J. Lu wrote:
>>> >> +      /* Symbol is defined.  Check if it is also defined in a regular
>>> >> +      input file only when it is currently defined in a dynamic
>>> >> +      object, since otherwise, it can't be a __start_<name> nor
>>> >> +      __stop_<name> symbol.  */
>>> >> +      if (!h->def_dynamic)
>>> >>       return NULL;
>>> > [snip]
>>> >> +         if (s != NULL)
>>> >> +           {
>>> >> +             h->root.u.undef.section = s;
>>> >> +             break;
>>> >> +           }
>>> >
>>> > You can't set u.undef here on a defined symbol.  That's just too ugly,
>>> > even if you later set it to undefined.  Better to force it
>>> > bfd_link_hash_undefined here.
>>>
>>> Like this?
>>
>> No, I meant that if you want to make use of the u.undef.section cache,
>> then set it to undefined first.  If you don't intend to set
>> u.undef.section then set to undefined later.
>>
>>> > This is getting quite messy, and I'm wondering if we even need
>>> > _bfd_elf_is_start_stop, except for gc-sections code.
>>
>> Note that when I wrote _bfd_elf_is_start_stop I thought it might come
>> in useful in check_relocs, to prevent __start_* and __stop_* from
>> being seen as dynamic.  Now that you're running both
>> find_statement_assignment and lang_do_assignments before check_relocs,
>> doing anything special with _bfd_elf_is_start_stop should not be
>> necessary.
>
>  __start_* and __stop_* aren't handled properly with shared library:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=20022
>
> _bfd_elf_is_start_stop needs to check symbols defined in a
> shared library.
>
>> find_statement_assignment has given the bfd backend a look at symbols,
>> and you've processed the PROVIDE (__start_sec = .) linker script
>> statements in lang_do_assignments.  So it ought to be possible to make
>> __start_sec a def_regular normal symbol by that point.  As I said
>> before, I'd expect that to work better if find_statement_assignment
>> ran before lang_do_assignments.  That allows
>> bfd_elf_record_link_assignment to force def_dynamic symbols to
>> bfd_link_hash_undefined so that ldexp.c code handling PROVIDE doesn't
>> need to know ELF specific details.
>>
>> However, you said that doing it that way didn't work.  What didn't
>> work, and what needs to change in bfd_elf_record_link_assignment to
>> make it work?
>
> The failed testcase is ld-elf/ehdr_start-userdef.  We have
>
>          /* Only adjust the export class if the symbol was referenced
>              and not defined, otherwise leave it alone.  */
>           if (h != NULL
>               && (h->root.type == bfd_link_hash_new
>                   || h->root.type == bfd_link_hash_undefined
>                   || h->root.type == bfd_link_hash_undefweak
>                   || h->root.type == bfd_link_hash_common))
>             {
>
> Since this is run before  lang_do_assignments, we don't see
>
> __ehdr_start = 0x12345678;
>
> in linker script and get
>
> (gdb) p *h
> $2 = {root = {root = {next = 0x0, string = 0x859bad "__ehdr_start",
>       hash = 78192523}, type = bfd_link_hash_undefined, non_ir_ref = 0,
>     linker_def = 0, u = {undef = {next = 0x0, abfd = 0x857100, section = 0x0},
>       def = {next = 0x0, section = 0x857100, value = 0}, i = {next = 0x0,
>         link = 0x857100, warning = 0x0}, c = {next = 0x0, p = 0x857100,
>         size = 0}}}, indx = -1, dynindx = -1, got = {refcount = 0, offset = 0,
>     glist = 0x0, plist = 0x0}, plt = {refcount = 0, offset = 0, glist = 0x0,
>     plist = 0x0}, size = 0, type = 0, other = 0, target_internal = 0,
>   ref_regular = 1, def_regular = 0, ref_dynamic = 0, def_dynamic = 0,
>   ref_regular_nonweak = 1, dynamic_adjusted = 0, needs_copy = 0,
>   needs_plt = 0, non_elf = 0, versioned = unversioned, forced_local = 0,
>   dynamic = 0, mark = 0, non_got_ref = 0, dynamic_def = 0,
>   ref_dynamic_nonweak = 0, pointer_equality_needed = 0, unique_global = 0,
>   protected_def = 0, dynstr_index = 0, u = {weakdef = 0x0,
>     elf_hash_value = 0}, verinfo = {verdef = 0x0, vertree = 0x0}, vtable = 0x0}
>
> We get
>
>      3: 0000000012345678     0 NOTYPE  LOCAL  DEFAULT  ABS __ehdr_start
>
>
> instead of
>
>      3: 0000000012345678     0 NOTYPE  GLOBAL  DEFAULT  ABS __ehdr_start
>
> in output.

I have a fix for it.  But _bfd_elf_is_start_stop still needs fix.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-29 13:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28 21:01 [PATCH] Add a record_link_assignments hook to ldemul H.J. Lu
2016-04-28 23:38 ` Alan Modra
2016-04-29  1:00   ` H.J. Lu
2016-04-29  6:35     ` Alan Modra
2016-04-29 12:30       ` H.J. Lu
2016-04-29 13:35         ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).