Re: [RFC][PATCH] Check ELF relocs after opening all all input files

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

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.


-- 
H.J.

From 316b611580f1dd4242863f0fcded588980fe9e07 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 record_link_assignments to ldemul

---
 ld/emultempl/elf32.em | 136 ++++++++++++++++++++++++++++----------------------
 ld/emultempl/linux.em |   3 +-
 ld/ldemul.c           |   7 +++
 ld/ldemul.h           |   6 +++
 ld/ldlang.c           |   2 +
 5 files changed, 93 insertions(+), 61 deletions(-)

diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 4f5d1a4..cc2f056 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -64,6 +64,7 @@ static void gld${EMULATION_NAME}_after_parse (void);
 static void gld${EMULATION_NAME}_after_open (void);
 static void gld${EMULATION_NAME}_before_allocation (void);
 static void gld${EMULATION_NAME}_after_allocation (void);
+static void gld${EMULATION_NAME}_restore_ehdr_start (void);
 static lang_output_section_statement_type *gld${EMULATION_NAME}_place_orphan
   (asection *, const char *, int);
 EOF
@@ -1330,6 +1331,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 +1400,77 @@ 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);
+    }
+}
+
+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 +1527,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 +1536,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 +1648,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 +2542,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/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/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..3ece902 100644
--- 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;
 
-- 
2.5.5