Re: PATCH: x86-64: Improve GOTPCREL relocation conversion

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Sep 5, 2017 at 11:04 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 10:21 AM, Steve Vormwald <sdvormwa@cray.com> wrote:
>> That looks great.
>>
>> Steven Vormwald
>> ________________________________________
>> From: H.J. Lu <hjl.tools@gmail.com>
>> Sent: Tuesday, September 5, 2017 12:19:52 PM
>> To: Steve Vormwald
>> Cc: Binutils
>> Subject: Re: Invalid R_X86_64_GOTPCREL -> R_X86_64_PC32 conversions with binutils 2.24/2.25?
>>
>> On Tue, Sep 5, 2017 at 9:51 AM, Steve Vormwald <sdvormwa@cray.com> wrote:
>>> I don't see any problems with adding an option to control this behavior.  The current error message is very confusing though when the object file in question doesn't contain any R_X86_64_PC32 relocations.  Is it possible to note that the error is/may be due to such a transformation, or at least hint that the user might try the --no-relax option?
>>>
>>
>> Good point.  How this?
>>
>> [hjl@gnu-6 pr18591]$ cat x.S
>> .hidden foo
>> .comm pad,0x80000000,8
>> .comm foo,8,8
>> .text
>> .globl bar
>> .type bar, @function
>> bar:
>> movq foo@GOTPCREL(%rip), %rax
>> [hjl@gnu-6 pr18591]$ make
>> gcc    -c -o x.o x.S
>> ./ld -z max-page-size=0x200000 -shared -o x.so x.o
>> ./ld: failed to convert GOTPCREL relocation; relink with --no-releax
>> make: *** [Makefile:25: x.so] Error 1
>> [hjl@gnu-6 pr18591]$
>
>
> This is the patch I am checking in.
>

Steve pointed out a typo.   Fixed here.


-- 
H.J.

From 67d04ab6a2b3a8f2485eaa2057c9404e023eab38 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 5 Sep 2017 10:41:47 -0700
Subject: [PATCH] x86-64: Improve GOTPCREL relocation conversion

When GOTPCREL relocation conversion leads to relocation overflow, we
may get a mysterious linker message, like

