public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gas: Extend .symver directive
@ 2020-04-07 12:10 H.J. Lu
  2020-04-07 12:41 ` Andreas Schwab
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-07 12:10 UTC (permalink / raw)
  To: binutils

Extend .symver directive to update visibility of the original symbol and
assign one original symbol to different versioned symbols:

  .symver foo, foo@VERS_1, local    # Change foo to a local symbol.
  .symver foo, foo@VERS_2, hidden   # Change foo to a hidden symbol.
  .symver foo, foo@@VERS_3, remove  # Remove foo from symbol table.
  .symver foo, bar@V1               # Assign foo to bar@V1 and baz@V2.
  .symver foo, baz@V2

	PR gas/23840
	PR gas/25295
	* NEWS: Mention .symver extension.
	* config/obj-elf.c (obj_elf_copy_symbol): New function.
	(obj_elf_symver): Make a fake local copy of the symbol if there
	is an existing versioned symbol.  Add local and hidden visibility
	support.
	(elf_frob_symbol): Use obj_elf_copy_symbol.  Update the original
	symbol to local, hidden or remove it from the symbol table.
	* config/obj-elf.h (original_visibility): New.
	(elf_obj_sy): Change local to bitfield. Add visibility.
	* doc/as.texi: Update .symver directive.
	* testsuite/gas/symver/symver.exp: Run all *.d tests.
	* testsuite/gas/symver/symver6.d: New file.
	* testsuite/gas/symver/symver7.d: Likewise.
	* testsuite/gas/symver/symver7.s: Likewise.
	* testsuite/gas/symver/symver8.d: Likewise.
	* testsuite/gas/symver/symver8.s: Likewise.
	* testsuite/gas/symver/symver9.s: Likewise.
	* testsuite/gas/symver/symver9a.d: Likewise.
	* testsuite/gas/symver/symver9b.d: Likewise.
	* testsuite/gas/symver/symver10.s: Likewise.
	* testsuite/gas/symver/symver10a.d: Likewise.
	* testsuite/gas/symver/symver10b.d: Likewise.
	* testsuite/gas/symver/symver6.l: Removed.
	* testsuite/gas/symver/symver6.s: Updated.
---
 gas/NEWS                             |   3 +
 gas/config/obj-elf.c                 | 131 +++++++++++++++++++--------
 gas/config/obj-elf.h                 |  13 ++-
 gas/doc/as.texi                      |  15 ++-
 gas/testsuite/gas/symver/symver.exp  |  10 +-
 gas/testsuite/gas/symver/symver10.s  |   8 ++
 gas/testsuite/gas/symver/symver10a.d |   8 ++
 gas/testsuite/gas/symver/symver10b.d |   8 ++
 gas/testsuite/gas/symver/symver6.d   |  11 +++
 gas/testsuite/gas/symver/symver6.l   |   3 -
 gas/testsuite/gas/symver/symver6.s   |   4 +-
 gas/testsuite/gas/symver/symver7.d   |   8 ++
 gas/testsuite/gas/symver/symver7.s   |   8 ++
 gas/testsuite/gas/symver/symver8.d   |   8 ++
 gas/testsuite/gas/symver/symver8.s   |   8 ++
 gas/testsuite/gas/symver/symver9.s   |   8 ++
 gas/testsuite/gas/symver/symver9a.d  |   8 ++
 gas/testsuite/gas/symver/symver9b.d  |   8 ++
 18 files changed, 217 insertions(+), 53 deletions(-)
 create mode 100644 gas/testsuite/gas/symver/symver10.s
 create mode 100644 gas/testsuite/gas/symver/symver10a.d
 create mode 100644 gas/testsuite/gas/symver/symver10b.d
 create mode 100644 gas/testsuite/gas/symver/symver6.d
 delete mode 100644 gas/testsuite/gas/symver/symver6.l
 create mode 100644 gas/testsuite/gas/symver/symver7.d
 create mode 100644 gas/testsuite/gas/symver/symver7.s
 create mode 100644 gas/testsuite/gas/symver/symver8.d
 create mode 100644 gas/testsuite/gas/symver/symver8.s
 create mode 100644 gas/testsuite/gas/symver/symver9.s
 create mode 100644 gas/testsuite/gas/symver/symver9a.d
 create mode 100644 gas/testsuite/gas/symver/symver9b.d

diff --git a/gas/NEWS b/gas/NEWS
index 6748c179f1..58d79caa41 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Extend .symver directive to update visibility of the original symbol
+  and assign one original symbol to different versioned symbols.
+
 * Add support for Intel SERIALIZE and TSXLDTRK instructions.
 
 * Add -mlfence-after-load=, -mlfence-before-indirect-branch= and
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index a6dcdaf4a7..8b14c91b2d 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -1515,6 +1515,35 @@ obj_elf_line (int ignore ATTRIBUTE_UNUSED)
   demand_empty_rest_of_line ();
 }
 
+static symbolS *
+obj_elf_copy_symbol (const char *name2, symbolS *symp)
+{
+  symbolS *symp2 = symbol_find_or_make (name2);
+
+  S_SET_SEGMENT (symp2, S_GET_SEGMENT (symp));
+
+  /* Subtracting out the frag address here is a hack
+     because we are in the middle of the final loop.  */
+  S_SET_VALUE (symp2,
+	       (S_GET_VALUE (symp)
+		- symbol_get_frag (symp)->fr_address));
+
+  symbol_set_frag (symp2, symbol_get_frag (symp));
+
+  /* This will copy over the size information.  */
+  copy_symbol_attributes (symp2, symp);
+
+  S_SET_OTHER (symp2, S_GET_OTHER (symp));
+
+  if (S_IS_WEAK (symp))
+    S_SET_WEAK (symp2);
+
+  if (S_IS_EXTERNAL (symp))
+    S_SET_EXTERNAL (symp2);
+
+  return symp2;
+}
+
 /* This handles the .symver pseudo-op, which is used to specify a
    symbol version.  The syntax is ``.symver NAME,SYMVERNAME''.
    SYMVERNAME may contain ELF_VER_CHR ('@') characters.  This
@@ -1528,6 +1557,7 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
   char c;
   char old_lexat;
   symbolS *sym;
+  symbolS *sym_orig;
 
   sym = get_sym_from_input_line_and_check ();
 
@@ -1555,12 +1585,20 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  sym_orig = sym;
+  if (symbol_get_obj (sym)->versioned_name
+      && strcmp (symbol_get_obj (sym)->versioned_name, name))
+    {
+      /* Make a fake local copy of the symbol if there is an existing
+	 versioned symbol.  */
+      sym = obj_elf_copy_symbol (FAKE_LABEL_NAME, sym);
+      symbol_get_obj (sym)->visibility = visibility_local;
+    }
+
   if (symbol_get_obj (sym)->versioned_name == NULL)
     {
       symbol_get_obj (sym)->versioned_name = xstrdup (name);
 
-      (void) restore_line_pointer (c);
-
       if (strchr (symbol_get_obj (sym)->versioned_name,
 		  ELF_VER_CHR) == NULL)
 	{
@@ -1571,18 +1609,32 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
 	  return;
 	}
     }
-  else
+
+  (void) restore_line_pointer (c);
+
+  if (*input_line_pointer == ',')
     {
-      if (strcmp (symbol_get_obj (sym)->versioned_name, name))
+      char *save = input_line_pointer;
+
+      ++input_line_pointer;
+      SKIP_WHITESPACE ();
+      if (strncmp (input_line_pointer, "local", 5) == 0)
 	{
-	  as_bad (_("multiple versions [`%s'|`%s'] for symbol `%s'"),
-		  name, symbol_get_obj (sym)->versioned_name,
-		  S_GET_NAME (sym));
-	  ignore_rest_of_line ();
-	  return;
+	  input_line_pointer += 5;
+	  symbol_get_obj (sym_orig)->visibility = visibility_local;
 	}
-
-      (void) restore_line_pointer (c);
+      else if (strncmp (input_line_pointer, "hidden", 6) == 0)
+	{
+	  input_line_pointer += 6;
+	  symbol_get_obj (sym_orig)->visibility = visibility_hidden;
+	}
+      else if (strncmp (input_line_pointer, "remove", 6) == 0)
+	{
+	  input_line_pointer += 6;
+	  symbol_get_obj (sym_orig)->visibility = visibility_remove;
+	}
+      else
+	input_line_pointer = save;
     }
 
   demand_empty_rest_of_line ();
@@ -2453,18 +2505,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	    }
 	  else
 	    {
-	      symbolS *symp2;
-
-	      /* FIXME: Creating a new symbol here is risky.  We're
-		 in the final loop over the symbol table.  We can
-		 get away with it only because the symbol goes to
-		 the end of the list, where the loop will still see
-		 it.  It would probably be better to do this in
-		 obj_frob_file_before_adjust.  */
-
-	      symp2 = symbol_find_or_make (sy_obj->versioned_name);
+	      asymbol *bfdsym;
+	      elf_symbol_type *elfsym;
 
-	      /* Now we act as though we saw symp2 = sym.  */
 	      if (S_IS_COMMON (symp))
 		{
 		  as_bad (_("`%s' can't be versioned to common symbol '%s'"),
@@ -2473,26 +2516,34 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 		  return;
 		}
 
-	      S_SET_SEGMENT (symp2, S_GET_SEGMENT (symp));
-
-	      /* Subtracting out the frag address here is a hack
-		 because we are in the middle of the final loop.  */
-	      S_SET_VALUE (symp2,
-			   (S_GET_VALUE (symp)
-			    - symbol_get_frag (symp)->fr_address));
-
-	      symbol_set_frag (symp2, symbol_get_frag (symp));
-
-	      /* This will copy over the size information.  */
-	      copy_symbol_attributes (symp2, symp);
-
-	      S_SET_OTHER (symp2, S_GET_OTHER (symp));
+	      /* FIXME: Creating a new symbol here is risky.  We're
+		 in the final loop over the symbol table.  We can
+		 get away with it only because the symbol goes to
+		 the end of the list, where the loop will still see
+		 it.  It would probably be better to do this in
+		 obj_frob_file_before_adjust.  */
 
-	      if (S_IS_WEAK (symp))
-		S_SET_WEAK (symp2);
+	      obj_elf_copy_symbol (sy_obj->versioned_name, symp);
 
-	      if (S_IS_EXTERNAL (symp))
-		S_SET_EXTERNAL (symp2);
+	      switch (symbol_get_obj (symp)->visibility)
+		{
+		case visibility_unchanged:
+		  break;
+		case visibility_hidden:
+		  bfdsym = symbol_get_bfdsym (symp);
+		  elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym),
+					    bfdsym);
+		  elfsym->internal_elf_sym.st_other &= ~3;
+		  elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
+		  break;
+		case visibility_remove:
+		  /* Change the remooved label to fake label.  */
+		  S_SET_NAME (symp, FAKE_LABEL_NAME);
+		  /* FALLTHROUGH.  */
+		case visibility_local:
+		  S_CLEAR_EXTERNAL (symp);
+		  break;
+		}
 	    }
 	}
     }
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 54af9ebc0e..b85dfbcffe 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -55,11 +55,22 @@ extern int mips_flag_mdebug;
 #endif
 #endif
 
