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: [PATCH] PR ld/19939: Incorrect error message for undefined symbols


On Mon, Apr 11, 2016 at 5:40 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Apr 11, 2016 at 10:27:15AM -0700, H.J. Lu wrote:
>>       * bfdlink.h (bfd_link_info): Add readonly_dynrelocs_against_ifunc.
>
> A flag that is only used by the x86 BFD backend doesn't belong in
> bfd_link_info, unless there's a really good reason I'm missing.
>

Good point.  This is the updated patch I am going to check in.


-- 
H.J.
From ed547f53955bf07d7daf20b9f947deca3e0b093a Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 11 Apr 2016 09:21:27 -0700
Subject: [PATCH] Properly handle dynamic reloc against normal symbol

We shouldn't issue an error for read-only segment with dynamic IFUNC
relocations when dynamic relocations are against normal symbols.

bfd/

	PR ld/19939
	* elf-bfd.h (_bfd_elf_allocate_ifunc_dyn_relocs): Add a pointer
	to bfd_boolean.
	* elf-ifunc.c (_bfd_elf_allocate_ifunc_dyn_relocs): Updated.
	Set *readonly_dynrelocs_against_ifunc_p to TRUE if dynamic reloc
	applies to read-only section.
	* elf32-i386.c (elf_i386_link_hash_table): Add
	readonly_dynrelocs_against_ifunc.
	(elf_i386_allocate_dynrelocs): Updated.
	(elf_i386_size_dynamic_sections): Issue an error for read-only
	segment with dynamic IFUNC relocations only if
	readonly_dynrelocs_against_ifunc is TRUE.
	* elf64-x86-64.c (elf_x86_64_link_hash_table): Add
	readonly_dynrelocs_against_ifunc.
	(elf_x86_64_allocate_dynrelocs): Updated.
	(elf_x86_64_size_dynamic_sections): Issue an error for read-only
	segment with dynamic IFUNC relocations only if
	readonly_dynrelocs_against_ifunc is TRUE.
	* elfnn-aarch64.c (elfNN_aarch64_allocate_ifunc_dynrelocs):
	Updated.

ld/

	PR ld/19939
	* testsuite/ld-i386/i386.exp: Run PR ld/19939 tests.
	* testsuite/ld-x86-64/x86-64.exp: Likewise.
	* testsuite/ld-i386/pr19939.s: New file.
	* testsuite/ld-i386/pr19939a.d: Likewise.
	* testsuite/ld-i386/pr19939b.d: Likewise.
	* testsuite/ld-x86-64/pr19939.s: Likewise.
	* testsuite/ld-x86-64/pr19939a.d: Likewise.
	* testsuite/ld-x86-64/pr19939b.d: Likewise.
---
 bfd/elf-bfd.h                     |  3 ++-
 bfd/elf-ifunc.c                   | 13 +++++++++++++
 bfd/elf32-i386.c                  | 10 +++++++---
 bfd/elf64-x86-64.c                |  8 ++++++--
 bfd/elfnn-aarch64.c               |  1 +
 ld/testsuite/ld-i386/i386.exp     |  2 ++
 ld/testsuite/ld-i386/pr19939.s    |  9 +++++++++
 ld/testsuite/ld-i386/pr19939a.d   |  4 ++++
 ld/testsuite/ld-i386/pr19939b.d   |  8 ++++++++
 ld/testsuite/ld-x86-64/pr19939.s  |  9 +++++++++
 ld/testsuite/ld-x86-64/pr19939a.d |  4 ++++
 ld/testsuite/ld-x86-64/pr19939b.d |  8 ++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  2 ++
 13 files changed, 75 insertions(+), 6 deletions(-)
 create mode 100644 ld/testsuite/ld-i386/pr19939.s
 create mode 100644 ld/testsuite/ld-i386/pr19939a.d
 create mode 100644 ld/testsuite/ld-i386/pr19939b.d
 create mode 100644 ld/testsuite/ld-x86-64/pr19939.s
 create mode 100644 ld/testsuite/ld-x86-64/pr19939a.d
 create mode 100644 ld/testsuite/ld-x86-64/pr19939b.d

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index ea4d59a..f820e85 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2490,7 +2490,8 @@ extern bfd_boolean _bfd_elf_create_ifunc_sections
   (bfd *, struct bfd_link_info *);
 extern bfd_boolean _bfd_elf_allocate_ifunc_dyn_relocs
   (struct bfd_link_info *, struct elf_link_hash_entry *,
-   struct elf_dyn_relocs **, unsigned int, unsigned int, unsigned int);
+   struct elf_dyn_relocs **, bfd_boolean *, unsigned int,
+   unsigned int, unsigned int);
 extern long _bfd_elf_ifunc_get_synthetic_symtab
   (bfd *, long, asymbol **, long, asymbol **, asymbol **, asection *,
    bfd_vma *(*) (bfd *, asymbol **, asection *, asection *));
