This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: Binutils is broken by the linker change
It looks OK. We will know if it breaks something :-).
H.J.
On Fri, Mar 19, 2004 at 11:17:51AM +1030, Alan Modra wrote:
> On Thu, Mar 18, 2004 at 03:56:31PM -0800, H. J. Lu wrote:
> > On Fri, Mar 19, 2004 at 09:57:20AM +1030, Alan Modra wrote:
> > > Please, please, add this testcase to the ld testsuite! With weak and
> > > strong foo returning different values or somesuch so you can check
> > > which one is called..
> >
> > I will add it.
>
> Thanks!
>
> > > Is there a reason why you moved this code above the type/size change
> > > code? Also, I don't think there is any need to add newdef and olddef
> > > to the tests. That just confuses the meaning of newweak and oldweak.
> > >
> >
> > We don't want to change type/size if a weak definition is treated
> > as strong.
>
> Well, setting *type_change_ok and/or *size_change_ok doesn't affect
> whether we change the size or not. It merely controls whether you
> get a warning or not.. In practice, I don't think it matters whether
> *type_change_ok and *size_change_ok are set before or after the
> twiddles to newweak and oldweak. New _definitions_ are the only
> things we care about with respect to type/size changes, and the
> newweak/oldweak twiddles prevent new definitions from happening.
>
> However, since moving *type_change and *size_change back to where they
> were previously will potentially warn in more cases, I suppose it is
> reasonable to do that.
>
> > If we don't check newdef and olddef, we may treat a
> > weak reference as strong.
>
> This one I care about a little more. As I said, adding newdef/olddef
> into the tests confuses the meaning of the flags. It would have been
> appropriate when we had newweakdef and newweakundef etc., but not now.
> It's also unnecessary, because newdef and/or olddef are tested whenever
> newweak/oldweak are used, except in a couple of places where newweak and
> oldweak are now unnecessary. It also might be wrong, because a common
> sym isn't a newdef, yet later code treats commons like defs in some
> cases.
>
> How does this look?
>
> * elflink.c (_bfd_elf_merge_symbol): Revert last change. Move
> type and size change code to where it was previously. Remove
> dt_needed param. Treat old weak syms as strong if new sym is
> from a shared lib, even when old sym is from another shared
> lib. Remove unnecessary tests of oldweak and newweak. Correct
> comments.
> (_bfd_elf_add_default_symbol): Remove dt_needed param. Update
> _bfd_elf_merge_symbol calls.
> * elflink.h (elf_link_add_object_symbols): Update calls. Remove
> dt_needed local var. Update comments.
> * elf-bfd.h (_bfd_elf_merge_symbol): Update prototype.
> (_bfd_elf_add_default_symbol): Likewise.
>
> Index: bfd/elf-bfd.h
> ===================================================================
> RCS file: /cvs/src/src/bfd/elf-bfd.h,v
> retrieving revision 1.128
> diff -u -p -r1.128 elf-bfd.h
> --- bfd/elf-bfd.h 18 Mar 2004 12:50:19 -0000 1.128
> +++ bfd/elf-bfd.h 19 Mar 2004 00:35:05 -0000
> @@ -1476,12 +1476,12 @@ extern bfd_boolean _bfd_elf_maybe_strip_
> extern bfd_boolean _bfd_elf_merge_symbol
> (bfd *, struct bfd_link_info *, const char *, Elf_Internal_Sym *,
> asection **, bfd_vma *, struct elf_link_hash_entry **, bfd_boolean *,
> - bfd_boolean *, bfd_boolean *, bfd_boolean *, bfd_boolean);
> + bfd_boolean *, bfd_boolean *, bfd_boolean *);
>
> extern bfd_boolean _bfd_elf_add_default_symbol
> (bfd *, struct bfd_link_info *, struct elf_link_hash_entry *,
> const char *, Elf_Internal_Sym *, asection **, bfd_vma *,
> - bfd_boolean *, bfd_boolean, bfd_boolean);
> + bfd_boolean *, bfd_boolean);
>
> extern bfd_boolean _bfd_elf_export_symbol
> (struct elf_link_hash_entry *, void *);
> Index: bfd/elflink.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/elflink.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 elflink.c
> --- bfd/elflink.c 18 Mar 2004 23:02:06 -0000 1.58
> +++ bfd/elflink.c 19 Mar 2004 00:35:07 -0000
> @@ -653,8 +653,7 @@ _bfd_elf_link_renumber_dynsyms (bfd *out
> TYPE_CHANGE_OK if it is OK for the type to change. We set
> SIZE_CHANGE_OK if it is OK for the size to change. By OK to
> change, we mean that we shouldn't warn if the type or size does
> - change. DT_NEEDED indicates if it comes from a DT_NEEDED entry of
> - a shared object. */
> + change. */
>
> bfd_boolean
> _bfd_elf_merge_symbol (bfd *abfd,
> @@ -667,8 +666,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
> bfd_boolean *skip,
> bfd_boolean *override,
> bfd_boolean *type_change_ok,
> - bfd_boolean *size_change_ok,
> - bfd_boolean dt_needed)
> + bfd_boolean *size_change_ok)
> {
> asection *sec;
> struct elf_link_hash_entry *h;
> @@ -887,6 +885,18 @@ _bfd_elf_merge_symbol (bfd *abfd,
> oldweak = (h->root.type == bfd_link_hash_defweak
> || h->root.type == bfd_link_hash_undefweak);
>
> + /* If a new weak symbol comes from a regular file and the old symbol
> + comes from a dynamic library, we treat the new one as strong.
> + Similarly, an old weak symbol from a regular file is treated as
> + strong when the new symbol comes from a dynamic library. Further,
> + an old weak symbol from a dynamic library is treated as strong if
> + the new symbol is from a dynamic library. This reflects the way
> + glibc's ld.so works. */
> + if (!newdyn && olddyn)
> + newweak = FALSE;
> + if (newdyn)
> + oldweak = FALSE;
> +
> /* It's OK to change the type if either the existing symbol or the
> new symbol is weak. A type change is also OK if the old symbol
> is undefined and the new symbol is defined. */
> @@ -904,17 +914,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
> || h->root.type == bfd_link_hash_undefined)
> *size_change_ok = TRUE;
>
> - /* If a new weak symbol comes from a regular file and the old symbol
> - comes from a dynamic library, we treat the new one as strong.
> - Similarly, an old weak symbol from a regular file is treated as
> - strong when the new symbol comes from a dynamic library. Further,
> - an old weak symbol from a dynamic library is treated as strong if
> - the new symbol is from a DT_NEEDED dynamic library. */
> - if (!newdyn && olddyn)
> - newweak = FALSE;
> - if ((!olddyn || dt_needed) && newdyn)
> - oldweak = FALSE;
> -
> /* NEWDYNCOMMON and OLDDYNCOMMON indicate whether the new or old
> symbol, respectively, appears to be a common symbol in a dynamic
> object. If a symbol appears in an uninitialized section, and is
> @@ -1005,8 +1004,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
> && (olddef
> || (h->root.type == bfd_link_hash_common
> && (newweak
> - || ELF_ST_TYPE (sym->st_info) == STT_FUNC)))
> - && (!oldweak || newweak))
> + || ELF_ST_TYPE (sym->st_info) == STT_FUNC))))
> {
> *override = TRUE;
> newdef = FALSE;
> @@ -1050,10 +1048,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
>
> As above, we again permit a common symbol in a regular object to
> override a definition in a shared object if the shared object
> - symbol is a function or is weak.
> -
> - As above, we permit a non-weak definition in a shared object to
> - override a weak definition in a regular object. */
> + symbol is a function or is weak. */
>
> flip = NULL;
> if (! newdyn
> @@ -1063,8 +1058,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
> || h->type == STT_FUNC)))
> && olddyn
> && olddef
> - && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
> - && (!newweak || oldweak))
> + && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
> {
> /* Change the hash table entry to undefined, and let
> _bfd_generic_link_add_one_symbol do the right thing with the
> @@ -1153,38 +1147,13 @@ _bfd_elf_merge_symbol (bfd *abfd,
> }
> }
>
> - /* Handle the special case of a weak definition in one shared object
> - followed by a non-weak definition in another. We are covering for
> - a deficiency of _bfd_generic_link_add_one_symbol here. A new
> - strong definition of an indirect symbol is treated as a multiple
> - definition even when the indirect symbol points to a weak sym. */
> - if (olddef
> - && oldweak
> - && olddyn
> - && newdef
> - && !newweak
> - && newdyn)
> - {
> - /* To make this work we have to frob the flags so that the rest
> - of the code does not think we are using the old definition. */
> - h->elf_link_hash_flags |= ELF_LINK_HASH_REF_DYNAMIC;
> - h->elf_link_hash_flags &= ~ELF_LINK_HASH_DEF_DYNAMIC;
> -
> - /* If H is the target of an indirection, we want the caller to
> - use H rather than the indirect symbol. Otherwise if we are
> - defining a new indirect symbol we will wind up attaching it
> - to the entry we are overriding. */
> - *sym_hash = h;
> - }
> -
> return TRUE;
> }
>
> /* This function is called to create an indirect symbol from the
> default for the symbol with the default version if needed. The
> symbol is described by H, NAME, SYM, PSEC, VALUE, and OVERRIDE. We
> - set DYNSYM if the new indirect symbol is dynamic. DT_NEEDED
> - indicates if it comes from a DT_NEEDED entry of a shared object. */
> + set DYNSYM if the new indirect symbol is dynamic. */
>
> bfd_boolean
> _bfd_elf_add_default_symbol (bfd *abfd,
> @@ -1195,8 +1164,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
> asection **psec,
> bfd_vma *value,
> bfd_boolean *dynsym,
> - bfd_boolean override,
> - bfd_boolean dt_needed)
> + bfd_boolean override)
> {
> bfd_boolean type_change_ok;
> bfd_boolean size_change_ok;
> @@ -1257,7 +1225,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
> sec = *psec;
> if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &sec, value,
> &hi, &skip, &override, &type_change_ok,
> - &size_change_ok, dt_needed))
> + &size_change_ok))
> return FALSE;
>
> if (skip)
> @@ -1364,7 +1332,7 @@ nondefault:
> sec = *psec;
> if (!_bfd_elf_merge_symbol (abfd, info, shortname, sym, &sec, value,
> &hi, &skip, &override, &type_change_ok,
> - &size_change_ok, dt_needed))
> + &size_change_ok))
> return FALSE;
>
> if (skip)
> Index: bfd/elflink.h
> ===================================================================
> RCS file: /cvs/src/src/bfd/elflink.h,v
> retrieving revision 1.256
> diff -u -p -r1.256 elflink.h
> --- bfd/elflink.h 18 Mar 2004 12:50:19 -0000 1.256
> +++ bfd/elflink.h 19 Mar 2004 00:35:10 -0000
> @@ -147,7 +147,6 @@ elf_link_add_object_symbols (bfd *abfd,
> Elf_Internal_Sym *isym;
> Elf_Internal_Sym *isymend;
> const struct elf_backend_data *bed;
> - bfd_boolean dt_needed;
> bfd_boolean add_needed;
> struct elf_link_hash_table * hash_table;
> bfd_size_type amt;
> @@ -254,7 +253,6 @@ elf_link_add_object_symbols (bfd *abfd,
> }
> }
>
> - dt_needed = FALSE;
> add_needed = TRUE;
> if (! dynamic)
> {
> @@ -290,19 +288,9 @@ elf_link_add_object_symbols (bfd *abfd,
>
> /* If this dynamic lib was specified on the command line with
> --as-needed in effect, then we don't want to add a DT_NEEDED
> - tag unless the lib is actually used.
> - For libs brought in by another lib's DT_NEEDED we do the same,
> - and also modify handling of weak syms. */
> - switch elf_dyn_lib_class (abfd)
> - {
> - case DYN_NORMAL:
> - break;
> - case DYN_DT_NEEDED:
> - dt_needed = TRUE;
> - /* Fall thru */
> - case DYN_AS_NEEDED:
> - add_needed = FALSE;
> - }
> + tag unless the lib is actually used. Similary for libs brought
> + in by another lib's DT_NEEDED. */
> + add_needed = elf_dyn_lib_class (abfd) == DYN_NORMAL;
>
> s = bfd_get_section_by_name (abfd, ".dynamic");
> if (s != NULL)
> @@ -775,8 +763,7 @@ elf_link_add_object_symbols (bfd *abfd,
>
> if (!_bfd_elf_merge_symbol (abfd, info, name, isym, &sec, &value,
> sym_hash, &skip, &override,
> - &type_change_ok, &size_change_ok,
> - dt_needed))
> + &type_change_ok, &size_change_ok))
> goto error_free_vers;
>
> if (skip)
> @@ -1033,7 +1020,7 @@ elf_link_add_object_symbols (bfd *abfd,
> if (definition || h->root.type == bfd_link_hash_common)
> if (!_bfd_elf_add_default_symbol (abfd, info, h, name, isym,
> &sec, &value, &dynsym,
> - override, dt_needed))
> + override))
> goto error_free_vers;
>
> if (definition && !dynamic)
>
> --
> Alan Modra
> IBM OzLabs - Linux Technology Centre