ELF Symbol Visibility

["Martin v. Loewis" <martin at loewis dot home dot cs dot tu-berlin dot de>]

Hi Folks,

This patch implements symbol visibility according to my understanding
of the new gABI. It is on top of the recent include/elf/common.h
changes.

Symbol visibility allows lookup of symbols to be restricted within the
shared library or executable. It must be implemented both in binutils
and the dynamic linker.

I'm aware of three issues with this patch:

1. assembler support. I've chosen the pseudos .hidden, .internal and
   .protected to represent the visibilities; they work like .weak or
   .globl. I don't know whether the Monterey/whatever assembler does
   it differently.

2. st_other merging. The patch below implements the gABI definition of
   multiple non-zero st_other values: the more restrictive visibility
   wins. If st_other has additional semantics on other architectures
   (STO_ALPHA_STD_GPLOAD, STO_ALPHA_NOPV, V850_OTHER_SDA, ...,
   STO_MIPS16), and if these can be combined with visibility, then
   platform-specific merging needs to occur.

   This is probably specifically bad for V850, where an st_other value
   of, say, 0x3 could indicate either protected visibility, or that
   the symbol had both SDA and ZDA relocations (whatever this is). I
   guess the V850 ABI will need to adopt.

3. making hidden symbols local. The ABI draft says that hidden and
   internal symbols must be removed or turned into local
   symbols. Removing them is not possible when they appear in PLT or
   GOT entries, is it? So I turn them into locals. 

   However, the current CVS glibc will ignore all locals in .dynsym,
   even though it would process a STV_HIDDEN symbol with STB_GLOBAL
   correctly (i.e. not make it available outside the shared library).
   As a result, .hidden symbols in a shared library result in a
   unresolved symbol dynamically.  I'm not sure whether this is a bug
   in the ABI, in glibc, or a misinterpretation of the ABI on my part.

Please let me know if you need further modifications before you can
accept this patch.

Regards,
Martin

Index: bfd/ChangeLog
===================================================================
RCS file: /cvs/binutils/binutils/bfd/ChangeLog,v
retrieving revision 1.309
diff -u -p -r1.309 ChangeLog
--- ChangeLog	1999/12/29 19:50:13	1.309
+++ ChangeLog	1999/12/30 12:30:03
@@ -1,3 +1,10 @@
+1999-12-30  Martin v. Löwis  <loewis@informatik.hu-berlin.de>
+
+	* elflink.c (_bfd_elf_link_record_dynamic_symbol): Process symbol
+	visibility.
+	* elflink.h (elf_link_add_object_symbols): Combine visibilities.
+	* elf.c (bfd_elf_print_symbol): Interpret st_other as visibility.
+
 1999-12-29  Richard Henderson  <rth@cygnus.com>
 
 	* elflink.h (bfd_elf,size_dynamic_sections): Don't export all
Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf-bfd.h,v
retrieving revision 1.16
diff -u -p -r1.16 elf-bfd.h
--- elf-bfd.h	1999/11/26 09:52:23	1.16
+++ elf-bfd.h	1999/12/30 12:30:05
@@ -157,7 +157,7 @@ struct elf_link_hash_entry
   /* Symbol type (STT_NOTYPE, STT_OBJECT, etc.).  */
   char type;
 
-  /* Symbol st_other value.  */
+  /* Symbol st_other value, symbol visibility.  */
   unsigned char other;
 
   /* Hash value of the name computed using the ELF hash function.  */
Index: bfd/elf.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf.c,v
retrieving revision 1.20
diff -u -p -r1.20 elf.c
--- elf.c	1999/12/07 23:51:02	1.20
+++ elf.c	1999/12/30 12:30:10
@@ -761,6 +761,7 @@ bfd_elf_print_symbol (abfd, filep, symbo
 	CONST char *section_name;
 	CONST char *name = NULL;
 	struct elf_backend_data *bed;
+	unsigned char st_other;
 	
 	section_name = symbol->section ? symbol->section->name : "(*none*)";
 
@@ -836,11 +837,22 @@ bfd_elf_print_symbol (abfd, filep, symbo
 	  }
 
 	/* If the st_other field is not zero, print it.  */
-	if (((elf_symbol_type *) symbol)->internal_elf_sym.st_other != 0)
-	  fprintf (file, " 0x%02x",
-		   ((unsigned int)
-		    ((elf_symbol_type *) symbol)->internal_elf_sym.st_other));
-
+	st_other = ((elf_symbol_type *) symbol)->internal_elf_sym.st_other;
+	switch(st_other)
+	  {
+	  case 0:
+	    break;
+	  case STV_INTERNAL:
+	    fprintf (file, " .internal");break;
+	  case STV_HIDDEN:
+	    fprintf (file, " .hidden");break;
+	  case STV_PROTECTED:
+	    fprintf (file, " .protected");break;
+	  default:
+	    /* Some other non-defined flags are also present, so print
+	       everything hex.  */
+	    fprintf (file, " 0x%02x", (unsigned int)st_other);
+	  }
 	fprintf (file, " %s", name);
       }
       break;
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elflink.c,v
retrieving revision 1.7
diff -u -p -r1.7 elflink.c
--- elflink.c	1999/07/13 18:31:16	1.7
+++ elflink.c	1999/12/30 12:30:10
@@ -218,6 +218,25 @@ _bfd_elf_link_record_dynamic_symbol (inf
       boolean copy;
       bfd_size_type indx;
 
+      /* XXX: The ABI draft says the linker must turn hidden and
+	 internal symbols into STB_LOCAL symbols when producing the
+	 DSO. However, if ld.so honors st_other in the dynamic table,
+	 this would not be necessary.  */
+      switch (ELF_ST_VISIBILITY (h->other))
+	{
+	case STV_INTERNAL:case STV_HIDDEN:
+	  if (h->root.type == bfd_link_hash_undefined)
+	    {
+	      /* This symbol must be defined in the shared object or
+		 executable.  */
+	      bfd *abfd = h->root.u.undef.abfd;
+	      char *name = h->root.root.string;
+	      (*info->callbacks->undefined_symbol)
+		(info, name, abfd, bfd_und_section_ptr, 0);
+	    }
+	  h->elf_link_hash_flags |= ELF_LINK_FORCED_LOCAL;
+	}
+
       h->dynindx = elf_hash_table (info)->dynsymcount;
       ++elf_hash_table (info)->dynsymcount;
 
Index: bfd/elflink.h
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elflink.h,v
retrieving revision 1.42
diff -u -p -r1.42 elflink.h
--- elflink.h	1999/12/29 19:50:13	1.42
+++ elflink.h	1999/12/30 12:30:16
@@ -1566,9 +1566,22 @@ elf_link_add_object_symbols (abfd, info)
 	      h->type = ELF_ST_TYPE (sym.st_info);
 	    }
 
