This is the mail archive of the binutils@sourceware.org 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: PATCH: Fix ELF visibility handling


This tidies usage of dynamic_def and dynamic_weak, two flags added a
long time ago http://sourceware.org/ml/binutils/2003-04/msg00239.html
to report errors when symbols referenced by dynamic libraries are
forced local for some reason.  I flipped the sense of dynamic_weak,
calling it ref_dynamic_nonweak (cf. ref_regular_nonweak) since that
simplifies setting of the flag.  There's no need then to special case
the first time setting as was done in _bfd_elf_mark_dynamic_def_weak.
I also moved setting of the flag earlier, before any of the early
returns in _bfd_elf_merge_symbol.  That removes the need to fiddle
with dynamic_def elsewhere.

	* elf-bfd.h (struct elf_link_hash_entry): Delete dynamic_weak.
	Add ref_dynamic_nonweak.
	* elflink.c (_bfd_elf_mark_dynamic_def_weak): Delete.
	(_bfd_elf_merge_symbol): Don't call above function.  Move
	setting of ref_dynamic_nonweak and dynamic_def earlier.  Don't
	clear dynamic_def.
	(elf_link_add_object_symbols): Delete redundant "override" test.
	Don't set dynamic_def here.
	(elf_link_output_extsym): Update.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.354
diff -u -p -r1.354 elf-bfd.h
--- bfd/elf-bfd.h	11 Jan 2013 14:09:58 -0000	1.354
+++ bfd/elf-bfd.h	13 Jan 2013 03:08:34 -0000
@@ -191,8 +191,8 @@ struct elf_link_hash_entry
      FIXME: There is no real need for this field if def_dynamic is never
      cleared and all places that test def_dynamic also test def_regular.  */
   unsigned int dynamic_def : 1;
-  /* Symbol is weak in all shared objects.  */
-  unsigned int dynamic_weak : 1;
+  /* Symbol has a non-weak reference from a shared object.  */
+  unsigned int ref_dynamic_nonweak : 1;
   /* Symbol is referenced with a relocation where C/C++ pointer equality
      matters.  */
   unsigned int pointer_equality_needed : 1;
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.462
diff -u -p -r1.462 elflink.c
--- bfd/elflink.c	11 Jan 2013 14:09:59 -0000	1.462
+++ bfd/elflink.c	13 Jan 2013 05:25:05 -0000
@@ -895,33 +895,6 @@ elf_merge_st_other (bfd *abfd, struct el
     }
 }
 
-/* Mark if a symbol has a definition in a dynamic object or is
-   weak in all dynamic objects.  */
-
-static void
-_bfd_elf_mark_dynamic_def_weak (struct elf_link_hash_entry *h,
-				asection *sec, int bind)
-{
-  if (!h->dynamic_def)
-    {
-      if (!bfd_is_und_section (sec))
-	h->dynamic_def = 1;
-      else
-	{
-	  /* Check if this symbol is weak in all dynamic objects. If it
-	     is the first time we see it in a dynamic object, we mark
-	     if it is weak. Otherwise, we clear it.  */
-	  if (!h->ref_dynamic)
-	    {
-	      if (bind == STB_WEAK)
-		h->dynamic_weak = 1;
-	    }
-	  else if (bind != STB_WEAK)
-	    h->dynamic_weak = 0;
-	}
-    }
-}
-
 /* This function is called when we want to define a new symbol.  It
    handles the various cases which arise when we find a definition in
    a dynamic object, or when there is already a definition in a
@@ -998,10 +971,39 @@ _bfd_elf_merge_symbol (bfd *abfd,
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
   /* We have to check it for every instance since the first few may be
-     refereences and not all compilers emit symbol type for undefined
+     references and not all compilers emit symbol type for undefined
      symbols.  */
   bfd_elf_link_mark_dynamic_symbol (info, h, sym);
 
+  /* NEWDYN and OLDDYN indicate whether the new or old symbol,
+     respectively, is from a dynamic object.  */
+
+  newdyn = (abfd->flags & DYNAMIC) != 0;
+
+  /* ref_dynamic_nonweak and dynamic_def flags track actual undefined
+     syms and defined syms in dynamic libraries respectively.
+     ref_dynamic on the other hand can be set for a symbol defined in
+     a dynamic library, and def_dynamic may not be set;  When the
+     definition in a dynamic lib is overridden by a definition in the
+     executable use of the symbol in the dynamic lib becomes a
+     reference to the executable symbol.  */
+  if (newdyn)
+    {
+      if (bfd_is_und_section (sec))
+	{
+	  if (bind != STB_WEAK)
+	    {
+	      h->ref_dynamic_nonweak = 1;
+	      hi->ref_dynamic_nonweak = 1;
+	    }
+	}
+      else
+	{
+	  h->dynamic_def = 1;
+	  hi->dynamic_def = 1;
+	}
+    }
+
   /* If we just created the symbol, mark it as being an ELF symbol.
      Other than that, there is nothing to do--there is no merge issue
      with a newly defined symbol--so we just return.  */
@@ -1059,11 +1061,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
 	  || !h->def_regular))
     return TRUE;
 
-  /* NEWDYN and OLDDYN indicate whether the new or old symbol,
-     respectively, is from a dynamic object.  */
-
-  newdyn = (abfd->flags & DYNAMIC) != 0;
-
   olddyn = FALSE;
   if (oldbfd != NULL)
     olddyn = (oldbfd->flags & DYNAMIC) != 0;
@@ -1167,16 +1164,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
       return FALSE;
     }
 
-  /* We need to remember if a symbol has a definition in a dynamic
-     object or is weak in all dynamic objects. Internal and hidden
-     visibility will make it unavailable to dynamic objects.  */
-  if (newdyn)
-    {
-      _bfd_elf_mark_dynamic_def_weak (h, sec, bind);
-      if (h != hi)
-	_bfd_elf_mark_dynamic_def_weak (hi, sec, bind);
-    }
-
   /* If the old symbol has non-default visibility, we ignore the new
      definition from a dynamic object.  */
   if (newdyn
@@ -1230,7 +1217,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
 		h->ref_dynamic = 1;
 
 	      h->def_dynamic = 0;
-	      h->dynamic_def = 0;
 	      /* FIXME: Should we check type and size for protected symbol?  */
 	      h->size = 0;
 	      h->type = 0;
@@ -1270,7 +1256,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
       else
 	h->ref_dynamic = 1;
       h->def_dynamic = 0;
-      h->dynamic_def = 0;
       /* FIXME: Should we check type and size for protected symbol?  */
       h->size = 0;
       h->type = 0;
@@ -4190,7 +4175,6 @@ error_free_dyn:
 	    }
 
 	  if (elf_tdata (abfd)->verdef != NULL
-	      && ! override
 	      && vernum > 1
 	      && definition)
 	    h->verinfo.verdef = &elf_tdata (abfd)->verdef[vernum - 1];
@@ -4415,9 +4399,7 @@ error_free_dyn:
 	      else
 		{
 		  h->def_dynamic = 1;
-		  h->dynamic_def = 1;
 		  hi->def_dynamic = 1;
-		  hi->dynamic_def = 1;
 		}
 
 	      /* If the indirect symbol has been forced local, don't
@@ -8811,7 +8793,7 @@ elf_link_output_extsym (struct bfd_hash_
       && h->ref_dynamic
       && h->def_regular
       && !h->dynamic_def
-      && !h->dynamic_weak
+      && h->ref_dynamic_nonweak
       && !elf_link_check_versioned_symbol (flinfo->info, bed, h))
     {
       bfd *def_bfd;

-- 
Alan Modra
Australia Development Lab, IBM


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