relocation truncated to fit: R_X86_64_32S against symbol `foo'

This patch changes the linker message to

failed to convert GOTPCREL relocation; relink with --no-relax

bfd/

	* elf64-x86-64.c (elf_x86_64_convert_load_reloc): Remove the sec
	argument.  Don't check relocation overflow.  Avoid relocation
	overflow if --no-relax is used.  Set converted_reloc on symbol
	if a GOTPCREL relocation is converted.
	(elf_x86_64_relocate_section): Issue a fatal error and suggest
	--no-relax if GOTPCREL relocation conversion leads to relocation
	overflow.
	* elfxx-x86.h (elf_x86_link_hash_entry): Add converted_reloc.

ld/

	* testsuite/ld-x86-64/pr19609-4e.d: Updated.
	* testsuite/ld-x86-64/pr19609-6a.d: Likewise.
---
 bfd/elf64-x86-64.c                  | 132 +++++++-----------------------------
 bfd/elfxx-x86.h                     |   5 ++
 ld/testsuite/ld-x86-64/pr19609-4e.d |   4 +-
 ld/testsuite/ld-x86-64/pr19609-6a.d |   2 +-
 4 files changed, 33 insertions(+), 110 deletions(-)

diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index fa295a5ca9..ddbf1330e4 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1438,7 +1438,7 @@ elf_x86_64_need_pic (struct bfd_link_info *info,
    instructions.  */
 
 static bfd_boolean
-elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
+elf_x86_64_convert_load_reloc (bfd *abfd,
 			       bfd_byte *contents,
 			       Elf_Internal_Rela *irel,
 			       struct elf_link_hash_entry *h,
@@ -1447,17 +1447,15 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
 {
   struct elf_x86_link_hash_table *htab;
   bfd_boolean is_pic;
-  bfd_boolean require_reloc_pc32;
+  bfd_boolean no_overflow;
   bfd_boolean relocx;
   bfd_boolean to_reloc_pc32;
   asection *tsec;
-  char symtype;
   bfd_signed_vma raddend;
   unsigned int opcode;
   unsigned int modrm;
   unsigned int r_type = ELF32_R_TYPE (irel->r_info);
   unsigned int r_symndx;
-  bfd_vma toff;
   bfd_vma roff = irel->r_offset;
 
   if (roff < (r_type == R_X86_64_REX_GOTPCRELX ? 3 : 2))
@@ -1474,10 +1472,8 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
   relocx = (r_type == R_X86_64_GOTPCRELX
 	    || r_type == R_X86_64_REX_GOTPCRELX);
 
-  /* TRUE if we can convert only to R_X86_64_PC32.  Enable it for
-     --no-relax.  */
-  require_reloc_pc32
-    = link_info->disable_target_specific_optimizations > 1;
+  /* TRUE if --no-relax is used.  */
+  no_overflow = link_info->disable_target_specific_optimizations > 1;
 
   r_symndx = htab->r_sym (irel->r_info);
 
@@ -1496,12 +1492,12 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
   /* We convert only to R_X86_64_PC32:
      1. Branch.
      2. R_X86_64_GOTPCREL since we can't modify REX byte.
-     3. require_reloc_pc32 is true.
+     3. no_overflow is true.
      4. PIC.
      */
   to_reloc_pc32 = (opcode == 0xff
 		   || !relocx
-		   || require_reloc_pc32
+		   || no_overflow
 		   || is_pic);
 
   /* Get the symbol referred to by the reloc.  */
@@ -1514,8 +1510,6 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
       if (isym->st_shndx == SHN_UNDEF)
 	return TRUE;
 
-      symtype = ELF_ST_TYPE (isym->st_info);
-
       if (isym->st_shndx == SHN_ABS)
 	tsec = bfd_abs_section_ptr;
       else if (isym->st_shndx == SHN_COMMON)
@@ -1524,8 +1518,6 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
 	tsec = &_bfd_elf_large_com_section;
       else
 	tsec = bfd_section_from_elf_index (abfd, isym->st_shndx);
-
-      toff = isym->st_value;
     }
   else
     {
@@ -1545,7 +1537,7 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
 	    {
 	      /* Skip for branch instructions since R_X86_64_PC32
 		 may overflow.  */
-	      if (require_reloc_pc32)
+	      if (no_overflow)
 		return TRUE;
 	    }
 	  else if (relocx)
@@ -1586,13 +1578,11 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
 			  && h->root.u.def.section == bfd_und_section_ptr))))
 	    {
 	      /* Skip since R_X86_64_32/R_X86_64_32S may overflow.  */
-	      if (require_reloc_pc32)
+	      if (no_overflow)
 		return TRUE;
 	      goto convert;
 	    }
 	  tsec = h->root.u.def.section;
-	  toff = h->root.u.def.value;
-	  symtype = h->type;
 	}
       else
 	return TRUE;
@@ -1603,92 +1593,12 @@ elf_x86_64_convert_load_reloc (bfd *abfd, asection *sec,
       && (elf_section_flags (tsec) & SHF_X86_64_LARGE) != 0)
     return TRUE;
 
-  /* We can only estimate relocation overflow for R_X86_64_PC32.  */
-  if (!to_reloc_pc32)
-    goto convert;
-
-  if (tsec->sec_info_type == SEC_INFO_TYPE_MERGE)
-    {
-      /* At this stage in linking, no SEC_MERGE symbol has been
-	 adjusted, so all references to such symbols need to be
-	 passed through _bfd_merged_section_offset.  (Later, in
-	 relocate_section, all SEC_MERGE symbols *except* for
-	 section symbols have been adjusted.)
-
-	 gas may reduce relocations against symbols in SEC_MERGE
-	 sections to a relocation against the section symbol when
-	 the original addend was zero.  When the reloc is against
-	 a section symbol we should include the addend in the
-	 offset passed to _bfd_merged_section_offset, since the
-	 location of interest is the original symbol.  On the
-	 other hand, an access to "sym+addend" where "sym" is not
-	 a section symbol should not include the addend;  Such an
-	 access is presumed to be an offset from "sym";  The
-	 location of interest is just "sym".  */
-      if (symtype == STT_SECTION)
-	toff += raddend;
-
-      toff = _bfd_merged_section_offset (abfd, &tsec,
-					 elf_section_data (tsec)->sec_info,
-					 toff);
-
-      if (symtype != STT_SECTION)
-	toff += raddend;
-    }
-  else
-    toff += raddend;
-
-  /* Don't convert if R_X86_64_PC32 relocation overflows.  */
-  if (tsec->output_section == sec->output_section)
-    {
-      if ((toff - roff + 0x80000000) > 0xffffffff)
-	return TRUE;
-    }
-  else
-    {
-      bfd_signed_vma distance;
-
-      /* At this point, we don't know the load addresses of TSEC
-	 section nor SEC section.  We estimate the distrance between
-	 SEC and TSEC.  We store the estimated distances in the
-	 compressed_size field of the output section, which is only
-	 used to decompress the compressed input section.  */
-      if (sec->output_section->compressed_size == 0)
-	{
-	  asection *asect;
-	  bfd_size_type size = 0;
-	  for (asect = link_info->output_bfd->sections;
-	       asect != NULL;
-	       asect = asect->next)
-	    /* Skip debug sections since compressed_size is used to
-	       compress debug sections.  */
-	    if ((asect->flags & SEC_DEBUGGING) == 0)
-	      {
-		asection *i;
-		for (i = asect->map_head.s;
-		     i != NULL;
-		     i = i->map_head.s)
-		  {
-		    size = align_power (size, i->alignment_power);
-		    size += i->size;
-		  }
-		asect->compressed_size = size;
-	      }
-	}
-
-      /* Don't convert GOTPCREL relocations if TSEC isn't placed
-	 after SEC.  */
-      distance = (tsec->output_section->compressed_size
-		  - sec->output_section->compressed_size);
-      if (distance < 0)
-	return TRUE;
+  /* Skip since R_X86_64_PC32/R_X86_64_32/R_X86_64_32S may overflow.  */
+  if (no_overflow)
+    return TRUE;
 
-      /* Take PT_GNU_RELRO segment into account by adding
-	 maxpagesize.  */
-      if ((toff + distance + get_elf_backend_data (abfd)->maxpagesize
-	   - roff + 0x80000000) > 0xffffffff)
-	return TRUE;
-    }
+  if (h != NULL)
+    ((struct elf_x86_link_hash_entry *) h)->converted_reloc = 1;
 
 convert:
   if (opcode == 0xff)
@@ -2467,7 +2377,7 @@ _bfd_x86_64_elf_convert_load (bfd *abfd, asection *sec,
 	continue;
 
       converted = FALSE;
-      if (!elf_x86_64_convert_load_reloc (abfd, sec, contents, irel, h,
+      if (!elf_x86_64_convert_load_reloc (abfd, contents, irel, h,
 					  &converted, link_info))
 	goto error_return;
 
@@ -4099,9 +4009,17 @@ check_relocation_error:
 	    }
 
 	  if (r == bfd_reloc_overflow)
-	    (*info->callbacks->reloc_overflow)
-	      (info, (h ? &h->root : NULL), name, howto->name,
-	       (bfd_vma) 0, input_bfd, input_section, rel->r_offset);
+	    {
+	      if (eh != NULL && eh->converted_reloc)
+		{
+		  info->callbacks->einfo
+		    (_("%F%P: failed to convert GOTPCREL relocation; relink with --no-relax\n"));
+		  return FALSE;
+		}
+	      (*info->callbacks->reloc_overflow)
+		(info, (h ? &h->root : NULL), name, howto->name,
+		 (bfd_vma) 0, input_bfd, input_section, rel->r_offset);
+	    }
 	  else
 	    {
 	      _bfd_error_handler
diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h
index 9c0dcbbe87..775e025477 100644
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -110,6 +110,11 @@ struct elf_x86_link_hash_entry
      is only used by x86-64.  */
   unsigned int needs_copy : 1;
 
+  /* TRUE if a symbol with GOTPCREL relocations which have been converted
+     to R_X86_64_PC32, R_X86_64_32 or R_X86_64_32S.  This is only used by
+     x86-64 for now.  */
+  unsigned int converted_reloc : 1;
+
   /* Reference count of C/C++ function pointer relocations in read-write
      section which can be resolved at run-time.  */
   bfd_signed_vma func_pointer_refcount;
diff --git a/ld/testsuite/ld-x86-64/pr19609-4e.d b/ld/testsuite/ld-x86-64/pr19609-4e.d
index 527fe5d9f4..f2634947d6 100644
--- a/ld/testsuite/ld-x86-64/pr19609-4e.d
+++ b/ld/testsuite/ld-x86-64/pr19609-4e.d
@@ -9,5 +9,5 @@
 Disassembly of section .text:
 
 0+70000000 <_start>:
-[ 	]*[a-f0-9]+:	48 8d 05 f9 ff ff 2f 	lea    0x2ffffff9\(%rip\),%rax        # a0000000 <foo>
-[ 	]*[a-f0-9]+:	4c 8d 1d f2 ff ff 2f 	lea    0x2ffffff2\(%rip\),%r11        # a0000000 <foo>
+[ 	]*[a-f0-9]+:	48 8b 05 ([0-9a-f]{2} ){4} *	mov    [-]?0x[a-f0-9]+\(%rip\),%rax        # [a-f0-9]+ <.got>
+[ 	]*[a-f0-9]+:	4c 8b 1d ([0-9a-f]{2} ){4} *	mov    [-]?0x[a-f0-9]+\(%rip\),%r11        # [a-f0-9]+ <.got>
diff --git a/ld/testsuite/ld-x86-64/pr19609-6a.d b/ld/testsuite/ld-x86-64/pr19609-6a.d
index 4802ffea4b..3c011d9b05 100644
--- a/ld/testsuite/ld-x86-64/pr19609-6a.d
+++ b/ld/testsuite/ld-x86-64/pr19609-6a.d
@@ -1,4 +1,4 @@
 #source: pr19609-6.s
 #as: --64 -mrelax-relocations=yes
 #ld: -melf_x86_64 --defsym foobar=0x80000000
-#error: .*relocation truncated to fit: R_X86_64_32S .*
+#error: failed to convert GOTPCREL relocation; relink with --no-relax
-- 
2.13.5