[2/6] Always initialise ARM got entries in final_link_relocate

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Current linkers generate dynamic GOT relocations for global-but-locally-
binding symbols in executables.  E.g.:

	.globl	foo
	.type	fo,%function
foo:
	ldr	r1,1f
1:
	.word	foo(GOT)
	.size	foo,.-foo

generates a dynamic R_ARM_GLOB_DAT entry for foo (even though, of course,
all other references to foo, such as through absolute accesses or calls,
would not have dynamic reloctions).  This behaviour isn't specific to ARM,
but it seems like unnecessary bloat.

The reason I looked at this is that we currently have a rather awkward
split in the way we initialise GOT entries.  We initialise GOT entries for
dynamic symbols in elf32_arm_finish_dynamic_symbol, but we initalise all
other GOT entries in elf32_arm_final_link_relocate.  We then have to be
careful which case we're dealing with:

	  if (! WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, info->shared, h)
	      || (info->shared
		  && SYMBOL_REFERENCES_LOCAL (info, h))
	      || (ELF_ST_VISIBILITY (h->other)
		  && h->root.type == bfd_link_hash_undefweak))
	    {
	      /* This is actually a static link, or it is a -Bsymbolic link
		 and the symbol is defined locally.  We must initialize this
		 entry in the global offset table.  Since the offset must
		 always be a multiple of 4, we use the least significant bit
		 to record whether we have initialized it already.

		 When doing a dynamic link, we create a .rel(a).got relocation
		 entry to initialize the value.  This is done in the
		 finish_dynamic_symbol routine.  */

This gets even more complicated with ifuncs, where we may need irelative
relocations.  It seemed easier to handle everything in one place, and to
fix the unnecessary relocs described above while there.

The patch also includes a testcase (unresolved-1) to show why the
dynamic_sections_created condition is important.  We already pass
the test, but I thought it was worth including to make sure that
we don't regress in future.

Tested on arm-linux-gnueabi.  OK to install?

Richard


bfd/
	* elf32-arm.c (elf32_arm_final_link_relocate): Always fill in the
	GOT entry here, rather than leaving it to finish_dynamic_symbol.
	Only create a dynamic relocation for local references if
	info->shared.
	(allocate_dynrelocs_for_symbol): Update dynamic relocation
	allocation accordingly.
	(elf32_arm_finish_dynamic_symbol): Don't initialise the GOT entry here.

ld/testsuite/
	* ld-arm/exec-got-1a.s, ld-arm/exec-got-1b.s, ld-arm/exec-got-1.d,
	ld-arm/unresolved-1.s, ld-arm/unresolved-1.d,
	ld-arm/unresolved-1-dyn.d: New tests.
	* ld-arm/arm-elf.exp: Run them.

Index: bfd/elf32-arm.c
===================================================================
--- bfd/elf32-arm.c	2011-03-04 10:41:18.000000000 +0000
+++ bfd/elf32-arm.c	2011-03-04 15:57:59.000000000 +0000
@@ -8278,45 +8278,67 @@ elf32_arm_final_link_relocate (reloc_how
       if (h != NULL)
 	{
 	  bfd_vma off;
-	  bfd_boolean dyn;
 
 	  off = h->got.offset;
 	  BFD_ASSERT (off != (bfd_vma) -1);
-	  dyn = globals->root.dynamic_sections_created;
+	  if ((off & 1) != 0)
+	    {
+	      /* We have already processsed one GOT relocation against
+		 this symbol.  */
+	      off &= ~1;
+	      if (globals->root.dynamic_sections_created
+		  && !SYMBOL_REFERENCES_LOCAL (info, h))
+		*unresolved_reloc_p = FALSE;
+	    }
+	  else
+	    {
+	      Elf_Internal_Rela outrel;
 
-	  if (! WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, info->shared, h)
-	      || (info->shared
-		  && SYMBOL_REFERENCES_LOCAL (info, h))
-	      || (ELF_ST_VISIBILITY (h->other)
-		  && h->root.type == bfd_link_hash_undefweak))
-	    {
-	      /* This is actually a static link, or it is a -Bsymbolic link
-		 and the symbol is defined locally.  We must initialize this
-		 entry in the global offset table.  Since the offset must
-		 always be a multiple of 4, we use the least significant bit
-		 to record whether we have initialized it already.
-
-		 When doing a dynamic link, we create a .rel(a).got relocation
-		 entry to initialize the value.  This is done in the
-		 finish_dynamic_symbol routine.  */
-	      if ((off & 1) != 0)
-		off &= ~1;
+	      if (!SYMBOL_REFERENCES_LOCAL (info, h))
+		{
+		  /* If the symbol doesn't resolve locally in a static
+		     object, we have an undefined reference.  If the
+		     symbol doesn't resolve locally in a dynamic object,
+		     it should be resolved by the dynamic linker.  */
+		  if (globals->root.dynamic_sections_created)
+		    {
+		      outrel.r_info = ELF32_R_INFO (h->dynindx, R_ARM_GLOB_DAT);
+		      *unresolved_reloc_p = FALSE;
+		    }
+		  else
+		    outrel.r_info = 0;
+		  outrel.r_addend = 0;
+		}
 	      else
 		{
-		  /* If we are addressing a Thumb function, we need to
-		     adjust the address by one, so that attempts to
-		     call the function pointer will correctly
-		     interpret it as Thumb code.  */
+		  if (info->shared)
+		    outrel.r_info = ELF32_R_INFO (0, R_ARM_RELATIVE);
+		  else
+		    outrel.r_info = 0;
+		  outrel.r_addend = value;
 		  if (sym_flags == STT_ARM_TFUNC)
-		    value |= 1;
+		    outrel.r_addend |= 1;
+		}
 
-		  bfd_put_32 (output_bfd, value, sgot->contents + off);
-		  h->got.offset |= 1;
+	      /* The GOT entry is initialized to zero by default.
+		 See if we should install a different value.  */
+	      if (outrel.r_addend != 0
+		  && (outrel.r_info == 0 || globals->use_rel))
+		{
+		  bfd_put_32 (output_bfd, outrel.r_addend,
+			      sgot->contents + off);
+		  outrel.r_addend = 0;
 		}
-	    }
-	  else
-	    *unresolved_reloc_p = FALSE;
 
+	      if (outrel.r_info != 0)
+		{
+		  outrel.r_offset = (sgot->output_section->vma
+				     + sgot->output_offset
+				     + off);
+		  elf32_arm_add_dynreloc (output_bfd, info, srelgot, &outrel);
+		}
+	      h->got.offset |= 1;
+	    }
 	  value = sgot->output_offset + off;
 	}
       else
@@ -12222,10 +12244,14 @@ allocate_dynrelocs (struct elf_link_hash
 	      if ((tls_type & GOT_TLS_GD) && indx != 0)  
 		elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1);
 	    }
