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]

[hjl@lucon.org: Re: PATCH: A symbol visibility problem]


Could someone please comment on it?

Thanks.


H.J.
----- Forwarded message from "H. J. Lu" <hjl@lucon.org> -----

Date: Thu, 29 Jan 2004 09:02:17 -0800
From: "H. J. Lu" <hjl@lucon.org>
To: Nick Clifton <nickc@redhat.com>
Cc: binutils@sources.redhat.com
Subject: Re: PATCH: A symbol visibility problem
User-Agent: Mutt/1.4.1i

On Wed, Jan 28, 2004 at 12:30:49PM -0800, H. J. Lu wrote:
> 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.
> 

It turns out we don't need to touch the linker hash table undefs list
since the old definition came from a DSO, which means the symbol
was never removed from the undefs list.


H.J.
-----
2004-01-29  H.J. Lu  <hongjiu.lu@intel.com>

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

--- bfd/elflink.c.undef	2003-12-04 10:43:10.000000000 -0800
+++ bfd/elflink.c	2004-01-29 08:44:36.000000000 -0800
@@ -843,8 +843,26 @@ _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 make sure
+	     _bfd_generic_link_add_one_symbol doesn't mess
+	     up the linker hash table undefs list. Since the old
+	     definition came from a dynamic object, it is still on the
+	     undefs list.  */
+	  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;

----- End forwarded message -----


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