[PATCH] gas: Extend .symver directive

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gas: Extend .symver directive

Andreas Schwab-2
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, [hidden email]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Tue, Apr 7, 2020 at 5:41 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] gas: Extend .symver directive

Andreas Schwab-2
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, [hidden email]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

V2 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.

0001-gas-Extend-.symver-directive.patch (24K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] gas: Extend .symver directive

Fangrui Song-2
On 2020-04-07, H.J. Lu via Binutils wrote:

>On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:

>
> On 2020-04-07, H.J. Lu via Binutils wrote:
> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] gas: Extend .symver directive

Fangrui Song-2
On 2020-04-07, H.J. Lu wrote:

>On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
>>
>> On 2020-04-07, H.J. Lu via Binutils wrote:
>> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <[hidden email]> wrote:

>
> On 2020-04-07, H.J. Lu wrote:
> >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
> >>
> >> On 2020-04-07, H.J. Lu via Binutils wrote:
> >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

V3 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <[hidden email]> wrote:

>
> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <[hidden email]> wrote:
> >
> > On 2020-04-07, H.J. Lu wrote:
> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
> > >>
> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.

0001-gas-Extend-.symver-directive.patch (25K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: V3 [PATCH] gas: Extend .symver directive

Fangrui Song-2
On 2020-04-08, H.J. Lu wrote:

>On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <[hidden email]> wrote:
>>
>> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <[hidden email]> wrote:
>> >
>> > On 2020-04-07, H.J. Lu wrote:
>> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
>> > >>
>> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
>> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Thu, Apr 9, 2020 at 10:16 AM Fangrui Song <[hidden email]> wrote:

>
> On 2020-04-08, H.J. Lu wrote:
> >On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <[hidden email]> wrote:
> >>
> >> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <[hidden email]> wrote:
> >> >
> >> > On 2020-04-07, H.J. Lu wrote:
> >> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
> >> > >>
> >> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> >> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.

0001-gas-Extend-.symver-directive.patch (41K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Sat, Apr 11, 2020 at 7:22 AM H.J. Lu <[hidden email]> wrote:

>
> On Thu, Apr 9, 2020 at 10:16 AM Fangrui Song <[hidden email]> wrote:
> >
> > On 2020-04-08, H.J. Lu wrote:
> > >On Wed, Apr 8, 2020 at 5:53 AM H.J. Lu <[hidden email]> wrote:
> > >>
> > >> On Tue, Apr 7, 2020 at 4:58 PM Fangrui Song <[hidden email]> wrote:
> > >> >
> > >> > On 2020-04-07, H.J. Lu wrote:
> > >> > >On Tue, Apr 7, 2020 at 2:23 PM Fangrui Song <[hidden email]> wrote:
> > >> > >>
> > >> > >> On 2020-04-07, H.J. Lu via Binutils wrote:
> > >> > >> >On Tue, Apr 7, 2020 at 5:55 AM Andreas Schwab <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
Hi H.J.

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

Approved - please apply.

Cheers
  Nick


Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
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
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
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
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Tue, Apr 21, 2020 at 4:52 PM Alan Modra <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: PING: V4 [PATCH] gas: Extend .symver directive

Sourceware - binutils list mailing list
On Wed, Apr 22, 2020 at 1:52 AM Alan Modra <[hidden email]> 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 <[hidden email]> 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.