This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[hjl@lucon.org: Re: PATCH: A symbol visibility problem]
- From: "H. J. Lu" <hjl at lucon dot org>
- To: binutils at sources dot redhat dot com
- Date: Fri, 6 Feb 2004 09:20:11 -0800
- Subject: [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 -----