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

[H.J. Lu]

On Mon, Mar 1, 2021 at 4:19 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Sat, Feb 27, 2021 at 06:38:24PM -0800, H.J. Lu wrote:
> > On Wed, Feb 24, 2021 at 6:09 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Tue, Feb 23, 2021 at 09:50:20AM -0800, H.J. Lu via Binutils wrote:
> > > >       * reloc.c (bfd_perform_relocation): Add a link_info argument.
> > > >       Subtract __ImageBase for R_AMD64_IMAGEBASE.
> > >
> > > H.J., did you consider moving all of this, including the code you
> > > added in git commit 36068e2fa54, to the relevant howto special
> > > functions?  None of it really belongs in bfd_perform_relocation.
> >
> > The howto special function, coff_amd64_reloc, isn't changed since
> >
> > 1. Any changes to coff_amd64_reloc may break Windows x86-64 linker.
>
> I am sure you are capable of making those changes without breaking
> windows.

The difficult part is I can't test it.

> > 2. bfd_perform_relocation changes are safe since they are limited to
> > generating x86-64 ELF executable from Windows x86-64 relocatable object
> > files.
>
> It is the wrong place.  Every time bfd_perform_relocation is called
> with a coff input it now needs to test for output to ELF.  If you did
> this properly in a howto function then the extra tests would only be

bfd_perform_relocation is called with

           r = bfd_perform_relocation (input_bfd,
                                        *parent,
                                        data,
                                        input_section,
                                        relocatable ? abfd : NULL,
<<< output_bfd
                                        &error_message);

and coff_amd64_reloc is very hard to understand.

> done for x86_64 coff.  Besides the performance impact, nobody
> maintaining reloc.c wants to deal with code specially for pe-x86-64
> and elf64-x84-64.  The special case for coff there is already
> horrible.  It shouldn't be made worse.

 I made the change in bfd_perform_relocation since the COFF
linker is very delicate.

> Did anyone review your commit 36068e2fa54?

No.  My change is limited to a very special case of ELF/x86-64.

bfd_perform_relocation has

      /* WTF?? */
      if (abfd->xvec->flavour == bfd_target_coff_flavour
          && strcmp (abfd->xvec->name, "coff-Intel-little") != 0
          && strcmp (abfd->xvec->name, "coff-Intel-big") != 0)
        {

with a long comment.   I added:

 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)

Its performance impact should be low.

-- 
H.J.