This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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.

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
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]