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

[Alan Modra]

On Thu, Mar 04, 2021 at 09:29:48PM -0800, H.J. Lu wrote:
> --- a/bfd/coff-x86_64.c
> +++ b/bfd/coff-x86_64.c
> @@ -122,13 +122,39 @@ coff_amd64_reloc (bfd *abfd,
>  #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 ((bfd_get_flavour (input_section->output_section->owner)
> +	   == bfd_target_elf_flavour)
> +	  && reloc_entry->howto->type == R_AMD64_IMAGEBASE)
> +	{
> +	  /* Subtract __ImageBase.  */
> +	  struct bfd_link_info *link_info;
> +	  struct bfd_link_hash_entry *h;
> +	  link_info
> +	    = _bfd_get_link_info (input_section->output_section->owner);
> +	  if (link_info == NULL)
> +	    abort ();
> +	  h = bfd_link_hash_lookup (link_info->hash, "__ImageBase",
> +				    FALSE, FALSE, FALSE);
> +	  if (h == NULL)
> +	    abort ();
> +	  while (h->type == bfd_link_hash_indirect)
> +	    h = h->u.i.link;
> +	  /* ELF symbols in relocatable files are section relative,
> +	     but in nonrelocatable files they are virtual addresses.  */
> +	  diff -= (h->u.def.value
> +		   + h->u.def.section->output_offset
> +		   + h->u.def.section->output_section->vma);
> +	}
> +      else
> +	{
> +	  /* 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;
> +	  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;
> +	}
>      }

I think this would be better style if the pc_relative and PCRLONG
adjustments were left as they were, and then write

    if (reloc_entry->howto->type == R_AMD64_IMAGEBASE)
      {
	bfd *obfd = input_section->output_section->owner;

	switch (bfd_get_flavour (obfd))
	  {
	  case bfd_target_coff_flavour:
	    diff -= pe_data (obfd)->pe_opthdr.ImageBase;
	    break;

	  case bfd_target_elf_flavour:
	    {
	      your new code
	    }
	    break;
	  }
      }

That should get rid of the FIXME.

>  
>    /* FIXME: How should this case be handled?  */


> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index dd66d98883e..c2aaab0e5bf 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -1887,6 +1887,9 @@ struct output_elf_obj_tdata
>    /* Used when laying out sections.  */
>    file_ptr next_file_pos;
>  
> +  /* Optional linker information.  */
> +  struct bfd_link_info *link_info;
> +

That's a nice trick that might be handy elsewhere, if you always set
it rather than making it optional.

>    int num_section_syms;
>    unsigned int shstrtab_section, strtab_section;
>  
> @@ -2064,6 +2067,7 @@ struct elf_obj_tdata
>  #define elf_elfsections(bfd)	(elf_tdata(bfd) -> elf_sect_ptr)
>  #define elf_numsections(bfd)	(elf_tdata(bfd) -> num_elf_sections)
>  #define elf_seg_map(bfd)	(elf_tdata(bfd) -> o->seg_map)
> +#define elf_link_info(bfd)	(elf_tdata(bfd) -> o->link_info)
>  #define elf_next_file_pos(bfd)	(elf_tdata(bfd) -> o->next_file_pos)
>  #define elf_eh_frame_hdr(bfd)	(elf_tdata(bfd) -> o->eh_frame_hdr)
>  #define elf_stack_flags(bfd)	(elf_tdata(bfd) -> o->stack_flags)
> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
> index 93ad38c5eb0..e60d87ff981 100644
> --- a/bfd/elfxx-x86.c
> +++ b/bfd/elfxx-x86.c
> @@ -3174,6 +3174,8 @@ _bfd_elf_linker_x86_set_options (struct bfd_link_info * info,
>      = get_elf_backend_data (info->output_bfd);
>    struct elf_x86_link_hash_table *htab
>      = elf_x86_hash_table (info, bed->target_id);
> +  if (info->output_bfd->xvec->flavour == bfd_target_elf_flavour)
> +    elf_link_info (info->output_bfd) = info;

Move this to somewhere all ELF targets will have elf_link_info, a new
ldelf.c:elf_set_output_arch perhaps?

>    if (htab != NULL)
>      htab->params = params;
>  }
> diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
> index 2dc20ec1b19..62b1cee0af0 100644
> --- a/bfd/libbfd-in.h
> +++ b/bfd/libbfd-in.h
> @@ -899,6 +899,8 @@ extern bfd_vma _bfd_safe_read_leb128
>  extern bfd_byte * _bfd_write_unsigned_leb128
>    (bfd_byte *, bfd_byte *, bfd_vma) ATTRIBUTE_HIDDEN;
>  
> +extern struct bfd_link_info *_bfd_get_link_info (bfd *);
> +
>  #if GCC_VERSION >= 7000
>  #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
>  #else
> diff --git a/bfd/libbfd.h b/bfd/libbfd.h
> index 7271a2ad5a1..3a481ea468f 100644
> --- a/bfd/libbfd.h
> +++ b/bfd/libbfd.h
> @@ -904,6 +904,8 @@ extern bfd_vma _bfd_safe_read_leb128
>  extern bfd_byte * _bfd_write_unsigned_leb128
>    (bfd_byte *, bfd_byte *, bfd_vma) ATTRIBUTE_HIDDEN;
>  
> +extern struct bfd_link_info *_bfd_get_link_info (bfd *);
> +
>  #if GCC_VERSION >= 7000
>  #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
>  #else
> diff --git a/ld/ldelf.c b/ld/ldelf.c
> index 049992544a2..c7de958dbdc 100644
> --- a/ld/ldelf.c
> +++ b/ld/ldelf.c
> @@ -81,6 +81,30 @@ ldelf_load_symbols (lang_input_statement_type *entry)
>  {
>    int link_class = 0;
>  
> +  if (bfd_link_pde (&link_info)
> +      && entry->the_bfd->xvec->flavour == bfd_target_coff_flavour
> +      && strcmp (entry->the_bfd->xvec->name, "pe-x86-64") == 0
> +      && strcmp (link_info.output_bfd->xvec->name, "elf64-x86-64") == 0)
> +    {
> +      /* NB: When linking Windows x86-64 relocatable object files to
> +	 generate ELF executable, create an indirect reference to
> +	 __executable_start for __ImageBase to support R_AMD64_IMAGEBASE
> +	 relocation which is relative to __ImageBase.  */
> +      struct elf_link_hash_table *htab = elf_hash_table (&link_info);
> +      struct elf_link_hash_entry *h, *hi;
> +      hi = elf_link_hash_lookup (htab, "__ImageBase", TRUE, FALSE,
> +				 FALSE);
> +      if (hi->root.type == bfd_link_hash_new
> +	  || hi->type == bfd_link_hash_undefined
> +	  || hi->type == bfd_link_hash_undefweak)
> +	{
> +	  h = elf_link_hash_lookup (htab, "__executable_start",
> +				    TRUE, FALSE, TRUE);
> +	  hi->root.type = bfd_link_hash_indirect;
> +	  hi->root.u.i.link = (struct bfd_link_hash_entry *) h;
> +	}
> +    }
> +
>    /* Tell the ELF linker that we don't want the output file to have a
>       DT_NEEDED entry for this file, unless it is used to resolve
>       references in a regular object.  */

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.

-- 
Alan Modra
Australia Development Lab, IBM