public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* A weak symbol patch
@ 1999-07-06 13:34 H.J. Lu
  1999-07-07 11:20 ` Ian Lance Taylor
  0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 1999-07-06 13:34 UTC (permalink / raw)
  To: Ian Lance Taylor, binutils; +Cc: geoffk

Hi, Ian,

I believe there is a linker bug regarding the weak symbol handling. I
have sent 2 testcases for the bug. Here is a patch. Could you please
take a look?

Thanks.


-- 
H.J. Lu (hjl@gnu.org)
---
Tue Jul  6 13:10:00 1998  H.J. Lu  (hjl@gnu.org)

	* elflink.h (elf_merge_symbol): No type change if the new
	definition is weak and the old one is strong or common.
	(elf_merge_symbol): Use the the strong definition in a dynamic
	object if the old one is weak.
	(elf_merge_symbol): Don't override the strong definition in
	a dynamic object with a weak one from a regular object.
	(elf_link_add_object_symbols): When a definition in a dynamic
	object is used, clear ELF_LINK_HASH_DEF_REGULAR and set
	ELF_LINK_HASH_REF_REGULAR if ELF_LINK_HASH_DEF_REGULAR is set.

Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 elflink.h
--- elflink.h	1999/06/27 01:02:24	1.1.1.3
+++ elflink.h	1999/07/06 20:17:26
@@ -441,7 +441,9 @@ elf_merge_symbol (abfd, info, name, sym,
 
   if (h->root.type == bfd_link_hash_defweak
       || h->root.type == bfd_link_hash_undefweak
-      || bind == STB_WEAK)
+      || (bind == STB_WEAK
+	  && h->root.type != bfd_link_hash_defined
+	  && h->root.type != bfd_link_hash_common))
     *type_change_ok = true;
 
   /* It's OK to change the size if either the existing symbol or the
@@ -497,6 +499,9 @@ elf_merge_symbol (abfd, info, name, sym,
 	      && (bind == STB_WEAK
 		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC))))
     {
+      if (bind != STB_WEAK && h->root.type == bfd_link_hash_defweak)
+      	return true;
+
       *override = true;
       newdef = false;
       newdyncommon = false;
@@ -550,6 +555,12 @@ elf_merge_symbol (abfd, info, name, sym,
       && olddef
       && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
     {
+      if (bind == STB_WEAK)
+        {
+	  *override = true;
+	  return true;
+	}
+
       /* Change the hash table entry to undefined, and let
 	 _bfd_generic_link_add_one_symbol do the right thing with the
 	 new definition.  */
@@ -1343,7 +1354,14 @@ elf_link_add_object_symbols (abfd, info)
 	      if (! definition)
 		new_flag = ELF_LINK_HASH_REF_DYNAMIC;
 	      else
-		new_flag = ELF_LINK_HASH_DEF_DYNAMIC;
+		{
+		  new_flag = ELF_LINK_HASH_DEF_DYNAMIC;
+		  if (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR)
+		    {
+		      h->elf_link_hash_flags &= ~ELF_LINK_HASH_DEF_REGULAR;
+		      h->elf_link_hash_flags |= ELF_LINK_HASH_REF_REGULAR;
+		    }
+	        }
 	      if ((old_flags & (ELF_LINK_HASH_DEF_REGULAR
 				| ELF_LINK_HASH_REF_REGULAR)) != 0
 		  || (h->weakdef != NULL

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A weak symbol patch
  1999-07-06 13:34 A weak symbol patch H.J. Lu
@ 1999-07-07 11:20 ` Ian Lance Taylor
  1999-07-07 13:53   ` H.J. Lu
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Lance Taylor @ 1999-07-07 11:20 UTC (permalink / raw)
  To: hjl; +Cc: binutils, geoffk

   Date: Tue, 6 Jul 1999 13:34:12 -0700 (PDT)
   From: hjl@varesearch.com (H.J. Lu)

   I believe there is a linker bug regarding the weak symbol handling. I
   have sent 2 testcases for the bug. Here is a patch. Could you please
   take a look?

I am about to check in the appended patch, which is based on yours.  I
think this will fix your test case.

Ian

Index: elflink.h
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elflink.h,v
retrieving revision 1.9
diff -u -r1.9 elflink.h
--- elflink.h	1999/07/01 23:20:07	1.9
+++ elflink.h	1999/07/07 18:18:39
@@ -492,14 +492,19 @@
      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.  */
+     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.  */
+
   if (newdyn
       && newdef
       && (olddef
 	  || (h->root.type == bfd_link_hash_common
 	      && (bind == STB_WEAK
-		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC))))
+		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC)))
+      && (h->root.type != bfd_link_hash_defweak
+	  || bind == STB_WEAK))
     {
       *override = true;
       newdef = false;
@@ -543,7 +548,10 @@
 
      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.  */
+     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.  */
 
   if (! newdyn
       && (newdef
@@ -552,7 +560,9 @@
 		  || h->type == STT_FUNC)))
       && olddyn
       && olddef
-      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
+      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
+      && (bind != STB_WEAK
+	  || h->root.type == bfd_link_hash_defweak))
     {
       /* Change the hash table entry to undefined, and let
 	 _bfd_generic_link_add_one_symbol do the right thing with the
@@ -626,6 +636,31 @@
       h->verinfo.vertree = NULL;
     }
 
+  /* 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.  To make
+     this work we have to frob the flags.  */
+  if (olddef
+      && ! olddyn
+      && h->root.type == bfd_link_hash_defweak
+      && newdef
+      && newdyn
+      && bind != STB_WEAK)
+    h->elf_link_hash_flags &= ~ ELF_LINK_HASH_DEF_REGULAR;
+
+  /* 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 to 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
+      && h->root.type != bfd_link_hash_defweak
+      && newdef
+      && ! newdyn
+      && bind == STB_WEAK)
+    *override = true;
+
   return true;
 }
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A weak symbol patch
  1999-07-07 11:20 ` Ian Lance Taylor
