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: RFC: linker enhancements


Fixes the dt_needed bug.  Removes dead code from _bfd_elf_merge_symbol,
and simplifies weak sym handling, removing some bugs in the process.
For instance, setting *type_change_ok should use the original newweak
and oldweak flags, not the values after twiddling for dynamic linker
behaviour.  Also noticed this one:

      if (olddef)
	{
	   newweakdef = TRUE;

Why ever were we testing olddef here?  Shouldn't it be newdef?

Because of the possbility that this change introduces bugs, I'm not
putting it on the 2.15 branch until we have a week or so of testing.

	* elflink.c (_bfd_elf_merge_symbol): Rewrite weak symbol handling.
	(_bfd_elf_add_default_symbol): Remove indirect BFD_ASSERTs.
	* elflink.h (elf_link_add_object_symbols): Don't clear dt_needed in
	symbol loop.  Instead use add_needed to flag tag as written.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.56
diff -c -p -r1.56 elflink.c
*** bfd/elflink.c	16 Mar 2004 10:31:18 -0000	1.56
--- bfd/elflink.c	18 Mar 2004 03:31:11 -0000
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 676,682 ****
    int bind;
    bfd *oldbfd;
    bfd_boolean newdyn, olddyn, olddef, newdef, newdyncommon, olddyncommon;
!   bfd_boolean newweakdef, oldweakdef, newweakundef, oldweakundef;
  
    *skip = FALSE;
    *override = FALSE;
--- 676,682 ----
    int bind;
    bfd *oldbfd;
    bfd_boolean newdyn, olddyn, olddef, newdef, newdyncommon, olddyncommon;
!   bfd_boolean newweak, oldweak;
  
    *skip = FALSE;
    *override = FALSE;
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 882,929 ****
        return TRUE;
      }
  
!   /* We need to treat weak definition right, depending on if there is a
!      definition from a dynamic object.  */
!   if (bind == STB_WEAK)
!     {
!       if (olddef)
! 	{
! 	   newweakdef = TRUE;
! 	   newweakundef = FALSE;
! 	}
!       else
! 	{
! 	   newweakdef = FALSE;
! 	   newweakundef = TRUE;
! 	}
!     }
!   else
!     newweakdef = newweakundef = FALSE;
  
!   /* If the new weak definition comes from a relocatable file and the
!      old symbol comes from a dynamic object, we treat the new one as
!      strong.  */
!   if (newweakdef && !newdyn && olddyn)
!     newweakdef = FALSE;
  
!   if (h->root.type == bfd_link_hash_defweak)
!     {
!       oldweakdef = TRUE;
!       oldweakundef = FALSE;
!     }
!   else if (h->root.type == bfd_link_hash_undefweak)
!     {
!       oldweakdef = FALSE;
!       oldweakundef = TRUE;
!     }
!   else
!     oldweakdef = oldweakundef = FALSE;
  
!   /* If the old weak definition comes from a relocatable file and the
!      new symbol comes from a dynamic object, we treat the old one as
!      strong.  */
!   if (oldweakdef && !olddyn && newdyn)
!     oldweakdef = FALSE;
  
    /* NEWDYNCOMMON and OLDDYNCOMMON indicate whether the new or old
       symbol, respectively, appears to be a common symbol in a dynamic
--- 882,919 ----
        return TRUE;
      }
  
!   /* Differentiate strong and weak symbols.  */
!   newweak = bind == STB_WEAK;
!   oldweak = (h->root.type == bfd_link_hash_defweak
! 	     || h->root.type == bfd_link_hash_undefweak);
  
!   /* 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.  */
  