diff --git a/bfd/elf-ifunc.c b/bfd/elf-ifunc.c
index 1368a8e..1377fed 100644
--- a/bfd/elf-ifunc.c
+++ b/bfd/elf-ifunc.c
@@ -110,6 +110,7 @@ bfd_boolean
 _bfd_elf_allocate_ifunc_dyn_relocs (struct bfd_link_info *info,
 				    struct elf_link_hash_entry *h,
 				    struct elf_dyn_relocs **head,
+				    bfd_boolean *readonly_dynrelocs_against_ifunc_p,
 				    unsigned int plt_entry_size,
 				    unsigned int plt_header_size,
 				    unsigned int got_entry_size)
@@ -119,6 +120,7 @@ _bfd_elf_allocate_ifunc_dyn_relocs (struct bfd_link_info *info,
   unsigned int sizeof_reloc;
   const struct elf_backend_data *bed;
   struct elf_link_hash_table *htab;
+  bfd_boolean readonly_dynrelocs_against_ifunc;
 
   /* When a shared library references a STT_GNU_IFUNC symbol defined
      in executable, the address of the resolved function may be used.
@@ -224,6 +226,8 @@ keep:
       || !h->non_got_ref)
     *head = NULL;
 
+  readonly_dynrelocs_against_ifunc = FALSE;
+
   /* Finally, allocate space.  */
   p = *head;
   if (p != NULL)
@@ -231,6 +235,12 @@ keep:
       bfd_size_type count = 0;
       do
 	{
+	  if (!readonly_dynrelocs_against_ifunc)
+	    {
+	      asection *s = p->sec->output_section;
+	      if (s != NULL && (s->flags & SEC_READONLY) != 0)
+		readonly_dynrelocs_against_ifunc = TRUE;
+	    }
 	  count += p->count;
 	  p = p->next;
 	}
@@ -238,6 +248,9 @@ keep:
       htab->irelifunc->size += count * sizeof_reloc;
     }
 
+  if (readonly_dynrelocs_against_ifunc_p)
+    *readonly_dynrelocs_against_ifunc_p = readonly_dynrelocs_against_ifunc;
+
   /* For STT_GNU_IFUNC symbol, .got.plt has the real function address
      and .got has the PLT entry adddress.  We will load the GOT entry
      with the PLT entry in finish_dynamic_symbol if it is used.  For
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 1ebb615..376f2cc 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -878,6 +878,10 @@ struct elf_i386_link_hash_table
 
   /* The index of the next unused R_386_IRELATIVE slot in .rel.plt.  */
   bfd_vma next_irelative_index;
+
+  /* TRUE if there are dynamic relocs against IFUNC symbols that apply
+     to read-only sections.  */
+  bfd_boolean readonly_dynrelocs_against_ifunc;
 };
 
 /* Get the i386 ELF linker hash table from a link_info structure.  */
@@ -2442,7 +2446,8 @@ elf_i386_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
   if (h->type == STT_GNU_IFUNC
       && h->def_regular)
     return _bfd_elf_allocate_ifunc_dyn_relocs (info, h, &eh->dyn_relocs,
-                                               plt_entry_size,
+					       &htab->readonly_dynrelocs_against_ifunc,
+					       plt_entry_size,
 					       plt_entry_size, 4);
   /* Don't create the PLT entry if there are only function pointer
      relocations which can be resolved at run-time.  */
