Re: [PATCH] Fix PR ld/17277: bogus dynamic relocs and TEXTREL for ARM PC-relative relocs

[Roland McGrath <mcgrathr at google dot com>]

On Sun, Aug 17, 2014 at 4:30 PM, Alan Modra <amodra@gmail.com> wrote:
> It seems to me the problem here is that may_become_dynamic_p should
> not be set for pc_relative relocations against a local symbol.

Thanks for the pointer.  That makes lots of sense: the tests controlling
that were the other places that had been explicitly checking for just
R_ARM_REL32{,_NOI} and it seems right that all those tests should be
consistent (now all changed to check howto->pc_relative).

I've extended my test case to cover the local-symbol case as well.  The new
version of the change passes that new test and still has no regressions.
Likewise for the 2.24 backport.

OK for trunk and 2.24 now?


Thanks,
Roland


bfd/
2014-08-19  Roland McGrath  <mcgrathr@google.com>

	PR ld/17277
	* elf32-arm.c (elf32_arm_check_relocs): Increment P->pc_count for
	all reloc types with pc_relative set in the howto, not just for
	R_ARM_REL32 and R_ARM_REL32_NOI.
	(allocate_dynrelocs_for_symbol): Update comment.
	(elf32_arm_gc_sweep_hook): For all reloc types with pc_relative
	set in the howto, set call_reloc_p and may_need_local_target_p but
	not may_become_dynamic_p; not only for R_ARM_REL32 and R_ARM_REL32_NOI.
	(elf32_arm_check_relocs): Likewise.

ld/testsuite/
2014-08-19  Roland McGrath  <mcgrathr@google.com>

	PR ld/17277
	* ld-arm/pcrel-shared.s: New file.
	* ld-arm/pcrel-shared.rd: New file.
	* ld-arm/arm-elf.exp (armelftests_common): Add it.

--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -12500,7 +12500,7 @@ elf32_arm_gc_sweep_hook (bfd *                     abfd,
 	      && (sec->flags & SEC_ALLOC) != 0)
 	    {
 	      if (h == NULL
-		  && (r_type == R_ARM_REL32 || r_type == R_ARM_REL32_NOI))
+		  && elf32_arm_howto_from_type (r_type)->pc_relative)
 		{
 		  call_reloc_p = TRUE;
 		  may_need_local_target_p = TRUE;
@@ -12826,7 +12826,7 @@ elf32_arm_check_relocs (bfd *abfd, struct
bfd_link_info *info,
 		&& (sec->flags & SEC_ALLOC) != 0)
 	      {
 		if (h == NULL
-		    && (r_type == R_ARM_REL32 || r_type == R_ARM_REL32_NOI))
+		    && elf32_arm_howto_from_type (r_type)->pc_relative)
 		  {
 		    /* In shared libraries and relocatable executables,
 		       we treat local relative references as calls;
@@ -12972,7 +12972,7 @@ elf32_arm_check_relocs (bfd *abfd, struct
bfd_link_info *info,
 	      p->pc_count = 0;
 	    }

-	  if (r_type == R_ARM_REL32 || r_type == R_ARM_REL32_NOI)
+	  if (elf32_arm_howto_from_type (r_type)->pc_relative)
 	    p->pc_count += 1;
 	  p->count += 1;
 	}
@@ -13551,12 +13551,12 @@ allocate_dynrelocs_for_symbol (struct
elf_link_hash_entry *h, void * inf)

   if (info->shared || htab->root.is_relocatable_executable)
     {
-      /* The only relocs that use pc_count are R_ARM_REL32 and
-	 R_ARM_REL32_NOI, which will appear on something like
-	 ".long foo - .".  We want calls to protected symbols to resolve
-	 directly to the function rather than going via the plt.  If people
-	 want function pointer comparisons to work as expected then they
-	 should avoid writing assembly like ".long foo - .".  */
+      /* Relocs that use pc_count are PC-relative forms, which will appear
+	 on something like ".long foo - ." or "movw REG, foo - .".  We want
+	 calls to protected symbols to resolve directly to the function
+	 rather than going via the plt.  If people want function pointer
+	 comparisons to work as expected then they should avoid writing
+	 assembly like ".long foo - .".  */
       if (SYMBOL_CALLS_LOCAL (info, h))
 	{
 	  struct elf_dyn_relocs **pp;
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -203,6 +203,10 @@ set armelftests_common {
     {"EABI ABI flags ld -r" "-r" "" "-mfloat-abi=soft -meabi=5"
{eabi-soft-float.s}
      {{readelf -h eabi-soft-float-r.d}}
      "eabi-soft-float-r.o"}
+    {"PC-relative in -shared" "-shared" ""
+     "" {pcrel-shared.s}
+     {{readelf -dr pcrel-shared.rd}}
+     "pcrel-shared.so"}
 }

 set armelftests_nonacl {
--- /dev/null
+++ b/ld/testsuite/ld-arm/pcrel-shared.rd
@@ -0,0 +1,16 @@
+Dynamic section at offset 0x[0-9a-f]+ contains \d+ entries:
+\s+Tag\s+Type\s+Name/Value
+\s*0x[0-9a-f]+ \(HASH\).*
+\s*0x[0-9a-f]+ \(STRTAB\).*
+\s*0x[0-9a-f]+ \(SYMTAB\).*
+\s*0x[0-9a-f]+ \(STRSZ\).*
+\s*0x[0-9a-f]+ \(SYMENT\).*
+# Specifically want *not* to see here:
+# (REL)
+# (RELSZ)
+# (RELENT)
+# (TEXTREL)
+#...
+\s*0x[0-9a-f]+ \(NULL\).*
+
+There are no relocations in this file\.
--- /dev/null
+++ b/ld/testsuite/ld-arm/pcrel-shared.s
@@ -0,0 +1,25 @@
+# This tests PR ld/17277, wherein ld -shared for cross-section PC-relative
+# relocs (other than plain R_ARM_REL32, as in data) produce bogus dynamic
+# relocs and TEXTREL markers.
+
+	.syntax unified
+	.arm
+	.arch armv7-a
+
+	.text
+	.globl foo
+	.type foo,%function
+foo:	movw r0, #:lower16:symbol - 1f - 8
+	movt r0, #:upper16:symbol - 1f - 8
+1:	add r0, pc
+	@ And now a case with a local symbol.
+	movw r0, #:lower16:3f - 2f - 8
+	movt r0, #:upper16:3f - 2f - 8
+2:	add r0, pc
+	bx lr
+
+.data
+	.globl symbol
+	.hidden symbol
+symbol:	.long 23
+3:	.long 17