+enum original_visibility
+{
+  visibility_unchanged = 0,
+  visibility_local,
+  visibility_hidden,
+  visibility_remove
+};
+
 /* Additional information we keep for each symbol.  */
 struct elf_obj_sy
 {
   /* Whether the symbol has been marked as local.  */
-  int local;
+  unsigned int local : 1;
+
+  /* Whether visibility of the original symbol should be changed.  */
+  ENUM_BITFIELD (original_visibility) visibility : 2;
 
   /* Use this to keep track of .size expressions that involve
      differences that we can't compute yet.  */
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 0a6727ef84..77f99defbc 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4509,7 +4509,7 @@ Some machine configurations provide additional directives.
 * Struct::			@code{.struct @var{expression}}
 @ifset ELF
 * SubSection::                  @code{.subsection}
-* Symver::                      @code{.symver @var{name},@var{name2@@nodename}}
+* Symver::                      @code{.symver @var{name},@var{name2@@nodename}[,@var{visibility}]}
 @end ifset
 
 @ifset COFF
@@ -7112,9 +7112,9 @@ shared library.
 
 For ELF targets, the @code{.symver} directive can be used like this:
 @smallexample
-.symver @var{name}, @var{name2@@nodename}
+.symver @var{name}, @var{name2@@nodename}[ ,@var{visibility}]
 @end smallexample
-If the symbol @var{name} is defined within the file
+If the original symbol @var{name} is defined within the file
 being assembled, the @code{.symver} directive effectively creates a symbol
 alias with the name @var{name2@@nodename}, and in fact the main reason that we
 just don't try and create a regular alias is that the @var{@@} character isn't
@@ -7127,7 +7127,14 @@ function is being mentioned.  The @var{nodename} portion of the alias should be
 the name of a node specified in the version script supplied to the linker when
 building a shared library.  If you are attempting to override a versioned
 symbol from a shared library, then @var{nodename} should correspond to the
-nodename of the symbol you are trying to override.
+nodename of the symbol you are trying to override.  The optional
+@var{visibility} updates visibility of the original symbol.  The valid
+visibilities are @code{local}, @code {hidden}, and @code {remove}.
+@code{local} makes the original symbol a local symbol (@pxref{Local}).
+@code{hidden} sets the visibility of the original symbol to
+@code{hidden} (@pxref{Hidden}).  @code{remove} removes the original
+symbol from the symbol table.  If visibility isn't specified, the
+original symbol is unchanged.
 
 If the symbol @var{name} is not defined within the file being assembled, all
 references to @var{name} will be changed to @var{name2@@nodename}.  If no
diff --git a/gas/testsuite/gas/symver/symver.exp b/gas/testsuite/gas/symver/symver.exp
index de122eb61c..b33af960da 100644
--- a/gas/testsuite/gas/symver/symver.exp
+++ b/gas/testsuite/gas/symver/symver.exp
@@ -46,8 +46,13 @@ if { [is_elf_format] } then {
       return
     }
 
-    run_dump_test "symver0" 
-    run_dump_test "symver1" 
+    set test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
+    foreach t $test_list {
+	# We need to strip the ".d", but can leave the dirname.
+	verbose [file rootname $t]
+	run_dump_test [file rootname $t]
+    }
+
     run_error_test "symver2" ""
     run_error_test "symver3" ""
     # We have to comment out symver4 and symver5, which check the
@@ -56,5 +61,4 @@ if { [is_elf_format] } then {
     # version name.
 #    run_error_test "symver4" ""
 #    run_error_test "symver5" ""
-    run_error_test "symver6" ""
 }
diff --git a/gas/testsuite/gas/symver/symver10.s b/gas/testsuite/gas/symver/symver10.s
new file mode 100644
index 0000000000..967a692a73
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,remove
+	.symver foo,foo@version1
diff --git a/gas/testsuite/gas/symver/symver10a.d b/gas/testsuite/gas/symver/symver10a.d
new file mode 100644
index 0000000000..e19ed2b0bf
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10a.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver10b.d b/gas/testsuite/gas/symver/symver10b.d
new file mode 100644
index 0000000000..17d0bfdd19
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10b.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
diff --git a/gas/testsuite/gas/symver/symver6.d b/gas/testsuite/gas/symver/symver6.d
new file mode 100644
index 0000000000..cddf7ec703
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver6.d
@@ -0,0 +1,11 @@
+#nm: -n
+#name: symver symver6
+#
+
+#...
+[ 	]+U foo
+#...
+0+00000.. D foo1
+0+0000000 D foo@@version1
+0+00000.. D foo@version1
+0+00000.. d L_foo1
diff --git a/gas/testsuite/gas/symver/symver6.l b/gas/testsuite/gas/symver/symver6.l
deleted file mode 100644
index c2d12ae060..0000000000
--- a/gas/testsuite/gas/symver/symver6.l
+++ /dev/null
@@ -1,3 +0,0 @@
-.*: Assembler messages:
-.*:7: Error: multiple versions \[`foo@version1'|`foo@@version1'\] for symbol `foo'
-#pass
diff --git a/gas/testsuite/gas/symver/symver6.s b/gas/testsuite/gas/symver/symver6.s
index 23d9fe20ee..b0bc0b8b22 100644
--- a/gas/testsuite/gas/symver/symver6.s
+++ b/gas/testsuite/gas/symver/symver6.s
@@ -3,7 +3,7 @@
 	.type foo1,object
 foo1:
 	.long foo
-	.symver foo,foo@@version1
-	.symver foo,foo@version1
+	.symver foo1,foo@@version1
+	.symver foo1,foo@version1
 L_foo1:
 	.size foo1,L_foo1-foo1
diff --git a/gas/testsuite/gas/symver/symver7.d b/gas/testsuite/gas/symver/symver7.d
new file mode 100644
index 0000000000..eb680083da
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.d
@@ -0,0 +1,8 @@
+#readelf: -sW
+#name: symver symver7
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +HIDDEN +[0-9]+ +foo
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver7.s b/gas/testsuite/gas/symver/symver7.s
new file mode 100644
index 0000000000..20c11b7cc0
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,local
+	.symver foo,foo@version1,hidden
diff --git a/gas/testsuite/gas/symver/symver8.d b/gas/testsuite/gas/symver/symver8.d
new file mode 100644
index 0000000000..52c83a7aa3
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.d
@@ -0,0 +1,8 @@
+#readelf: -sW
+#name: symver symver8
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +LOCAL +DEFAULT +[0-9]+ +foo
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver8.s b/gas/testsuite/gas/symver/symver8.s
new file mode 100644
index 0000000000..17ab037040
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,hidden
+	.symver foo,foo@version1,local
diff --git a/gas/testsuite/gas/symver/symver9.s b/gas/testsuite/gas/symver/symver9.s
new file mode 100644
index 0000000000..2f608972f5
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2
+	.symver foo,foo@version1,remove
diff --git a/gas/testsuite/gas/symver/symver9a.d b/gas/testsuite/gas/symver/symver9a.d
new file mode 100644
index 0000000000..62dda20bed
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9a.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver9b.d b/gas/testsuite/gas/symver/symver9b.d
new file mode 100644
index 0000000000..383d1bd080
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9b.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
-- 
2.25.1


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

* Re: [PATCH] gas: Extend .symver directive
  2020-04-07 12:10 [PATCH] gas: Extend .symver directive H.J. Lu
@ 2020-04-07 12:41 ` Andreas Schwab
  2020-04-07 12:49   ` H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2020-04-07 12:41 UTC (permalink / raw)
  To: H.J. Lu via Binutils

On Apr 07 2020, H.J. Lu via Binutils wrote:

> @@ -7112,9 +7112,9 @@ shared library.
>  
>  For ELF targets, the @code{.symver} directive can be used like this:
>  @smallexample
> -.symver @var{name}, @var{name2@@nodename}
> +.symver @var{name}, @var{name2@@nodename}[ ,@var{visibility}]
>  @end smallexample
> -If the symbol @var{name} is defined within the file
> +If the original symbol @var{name} is defined within the file
>  being assembled, the @code{.symver} directive effectively creates a symbol
>  alias with the name @var{name2@@nodename}, and in fact the main reason that we
>  just don't try and create a regular alias is that the @var{@@} character isn't
> @@ -7127,7 +7127,14 @@ function is being mentioned.  The @var{nodename} portion of the alias should be
>  the name of a node specified in the version script supplied to the linker when
>  building a shared library.  If you are attempting to override a versioned
>  symbol from a shared library, then @var{nodename} should correspond to the
> -nodename of the symbol you are trying to override.
> +nodename of the symbol you are trying to override.  The optional
                                                                    argument

> +@var{visibility} updates visibility of the original symbol.  The valid
                           the

> +visibilities are @code{local}, @code {hidden}, and @code {remove}.
> +@code{local} makes the original symbol a local symbol (@pxref{Local}).
> +@code{hidden} sets the visibility of the original symbol to
> +@code{hidden} (@pxref{Hidden}).  @code{remove} removes the original

A sentence should always start with a capitalized word, please reword.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* Re: [PATCH] gas: Extend .symver directive
  2020-04-07 12:41 ` Andreas Schwab
@ 2020-04-07 12:49   ` H.J. Lu
  2020-04-07 12:55     ` Andreas Schwab
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-07 12:49 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: H.J. Lu via Binutils

On Tue, Apr 7, 2020 at 5:41 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Apr 07 2020, H.J. Lu via Binutils wrote:
>
> > @@ -7112,9 +7112,9 @@ shared library.
> >
> >  For ELF targets, the @code{.symver} directive can be used like this:
> >  @smallexample
> > -.symver @var{name}, @var{name2@@nodename}
> > +.symver @var{name}, @var{name2@@nodename}[ ,@var{visibility}]
> >  @end smallexample
> > -If the symbol @var{name} is defined within the file
> > +If the original symbol @var{name} is defined within the file
> >  being assembled, the @code{.symver} directive effectively creates a symbol
> >  alias with the name @var{name2@@nodename}, and in fact the main reason that we
> >  just don't try and create a regular alias is that the @var{@@} character isn't
> > @@ -7127,7 +7127,14 @@ function is being mentioned.  The @var{nodename} portion of the alias should be
> >  the name of a node specified in the version script supplied to the linker when
> >  building a shared library.  If you are attempting to override a versioned
> >  symbol from a shared library, then @var{nodename} should correspond to the
> > -nodename of the symbol you are trying to override.
> > +nodename of the symbol you are trying to override.  The optional
>                                                                     argument
>
> > +@var{visibility} updates visibility of the original symbol.  The valid
>                            the
>
> > +visibilities are @code{local}, @code {hidden}, and @code {remove}.
> > +@code{local} makes the original symbol a local symbol (@pxref{Local}).
> > +@code{hidden} sets the visibility of the original symbol to
> > +@code{hidden} (@pxref{Hidden}).  @code{remove} removes the original
>
> A sentence should always start with a capitalized word, please reword.

How about this?

The optional argument VISIBILITY updates the visibility of
the original symbol.  The valid visibilities are 'local', 'hidden', and
'remove'.  The 'local' visibility makes the original symbol a local
symbol (*note Local::).  The 'hidden' visibility sets the visibility of
the original symbol to 'hidden' (*note Hidden::).  The 'remove'
visibility removes the original symbol from the symbol table.  If
visibility isn't specified, the original symbol is unchanged.

-- 
H.J.

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

* Re: [PATCH] gas: Extend .symver directive
  2020-04-07 12:49   ` H.J. Lu
@ 2020-04-07 12:55     ` Andreas Schwab
  2020-04-07 12:57       ` V2 " H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2020-04-07 12:55 UTC (permalink / raw)
  To: H.J. Lu; +Cc: H.J. Lu via Binutils

On Apr 07 2020, H.J. Lu wrote:

> The optional argument VISIBILITY updates the visibility of
> the original symbol.  The valid visibilities are 'local', 'hidden', and
> 'remove'.  The 'local' visibility makes the original symbol a local
> symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> visibility removes the original symbol from the symbol table.  If
> visibility isn't specified, the original symbol is unchanged.

Looks good.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* V2 [PATCH] gas: Extend .symver directive
  2020-04-07 12:55     ` Andreas Schwab
@ 2020-04-07 12:57       ` H.J. Lu
  2020-04-07 21:22         ` Fangrui Song
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-07 12:57 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: H.J. Lu via Binutils

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Apr 07 2020, H.J. Lu wrote:
>
> > The optional argument VISIBILITY updates the visibility of
> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> > 'remove'.  The 'local' visibility makes the original symbol a local
> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> > visibility removes the original symbol from the symbol table.  If
> > visibility isn't specified, the original symbol is unchanged.
>
> Looks good.

Here is the updated patch.

-- 
H.J.

[-- Attachment #2: 0001-gas-Extend-.symver-directive.patch --]
[-- Type: application/x-patch, Size: 18129 bytes --]

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

* Re: V2 [PATCH] gas: Extend .symver directive
  2020-04-07 12:57       ` V2 " H.J. Lu
@ 2020-04-07 21:22         ` Fangrui Song
  2020-04-07 23:15           ` H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song @ 2020-04-07 21:22 UTC (permalink / raw)
  To: H.J. Lu via Binutils; +Cc: Andreas Schwab

On 2020-04-07, H.J. Lu via Binutils wrote:
>On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>>
>> On Apr 07 2020, H.J. Lu wrote:
>>
>> > The optional argument VISIBILITY updates the visibility of
>> > the original symbol.  The valid visibilities are 'local', 'hidden', and
>> > 'remove'.  The 'local' visibility makes the original symbol a local
>> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
>> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
>> > visibility removes the original symbol from the symbol table.  If
>> > visibility isn't specified, the original symbol is unchanged.
>>
>> Looks good.
>
>Here is the updated patch.
>
>-- 
>H.J.

Let me summarize the current status:

@@@ has the renaming semantics. (invented in 2000)
@ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.

We have received some requests:

* Have a way to not retain the original symbol
* Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`


We have many choices.

A. make @ @@ similar to @@@
   For @@, because of the linking rule (a @@ definition can satisfy a
   referenced without a version), there should be no difference.

   For @, this will be a semantic break. Personally I don't think this
   matters. I believe 99% projects don't need the original symbol and
   will not notice anything. I also checked with FreeBSD developers.

   However, given the general conservative attitude of the binutils
   community, I think there might be some objection.

   Reusing the same stem name is very likely a mistake.
   
   .globl foo_v1
   .symver foo_v1,foo@v1
   foo_v1:
   
   A non-default version can be used to reject static archive linking
   while keep shared object compatibility. An undefined reference foo
   without a version cannot be satisfied by the definition.

   Keeping `foo` + .symver foo,foo@v1 will nullify the use case.

B. Add an optional argument to change binding/visibility or remove the
   original symbol (this patch).
   I am mostly worried that this makes the .symver semantics even harder
   to understand / explain in the documentation.

   There seems to make @@@ even more inconsistent with @/@@.

   BTW, have you checked .localentry (which can set the non-visibility
   bits of st_other)

   I ask for clarification from GCC developers why some special
   attributes (local/hidden) are needed, not other general attribute
   flags.

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

* Re: V2 [PATCH] gas: Extend .symver directive
  2020-04-07 21:22         ` Fangrui Song
@ 2020-04-07 23:15           ` H.J. Lu
  2020-04-07 23:58             ` Fangrui Song
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-07 23:15 UTC (permalink / raw)
  To: Fangrui Song; +Cc: H.J. Lu via Binutils, Andreas Schwab

On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-04-07, H.J. Lu via Binutils wrote:
> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >>
> >> On Apr 07 2020, H.J. Lu wrote:
> >>
> >> > The optional argument VISIBILITY updates the visibility of
> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> >> > 'remove'.  The 'local' visibility makes the original symbol a local
> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> >> > visibility removes the original symbol from the symbol table.  If
> >> > visibility isn't specified, the original symbol is unchanged.
> >>
> >> Looks good.
> >
> >Here is the updated patch.
> >
> >--
> >H.J.
>
> Let me summarize the current status:
>
> @@@ has the renaming semantics. (invented in 2000)
> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
>
> We have received some requests:
>
> * Have a way to not retain the original symbol
> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
>
>
> We have many choices.
>
> A. make @ @@ similar to @@@
>    For @@, because of the linking rule (a @@ definition can satisfy a
>    referenced without a version), there should be no difference.
>
>    For @, this will be a semantic break. Personally I don't think this
>    matters. I believe 99% projects don't need the original symbol and
>    will not notice anything. I also checked with FreeBSD developers.

The original symbol name is used in glibc to bypass PLT within
libc.so.

>    However, given the general conservative attitude of the binutils
>    community, I think there might be some objection.
>
>    Reusing the same stem name is very likely a mistake.
>
>    .globl foo_v1
>    .symver foo_v1,foo@v1
>    foo_v1:
>
>    A non-default version can be used to reject static archive linking
>    while keep shared object compatibility. An undefined reference foo
>    without a version cannot be satisfied by the definition.
>
>    Keeping `foo` + .symver foo,foo@v1 will nullify the use case.
>
> B. Add an optional argument to change binding/visibility or remove the
>    original symbol (this patch).
>    I am mostly worried that this makes the .symver semantics even harder
>    to understand / explain in the documentation.
>
>    There seems to make @@@ even more inconsistent with @/@@.
>
>    BTW, have you checked .localentry (which can set the non-visibility
>    bits of st_other)

That is a PPC specific directive.

>    I ask for clarification from GCC developers why some special
>    attributes (local/hidden) are needed, not other general attribute
>    flags.

.symver was designed to be used in assembly codes for glibc.
Only recently GCC is trying to use it.  That is one reason why
it is the way it is.

-- 
H.J.

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

* Re: V2 [PATCH] gas: Extend .symver directive
  2020-04-07 23:15           ` H.J. Lu
@ 2020-04-07 23:58             ` Fangrui Song
  2020-04-08 12:53               ` H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song @ 2020-04-07 23:58 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils, Andreas Schwab

On 2020-04-07, H.J. Lu wrote:
>On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On 2020-04-07, H.J. Lu via Binutils wrote:
>> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>> >>
>> >> On Apr 07 2020, H.J. Lu wrote:
>> >>
>> >> > The optional argument VISIBILITY updates the visibility of
>> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
>> >> > 'remove'.  The 'local' visibility makes the original symbol a local
>> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
>> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
>> >> > visibility removes the original symbol from the symbol table.  If
>> >> > visibility isn't specified, the original symbol is unchanged.
>> >>
>> >> Looks good.
>> >
>> >Here is the updated patch.
>> >
>> >--
>> >H.J.
>>
>> Let me summarize the current status:
>>
>> @@@ has the renaming semantics. (invented in 2000)
>> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
>>
>> We have received some requests:
>>
>> * Have a way to not retain the original symbol
>> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
>>
>>
>> We have many choices.
>>
>> A. make @ @@ similar to @@@
>>    For @@, because of the linking rule (a @@ definition can satisfy a
>>    referenced without a version), there should be no difference.
>>
>>    For @, this will be a semantic break. Personally I don't think this
>>    matters. I believe 99% projects don't need the original symbol and
>>    will not notice anything. I also checked with FreeBSD developers.
>
>The original symbol name is used in glibc to bypass PLT within
>libc.so.

This does not seem correct. By convention, the hidden aliases are those prefixed with __
They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
The original symbol does not have the prefix.

When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
"memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
"memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.

If the definition of "memcpy" also exists, I think it is somewhat special cased
in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
with indirect symbol involved. I believe the whole thing can be simplified a
lot by using renaming semantic. I debugged this area last year and may
misremember something.

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

* Re: V2 [PATCH] gas: Extend .symver directive
  2020-04-07 23:58             ` Fangrui Song
@ 2020-04-08 12:53               ` H.J. Lu
  2020-04-08 13:21                 ` V3 " H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-08 12:53 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils, Andreas Schwab

On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-04-07, H.J. Lu wrote:
> >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
> >>
> >> On 2020-04-07, H.J. Lu via Binutils wrote:
> >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >> >>
> >> >> On Apr 07 2020, H.J. Lu wrote:
> >> >>
> >> >> > The optional argument VISIBILITY updates the visibility of
> >> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> >> >> > 'remove'.  The 'local' visibility makes the original symbol a local
> >> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> >> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> >> >> > visibility removes the original symbol from the symbol table.  If
> >> >> > visibility isn't specified, the original symbol is unchanged.
> >> >>
> >> >> Looks good.
> >> >
> >> >Here is the updated patch.
> >> >
> >> >--
> >> >H.J.
> >>
> >> Let me summarize the current status:
> >>
> >> @@@ has the renaming semantics. (invented in 2000)
> >> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
> >>
> >> We have received some requests:
> >>
> >> * Have a way to not retain the original symbol
> >> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
> >>
> >>
> >> We have many choices.
> >>
> >> A. make @ @@ similar to @@@
> >>    For @@, because of the linking rule (a @@ definition can satisfy a
> >>    referenced without a version), there should be no difference.
> >>
> >>    For @, this will be a semantic break. Personally I don't think this
> >>    matters. I believe 99% projects don't need the original symbol and
> >>    will not notice anything. I also checked with FreeBSD developers.
> >
> >The original symbol name is used in glibc to bypass PLT within
> >libc.so.
>
> This does not seem correct. By convention, the hidden aliases are those prefixed with __
> They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
> The original symbol does not have the prefix.
>
> When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
> "memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
> "memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.

As I said before, the original purpose of .symver is for glibc to
implement symbol
versioning.  Given:

---
const int _sys_nerr_internal = 200;
__asm__ (".symver _sys_nerr_internal, sys_nerr@@GLIBC_2.12");
---

_sys_nerr_internal is used internally in glibc:

File: libc_pic.a(_strerror.os)

Relocation section '.rela.text' at offset 0x14e0 contains 17 entries:
    Offset             Info             Type               Symbol's
Value  Symbol's Name + Addend
000000000000001c  0000001500000002 R_X86_64_PC32
0000000000000000 _sys_nerr_internal - 4
000000000000002c  0000001600000002 R_X86_64_PC32
0000000000000000 _sys_errlist_internal - 4

---
char *
__strerror_r (int errnum, char *buf, size_t buflen)
{
  ...
  return (char *) _(_sys_errlist_internal[errnum]);
}
---

Also with -g, _sys_nerr_internal is also referenced in debug info.  We
just can't change .symver to rename.

> If the definition of "memcpy" also exists, I think it is somewhat special cased
> in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
> with indirect symbol involved. I believe the whole thing can be simplified a
> lot by using renaming semantic. I debugged this area last year and may
> misremember something.

It is OK to extend .symver directive.  Change it to rename semantic isn't an
option.

-- 
H.J.

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

* V3 [PATCH] gas: Extend .symver directive
  2020-04-08 12:53               ` H.J. Lu
@ 2020-04-08 13:21                 ` H.J. Lu
  2020-04-09 17:16                   ` Fangrui Song
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-08 13:21 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils, Andreas Schwab

[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]

On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <i@maskray.me> wrote:
> >
> > On 2020-04-07, H.J. Lu wrote:
> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
> > >>
> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> > >> >>
> > >> >> On Apr 07 2020, H.J. Lu wrote:
> > >> >>
> > >> >> > The optional argument VISIBILITY updates the visibility of
> > >> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> > >> >> > 'remove'.  The 'local' visibility makes the original symbol a local
> > >> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> > >> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> > >> >> > visibility removes the original symbol from the symbol table.  If
> > >> >> > visibility isn't specified, the original symbol is unchanged.
> > >> >>
> > >> >> Looks good.
> > >> >
> > >> >Here is the updated patch.
> > >> >
> > >> >--
> > >> >H.J.
> > >>
> > >> Let me summarize the current status:
> > >>
> > >> @@@ has the renaming semantics. (invented in 2000)
> > >> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
> > >>
> > >> We have received some requests:
> > >>
> > >> * Have a way to not retain the original symbol
> > >> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
> > >>
> > >>
> > >> We have many choices.
> > >>
> > >> A. make @ @@ similar to @@@
> > >>    For @@, because of the linking rule (a @@ definition can satisfy a
> > >>    referenced without a version), there should be no difference.
> > >>
> > >>    For @, this will be a semantic break. Personally I don't think this
> > >>    matters. I believe 99% projects don't need the original symbol and
> > >>    will not notice anything. I also checked with FreeBSD developers.
> > >
> > >The original symbol name is used in glibc to bypass PLT within
> > >libc.so.
> >
> > This does not seem correct. By convention, the hidden aliases are those prefixed with __
> > They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
> > The original symbol does not have the prefix.
> >
> > When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
> > "memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
> > "memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.
>
> As I said before, the original purpose of .symver is for glibc to
> implement symbol
> versioning.  Given:
>
> ---
> const int _sys_nerr_internal = 200;
> __asm__ (".symver _sys_nerr_internal, sys_nerr@@GLIBC_2.12");
> ---
>
> _sys_nerr_internal is used internally in glibc:
>
> File: libc_pic.a(_strerror.os)
>
> Relocation section '.rela.text' at offset 0x14e0 contains 17 entries:
>     Offset             Info             Type               Symbol's
> Value  Symbol's Name + Addend
> 000000000000001c  0000001500000002 R_X86_64_PC32
> 0000000000000000 _sys_nerr_internal - 4
> 000000000000002c  0000001600000002 R_X86_64_PC32
> 0000000000000000 _sys_errlist_internal - 4
>
> ---
> char *
> __strerror_r (int errnum, char *buf, size_t buflen)
> {
>   ...
>   return (char *) _(_sys_errlist_internal[errnum]);
> }
> ---
>
> Also with -g, _sys_nerr_internal is also referenced in debug info.  We
> just can't change .symver to rename.
>
> > If the definition of "memcpy" also exists, I think it is somewhat special cased
> > in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
> > with indirect symbol involved. I believe the whole thing can be simplified a
> > lot by using renaming semantic. I debugged this area last year and may
> > misremember something.
>
> It is OK to extend .symver directive.  Change it to rename semantic isn't an
> option.
>

Updated patch to call symbol_remove to remove the symbol
if it isn't used in relocation.

-- 
H.J.

[-- Attachment #2: 0001-gas-Extend-.symver-directive.patch --]
[-- Type: text/x-patch, Size: 19302 bytes --]

From 42e4d9ac4a880fb6ad829a2959b992df9b816349 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 6 Apr 2020 11:30:53 -0700
Subject: [PATCH] gas: Extend .symver directive

Extend .symver directive to update visibility of the original symbol and
assign one original symbol to different versioned symbols:

  .symver foo, foo@VERS_1, local    # Change foo to a local symbol.
  .symver foo, foo@VERS_2, hidden   # Change foo to a hidden symbol.
  .symver foo, foo@@VERS_3, remove  # Remove foo from symbol table.
  .symver foo, bar@V1               # Assign foo to bar@V1 and baz@V2.
  .symver foo, baz@V2

	PR gas/23840
	PR gas/25295
	* NEWS: Mention .symver extension.
	* config/obj-elf.c (obj_elf_copy_symbol): New function.
	(obj_elf_symver): Make a fake copy of the symbol and mark it to
	be removed if there is an existing versioned symbol.  Add local,
	hidden and remove visibility support.
	(elf_frob_symbol): Use obj_elf_copy_symbol.  Update the original
	symbol to local, hidden or remove it from the symbol table.
	* config/obj-elf.h (original_visibility): New.
	(elf_obj_sy): Change local to bitfield. Add visibility.
	* doc/as.texi: Update .symver directive.
	* testsuite/gas/symver/symver.exp: Run all *.d tests.
	* testsuite/gas/symver/symver6.d: New file.
	* testsuite/gas/symver/symver7.d: Likewise.
	* testsuite/gas/symver/symver7.s: Likewise.
	* testsuite/gas/symver/symver8.d: Likewise.
	* testsuite/gas/symver/symver8.s: Likewise.
	* testsuite/gas/symver/symver9.s: Likewise.
	* testsuite/gas/symver/symver9a.d: Likewise.
	* testsuite/gas/symver/symver9b.d: Likewise.
	* testsuite/gas/symver/symver10.s: Likewise.
	* testsuite/gas/symver/symver10a.d: Likewise.
	* testsuite/gas/symver/symver10b.d: Likewise.
	* testsuite/gas/symver/symver11.d: Likewise.
	* testsuite/gas/symver/symver11.s: Likewise.
	* testsuite/gas/symver/symver6.l: Removed.
	* testsuite/gas/symver/symver6.s: Updated.
---
 gas/NEWS                             |   3 +
 gas/config/obj-elf.c                 | 132 +++++++++++++++++++--------
 gas/config/obj-elf.h                 |  13 ++-
 gas/doc/as.texi                      |  16 +++-
 gas/testsuite/gas/symver/symver.exp  |  10 +-
 gas/testsuite/gas/symver/symver10.s  |   8 ++
 gas/testsuite/gas/symver/symver10a.d |   8 ++
 gas/testsuite/gas/symver/symver10b.d |   8 ++
 gas/testsuite/gas/symver/symver11.d  |   8 ++
 gas/testsuite/gas/symver/symver11.s  |   9 ++
 gas/testsuite/gas/symver/symver6.d   |  11 +++
 gas/testsuite/gas/symver/symver6.l   |   3 -
 gas/testsuite/gas/symver/symver6.s   |   4 +-
 gas/testsuite/gas/symver/symver7.d   |   8 ++
 gas/testsuite/gas/symver/symver7.s   |   8 ++
 gas/testsuite/gas/symver/symver8.d   |   9 ++
 gas/testsuite/gas/symver/symver8.s   |   8 ++
 gas/testsuite/gas/symver/symver9.s   |   8 ++
 gas/testsuite/gas/symver/symver9a.d  |   8 ++
 gas/testsuite/gas/symver/symver9b.d  |   8 ++
 20 files changed, 237 insertions(+), 53 deletions(-)
 create mode 100644 gas/testsuite/gas/symver/symver10.s
 create mode 100644 gas/testsuite/gas/symver/symver10a.d
 create mode 100644 gas/testsuite/gas/symver/symver10b.d
 create mode 100644 gas/testsuite/gas/symver/symver11.d
 create mode 100644 gas/testsuite/gas/symver/symver11.s
 create mode 100644 gas/testsuite/gas/symver/symver6.d
 delete mode 100644 gas/testsuite/gas/symver/symver6.l
 create mode 100644 gas/testsuite/gas/symver/symver7.d
 create mode 100644 gas/testsuite/gas/symver/symver7.s
 create mode 100644 gas/testsuite/gas/symver/symver8.d
 create mode 100644 gas/testsuite/gas/symver/symver8.s
 create mode 100644 gas/testsuite/gas/symver/symver9.s
 create mode 100644 gas/testsuite/gas/symver/symver9a.d
 create mode 100644 gas/testsuite/gas/symver/symver9b.d

diff --git a/gas/NEWS b/gas/NEWS
index 6748c179f1..58d79caa41 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Extend .symver directive to update visibility of the original symbol
+  and assign one original symbol to different versioned symbols.
+
 * Add support for Intel SERIALIZE and TSXLDTRK instructions.
 
 * Add -mlfence-after-load=, -mlfence-before-indirect-branch= and
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index a6dcdaf4a7..a53a97fda1 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -1515,6 +1515,35 @@ obj_elf_line (int ignore ATTRIBUTE_UNUSED)
   demand_empty_rest_of_line ();
 }
 
+static symbolS *
+obj_elf_copy_symbol (const char *name2, symbolS *symp)
+{
+  symbolS *symp2 = symbol_find_or_make (name2);
+
+  S_SET_SEGMENT (symp2, S_GET_SEGMENT (symp));
+
+  /* Subtracting out the frag address here is a hack
+     because we are in the middle of the final loop.  */
+  S_SET_VALUE (symp2,
+	       (S_GET_VALUE (symp)
+		- symbol_get_frag (symp)->fr_address));
+
+  symbol_set_frag (symp2, symbol_get_frag (symp));
+
+  /* This will copy over the size information.  */
+  copy_symbol_attributes (symp2, symp);
+
+  S_SET_OTHER (symp2, S_GET_OTHER (symp));
+
+  if (S_IS_WEAK (symp))
+    S_SET_WEAK (symp2);
+
+  if (S_IS_EXTERNAL (symp))
+    S_SET_EXTERNAL (symp2);
+
+  return symp2;
+}
+
 /* This handles the .symver pseudo-op, which is used to specify a
    symbol version.  The syntax is ``.symver NAME,SYMVERNAME''.
    SYMVERNAME may contain ELF_VER_CHR ('@') characters.  This
@@ -1528,6 +1557,7 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
   char c;
   char old_lexat;
   symbolS *sym;
+  symbolS *sym_orig;
 
   sym = get_sym_from_input_line_and_check ();
 
@@ -1555,12 +1585,20 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
       return;
     }
 
+  sym_orig = sym;
+  if (symbol_get_obj (sym)->versioned_name
+      && strcmp (symbol_get_obj (sym)->versioned_name, name))
+    {
+      /* Make a fake copy of the symbol and mark it to be removed if
+	 there is an existing versioned symbol.  */
+      sym = obj_elf_copy_symbol (FAKE_LABEL_NAME, sym);
+      symbol_get_obj (sym)->visibility = visibility_remove;
+    }
+
   if (symbol_get_obj (sym)->versioned_name == NULL)
     {
       symbol_get_obj (sym)->versioned_name = xstrdup (name);
 
-      (void) restore_line_pointer (c);
-
       if (strchr (symbol_get_obj (sym)->versioned_name,
 		  ELF_VER_CHR) == NULL)
 	{
@@ -1571,18 +1609,32 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
 	  return;
 	}
     }
-  else
+
+  (void) restore_line_pointer (c);
+
+  if (*input_line_pointer == ',')
     {
-      if (strcmp (symbol_get_obj (sym)->versioned_name, name))
+      char *save = input_line_pointer;
+
+      ++input_line_pointer;
+      SKIP_WHITESPACE ();
+      if (strncmp (input_line_pointer, "local", 5) == 0)
 	{
-	  as_bad (_("multiple versions [`%s'|`%s'] for symbol `%s'"),
-		  name, symbol_get_obj (sym)->versioned_name,
-		  S_GET_NAME (sym));
-	  ignore_rest_of_line ();
-	  return;
+	  input_line_pointer += 5;
+	  symbol_get_obj (sym_orig)->visibility = visibility_local;
 	}
-
-      (void) restore_line_pointer (c);
+      else if (strncmp (input_line_pointer, "hidden", 6) == 0)
+	{
+	  input_line_pointer += 6;
+	  symbol_get_obj (sym_orig)->visibility = visibility_hidden;
+	}
+      else if (strncmp (input_line_pointer, "remove", 6) == 0)
+	{
+	  input_line_pointer += 6;
+	  symbol_get_obj (sym_orig)->visibility = visibility_remove;
+	}
+      else
+	input_line_pointer = save;
     }
 
   demand_empty_rest_of_line ();
@@ -2453,18 +2505,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	    }
 	  else
 	    {
-	      symbolS *symp2;
+	      asymbol *bfdsym;
+	      elf_symbol_type *elfsym;
 
-	      /* FIXME: Creating a new symbol here is risky.  We're
-		 in the final loop over the symbol table.  We can
-		 get away with it only because the symbol goes to
-		 the end of the list, where the loop will still see
-		 it.  It would probably be better to do this in
-		 obj_frob_file_before_adjust.  */
-
-	      symp2 = symbol_find_or_make (sy_obj->versioned_name);
-
-	      /* Now we act as though we saw symp2 = sym.  */
 	      if (S_IS_COMMON (symp))
 		{
 		  as_bad (_("`%s' can't be versioned to common symbol '%s'"),
@@ -2473,26 +2516,35 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 		  return;
 		}
 
