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]

[AArch64] Allocate dynrelocs for other relocation types which may related with copy relocation


On 09/06/17 11:58, Jiong Wang wrote:
On 07/06/17 16:17, Jiong Wang wrote:

  The ELIMINATE_COPY_RELOCS has been set to true and it's
underlying infrastructure has been improved so that the COPY reloc elimination
at least working on absoluate relocations (ABS64) on AArch64 and testcases on
the PR has passed.
   There are several other relocation types might need similar supports as well,
I will try to post following up patches to clean up them.

Without the following up patches to cleanup some other PC-relative relocation types,  this patch
will leak PC relative relocation types in and cause incomplete support on copy relocation elimination.

This is revealed by some g++/libstdc++ test:
unresolvable R_AARCH64_ADR_PREL_PG_HI21 relocation against symbol `_ZTIi@@CXXABI_1.3'

I will revert this patch temporarily and will commit it later after the work and review on PC-relative support finished.

Above failure is caused by the current elfNN_aarch64_check_relocs only allocate
dynrelocs for R_AARCH64_NN.

If one symbol is reference both by R_AARCH64_NN and other relocation types, then
the requirements from the latter will be ignored.

For example, above failure was because the symbol "_ZTIi@@CXXABI_1.3" was
referenced by R_AARCH64_ABS_64 and R_AARCH64_ADR_PREL_PG_HI21.  The latter
reference requires LD to keep copy reloction, however because check_relocs
haven't allocated dynrelocs for R_AARCH64_ADR_PREL_PG_HI21, its impact on the
symbol handling was ignored.  So, later in elfNN_aarch64_adjust_dynamic_symbol,
LD had not known the symbol is reference by a pc-rel relocation against text
section that copy relcation should be generated.  Instead,
R_AARCH64_ADR_PREL_PG_HI21 relocation was propagated into runtime against
external symbol and with the wish the value of the external symbol will be
satisifed by shared library.  This then triggered unresolvable relocation error
on AArch64 as we don't expect unresolved symbol which binds externally to reach
the final relocation for pc-relative relocation types.

From my understanding, the BFD linker copy relocation elimination framwork
requires the backend to always allocate dynrelocs for all those relocation types
that are possible to introduce copy relocations.  These relocation types are
actually relocation types which are used to access external object in non-PIC
executable (related with forming address).

As on AArch64, for tiny model, they are:

  BFD_RELOC_AARCH64_ADR_LO21_PCREL
  BFD_RELOC_AARCH64_LD_LO19_PCREL

For small model, they are:

  BFD_RELOC_AARCH64_ADR_HI21_NC_PCREL:
  BFD_RELOC_AARCH64_ADR_HI21_PCREL:
  BFD_RELOC_AARCH64_ADD_LO12:
  BFD_RELOC_AARCH64_LDST128_LO12:
  BFD_RELOC_AARCH64_LDST16_LO12:
  BFD_RELOC_AARCH64_LDST32_LO12:
  BFD_RELOC_AARCH64_LDST64_LO12:
  BFD_RELOC_AARCH64_LDST8_LO12:
  BFD_RELOC_AARCH64_LD_LO19_PCREL:

For large model, they are:

  BFD_RELOC_AARCH64_MOVW_G0_NC:
  BFD_RELOC_AARCH64_MOVW_G1_NC:
  BFD_RELOC_AARCH64_MOVW_G2_NC:
  BFD_RELOC_AARCH64_MOVW_G3:


So, this patch does the following change:

  * Also allocate dynrelocs for above relocations in elfNN_aarch64_check_relocs,
    so that elfNN_aarch64_adjust_dynamic_symbol could get full symbol reference
    information.

  * In need_copy_relocation_p, return TRUE if there is any pc-relative
    relocation reference on the symbol.  This can make sure LD generate copy
    relocation instead of propagating it to runtime which doesn't have
    pc-relative support on AArch64.

  * In elfNN_aarch64_gc_sweep_hook, the relocation types that need decrease on
    h->plt.refcount is synced.

  * Use !bfd_link_pic instead of bfd_link_executable when checking copy relocation,
    as -pie is supposed to access external objects indirectly.


  After this patch, there is no regression on:

    cross & native check-gcc/g++/libstdc++/as/ld/binutils.
    build kernel/llvm/clang.

  After this patch, I noticed the following failures also get fixed:

    https://sourceware.org/ml/binutils/2017-04/msg00159.html

    FAIL: PR ld/21233 dynamic symbols with section GC (--undefined)
    FAIL: PR ld/21233 dynamic symbols with section GC (--require-defined)
    FAIL: PR ld/21233 dynamic symbols with section GC (EXTERN)


  The reason looks is the same as explained at:

    https://sourceware.org/ml/binutils/2017-04/msg00035.html

  OK for master?

  (This patch will be committed together with the prerequisite patch
   https://sourceware.org/ml/binutils/2017-06/msg00070.html)

bfd/
2017-06-15  Jiong Wang  <jiong.wang@arm.com>

        PR ld/21532
        * bfd/elfnn-aarch64.c (alias_readonly_dynrelocs): Improve comments.
        Handle pc-relative reference.  Rename to ...
        (need_copy_relocation_p): ... this.
        (elfNN_aarch64_adjust_dynamic_symbol): Use need_copy_relocation_p.
        (elfNN_aarch64_check_relocs): Allocate dynrelocs for other relocation
        types that may be used to form address.
        (elfNN_aarch64_gc_sweep_hook): Sync the relocation types that need
        decrease of h->plt.refcount.

ld/
        * testsuite/ld-aarch64/copy-reloc-exe-2.s: New test source.
        * testsuite/ld-aarch64/copy-reloc-so.s: Define new global objects.
        * testsuite/ld-aarch64/copy-reloc-2.d: New test.
        * testsuite/ld-aarch64/aarch64-elf.exp: Run new test.


diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 542c98a..e27f067 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -6859,15 +6859,22 @@ elfNN_aarch64_gc_sweep_hook (bfd *abfd,
 	    h->plt.refcount -= 1;
 	  break;
 
+	case BFD_RELOC_AARCH64_ADD_LO12:
 	case BFD_RELOC_AARCH64_ADR_HI21_NC_PCREL:
 	case BFD_RELOC_AARCH64_ADR_HI21_PCREL:
 	case BFD_RELOC_AARCH64_ADR_LO21_PCREL:
+	case BFD_RELOC_AARCH64_LDST128_LO12:
+	case BFD_RELOC_AARCH64_LDST16_LO12:
+	case BFD_RELOC_AARCH64_LDST32_LO12:
+	case BFD_RELOC_AARCH64_LDST64_LO12:
+	case BFD_RELOC_AARCH64_LDST8_LO12:
+	case BFD_RELOC_AARCH64_LD_LO19_PCREL:
 	case BFD_RELOC_AARCH64_MOVW_G0_NC:
 	case BFD_RELOC_AARCH64_MOVW_G1_NC:
 	case BFD_RELOC_AARCH64_MOVW_G2_NC:
 	case BFD_RELOC_AARCH64_MOVW_G3:
 	case BFD_RELOC_AARCH64_NN:
-	  if (h != NULL && bfd_link_executable (info))
+	  if (h != NULL && !bfd_link_pic (info))
 	    {
 	      if (h->plt.refcount > 0)
 		h->plt.refcount -= 1;
@@ -6882,18 +6889,24 @@ elfNN_aarch64_gc_sweep_hook (bfd *abfd,
   return TRUE;
 }
 
-/* Return true if we have dynamic relocs against EH or any of its weak
-   aliases, that apply to read-only sections.  */
+/* Return true if we need copy relocation against EH.  */
 
 static bfd_boolean
-alias_readonly_dynrelocs (struct elf_aarch64_link_hash_entry *eh)
+need_copy_relocation_p (struct elf_aarch64_link_hash_entry *eh)
 {
   struct elf_dyn_relocs *p;
   asection *s;
 
   for (p = eh->dyn_relocs; p != NULL; p = p->next)
     {
+      /* If there is any pc-relative reference, we need to keep copy relocation
+	 to avoid propagating the relocation into runtime that current glibc
+	 does not support.  */
+      if (p->pc_count)
+	return TRUE;
+
       s = p->sec->output_section;
+      /* Need copy relocation if it's against read-only section.  */
       if (s != NULL && (s->flags & SEC_READONLY) != 0)
 	return TRUE;
     }
@@ -6980,7 +6993,7 @@ elfNN_aarch64_adjust_dynamic_symbol (struct bfd_link_info *info,
       /* If we didn't find any dynamic relocs in read-only sections, then
 	 we'll be keeping the dynamic relocs and avoiding the copy reloc.  */
       eh = (struct elf_aarch64_link_hash_entry *) h;
-      if (eh->dyn_relocs && !alias_readonly_dynrelocs (eh))
+      if (!need_copy_relocation_p (eh))
 	{
 	  h->non_got_ref = 0;
 	  return TRUE;
@@ -7241,6 +7254,41 @@ elfNN_aarch64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 
       switch (bfd_r_type)
 	{
+	case BFD_RELOC_AARCH64_MOVW_G0_NC:
+	case BFD_RELOC_AARCH64_MOVW_G1_NC:
+	case BFD_RELOC_AARCH64_MOVW_G2_NC:
+	case BFD_RELOC_AARCH64_MOVW_G3:
+	  if (bfd_link_pic (info))
+	    {
+	      int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
+	      _bfd_error_handler
+		/* xgettext:c-format */
+		(_("%B: relocation %s against `%s' can not be used when making "
+		   "a shared object; recompile with -fPIC"),
+		 abfd, elfNN_aarch64_howto_table[howto_index].name,
+		 (h) ? h->root.root.string : "a local symbol");
+	      bfd_set_error (bfd_error_bad_value);
+	      return FALSE;
+	    }
+	  /* Fall through.  */
+
+	case BFD_RELOC_AARCH64_16_PCREL:
+	case BFD_RELOC_AARCH64_32_PCREL:
+	case BFD_RELOC_AARCH64_64_PCREL:
+	case BFD_RELOC_AARCH64_ADD_LO12:
+	case BFD_RELOC_AARCH64_ADR_HI21_NC_PCREL:
+	case BFD_RELOC_AARCH64_ADR_HI21_PCREL:
+	case BFD_RELOC_AARCH64_ADR_LO21_PCREL:
+	case BFD_RELOC_AARCH64_LDST128_LO12:
+	case BFD_RELOC_AARCH64_LDST16_LO12:
+	case BFD_RELOC_AARCH64_LDST32_LO12:
+	case BFD_RELOC_AARCH64_LDST64_LO12:
+	case BFD_RELOC_AARCH64_LDST8_LO12:
+	case BFD_RELOC_AARCH64_LD_LO19_PCREL:
+	  if (h == NULL || bfd_link_pic (info))
+	    break;
+	  /* Fall through.  */
+
 	case BFD_RELOC_AARCH64_NN:
 
 	  /* We don't need to handle relocs into sections not going into
@@ -7263,7 +7311,17 @@ elfNN_aarch64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 		/* If on the other hand, we are creating an executable, we
 		   may need to keep relocations for symbols satisfied by a
 		   dynamic library if we manage to avoid copy relocs for the
-		   symbol.  */
+		   symbol.
+
+		   NOTE: Currently, there is no support of copy relocs
+		   elimination on pc-relative relocation types, because there is
+		   no dynamic relocation support for them in glibc.  We still
+		   record the dynamic symbol reference for them.  This is
+		   because one symbol may be referenced by both absolute
+		   relocation (for example, BFD_RELOC_AARCH64_NN) and
+		   pc-relative relocation.  We need full symbol reference
+		   information to make correct decision later in
+		   elfNN_aarch64_adjust_dynamic_symbol.  */
 		|| (ELIMINATE_COPY_RELOCS
 		    && !bfd_link_pic (info)
 		    && h != NULL
@@ -7274,6 +7332,7 @@ elfNN_aarch64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	  {
 	    struct elf_dyn_relocs *p;
 	    struct elf_dyn_relocs **head;
+	    int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
 
 	    /* We must copy these reloc types into the output file.
 	       Create a reloc section in dynobj and make room for
@@ -7337,6 +7396,8 @@ elfNN_aarch64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 
 	    p->count += 1;
 
+	    if (elfNN_aarch64_howto_table[howto_index].pc_relative)
+	      p->pc_count += 1;
 	  }
 	  break;
 
@@ -7440,44 +7501,6 @@ elfNN_aarch64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	    break;
 	  }
 
-	case BFD_RELOC_AARCH64_MOVW_G0_NC:
-	case BFD_RELOC_AARCH64_MOVW_G1_NC:
-	case BFD_RELOC_AARCH64_MOVW_G2_NC:
-	case BFD_RELOC_AARCH64_MOVW_G3:
-	  if (bfd_link_pic (info))
-	    {
-	      int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
-	      _bfd_error_handler
-		/* xgettext:c-format */
-		(_("%B: relocation %s against `%s' can not be used when making "
-		   "a shared object; recompile with -fPIC"),
-		 abfd, elfNN_aarch64_howto_table[howto_index].name,
-		 (h) ? h->root.root.string : "a local symbol");
-	      bfd_set_error (bfd_error_bad_value);
-	      return FALSE;
-	    }
-	  /* Fall through.  */
-
-	case BFD_RELOC_AARCH64_ADR_HI21_NC_PCREL:
-	case BFD_RELOC_AARCH64_ADR_HI21_PCREL:
-	case BFD_RELOC_AARCH64_ADR_LO21_PCREL:
-	  if (h != NULL && bfd_link_executable (info))
-	    {
-	      /* If this reloc is in a read-only section, we might
-		 need a copy reloc.  We can't check reliably at this
-		 stage whether the section is read-only, as input
-		 sections have not yet been mapped to output sections.
-		 Tentatively set the flag for now, and correct in
-		 adjust_dynamic_symbol.  */
-	      h->non_got_ref = 1;
-	      h->plt.refcount += 1;
-	      h->pointer_equality_needed = 1;
-	    }
-	  /* FIXME:: RR need to handle these in shared libraries
-	     and essentially bomb out as these being non-PIC
-	     relocations in shared libraries.  */
-	  break;
-
 	case BFD_RELOC_AARCH64_CALL26:
 	case BFD_RELOC_AARCH64_JUMP26:
 	  /* If this is a local symbol then we resolve it
diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index 66e7e64..f171f6f 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -335,6 +335,8 @@ set aarch64elflinktests {
     {} "copy-reloc-so.so"}
   {"ld-aarch64/exe with copy relocation" "-e0 tmpdir/copy-reloc-so.so" "" ""
     {copy-reloc-exe.s} {{objdump -R copy-reloc.d}} "copy-reloc"}
+  {"ld-aarch64/exe with copy relocation 2" "-e0 tmpdir/copy-reloc-so.so" "" ""
+    {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
   {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
     {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
   {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
diff --git a/ld/testsuite/ld-aarch64/copy-reloc-2.d b/ld/testsuite/ld-aarch64/copy-reloc-2.d
new file mode 100644
index 0000000..87ddccd
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/copy-reloc-2.d
@@ -0,0 +1,7 @@
+.*
+DYNAMIC RELOCATION RECORDS
+OFFSET.*TYPE.*VALUE.*
+.*R_AARCH64_COPY.*global_[abcd]
+.*R_AARCH64_COPY.*global_[abcd]
+.*R_AARCH64_COPY.*global_[abcd]
+.*R_AARCH64_COPY.*global_[abcd]
diff --git a/ld/testsuite/ld-aarch64/copy-reloc-exe-2.s b/ld/testsuite/ld-aarch64/copy-reloc-exe-2.s
new file mode 100644
index 0000000..d83658c
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/copy-reloc-exe-2.s
@@ -0,0 +1,32 @@
+	# expect copy relocation for all these scenarios.
+	.global	p
+	.global	q
+	.global	r
+	.section	.data.rel.ro,"aw",%progbits
+	.align	3
+	.type	p, %object
+	.size	p, 8
+p:
+	.xword	global_a
+
+	.type	q, %object
+	.size	q, 8
+q:
+	.xword	global_b
+
+	.type	r, %object
+	.size	r, 8
+r:
+	# Any pc-rel relocation as no dynamic linker support on AArch64.
+	.xword	global_c - .
+
+	.text
+	.global main
+main:
+	# Symbols are referenced by any other relocation against read-only
+	# section.
+	movz x0, :abs_g0_nc:global_a
+	adrp x1, global_b
+	# pc-rel.
+	adrp x2, global_d
+	add x2, x2, #:lo12:global_c
diff --git a/ld/testsuite/ld-aarch64/copy-reloc-so.s b/ld/testsuite/ld-aarch64/copy-reloc-so.s
index 07ec44a..af40f69 100644
--- a/ld/testsuite/ld-aarch64/copy-reloc-so.s
+++ b/ld/testsuite/ld-aarch64/copy-reloc-so.s
@@ -1,6 +1,25 @@
 	.global global_a
 	.type	global_a, %object
 	.size	global_a, 4
+
+	.global global_b
+	.type	global_b, %object
+	.size	global_b, 4
+
+	.global global_c
+	.type	global_c, %object
+	.size	global_c, 4
+
+	.global global_d
+	.type	global_d, %object
+	.size	global_d, 4
+
 	.data
 global_a:
 	.word 0xcafedead
+global_b:
+	.word 0xcafecafe
+global_c:
+	.word 0xdeadcafe
+global_d:
+	.word 0xdeaddead

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