public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC][PATCH] Check ELF relocs after opening all all input files
@ 2016-04-19 13:50 H.J. Lu
  2016-04-19 16:01 ` Nick Clifton
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-19 13:50 UTC (permalink / raw)
  To: binutils

Delaying checking ELF relocations until opening all input files so
that symbol information is final when relocations are checked.  This
is only enabled for x86 targets.

Any comments, feedbacks?


H.J.
---
bfd/

	* elf-bfd.h (_bfd_elf_link_check_relocs): New.
	* elf32-i386.c (elf_i386_tls_transition): Use
	info->callbacks->einfo instead of _bfd_error_handler on
	error.
	* elflink.c (_bfd_elf_link_check_relocs): New function.
	(elf_link_add_object_symbols): Call _bfd_elf_link_check_relocs
	if check_relocs_after_open_input is FALSE.

include/

	* bfdlink.h (bfd_link_info): Add check_relocs_after_open_input.

ld/

	* ldmisc.c (vfinfo): Add support for %x and %lx.
	* emulparams/elf32_x86_64.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	New.
	* emulparams/elf_i386.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_be.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_chaos.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_ldso.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_vxworks.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_x86_64.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/i386nto.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse):
	Set check_relocs_after_open_input to TRUE if
	CHECK_RELOCS_AFTER_OPEN_INPUT is yes.
	(gld${EMULATION_NAME}_after_open): Call
	_bfd_elf_link_check_relocs on all inputs if
	check_relocs_after_open_input is TRUE.
---
 bfd/elf-bfd.h                     |   2 +
 bfd/elf32-i386.c                  |  10 ++--
 bfd/elf64-x86-64.c                |   8 +--
 bfd/elflink.c                     | 117 +++++++++++++++++++++-----------------
 include/bfdlink.h                 |   4 ++
 ld/emulparams/elf32_x86_64.sh     |   1 +
 ld/emulparams/elf_i386.sh         |   1 +
 ld/emulparams/elf_i386_be.sh      |   1 +
 ld/emulparams/elf_i386_chaos.sh   |   1 +
 ld/emulparams/elf_i386_ldso.sh    |   1 +
 ld/emulparams/elf_i386_vxworks.sh |   1 +
 ld/emulparams/elf_x86_64.sh       |   1 +
 ld/emulparams/i386nto.sh          |   1 +
 ld/emultempl/elf32.em             |  11 ++++
 ld/ldmisc.c                       |  13 +++++
 15 files changed, 113 insertions(+), 60 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5c93d78..5dce70e 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2261,6 +2261,8 @@ extern bfd_boolean bfd_elf_link_add_symbols
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_add_dynamic_entry
   (struct bfd_link_info *, bfd_vma, bfd_vma);
+extern bfd_boolean _bfd_elf_link_check_relocs
+  (bfd *, struct bfd_link_info *);
 
 extern bfd_boolean bfd_elf_link_record_dynamic_symbol
   (struct bfd_link_info *, struct elf_link_hash_entry *);
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 470fcd1..d745a89 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1491,11 +1491,11 @@ elf_i386_tls_transition (struct bfd_link_info *info, bfd *abfd,
 	    }
 	}
 
-      (*_bfd_error_handler)
-	(_("%B: TLS transition from %s to %s against `%s' at 0x%lx "
-	   "in section `%A' failed"),
-	 abfd, sec, from->name, to->name, name,
-	 (unsigned long) rel->r_offset);
+      info->callbacks->einfo
+	(_("%B%X: TLS transition from %s to %s against `%s' at 0x%lx "
+	   "in section `%A' failed\n"),
+	 abfd, from->name, to->name, name,
+	 (unsigned long) rel->r_offset, sec);
       bfd_set_error (bfd_error_bad_value);
       return FALSE;
     }
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 5533b4a..14dc3a1 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1572,11 +1572,11 @@ elf_x86_64_tls_transition (struct bfd_link_info *info, bfd *abfd,
 	    }
 	}
 
-      (*_bfd_error_handler)
-	(_("%B: TLS transition from %s to %s against `%s' at 0x%lx "
+      info->callbacks->einfo
+	(_("%B%X: TLS transition from %s to %s against `%s' at 0x%lx "
 	   "in section `%A' failed"),
-	 abfd, sec, from->name, to->name, name,
-	 (unsigned long) rel->r_offset);
+	 abfd, from->name, to->name, name,
+	 (unsigned long) rel->r_offset, sec);
       bfd_set_error (bfd_error_bad_value);
       return FALSE;
     }
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 37638b2..5af334a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3480,6 +3480,69 @@ _bfd_elf_notice_as_needed (bfd *ibfd,
   return (*info->callbacks->notice) (info, NULL, NULL, ibfd, NULL, act, 0);
 }
 
+/* Check relocations an ELF object file.  */
+
+bfd_boolean
+_bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
+{
+  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+  struct elf_link_hash_table *htab = elf_hash_table (info);
+
+  /* If this object is the same format as the output object, and it is
+     not a shared library, then let the backend look through the
+     relocs.
+
+     This is required to build global offset table entries and to
+     arrange for dynamic relocs.  It is not required for the
+     particular common case of linking non PIC code, even when linking
+     against shared libraries, but unfortunately there is no way of
+     knowing whether an object file has been compiled PIC or not.
+     Looking through the relocs is not particularly time consuming.
+     The problem is that we must either (1) keep the relocs in memory,
+     which causes the linker to require additional runtime memory or
+     (2) read the relocs twice from the input file, which wastes time.
+     This would be a good case for using mmap.
+
+     I have no idea how to handle linking PIC code into a file of a
+     different format.  It probably can't be done.  */
+  if ((abfd->flags & DYNAMIC) == 0
+      && is_elf_hash_table (htab)
+      && bed->check_relocs != NULL
+      && elf_object_id (abfd) == elf_hash_table_id (htab)
+      && (*bed->relocs_compatible) (abfd->xvec, info->output_bfd->xvec))
+    {
+      asection *o;
+
+      for (o = abfd->sections; o != NULL; o = o->next)
+	{
+	  Elf_Internal_Rela *internal_relocs;
+	  bfd_boolean ok;
+
+	  if ((o->flags & SEC_RELOC) == 0
+	      || o->reloc_count == 0
+	      || ((info->strip == strip_all || info->strip == strip_debugger)
+		  && (o->flags & SEC_DEBUGGING) != 0)
+	      || bfd_is_abs_section (o->output_section))
+	    continue;
+
+	  internal_relocs = _bfd_elf_link_read_relocs (abfd, o, NULL, NULL,
+						       info->keep_memory);
+	  if (internal_relocs == NULL)
+	    return FALSE;
+
+	  ok = (*bed->check_relocs) (abfd, info, o, internal_relocs);
+
+	  if (elf_section_data (o)->relocs != internal_relocs)
+	    free (internal_relocs);
+
+	  if (! ok)
+	    return FALSE;
+	}
+    }
+
+  return TRUE;
+}
+
 /* Add symbols from an ELF object file to the linker hash table.  */
 
 static bfd_boolean
@@ -4950,57 +5013,9 @@ error_free_dyn:
       && !(*bed->check_directives) (abfd, info))
     return FALSE;
 
-  /* If this object is the same format as the output object, and it is
-     not a shared library, then let the backend look through the
-     relocs.
-
-     This is required to build global offset table entries and to
-     arrange for dynamic relocs.  It is not required for the
-     particular common case of linking non PIC code, even when linking
-     against shared libraries, but unfortunately there is no way of
-     knowing whether an object file has been compiled PIC or not.
-     Looking through the relocs is not particularly time consuming.
-     The problem is that we must either (1) keep the relocs in memory,
-     which causes the linker to require additional runtime memory or
-     (2) read the relocs twice from the input file, which wastes time.
-     This would be a good case for using mmap.
-
-     I have no idea how to handle linking PIC code into a file of a
-     different format.  It probably can't be done.  */
-  if (! dynamic
-      && is_elf_hash_table (htab)
-      && bed->check_relocs != NULL
-      && elf_object_id (abfd) == elf_hash_table_id (htab)
-      && (*bed->relocs_compatible) (abfd->xvec, info->output_bfd->xvec))
-    {
-      asection *o;
-
-      for (o = abfd->sections; o != NULL; o = o->next)
-	{
-	  Elf_Internal_Rela *internal_relocs;
-	  bfd_boolean ok;
-
-	  if ((o->flags & SEC_RELOC) == 0
-	      || o->reloc_count == 0
-	      || ((info->strip == strip_all || info->strip == strip_debugger)
-		  && (o->flags & SEC_DEBUGGING) != 0)
-	      || bfd_is_abs_section (o->output_section))
-	    continue;
-
-	  internal_relocs = _bfd_elf_link_read_relocs (abfd, o, NULL, NULL,
-						       info->keep_memory);
-	  if (internal_relocs == NULL)
-	    goto error_return;
-
-	  ok = (*bed->check_relocs) (abfd, info, o, internal_relocs);
-
-	  if (elf_section_data (o)->relocs != internal_relocs)
-	    free (internal_relocs);
-
-	  if (! ok)
-	    goto error_return;
-	}
-    }
+  if (!info->check_relocs_after_open_input
+      && !_bfd_elf_link_check_relocs (abfd, info))
+    return FALSE;
 
   /* If this is a non-traditional link, try to optimize the handling
      of the .stab/.stabstr sections.  */
diff --git a/include/bfdlink.h b/include/bfdlink.h
index a285f6d..90467b5 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -440,6 +440,10 @@ struct bfd_link_info
   /* TRUE if the linker script contained an explicit PHDRS command.  */
   unsigned int user_phdrs: 1;
 
+  /* TRUE if we should check relocations after all input files have
+     been opened.  */
+  unsigned int check_relocs_after_open_input: 1;
+
   /* TRUE if BND prefix in PLT entries is always generated.  */
   unsigned int bndplt: 1;
 
diff --git a/ld/emulparams/elf32_x86_64.sh b/ld/emulparams/elf32_x86_64.sh
index 967c1b4..9050730 100644
--- a/ld/emulparams/elf32_x86_64.sh
+++ b/ld/emulparams/elf32_x86_64.sh
@@ -6,6 +6,7 @@
 SCRIPT_NAME=elf
 ELFSIZE=32
 OUTPUT_FORMAT="elf32-x86-64"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_REL_RELOCS=yes
 TEXT_START_ADDR=0x400000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386.sh b/ld/emulparams/elf_i386.sh
index 3451bb2..b08e661 100644
--- a/ld/emulparams/elf_i386.sh
+++ b/ld/emulparams/elf_i386.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_be.sh b/ld/emulparams/elf_i386_be.sh
index 70db443..4a24b02 100644
--- a/ld/emulparams/elf_i386_be.sh
+++ b/ld/emulparams/elf_i386_be.sh
@@ -3,6 +3,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x80000000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_chaos.sh b/ld/emulparams/elf_i386_chaos.sh
index aa36cb5..5349108 100644
--- a/ld/emulparams/elf_i386_chaos.sh
+++ b/ld/emulparams/elf_i386_chaos.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf_chaos
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 TEXT_START_ADDR=0x40000000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
 ARCH=i386
