This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Consistency check for merge sections


On Wed, Sep 30, 2015 at 3:13 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 05:40:35AM -0700, H.J. Lu wrote:
>> On Wed, Sep 30, 2015 at 04:11:01PM +0930, Alan Modra wrote:
>> > We can't allow sections to be merged and sized by the ELF linker
>> > backend and then later be output by the generic linker backend.  The
>> > generic linker backend doesn't understand merge sections.
>> >
>> >     PR ld/19013
>> >     * elflink.c (_bfd_elf_merge_sections): Only merge input bfds that
>> >     will be handled by elf_link_input_bfd.  Rename abfd param to obfd.
>> >
>>
>> This isn't sufficient.  When we check consistency for merge ELF sections,
>> we should not only check EI_CLASS, but also compatible e_machine.  I
>> checked in this to check e_machine.
>>
>> H.J.
>> ---
>> bfd/
>>
>>       PR ld/19013
>>       * elflink.c (_bfd_elf_merge_sections): Only merge input bfds
>>       that have the compatible ELF machine code with the output bfd.
>
> The whole point of my patch was as I said, to merge sections only when
> the output will be done by the ELF linker backend.  ie. the condition
> under which we merge sections needs to match this code in
> bfd_elf_final_link:
>
>           if (p->type == bfd_indirect_link_order
>               && (bfd_get_flavour ((sub = p->u.indirect.section->owner))
>                   == bfd_target_elf_flavour)
>               && elf_elfheader (sub)->e_ident[EI_CLASS] == bed->s->elfclass)
>             {
>               if (! sub->output_has_begun)
>                 {
>                   if (! elf_link_input_bfd (&flinfo, sub))
>                     goto error_return;
>                   sub->output_has_begun = TRUE;
>                 }
>
> You added an extra check for no good reason as far as I can see.  I am
> reverting your elflink.c patch until you can explain why it is needed.
>
> commit 017e6bceee1a96d4b57175687560b4d625fdb150
> Author: Alan Modra <amodra@gmail.com>
> Date:   Thu Oct 1 07:38:07 2015 +0930
>
>     Revert "Also check e_machine when merging sections"
>
>     Commit 9865bd0d added a bogus check in _bfd_elf_merge_sections.
>
>     bfd/
>         PR ld/19013
>         * elflink.c (_bfd_elf_merge_sections): Revert last change.
>     ld/testsuite/
>         * ld-x86-64/pr19013-x32.d: Update.
>
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 3610948..76c8a5c 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-10-01  Alan Modra  <amodra@gmail.com>
> +
> +       PR ld/19013
> +       * elflink.c (_bfd_elf_merge_sections): Revert last change.
> +
>  2015-09-30  H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR ld/19031
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 70c231b..90af6cf 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -6818,23 +6818,15 @@ _bfd_elf_merge_sections (bfd *obfd, struct bfd_link_info *info)
>  {
>    bfd *ibfd;
>    asection *sec;
> -  const struct elf_backend_data *bed;
>
>    if (!is_elf_hash_table (info->hash))
>      return FALSE;
>
> -  bed = get_elf_backend_data (obfd);
>    for (ibfd = info->input_bfds; ibfd != NULL; ibfd = ibfd->link.next)
>      if ((ibfd->flags & DYNAMIC) == 0
>         && bfd_get_flavour (ibfd) == bfd_target_elf_flavour
> -       && (elf_elfheader (ibfd)->e_ident[EI_CLASS] == bed->s->elfclass)
> -       && (bed->elf_machine_code == elf_elfheader (ibfd)->e_machine
> -           || (bed->elf_machine_alt1 != 0
> -               && (bed->elf_machine_alt1
> -                   == elf_elfheader (ibfd)->e_machine))
> -           || (bed->elf_machine_alt2 != 0
> -               && (bed->elf_machine_alt2
> -                   == elf_elfheader (ibfd)->e_machine))))
> +       && (elf_elfheader (ibfd)->e_ident[EI_CLASS]
> +           == get_elf_backend_data (obfd)->s->elfclass))
>        for (sec = ibfd->sections; sec != NULL; sec = sec->next)
>         if ((sec->flags & SEC_MERGE) != 0
>             && !bfd_is_abs_section (sec->output_section))
> diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
> index e73d09f..b8724af 100644
> --- a/ld/testsuite/ChangeLog
> +++ b/ld/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-10-01  Alan Modra  <amodra@gmail.com>
> +
> +       * ld-x86-64/pr19013-x32.d: Update.
> +
>  2015-09-30  H.J. Lu  <hongjiu.lu@intel.com>
>
>         * ld-x86-64/pr19013.d (ld): Add -m elf_x86_64.
> diff --git a/ld/testsuite/ld-x86-64/pr19013-x32.d b/ld/testsuite/ld-x86-64/pr19013-x32.d
> index fb70966..97fb841 100644
> --- a/ld/testsuite/ld-x86-64/pr19013-x32.d
> +++ b/ld/testsuite/ld-x86-64/pr19013-x32.d
> @@ -5,5 +5,5 @@
>  #notarget: x86_64-*-nacl*
>
>  #...
> - [0-9a-f]+ 00000203 00414243 4400 +.....ABCD. +
> + [0-9a-f]+ 02030041 42434400 +...ABCD. +
>  #pass
>

You also need to update ld/testsuite/ld-x86-64/pr19013-nacl.d.
Otherwise, it will fail for x86_64-nacl target.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]