Re: RFC/A: Add a bfd hook for defining common symbols

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Wed, Apr 1, 2009 at 2:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Apr 1, 2009 at 1:37 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Thanks for the reply.
>>>
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>> On Wed, Apr 1, 2009 at 12:28 PM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> The XCOFF linker uses XCOFF_DEF_REGULAR to check whether a symbol
>>>>> has a regular definition. Âxcofflink.c:xcoff_post_gc_symbol therefore
>>>>> checks for common symbols that were defined by the linker:
>>>>>
>>>>> Â/* If this is a final link, and the symbol was defined as a common
>>>>> Â Â symbol in a regular object file, and there was no definition in
>>>>> Â Â any dynamic object, then the linker will have allocated space for
>>>>> Â Â the symbol in a common section but the XCOFF_DEF_REGULAR flag
>>>>> Â Â will not have been set. Â*/
>>>>> Âif (h->root.type == bfd_link_hash_defined
>>>>> Â Â Â&& (h->flags & XCOFF_DEF_REGULAR) == 0
>>>>> Â Â Â&& (h->flags & XCOFF_REF_REGULAR) != 0
>>>>> Â Â Â&& (h->flags & XCOFF_DEF_DYNAMIC) == 0
>>>>> Â Â Â&& (bfd_is_abs_section (h->root.u.def.section)
>>>>> Â Â Â Â Â|| (h->root.u.def.section->owner->flags & DYNAMIC) == 0))
>>>>> Â Âh->flags |= XCOFF_DEF_REGULAR;
>>>>>
>>>>> But this happens too late for things like xcoff_mark_auto_exports.
>>>>>
>>>>> We could just put this code in a function and use it instead of testing
>>>>> XCOFF_DEF_REGULAR directly. ÂThat seems a bit messy though, and I don't
>>>>> really like this idea of detecting what the linker has done after the fact.
>>>>>
>>>>> Another alternative is to add a linker emulation hook. ÂBut we'd
>>>>> then be exposing XCOFF_DEF_REGULAR outside BFD, which doesn't seem
>>>>> very clean either.
>>>>
>>>> FWIW, ELF linker emulation includes "elf-bfd.h" and uses plenty of
>>>> BFD/ELF things. I don't see why you can't do it for XCOFF. You don't
>>>> need to access XCOFF_DEF_REGULAR directly from linker. A macro
>>>> defined in BFD should work for you.
>>>
>>> True, but I was more meaning: it exposes the _existence_ of
>>> XCOFF_DEF_REGULAR outside bfd. ÂThat is, it exposes the fact
>>> there's this magic bfd function (or bfd macro) that has to be
>>> called after defining a common symbol on XCOFF, even though
>>> no such thing is needed for other targets. ÂThe reason we need
>>> the hook isn't that XCOFF common symbols are any different from
>>> other targets' common symbols. ÂThe reason is simply that we
>>> need to do some bfd-internal, XCOFF-specific book-keeping
>>> after defining a symbol.
>>>
>>> More generally, my attitude was that code should be in bfd if it
>>> represents a general concept that does not depend on information that
>>> only the linker proper has access to. ÂI think that's the case here.
>>> The current code is changing the type of a bfd hash table entry
>>> "behind bfd's back".
>>>
>>
>> ELF backend also does many common symbol related book-keeping
>> things. Why can't XCOFF backend use similar hooks.
>>
>
> See elf_link_add_object_symbols and _bfd_elf_merge_symbol
> for details.

Sorry H.J., but I don't see your point.  elf_link_add_object_symbols
is a subroutine of elf_link_add_symbols, which in turn is the
ELF implementation of the BFD _bfd_link_add_symbols hook.
And _bfd_link_add_symbols is exactly the kind of BFD hook
I'm suggesting we use here; it is part of BFD_JUMP_TABLE_LINK,
just like _bfd_define_common_symbol is in the proposed patch.

I don't think _bfd_link_add_symbols should try to predict what
lang_one_common is going to do, if that's what you're suggesting.
Even if that's possible, it seems no better than the status quo,
where we try to guess what ld has already done.

_bfd_elf_merge_symbol is internal to the bfd ELF backend.  It isn't used
directly by ld/.

What I'm trying to do is here is change something that ld/ does directly
in such a way that some internal bfd datastructures are kept up to date.
If we added a linker emulation hook, it would exist _purely_ for bookkeeping
reasons, not semantic reasons.  I can't really see anything in elf32.em or
linux.em that is analagous to this situation, although I could well have
missed something.

Richard