diff --git a/ld/emulparams/elf_i386_ldso.sh b/ld/emulparams/elf_i386_ldso.sh
index 1328520..dc4eef4 100644
--- a/ld/emulparams/elf_i386_ldso.sh
+++ b/ld/emulparams/elf_i386_ldso.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_vxworks.sh b/ld/emulparams/elf_i386_vxworks.sh
index aaea8c4..ac1bbeb 100644
--- a/ld/emulparams/elf_i386_vxworks.sh
+++ b/ld/emulparams/elf_i386_vxworks.sh
@@ -1,5 +1,6 @@
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386-vxworks"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_x86_64.sh b/ld/emulparams/elf_x86_64.sh
index e935f90..6055204 100644
--- a/ld/emulparams/elf_x86_64.sh
+++ b/ld/emulparams/elf_x86_64.sh
@@ -6,6 +6,7 @@
 SCRIPT_NAME=elf
 ELFSIZE=64
 OUTPUT_FORMAT="elf64-x86-64"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_REL_RELOCS=yes
 TEXT_START_ADDR=0x400000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/i386nto.sh b/ld/emulparams/i386nto.sh
index 626f9c1..51284be 100644
--- a/ld/emulparams/i386nto.sh
+++ b/ld/emulparams/i386nto.sh
@@ -1,5 +1,6 @@
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 TEXT_START_SYMBOLS='_btext = .;'
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 3e9f684..1cfe65c 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -104,6 +104,7 @@ gld${EMULATION_NAME}_before_parse (void)
   config.has_shared = `if test -n "$GENERATE_SHLIB_SCRIPT" ; then echo TRUE ; else echo FALSE ; fi`;
   config.separate_code = `if test "x${SEPARATE_CODE}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
   `if test -n "$CALL_NOP_BYTE" ; then echo link_info.call_nop_byte = $CALL_NOP_BYTE; fi`;
+  link_info.check_relocs_after_open_input = `if test "x${CHECK_RELOCS_AFTER_OPEN_INPUT}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
 }
 
 EOF
@@ -1025,6 +1026,16 @@ gld${EMULATION_NAME}_after_open (void)
   if (!is_elf_hash_table (htab))
     return;
 