-	      S_SET_SEGMENT (symp2, S_GET_SEGMENT (symp));
-
-	      /* Subtracting out the frag address here is a hack
-		 because we are in the middle of the final loop.  */
-	      S_SET_VALUE (symp2,
-			   (S_GET_VALUE (symp)
-			    - symbol_get_frag (symp)->fr_address));
-
-	      symbol_set_frag (symp2, symbol_get_frag (symp));
-
-	      /* This will copy over the size information.  */
-	      copy_symbol_attributes (symp2, symp);
-
-	      S_SET_OTHER (symp2, S_GET_OTHER (symp));
+	      /* FIXME: Creating a new symbol here is risky.  We're
+		 in the final loop over the symbol table.  We can
+		 get away with it only because the symbol goes to
+		 the end of the list, where the loop will still see
+		 it.  It would probably be better to do this in
+		 obj_frob_file_before_adjust.  */
 
-	      if (S_IS_WEAK (symp))
-		S_SET_WEAK (symp2);
+	      obj_elf_copy_symbol (sy_obj->versioned_name, symp);
 
-	      if (S_IS_EXTERNAL (symp))
-		S_SET_EXTERNAL (symp2);
+	      switch (symbol_get_obj (symp)->visibility)
+		{
+		case visibility_unchanged:
+		  break;
+		case visibility_hidden:
+		  bfdsym = symbol_get_bfdsym (symp);
+		  elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym),
+					    bfdsym);
+		  elfsym->internal_elf_sym.st_other &= ~3;
+		  elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
+		  break;
+		case visibility_remove:
+		  /* Remove the symbol if it isn't used in relocation.  */
+		  if (!symbol_used_in_reloc_p (symp))
+		    symbol_remove (symp, &symbol_rootP, &symbol_lastP);
+		  break;
+		case visibility_local:
+		  S_CLEAR_EXTERNAL (symp);
+		  break;
+		}
 	    }
 	}
     }
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 54af9ebc0e..b85dfbcffe 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -55,11 +55,22 @@ extern int mips_flag_mdebug;
 #endif
 #endif
 