@ 1999-07-07 13:53   ` H.J. Lu
  1999-07-07 15:30     ` Ian Lance Taylor
  0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 1999-07-07 13:53 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils

> 
>    Date: Tue, 6 Jul 1999 13:34:12 -0700 (PDT)
>    From: hjl@varesearch.com (H.J. Lu)
> 
>    I believe there is a linker bug regarding the weak symbol handling. I
>    have sent 2 testcases for the bug. Here is a patch. Could you please
>    take a look?
> 
> I am about to check in the appended patch, which is based on yours.  I
> think this will fix your test case.
> 

You missed a piece in mine. If you don't set ELF_LINK_HASH_REF_REGULAR
when clearing ELF_LINK_HASH_DEF_REGULAR, no relocation will be
generated at all and you will get a linker error:

# cc -o wfoo2 -O -g -B./ -DWEAK main.c wbar2.o libfoo.so -Wl,-rpath,.
./ld: /tmp/ccKTop3y.o: warning: unresolvable relocation against symbol `deallocate' from .text section

if deallocate is referenced and defined as weak in main.c.

It should be better to set both ELF_LINK_HASH_REF_REGULAR and
ELF_LINK_HASH_DEF_REGULAR when a symbol is both defined and referenced
in the same object. But I don't think we do it currently.

> Ian
> 
> Index: elflink.h
> ===================================================================
> RCS file: /cvs/binutils/binutils/bfd/elflink.h,v
> retrieving revision 1.9
> diff -u -r1.9 elflink.h
> --- elflink.h	1999/07/01 23:20:07	1.9
> +++ elflink.h	1999/07/07 18:18:39
> @@ -492,14 +492,19 @@
>       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.  */
> +     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.  */
> +
>    if (newdyn
>        && newdef
>        && (olddef
>  	  || (h->root.type == bfd_link_hash_common
>  	      && (bind == STB_WEAK
> -		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC))))
> +		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC)))
> +      && (h->root.type != bfd_link_hash_defweak
> +	  || bind == STB_WEAK))
>      {
>        *override = true;
>        newdef = false;
> @@ -543,7 +548,10 @@
>  
>       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.  */
> +     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.  */
>  
>    if (! newdyn
>        && (newdef
> @@ -552,7 +560,9 @@
>  		  || h->type == STT_FUNC)))
>        && olddyn
>        && olddef
> -      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
> +      && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
> +      && (bind != STB_WEAK
> +	  || h->root.type == bfd_link_hash_defweak))
>      {
>        /* Change the hash table entry to undefined, and let
>  	 _bfd_generic_link_add_one_symbol do the right thing with the
> @@ -626,6 +636,31 @@
>        h->verinfo.vertree = NULL;
>      }
>  
> +  /* 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.  To make
> +     this work we have to frob the flags.  */
> +  if (olddef
> +      && ! olddyn
> +      && h->root.type == bfd_link_hash_defweak
> +      && newdef
> +      && newdyn
> +      && bind != STB_WEAK)
> +    h->elf_link_hash_flags &= ~ ELF_LINK_HASH_DEF_REGULAR;
> +
> +  /* 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 to 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
> +      && h->root.type != bfd_link_hash_defweak
> +      && newdef
> +      && ! newdyn
> +      && bind == STB_WEAK)
> +    *override = true;
> +
>    return true;
>  }
>  
> 

Here is the patch against yours.

-- 
H.J. Lu (hjl@gnu.org)
---
--- elflink.h.ian	Wed Jul  7 13:46:13 1999
+++ elflink.h	Wed Jul  7 13:46:59 1999
@@ -642,7 +642,10 @@ elf_merge_symbol (abfd, info, name, sym,
       && newdef
       && newdyn
       && bind != STB_WEAK)
-    h->elf_link_hash_flags &= ~ ELF_LINK_HASH_DEF_REGULAR;
+    {
+      h->elf_link_hash_flags &= ~ ELF_LINK_HASH_DEF_REGULAR;
+      h->elf_link_hash_flags |= ELF_LINK_HASH_REF_REGULAR;
+    }
 
   /* Handle the special case of a non-weak definition in a shared
      object followed by a weak definition in a regular object.  In

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: A weak symbol patch
  1999-07-07 13:53   ` H.J. Lu
@ 1999-07-07 15:30     ` Ian Lance Taylor
  0 siblings, 0 replies; 4+ messages in thread
From: Ian Lance Taylor @ 1999-07-07 15:30 UTC (permalink / raw)
  To: hjl; +Cc: binutils

   Date: Wed, 7 Jul 1999 13:53:23 -0700 (PDT)
   From: hjl@varesearch.com (H.J. Lu)

   You missed a piece in mine. If you don't set ELF_LINK_HASH_REF_REGULAR
   when clearing ELF_LINK_HASH_DEF_REGULAR, no relocation will be
   generated at all and you will get a linker error:

Thanks for the note.  I tweaked the patch in a couple of ways.  I was
also running into trouble when dealing with weak versioned symbols;
the existing code let it wind up setting up an indirection from a
symbol to itself.

Ian

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~1999-07-07 15:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-07-06 13:34 A weak symbol patch H.J. Lu
1999-07-07 11:20 ` Ian Lance Taylor
1999-07-07 13:53   ` H.J. Lu
1999-07-07 15:30     ` Ian Lance Taylor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).