+  if (link_info.check_relocs_after_open_input)
+    {
+      bfd *abfd;
+
+      for (abfd = link_info.input_bfds;
+	   abfd != (bfd *) NULL; abfd = abfd->link.next)
+	if (!_bfd_elf_link_check_relocs (abfd, &link_info))
+	  return;
+    }
+
   if (emit_note_gnu_build_id != NULL)
     {
       bfd *abfd;
diff --git a/ld/ldmisc.c b/ld/ldmisc.c
index 70e12ea..a744586 100644
--- a/ld/ldmisc.c
+++ b/ld/ldmisc.c
@@ -58,6 +58,8 @@
  %d integer, like printf
  %ld long, like printf
  %lu unsigned long, like printf
+ %x integer in hex, like printf
+ %lx long in hex, like printf
  %p native (host) void* pointer, like printf
  %s arbitrary string, like printf
  %u integer, like printf
@@ -409,6 +411,11 @@ vfinfo (FILE *fp, const char *fmt, va_list arg, bfd_boolean is_warning)
 	      fprintf (fp, "%u", va_arg (arg, unsigned int));
 	      break;
 
+	    case 'x':
+	      /* integer in hex, like printf */
+	      fprintf (fp, "%x", va_arg (arg, int));
+	      break;
+
 	    case 'l':
 	      if (*fmt == 'd')
 		{
@@ -422,6 +429,12 @@ vfinfo (FILE *fp, const char *fmt, va_list arg, bfd_boolean is_warning)
 		  ++fmt;
 		  break;
 		}
+	      else if (*fmt == 'x')
+		{
+		  fprintf (fp, "%lx", va_arg (arg, long));
+		  ++fmt;
+		  break;
+		}
 	      /* Fall thru */
 
 	    default:
-- 
2.5.5

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 13:50 [RFC][PATCH] Check ELF relocs after opening all all input files H.J. Lu
@ 2016-04-19 16:01 ` Nick Clifton
  2016-04-19 18:03   ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Clifton @ 2016-04-19 16:01 UTC (permalink / raw)
  To: H.J. Lu, binutils

Hi H.J.

> Delaying checking ELF relocations until opening all input files so
> that symbol information is final when relocations are checked.  This
> is only enabled for x86 targets.

> Any comments, feedbacks?

What benefit is gained by doing this ?  I would guess that it is connected
with symbols changing type and/or visibility and/or protected status, but
it would be nice to know what you are hoping to achieve.

The code seems quite straightforward - but why limit the change to x86 
targets only ?  Are you expecting that it might break other targets ?

I assume that you have tested the patch with the x86 variants that you 
have changed - were there any regressions or changes ?

Would it be worth creating a test the demonstrates the value of the change.
If the test was not restricted to x86 targets then it could be used to
prompt a conversion of non-x86 targets as well.

Cheers
  Nick

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 16:01 ` Nick Clifton
@ 2016-04-19 18:03   ` H.J. Lu
  2016-04-19 23:25     ` Alan Modra
  2016-04-20  9:28     ` Nick Clifton
  0 siblings, 2 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-19 18:03 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Binutils

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

On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi H.J.
>
>> Delaying checking ELF relocations until opening all input files so
>> that symbol information is final when relocations are checked.  This
>> is only enabled for x86 targets.
>
>> Any comments, feedbacks?
>
> What benefit is gained by doing this ?  I would guess that it is connected
> with symbols changing type and/or visibility and/or protected status, but
> it would be nice to know what you are hoping to achieve.

Yes,  this change is motivated by that discussion.  Currently backend
check_relocs doesn't have the final information on symbols.  A better
linker can provide better diagnostics in check_relocs if symbol info is
final.

> The code seems quite straightforward - but why limit the change to x86
> targets only ?  Are you expecting that it might break other targets ?

I can't test non-x86 targets.  The problem may only show up on
native build.

> I assume that you have tested the patch with the x86 variants that you
> have changed - were there any regressions or changes ?

None.

> Would it be worth creating a test the demonstrates the value of the change.
> If the test was not restricted to x86 targets then it could be used to
> prompt a conversion of non-x86 targets as well.

I am working on it.

Here is a simpler patch.

-- 
H.J.

[-- Attachment #2: 0001-Check-ELF-relocs-after-opening-all-input-files.patch --]
[-- Type: text/x-patch, Size: 12228 bytes --]

From fbb62d88d8a4b499a949b40248a96771a9d2f6ef Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 19 Apr 2016 06:35:23 -0700
Subject: [PATCH] Check ELF relocs after opening all input files

Delaying checking ELF relocations until opening all input files so
that symbol information is final when relocations are checked.  This
is only enabled for x86 targets.

bfd/

	* elf-bfd.h (_bfd_elf_link_check_relocs): New.
	* elflink.c (_bfd_elf_link_check_relocs): New function.
	(elf_link_add_object_symbols): Call _bfd_elf_link_check_relocs
	if check_relocs_after_open_input is FALSE.

include/

	* bfdlink.h (bfd_link_info): Add check_relocs_after_open_input.

ld/

	* emulparams/elf32_x86_64.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	New.
	* emulparams/elf_i386.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_be.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_chaos.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_ldso.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_i386_vxworks.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/elf_x86_64.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emulparams/i386nto.sh (CHECK_RELOCS_AFTER_OPEN_INPUT):
	Likewise.
	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse):
	Set check_relocs_after_open_input to TRUE if
	CHECK_RELOCS_AFTER_OPEN_INPUT is yes.
	(gld${EMULATION_NAME}_after_open): Call
	_bfd_elf_link_check_relocs on all inputs if
	check_relocs_after_open_input is TRUE.
---
 bfd/elf-bfd.h                     |   2 +
 bfd/elflink.c                     | 117 +++++++++++++++++++++-----------------
 include/bfdlink.h                 |   4 ++
 ld/emulparams/elf32_x86_64.sh     |   1 +
 ld/emulparams/elf_i386.sh         |   1 +
 ld/emulparams/elf_i386_be.sh      |   1 +
 ld/emulparams/elf_i386_chaos.sh   |   1 +
 ld/emulparams/elf_i386_ldso.sh    |   1 +
 ld/emulparams/elf_i386_vxworks.sh |   1 +
 ld/emulparams/elf_x86_64.sh       |   1 +
 ld/emulparams/i386nto.sh          |   1 +
 ld/emultempl/elf32.em             |  15 +++++
 12 files changed, 95 insertions(+), 51 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5c93d78..5dce70e 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2261,6 +2261,8 @@ extern bfd_boolean bfd_elf_link_add_symbols
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_add_dynamic_entry
   (struct bfd_link_info *, bfd_vma, bfd_vma);
+extern bfd_boolean _bfd_elf_link_check_relocs
+  (bfd *, struct bfd_link_info *);
 
 extern bfd_boolean bfd_elf_link_record_dynamic_symbol
   (struct bfd_link_info *, struct elf_link_hash_entry *);
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 37638b2..5af334a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3480,6 +3480,69 @@ _bfd_elf_notice_as_needed (bfd *ibfd,
   return (*info->callbacks->notice) (info, NULL, NULL, ibfd, NULL, act, 0);
 }
 
+/* Check relocations an ELF object file.  */
+
+bfd_boolean
+_bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
+{
+  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+  struct elf_link_hash_table *htab = elf_hash_table (info);
+
+  /* If this object is the same format as the output object, and it is
+     not a shared library, then let the backend look through the
+     relocs.
+
+     This is required to build global offset table entries and to
+     arrange for dynamic relocs.  It is not required for the
+     particular common case of linking non PIC code, even when linking
+     against shared libraries, but unfortunately there is no way of
+     knowing whether an object file has been compiled PIC or not.
+     Looking through the relocs is not particularly time consuming.
+     The problem is that we must either (1) keep the relocs in memory,
+     which causes the linker to require additional runtime memory or
+     (2) read the relocs twice from the input file, which wastes time.
+     This would be a good case for using mmap.
+
+     I have no idea how to handle linking PIC code into a file of a
+     different format.  It probably can't be done.  */
+  if ((abfd->flags & DYNAMIC) == 0
+      && is_elf_hash_table (htab)
+      && bed->check_relocs != NULL
+      && elf_object_id (abfd) == elf_hash_table_id (htab)
+      && (*bed->relocs_compatible) (abfd->xvec, info->output_bfd->xvec))
+    {
+      asection *o;
+
+      for (o = abfd->sections; o != NULL; o = o->next)
+	{
+	  Elf_Internal_Rela *internal_relocs;
+	  bfd_boolean ok;
+
+	  if ((o->flags & SEC_RELOC) == 0
+	      || o->reloc_count == 0
+	      || ((info->strip == strip_all || info->strip == strip_debugger)
+		  && (o->flags & SEC_DEBUGGING) != 0)
+	      || bfd_is_abs_section (o->output_section))
+	    continue;
+
+	  internal_relocs = _bfd_elf_link_read_relocs (abfd, o, NULL, NULL,
+						       info->keep_memory);
+	  if (internal_relocs == NULL)
+	    return FALSE;
+
+	  ok = (*bed->check_relocs) (abfd, info, o, internal_relocs);
+
+	  if (elf_section_data (o)->relocs != internal_relocs)
+	    free (internal_relocs);
+
+	  if (! ok)
+	    return FALSE;
+	}
+    }
+
+  return TRUE;
+}
+
 /* Add symbols from an ELF object file to the linker hash table.  */
 
 static bfd_boolean
@@ -4950,57 +5013,9 @@ error_free_dyn:
       && !(*bed->check_directives) (abfd, info))
     return FALSE;
 
-  /* If this object is the same format as the output object, and it is
-     not a shared library, then let the backend look through the
-     relocs.
-
-     This is required to build global offset table entries and to
-     arrange for dynamic relocs.  It is not required for the
-     particular common case of linking non PIC code, even when linking
-     against shared libraries, but unfortunately there is no way of
-     knowing whether an object file has been compiled PIC or not.
-     Looking through the relocs is not particularly time consuming.
-     The problem is that we must either (1) keep the relocs in memory,
-     which causes the linker to require additional runtime memory or
-     (2) read the relocs twice from the input file, which wastes time.
-     This would be a good case for using mmap.
-
-     I have no idea how to handle linking PIC code into a file of a
-     different format.  It probably can't be done.  */
-  if (! dynamic
-      && is_elf_hash_table (htab)
-      && bed->check_relocs != NULL
-      && elf_object_id (abfd) == elf_hash_table_id (htab)
-      && (*bed->relocs_compatible) (abfd->xvec, info->output_bfd->xvec))
-    {
-      asection *o;
-
-      for (o = abfd->sections; o != NULL; o = o->next)
-	{
-	  Elf_Internal_Rela *internal_relocs;
-	  bfd_boolean ok;
-
-	  if ((o->flags & SEC_RELOC) == 0
-	      || o->reloc_count == 0
-	      || ((info->strip == strip_all || info->strip == strip_debugger)
-		  && (o->flags & SEC_DEBUGGING) != 0)
-	      || bfd_is_abs_section (o->output_section))
-	    continue;
-
-	  internal_relocs = _bfd_elf_link_read_relocs (abfd, o, NULL, NULL,
-						       info->keep_memory);
-	  if (internal_relocs == NULL)
-	    goto error_return;
-
-	  ok = (*bed->check_relocs) (abfd, info, o, internal_relocs);
-
-	  if (elf_section_data (o)->relocs != internal_relocs)
-	    free (internal_relocs);
-
-	  if (! ok)
-	    goto error_return;
-	}
-    }
+  if (!info->check_relocs_after_open_input
+      && !_bfd_elf_link_check_relocs (abfd, info))
+    return FALSE;
 
   /* If this is a non-traditional link, try to optimize the handling
      of the .stab/.stabstr sections.  */
diff --git a/include/bfdlink.h b/include/bfdlink.h
index a285f6d..90467b5 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -440,6 +440,10 @@ struct bfd_link_info
   /* TRUE if the linker script contained an explicit PHDRS command.  */
   unsigned int user_phdrs: 1;
 
+  /* TRUE if we should check relocations after all input files have
+     been opened.  */
+  unsigned int check_relocs_after_open_input: 1;
+
   /* TRUE if BND prefix in PLT entries is always generated.  */
   unsigned int bndplt: 1;
 
diff --git a/ld/emulparams/elf32_x86_64.sh b/ld/emulparams/elf32_x86_64.sh
index 967c1b4..9050730 100644
--- a/ld/emulparams/elf32_x86_64.sh
+++ b/ld/emulparams/elf32_x86_64.sh
@@ -6,6 +6,7 @@
 SCRIPT_NAME=elf
 ELFSIZE=32
 OUTPUT_FORMAT="elf32-x86-64"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_REL_RELOCS=yes
 TEXT_START_ADDR=0x400000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386.sh b/ld/emulparams/elf_i386.sh
index 3451bb2..b08e661 100644
--- a/ld/emulparams/elf_i386.sh
+++ b/ld/emulparams/elf_i386.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_be.sh b/ld/emulparams/elf_i386_be.sh
index 70db443..4a24b02 100644
--- a/ld/emulparams/elf_i386_be.sh
+++ b/ld/emulparams/elf_i386_be.sh
@@ -3,6 +3,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x80000000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_chaos.sh b/ld/emulparams/elf_i386_chaos.sh
index aa36cb5..5349108 100644
--- a/ld/emulparams/elf_i386_chaos.sh
+++ b/ld/emulparams/elf_i386_chaos.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf_chaos
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 TEXT_START_ADDR=0x40000000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
 ARCH=i386
diff --git a/ld/emulparams/elf_i386_ldso.sh b/ld/emulparams/elf_i386_ldso.sh
index 1328520..dc4eef4 100644
--- a/ld/emulparams/elf_i386_ldso.sh
+++ b/ld/emulparams/elf_i386_ldso.sh
@@ -4,6 +4,7 @@
 . ${srcdir}/emulparams/call_nop.sh
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_i386_vxworks.sh b/ld/emulparams/elf_i386_vxworks.sh
index aaea8c4..ac1bbeb 100644
--- a/ld/emulparams/elf_i386_vxworks.sh
+++ b/ld/emulparams/elf_i386_vxworks.sh
@@ -1,5 +1,6 @@
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386-vxworks"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/elf_x86_64.sh b/ld/emulparams/elf_x86_64.sh
index e935f90..6055204 100644
--- a/ld/emulparams/elf_x86_64.sh
+++ b/ld/emulparams/elf_x86_64.sh
@@ -6,6 +6,7 @@
 SCRIPT_NAME=elf
 ELFSIZE=64
 OUTPUT_FORMAT="elf64-x86-64"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_REL_RELOCS=yes
 TEXT_START_ADDR=0x400000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
diff --git a/ld/emulparams/i386nto.sh b/ld/emulparams/i386nto.sh
index 626f9c1..51284be 100644
--- a/ld/emulparams/i386nto.sh
+++ b/ld/emulparams/i386nto.sh
@@ -1,5 +1,6 @@
 SCRIPT_NAME=elf
 OUTPUT_FORMAT="elf32-i386"
+CHECK_RELOCS_AFTER_OPEN_INPUT=yes
 NO_RELA_RELOCS=yes
 TEXT_START_ADDR=0x08048000
 TEXT_START_SYMBOLS='_btext = .;'
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 3e9f684..312f935 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -104,6 +104,7 @@ gld${EMULATION_NAME}_before_parse (void)
   config.has_shared = `if test -n "$GENERATE_SHLIB_SCRIPT" ; then echo TRUE ; else echo FALSE ; fi`;
   config.separate_code = `if test "x${SEPARATE_CODE}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
   `if test -n "$CALL_NOP_BYTE" ; then echo link_info.call_nop_byte = $CALL_NOP_BYTE; fi`;
+  link_info.check_relocs_after_open_input = `if test "x${CHECK_RELOCS_AFTER_OPEN_INPUT}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
 }
 
 EOF
@@ -1025,6 +1026,20 @@ gld${EMULATION_NAME}_after_open (void)
   if (!is_elf_hash_table (htab))
     return;
 
+  if (link_info.check_relocs_after_open_input)
+    {
+      bfd *abfd;
+
+      for (abfd = link_info.input_bfds;
+	   abfd != (bfd *) NULL; abfd = abfd->link.next)
+	if (!_bfd_elf_link_check_relocs (abfd, &link_info))
+	  {
+	    /* no object output, fail return */
+	    config.make_executable = FALSE;
+	    return;
+	  }
+    }
+
   if (emit_note_gnu_build_id != NULL)
     {
       bfd *abfd;
-- 
2.5.5


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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 18:03   ` H.J. Lu
@ 2016-04-19 23:25     ` Alan Modra
  2016-04-19 23:47       ` H.J. Lu
                         ` (2 more replies)
  2016-04-20  9:28     ` Nick Clifton
  1 sibling, 3 replies; 25+ messages in thread
From: Alan Modra @ 2016-04-19 23:25 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Tue, Apr 19, 2016 at 11:03:15AM -0700, H.J. Lu wrote:
> On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
> > Hi H.J.
> >
> >> Delaying checking ELF relocations until opening all input files so
> >> that symbol information is final when relocations are checked.  This
> >> is only enabled for x86 targets.
> >
> >> Any comments, feedbacks?
> >
> > What benefit is gained by doing this ?  I would guess that it is connected
> > with symbols changing type and/or visibility and/or protected status, but
> > it would be nice to know what you are hoping to achieve.
> 
> Yes,  this change is motivated by that discussion.  Currently backend
> check_relocs doesn't have the final information on symbols.  A better
> linker can provide better diagnostics in check_relocs if symbol info is
> final.

There are other benefits too.  As it is the backend check_relocs
functions do things like "maybe this symbol remains undefined so
perhaps it needs a plt entry", and "maybe this symbol remains dynamic
so perhaps it needs dynamic relocations".  This complexity can
disappear if all of those "maybe" cases are resolved.

Now, if you run check_relocs even later, say at
ldemul_before_allocation, after gc-sections has run, then all the
reference counting code for plt and got relocs can be removed too.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 23:25     ` Alan Modra
@ 2016-04-19 23:47       ` H.J. Lu
  2016-04-20 12:23         ` H.J. Lu
  2016-04-20 18:34         ` H.J. Lu
  2016-04-20  0:14       ` Rich Felker
  2016-04-21  0:39       ` H.J. Lu
  2 siblings, 2 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-19 23:47 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Tue, Apr 19, 2016 at 4:24 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Apr 19, 2016 at 11:03:15AM -0700, H.J. Lu wrote:
>> On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
>> > Hi H.J.
>> >
>> >> Delaying checking ELF relocations until opening all input files so
>> >> that symbol information is final when relocations are checked.  This
>> >> is only enabled for x86 targets.
>> >
>> >> Any comments, feedbacks?
>> >
>> > What benefit is gained by doing this ?  I would guess that it is connected
>> > with symbols changing type and/or visibility and/or protected status, but
>> > it would be nice to know what you are hoping to achieve.
>>
>> Yes,  this change is motivated by that discussion.  Currently backend
>> check_relocs doesn't have the final information on symbols.  A better
>> linker can provide better diagnostics in check_relocs if symbol info is
>> final.
>
> There are other benefits too.  As it is the backend check_relocs
> functions do things like "maybe this symbol remains undefined so
> perhaps it needs a plt entry", and "maybe this symbol remains dynamic
> so perhaps it needs dynamic relocations".  This complexity can
> disappear if all of those "maybe" cases are resolved.

Yes,  that is the plan.

> Now, if you run check_relocs even later, say at
> ldemul_before_allocation, after gc-sections has run, then all the
> reference counting code for plt and got relocs can be removed too.
>

That is on my to-do list.  One step at a time :-).   We can start
with x86 and enable it for other after we have worked out all the issues

-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 23:25     ` Alan Modra
  2016-04-19 23:47       ` H.J. Lu
@ 2016-04-20  0:14       ` Rich Felker
  2016-04-21  0:39       ` H.J. Lu
  2 siblings, 0 replies; 25+ messages in thread