+enum original_visibility
+{
+  visibility_unchanged = 0,
+  visibility_local,
+  visibility_hidden,
+  visibility_remove
+};
+
 /* Additional information we keep for each symbol.  */
 struct elf_obj_sy
 {
   /* Whether the symbol has been marked as local.  */
-  int local;
+  unsigned int local : 1;
+
+  /* Whether visibility of the original symbol should be changed.  */
+  ENUM_BITFIELD (original_visibility) visibility : 2;
 
   /* Use this to keep track of .size expressions that involve
      differences that we can't compute yet.  */
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 0a6727ef84..8669879c87 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4509,7 +4509,7 @@ Some machine configurations provide additional directives.
 * Struct::			@code{.struct @var{expression}}
 @ifset ELF
 * SubSection::                  @code{.subsection}
-* Symver::                      @code{.symver @var{name},@var{name2@@nodename}}
+* Symver::                      @code{.symver @var{name},@var{name2@@nodename}[,@var{visibility}]}
 @end ifset
 
 @ifset COFF
@@ -7112,9 +7112,9 @@ shared library.
 
 For ELF targets, the @code{.symver} directive can be used like this:
 @smallexample
-.symver @var{name}, @var{name2@@nodename}
+.symver @var{name}, @var{name2@@nodename}[ ,@var{visibility}]
 @end smallexample
-If the symbol @var{name} is defined within the file
+If the original symbol @var{name} is defined within the file
 being assembled, the @code{.symver} directive effectively creates a symbol
 alias with the name @var{name2@@nodename}, and in fact the main reason that we
 just don't try and create a regular alias is that the @var{@@} character isn't
@@ -7127,7 +7127,15 @@ function is being mentioned.  The @var{nodename} portion of the alias should be
 the name of a node specified in the version script supplied to the linker when
 building a shared library.  If you are attempting to override a versioned
 symbol from a shared library, then @var{nodename} should correspond to the
-nodename of the symbol you are trying to override.
+nodename of the symbol you are trying to override.  The optional argument
+@var{visibility} updates the visibility of the original symbol.  The valid
+visibilities are @code{local}, @code {hidden}, and @code {remove}.  The
+@code{local} visibility makes the original symbol a local symbol
+(@pxref{Local}).  The @code{hidden} visibility sets the visibility of the
+original symbol to @code{hidden} (@pxref{Hidden}).  The @code{remove}
+visibility removes the original symbol from the symbol table if it isn't
+used in relocation.  If visibility isn't specified, the original symbol
+is unchanged.
 
 If the symbol @var{name} is not defined within the file being assembled, all
 references to @var{name} will be changed to @var{name2@@nodename}.  If no
diff --git a/gas/testsuite/gas/symver/symver.exp b/gas/testsuite/gas/symver/symver.exp
index de122eb61c..b33af960da 100644
--- a/gas/testsuite/gas/symver/symver.exp
+++ b/gas/testsuite/gas/symver/symver.exp
@@ -46,8 +46,13 @@ if { [is_elf_format] } then {
       return
     }
 
-    run_dump_test "symver0" 
-    run_dump_test "symver1" 
+    set test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
+    foreach t $test_list {
+	# We need to strip the ".d", but can leave the dirname.
+	verbose [file rootname $t]
+	run_dump_test [file rootname $t]
+    }
+
     run_error_test "symver2" ""
     run_error_test "symver3" ""
     # We have to comment out symver4 and symver5, which check the
@@ -56,5 +61,4 @@ if { [is_elf_format] } then {
     # version name.
 #    run_error_test "symver4" ""
 #    run_error_test "symver5" ""
-    run_error_test "symver6" ""
 }
diff --git a/gas/testsuite/gas/symver/symver10.s b/gas/testsuite/gas/symver/symver10.s
new file mode 100644
index 0000000000..967a692a73
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,remove
+	.symver foo,foo@version1
diff --git a/gas/testsuite/gas/symver/symver10a.d b/gas/testsuite/gas/symver/symver10a.d
new file mode 100644
index 0000000000..e19ed2b0bf
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10a.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver10b.d b/gas/testsuite/gas/symver/symver10b.d
new file mode 100644
index 0000000000..17d0bfdd19
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10b.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
diff --git a/gas/testsuite/gas/symver/symver11.d b/gas/testsuite/gas/symver/symver11.d
new file mode 100644
index 0000000000..0e3e7f14b7
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver11.d
@@ -0,0 +1,8 @@
+#readelf: -rsW
+#name: symver symver11
+
+#...
+[0-9a-f]+ +[0-9a-f]+ +R_.* +[0-9a-f]+ +foo *.*
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
diff --git a/gas/testsuite/gas/symver/symver11.s b/gas/testsuite/gas/symver/symver11.s
new file mode 100644
index 0000000000..547e8123f0
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver11.s
@@ -0,0 +1,9 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,remove
+	.symver foo,foo@version1
+	.dc.a foo
diff --git a/gas/testsuite/gas/symver/symver6.d b/gas/testsuite/gas/symver/symver6.d
new file mode 100644
index 0000000000..cddf7ec703
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver6.d
@@ -0,0 +1,11 @@
+#nm: -n
+#name: symver symver6
+#
+
+#...
+[ 	]+U foo
+#...
+0+00000.. D foo1
+0+0000000 D foo@@version1
+0+00000.. D foo@version1
+0+00000.. d L_foo1
diff --git a/gas/testsuite/gas/symver/symver6.l b/gas/testsuite/gas/symver/symver6.l
deleted file mode 100644
index c2d12ae060..0000000000
--- a/gas/testsuite/gas/symver/symver6.l
+++ /dev/null
@@ -1,3 +0,0 @@
-.*: Assembler messages:
-.*:7: Error: multiple versions \[`foo@version1'|`foo@@version1'\] for symbol `foo'
-#pass
diff --git a/gas/testsuite/gas/symver/symver6.s b/gas/testsuite/gas/symver/symver6.s
index 23d9fe20ee..b0bc0b8b22 100644
--- a/gas/testsuite/gas/symver/symver6.s
+++ b/gas/testsuite/gas/symver/symver6.s
@@ -3,7 +3,7 @@
 	.type foo1,object
 foo1:
 	.long foo
-	.symver foo,foo@@version1
-	.symver foo,foo@version1
+	.symver foo1,foo@@version1
+	.symver foo1,foo@version1
 L_foo1:
 	.size foo1,L_foo1-foo1
diff --git a/gas/testsuite/gas/symver/symver7.d b/gas/testsuite/gas/symver/symver7.d
new file mode 100644
index 0000000000..eb680083da
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.d
@@ -0,0 +1,8 @@
+#readelf: -sW
+#name: symver symver7
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +HIDDEN +[0-9]+ +foo
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver7.s b/gas/testsuite/gas/symver/symver7.s
new file mode 100644
index 0000000000..20c11b7cc0
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,local
+	.symver foo,foo@version1,hidden
diff --git a/gas/testsuite/gas/symver/symver8.d b/gas/testsuite/gas/symver/symver8.d
new file mode 100644
index 0000000000..3026c15383
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.d
@@ -0,0 +1,9 @@
+#readelf: -sW
+#name: symver symver8
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +LOCAL +DEFAULT +[0-9]+ +foo
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver8.s b/gas/testsuite/gas/symver/symver8.s
new file mode 100644
index 0000000000..17ab037040
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,hidden
+	.symver foo,foo@version1,local
diff --git a/gas/testsuite/gas/symver/symver9.s b/gas/testsuite/gas/symver/symver9.s
new file mode 100644
index 0000000000..2f608972f5
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2
+	.symver foo,foo@version1,remove
diff --git a/gas/testsuite/gas/symver/symver9a.d b/gas/testsuite/gas/symver/symver9a.d
new file mode 100644
index 0000000000..62dda20bed
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9a.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+#pass
diff --git a/gas/testsuite/gas/symver/symver9b.d b/gas/testsuite/gas/symver/symver9b.d
new file mode 100644
index 0000000000..383d1bd080
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9b.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
-- 
2.25.2


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

* Re: V3 [PATCH] gas: Extend .symver directive
  2020-04-08 13:21                 ` V3 " H.J. Lu
@ 2020-04-09 17:16                   ` Fangrui Song
  2020-04-11 14:22                     ` V4 " H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Fangrui Song @ 2020-04-09 17:16 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils, Andreas Schwab

On 2020-04-08, H.J. Lu wrote:
>On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <i@maskray.me> wrote:
>> >
>> > On 2020-04-07, H.J. Lu wrote:
>> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
>> > >>
>> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
>> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>> > >> >>
>> > >> >> On Apr 07 2020, H.J. Lu wrote:
>> > >> >>
>> > >> >> > The optional argument VISIBILITY updates the visibility of
>> > >> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
>> > >> >> > 'remove'.  The 'local' visibility makes the original symbol a local
>> > >> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
>> > >> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
>> > >> >> > visibility removes the original symbol from the symbol table.  If
>> > >> >> > visibility isn't specified, the original symbol is unchanged.
>> > >> >>
>> > >> >> Looks good.
>> > >> >
>> > >> >Here is the updated patch.
>> > >> >
>> > >> >--
>> > >> >H.J.
>> > >>
>> > >> Let me summarize the current status:
>> > >>
>> > >> @@@ has the renaming semantics. (invented in 2000)
>> > >> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
>> > >>
>> > >> We have received some requests:
>> > >>
>> > >> * Have a way to not retain the original symbol
>> > >> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
>> > >>
>> > >>
>> > >> We have many choices.
>> > >>
>> > >> A. make @ @@ similar to @@@
>> > >>    For @@, because of the linking rule (a @@ definition can satisfy a
>> > >>    referenced without a version), there should be no difference.
>> > >>
>> > >>    For @, this will be a semantic break. Personally I don't think this
>> > >>    matters. I believe 99% projects don't need the original symbol and
>> > >>    will not notice anything. I also checked with FreeBSD developers.
>> > >
>> > >The original symbol name is used in glibc to bypass PLT within
>> > >libc.so.
>> >
>> > This does not seem correct. By convention, the hidden aliases are those prefixed with __
>> > They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
>> > The original symbol does not have the prefix.
>> >
>> > When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
>> > "memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
>> > "memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.
>>
>> As I said before, the original purpose of .symver is for glibc to
>> implement symbol
>> versioning.  Given:
>>
>> ---
>> const int _sys_nerr_internal = 200;
>> __asm__ (".symver _sys_nerr_internal, sys_nerr@@GLIBC_2.12");
>> ---
>>
>> _sys_nerr_internal is used internally in glibc:
>>
>> File: libc_pic.a(_strerror.os)
>>
>> Relocation section '.rela.text' at offset 0x14e0 contains 17 entries:
>>     Offset             Info             Type               Symbol's
>> Value  Symbol's Name + Addend
>> 000000000000001c  0000001500000002 R_X86_64_PC32
>> 0000000000000000 _sys_nerr_internal - 4
>> 000000000000002c  0000001600000002 R_X86_64_PC32
>> 0000000000000000 _sys_errlist_internal - 4
>>
>> ---
>> char *
>> __strerror_r (int errnum, char *buf, size_t buflen)
>> {
>>   ...
>>   return (char *) _(_sys_errlist_internal[errnum]);
>> }
>> ---
>>
>> Also with -g, _sys_nerr_internal is also referenced in debug info.  We
>> just can't change .symver to rename.

OK.

>> > If the definition of "memcpy" also exists, I think it is somewhat special cased
>> > in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
>> > with indirect symbol involved. I believe the whole thing can be simplified a
>> > lot by using renaming semantic. I debugged this area last year and may
>> > misremember something.
>>
>> It is OK to extend .symver directive.  Change it to rename semantic isn't an
>> option.
>>
>
>Updated patch to call symbol_remove to remove the symbol
>if it isn't used in relocation.
>
>-- 
>H.J.

Is the second .symver required to be after the definition?

Case A

.symver foo,foo@@v2
.symver foo,foo@v1

.globl foo
foo:
   ret

% as-new a.s
a.s: Assembler messages:
a.s: Error: local label `"0" (instance number 0 of a dollar label)' is not defined

Case B

.globl foo
foo:
   ret

.symver foo,foo@@v2
.symver foo,foo@v1
.hidden foo

      5: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo
      6: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo@@v2
      7: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 foo@v1

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

* V4 [PATCH] gas: Extend .symver directive
  2020-04-09 17:16                   ` Fangrui Song
@ 2020-04-11 14:22                     ` H.J. Lu
  2020-04-20 14:03                       ` PING: " H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-11 14:22 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils, Andreas Schwab

[-- Attachment #1: Type: text/plain, Size: 5174 bytes --]

On Thu, Apr 9, 2020 at 10:16 AM Fangrui Song <i@maskray.me> wrote:
>
> On 2020-04-08, H.J. Lu wrote:
> >On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <i@maskray.me> wrote:
> >> >
> >> > On 2020-04-07, H.J. Lu wrote:
> >> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
> >> > >>
> >> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> >> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >> > >> >>
> >> > >> >> On Apr 07 2020, H.J. Lu wrote:
> >> > >> >>
> >> > >> >> > The optional argument VISIBILITY updates the visibility of
> >> > >> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> >> > >> >> > 'remove'.  The 'local' visibility makes the original symbol a local
> >> > >> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> >> > >> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> >> > >> >> > visibility removes the original symbol from the symbol table.  If
> >> > >> >> > visibility isn't specified, the original symbol is unchanged.
> >> > >> >>
> >> > >> >> Looks good.
> >> > >> >
> >> > >> >Here is the updated patch.
> >> > >> >
> >> > >> >--
> >> > >> >H.J.
> >> > >>
> >> > >> Let me summarize the current status:
> >> > >>
> >> > >> @@@ has the renaming semantics. (invented in 2000)
> >> > >> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
> >> > >>
> >> > >> We have received some requests:
> >> > >>
> >> > >> * Have a way to not retain the original symbol
> >> > >> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
> >> > >>
> >> > >>
> >> > >> We have many choices.
> >> > >>
> >> > >> A. make @ @@ similar to @@@
> >> > >>    For @@, because of the linking rule (a @@ definition can satisfy a
> >> > >>    referenced without a version), there should be no difference.
> >> > >>
> >> > >>    For @, this will be a semantic break. Personally I don't think this
> >> > >>    matters. I believe 99% projects don't need the original symbol and
> >> > >>    will not notice anything. I also checked with FreeBSD developers.
> >> > >
> >> > >The original symbol name is used in glibc to bypass PLT within
> >> > >libc.so.
> >> >
> >> > This does not seem correct. By convention, the hidden aliases are those prefixed with __
> >> > They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
> >> > The original symbol does not have the prefix.
> >> >
> >> > When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
> >> > "memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
> >> > "memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.
> >>
> >> As I said before, the original purpose of .symver is for glibc to
> >> implement symbol
> >> versioning.  Given:
> >>
> >> ---
> >> const int _sys_nerr_internal = 200;
> >> __asm__ (".symver _sys_nerr_internal, sys_nerr@@GLIBC_2.12");
> >> ---
> >>
> >> _sys_nerr_internal is used internally in glibc:
> >>
> >> File: libc_pic.a(_strerror.os)
> >>
> >> Relocation section '.rela.text' at offset 0x14e0 contains 17 entries:
> >>     Offset             Info             Type               Symbol's
> >> Value  Symbol's Name + Addend
> >> 000000000000001c  0000001500000002 R_X86_64_PC32
> >> 0000000000000000 _sys_nerr_internal - 4
> >> 000000000000002c  0000001600000002 R_X86_64_PC32
> >> 0000000000000000 _sys_errlist_internal - 4
> >>
> >> ---
> >> char *
> >> __strerror_r (int errnum, char *buf, size_t buflen)
> >> {
> >>   ...
> >>   return (char *) _(_sys_errlist_internal[errnum]);
> >> }
> >> ---
> >>
> >> Also with -g, _sys_nerr_internal is also referenced in debug info.  We
> >> just can't change .symver to rename.
>
> OK.
>
> >> > If the definition of "memcpy" also exists, I think it is somewhat special cased
> >> > in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
> >> > with indirect symbol involved. I believe the whole thing can be simplified a
> >> > lot by using renaming semantic. I debugged this area last year and may
> >> > misremember something.
> >>
> >> It is OK to extend .symver directive.  Change it to rename semantic isn't an
> >> option.
> >>
> >
> >Updated patch to call symbol_remove to remove the symbol
> >if it isn't used in relocation.
> >
> >--
> >H.J.
>
> Is the second .symver required to be after the definition?
>
> Case A
>
> .symver foo,foo@@v2
> .symver foo,foo@v1
>
> .globl foo
> foo:
>    ret
>
> % as-new a.s
> a.s: Assembler messages:
> a.s: Error: local label `"0" (instance number 0 of a dollar label)' is not defined
>
> Case B
>
> .globl foo
> foo:
>    ret
>
> .symver foo,foo@@v2
> .symver foo,foo@v1
> .hidden foo
>
>       5: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo
>       6: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo@@v2
>       7: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 foo@v1

Here is the updated patch with the above issues fixed.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-gas-Extend-.symver-directive.patch --]
[-- Type: text/x-patch, Size: 30729 bytes --]

From 70a08b656b679ab696bbc62e03d5b98623569a10 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 6 Apr 2020 11:30:53 -0700
Subject: [PATCH] gas: Extend .symver directive

Extend .symver directive to update visibility of the original symbol and
assign one original symbol to different versioned symbols:

  .symver foo, foo@VERS_1, local    # Change foo to a local symbol.
  .symver foo, foo@VERS_2, hidden   # Change foo to a hidden symbol.
  .symver foo, foo@@VERS_3, remove  # Remove foo from symbol table.
  .symver foo, bar@V1               # Assign foo to bar@V1 and baz@V2.
  .symver foo, baz@V2

	PR gas/23840
	PR gas/25295
	* NEWS: Mention .symver extension.
	* config/obj-elf.c (obj_elf_find_and_add_versioned_name): New
	function.
	(obj_elf_symver): Call obj_elf_find_and_add_versioned_name to
	add a version name.  Add local, hidden and remove visibility
	support.
	(elf_frob_symbol): Handle the list of version names.  Update the
	original symbol to local, hidden or remove it from the symbol
	table.
	(elf_frob_file_before_adjust): Handle the list of version names.
	* config/obj-elf.h (elf_visibility): New.
	(elf_versioned_name_list): Likewise.
	(elf_obj_sy): Change local to bitfield. Add rename, bad_version
	and visibility.  Change versioned_name pointer to struct
	elf_versioned_name_list.
	* doc/as.texi: Update .symver directive.
	* testsuite/gas/symver/symver.exp: Run all *.d tests.  Add more
	error checking tests.
	* testsuite/gas/symver/symver6.d: New file.
	* testsuite/gas/symver/symver7.d: Likewise.
	* testsuite/gas/symver/symver7.s: Likewise.
	* testsuite/gas/symver/symver8.d: Likewise.
	* testsuite/gas/symver/symver8.s: Likewise.
	* testsuite/gas/symver/symver9.s: Likewise.
	* testsuite/gas/symver/symver9a.d: Likewise.
	* testsuite/gas/symver/symver9b.d: Likewise.
	* testsuite/gas/symver/symver10.s: Likewise.
	* testsuite/gas/symver/symver10a.d: Likewise.
	* testsuite/gas/symver/symver10b.d: Likewise.
	* testsuite/gas/symver/symver11.d: Likewise.
	* testsuite/gas/symver/symver11.s: Likewise.
	* testsuite/gas/symver/symver12.d: Likewise.
	* testsuite/gas/symver/symver12.s: Likewise.
	* testsuite/gas/symver/symver13.d: Likewise.
	* testsuite/gas/symver/symver13.s: Likewise.
	* testsuite/gas/symver/symver14.d: Likewise.
	* testsuite/gas/symver/symver14.l: Likewise.
	* testsuite/gas/symver/symver15.d: Likewise.
	* testsuite/gas/symver/symver15.l: Likewise.
	* testsuite/gas/symver/symver6.l: Removed.
	* testsuite/gas/symver/symver6.s: Updated.
---
 gas/NEWS                             |   3 +
 gas/config/obj-elf.c                 | 318 ++++++++++++++++++---------
 gas/config/obj-elf.h                 |  29 ++-
 gas/doc/as.texi                      |  16 +-
 gas/testsuite/gas/symver/symver.exp  |  12 +-
 gas/testsuite/gas/symver/symver10.s  |   8 +
 gas/testsuite/gas/symver/symver10a.d |   8 +
 gas/testsuite/gas/symver/symver10b.d |   8 +
 gas/testsuite/gas/symver/symver11.d  |   8 +
 gas/testsuite/gas/symver/symver11.s  |   9 +
 gas/testsuite/gas/symver/symver12.d  |   9 +
 gas/testsuite/gas/symver/symver12.s  |  10 +
 gas/testsuite/gas/symver/symver13.d  |   9 +
 gas/testsuite/gas/symver/symver13.s  |  11 +
 gas/testsuite/gas/symver/symver14.l  |   2 +
 gas/testsuite/gas/symver/symver14.s  |   6 +
 gas/testsuite/gas/symver/symver15.l  |   2 +
 gas/testsuite/gas/symver/symver15.s  |   3 +
 gas/testsuite/gas/symver/symver6.d   |  11 +
 gas/testsuite/gas/symver/symver6.l   |   3 -
 gas/testsuite/gas/symver/symver6.s   |   4 +-
 gas/testsuite/gas/symver/symver7.d   |   8 +
 gas/testsuite/gas/symver/symver7.s   |   8 +
 gas/testsuite/gas/symver/symver8.d   |   9 +
 gas/testsuite/gas/symver/symver8.s   |   8 +
 gas/testsuite/gas/symver/symver9.s   |   8 +
 gas/testsuite/gas/symver/symver9a.d  |   8 +
 gas/testsuite/gas/symver/symver9b.d  |   8 +
 28 files changed, 424 insertions(+), 122 deletions(-)
 create mode 100644 gas/testsuite/gas/symver/symver10.s
 create mode 100644 gas/testsuite/gas/symver/symver10a.d
 create mode 100644 gas/testsuite/gas/symver/symver10b.d
 create mode 100644 gas/testsuite/gas/symver/symver11.d
 create mode 100644 gas/testsuite/gas/symver/symver11.s
 create mode 100644 gas/testsuite/gas/symver/symver12.d
 create mode 100644 gas/testsuite/gas/symver/symver12.s
 create mode 100644 gas/testsuite/gas/symver/symver13.d
 create mode 100644 gas/testsuite/gas/symver/symver13.s
 create mode 100644 gas/testsuite/gas/symver/symver14.l
 create mode 100644 gas/testsuite/gas/symver/symver14.s
 create mode 100644 gas/testsuite/gas/symver/symver15.l
 create mode 100644 gas/testsuite/gas/symver/symver15.s
 create mode 100644 gas/testsuite/gas/symver/symver6.d
 delete mode 100644 gas/testsuite/gas/symver/symver6.l
 create mode 100644 gas/testsuite/gas/symver/symver7.d
 create mode 100644 gas/testsuite/gas/symver/symver7.s
 create mode 100644 gas/testsuite/gas/symver/symver8.d
 create mode 100644 gas/testsuite/gas/symver/symver8.s
 create mode 100644 gas/testsuite/gas/symver/symver9.s
 create mode 100644 gas/testsuite/gas/symver/symver9a.d
 create mode 100644 gas/testsuite/gas/symver/symver9b.d

diff --git a/gas/NEWS b/gas/NEWS
index 6748c179f1..58d79caa41 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Extend .symver directive to update visibility of the original symbol
+  and assign one original symbol to different versioned symbols.
+
 * Add support for Intel SERIALIZE and TSXLDTRK instructions.
 
 * Add -mlfence-after-load=, -mlfence-before-indirect-branch= and
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index a6dcdaf4a7..f3f7fc5cb7 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -1515,6 +1515,70 @@ obj_elf_line (int ignore ATTRIBUTE_UNUSED)
   demand_empty_rest_of_line ();
 }
 
