[PATCH v5] elf/x86-64: Subtract __ImageBase for R_AMD64_IMAGEBASE
H.J. Lu
hjl.tools@gmail.com
Fri Mar 5 18:26:35 GMT 2021
On Fri, Mar 5, 2021 at 5:57 AM Alan Modra <amodra@gmail.com> wrote:
>
> 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.
Fixed.
> >
> > /* 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.
Fixed.
> > 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?
Added ldelf_set_output_arch.
> > 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.
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.
Here is the v5 patch. OK for master?
Thanks.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v5-0001-elf-x86-64-Subtract-__ImageBase-for-R_AMD64_IMAGE.patch
Type: text/x-patch
Size: 20616 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20210305/eb1af34a/attachment-0001.bin>
More information about the Binutils
mailing list