[PATCH v4] elf/x86-64: Subtract __ImageBase for R_AMD64_IMAGEBASE

[Alan Modra]

On Thu, Mar 04, 2021 at 05:54:20AM -0800, H.J. Lu wrote:
> Here is the v4 patch on top of yours.

No patch?

This is what I'm about to commit, now rather than in a week's time
since you seem happy with it.  Changes from the previous patch are:
 - removed test of ELF output, the fixes are generic
 - untangled and corrected pc-relative adjustment to use
   bfd_get_reloc_size

There are some further obvious fixes to make as noted in the commit
log, but I'll leave that to someone with better access to PE
compilers.

----
Subject: Move x86_64 PE changes out of bfd_perform_relocation

bfd_perform_relocation should not have special case target code.  This
patch moves the code that was there for x86_64 PE linking to ELF
output into the x86_64 PE howto special function, correcting that
function for linking to targets other than ELF too.  The fixes in
bfd_perform_relocation were over-complicated due to needing to
compensate for things that had already gone wrong in coff_amd64_reloc.
In particular, an adjustment for pc-relative relocs was done in a way
that meant adjustment for things related to symbol offsets was lost.
I think those two things are orthogonal, but who knows with COFF where
addends and symbol values are found randomly in the section contents.

Note that linking natively to an x86_64 PE output relocates by
coff_pe_amd64_relocate_section, which does not use arelent relocs or
bfd_perform_relocation, but be aware of coff_amd64_rtype_to_howto
hacking addends for relocations.  The adjustments for a particular
relocation type there and in coff_amd64_reloc ought to match after
taking into consideration CALC_ADDEND.  They don't.  For example,
the pc-relative adjustment for R_PCRWORD is 2 bytes in
coff_amd64_reloc and 4 bytes in coff_amd64_rtype_to_howto.

	* reloc.c (bfd_perform_relocation): Revert 2021-01-12 and
	2020-09-16 changes.
	* coff-x86_64.c (coff_amd64_reloc): Do more or less the same
	adjustments here instead.  Separate pc-relative adjustments
	from symbol related adjustments.  Tidy comments and formatting.

diff --git a/bfd/coff-x86_64.c b/bfd/coff-x86_64.c
index adab60cd11..5b09023f3c 100644
--- a/bfd/coff-x86_64.c
+++ b/bfd/coff-x86_64.c
@@ -75,14 +75,14 @@ coff_amd64_reloc (bfd *abfd,
 {
   symvalue diff;
 
-#if !defined(COFF_WITH_PE)
+#if !defined (COFF_WITH_PE)
   if (output_bfd == NULL)
     return bfd_reloc_continue;
 #endif
 
   if (bfd_is_com_section (symbol->section))
     {
-#if !defined(COFF_WITH_PE)
+#if !defined (COFF_WITH_PE)
       /* We are relocating a common symbol.  The current value in the
 	 object file is ORIG + OFFSET, where ORIG is the value of the
 	 common symbol as seen by the object file when it was compiled
@@ -106,21 +106,10 @@ coff_amd64_reloc (bfd *abfd,
 	 ignores the addend for a COFF target when producing
 	 relocatable output.  This seems to be always wrong for 386
 	 COFF, so we handle the addend here instead.  */
-#if defined(COFF_WITH_PE)
+#if defined (COFF_WITH_PE)
       if (output_bfd == NULL)
 	{
-	  reloc_howto_type *howto = reloc_entry->howto;
-
-	  /* Although PC relative relocations are very similar between
-	     PE and non-PE formats, but they are off by 1 << howto->size
-	     bytes. For the external relocation, PE is very different
-	     from others. See md_apply_fix3 () in gas/config/tc-amd64.c.
-	     When we link PE and non-PE object files together to
-	     generate a non-PE executable, we have to compensate it
-	     here.  */
-	  if(howto->pc_relative && howto->pcrel_offset)
-	    diff = -(1 << howto->size);
-	  else if(symbol->flags & BSF_WEAK)
+	  if (symbol->flags & BSF_WEAK)
 	    diff = reloc_entry->addend - symbol->value;
 	  else
 	    diff = -reloc_entry->addend;
@@ -130,7 +119,18 @@ coff_amd64_reloc (bfd *abfd,
 	diff = reloc_entry->addend;
     }
 
-#if defined(COFF_WITH_PE)
+#if defined (COFF_WITH_PE)
+  if (output_bfd == NULL)
+    {
+      /* PC relative relocations are off by their size.  */
+      if (reloc_entry->howto->pc_relative)
+	diff -= bfd_get_reloc_size (reloc_entry->howto);
+
+      if (reloc_entry->howto->type >= R_AMD64_PCRLONG_1
+	  && reloc_entry->howto->type <= R_AMD64_PCRLONG_5)
+	diff -= reloc_entry->howto->type - R_AMD64_PCRLONG;
+    }
+
   /* FIXME: How should this case be handled?  */
   if (reloc_entry->howto->type == R_AMD64_IMAGEBASE
       && output_bfd != NULL
diff --git a/bfd/reloc.c b/bfd/reloc.c
index a7547187eb..5ed7bb8e59 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -51,7 +51,7 @@ SECTION
 #include "bfdlink.h"
 #include "libbfd.h"
 #include "bfdver.h"
-#include "coff/x86_64.h"
+
 /*
 DOCDD
 INODE
@@ -905,30 +905,6 @@ space consuming.  For each target:
 	    }
 	}
     }
-  else if (abfd->xvec->flavour == bfd_target_coff_flavour
-	   && (input_section->output_section->owner->xvec->flavour
-	       == bfd_target_elf_flavour)
-	   && strcmp (abfd->xvec->name, "pe-x86-64") == 0
-	   && strcmp (input_section->output_section->owner->xvec->name,
-		      "elf64-x86-64") == 0)
-    {
-      /* NB: bfd_perform_relocation isn't called to generate PE binary.
-	 _bfd_relocate_contents is called instead.  When linking PE
-	 object files to generate ELF output, _bfd_relocate_contents
-	 isn't called and bfd_perform_relocation is used.  We need to
-	 adjust relocation here.  */
-      relocation -= reloc_entry->addend;
-      if (howto->type >= R_AMD64_PCRLONG_1
-	  && howto->type <= R_AMD64_PCRLONG_5)
-	relocation -= (bfd_vma)(howto->type - R_AMD64_PCRLONG);
-      else if (howto->type == R_AMD64_DIR64
-	       || howto->type == R_AMD64_DIR32)
-	{
-	  bfd_vma val = read_reloc (abfd, (bfd_byte *) data + octets,
-				    howto);
-	  relocation -= val & howto->src_mask;
-	}
-    }
 
   /* FIXME: This overflow checking is incomplete, because the value
      might have overflowed before we get here.  For a correct check we


-- 
Alan Modra
Australia Development Lab, IBM