+static struct elf_versioned_name_list *
+obj_elf_find_and_add_versioned_name (const char *version_name,
+				     const char *sym_name,
+				     const char *ver,
+				     struct elf_obj_sy *sy_obj)
+{
+  struct elf_versioned_name_list *versioned_name;
+  const char *p;
+
+  for (p = ver + 1; *p == ELF_VER_CHR; p++)
+    ;
+
+  /* NB: Since some tests in ld/testsuite/ld-elfvers have no version
+     names, we have to disable this.  */
+  if (0 && *p == '\0')
+    {
+      as_bad (_("missing version name in `%s' for symbol `%s'"),
+	      version_name, sym_name);
+      return NULL;
+    }
+
+  versioned_name = sy_obj->versioned_name;
+
+  switch (p - ver)
+    {
+    case 1:
+    case 2:
+      break;
+    case 3:
+      if (sy_obj->rename)
+	{
+	  if (strcmp (versioned_name->name, version_name) == 0)
+	    return versioned_name;
+	  else
+	    {
+	      as_bad (_("only one version name with `@@@' is allowed "
+			"for symbol `%s'"), sym_name);
+	      return NULL;
+	    }
+	}
+      sy_obj->rename = TRUE;
+      break;
+    default:
+      as_bad (_("invalid version name '%s' for symbol `%s'"),
+	      version_name, sym_name);
+      return NULL;
+    }
+
+  for (;
+       versioned_name != NULL;
+       versioned_name = versioned_name->next)
+    if (strcmp (versioned_name->name, version_name) == 0)
+      return versioned_name;
+
+  /* Add this versioned name to the head of the list,  */
+  versioned_name = (struct elf_versioned_name_list *)
+    xmalloc (sizeof (*versioned_name));
+  versioned_name->name = xstrdup (version_name);
+  versioned_name->next = sy_obj->versioned_name;
+  sy_obj->versioned_name = versioned_name;
+
+  return versioned_name;
+}
+
 /* This handles the .symver pseudo-op, which is used to specify a
    symbol version.  The syntax is ``.symver NAME,SYMVERNAME''.
    SYMVERNAME may contain ELF_VER_CHR ('@') characters.  This
@@ -1525,9 +1589,12 @@ static void
 obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
 {
   char *name;
+  const char *sym_name;
   char c;
   char old_lexat;
   symbolS *sym;
+  struct elf_obj_sy *sy_obj;
+  char *p;
 
   sym = get_sym_from_input_line_and_check ();
 
@@ -1546,43 +1613,59 @@ obj_elf_symver (int ignore ATTRIBUTE_UNUSED)
   lex_type[(unsigned char) '@'] |= LEX_NAME;
   c = get_symbol_name (& name);
   lex_type[(unsigned char) '@'] = old_lexat;
+  sym_name = S_GET_NAME (sym);
 
   if (S_IS_COMMON (sym))
     {
       as_bad (_("`%s' can't be versioned to common symbol '%s'"),
-	      name, S_GET_NAME (sym));
+	      name, sym_name);
       ignore_rest_of_line ();
       return;
     }
 
-  if (symbol_get_obj (sym)->versioned_name == NULL)
+  p = strchr (name, ELF_VER_CHR);
+  if (p == NULL)
     {
-      symbol_get_obj (sym)->versioned_name = xstrdup (name);
+      as_bad (_("missing version name in `%s' for symbol `%s'"),
+	      name, sym_name);
+      ignore_rest_of_line ();
+      return;
+    }
+
+  sy_obj = symbol_get_obj (sym);
+  if (obj_elf_find_and_add_versioned_name (name, sym_name,
+					   p, sy_obj) == NULL)
+    {
+      sy_obj->bad_version = TRUE;
+      ignore_rest_of_line ();
+      return;
+    }
 
-      (void) restore_line_pointer (c);
+  (void) restore_line_pointer (c);
 
-      if (strchr (symbol_get_obj (sym)->versioned_name,
-		  ELF_VER_CHR) == NULL)
+  if (*input_line_pointer == ',')
+    {
+      char *save = input_line_pointer;
+
+      ++input_line_pointer;
+      SKIP_WHITESPACE ();
+      if (strncmp (input_line_pointer, "local", 5) == 0)
 	{
-	  as_bad (_("missing version name in `%s' for symbol `%s'"),
-		  symbol_get_obj (sym)->versioned_name,
-		  S_GET_NAME (sym));
-	  ignore_rest_of_line ();
-	  return;
+	  input_line_pointer += 5;
+	  sy_obj->visibility = visibility_local;
 	}
-    }
-  else
-    {
-      if (strcmp (symbol_get_obj (sym)->versioned_name, name))
+      else if (strncmp (input_line_pointer, "hidden", 6) == 0)
 	{
-	  as_bad (_("multiple versions [`%s'|`%s'] for symbol `%s'"),
-		  name, symbol_get_obj (sym)->versioned_name,
-		  S_GET_NAME (sym));
-	  ignore_rest_of_line ();
-	  return;
+	  input_line_pointer += 6;
+	  sy_obj->visibility = visibility_hidden;
 	}
-
-      (void) restore_line_pointer (c);
+      else if (strncmp (input_line_pointer, "remove", 6) == 0)
+	{
+	  input_line_pointer += 6;
+	  sy_obj->visibility = visibility_remove;
+	}
+      else
+	input_line_pointer = save;
     }
 
   demand_empty_rest_of_line ();
@@ -2378,6 +2461,7 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 {
   struct elf_obj_sy *sy_obj;
   expressionS *size;
+  struct elf_versioned_name_list *versioned_name;
 
 #ifdef NEED_ECOFF_DEBUG
   if (ECOFF_DEBUGGING)
@@ -2405,73 +2489,47 @@ elf_frob_symbol (symbolS *symp, int *puntp)
       sy_obj->size = NULL;
     }
 
-  if (sy_obj->versioned_name != NULL)
+  versioned_name = sy_obj->versioned_name;
+  if (versioned_name)
     {
-      char *p;
-
-      p = strchr (sy_obj->versioned_name, ELF_VER_CHR);
-      if (p == NULL)
-	/* We will have already reported an error about a missing version.  */
-	*puntp = TRUE;
-
       /* This symbol was given a new name with the .symver directive.
-
 	 If this is an external reference, just rename the symbol to
 	 include the version string.  This will make the relocs be
-	 against the correct versioned symbol.
-
-	 If this is a definition, add an alias.  FIXME: Using an alias
-	 will permit the debugging information to refer to the right
-	 symbol.  However, it's not clear whether it is the best
-	 approach.  */
+	 against the correct versioned symbol.  */
 