@@ -3553,8 +3558,7 @@ elf_i386_size_dynamic_sections (bfd *output_bfd, struct bfd_link_info *info)
 
 	  if ((info->flags & DF_TEXTREL) != 0)
 	    {
-	      if ((elf_tdata (output_bfd)->has_gnu_symbols
-		   & elf_gnu_symbol_ifunc) == elf_gnu_symbol_ifunc)
+	      if (htab->readonly_dynrelocs_against_ifunc)
 		{
 		  info->callbacks->einfo
 		    (_("%P%X: read-only segment has dynamic IFUNC relocations; recompile with -fPIC\n"));
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index dc7738a..b58a699 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -900,6 +900,10 @@ struct elf_x86_64_link_hash_table
   bfd_vma next_jump_slot_index;
   /* The index of the next R_X86_64_IRELATIVE entry in .rela.plt.  */
   bfd_vma next_irelative_index;
+
+  /* TRUE if there are dynamic relocs against IFUNC symbols that apply
+     to read-only sections.  */
+  bfd_boolean readonly_dynrelocs_against_ifunc;
 };
 
 /* Get the x86-64 ELF linker hash table from a link_info structure.  */
@@ -2659,6 +2663,7 @@ elf_x86_64_allocate_dynrelocs (struct elf_link_hash_entry *h, void * inf)
     {
       if (_bfd_elf_allocate_ifunc_dyn_relocs (info, h,
 					      &eh->dyn_relocs,
+					      &htab->readonly_dynrelocs_against_ifunc,
 					      plt_entry_size,
 					      plt_entry_size,
 					      GOT_ENTRY_SIZE))
@@ -3899,8 +3904,7 @@ elf_x86_64_size_dynamic_sections (bfd *output_bfd,
 
 	  if ((info->flags & DF_TEXTREL) != 0)
 	    {
-	      if ((elf_tdata (output_bfd)->has_gnu_symbols
-		   & elf_gnu_symbol_ifunc) == elf_gnu_symbol_ifunc)
+	      if (htab->readonly_dynrelocs_against_ifunc)
 		{
 		  info->callbacks->einfo
 		    (_("%P%X: read-only segment has dynamic IFUNC relocations; recompile with -fPIC\n"));
diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index d14b734..5b9878e 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -8249,6 +8249,7 @@ elfNN_aarch64_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h,
       && h->def_regular)
     return _bfd_elf_allocate_ifunc_dyn_relocs (info, h,
 					       &eh->dyn_relocs,
+					       NULL,
 					       htab->plt_entry_size,
 					       htab->plt_header_size,
 					       GOT_ENTRY_SIZE);
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 2b7a09e..c6708ca 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -390,6 +390,8 @@ run_dump_test "pr17935-1"
 run_dump_test "pr17935-2"
 run_dump_test "pr18801"
 run_dump_test "pr18815"
+run_dump_test "pr19939a"
+run_dump_test "pr19939b"
 
 proc undefined_weak {cflags ldflags} {
     set testname "Undefined weak symbol"
diff --git a/ld/testsuite/ld-i386/pr19939.s b/ld/testsuite/ld-i386/pr19939.s
new file mode 100644
index 0000000..fdd1c04
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr19939.s
@@ -0,0 +1,9 @@
+	.text
+selector:
+	movl	foo@GOT(%eax), %eax
+	mov	$bar, %ebx
+	ret
+	.type   selector, %gnu_indirect_function
+	.globl	bar
+bar:
+	jmp	selector@PLT
diff --git a/ld/testsuite/ld-i386/pr19939a.d b/ld/testsuite/ld-i386/pr19939a.d
new file mode 100644
index 0000000..5883cae
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr19939a.d
@@ -0,0 +1,4 @@
+#source: pr19939.s
+#as: --32
+#ld: -melf_i386 -shared -z defs
+#error: undefined reference to `foo'
diff --git a/ld/testsuite/ld-i386/pr19939b.d b/ld/testsuite/ld-i386/pr19939b.d
new file mode 100644
index 0000000..5124ae1
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr19939b.d
@@ -0,0 +1,8 @@
+#source: pr19939.s
+#as: --32
+#ld: -melf_i386 -shared
+#readelf: -d --wide
+
+#...
+.*\(TEXTREL\).*
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr19939.s b/ld/testsuite/ld-x86-64/pr19939.s
new file mode 100644
index 0000000..7eadaaa
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr19939.s
@@ -0,0 +1,9 @@
+	.text
+selector:
+	movq	foo@GOTPCREL(%rip), %rax
+	movabs	$bar, %rbx
+	ret
+	.type   selector, %gnu_indirect_function
+	.globl	bar
+bar:
+	jmp	selector@PLT
diff --git a/ld/testsuite/ld-x86-64/pr19939a.d b/ld/testsuite/ld-x86-64/pr19939a.d
new file mode 100644
index 0000000..af5b976
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr19939a.d
@@ -0,0 +1,4 @@
+#source: pr19939.s
+#as: --64
+#ld: -melf_x86_64 -shared -z defs
+#error: undefined reference to `foo'
diff --git a/ld/testsuite/ld-x86-64/pr19939b.d b/ld/testsuite/ld-x86-64/pr19939b.d
new file mode 100644
index 0000000..2616151
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr19939b.d
@@ -0,0 +1,8 @@
+#source: pr19939.s
+#as: --64
+#ld: -melf_x86_64 -shared
+#readelf: -d --wide
+
+#...
+.*\(TEXTREL\).*
+#pass
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 603ef37..8abccd4 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -438,6 +438,8 @@ run_dump_test "pr19609-7a"
 run_dump_test "pr19609-7b"
 run_dump_test "pr19609-7c"
 run_dump_test "pr19609-7d"
+run_dump_test "pr19939a"
+run_dump_test "pr19939b"
 
 proc undefined_weak {cflags ldflags} {
     set testname "Undefined weak symbol"
-- 
2.5.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]