-	  if (sym.st_other != 0
-	      && (definition || h->other == 0))
-	    h->other = sym.st_other;
+	  /* If st_other has a processor-specific meaning, specific code
+	     might be needed here.  */
+	  if (sym.st_other != 0)
+	    {
+	      /* Combine visibilities, using the most constraining one. */
+	      unsigned char hvis = ELF_ST_VISIBILITY (h->other);
+	      unsigned char symvis = ELF_ST_VISIBILITY (sym.st_other);
+	      if (symvis && (hvis > symvis || hvis == 0))
+		h->other = sym.st_other;
+	      /* If neither has visibility, use the st_other of the
+	         definition. This is an arbitrary choice, since the
+	         other bits have no general meaning. */
+	      if (!symvis && !hvis
+		  && (definition || h->other == 0))
+		h->other = sym.st_other;
+	    }
 
 	  /* Set a flag in the hash table entry indicating the type of
 	     reference or definition we just found.  Keep a count of
Index: gas/ChangeLog
===================================================================
RCS file: /cvs/binutils/binutils/gas/ChangeLog,v
retrieving revision 1.230
diff -u -p -r1.230 ChangeLog
--- ChangeLog	1999/12/27 16:10:30	1.230
+++ ChangeLog	1999/12/30 12:30:24
@@ -1,3 +1,8 @@
+1999-12-30  Martin v. Löwis  <loewis@informatik.hu-berlin.de>
+
+	* config/obj-elf.c (elf_pseudo_table): Define visibility pseudos.
+	(obj_elf_visibility): New function.
+
 1999-12-27  Alan Modra  <alan@spri.levels.unisa.edu.au>
 
 	* config/tc-i386.c (MATCH): Relax JumpAbsolute check.  Emit a
Index: gas/config/obj-elf.c
===================================================================
RCS file: /cvs/binutils/binutils/gas/config/obj-elf.c,v
retrieving revision 1.10
diff -u -p -r1.10 obj-elf.c
--- obj-elf.c	1999/11/03 22:13:33	1.10
+++ obj-elf.c	1999/12/30 12:30:26
@@ -65,6 +65,7 @@ static void obj_elf_type PARAMS ((int));
 static void obj_elf_ident PARAMS ((int));
 static void obj_elf_weak PARAMS ((int));
 static void obj_elf_local PARAMS ((int));
+static void obj_elf_visibility PARAMS ((int));
 static void obj_elf_symver PARAMS ((int));
 static void obj_elf_vtable_inherit PARAMS ((int));
 static void obj_elf_vtable_entry PARAMS ((int));
@@ -88,6 +89,11 @@ static const pseudo_typeS elf_pseudo_tab
   {"type", obj_elf_type, 0},
   {"version", obj_elf_version, 0},
   {"weak", obj_elf_weak, 0},
+  
+  /* These define symbol visibility. */
+  {"internal", obj_elf_visibility, STV_INTERNAL},
+  {"hidden", obj_elf_visibility, STV_HIDDEN},
+  {"protected", obj_elf_visibility, STV_PROTECTED},
 
   /* These are used for stabs-in-elf configurations.  */
   {"line", obj_elf_line, 0},
@@ -462,6 +468,39 @@ obj_elf_weak (ignore)
       SKIP_WHITESPACE ();
       S_SET_WEAK (symbolP);
       symbol_get_obj (symbolP)->local = 1;
+      if (c == ',')
+	{
+	  input_line_pointer++;
+	  SKIP_WHITESPACE ();
+	  if (*input_line_pointer == '\n')
+	    c = '\n';
+	}
+    }
+  while (c == ',');
+  demand_empty_rest_of_line ();
+}
+
+static void
+obj_elf_visibility (visibility)
+     int visibility;
+{
+  char *name;
+  int c;
+  symbolS *symbolP;
+  asymbol *bfdsym;
+  elf_symbol_type *elfsym;
+
+  do
+    {
+      name = input_line_pointer;
+      c = get_symbol_end ();
+      symbolP = symbol_find_or_make (name);
+      *input_line_pointer = c;
+      SKIP_WHITESPACE ();
+      bfdsym = symbol_get_bfdsym (symbolP);
+      elfsym = elf_symbol_from (bfd_asymbol_bfd(bfdsym), bfdsym);
+      assert(elfsym);
+      elfsym->internal_elf_sym.st_other = visibility;
       if (c == ',')
 	{
 	  input_line_pointer++;