From: Rich Felker @ 2016-04-20  0:14 UTC (permalink / raw)
  To: Alan Modra; +Cc: H.J. Lu, Nick Clifton, Binutils

On Wed, Apr 20, 2016 at 08:54:57AM +0930, Alan Modra wrote:
> On Tue, Apr 19, 2016 at 11:03:15AM -0700, H.J. Lu wrote:
> > On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
> > > Hi H.J.
> > >
> > >> Delaying checking ELF relocations until opening all input files so
> > >> that symbol information is final when relocations are checked.  This
> > >> is only enabled for x86 targets.
> > >
> > >> Any comments, feedbacks?
> > >
> > > What benefit is gained by doing this ?  I would guess that it is connected
> > > with symbols changing type and/or visibility and/or protected status, but
> > > it would be nice to know what you are hoping to achieve.
> > 
> > Yes,  this change is motivated by that discussion.  Currently backend
> > check_relocs doesn't have the final information on symbols.  A better
> > linker can provide better diagnostics in check_relocs if symbol info is
> > final.
> 
> There are other benefits too.  As it is the backend check_relocs
> functions do things like "maybe this symbol remains undefined so
> perhaps it needs a plt entry", and "maybe this symbol remains dynamic
> so perhaps it needs dynamic relocations".  This complexity can
> disappear if all of those "maybe" cases are resolved.
> 
> Now, if you run check_relocs even later, say at
> ldemul_before_allocation, after gc-sections has run, then all the
> reference counting code for plt and got relocs can be removed too.

Sounds nice! This would probably have avoided bug #19516 ever
happening:

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

Rich

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 18:03   ` H.J. Lu
  2016-04-19 23:25     ` Alan Modra
@ 2016-04-20  9:28     ` Nick Clifton
  1 sibling, 0 replies; 25+ messages in thread
From: Nick Clifton @ 2016-04-20  9:28 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

Hi H.J.

>> What benefit is gained by doing this ?  I would guess that it is connected
>> with symbols changing type and/or visibility and/or protected status, but
>> it would be nice to know what you are hoping to achieve.
> 
> Yes,  this change is motivated by that discussion.  Currently backend
> check_relocs doesn't have the final information on symbols.  A better
> linker can provide better diagnostics in check_relocs if symbol info is
> final.

Thanks for the clarification.

> Here is a simpler patch.

Well I for one like the patch, so please consider it approved.

Cheers
  Nick

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 23:47       ` H.J. Lu
@ 2016-04-20 12:23         ` H.J. Lu
  2016-04-20 13:55           ` Alan Modra
  2016-04-20 18:34         ` H.J. Lu
  1 sibling, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-20 12:23 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Tue, Apr 19, 2016 at 4:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> Now, if you run check_relocs even later, say at
>> ldemul_before_allocation, after gc-sections has run, then all the
>> reference counting code for plt and got relocs can be removed too.
>>
>
> That is on my to-do list.  One step at a time :-).   We can start
> with x86 and enable it for other after we have worked out all the issues
>

ldemul_before_allocation is too late.  It should be called before
map_input_to_output_sections.  I will check in my current patch
for now and change it to right after gc-sections later.

-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-20 12:23         ` H.J. Lu
@ 2016-04-20 13:55           ` Alan Modra
  2016-04-20 14:25             ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Alan Modra @ 2016-04-20 13:55 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Wed, Apr 20, 2016 at 05:23:11AM -0700, H.J. Lu wrote:
> ldemul_before_allocation is too late.  It should be called before
> map_input_to_output_sections.

Oh yeah, check_relocs creates sections, that need to be mapped.  Are
there any other reasons?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-20 13:55           ` Alan Modra
@ 2016-04-20 14:25             ` H.J. Lu
  0 siblings, 0 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-20 14:25 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Wed, Apr 20, 2016 at 6:55 AM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Apr 20, 2016 at 05:23:11AM -0700, H.J. Lu wrote:
>> ldemul_before_allocation is too late.  It should be called before
>> map_input_to_output_sections.
>
> Oh yeah, check_relocs creates sections, that need to be mapped.  Are
> there any other reasons?

Only one I found so far.  I am in the process to remove refcount
from x86 backend.


-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 23:47       ` H.J. Lu
  2016-04-20 12:23         ` H.J. Lu
@ 2016-04-20 18:34         ` H.J. Lu
  2016-04-20 23:32           ` Alan Modra
  2016-04-21 10:33           ` Nick Clifton
  1 sibling, 2 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-20 18:34 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

On Tue, Apr 19, 2016 at 4:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Now, if you run check_relocs even later, say at
>> ldemul_before_allocation, after gc-sections has run, then all the
>> reference counting code for plt and got relocs can be removed too.
>>
>
> That is on my to-do list.  One step at a time :-).   We can start
> with x86 and enable it for other after we have worked out all the issues
>

Here is the first step.  OK for master?


-- 
H.J.

