PATCH: A symbol visibility problem

[H. J. Lu]

On Wed, Jan 28, 2004 at 11:46:36AM -0800, H. J. Lu wrote:
> On Wed, Jan 28, 2004 at 11:32:42AM +0000, Nick Clifton wrote:
> > Hi H. J. Lu" <hjl@lucon.org> writes:
> > 
> > > The problem is
> > >
> > >       /* If the new symbol with non-default visibility comes from a
> > >          relocatable file and the old definition comes from a dynamic
> > >          object, we remove the old definition.  */
> > >       if ((*sym_hash)->root.type == bfd_link_hash_indirect)
> > >         h = *sym_hash;
> > >       h->root.type = bfd_link_hash_new;
> > >       h->root.u.undef.abfd = NULL;
> > >  
> > > If the new entry with non-default visibility is undefined, then
> > > setting the type to bfd_link_hash_new will lead to the
> > > assertion. Should we use
> > >
> > > 	h->root.type = bfd_link_hash_undefined;
> > >
> > > instead and let _bfd_generic_link_add_one_symbol take care of it?
> > 
> > That seems sensible, but I think only if the new symbol is undefined.
> > 
> > Do you have a patch to propose for this ?
> > 
> 
> It is more complex than I thought. When we remove the old definition
> coming from a DSO, we need to restore the previous state if the new
> symbol is undefined. Set it to bfd_link_hash_undefined is wrong if
> it was referenced before because the new symbol can be weak undefined,
> which may lead to undefined symbol error. We can tell if it has been
> referenced by checking h->root.und_next. The main problem is how to
> maintain the linker hash table undefs list. If h->root.und_next is
> not NULL, h may or may not be on the linker hash table undefs list.
> We need to make sure it is on the linker hash table undefs list and
> the undefs list won't be corrupted by it.
> 
> 

This patch tries to do it right.


H.J.
-------------- next part --------------
2004-01-28  H.J. Lu  <hongjiu.lu@intel.com>

	* elflink.c (_bfd_elf_merge_symbol): Properly handle undefined
	symbols with non-default visibility.

--- bfd/elflink.c.vsb	2003-12-04 10:43:10.000000000 -0800
+++ bfd/elflink.c	2004-01-28 12:24:26.000000000 -0800
@@ -843,8 +843,43 @@ _bfd_elf_merge_symbol (bfd *abfd,
 	 object, we remove the old definition.  */
       if ((*sym_hash)->root.type == bfd_link_hash_indirect)
 	h = *sym_hash;
-      h->root.type = bfd_link_hash_new;
-      h->root.u.undef.abfd = NULL;
+
+      if ((h->root.und_next || info->hash->undefs_tail == &h->root)
+	  && bfd_is_und_section (sec))
+	{
+	  /* If the new symbol is undefined and the old symbol was
+	     also undefined before, we need to handle it ourselves and
+	     make sure _bfd_generic_link_add_one_symbol doesn't mess
+	     up the linker hash table undefs list.  */
+
+	  if (h->root.und_next)
+	    {
+	      /* First make sure it is on the linker hash table undefs
+		 list.  */
+	      struct bfd_link_hash_entry *undef;
+	      
+	      for (undef = info->hash->undefs;
+		   undef != NULL && undef != &h->root;
+		   undef = undef->und_next);
+	      
+	      if (undef != NULL)
+		{
+		  /* Add it to the undefs list if needed.  */
+		  h->root.und_next = NULL;
+		  bfd_link_add_undef (info->hash, &h->root);
+		}
+	    }
+
+	  h->root.type = bfd_link_hash_undefined;
+	  /* FIXME: What if the new symbol is weak undefined?  */
+	  h->root.u.undef.abfd = abfd;
+	}
+      else
+	{
+	  h->root.type = bfd_link_hash_new;
+	  h->root.u.undef.abfd = NULL;
+	}
+
       if (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC)
 	{
 	  h->elf_link_hash_flags &= ~ELF_LINK_HASH_DEF_DYNAMIC;