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

[Alan Modra <amodra@gmail.com>]

On Fri, Mar 05, 2021 at 10:26:35AM -0800, H.J. Lu wrote:
> On Fri, Mar 5, 2021 at 5:57 AM Alan Modra <amodra@gmail.com> wrote:
> > This doesn't look like the right place to set up a reference to
> > __executable_start.  Doing so in a new check_relocs in pei-x86_64.c
> > that tests for an ELF output would be better.  I'm not suggesting that
> > you run over relocs looking for R_AMD64_IMAGEBASE, that could be done
> > but seems unnecessary.
> 
> check_relocs is too late.  We need to do it in load_symbols ().  Otherwise,
> __executable_start won't be defined.  I added pex64_link_add_symbols to
> do it.

Ah, I was forgetting that check_relocs now runs much later.  Even so,
I would have thought that the PROVIDE would define the symbol?  The
question is academic, I'm happy with pex64_link_add_symbols.

> Here is the v5 patch.  OK for master?

OK with three further changes.
1) The R_AMD64_IMAGEBASE handling should to be inside the
   output_bfd == NULL block.  It is wrong to apply the image base
   adjustment to the section for relocatable linking.  Doing so would
   effectively change the reloc addend.
2) I worry about the abort, which I think you might hit if trying to
   link PE objects into a pie.  One possible fix would be to change
   the bfd_link_pde in pex64_link_add_symbols to !bfd_link_relocatable.
   That might just be sweeping a problem under the rug, so an
   alternative fix would be to remove the abort and instead return
   with bfd_reloc_dangerous.  Either fix is fine with me.
3) No need to check for bfd_target_elf_flavour in set_output_arch.

-- 
Alan Modra
Australia Development Lab, IBM