-      else if (! S_IS_DEFINED (symp))
+      /* We will have already reported an version error.  */
+      if (sy_obj->bad_version)
+	*puntp = TRUE;
+      /* elf_frob_file_before_adjust only allows one version symbol for
+	 renamed symbol.  */
+      else if (sy_obj->rename)
+	S_SET_NAME (symp, versioned_name->name);
+      else if (S_IS_COMMON (symp))
 	{
-	  /* Verify that the name isn't using the @@ syntax--this is
-	     reserved for definitions of the default version to link
-	     against.  */
-	  if (p[1] == ELF_VER_CHR)
-	    {
-	      as_bad (_("invalid attempt to declare external version name"
-			" as default in symbol `%s'"),
-		      sy_obj->versioned_name);
-	      *puntp = TRUE;
-	    }
-	  S_SET_NAME (symp, sy_obj->versioned_name);
+	  as_bad (_("`%s' can't be versioned to common symbol '%s'"),
+		  versioned_name->name, S_GET_NAME (symp));
+	  *puntp = TRUE;
 	}
       else
 	{
-	  if (p[1] == ELF_VER_CHR && p[2] == ELF_VER_CHR)
-	    {
-	      size_t l;
-
-	      /* The @@@ syntax is a special case. It renames the
-		 symbol name to versioned_name with one `@' removed.  */
-	      l = strlen (&p[3]) + 1;
-	      memmove (&p[2], &p[3], l);
-	      S_SET_NAME (symp, sy_obj->versioned_name);
-	    }
-	  else
+	  asymbol *bfdsym;
+	  elf_symbol_type *elfsym;
+
+	  /* This is a definition.  Add an alias for each version.
+	     FIXME: Using an alias will permit the debugging information
+	     to refer to the right symbol.  However, it's not clear
+	     whether it is the best approach.  */
+
+	  /* FIXME: Creating a new symbol here is risky.  We're
+	     in the final loop over the symbol table.  We can
+	     get away with it only because the symbol goes to
+	     the end of the list, where the loop will still see
+	     it.  It would probably be better to do this in
+	     obj_frob_file_before_adjust.  */
+	  for (; versioned_name != NULL;
+	       versioned_name = versioned_name->next)
 	    {
-	      symbolS *symp2;
-
-	      /* FIXME: Creating a new symbol here is risky.  We're
-		 in the final loop over the symbol table.  We can
-		 get away with it only because the symbol goes to
-		 the end of the list, where the loop will still see
-		 it.  It would probably be better to do this in
-		 obj_frob_file_before_adjust.  */
-
-	      symp2 = symbol_find_or_make (sy_obj->versioned_name);
-
-	      /* Now we act as though we saw symp2 = sym.  */
-	      if (S_IS_COMMON (symp))
-		{
-		  as_bad (_("`%s' can't be versioned to common symbol '%s'"),
-			  sy_obj->versioned_name, S_GET_NAME (symp));
-		  *puntp = TRUE;
-		  return;
-		}
+	      symbolS *symp2 = symbol_find_or_make (versioned_name->name);
 
 	      S_SET_SEGMENT (symp2, S_GET_SEGMENT (symp));
 
@@ -2494,6 +2552,27 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	      if (S_IS_EXTERNAL (symp))
 		S_SET_EXTERNAL (symp2);
 	    }
+
+	  switch (symbol_get_obj (symp)->visibility)
+	    {
+	    case visibility_unchanged:
+	      break;
+	    case visibility_hidden:
+	      bfdsym = symbol_get_bfdsym (symp);
+	      elfsym = elf_symbol_from (bfd_asymbol_bfd (bfdsym),
+					bfdsym);
+	      elfsym->internal_elf_sym.st_other &= ~3;
+	      elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
+	      break;
+	    case visibility_remove:
+	      /* Remove the symbol if it isn't used in relocation.  */
+	      if (!symbol_used_in_reloc_p (symp))
+		symbol_remove (symp, &symbol_rootP, &symbol_lastP);
+	      break;
+	    case visibility_local:
+	      S_CLEAR_EXTERNAL (symp);
+	      break;
+	    }
 	}
     }
 
@@ -2672,36 +2751,61 @@ elf_frob_file_before_adjust (void)
       symbolS *symp;
 
       for (symp = symbol_rootP; symp; symp = symbol_next (symp))
-	if (!S_IS_DEFINED (symp))
-	  {
-	    if (symbol_get_obj (symp)->versioned_name)
-	      {
-		char *p;
-
-		/* The @@@ syntax is a special case. If the symbol is
-		   not defined, 2 `@'s will be removed from the
-		   versioned_name.  */
-
-		p = strchr (symbol_get_obj (symp)->versioned_name,
-			    ELF_VER_CHR);
-		if (p != NULL && p[1] == ELF_VER_CHR && p[2] == ELF_VER_CHR)
-		  {
-		    size_t l = strlen (&p[3]) + 1;
-		    memmove (&p[1], &p[3], l);
-		  }
-		if (symbol_used_p (symp) == 0
-		    && symbol_used_in_reloc_p (symp) == 0)
-		  symbol_remove (symp, &symbol_rootP, &symbol_lastP);
-	      }
+	{
+	  struct elf_obj_sy *sy_obj = symbol_get_obj (symp);
+	  int is_defined = !!S_IS_DEFINED (symp);
 
-	    /* If there was .weak foo, but foo was neither defined nor
-	       used anywhere, remove it.  */
+	  if (sy_obj->versioned_name)
+	    {
+	      char *p = strchr (sy_obj->versioned_name->name,
+				ELF_VER_CHR);
 
-	    else if (S_IS_WEAK (symp)
-		     && symbol_used_p (symp) == 0
-		     && symbol_used_in_reloc_p (symp) == 0)
-	      symbol_remove (symp, &symbol_rootP, &symbol_lastP);
-	  }
+	      if (sy_obj->rename)
+		{
+		  /* The @@@ syntax is a special case. If the symbol is
+		     not defined, 2 `@'s will be removed from the
+		     versioned_name. Otherwise, 1 `@' will be removed.   */
+		  size_t l = strlen (&p[3]) + 1;
+		  memmove (&p[1 + is_defined], &p[3], l);
+		}
+
+	      if (!is_defined)
+		{
+		  /* Verify that the name isn't using the @@ syntax--this
+		     is reserved for definitions of the default version
+		     to link against.  */
+		  if (!sy_obj->rename && p[1] == ELF_VER_CHR)
+		    {
+		      as_bad (_("invalid attempt to declare external "
+				"version name as default in symbol `%s'"),
+			      sy_obj->versioned_name->name);
+		      return;
+		    }
+
+		  /* Only one version symbol is allowed for undefined
+		     symbol.  */
+		  if (sy_obj->versioned_name->next)
+		    {
+		      as_bad (_("multiple versions [`%s'|`%s'] for "
+				"symbol `%s'"),
+			      sy_obj->versioned_name->name,
+			      sy_obj->versioned_name->next->name,
+			      S_GET_NAME (symp));
+		      return;
+		    }
+
+		  sy_obj->rename = TRUE;
+		}
+	    }
+
+	  /* If there was .symver or .weak, but symbol was neither
+	     defined nor used anywhere, remove it.  */
+	  if (!is_defined
+	      && (sy_obj->versioned_name || S_IS_WEAK (symp))
+	      && symbol_used_p (symp) == 0
+	      && symbol_used_in_reloc_p (symp) == 0)
+	    symbol_remove (symp, &symbol_rootP, &symbol_lastP);
+	}
     }
 }
 
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 54af9ebc0e..b39a1a1ab6 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -55,18 +55,41 @@ extern int mips_flag_mdebug;
 #endif
 #endif
 