[-- Attachment #2: 0001-Move-ELF-relocation-check-after-lang_gc_sections.patch --]
[-- Type: text/x-patch, Size: 2440 bytes --]

From 9b5c177dd8957627b028c0bb52d09fef3155a093 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 20 Apr 2016 05:23:45 -0700
Subject: [PATCH] Move ELF relocation check after lang_gc_sections

Move ELF relocation check after lang_gc_sections so that all the
reference counting code for plt and got relocs can be removed.  This
only affects ELF targets which check relocations after opening all
input file.

	* ldlang.c (lang_check_relocs): New function.
	(lang_process): Call lang_check_relocs after lang_gc_sections.
	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Don't
	call _bfd_elf_link_check_relocs here.
---
 ld/emultempl/elf32.em | 14 --------------
 ld/ldlang.c           | 24 ++++++++++++++++++++++++
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 312f935..4f5d1a4 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1026,20 +1026,6 @@ gld${EMULATION_NAME}_after_open (void)
   if (!is_elf_hash_table (htab))
     return;
 
-  if (link_info.check_relocs_after_open_input)
-    {
-      bfd *abfd;
-
-      for (abfd = link_info.input_bfds;
-	   abfd != (bfd *) NULL; abfd = abfd->link.next)
-	if (!_bfd_elf_link_check_relocs (abfd, &link_info))
-	  {
-	    /* no object output, fail return */
-	    config.make_executable = FALSE;
-	    return;
-	  }
-    }
-
   if (emit_note_gnu_build_id != NULL)
     {
       bfd *abfd;
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 856e3e2..2ae3640 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6778,6 +6778,27 @@ lang_add_gc_name (const char * name)
   link_info.gc_sym_list = sym;
 }
 
+/* Check relocations.  */
+
+static void
+lang_check_relocs (void)
+{
+  if (link_info.check_relocs_after_open_input
+      && bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour)
+    {
+      bfd *abfd;
+
+      for (abfd = link_info.input_bfds;
+	   abfd != (bfd *) NULL; abfd = abfd->link.next)
+	if (!_bfd_elf_link_check_relocs (abfd, &link_info))
+	  {
+	    /* no object output, fail return */
+	    config.make_executable = FALSE;
+	    break;
+	  }
+    }
+}
+
 void
 lang_process (void)
 {
@@ -6917,6 +6938,9 @@ lang_process (void)
   /* Remove unreferenced sections if asked to.  */
   lang_gc_sections ();
 
+  /* Check relocations.  */
+  lang_check_relocs ();
+
   /* Update wild statements.  */
   update_wild_statements (statement_list.head);
 
-- 
2.5.5


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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-20 18:34         ` H.J. Lu
@ 2016-04-20 23:32           ` Alan Modra
  2016-04-21 10:33           ` Nick Clifton
  1 sibling, 0 replies; 25+ messages in thread
From: Alan Modra @ 2016-04-20 23:32 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Wed, Apr 20, 2016 at 11:34:07AM -0700, H.J. Lu wrote:
> 	* ldlang.c (lang_check_relocs): New function.
> 	(lang_process): Call lang_check_relocs after lang_gc_sections.
> 	* emultempl/elf32.em (gld${EMULATION_NAME}_before_parse): Don't
> 	call _bfd_elf_link_check_relocs here.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-19 23:25     ` Alan Modra
  2016-04-19 23:47       ` H.J. Lu
  2016-04-20  0:14       ` Rich Felker
@ 2016-04-21  0:39       ` H.J. Lu
  2016-04-26 14:25         ` H.J. Lu
  2 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-21  0:39 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Tue, Apr 19, 2016 at 4:24 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Apr 19, 2016 at 11:03:15AM -0700, H.J. Lu wrote:
>> On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
>> > Hi H.J.
>> >
>> >> Delaying checking ELF relocations until opening all input files so
>> >> that symbol information is final when relocations are checked.  This
>> >> is only enabled for x86 targets.
>> >
>> >> Any comments, feedbacks?
>> >
>> > What benefit is gained by doing this ?  I would guess that it is connected
>> > with symbols changing type and/or visibility and/or protected status, but
>> > it would be nice to know what you are hoping to achieve.
>>
>> Yes,  this change is motivated by that discussion.  Currently backend
>> check_relocs doesn't have the final information on symbols.  A better
>> linker can provide better diagnostics in check_relocs if symbol info is
>> final.
>
> There are other benefits too.  As it is the backend check_relocs
> functions do things like "maybe this symbol remains undefined so
> perhaps it needs a plt entry", and "maybe this symbol remains dynamic
> so perhaps it needs dynamic relocations".  This complexity can
> disappear if all of those "maybe" cases are resolved.

The first phase of x86 backend is done.  I will take a look to see if I can
remove some checks in x86 check_relocs when I have time.

> Now, if you run check_relocs even later, say at
> ldemul_before_allocation, after gc-sections has run, then all the
> reference counting code for plt and got relocs can be removed too.

I  checked in:

commit e66cdd681f47dc51beaeee3d813f1c9cba27dedf
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Apr 20 17:12:46 2016 -0700

    Remove x86 gc_sweep_hook

    Since x86 backends never see the removed sections, there is no need
    for gc_sweep_hook.

      * elf32-i386.c (elf_i386_gc_sweep_hook): Removed.
      (elf_backend_gc_sweep_hook): Likewise.
      * elf64-x86-64.c (elf_x86_64_gc_sweep_hook): Likewise.
      (elf_backend_gc_sweep_hook): Likewise.

Other reference counting code for plt and got relocs in x86 backend
has to stay for TLS optimization and GOT/PLT optimization.

-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-20 18:34         ` H.J. Lu
  2016-04-20 23:32           ` Alan Modra
@ 2016-04-21 10:33           ` Nick Clifton
  2016-04-21 10:36             ` Nick Clifton
  1 sibling, 1 reply; 25+ messages in thread
From: Nick Clifton @ 2016-04-21 10:33 UTC (permalink / raw)
  To: H.J. Lu, Alan Modra; +Cc: Binutils

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

Hi H.J.

> Here is the first step.  OK for master?

I found a problem with this patch: non-ELF targets fail to build because
ldlang.c:lang_check_relocs() calls _bfd_elf_link_check_relocs() which is
not present in the BFD library.

Thinking about this it seems to me that we ought to allow non-ELF targets
a chance to check the relocs as well.  So what do you think of the attached
patch ?  It adds a new function pointer to the bfd_target structure which 
can be used to checks relocs.  For all non-ELF targets this vector is
initialised to a stub function which just returns true, but this can be
changed at a later date.  The patch also introduces a new, externally 
visible function called bfd_link_check_relocs() which will then call the
target function, and it changes lang_check_relocs() to use this new 
function.

Cheers
  Nick

[-- Attachment #2: elf_check_relocs.patch --]
[-- Type: text/x-patch, Size: 22518 bytes --]

diff --git a/bfd/aout-adobe.c b/bfd/aout-adobe.c
index 9556c89..57fdce7 100644
--- a/bfd/aout-adobe.c
+++ b/bfd/aout-adobe.c
@@ -472,6 +472,7 @@ aout_adobe_sizeof_headers (bfd *ignore_abfd ATTRIBUTE_UNUSED,
   _bfd_generic_copy_link_hash_symbol_type
 #define aout_32_bfd_final_link		            _bfd_generic_final_link
 #define aout_32_bfd_link_split_section	            _bfd_generic_link_split_section
+#define aout_32_bfd_link_check_relocs               _bfd_generic_link_check_relocs
 
 const bfd_target aout_adobe_vec =
 {
diff --git a/bfd/aout-target.h b/bfd/aout-target.h
index 7d96260..c7f5b1b 100644
--- a/bfd/aout-target.h
+++ b/bfd/aout-target.h
@@ -542,6 +542,10 @@ MY_bfd_final_link (bfd *abfd, struct bfd_link_info *info)
 #define MY_bfd_link_split_section  _bfd_generic_link_split_section
 #endif
 
+#ifndef MY_bfd_link_check_relocs
+#define MY_bfd_link_check_relocs   _bfd_generic_link_check_relocs
+#endif
+
 #ifndef MY_bfd_copy_private_bfd_data
 #define MY_bfd_copy_private_bfd_data _bfd_generic_bfd_copy_private_bfd_data
 #endif
diff --git a/bfd/aout-tic30.c b/bfd/aout-tic30.c
index f73b7e4..468944c 100644
--- a/bfd/aout-tic30.c
+++ b/bfd/aout-tic30.c
@@ -997,6 +997,10 @@ tic30_aout_set_arch_mach (bfd *abfd,
 #define MY_bfd_link_split_section  _bfd_generic_link_split_section
 #endif
 
+#ifndef MY_bfd_link_check_relocs
+#define MY_bfd_link_check_relocs   _bfd_generic_link_check_relocs
+#endif
+
 #ifndef MY_bfd_copy_private_bfd_data
 #define MY_bfd_copy_private_bfd_data _bfd_generic_bfd_copy_private_bfd_data
 #endif
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 6532f6e..a19a651 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7413,6 +7413,7 @@ typedef struct bfd_target
   NAME##_bfd_copy_link_hash_symbol_type, \
   NAME##_bfd_final_link, \
   NAME##_bfd_link_split_section, \
+  NAME##_bfd_link_check_relocs, \
   NAME##_bfd_gc_sections, \
   NAME##_bfd_lookup_section_flags, \
   NAME##_bfd_merge_sections, \
@@ -7454,6 +7455,9 @@ typedef struct bfd_target
   /* Should this section be split up into smaller pieces during linking.  */
   bfd_boolean (*_bfd_link_split_section) (bfd *, struct bfd_section *);
 
+  /* Check the relocations in the bfd for validity.  */
+  bfd_boolean (* _bfd_link_check_relocs)(bfd *, struct bfd_link_info *);
+
   /* Remove sections that are not referenced from the output.  */
   bfd_boolean (*_bfd_gc_sections) (bfd *, struct bfd_link_info *);
 
@@ -7566,6 +7570,12 @@ struct bfd_elf_version_tree * bfd_find_version_for_sym
 bfd_boolean bfd_hide_sym_by_version
    (struct bfd_elf_version_tree *verdefs, const char *sym_name);
 
+bfd_boolean bfd_link_check_relocs
+   (bfd *abfd, struct bfd_link_info *info);
+
+bfd_boolean _bfd_generic_link_check_relocs
+   (bfd *abfd, struct bfd_link_info *info);
+
 /* Extracted from simple.c.  */
 bfd_byte *bfd_simple_get_relocated_section_contents
    (bfd *abfd, asection *sec, bfd_byte *outbuf, asymbol **symbol_table);
diff --git a/bfd/binary.c b/bfd/binary.c
index ce94af2..2223b5d 100644
--- a/bfd/binary.c
+++ b/bfd/binary.c
@@ -309,12 +309,12 @@ binary_sizeof_headers (bfd *abfd ATTRIBUTE_UNUSED,
 #define binary_bfd_define_common_symbol            bfd_generic_define_common_symbol
 #define binary_bfd_link_hash_table_create         _bfd_generic_link_hash_table_create
 #define binary_bfd_link_just_syms                 _bfd_generic_link_just_syms
-#define binary_bfd_copy_link_hash_symbol_type \
-  _bfd_generic_copy_link_hash_symbol_type
+#define binary_bfd_copy_link_hash_symbol_type     _bfd_generic_copy_link_hash_symbol_type
 #define binary_bfd_link_add_symbols               _bfd_generic_link_add_symbols
 #define binary_bfd_final_link                     _bfd_generic_final_link
 #define binary_bfd_link_split_section             _bfd_generic_link_split_section
 #define binary_get_section_contents_in_window     _bfd_generic_get_section_contents_in_window
+#define binary_bfd_link_check_relocs              _bfd_generic_link_check_relocs
 
 const bfd_target binary_vec =
 {
diff --git a/bfd/bout.c b/bfd/bout.c
index 77023db..0718d3e 100644
--- a/bfd/bout.c
+++ b/bfd/bout.c
@@ -1393,6 +1393,7 @@ b_out_bfd_get_relocated_section_contents (bfd *output_bfd,
 #define b_out_section_already_linked           _bfd_generic_section_already_linked
 #define b_out_bfd_define_common_symbol         bfd_generic_define_common_symbol
 #define aout_32_get_section_contents_in_window _bfd_generic_get_section_contents_in_window
+#define b_out_bfd_link_check_relocs            _bfd_generic_link_check_relocs
 
 extern const bfd_target bout_le_vec;
 
diff --git a/bfd/coff-alpha.c b/bfd/coff-alpha.c
index 70388f7..55fd350 100644
--- a/bfd/coff-alpha.c
+++ b/bfd/coff-alpha.c
@@ -2351,6 +2351,7 @@ static const struct ecoff_backend_data alpha_ecoff_backend_data =
 #define _bfd_ecoff_section_already_linked \
   _bfd_coff_section_already_linked
 #define _bfd_ecoff_bfd_define_common_symbol bfd_generic_define_common_symbol
+#define _bfd_ecoff_bfd_link_check_relocs    _bfd_generic_link_check_relocs
 
 const bfd_target alpha_ecoff_le_vec =
 {
diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index e7a215b..ffef1ba 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -4014,6 +4014,7 @@ const struct xcoff_dwsect_name xcoff_dwsect_names[] = {
 #define _bfd_xcoff_bfd_discard_group bfd_generic_discard_group
 #define _bfd_xcoff_section_already_linked _bfd_generic_section_already_linked
 #define _bfd_xcoff_bfd_define_common_symbol _bfd_xcoff_define_common_symbol
+#define _bfd_xcoff_bfd_link_check_relocs    _bfd_generic_link_check_relocs
 
 /* For dynamic symbols and relocs entry points.  */
 #define _bfd_xcoff_get_synthetic_symtab _bfd_nodynamic_get_synthetic_symtab
diff --git a/bfd/coff64-rs6000.c b/bfd/coff64-rs6000.c
index a0bc160..7b5bb24 100644
--- a/bfd/coff64-rs6000.c
+++ b/bfd/coff64-rs6000.c
@@ -2741,6 +2741,7 @@ const bfd_target rs6000_xcoff64_vec =
     _bfd_generic_copy_link_hash_symbol_type,
     _bfd_xcoff_bfd_final_link,
     _bfd_generic_link_split_section,
+    _bfd_generic_link_check_relocs,
     bfd_generic_gc_sections,
     bfd_generic_lookup_section_flags,
     bfd_generic_merge_sections,
@@ -2999,6 +3000,7 @@ const bfd_target rs6000_xcoff64_aix_vec =
     _bfd_generic_copy_link_hash_symbol_type,
     _bfd_xcoff_bfd_final_link,
     _bfd_generic_link_split_section,
+    _bfd_generic_link_check_relocs,
     bfd_generic_gc_sections,
     bfd_generic_lookup_section_flags,
     bfd_generic_merge_sections,
diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 798f7f7..eef3fa8 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -5480,6 +5480,8 @@ dummy_reloc16_extra_cases (bfd *abfd ATTRIBUTE_UNUSED,
   _bfd_generic_copy_link_hash_symbol_type
 #define coff_bfd_link_split_section  _bfd_generic_link_split_section
 
+#define coff_bfd_link_check_relocs   _bfd_generic_link_check_relocs
+
 #ifndef coff_start_final_link
 #define coff_start_final_link NULL
 #endif
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index c179721..fde34b7 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -287,6 +287,10 @@
 #define bfd_elfNN_bfd_link_split_section _bfd_generic_link_split_section
 #endif
 
+#ifndef bfd_elfNN_bfd_link_check_relocs
+#define bfd_elfNN_bfd_link_check_relocs  _bfd_elf_link_check_relocs
+#endif
+
 #ifndef bfd_elfNN_archive_p
 #define bfd_elfNN_archive_p bfd_generic_archive_p
 #endif
diff --git a/bfd/i386msdos.c b/bfd/i386msdos.c
index fd24e8b..7be9544 100644
--- a/bfd/i386msdos.c
+++ b/bfd/i386msdos.c
@@ -157,6 +157,7 @@ msdos_set_section_contents (bfd *abfd,
 #define msdos_bfd_final_link _bfd_generic_final_link
 #define msdos_bfd_link_split_section _bfd_generic_link_split_section
 #define msdos_set_arch_mach _bfd_generic_set_arch_mach
+#define msdos_bfd_link_check_relocs _bfd_generic_link_check_relocs
 
 #define msdos_get_symtab_upper_bound _bfd_nosymbols_get_symtab_upper_bound
 #define msdos_canonicalize_symtab _bfd_nosymbols_canonicalize_symtab
diff --git a/bfd/i386os9k.c b/bfd/i386os9k.c
index 6bfe218..dc5624a 100644
--- a/bfd/i386os9k.c
+++ b/bfd/i386os9k.c
@@ -183,6 +183,7 @@ os9k_sizeof_headers (bfd *abfd ATTRIBUTE_UNUSED,
   _bfd_generic_copy_link_hash_symbol_type
 #define os9k_bfd_final_link _bfd_generic_final_link
 #define os9k_bfd_link_split_section  _bfd_generic_link_split_section
+#define os9k_bfd_link_check_relocs   _bfd_generic_link_check_relocs
 
 const bfd_target i386_aout_os9k_vec =
   {
diff --git a/bfd/ieee.c b/bfd/ieee.c
index f65f0f0..73b3f98 100644
--- a/bfd/ieee.c
+++ b/bfd/ieee.c
@@ -3863,6 +3863,7 @@ ieee_sizeof_headers (bfd *abfd ATTRIBUTE_UNUSED,
   _bfd_generic_copy_link_hash_symbol_type
 #define ieee_bfd_final_link _bfd_generic_final_link
 #define ieee_bfd_link_split_section  _bfd_generic_link_split_section
+#define ieee_bfd_link_check_relocs   _bfd_generic_link_check_relocs
 
 const bfd_target ieee_vec =
 {
diff --git a/bfd/ihex.c b/bfd/ihex.c
index ba3c087..27e197d 100644
--- a/bfd/ihex.c
+++ b/bfd/ihex.c
@@ -940,10 +940,10 @@ ihex_sizeof_headers (bfd *abfd ATTRIBUTE_UNUSED,
 #define ihex_bfd_link_hash_table_create           _bfd_generic_link_hash_table_create
 #define ihex_bfd_link_add_symbols                 _bfd_generic_link_add_symbols
 #define ihex_bfd_link_just_syms                   _bfd_generic_link_just_syms
-#define ihex_bfd_copy_link_hash_symbol_type \
-  _bfd_generic_copy_link_hash_symbol_type
+#define ihex_bfd_copy_link_hash_symbol_type       _bfd_generic_copy_link_hash_symbol_type
 #define ihex_bfd_final_link                       _bfd_generic_final_link
 #define ihex_bfd_link_split_section               _bfd_generic_link_split_section
+#define ihex_bfd_link_check_relocs                _bfd_generic_link_check_relocs
 
 /* The Intel Hex target vector.  */
 
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 89d215b..5f28863 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -499,6 +499,8 @@ extern bfd_boolean _bfd_generic_set_section_contents
 #define _bfd_nolink_bfd_define_common_symbol \
   ((bfd_boolean (*) (bfd *, struct bfd_link_info *, \
 		     struct bfd_link_hash_entry *)) bfd_false)
+#define _bfd_nolink_bfd_link_check_relocs \
+  _bfd_generic_link_check_relocs
 
 /* Routines to use for BFD_JUMP_TABLE_DYNAMIC for targets which do not
    have dynamic symbols or relocs.  Use BFD_JUMP_TABLE_DYNAMIC
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 16c0aee..5fe45ec 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -504,6 +504,8 @@ extern bfd_boolean _bfd_generic_set_section_contents
 #define _bfd_nolink_bfd_define_common_symbol \
   ((bfd_boolean (*) (bfd *, struct bfd_link_info *, \
 		     struct bfd_link_hash_entry *)) bfd_false)
+#define _bfd_nolink_bfd_link_check_relocs \
+  _bfd_generic_link_check_relocs
 
 /* Routines to use for BFD_JUMP_TABLE_DYNAMIC for targets which do not
    have dynamic symbols or relocs.  Use BFD_JUMP_TABLE_DYNAMIC
diff --git a/bfd/libecoff.h b/bfd/libecoff.h
index 0d97c46..e0991ee 100644
--- a/bfd/libecoff.h
+++ b/bfd/libecoff.h
@@ -242,6 +242,7 @@ extern bfd_boolean _bfd_ecoff_get_section_contents
   (bfd *, asection *, void * location, file_ptr, bfd_size_type);
 
 #define _bfd_ecoff_bfd_link_split_section _bfd_generic_link_split_section
+#define _bfd_ecoff_bfd_link_check_relocs  _bfd_generic_link_check_relocs
 
 extern bfd_boolean _bfd_ecoff_bfd_copy_private_bfd_data
   (bfd *, bfd *);
diff --git a/bfd/linker.c b/bfd/linker.c
index 97b5d18..082eea0 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -3304,3 +3304,46 @@ bfd_hide_sym_by_version (struct bfd_elf_version_tree *verdefs,
   bfd_find_version_for_sym (verdefs, sym_name, &hidden);
   return hidden;
 }
+
+/*
+FUNCTION
+	bfd_link_check_relocs
+
+SYNOPSIS
+	bfd_boolean bfd_link_check_relocs
+	  (bfd *abfd, struct bfd_link_info *info);
+
+DESCRIPTION
+	Checks the relocs in ABFD for validity.
+	Does not execute the relocs.
+	Return TRUE if everything is OK, FALSE otherwise.
+	This is the external entry point to this code.
+*/
+
+bfd_boolean
+bfd_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
+{
+  return BFD_SEND (abfd, _bfd_link_check_relocs, (abfd, info));
+}
+
+/*
+FUNCTION
+	_bfd_generic_link_check_relocs
+
+SYNOPSIS
+	bfd_boolean _bfd_generic_link_check_relocs
+	  (bfd *abfd, struct bfd_link_info *info);
+
+DESCRIPTION
+        Stub function for targets that do not implement reloc checking.
+	Return TRUE.
+	This is an internal function.  It should not be called from
+	outside the BFD library.
+*/
+
+bfd_boolean
+_bfd_generic_link_check_relocs (bfd *abfd ATTRIBUTE_UNUSED,
+				struct bfd_link_info *info ATTRIBUTE_UNUSED)
+{
+  return TRUE;
+}
diff --git a/bfd/mach-o-target.c b/bfd/mach-o-target.c
index bf956fd..6b615ed 100644
--- a/bfd/mach-o-target.c
+++ b/bfd/mach-o-target.c
@@ -44,6 +44,7 @@
   _bfd_generic_copy_link_hash_symbol_type
 #define bfd_mach_o_bfd_final_link                     _bfd_generic_final_link
 #define bfd_mach_o_bfd_link_split_section             _bfd_generic_link_split_section
+#define bfd_mach_o_bfd_link_check_relocs              _bfd_generic_link_check_relocs
 #define bfd_mach_o_bfd_merge_private_bfd_data         _bfd_generic_bfd_merge_private_bfd_data
 #define bfd_mach_o_bfd_set_private_flags              bfd_mach_o_bfd_set_private_flags
 #define bfd_mach_o_get_section_contents               _bfd_generic_get_section_contents
diff --git a/bfd/mmo.c b/bfd/mmo.c
index d396fd7..b8af63a 100644
--- a/bfd/mmo.c
+++ b/bfd/mmo.c
@@ -3278,6 +3278,7 @@ mmo_write_object_contents (bfd *abfd)
   _bfd_generic_copy_link_hash_symbol_type
 #define mmo_bfd_final_link _bfd_generic_final_link
 #define mmo_bfd_link_split_section _bfd_generic_link_split_section
+#define mmo_bfd_link_check_relocs  _bfd_generic_link_check_relocs
 
 /* Strictly speaking, only MMIX uses this restricted format, but let's not
    stop anybody from shooting themselves in the foot.  */
diff --git a/bfd/nlm-target.h b/bfd/nlm-target.h
index 082e856..6ed0b5c 100644
--- a/bfd/nlm-target.h
+++ b/bfd/nlm-target.h
@@ -59,6 +59,7 @@
   _bfd_generic_copy_link_hash_symbol_type
 #define nlm_bfd_final_link                      _bfd_generic_final_link
 #define nlm_bfd_link_split_section              _bfd_generic_link_split_section
+#define nlm_bfd_link_check_relocs               _bfd_generic_link_check_relocs
 
 /* This structure contains everything that BFD knows about a target.
    It includes things like its byte order, name, what routines to call
diff --git a/bfd/oasys.c b/bfd/oasys.c
index f7791d2..31555e0 100644
--- a/bfd/oasys.c
+++ b/bfd/oasys.c
@@ -1192,6 +1192,7 @@ oasys_sizeof_headers (bfd *abfd ATTRIBUTE_UNUSED,
   _bfd_generic_copy_link_hash_symbol_type
 #define oasys_bfd_final_link                       _bfd_generic_final_link
 #define oasys_bfd_link_split_section               _bfd_generic_link_split_section
+#define oasys_bfd_link_check_relocs                _bfd_generic_link_check_relocs
 
 const bfd_target oasys_vec =
 {
diff --git a/bfd/pef.c b/bfd/pef.c
index fb9a07a..30c3179 100644
--- a/bfd/pef.c
+++ b/bfd/pef.c
@@ -67,6 +67,7 @@
 #define bfd_pef_bfd_final_link                      _bfd_generic_final_link
 #define bfd_pef_bfd_link_split_section              _bfd_generic_link_split_section
 #define bfd_pef_get_section_contents_in_window      _bfd_generic_get_section_contents_in_window
+#define bfd_pef_bfd_link_check_relocs               _bfd_generic_link_check_relocs
 
 static int
 bfd_pef_parse_traceback_table (bfd *abfd,
diff --git a/bfd/plugin.c b/bfd/plugin.c
index f57833c..fd7bac0 100644
--- a/bfd/plugin.c
+++ b/bfd/plugin.c
@@ -106,6 +106,7 @@ dlerror (void)
 #define bfd_plugin_section_already_linked             _bfd_generic_section_already_linked
 #define bfd_plugin_bfd_define_common_symbol           bfd_generic_define_common_symbol
 #define bfd_plugin_bfd_copy_link_hash_symbol_type     _bfd_generic_copy_link_hash_symbol_type
+#define bfd_plugin_bfd_link_check_relocs              _bfd_generic_link_check_relocs
 
 static enum ld_plugin_status
 message (int level ATTRIBUTE_UNUSED,
diff --git a/bfd/ppcboot.c b/bfd/ppcboot.c
index ea54d62..afb6ce0 100644
--- a/bfd/ppcboot.c
+++ b/bfd/ppcboot.c
@@ -465,6 +465,7 @@ ppcboot_bfd_print_private_bfd_data (bfd *abfd, void * farg)
 #define ppcboot_bfd_link_split_section _bfd_generic_link_split_section
 #define ppcboot_get_section_contents_in_window \
   _bfd_generic_get_section_contents_in_window
+#define ppcboot_bfd_link_check_relocs _bfd_generic_link_check_relocs
 
 #define ppcboot_bfd_copy_private_bfd_data _bfd_generic_bfd_copy_private_bfd_data
 #define ppcboot_bfd_merge_private_bfd_data _bfd_generic_bfd_merge_private_bfd_data
diff --git a/bfd/som.c b/bfd/som.c
index 859e886..635727c 100644
--- a/bfd/som.c
+++ b/bfd/som.c
@@ -6758,6 +6758,7 @@ som_bfd_link_split_section (bfd *abfd ATTRIBUTE_UNUSED, asection *sec)
 #define som_bfd_copy_private_header_data	_bfd_generic_bfd_copy_private_header_data
 #define som_bfd_set_private_flags		_bfd_generic_bfd_set_private_flags
 #define som_find_inliner_info			_bfd_nosymbols_find_inliner_info
+#define som_bfd_link_check_relocs               _bfd_generic_link_check_relocs
 
 const bfd_target hppa_som_vec =
 {
diff --git a/bfd/srec.c b/bfd/srec.c
index 02b8dad..5fdd68b 100644
--- a/bfd/srec.c
+++ b/bfd/srec.c
@@ -1275,10 +1275,10 @@ srec_print_symbol (bfd *abfd,
 #define srec_bfd_link_hash_table_create           _bfd_generic_link_hash_table_create
 #define srec_bfd_link_add_symbols                 _bfd_generic_link_add_symbols
 #define srec_bfd_link_just_syms                   _bfd_generic_link_just_syms
-#define srec_bfd_copy_link_hash_symbol_type \
-  _bfd_generic_copy_link_hash_symbol_type
+#define srec_bfd_copy_link_hash_symbol_type       _bfd_generic_copy_link_hash_symbol_type
 #define srec_bfd_final_link                       _bfd_generic_final_link
 #define srec_bfd_link_split_section               _bfd_generic_link_split_section
+#define srec_bfd_link_check_relocs                _bfd_generic_link_check_relocs
 
 const bfd_target srec_vec =
 {
diff --git a/bfd/targets.c b/bfd/targets.c
index 50f3712..a9edd4c 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -452,6 +452,7 @@ BFD_JUMP_TABLE macros.
 .  NAME##_bfd_copy_link_hash_symbol_type, \
 .  NAME##_bfd_final_link, \
 .  NAME##_bfd_link_split_section, \
+.  NAME##_bfd_link_check_relocs, \
 .  NAME##_bfd_gc_sections, \
 .  NAME##_bfd_lookup_section_flags, \
 .  NAME##_bfd_merge_sections, \
@@ -493,6 +494,9 @@ BFD_JUMP_TABLE macros.
 .  {* Should this section be split up into smaller pieces during linking.  *}
 .  bfd_boolean (*_bfd_link_split_section) (bfd *, struct bfd_section *);
 .
+.  {* Check the relocations in the bfd for validity.  *}
+.  bfd_boolean (* _bfd_link_check_relocs)(bfd *, struct bfd_link_info *);
+.
 .  {* Remove sections that are not referenced from the output.  *}
 .  bfd_boolean (*_bfd_gc_sections) (bfd *, struct bfd_link_info *);
 .
diff --git a/bfd/tekhex.c b/bfd/tekhex.c
index 7de2f24..b94f843 100644
--- a/bfd/tekhex.c
+++ b/bfd/tekhex.c
@@ -980,11 +980,11 @@ tekhex_print_symbol (bfd *abfd,
 #define tekhex_bfd_link_hash_table_create           _bfd_generic_link_hash_table_create
 #define tekhex_bfd_link_add_symbols                 _bfd_generic_link_add_symbols
 #define tekhex_bfd_link_just_syms                   _bfd_generic_link_just_syms
-#define tekhex_bfd_copy_link_hash_symbol_type \
-  _bfd_generic_copy_link_hash_symbol_type
+#define tekhex_bfd_copy_link_hash_symbol_type       _bfd_generic_copy_link_hash_symbol_type
 #define tekhex_bfd_final_link                       _bfd_generic_final_link
 #define tekhex_bfd_link_split_section               _bfd_generic_link_split_section
 #define tekhex_get_section_contents_in_window       _bfd_generic_get_section_contents_in_window
+#define tekhex_bfd_link_check_relocs                _bfd_generic_link_check_relocs
 
 const bfd_target tekhex_vec =
 {
diff --git a/bfd/versados.c b/bfd/versados.c
index c8b3c48..ed46e3b 100644
--- a/bfd/versados.c
+++ b/bfd/versados.c
@@ -873,6 +873,7 @@ versados_canonicalize_reloc (bfd *abfd,
   _bfd_generic_copy_link_hash_symbol_type
 #define versados_bfd_final_link                       _bfd_generic_final_link
 #define versados_bfd_link_split_section               _bfd_generic_link_split_section
+#define versados_bfd_link_check_relocs                _bfd_generic_link_check_relocs
 
 const bfd_target m68k_versados_vec =
 {
diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index d55780e..e6cfc1f 100644
--- a/bfd/vms-alpha.c
+++ b/bfd/vms-alpha.c
@@ -9260,6 +9260,7 @@ bfd_vms_get_data (bfd *abfd)
   _bfd_nodynamic_get_dynamic_reloc_upper_bound
 #define alpha_vms_canonicalize_dynamic_reloc \
   _bfd_nodynamic_canonicalize_dynamic_reloc
+#define alpha_vms_bfd_link_check_relocs              _bfd_generic_link_check_relocs
 
 const bfd_target alpha_vms_vec =
 {
diff --git a/bfd/xsym.c b/bfd/xsym.c
index c637f74..62cc02e 100644
--- a/bfd/xsym.c
+++ b/bfd/xsym.c
@@ -59,6 +59,7 @@
 #define bfd_sym_bfd_final_link                      _bfd_generic_final_link
 #define bfd_sym_bfd_link_split_section              _bfd_generic_link_split_section
 #define bfd_sym_get_section_contents_in_window      _bfd_generic_get_section_contents_in_window
+#define bfd_sym_bfd_link_check_relocs               _bfd_generic_link_check_relocs
 
 extern const bfd_target sym_vec;
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 90467b5..728fbe7 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -226,6 +226,11 @@ extern void bfd_link_repair_undef_list
 /* Read symbols and cache symbol pointer array in outsymbols.  */
 extern bfd_boolean bfd_generic_link_read_symbols (bfd *);
 
+/* Check the relocs in the BFD.  Called after all the input
+   files have been loaded, and garbage collection has tagged
+   any unneeded sections.  */
+extern bfd_boolean bfd_link_check_relocs (bfd *,struct bfd_link_info *);
+
 struct bfd_sym_chain
 {
   struct bfd_sym_chain *next;
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 2ae3640..b44fc8f 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6790,11 +6790,13 @@ lang_check_relocs (void)
 
       for (abfd = link_info.input_bfds;
 	   abfd != (bfd *) NULL; abfd = abfd->link.next)
-	if (!_bfd_elf_link_check_relocs (abfd, &link_info))
+	if (!bfd_link_check_relocs (abfd, &link_info))
 	  {
-	    /* no object output, fail return */
+	    /* No object output, fail return.  */
 	    config.make_executable = FALSE;
-	    break;
+	    /* Note: we do not abort the loop, but rather
+	       continue the scan in case there are other
+	       bad relocations to report.  */
 	  }
     }
 }

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-21 10:33           ` Nick Clifton
@ 2016-04-21 10:36             ` Nick Clifton
  2016-04-21 11:18               ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Clifton @ 2016-04-21 10:36 UTC (permalink / raw)
  To: H.J. Lu, Alan Modra; +Cc: Binutils

Hi H.J.

Ooo - I forgot to mention one other feature of the patch, which 
you might find objectionable.  It changes the loop in lang_check_relocs() 
so that it does not terminate when a reloc check fails.  Instead 
I thought it better to carry on processing the remaining bfds, in
case there are further errors that can be reported to the user.

Cheers
  Nick

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-21 10:36             ` Nick Clifton
@ 2016-04-21 11:18               ` H.J. Lu
  0 siblings, 0 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-21 11:18 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Alan Modra, Binutils

On Thu, Apr 21, 2016 at 3:35 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi H.J.
>
> Ooo - I forgot to mention one other feature of the patch, which
> you might find objectionable.  It changes the loop in lang_check_relocs()
> so that it does not terminate when a reloc check fails.  Instead
> I thought it better to carry on processing the remaining bfds, in
> case there are further errors that can be reported to the user.
>
> Cheers
>   Nick

It is OK as long as there is no regression.   BTW, you
should remove

&& bfd_get_flavour (link_info.output_bfd) == bfd_target_elf_flavour)

before the loop.


-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-21  0:39       ` H.J. Lu
@ 2016-04-26 14:25         ` H.J. Lu
  2016-04-27  2:29           ` Alan Modra
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-26 14:25 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Wed, Apr 20, 2016 at 5:39 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Apr 19, 2016 at 4:24 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Tue, Apr 19, 2016 at 11:03:15AM -0700, H.J. Lu wrote:
>>> On Tue, Apr 19, 2016 at 9:01 AM, Nick Clifton <nickc@redhat.com> wrote:
>>> > Hi H.J.
>>> >
>>> >> Delaying checking ELF relocations until opening all input files so
>>> >> that symbol information is final when relocations are checked.  This
>>> >> is only enabled for x86 targets.
>>> >
>>> >> Any comments, feedbacks?
>>> >
>>> > What benefit is gained by doing this ?  I would guess that it is connected
>>> > with symbols changing type and/or visibility and/or protected status, but
>>> > it would be nice to know what you are hoping to achieve.
>>>
>>> Yes,  this change is motivated by that discussion.  Currently backend
>>> check_relocs doesn't have the final information on symbols.  A better
>>> linker can provide better diagnostics in check_relocs if symbol info is
>>> final.
>>
>> There are other benefits too.  As it is the backend check_relocs
>> functions do things like "maybe this symbol remains undefined so
>> perhaps it needs a plt entry", and "maybe this symbol remains dynamic
>> so perhaps it needs dynamic relocations".  This complexity can
>> disappear if all of those "maybe" cases are resolved.
>
> The first phase of x86 backend is done.  I will take a look to see if I can
> remove some checks in x86 check_relocs when I have time.
>
>> Now, if you run check_relocs even later, say at
>> ldemul_before_allocation, after gc-sections has run, then all the
>> reference counting code for plt and got relocs can be removed too.
>
> I  checked in:
>
> commit e66cdd681f47dc51beaeee3d813f1c9cba27dedf
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Wed Apr 20 17:12:46 2016 -0700
>
>     Remove x86 gc_sweep_hook
>
>     Since x86 backends never see the removed sections, there is no need
>     for gc_sweep_hook.
>
>       * elf32-i386.c (elf_i386_gc_sweep_hook): Removed.
>       (elf_backend_gc_sweep_hook): Likewise.
>       * elf64-x86-64.c (elf_x86_64_gc_sweep_hook): Likewise.
>       (elf_backend_gc_sweep_hook): Likewise.
>
> Other reference counting code for plt and got relocs in x86 backend
> has to stay for TLS optimization and GOT/PLT optimization.

Another problem is when check_relocs is run, linker defined symbols
haven't been processed yet.

-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-26 14:25         ` H.J. Lu
@ 2016-04-27  2:29           ` Alan Modra
  2016-04-27 22:24             ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Alan Modra @ 2016-04-27  2:29 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

On Tue, Apr 26, 2016 at 07:25:01AM -0700, H.J. Lu wrote:
> Another problem is when check_relocs is run, linker defined symbols
> haven't been processed yet.

__start_<section> and __stop_<section>?  Yes, they are a pain.

Perhaps this piece of infrastructure will help.

include/
	* bfdlink.h (struct bfd_link_hash_entry): Add "section" field to
	undef.  Formatting.
bfd/
	* elflink.c (_bfd_elf_is_start_stop): New function.
	(_bfd_elf_gc_mark_rsec): Use it.
	* elf-bfd.h (_bfd_elf_is_start_stop): Declare.

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 7447629..6c05b55 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2336,6 +2336,9 @@ extern bfd_boolean bfd_elf_gc_common_finalize_got_offsets
 extern bfd_boolean bfd_elf_gc_common_final_link
   (bfd *, struct bfd_link_info *);
 
+extern asection *_bfd_elf_is_start_stop
+  (const struct bfd_link_info *, struct elf_link_hash_entry *);
+
 extern bfd_boolean bfd_elf_reloc_symbol_deleted_p
   (bfd_vma, void *);
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index a6b3c94..b6ff6b6 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -12230,6 +12230,55 @@ _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.  */
+
+asection *
+_bfd_elf_is_start_stop (const struct bfd_link_info *info,
+			struct elf_link_hash_entry *h)
+{
+  asection *s;
+  const char *sec_name;
+
+  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 (s == (asection *) 0 - 1)
+	return NULL;
+      return s;
+    }
+
+  sec_name = NULL;
+  if (strncmp (h->root.root.string, "__start_", 8) == 0)
+    sec_name = h->root.root.string + 8;
+  else if (strncmp (h->root.root.string, "__stop_", 7) == 0)
+    sec_name = h->root.root.string + 7;
+
+  if (sec_name != NULL && *sec_name != '\0')
+    {
+      bfd *i;
+
+      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 (s == NULL)
+    h->root.u.undef.section = (asection *) 0 - 1;
+
+  return s;
+}
+
 /* COOKIE->rel describes a relocation against section SEC, which is
    a section we've decided to keep.  Return the section that contains
    the relocation symbol, or NULL if no section contains it.  */
@@ -12268,34 +12317,19 @@ _bfd_elf_gc_mark_rsec (struct bfd_link_info *info, asection *sec,
       if (h->u.weakdef != NULL)
 	h->u.weakdef->mark = 1;
 
-      if (start_stop != NULL
-	  && (h->root.type == bfd_link_hash_undefined
-	      || h->root.type == bfd_link_hash_undefweak))
+      if (start_stop != NULL)
 	{
 	  /* To work around a glibc bug, mark all XXX input sections
 	     when there is an as yet undefined reference to __start_XXX
 	     or __stop_XXX symbols.  The linker will later define such
 	     symbols for orphan input sections that have a name
 	     representable as a C identifier.  */
-	  const char *sec_name = NULL;
-	  if (strncmp (h->root.root.string, "__start_", 8) == 0)
-	    sec_name = h->root.root.string + 8;
-	  else if (strncmp (h->root.root.string, "__stop_", 7) == 0)
-	    sec_name = h->root.root.string + 7;
+	  asection *s = _bfd_elf_is_start_stop (info, h);
 
-	  if (sec_name != NULL && *sec_name != '\0')
+	  if (s != NULL)
 	    {
-	      bfd *i;
-
-	      for (i = info->input_bfds; i != NULL; i = i->link.next)
-		{
-		  asection *s = bfd_get_section_by_name (i, sec_name);
-		  if (s != NULL && !s->gc_mark)
-		    {
-		      *start_stop = TRUE;
-		      return s;
-		    }
-		}
+	      *start_stop = !s->gc_mark;
+	      return s;
 	    }
 	}
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 728fbe7..56ab038 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -135,21 +135,29 @@ struct bfd_link_hash_entry
 	     automatically be non-NULL since the symbol will have been on the
 	     undefined symbol list.  */
 	  struct bfd_link_hash_entry *next;
-	  bfd *abfd;		/* BFD symbol was found in.  */
+	  /* BFD symbol was found in.  */
+	  bfd *abfd;
+	  /* For __start_<name> and __stop_<name> symbols, the first
+	     input section matching the name.  */
+	  asection *section;
 	} undef;
       /* bfd_link_hash_defined, bfd_link_hash_defweak.  */
       struct
 	{
 	  struct bfd_link_hash_entry *next;
-	  asection *section;	/* Symbol section.  */
-	  bfd_vma value;	/* Symbol value.  */
+	  /* Symbol section.  */
+	  asection *section;
+	  /* Symbol value.  */
+	  bfd_vma value;
 	} def;
       /* bfd_link_hash_indirect, bfd_link_hash_warning.  */
       struct
 	{
 	  struct bfd_link_hash_entry *next;
-	  struct bfd_link_hash_entry *link;	/* Real symbol.  */
-	  const char *warning;	/* Warning (bfd_link_hash_warning only).  */
+	  /* Real symbol.  */
+	  struct bfd_link_hash_entry *link;
+	  /* Warning message (bfd_link_hash_warning only).  */
+	  const char *warning;
 	} i;
       /* bfd_link_hash_common.  */
       struct
@@ -165,7 +173,8 @@ struct bfd_link_hash_entry
 	     the union; this structure is a major space user in the
 	     linker.  */
 	  struct bfd_link_hash_common_entry *p;
-	  bfd_size_type size;	/* Common symbol size.  */
+	  /* Common symbol size.  */
+	  bfd_size_type size;
 	} c;
     } u;
 };

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-27  2:29           ` Alan Modra
@ 2016-04-27 22:24             ` H.J. Lu
  2016-04-28  1:12               ` Alan Modra
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-27 22:24 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

On Tue, Apr 26, 2016 at 7:29 PM, Alan Modra <amodra@gmail.com> wrote:
> On Tue, Apr 26, 2016 at 07:25:01AM -0700, H.J. Lu wrote:
>> Another problem is when check_relocs is run, linker defined symbols
>> haven't been processed yet.
>
> __start_<section> and __stop_<section>?  Yes, they are a pain.
>
> Perhaps this piece of infrastructure will help.
>
> include/
>         * bfdlink.h (struct bfd_link_hash_entry): Add "section" field to
>         undef.  Formatting.
> bfd/
>         * elflink.c (_bfd_elf_is_start_stop): New function.
>         (_bfd_elf_gc_mark_rsec): Use it.
>         * elf-bfd.h (_bfd_elf_is_start_stop): Declare.
>

It helps.  However, I ran into another problem.  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.


-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-27 22:24             ` H.J. Lu
@ 2016-04-28  1:12               ` Alan Modra
  2016-04-28 12:50                 ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Alan Modra @ 2016-04-28  1:12 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

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.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-28  1:12               ` Alan Modra
@ 2016-04-28 12:50                 ` H.J. Lu
  2016-04-28 13:07                   ` Alan Modra
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-28 12:50 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

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.

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

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


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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-28 12:50                 ` H.J. Lu
@ 2016-04-28 13:07                   ` Alan Modra
  2016-04-28 13:09                     ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Alan Modra @ 2016-04-28 13:07 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Binutils

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 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.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-28 13:07                   ` Alan Modra
@ 2016-04-28 13:09                     ` H.J. Lu
  2016-04-28 13:40                       ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-28 13:09 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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.
>
> --
> Alan Modra
> Australia Development Lab, IBM



-- 
H.J.

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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-28 13:09                     ` H.J. Lu
@ 2016-04-28 13:40                       ` H.J. Lu
  2016-04-28 17:11                         ` H.J. Lu
  0 siblings, 1 reply; 25+ messages in thread
From: H.J. Lu @ 2016-04-28 13:40 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

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.



-- 
H.J.

[-- Attachment #2: 0001-Add-_bfd_elf_record_start_stop.patch --]
[-- Type: text/x-patch, Size: 2193 bytes --]

From 16526e42c26b00e2424a0d0b5a73c04f010f7a9e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 28 Apr 2016 06:38:41 -0700
Subject: [PATCH] Add _bfd_elf_record_start_stop

---
 bfd/elf-bfd.h         |  3 +++
 bfd/elflink.c         | 21 +++++++++++++++++++++
 ld/emultempl/elf32.em |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 6c05b55..480f1cb 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
+  (const 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..6c2ddb4 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -13791,3 +13791,24 @@ 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);
 }
+
+/* For undefined __start_<name> and __stop_<name> symbols, set
+   def_regular to 1.  This is called via elf_link_hash_traverse.  */
+
+static bfd_boolean
+elf_link_record_start_stop (struct elf_link_hash_entry *h, void *data)
+{
+  const struct bfd_link_info * info
+    = (const struct bfd_link_info *) data;
+  if (_bfd_elf_is_start_stop (info, h) != NULL)
+    h->def_regular = 1;
+  return TRUE;
+}
+
+void
+_bfd_elf_record_start_stop (const struct bfd_link_info * info)
+{
+  elf_link_hash_traverse (elf_hash_table (info),
+			  elf_link_record_start_stop,
+			  (void *) info);
+}
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index cc2f056..ad48ce4 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1445,6 +1445,8 @@ gld${EMULATION_NAME}_record_link_assignments (lang_phase_type phase)
 	 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_record_start_stop (&link_info);
     }
 }
 
-- 
2.5.5


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

* Re: [RFC][PATCH] Check ELF relocs after opening all all input files
  2016-04-28 13:40                       ` H.J. Lu
@ 2016-04-28 17:11                         ` H.J. Lu
  0 siblings, 0 replies; 25+ messages in thread
From: H.J. Lu @ 2016-04-28 17:11 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Binutils

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

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.


-- 
H.J.

[-- Attachment #2: 0001-Add-_bfd_elf_record_start_stop.patch --]
[-- Type: text/x-patch, Size: 4843 bytes --]

From b6809eeed88f48dcbe71c1e4ce1213748b0bfe22 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 28 Apr 2016 06:38:41 -0700
Subject: [PATCH] Add _bfd_elf_record_start_stop

---
 bfd/elf-bfd.h         |  3 ++
 bfd/elflink.c         | 87 ++++++++++++++++++++++++++++++++++++++++-----------
 ld/emultempl/elf32.em |  4 +++
 3 files changed, 75 insertions(+), 19 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/elf32.em b/ld/emultempl/elf32.em
index cc2f056..86105fe 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1445,6 +1445,10 @@ gld${EMULATION_NAME}_record_link_assignments (lang_phase_type phase)
 	 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);
     }
 }
 
-- 
2.5.5


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

end of thread, other threads:[~2016-04-28 17:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-19 13:50 [RFC][PATCH] Check ELF relocs after opening all all input files H.J. Lu
2016-04-19 16:01 ` Nick Clifton
2016-04-19 18:03   ` H.J. Lu
2016-04-19 23:25     ` Alan Modra
2016-04-19 23:47       ` H.J. Lu
2016-04-20 12:23         ` H.J. Lu
2016-04-20 13:55           ` Alan Modra
2016-04-20 14:25             ` H.J. Lu
2016-04-20 18:34         ` H.J. Lu
2016-04-20 23:32           ` Alan Modra
2016-04-21 10:33           ` Nick Clifton
2016-04-21 10:36             ` Nick Clifton
2016-04-21 11:18               ` H.J. Lu
2016-04-20  0:14       ` Rich Felker
2016-04-21  0:39       ` H.J. Lu
2016-04-26 14:25         ` H.J. Lu
2016-04-27  2:29           ` Alan Modra
2016-04-27 22:24             ` H.J. Lu
2016-04-28  1:12               ` Alan Modra
2016-04-28 12:50                 ` H.J. Lu
2016-04-28 13:07                   ` Alan Modra
2016-04-28 13:09                     ` H.J. Lu
2016-04-28 13:40                       ` H.J. Lu
2016-04-28 17:11                         ` H.J. Lu
2016-04-20  9:28     ` Nick Clifton

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).