-	  else if ((ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
-		    || h->root.type != bfd_link_hash_undefweak)
-		   && (info->shared
-	    	   || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h)))
+	  else if (!SYMBOL_REFERENCES_LOCAL (info, h))
+	    {
+	      if (htab->root.dynamic_sections_created)
+		/* Reserve room for the GOT entry's R_ARM_GLOB_DAT relocation.  */
+		elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1);
+	    }
+	  else if (info->shared)
+	    /* Reserve room for the GOT entry's R_ARM_RELATIVE relocation.  */
 	    elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1);
 	}
     }
@@ -13016,53 +13042,6 @@ elf32_arm_finish_dynamic_symbol (bfd * o
 	}
     }
 
-  if (h->got.offset != (bfd_vma) -1
-      && (! GOT_TLS_GD_ANY_P (elf32_arm_hash_entry (h)->tls_type)) 
-      && (elf32_arm_hash_entry (h)->tls_type & GOT_TLS_IE) == 0)
-    {
-      asection * sgot;
-      asection * srel;
-      Elf_Internal_Rela rel;
-      bfd_vma offset;
-
-      /* This symbol has an entry in the global offset table.  Set it
-	 up.  */
-      sgot = htab->root.sgot;
-      srel = htab->root.srelgot;
-      BFD_ASSERT (sgot != NULL && srel != NULL);
-
-      offset = (h->got.offset & ~(bfd_vma) 1);
-      rel.r_addend = 0;
-      rel.r_offset = (sgot->output_section->vma
-		      + sgot->output_offset
-		      + offset);
-
-      /* If this is a static link, or it is a -Bsymbolic link and the
-	 symbol is defined locally or was forced to be local because
-	 of a version file, we just want to emit a RELATIVE reloc.
-	 The entry in the global offset table will already have been
-	 initialized in the relocate_section function.  */
-      if (info->shared
-	  && SYMBOL_REFERENCES_LOCAL (info, h))
-	{
-	  BFD_ASSERT ((h->got.offset & 1) != 0);
-	  rel.r_info = ELF32_R_INFO (0, R_ARM_RELATIVE);
-	  if (!htab->use_rel)
-	    {
-	      rel.r_addend = bfd_get_32 (output_bfd, sgot->contents + offset);
-	      bfd_put_32 (output_bfd, (bfd_vma) 0, sgot->contents + offset);
-	    }
-	}
-      else
-	{
-	  BFD_ASSERT ((h->got.offset & 1) == 0);
-	  bfd_put_32 (output_bfd, (bfd_vma) 0, sgot->contents + offset);
-	  rel.r_info = ELF32_R_INFO (h->dynindx, R_ARM_GLOB_DAT);
-	}
-
-      elf32_arm_add_dynreloc (output_bfd, info, srel, &rel);
-    }
-
   if (h->needs_copy)
     {
       asection * s;
Index: ld/testsuite/ld-arm/exec-got-1a.s
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/exec-got-1a.s	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,5 @@
+	.globl	foo
+	.type	foo,%object
+	.size	foo,4
+	.data
+foo:	.word	1
Index: ld/testsuite/ld-arm/exec-got-1b.s
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/exec-got-1b.s	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,10 @@
+	.globl	_start
+	.type	_start,%function
+_start:
+	ldr	r1,1f
+	ldr	r1,2f
+1:
+	.word	foo(GOT)
+2:
+	.word	_start(GOT)
+	.size	_start,.-_start
Index: ld/testsuite/ld-arm/exec-got-1.d
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/exec-got-1.d	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,4 @@
+
+Relocation section '\.rel\.dyn' .*
+ Offset .*
+.* R_ARM_GLOB_DAT * 00000000 * foo
Index: ld/testsuite/ld-arm/unresolved-1.s
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/unresolved-1.s	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,6 @@
+	.globl	_start
+_start:
+	ldr	r4,1f
+	mov	pc,lr
+1:
+	.word	foo(GOT)
Index: ld/testsuite/ld-arm/unresolved-1.d
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/unresolved-1.d	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,8 @@
+#ld: --warn-unresolved
+#warning: \(\.text\+0x8\): warning: undefined reference to `foo'
+#objdump: -sj.rel.dyn -sj.got
+
+.*
+
+Contents of section \.got:
+ *[^ ]* 00000000 00000000 00000000 00000000  .*
Index: ld/testsuite/ld-arm/unresolved-1-dyn.d
===================================================================
--- /dev/null	2011-02-21 12:47:04.267827113 +0000
+++ ld/testsuite/ld-arm/unresolved-1-dyn.d	2011-03-04 12:57:49.000000000 +0000
@@ -0,0 +1,8 @@
+#source: unresolved-1.s
+#ld: --warn-unresolved tmpdir/mixed-lib.so
+#warning: \(\.text\+0x8\): warning: undefined reference to `foo'
+#readelf: -r
+
+Relocation section '\.rel\.dyn' .*
+ Offset .*
+.* R_ARM_GLOB_DAT +00000000 +foo
Index: ld/testsuite/ld-arm/arm-elf.exp
===================================================================
--- ld/testsuite/ld-arm/arm-elf.exp	2011-03-04 10:40:28.000000000 +0000
+++ ld/testsuite/ld-arm/arm-elf.exp	2011-03-04 15:57:05.000000000 +0000
@@ -295,6 +295,14 @@ set armelftests {
     {"Data only mapping symbols" "-T data-only-map.ld -Map map" "" {data-only-map.s}
      {{objdump -dr data-only-map.d}}
      "data-only-map"}
+    {"GOT relocations in executables (setup)" "-shared"
+     "" {exec-got-1a.s}
+     {}
+     "exec-got-1.so"}
+    {"GOT relocations in executables" "tmpdir/exec-got-1.so"
+     "" {exec-got-1b.s}
+     {{readelf --relocs exec-got-1.d}}
+     "exec-got-1"}
 }
 
 run_ld_link_tests $armelftests
@@ -582,3 +590,5 @@ run_dump_test "attr-merge-vfp-5r"
 run_dump_test "attr-merge-vfp-6"
 run_dump_test "attr-merge-vfp-6r"
 run_dump_test "attr-merge-incompatible"
+run_dump_test "unresolved-1"
+run_dump_test "unresolved-1-dyn"