!   if (oldweak
!       || newweak
!       || (newdef
! 	  && h->root.type == bfd_link_hash_undefined))
!     *type_change_ok = TRUE;
! 
!   /* It's OK to change the size if either the existing symbol or the
!      new symbol is weak, or if the old symbol is undefined.  */
! 
!   if (*type_change_ok
!       || 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
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 950,960 ****
  
    if (newdyn
        && newdef
        && (sec->flags & SEC_ALLOC) != 0
        && (sec->flags & SEC_LOAD) == 0
        && sym->st_size > 0
-       && !newweakdef
-       && !newweakundef
        && ELF_ST_TYPE (sym->st_info) != STT_FUNC)
      newdyncommon = TRUE;
    else
--- 940,949 ----
  
    if (newdyn
        && newdef
+       && !newweak
        && (sec->flags & SEC_ALLOC) != 0
        && (sec->flags & SEC_LOAD) == 0
        && sym->st_size > 0
        && ELF_ST_TYPE (sym->st_info) != STT_FUNC)
      newdyncommon = TRUE;
    else
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 972,999 ****
    else
      olddyncommon = FALSE;
  
-   /* It's OK to change the type if either the existing symbol or the
-      new symbol is weak unless it comes from a DT_NEEDED entry of
-      a shared object, in which case, the DT_NEEDED entry may not be
-      required at the run time. The type change is also OK if the
-      old symbol is undefined and the new symbol is defined.  */
- 
-   if ((! dt_needed && oldweakdef)
-       || oldweakundef
-       || newweakdef
-       || newweakundef
-       || (newdef
- 	  && (h->root.type == bfd_link_hash_undefined
- 	      || h->root.type == bfd_link_hash_undefweak)))
-     *type_change_ok = TRUE;
- 
-   /* It's OK to change the size if either the existing symbol or the
-      new symbol is weak, or if the old symbol is undefined.  */
- 
-   if (*type_change_ok
-       || h->root.type == bfd_link_hash_undefined)
-     *size_change_ok = TRUE;
- 
    /* If both the old and the new symbols look like common symbols in a
       dynamic object, set the size of the symbol to the larger of the
       two.  */
--- 961,966 ----
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 1031,1054 ****
       represent variables; this can cause confusion in principle, but
       any such confusion would seem to indicate an erroneous program or
       shared library.  We also permit a common symbol in a regular
!      object to override a weak symbol in a shared object.
! 
!      We prefer a non-weak definition in a shared library to a weak
!      definition in the executable unless it comes from a DT_NEEDED
!      entry of a shared object, in which case, the DT_NEEDED entry
!      may not be required at the run time.  */
  
    if (newdyn
        && newdef
        && (olddef
  	  || (h->root.type == bfd_link_hash_common
! 	      && (newweakdef
! 		  || newweakundef
  		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC)))
!       && (!oldweakdef
! 	  || dt_needed
! 	  || newweakdef
! 	  || newweakundef))
      {
        *override = TRUE;
        newdef = FALSE;
--- 998,1012 ----
       represent variables; this can cause confusion in principle, but
       any such confusion would seem to indicate an erroneous program or
       shared library.  We also permit a common symbol in a regular
!      object to override a weak symbol in a shared object.  */
  
    if (newdyn
        && newdef
        && (olddef
  	  || (h->root.type == bfd_link_hash_common
! 	      && (newweak
  		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC)))
!       && (!oldweak || newweak))
      {
        *override = TRUE;
        newdef = FALSE;
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 1101,1111 ****
    if (! newdyn
        && (newdef
  	  || (bfd_is_com_section (sec)
! 	      && (oldweakdef || h->type == STT_FUNC)))
        && olddyn
        && olddef
        && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
!       && ((!newweakdef && !newweakundef) || oldweakdef))
      {
        /* Change the hash table entry to undefined, and let
  	 _bfd_generic_link_add_one_symbol do the right thing with the
--- 1059,1070 ----
    if (! newdyn
        && (newdef
  	  || (bfd_is_com_section (sec)
! 	      && (oldweak
! 		  || h->type == STT_FUNC)))
        && olddyn
        && olddef
        && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
!       && (!newweak || oldweak))
      {
        /* Change the hash table entry to undefined, and let
  	 _bfd_generic_link_add_one_symbol do the right thing with the
*************** _bfd_elf_merge_symbol (bfd *abfd,
*** 1194,1242 ****
  	}
      }
  
-   /* Handle the special case of a weak definition in a regular object
-      followed by a non-weak definition in a shared object.  In this
-      case, we prefer the definition in the shared object unless it
-      comes from a DT_NEEDED entry of a shared object, in which case,
-      the DT_NEEDED entry may not be required at the run time.  */
-   if (olddef
-       && ! dt_needed
-       && oldweakdef
-       && newdef
-       && newdyn
-       && !newweakdef
-       && !newweakundef)
-     {
-       /* To make this work we have to frob the flags so that the rest
- 	 of the code does not think we are using the regular
- 	 definition.  */
-       if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) != 0)
- 	h->elf_link_hash_flags |= ELF_LINK_HASH_REF_REGULAR;
-       else if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
- 	h->elf_link_hash_flags |= ELF_LINK_HASH_REF_DYNAMIC;
-       h->elf_link_hash_flags &= ~ (ELF_LINK_HASH_DEF_REGULAR
- 				   | 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;
-     }
- 
-   /* Handle the special case of a non-weak definition in a shared
-      object followed by a weak definition in a regular object.  In
-      this case we prefer the definition in the shared object.  To make
-      this work we have to tell the caller to not treat the new symbol
-      as a definition.  */
-   if (olddef
-       && olddyn
-       && !oldweakdef
-       && newdef
-       && ! newdyn
-       && (newweakdef || newweakundef))
-     *override = TRUE;
- 
    return TRUE;
  }
  
--- 1153,1158 ----
*************** _bfd_elf_add_default_symbol (bfd *abfd,
*** 1384,1395 ****
      {
        struct elf_link_hash_entry *ht;
  
-       /* If the symbol became indirect, then we assume that we have
- 	 not seen a definition before.  */
-       BFD_ASSERT ((hi->elf_link_hash_flags
- 		   & (ELF_LINK_HASH_DEF_DYNAMIC
- 		      | ELF_LINK_HASH_DEF_REGULAR)) == 0);
- 
        ht = (struct elf_link_hash_entry *) hi->root.u.i.link;
        (*bed->elf_backend_copy_indirect_symbol) (bed, ht, hi);
  
--- 1300,1305 ----
*************** nondefault:
*** 1462,1473 ****
  
        if (hi->root.type == bfd_link_hash_indirect)
  	{
- 	  /* If the symbol became indirect, then we assume that we have
- 	     not seen a definition before.  */
- 	  BFD_ASSERT ((hi->elf_link_hash_flags
- 		       & (ELF_LINK_HASH_DEF_DYNAMIC
- 			  | ELF_LINK_HASH_DEF_REGULAR)) == 0);
- 
  	  (*bed->elf_backend_copy_indirect_symbol) (bed, h, hi);
  
  	  /* See if the new flags lead us to realize that the symbol
--- 1372,1377 ----
Index: bfd/elflink.h
===================================================================
RCS file: /cvs/src/src/bfd/elflink.h,v
retrieving revision 1.254
diff -c -p -r1.254 elflink.h
*** bfd/elflink.h	29 Feb 2004 06:11:52 -0000	1.254
--- bfd/elflink.h	18 Mar 2004 03:31:14 -0000
*************** elf_link_add_object_symbols (bfd *abfd, 
*** 95,100 ****
--- 95,101 ----
    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;
  
*************** elf_link_add_object_symbols (bfd *abfd, 
*** 201,206 ****
--- 202,208 ----
      }
  
    dt_needed = FALSE;
+   add_needed = FALSE;
    if (! dynamic)
      {
        /* If we are creating a shared library, create all the dynamic
*************** elf_link_add_object_symbols (bfd *abfd, 
*** 222,228 ****
    else
      {
        asection *s;
-       bfd_boolean add_needed;
        const char *name;
        bfd_size_type oldsize;
        bfd_size_type strindex;
--- 224,229 ----
*************** elf_link_add_object_symbols (bfd *abfd, 
*** 1050,1056 ****
  		break;
  	      }
  
! 	  if (dt_needed && definition
  	      && (h->elf_link_hash_flags
  		  & ELF_LINK_HASH_REF_REGULAR) != 0)
  	    {
--- 1051,1057 ----
  		break;
  	      }
  
! 	  if (dt_needed && !add_needed && definition
  	      && (h->elf_link_hash_flags
  		  & ELF_LINK_HASH_REF_REGULAR) != 0)
  	    {
*************** elf_link_add_object_symbols (bfd *abfd, 
*** 1061,1067 ****
  		 the regular object to create a dynamic executable. We
  		 have to make sure there is a DT_NEEDED entry for it.  */
  
! 	      dt_needed = FALSE;
  	      oldsize = _bfd_elf_strtab_size (hash_table->dynstr);
  	      strindex = _bfd_elf_strtab_add (hash_table->dynstr,
  					      elf_dt_soname (abfd), FALSE);
--- 1062,1068 ----
  		 the regular object to create a dynamic executable. We
  		 have to make sure there is a DT_NEEDED entry for it.  */
  
! 	      add_needed = TRUE;
  	      oldsize = _bfd_elf_strtab_size (hash_table->dynstr);
  	      strindex = _bfd_elf_strtab_add (hash_table->dynstr,
  					      elf_dt_soname (abfd), FALSE);

-- 
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]