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

[Kai Tietz <ktietz70 at googlemail dot com>]

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