This is the mail archive of the binutils@sources.redhat.com 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: 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


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