+enum elf_visibility
+{
+  visibility_unchanged = 0,
+  visibility_local,
+  visibility_hidden,
+  visibility_remove
+};
+
+struct elf_versioned_name_list
+{
+  char *name;
+  struct elf_versioned_name_list *next;
+};
+
 /* Additional information we keep for each symbol.  */
 struct elf_obj_sy
 {
   /* Whether the symbol has been marked as local.  */
-  int local;
+  unsigned int local : 1;
+
+  /* Whether the symbol has been marked for rename with @@@.  */
+  unsigned int rename : 1;
+
+  /* Whether the symbol has a bad version name.  */
+  unsigned int bad_version : 1;
+
+  /* Whether visibility of the symbol should be changed.  */
+  ENUM_BITFIELD (elf_visibility) visibility : 2;
 
   /* Use this to keep track of .size expressions that involve
      differences that we can't compute yet.  */
   expressionS *size;
 
-  /* The name specified by the .symver directive.  */
-  char *versioned_name;
+  /* The list of names specified by the .symver directive.  */
+  struct elf_versioned_name_list *versioned_name;
 
 #ifdef ECOFF_DEBUGGING
   /* If we are generating ECOFF debugging information, we need some
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 0a6727ef84..8669879c87 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4509,7 +4509,7 @@ Some machine configurations provide additional directives.
 * Struct::			@code{.struct @var{expression}}
 @ifset ELF
 * SubSection::                  @code{.subsection}
-* Symver::                      @code{.symver @var{name},@var{name2@@nodename}}
+* Symver::                      @code{.symver @var{name},@var{name2@@nodename}[,@var{visibility}]}
 @end ifset
 
 @ifset COFF
@@ -7112,9 +7112,9 @@ shared library.
 
 For ELF targets, the @code{.symver} directive can be used like this:
 @smallexample
-.symver @var{name}, @var{name2@@nodename}
+.symver @var{name}, @var{name2@@nodename}[ ,@var{visibility}]
 @end smallexample
-If the symbol @var{name} is defined within the file
+If the original symbol @var{name} is defined within the file
 being assembled, the @code{.symver} directive effectively creates a symbol
 alias with the name @var{name2@@nodename}, and in fact the main reason that we
 just don't try and create a regular alias is that the @var{@@} character isn't
@@ -7127,7 +7127,15 @@ function is being mentioned.  The @var{nodename} portion of the alias should be
 the name of a node specified in the version script supplied to the linker when
 building a shared library.  If you are attempting to override a versioned
 symbol from a shared library, then @var{nodename} should correspond to the
-nodename of the symbol you are trying to override.
+nodename of the symbol you are trying to override.  The optional argument
+@var{visibility} updates the visibility of the original symbol.  The valid
+visibilities are @code{local}, @code {hidden}, and @code {remove}.  The
+@code{local} visibility makes the original symbol a local symbol
+(@pxref{Local}).  The @code{hidden} visibility sets the visibility of the
+original symbol to @code{hidden} (@pxref{Hidden}).  The @code{remove}
+visibility removes the original symbol from the symbol table if it isn't
+used in relocation.  If visibility isn't specified, the original symbol
+is unchanged.
 
 If the symbol @var{name} is not defined within the file being assembled, all
 references to @var{name} will be changed to @var{name2@@nodename}.  If no
diff --git a/gas/testsuite/gas/symver/symver.exp b/gas/testsuite/gas/symver/symver.exp
index de122eb61c..6b29a12d31 100644
--- a/gas/testsuite/gas/symver/symver.exp
+++ b/gas/testsuite/gas/symver/symver.exp
@@ -46,8 +46,13 @@ if { [is_elf_format] } then {
       return
     }
 
-    run_dump_test "symver0" 
-    run_dump_test "symver1" 
+    set test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
+    foreach t $test_list {
+	# We need to strip the ".d", but can leave the dirname.
+	verbose [file rootname $t]
+	run_dump_test [file rootname $t]
+    }
+
     run_error_test "symver2" ""
     run_error_test "symver3" ""
     # We have to comment out symver4 and symver5, which check the
@@ -56,5 +61,6 @@ if { [is_elf_format] } then {
     # version name.
 #    run_error_test "symver4" ""
 #    run_error_test "symver5" ""
-    run_error_test "symver6" ""
+    run_error_test "symver14" ""
+    run_error_test "symver15" ""
 }
diff --git a/gas/testsuite/gas/symver/symver10.s b/gas/testsuite/gas/symver/symver10.s
new file mode 100644
index 0000000000..967a692a73
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,remove
+	.symver foo,foo@version1
diff --git a/gas/testsuite/gas/symver/symver10a.d b/gas/testsuite/gas/symver/symver10a.d
new file mode 100644
index 0000000000..e9bd1594eb
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10a.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+#pass
diff --git a/gas/testsuite/gas/symver/symver10b.d b/gas/testsuite/gas/symver/symver10b.d
new file mode 100644
index 0000000000..17d0bfdd19
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver10b.d
@@ -0,0 +1,8 @@
+#source: symver10.s
+#readelf: -sW
+#name: symver symver10b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
diff --git a/gas/testsuite/gas/symver/symver11.d b/gas/testsuite/gas/symver/symver11.d
new file mode 100644
index 0000000000..0e3e7f14b7
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver11.d
@@ -0,0 +1,8 @@
+#readelf: -rsW
+#name: symver symver11
+
+#...
+[0-9a-f]+ +[0-9a-f]+ +R_.* +[0-9a-f]+ +foo *.*
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
diff --git a/gas/testsuite/gas/symver/symver11.s b/gas/testsuite/gas/symver/symver11.s
new file mode 100644
index 0000000000..547e8123f0
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver11.s
@@ -0,0 +1,9 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,remove
+	.symver foo,foo@version1
+	.dc.a foo
diff --git a/gas/testsuite/gas/symver/symver12.d b/gas/testsuite/gas/symver/symver12.d
new file mode 100644
index 0000000000..3878f4dd49
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver12.d
@@ -0,0 +1,9 @@
+#readelf: -sW
+#name: symver symver12
+
+#...
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +DEFAULT +[0-9]+ +foo
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +DEFAULT +[0-9]+ +foo@VERS_2
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +DEFAULT +[0-9]+ +foo@VERS_1
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +DEFAULT +[0-9]+ +foo@@VERS_2
+#pass
diff --git a/gas/testsuite/gas/symver/symver12.s b/gas/testsuite/gas/symver/symver12.s
new file mode 100644
index 0000000000..724c3efcec
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver12.s
@@ -0,0 +1,10 @@
+	.text
+	.space 13
+	.symver foo, foo@@VERS_2
+	.symver foo, foo@VERS_1
+	.symver foo, foo@VERS_2
+	.globl  foo
+	.type   foo, %function
+foo:
+	.byte 0
+	.size foo,.-foo
diff --git a/gas/testsuite/gas/symver/symver13.d b/gas/testsuite/gas/symver/symver13.d
new file mode 100644
index 0000000000..dce8579f5d
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver13.d
@@ -0,0 +1,9 @@
+#readelf: -sW
+#name: symver symver13
+
+#...
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +HIDDEN +[0-9]+ +foo
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +HIDDEN +[0-9]+ +foo@VERS_1
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +HIDDEN +[0-9]+ +foo@@VERS_2
+ +[0-9]+: +0+d +1 +FUNC +GLOBAL +HIDDEN +[0-9]+ +foo@VERS_2
+#pass
diff --git a/gas/testsuite/gas/symver/symver13.s b/gas/testsuite/gas/symver/symver13.s
new file mode 100644
index 0000000000..5426d8682a
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver13.s
@@ -0,0 +1,11 @@
+	.text
+	.space 13
+	.globl  foo
+	.type   foo, %function
+foo:
+	.byte 0
+	.symver foo, foo@VERS_2
+	.symver foo, foo@@VERS_2
+	.symver foo, foo@VERS_1
+	.hidden foo
+	.size foo,.-foo
diff --git a/gas/testsuite/gas/symver/symver14.l b/gas/testsuite/gas/symver/symver14.l
new file mode 100644
index 0000000000..53fb7f44e7
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver14.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Error: only one version name with `@@@' is allowed for symbol `foo'
diff --git a/gas/testsuite/gas/symver/symver14.s b/gas/testsuite/gas/symver/symver14.s
new file mode 100644
index 0000000000..6693ff6b00
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver14.s
@@ -0,0 +1,6 @@
+	.data
+	.global foo
+foo:
+	.byte 1
+	.symver foo,foo@@@version1
+	.symver foo,foo@@@version2
diff --git a/gas/testsuite/gas/symver/symver15.l b/gas/testsuite/gas/symver/symver15.l
new file mode 100644
index 0000000000..903a67c601
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver15.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*: Error: multiple versions \[`foo@version2'|`foo@version1'\] for symbol `foo'
diff --git a/gas/testsuite/gas/symver/symver15.s b/gas/testsuite/gas/symver/symver15.s
new file mode 100644
index 0000000000..abf18600a8
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver15.s
@@ -0,0 +1,3 @@
+	.data
+	.symver foo,foo@version1
+	.symver foo,foo@version2
diff --git a/gas/testsuite/gas/symver/symver6.d b/gas/testsuite/gas/symver/symver6.d
new file mode 100644
index 0000000000..cddf7ec703
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver6.d
@@ -0,0 +1,11 @@
+#nm: -n
+#name: symver symver6
+#
+
+#...
+[ 	]+U foo
+#...
+0+00000.. D foo1
+0+0000000 D foo@@version1
+0+00000.. D foo@version1
+0+00000.. d L_foo1
diff --git a/gas/testsuite/gas/symver/symver6.l b/gas/testsuite/gas/symver/symver6.l
deleted file mode 100644
index c2d12ae060..0000000000
--- a/gas/testsuite/gas/symver/symver6.l
+++ /dev/null
@@ -1,3 +0,0 @@
-.*: Assembler messages:
-.*:7: Error: multiple versions \[`foo@version1'|`foo@@version1'\] for symbol `foo'
-#pass
diff --git a/gas/testsuite/gas/symver/symver6.s b/gas/testsuite/gas/symver/symver6.s
index 23d9fe20ee..b0bc0b8b22 100644
--- a/gas/testsuite/gas/symver/symver6.s
+++ b/gas/testsuite/gas/symver/symver6.s
@@ -3,7 +3,7 @@
 	.type foo1,object
 foo1:
 	.long foo
-	.symver foo,foo@@version1
-	.symver foo,foo@version1
+	.symver foo1,foo@@version1
+	.symver foo1,foo@version1
 L_foo1:
 	.size foo1,L_foo1-foo1
diff --git a/gas/testsuite/gas/symver/symver7.d b/gas/testsuite/gas/symver/symver7.d
new file mode 100644
index 0000000000..5152678a48
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.d
@@ -0,0 +1,8 @@
+#readelf: -sW
+#name: symver symver7
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +HIDDEN +[0-9]+ +foo
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+#pass
diff --git a/gas/testsuite/gas/symver/symver7.s b/gas/testsuite/gas/symver/symver7.s
new file mode 100644
index 0000000000..20c11b7cc0
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver7.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,local
+	.symver foo,foo@version1,hidden
diff --git a/gas/testsuite/gas/symver/symver8.d b/gas/testsuite/gas/symver/symver8.d
new file mode 100644
index 0000000000..8938aecb59
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.d
@@ -0,0 +1,9 @@
+#readelf: -sW
+#name: symver symver8
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +LOCAL +DEFAULT +[0-9]+ +foo
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+#pass
diff --git a/gas/testsuite/gas/symver/symver8.s b/gas/testsuite/gas/symver/symver8.s
new file mode 100644
index 0000000000..17ab037040
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver8.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2,hidden
+	.symver foo,foo@version1,local
diff --git a/gas/testsuite/gas/symver/symver9.s b/gas/testsuite/gas/symver/symver9.s
new file mode 100644
index 0000000000..2f608972f5
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9.s
@@ -0,0 +1,8 @@
+	.data
+	.globl foo
+	.type foo,%object
+foo:
+	.byte 0
+	.size foo,.-foo
+	.symver foo,foo@@version2
+	.symver foo,foo@version1,remove
diff --git a/gas/testsuite/gas/symver/symver9a.d b/gas/testsuite/gas/symver/symver9a.d
new file mode 100644
index 0000000000..1cdbce8efc
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9a.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9a
+
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
+#pass
diff --git a/gas/testsuite/gas/symver/symver9b.d b/gas/testsuite/gas/symver/symver9b.d
new file mode 100644
index 0000000000..383d1bd080
--- /dev/null
+++ b/gas/testsuite/gas/symver/symver9b.d
@@ -0,0 +1,8 @@
+#source: symver9.s
+#readelf: -sW
+#name: symver symver9b
+
+#failif
+#...
+ +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
+#pass
-- 
2.25.2


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

* PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-11 14:22                     ` V4 " H.J. Lu
@ 2020-04-20 14:03                       ` H.J. Lu
  2020-04-21 10:21                         ` Nick Clifton
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-20 14:03 UTC (permalink / raw)
  To: Fangrui Song, Nick Clifton, Alan Modra; +Cc: Binutils, Andreas Schwab

On Sat, Apr 11, 2020 at 7:22 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Apr 9, 2020 at 10:16 AM Fangrui Song <i@maskray.me> wrote:
> >
> > On 2020-04-08, H.J. Lu wrote:
> > >On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >>
> > >> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <i@maskray.me> wrote:
> > >> >
> > >> > On 2020-04-07, H.J. Lu wrote:
> > >> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <i@maskray.me> wrote:
> > >> > >>
> > >> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> > >> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> > >> > >> >>
> > >> > >> >> On Apr 07 2020, H.J. Lu wrote:
> > >> > >> >>
> > >> > >> >> > The optional argument VISIBILITY updates the visibility of
> > >> > >> >> > the original symbol.  The valid visibilities are 'local', 'hidden', and
> > >> > >> >> > 'remove'.  The 'local' visibility makes the original symbol a local
> > >> > >> >> > symbol (*note Local::).  The 'hidden' visibility sets the visibility of
> > >> > >> >> > the original symbol to 'hidden' (*note Hidden::).  The 'remove'
> > >> > >> >> > visibility removes the original symbol from the symbol table.  If
> > >> > >> >> > visibility isn't specified, the original symbol is unchanged.
> > >> > >> >>
> > >> > >> >> Looks good.
> > >> > >> >
> > >> > >> >Here is the updated patch.
> > >> > >> >
> > >> > >> >--
> > >> > >> >H.J.
> > >> > >>
> > >> > >> Let me summarize the current status:
> > >> > >>
> > >> > >> @@@ has the renaming semantics. (invented in 2000)
> > >> > >> @ and @@ has the copying semantics. The original symbol remains which is usually cumbersome.
> > >> > >>
> > >> > >> We have received some requests:
> > >> > >>
> > >> > >> * Have a way to not retain the original symbol
> > >> > >> * Have a way to define multiple versions `.symver something,foo@v1; .symver something,foo@v2. symver something,foo@@v3`
> > >> > >>
> > >> > >>
> > >> > >> We have many choices.
> > >> > >>
> > >> > >> A. make @ @@ similar to @@@
> > >> > >>    For @@, because of the linking rule (a @@ definition can satisfy a
> > >> > >>    referenced without a version), there should be no difference.
> > >> > >>
> > >> > >>    For @, this will be a semantic break. Personally I don't think this
> > >> > >>    matters. I believe 99% projects don't need the original symbol and
> > >> > >>    will not notice anything. I also checked with FreeBSD developers.
> > >> > >
> > >> > >The original symbol name is used in glibc to bypass PLT within
> > >> > >libc.so.
> > >> >
> > >> > This does not seem correct. By convention, the hidden aliases are those prefixed with __
> > >> > They are called to bypass PLT (STV_HIDDEN implies the non-preemptible property).
> > >> > The original symbol does not have the prefix.
> > >> >
> > >> > When a linker sees memcpy@@GLIBC_2.14 , basically it inserts both "memcpy" and
> > >> > "memcpy@GLIBC_2.14" into the symbol table.  Both a reference without a version
> > >> > "memcpy" and a reference with a version "memcpy@GLIBC_2.14" can be satisfied.
> > >>
> > >> As I said before, the original purpose of .symver is for glibc to
> > >> implement symbol
> > >> versioning.  Given:
> > >>
> > >> ---
> > >> const int _sys_nerr_internal = 200;
> > >> __asm__ (".symver _sys_nerr_internal, sys_nerr@@GLIBC_2.12");
> > >> ---
> > >>
> > >> _sys_nerr_internal is used internally in glibc:
> > >>
> > >> File: libc_pic.a(_strerror.os)
> > >>
> > >> Relocation section '.rela.text' at offset 0x14e0 contains 17 entries:
> > >>     Offset             Info             Type               Symbol's
> > >> Value  Symbol's Name + Addend
> > >> 000000000000001c  0000001500000002 R_X86_64_PC32
> > >> 0000000000000000 _sys_nerr_internal - 4
> > >> 000000000000002c  0000001600000002 R_X86_64_PC32
> > >> 0000000000000000 _sys_errlist_internal - 4
> > >>
> > >> ---
> > >> char *
> > >> __strerror_r (int errnum, char *buf, size_t buflen)
> > >> {
> > >>   ...
> > >>   return (char *) _(_sys_errlist_internal[errnum]);
> > >> }
> > >> ---
> > >>
> > >> Also with -g, _sys_nerr_internal is also referenced in debug info.  We
> > >> just can't change .symver to rename.
> >
> > OK.
> >
> > >> > If the definition of "memcpy" also exists, I think it is somewhat special cased
> > >> > in GNU ld and/or gold. In GNU ld, the actual implementation may be more complex
> > >> > with indirect symbol involved. I believe the whole thing can be simplified a
> > >> > lot by using renaming semantic. I debugged this area last year and may
> > >> > misremember something.
> > >>
> > >> It is OK to extend .symver directive.  Change it to rename semantic isn't an
> > >> option.
> > >>
> > >
> > >Updated patch to call symbol_remove to remove the symbol
> > >if it isn't used in relocation.
> > >
> > >--
> > >H.J.
> >
> > Is the second .symver required to be after the definition?
> >
> > Case A
> >
> > .symver foo,foo@@v2
> > .symver foo,foo@v1
> >
> > .globl foo
> > foo:
> >    ret
> >
> > % as-new a.s
> > a.s: Assembler messages:
> > a.s: Error: local label `"0" (instance number 0 of a dollar label)' is not defined
> >
> > Case B
> >
> > .globl foo
> > foo:
> >    ret
> >
> > .symver foo,foo@@v2
> > .symver foo,foo@v1
> > .hidden foo
> >
> >       5: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo
> >       6: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN     1 foo@@v2
> >       7: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT    1 foo@v1
>
> Here is the updated patch with the above issues fixed.
>
> Thanks.
>

PING:

https://sourceware.org/pipermail/binutils/2020-April/110622.html

-- 
H.J.

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-20 14:03                       ` PING: " H.J. Lu
@ 2020-04-21 10:21                         ` Nick Clifton
  2020-04-21 23:20                           ` Alan Modra
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Clifton @ 2020-04-21 10:21 UTC (permalink / raw)
  To: H.J. Lu, Fangrui Song, Alan Modra; +Cc: Binutils, Andreas Schwab

Hi H.J.

> PING:
> 
> https://sourceware.org/pipermail/binutils/2020-April/110622.html

Approved - please apply.

Cheers
  Nick



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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-21 10:21                         ` Nick Clifton
@ 2020-04-21 23:20                           ` Alan Modra
  2020-04-21 23:52                             ` Alan Modra
  0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2020-04-21 23:20 UTC (permalink / raw)
  Cc: H.J. Lu, Binutils

avr-elf  +FAIL: symver symver11
d10v-elf  +FAIL: symver symver11
dlx-elf  +FAIL: symver symver11
ip2k-elf  +FAIL: symver symver11
m68k-elf  +FAIL: symver symver11
mcore-elf  +FAIL: symver symver11
msp430-elf  +FAIL: symver symver7
pj-elf  +FAIL: symver symver11
s12z-elf  +FAIL: symver symver11
shle-unknown-netbsdelf  +FAIL: symver symver11
sh-linux  +FAIL: symver symver11
sh-nto  +FAIL: symver symver11
sh-rtems  +FAIL: symver symver11
visium-elf  +FAIL: symver symver11
xc16x-elf  +FAIL: symver symver11
z80-elf  +FAIL: symver symver11

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-21 23:20                           ` Alan Modra
@ 2020-04-21 23:52                             ` Alan Modra
  2020-04-22  1:19                               ` H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2020-04-21 23:52 UTC (permalink / raw)
  To: H.J. Lu, Binutils

On Wed, Apr 22, 2020 at 08:50:06AM +0930, Alan Modra wrote:
> avr-elf  +FAIL: symver symver11
> d10v-elf  +FAIL: symver symver11
> dlx-elf  +FAIL: symver symver11
> ip2k-elf  +FAIL: symver symver11
> m68k-elf  +FAIL: symver symver11
> mcore-elf  +FAIL: symver symver11
> msp430-elf  +FAIL: symver symver7
> pj-elf  +FAIL: symver symver11
> s12z-elf  +FAIL: symver symver11
> shle-unknown-netbsdelf  +FAIL: symver symver11
> sh-linux  +FAIL: symver symver11
> sh-nto  +FAIL: symver symver11
> sh-rtems  +FAIL: symver symver11
> visium-elf  +FAIL: symver symver11
> xc16x-elf  +FAIL: symver symver11
> z80-elf  +FAIL: symver symver11

All of the symver11 fails except the sh ones are due to the symbol
actually being removed!  As it is supposed to be, if not used in a
relocation.  And those targets happen to reduce the reference to foo
down to a section symbol.

I wonder if ".symver intsym, extsym@@nodename, remove" ought to really
remove the symbol resulting in an assembly error if referenced?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-21 23:52                             ` Alan Modra
@ 2020-04-22  1:19                               ` H.J. Lu
  2020-04-22  2:21                                 ` Alan Modra
  0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2020-04-22  1:19 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

On Tue, Apr 21, 2020 at 4:52 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Apr 22, 2020 at 08:50:06AM +0930, Alan Modra wrote:
> > avr-elf  +FAIL: symver symver11
> > d10v-elf  +FAIL: symver symver11
> > dlx-elf  +FAIL: symver symver11
> > ip2k-elf  +FAIL: symver symver11
> > m68k-elf  +FAIL: symver symver11
> > mcore-elf  +FAIL: symver symver11
> > msp430-elf  +FAIL: symver symver7
> > pj-elf  +FAIL: symver symver11
> > s12z-elf  +FAIL: symver symver11
> > shle-unknown-netbsdelf  +FAIL: symver symver11
> > sh-linux  +FAIL: symver symver11

I am checking in this for sh targets:

diff --git a/gas/testsuite/gas/symver/symver11.s
b/gas/testsuite/gas/symver/symver11.s
index 547e8123f0..08416be0f0 100644
--- a/gas/testsuite/gas/symver/symver11.s
+++ b/gas/testsuite/gas/symver/symver11.s
@@ -6,4 +6,5 @@ foo:
   .size foo,.-foo
   .symver foo,foo@@version2,remove
   .symver foo,foo@version1
+  .balign 8
   .dc.a foo

> > sh-nto  +FAIL: symver symver11
> > sh-rtems  +FAIL: symver symver11
> > visium-elf  +FAIL: symver symver11
> > xc16x-elf  +FAIL: symver symver11
> > z80-elf  +FAIL: symver symver11
>
> All of the symver11 fails except the sh ones are due to the symbol
> actually being removed!  As it is supposed to be, if not used in a
> relocation.  And those targets happen to reduce the reference to foo
> down to a section symbol.

foo is a global symbol.  Should assembler not reduce its reference
to a section symbol?  If these targets have to do it, I can skip this test
for these targets.

> I wonder if ".symver intsym, extsym@@nodename, remove" ought to really
> remove the symbol resulting in an assembly error if referenced?
>

A testcase?

-- 
H.J.

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-22  1:19                               ` H.J. Lu
@ 2020-04-22  2:21                                 ` Alan Modra
  2020-04-22  8:51                                   ` Alan Modra
  0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2020-04-22  2:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Tue, Apr 21, 2020 at 06:19:45PM -0700, H.J. Lu wrote:
> On Tue, Apr 21, 2020 at 4:52 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Wed, Apr 22, 2020 at 08:50:06AM +0930, Alan Modra wrote:
> > > avr-elf  +FAIL: symver symver11
> > > d10v-elf  +FAIL: symver symver11
> > > dlx-elf  +FAIL: symver symver11
> > > ip2k-elf  +FAIL: symver symver11
> > > m68k-elf  +FAIL: symver symver11
> > > mcore-elf  +FAIL: symver symver11
> > > msp430-elf  +FAIL: symver symver7
> > > pj-elf  +FAIL: symver symver11
> > > s12z-elf  +FAIL: symver symver11
> > > shle-unknown-netbsdelf  +FAIL: symver symver11
> > > sh-linux  +FAIL: symver symver11
> 
> I am checking in this for sh targets:
> 
> diff --git a/gas/testsuite/gas/symver/symver11.s
> b/gas/testsuite/gas/symver/symver11.s
> index 547e8123f0..08416be0f0 100644
> --- a/gas/testsuite/gas/symver/symver11.s
> +++ b/gas/testsuite/gas/symver/symver11.s
> @@ -6,4 +6,5 @@ foo:
>    .size foo,.-foo
>    .symver foo,foo@@version2,remove
>    .symver foo,foo@version1
> +  .balign 8
>    .dc.a foo

Sure.  The msp430 symver7 fix is easy too.  Allow other random symbols
between the ones you check.

> > > sh-nto  +FAIL: symver symver11
> > > sh-rtems  +FAIL: symver symver11
> > > visium-elf  +FAIL: symver symver11
> > > xc16x-elf  +FAIL: symver symver11
> > > z80-elf  +FAIL: symver symver11
> >
> > All of the symver11 fails except the sh ones are due to the symbol
> > actually being removed!  As it is supposed to be, if not used in a
> > relocation.  And those targets happen to reduce the reference to foo
> > down to a section symbol.
> 
> foo is a global symbol.  Should assembler not reduce its reference
> to a section symbol?  If these targets have to do it, I can skip this test
> for these targets.
> 
> > I wonder if ".symver intsym, extsym@@nodename, remove" ought to really
> > remove the symbol resulting in an assembly error if referenced?
> >
> 
> A testcase?

I mean this:

diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 409ea4d6be..4bdddc9056 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -2569,9 +2569,7 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	      elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
 	      break;
 	    case visibility_remove:
-	      /* Remove the symbol if it isn't used in relocation.  */
-	      if (!symbol_used_in_reloc_p (symp))
-		symbol_remove (symp, &symbol_rootP, &symbol_lastP);
+	      symbol_remove (symp, &symbol_rootP, &symbol_lastP);
 	      break;
 	    case visibility_local:
 	      S_CLEAR_EXTERNAL (symp);

And then your symver11 testcase fails on x64 with
symver11.s:9: Error: redefined symbol cannot be used on reloc

Of course on the other targets that reduce the foo reference to .data
you won't get an error.  One way to make those targets behave the
same would be to make foo weak.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-22  2:21                                 ` Alan Modra
@ 2020-04-22  8:51                                   ` Alan Modra
  2020-04-22 12:14                                     ` H.J. Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2020-04-22  8:51 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Wed, Apr 22, 2020 at 11:51:10AM +0930, Alan Modra wrote:
> On Tue, Apr 21, 2020 at 06:19:45PM -0700, H.J. Lu wrote:
> > On Tue, Apr 21, 2020 at 4:52 PM Alan Modra <amodra@gmail.com> wrote:
> > > I wonder if ".symver intsym, extsym@@nodename, remove" ought to really
> > > remove the symbol resulting in an assembly error if referenced?

Turning this proposal into a proper patch.  What do you think?

	* config/obj-elf.c (elf_frob_symbol): Unconditionally remove
	symbol for ".symver .. remove".
	* doc/as.texi (.symver): Update.
	* testsuite/gas/symver/symver11.s: Make foo weak.
	* testsuite/gas/symver/symver11.d: Expect an error.
	* testsuite/gas/symver/symver7.d: Allow other random symbols.

diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 409ea4d6be..4bdddc9056 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -2569,9 +2569,7 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 	      elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
 	      break;
 	    case visibility_remove:
-	      /* Remove the symbol if it isn't used in relocation.  */
-	      if (!symbol_used_in_reloc_p (symp))
-		symbol_remove (symp, &symbol_rootP, &symbol_lastP);
+	      symbol_remove (symp, &symbol_rootP, &symbol_lastP);
 	      break;
 	    case visibility_local:
 	      S_CLEAR_EXTERNAL (symp);
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 8669879c87..a65ddad5f5 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -7129,13 +7129,12 @@ building a shared library.  If you are attempting to override a versioned
 symbol from a shared library, then @var{nodename} should correspond to the
 nodename of the symbol you are trying to override.  The optional argument
 @var{visibility} updates the visibility of the original symbol.  The valid
-visibilities are @code{local}, @code {hidden}, and @code {remove}.  The
+visibilities are @code{local}, @code{hidden}, and @code{remove}.  The
 @code{local} visibility makes the original symbol a local symbol
 (@pxref{Local}).  The @code{hidden} visibility sets the visibility of the
 original symbol to @code{hidden} (@pxref{Hidden}).  The @code{remove}
-visibility removes the original symbol from the symbol table if it isn't
-used in relocation.  If visibility isn't specified, the original symbol
-is unchanged.
+visibility removes the original symbol from the symbol table.  If visibility
+isn't specified, the original symbol is unchanged.
 
 If the symbol @var{name} is not defined within the file being assembled, all
 references to @var{name} will be changed to @var{name2@@nodename}.  If no
diff --git a/gas/testsuite/gas/symver/symver11.d b/gas/testsuite/gas/symver/symver11.d
index 0e3e7f14b7..caa76e167d 100644
--- a/gas/testsuite/gas/symver/symver11.d
+++ b/gas/testsuite/gas/symver/symver11.d
@@ -1,8 +1,2 @@
-#readelf: -rsW
 #name: symver symver11
-
-#...
-[0-9a-f]+ +[0-9a-f]+ +R_.* +[0-9a-f]+ +foo *.*
-#...
- +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
-#pass
+#error: .*symbol cannot be used on reloc
diff --git a/gas/testsuite/gas/symver/symver11.s b/gas/testsuite/gas/symver/symver11.s
index 08416be0f0..2c7c6e7f6b 100644
--- a/gas/testsuite/gas/symver/symver11.s
+++ b/gas/testsuite/gas/symver/symver11.s
@@ -1,5 +1,5 @@
 	.data
-	.globl foo
+	.weak foo
 	.type foo,%object
 foo:
 	.byte 0
diff --git a/gas/testsuite/gas/symver/symver7.d b/gas/testsuite/gas/symver/symver7.d
index 5152678a48..2e956a6a1b 100644
--- a/gas/testsuite/gas/symver/symver7.d
+++ b/gas/testsuite/gas/symver/symver7.d
@@ -3,6 +3,7 @@
 
 #...
  +[0-9]+: +0+ +1 +OBJECT +GLOBAL +HIDDEN +[0-9]+ +foo
+#...
  +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
  +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
 #pass

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PING: V4 [PATCH] gas: Extend .symver directive
  2020-04-22  8:51                                   ` Alan Modra
@ 2020-04-22 12:14                                     ` H.J. Lu
  0 siblings, 0 replies; 20+ messages in thread
From: H.J. Lu @ 2020-04-22 12:14 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

On Wed, Apr 22, 2020 at 1:52 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Apr 22, 2020 at 11:51:10AM +0930, Alan Modra wrote:
> > On Tue, Apr 21, 2020 at 06:19:45PM -0700, H.J. Lu wrote:
> > > On Tue, Apr 21, 2020 at 4:52 PM Alan Modra <amodra@gmail.com> wrote:
> > > > I wonder if ".symver intsym, extsym@@nodename, remove" ought to really
> > > > remove the symbol resulting in an assembly error if referenced?
>
> Turning this proposal into a proper patch.  What do you think?
>
>         * config/obj-elf.c (elf_frob_symbol): Unconditionally remove
>         symbol for ".symver .. remove".
>         * doc/as.texi (.symver): Update.
>         * testsuite/gas/symver/symver11.s: Make foo weak.
>         * testsuite/gas/symver/symver11.d: Expect an error.
>         * testsuite/gas/symver/symver7.d: Allow other random symbols.
>
> diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
> index 409ea4d6be..4bdddc9056 100644
> --- a/gas/config/obj-elf.c
> +++ b/gas/config/obj-elf.c
> @@ -2569,9 +2569,7 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>               elfsym->internal_elf_sym.st_other |= STV_HIDDEN;
>               break;
>             case visibility_remove:
> -             /* Remove the symbol if it isn't used in relocation.  */
> -             if (!symbol_used_in_reloc_p (symp))
> -               symbol_remove (symp, &symbol_rootP, &symbol_lastP);
> +             symbol_remove (symp, &symbol_rootP, &symbol_lastP);
>               break;
>             case visibility_local:
>               S_CLEAR_EXTERNAL (symp);
> diff --git a/gas/doc/as.texi b/gas/doc/as.texi
> index 8669879c87..a65ddad5f5 100644
> --- a/gas/doc/as.texi
> +++ b/gas/doc/as.texi
> @@ -7129,13 +7129,12 @@ building a shared library.  If you are attempting to override a versioned
>  symbol from a shared library, then @var{nodename} should correspond to the
>  nodename of the symbol you are trying to override.  The optional argument
>  @var{visibility} updates the visibility of the original symbol.  The valid
> -visibilities are @code{local}, @code {hidden}, and @code {remove}.  The
> +visibilities are @code{local}, @code{hidden}, and @code{remove}.  The
>  @code{local} visibility makes the original symbol a local symbol
>  (@pxref{Local}).  The @code{hidden} visibility sets the visibility of the
>  original symbol to @code{hidden} (@pxref{Hidden}).  The @code{remove}
> -visibility removes the original symbol from the symbol table if it isn't
> -used in relocation.  If visibility isn't specified, the original symbol
> -is unchanged.
> +visibility removes the original symbol from the symbol table.  If visibility
> +isn't specified, the original symbol is unchanged.
>
>  If the symbol @var{name} is not defined within the file being assembled, all
>  references to @var{name} will be changed to @var{name2@@nodename}.  If no
> diff --git a/gas/testsuite/gas/symver/symver11.d b/gas/testsuite/gas/symver/symver11.d
> index 0e3e7f14b7..caa76e167d 100644
> --- a/gas/testsuite/gas/symver/symver11.d
> +++ b/gas/testsuite/gas/symver/symver11.d
> @@ -1,8 +1,2 @@
> -#readelf: -rsW
>  #name: symver symver11
> -
> -#...
> -[0-9a-f]+ +[0-9a-f]+ +R_.* +[0-9a-f]+ +foo *.*
> -#...
> - +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
> -#pass
> +#error: .*symbol cannot be used on reloc
> diff --git a/gas/testsuite/gas/symver/symver11.s b/gas/testsuite/gas/symver/symver11.s
> index 08416be0f0..2c7c6e7f6b 100644
> --- a/gas/testsuite/gas/symver/symver11.s
> +++ b/gas/testsuite/gas/symver/symver11.s
> @@ -1,5 +1,5 @@
>         .data
> -       .globl foo
> +       .weak foo
>         .type foo,%object
>  foo:
>         .byte 0
> diff --git a/gas/testsuite/gas/symver/symver7.d b/gas/testsuite/gas/symver/symver7.d
> index 5152678a48..2e956a6a1b 100644
> --- a/gas/testsuite/gas/symver/symver7.d
> +++ b/gas/testsuite/gas/symver/symver7.d
> @@ -3,6 +3,7 @@
>
>  #...
>   +[0-9]+: +0+ +1 +OBJECT +GLOBAL +HIDDEN +[0-9]+ +foo
> +#...
>   +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@version1
>   +[0-9]+: +0+ +1 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo@@version2
>  #pass
>
> --
> Alan Modra
> Australia Development Lab, IBM

Works for me.

Thanks.

-- 
H.J.

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

end of thread, other threads:[~2020-04-22 12:14 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07 12:10 [PATCH] gas: Extend .symver directive H.J. Lu
2020-04-07 12:41 ` Andreas Schwab
2020-04-07 12:49   ` H.J. Lu
2020-04-07 12:55     ` Andreas Schwab
2020-04-07 12:57       ` V2 " H.J. Lu
2020-04-07 21:22         ` Fangrui Song
2020-04-07 23:15           ` H.J. Lu
2020-04-07 23:58             ` Fangrui Song
2020-04-08 12:53               ` H.J. Lu
2020-04-08 13:21                 ` V3 " H.J. Lu
2020-04-09 17:16                   ` Fangrui Song
2020-04-11 14:22                     ` V4 " H.J. Lu
2020-04-20 14:03                       ` PING: " H.J. Lu
2020-04-21 10:21                         ` Nick Clifton
2020-04-21 23:20                           ` Alan Modra
2020-04-21 23:52                             ` Alan Modra
2020-04-22  1:19                               ` H.J. Lu
2020-04-22  2:21                                 ` Alan Modra
2020-04-22  8:51                                   ` Alan Modra
2020-04-22 12:14                                     ` H.J. Lu

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).