[patch bfd]: Adjust handling for plugin-generated sections for pe-coff targets

[Kai Tietz]

2011/10/8 Kai Tietz <ktietz70@googlemail.com>:
> 2011/10/6 Alan Modra <amodra@gmail.com>:
>> On Tue, Oct 04, 2011 at 08:50:20AM -0400, NightStrike wrote:
>>> Ping
>>
>> I've been away..
>>
>>> On Thu, Sep 29, 2011 at 4:56 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> > 2011/9/29 Alan Modra <amodra@gmail.com>:
>>> >> I think this is wrong.  You should never be applying a relocation
>>> >> against a symbol defined in an IR section.  If you are, then either ld
>>> >> has failed to redefine the symbol properly when given a real
>>> >> definition in LTO output, or gcc has failed to supply the real
>>> >> definition.  Either way, you shouldn't hide this error.
>>> >
>>> > Well, it might be wrong, if we assume that all objects are seen by IR.
>>> >  That isn't the case for now.  By this it happens (I can sent you test
>>> > for this issue offline - it is a bit too big and complex to do for it
>>> > a binutils testcase, if you are intetested) in cases that within a
>>> > used library the same object (call it x) as additional specified on
>>> > command-line is specified.  Here the object-file x on command-line
>>> > gets resolved later, so that IR assumes it has an IRONLY symbol from
>>> > its library version of x, but later on when the object x from
>>> > command-line is finally linked, the IR version of x is getting
>>> > discarded.  By this any function reference in library to a symbol of
>>> > library's object x version, are pointing to discarded version,  This
>>> > doesn't cause troubles, as on final linking the already linked
>>> > object-references are used instead.  Actually they aren't
>>> > multiple-times linked, just once.
>>
>> Your argument hasn't convinced me to approve your patch.  I still
>> believe it is wrong to ignore *all* relocs against symbols defined in
>> IR sections, as your patch does.  It might be reasonable to ignore
>> some, as the ELF linker does for relocs in debug or other special
>> handled sections.
>
> Well, the point here is that we don't want to warn about discarded
> sections here in some cases.  I see that we miss for non-IR sections
> to check for finfo->info->strip_discarded, as this means we won't have
> duplicates.  So a symbol from IR won't be written out (and here might
> be another issue in write-symbol, too), and therefore it isn't double
> defined.
>
> Secondly it seems to be not correct to check here for EXCLUDED flag,
> instead we should look always for section being no abs-one, and
> output-section being an abs-section.
> As if SEC_EXCLUDED is set, the output-section is always set to
> abs-section.  So check logic is more equal to ELF.
>
> Alter patch attached. I did a regression test for x86_64-w64-mingw32,
> i686-w64-mingw, and i686-pc-cygwin.
>
> Regards,
> Kai
>
> Index: cofflink.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/cofflink.c,v
> retrieving revision 1.79
> diff -u -r1.79 cofflink.c
> --- cofflink.c  17 Aug 2011 00:39:38 -0000      1.79
> +++ cofflink.c  8 Oct 2011 10:33:37 -0000
> @@ -2380,13 +2391,23 @@
>              while (h->root.type == bfd_link_hash_indirect
>                     || h->root.type == bfd_link_hash_warning)
>                h = (struct coff_link_hash_entry *) h->root.u.i.link;
> +             if ((h->root.type == bfd_link_hash_defined
> +                  || h->root.type == bfd_link_hash_defweak)
> +                 && ((finfo->info->strip_discarded
> +                      && h->root.u.def.section != NULL
> +                      && !bfd_is_abs_section (h->root.u.def.section)
> +                      && bfd_is_abs_section (h->root.u.def.section->output_section))
> +                     || (h->root.u.def.section->owner != NULL
> +                         && (h->root.u.def.section->owner->flags &
> BFD_PLUGIN) != 0)))
> +                continue;
>              if (h->root.type == bfd_link_hash_defined
>                  || h->root.type == bfd_link_hash_defweak)
>                ps = h->root.u.def.section;
>              if (ps == NULL)
>                continue;
>              /* Complain if definition comes from an excluded section.  */
> -             if (ps->flags & SEC_EXCLUDE)
> +             if (!bfd_is_abs_section (ps)
> +                 && bfd_is_abs_section ((ps)->output_section))
>                (*finfo->info->callbacks->einfo)
>                  (_("%X`%s' referenced in section `%A' of %B: "
>                     "defined in discarded section `%A' of %B\n"),
>

Well, without

> +                     || (h->root.u.def.section->owner != NULL
> +                         && (h->root.u.def.section->owner->flags & BFD_PLUGIN) != 0)))

it works too.  So checking here for plugin IR seems to be unnecessary.

Kai