Re: [AArch64] PR18270, fix handling of GOT entry for local symbol

[Jiong Wang <wong dot kwongyuan dot tools at gmail dot com>]

2015-04-24 15:23 GMT+01:00 Nicholas Clifton <nickc@redhat.com>:
> Hi Jiong,
>
>> bfd/
>>    PR18270
>>    * elfnn-aarch64.c (elfNN_aarch64_size_dynamic): Count local symbol for
>>    GOT_NORMAL for both sgot/srelgot section.
>>    (elfNN_aarch64_final_link_relocate): Relocate against GOT entry
>>    address and generate necessary runtime relocation for GOT entry.
>
>
> Approved - but ... I do not like having calls to abort() inside a library.
> It rarely helps the user.  It would be better I think to replace them with
> calls to bdf_error_handler, providing an error message that tells the user
> that there has been an internal error.

Nick,

  Thanks for the review.

  Agree, we should give more error information here. I was copying similar
  code from x86.

  Updated the abort code into.

+       if (locals == NULL)
+         {
+           int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
+           (*_bfd_error_handler)
+             (_("%B: Local symbol descriptor table be NULL when applying "
+                "relocation %s against local symbol"),
+              input_bfd, elfNN_aarch64_howto_table[howto_index].name);
+           abort ();
+         }

  Now, it's giving more information while still use abort because,
    * Let the linker stop earlier, as missing target private local
symbol descriptor table
      is quite serious and internal error. It's unlike gas parse .s
file where we want the
      assembler to report as many errors as they can in one time.

    * I haven't found proper return value here.
elfNN_aarch64_final_link_relocate's
      return type is bfd_reloc_status_type, no proper value to return,
it most proper
      maybe "bfd_reloc_other" but still not very good. And I also
found current AArch64
      code return "FALSE" in this function which is wrong, "FALSE"
equals 0 which is
      bfd_reloc_ok, also we also invoke bfd_set_error so that the
linker finally know there
      is something wrong happen and stop in later stage.

 Have checked in the revised patch.

 Thanks,
 Regards,
 Jiong

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 2a6b6a4..367b5ec 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,11 @@
+2015-04-24  Jiong Wang  <jiong.wang@arm.com>
+
+	PR ld/18270
+	* elfnn-aarch64.c (elfNN_aarch64_size_dynamic): Count local symbol for
+	GOT_NORMAL for both sgot/srelgot section.
+	(elfNN_aarch64_final_link_relocate): Relocate against GOT entry address
+	and generate necessary runtime relocation for GOT entry.
+
 2015-04-24  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR binutils/18209
diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index c98987b..c252b13 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -4447,10 +4447,11 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,
   bfd_reloc_code_real_type new_bfd_r_type;
   unsigned long r_symndx;
   bfd_byte *hit_data = contents + rel->r_offset;
-  bfd_vma place;
+  bfd_vma place, off;
   bfd_signed_vma signed_addend;
   struct elf_aarch64_link_hash_table *globals;
   bfd_boolean weak_undef_p;
+  asection *base_got;
 
   globals = elf_aarch64_hash_table (info);
 
@@ -4490,8 +4491,6 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,
     {
       asection *plt;
       const char *name;
-      asection *base_got;
-      bfd_vma off;
 
       if ((input_section->flags & SEC_ALLOC) == 0
 	  || h->plt.offset == (bfd_vma) -1)
@@ -4866,6 +4865,58 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,
 	  value = _bfd_aarch64_elf_resolve_relocation (bfd_r_type, place, value,
 						       0, weak_undef_p);
 	}
+      else
+      {
+	struct elf_aarch64_local_symbol *locals
+	  = elf_aarch64_locals (input_bfd);
+
+	if (locals == NULL)
+	  {
+	    int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
+	    (*_bfd_error_handler)
+	      (_("%B: Local symbol descriptor table be NULL when applying "
+		 "relocation %s against local symbol"),
+	       input_bfd, elfNN_aarch64_howto_table[howto_index].name);
+	    abort ();
+	  }
+
+	off = symbol_got_offset (input_bfd, h, r_symndx);
+	base_got = globals->root.sgot;
+	bfd_vma got_entry_addr = (base_got->output_section->vma
+				  + base_got->output_offset + off);
+
+	if (!symbol_got_offset_mark_p (input_bfd, h, r_symndx))
+	  {
+	    bfd_put_64 (output_bfd, value, base_got->contents + off);
+
+	    if (info->shared)
+	      {
+		asection *s;
+		Elf_Internal_Rela outrel;
+
+		/* For local symbol, we have done absolute relocation in static
+		   linking stageh. While for share library, we need to update
+		   the content of GOT entry according to the share objects
+		   loading base address. So we need to generate a
+		   R_AARCH64_RELATIVE reloc for dynamic linker.  */
+		s = globals->root.srelgot;
+		if (s == NULL)
+		  abort ();
+
+		outrel.r_offset = got_entry_addr;
+		outrel.r_info = ELFNN_R_INFO (0, AARCH64_R (RELATIVE));
+		outrel.r_addend = value;
+		elf_append_rela (output_bfd, s, &outrel);
+	      }
+
+	    symbol_got_offset_mark (input_bfd, h, r_symndx);
+	  }
+
+	/* Update the relocation value to GOT entry addr as we have transformed
+	   the direct data access into indirect data access through GOT.  */
+	value = got_entry_addr;
+      }
+
       break;
 
     case BFD_RELOC_AARCH64_TLSGD_ADR_PAGE21:
@@ -7521,7 +7572,8 @@ elfNN_aarch64_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 		  htab->root.sgot->size += GOT_ENTRY_SIZE * 2;
 		}
 
-	      if (got_type & GOT_TLS_IE)
+	      if (got_type & GOT_TLS_IE
+		  || got_type & GOT_NORMAL)
 		{
 		  locals[i].got_offset = htab->root.sgot->size;
 		  htab->root.sgot->size += GOT_ENTRY_SIZE;
@@ -7531,10 +7583,6 @@ elfNN_aarch64_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 		{
 		}
 
-	      if (got_type == GOT_NORMAL)
-		{
-		}
-
 	      if (info->shared)
 		{
 		  if (got_type & GOT_TLSDESC_GD)
@@ -7547,7 +7595,8 @@ elfNN_aarch64_size_dynamic_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
 		  if (got_type & GOT_TLS_GD)
 		    htab->root.srelgot->size += RELOC_SIZE (htab) * 2;
 
-		  if (got_type & GOT_TLS_IE)
+		  if (got_type & GOT_TLS_IE
+		      || got_type & GOT_NORMAL)
 		    htab->root.srelgot->size += RELOC_SIZE (htab);
 		}
 	    }