[PATCH 1/2] IPA symver: allow multiple symvers for a definition

[PATCH 1/2] IPA symver: allow multiple symvers for a definition

[Martin Liska]

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


gcc/ChangeLog:

	* cgraphunit.c (process_symver_attribute): Allow multiple
	symver attributes for one symbol.
	* doc/extend.texi: Document the change.

gcc/testsuite/ChangeLog:

	* lib/target-supports-dg.exp: Add dg-require-symver.
	* lib/target-supports.exp: Likewise.
	* gcc.dg/ipa/symver1.c: New test.
---
 gcc/cgraphunit.c                         | 143 ++++++++++++-----------
 gcc/doc/extend.texi                      |  10 +-
 gcc/testsuite/gcc.dg/ipa/symver1.c       |  11 ++
 gcc/testsuite/lib/target-supports-dg.exp |  10 ++
 gcc/testsuite/lib/target-supports.exp    |  12 ++
 5 files changed, 110 insertions(+), 76 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-IPA-symver-allow-multiple-symvers-for-a-definition.patch --]
[-- Type: text/x-patch; name="0001-IPA-symver-allow-multiple-symvers-for-a-definition.patch", Size: 7256 bytes --]

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 0b1009d0dea..fa3aec79a48 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -720,80 +720,81 @@ process_symver_attribute (symtab_node *n)
 {
   tree value = lookup_attribute ("symver", DECL_ATTRIBUTES (n->decl));
 
-  if (!value)
-    return;
-  if (lookup_attribute ("symver", TREE_CHAIN (value)))
+  for (; value != NULL; value = TREE_CHAIN (value))
     {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"multiple versions for one symbol");
-      return;
-    }
-  tree symver = get_identifier_with_length
-		  (TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))),
-		   TREE_STRING_LENGTH (TREE_VALUE (TREE_VALUE (value))));
-  symtab_node *def = symtab_node::get_for_asmname (symver);
+      /* Starting from bintuils 2.35 gas supports:
+	  # Assign foo to bar@V1 and baz@V2.
+	  .symver foo, bar@V1
+	  .symver foo, baz@V2
+      */
+
+      tree symver = get_identifier_with_length
+	(TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))),
+	 TREE_STRING_LENGTH (TREE_VALUE (TREE_VALUE (value))));
+      symtab_node *def = symtab_node::get_for_asmname (symver);
+
+      if (def)
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "duplicate definition of a symbol version");
+	  inform (DECL_SOURCE_LOCATION (def->decl),
+		  "same version was previously defined here");
+	  return;
+	}
+      if (!n->definition)
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "symbol needs to be defined to have a version");
+	  return;
+	}
+      if (DECL_COMMON (n->decl))
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "common symbol cannot be versioned");
+	  return;
+	}
+      if (DECL_COMDAT (n->decl))
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "comdat symbol cannot be versioned");
+	  return;
+	}
+      if (n->weakref)
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "%<weakref%> cannot be versioned");
+	  return;
+	}
+      if (!TREE_PUBLIC (n->decl))
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "versioned symbol must be public");
+	  return;
+	}
+      if (DECL_VISIBILITY (n->decl) != VISIBILITY_DEFAULT)
+	{
+	  error_at (DECL_SOURCE_LOCATION (n->decl),
+		    "versioned symbol must have default visibility");
+	  return;
+	}
 
-  if (def)
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"duplicate definition of a symbol version");
-      inform (DECL_SOURCE_LOCATION (def->decl),
-	      "same version was previously defined here");
-      return;
-    }
-  if (!n->definition)
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"symbol needs to be defined to have a version");
-      return;
-    }
-  if (DECL_COMMON (n->decl))
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"common symbol cannot be versioned");
-      return;
-    }
-  if (DECL_COMDAT (n->decl))
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"comdat symbol cannot be versioned");
-      return;
-    }
-  if (n->weakref)
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"%<weakref%> cannot be versioned");
-      return;
+      /* Create new symbol table entry representing the version.  */
+      tree new_decl = copy_node (n->decl);
+
+      DECL_INITIAL (new_decl) = NULL_TREE;
+      if (TREE_CODE (new_decl) == FUNCTION_DECL)
+	DECL_STRUCT_FUNCTION (new_decl) = NULL;
+      SET_DECL_ASSEMBLER_NAME (new_decl, symver);
+      TREE_PUBLIC (new_decl) = 1;
+      DECL_ATTRIBUTES (new_decl) = NULL;
+
+      symtab_node *symver_node = symtab_node::get_create (new_decl);
+      symver_node->alias = true;
+      symver_node->definition = true;
+      symver_node->symver = true;
+      symver_node->create_reference (n, IPA_REF_ALIAS, NULL);
+      symver_node->analyzed = true;
     }
-  if (!TREE_PUBLIC (n->decl))
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"versioned symbol must be public");
-      return;
-    }
-  if (DECL_VISIBILITY (n->decl) != VISIBILITY_DEFAULT)
-    {
-      error_at (DECL_SOURCE_LOCATION (n->decl),
-		"versioned symbol must have default visibility");
-      return;
-    }
-
-  /* Create new symbol table entry representing the version.  */
-  tree new_decl = copy_node (n->decl);
-
-  DECL_INITIAL (new_decl) = NULL_TREE;
-  if (TREE_CODE (new_decl) == FUNCTION_DECL)
-    DECL_STRUCT_FUNCTION (new_decl) = NULL;
-  SET_DECL_ASSEMBLER_NAME (new_decl, symver);
-  TREE_PUBLIC (new_decl) = 1;
-  DECL_ATTRIBUTES (new_decl) = NULL;
-
-  symtab_node *symver_node = symtab_node::get_create (new_decl);
-  symver_node->alias = true;
-  symver_node->definition = true;
-  symver_node->symver = true;
-  symver_node->create_reference (n, IPA_REF_ALIAS, NULL);
-  symver_node->analyzed = true;
 }
 
 /* Process attributes common for vars and functions.  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 2bb9b2f72f5..97ae1ee0843 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3742,13 +3742,13 @@ foo_v1 (void)
 Will produce a @code{.symver foo_v1, foo@@VERS_1} directive in the assembler
 output. 
 
-It's an error to define multiple version of a given symbol.  In such case
-an alias can be used.
+One can also define multiple version for a given symbol.
 
 @smallexample
-__attribute__ ((__symver__ ("foo@@VERS_2")))
-__attribute__ ((alias ("foo_v1")))
-int symver_foo_v1 (void);
+__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3")))
+int symver_foo_v1 (void)
+@{
+@}
 @end smallexample
 
 This example creates an alias of @code{foo_v1} with symbol name
diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c
new file mode 100644
index 00000000000..645de7ea259
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/symver1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+
+__attribute__ ((__symver__ ("foo@VER_2")))
+__attribute__ ((__symver__ ("foo@VER_3")))
+int foo()
+{
+  return 2;
+}
+
+/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */
+/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */
diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp
index 5bb99f4e8f9..4a03eaae9ce 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } {
 	return [dg-process-target-1 $selector]
     }
 }
+
+# If this target does not support the "symver" attribute, skip this
+# test.
+
+proc dg-require-symver { args } {
+    if { ![ check_symver_available ] } {
+	upvar dg-do-what dg-do-what
+	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
+    }
+}
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index c24330a27ab..f3fc5b80aea 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } {
 
     return 1
 }
+# Returns 1 if the target toolchain supports extended
+# syntax of .symver directive, 0 otherwise.
+
+proc check_symver_available { } {
+    return [check_no_compiler_messages symver_available object {
+	    int foo(void) { return 0; }
+	    int main (void) {
+		asm volatile (".symver foo,foo@VER_1, local");
+		return 0;
+	    }
+	}]
+}

[PATCH 2/2] IPA symver: support visibility and static symbols.

[Martin Liska]

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


gcc/ChangeLog:

	* cgraphunit.c (process_symver_attribute): Remove checks that
	are not needed now.
	(cgraph_node::assemble_thunks_and_aliases): Change second
	argument to decl.
	* config/elfos.h (ASM_OUTPUT_SYMVER_DIRECTIVE): Add new
	VISIBILITY parameter.
	* doc/extend.texi: Document that .symver supports visibility.
	* symtab.c (symtab_node::verify_base): Remove checks that
	are not needed now.
	* varasm.c (do_assemble_symver): Detect visibility .symver
	directive argument.
	* varpool.c (varpool_node::assemble_aliases): Change second
	argument to decl.

gcc/testsuite/ChangeLog:

	* gcc.dg/ipa/symver2.c: New test.
	* gcc.dg/ipa/symver3.c: New test.
---
 gcc/cgraphunit.c                   | 15 +--------------
 gcc/config/elfos.h                 | 20 +++++++++++---------
 gcc/doc/extend.texi                |  3 +--
 gcc/symtab.c                       | 16 ----------------
 gcc/testsuite/gcc.dg/ipa/symver2.c |  9 +++++++++
 gcc/testsuite/gcc.dg/ipa/symver3.c | 13 +++++++++++++
 gcc/varasm.c                       | 12 ++++++++++--
 gcc/varpool.c                      |  3 +--
 8 files changed, 46 insertions(+), 45 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver2.c
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver3.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-IPA-symver-support-visibility-and-static-symbols.patch --]
[-- Type: text/x-patch; name="0002-IPA-symver-support-visibility-and-static-symbols.patch", Size: 6097 bytes --]

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index fa3aec79a48..c85d3482c8b 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -765,18 +765,6 @@ process_symver_attribute (symtab_node *n)
 		    "%<weakref%> cannot be versioned");
 	  return;
 	}
-      if (!TREE_PUBLIC (n->decl))
-	{
-	  error_at (DECL_SOURCE_LOCATION (n->decl),
-		    "versioned symbol must be public");
-	  return;
-	}
-      if (DECL_VISIBILITY (n->decl) != VISIBILITY_DEFAULT)
-	{
-	  error_at (DECL_SOURCE_LOCATION (n->decl),
-		    "versioned symbol must have default visibility");
-	  return;
-	}
 
       /* Create new symbol table entry representing the version.  */
       tree new_decl = copy_node (n->decl);
@@ -2239,8 +2227,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
 	     of buffering it in same alias pairs.  */
 	  TREE_ASM_WRITTEN (decl) = 1;
 	  if (alias->symver)
-	    do_assemble_symver (alias->decl,
-				DECL_ASSEMBLER_NAME (decl));
+	    do_assemble_symver (alias->decl, decl);
 	  else
 	    do_assemble_alias (alias->decl,
 			       DECL_ASSEMBLER_NAME (decl));
diff --git a/gcc/config/elfos.h b/gcc/config/elfos.h
index 74a3eafda6b..6c9b73320b9 100644
--- a/gcc/config/elfos.h
+++ b/gcc/config/elfos.h
@@ -248,15 +248,17 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
     }					\
   while (0)
 
-#define ASM_OUTPUT_SYMVER_DIRECTIVE(FILE, NAME, NAME2)		\
-  do								\
-    {								\
-      fputs ("\t.symver\t", (FILE));				\
-      assemble_name ((FILE), (NAME));				\
-      fputs (", ", (FILE));					\
-      assemble_name ((FILE), (NAME2));				\
-      fputc ('\n', (FILE));					\
-    }								\
+#define ASM_OUTPUT_SYMVER_DIRECTIVE(FILE, NAME, NAME2, VISIBILITY)  \
+  do								    \
+    {								    \
+      fputs ("\t.symver\t", (FILE));				    \
+      assemble_name ((FILE), (NAME));				    \
+      fputs (", ", (FILE));					    \
+      assemble_name ((FILE), (NAME2));				    \
+      if (visibility != NULL)					    \
+	fprintf ((FILE), ", %s", (VISIBILITY));			    \
+      fputc ('\n', (FILE));					    \
+    }								    \
   while (0)
 
 /* The following macro defines the format used to output the second
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 97ae1ee0843..22e62f36714 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3729,8 +3729,7 @@ On ELF targets this attribute creates a symbol version.  The @var{name2} part
 of the parameter is the actual name of the symbol by which it will be
 externally referenced.  The @code{nodename} portion should be the name of a
 node specified in the version script supplied to the linker when building a
-shared library.  Versioned symbol must be defined and must be exported with
-default visibility.
+shared library.
 
 @smallexample
 __attribute__ ((__symver__ ("foo@@VERS_1"))) int
diff --git a/gcc/symtab.c b/gcc/symtab.c
index d7dfbb676df..51628fe625f 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -1178,22 +1178,6 @@ symtab_node::verify_base (void)
       error ("node is symver but not alias");
       error_found = true;
     }
-  /* Limitation of gas requires us to output targets of symver aliases as
-     global symbols.  This is binutils PR 25295.  */
-  if (symver
-      && (!TREE_PUBLIC (get_alias_target ()->decl)
-	  || DECL_VISIBILITY (get_alias_target ()->decl) != VISIBILITY_DEFAULT))
-    {
-      error ("symver target is not exported with default visibility");
-      error_found = true;
-    }
-  if (symver
-      && (!TREE_PUBLIC (decl)
-	  || DECL_VISIBILITY (decl) != VISIBILITY_DEFAULT))
-    {
-      error ("symver is not exported with default visibility");
-      error_found = true;
-    }
   if (same_comdat_group)
     {
       symtab_node *n = same_comdat_group;
diff --git a/gcc/testsuite/gcc.dg/ipa/symver2.c b/gcc/testsuite/gcc.dg/ipa/symver2.c
new file mode 100644
index 00000000000..9cb50a54e08
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/symver2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+__attribute__ ((__symver__ ("foo@VER_2")))
+static int foo()
+{
+  return 2;
+}
+
+/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2, remove" } } */
diff --git a/gcc/testsuite/gcc.dg/ipa/symver3.c b/gcc/testsuite/gcc.dg/ipa/symver3.c
new file mode 100644
index 00000000000..fb30503d808
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/symver3.c
@@ -0,0 +1,13 @@
+/* { dg-do link } */
+/* { dg-require-symver "" } */
+
+__attribute__ ((__symver__ ("foo@VERS_2")))
+int foo()
+{
+  return 2;
+}
+
+int main()
+{
+  return foo() - 2;
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 84df52013d7..9a0f12e17f3 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -5964,15 +5964,23 @@ do_assemble_alias (tree decl, tree target)
 /* Output .symver directive.  */
 
 void
-do_assemble_symver (tree decl, tree target)
+do_assemble_symver (tree decl, tree origin_decl)
 {
+  tree target = DECL_ASSEMBLER_NAME (origin_decl);
   tree id = DECL_ASSEMBLER_NAME (decl);
   ultimate_transparent_alias_target (&id);
   ultimate_transparent_alias_target (&target);
 #ifdef ASM_OUTPUT_SYMVER_DIRECTIVE
+  const char *visibility = NULL;
+  if (!TREE_PUBLIC (origin_decl))
+    visibility = "remove";
+  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL)
+    visibility = "local";
+  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
+    visibility = "hidden";
   ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,
 			       IDENTIFIER_POINTER (target),
-			       IDENTIFIER_POINTER (id));
+			       IDENTIFIER_POINTER (id), visibility);
 #else
   error ("symver is only supported on ELF platforms");
 #endif
diff --git a/gcc/varpool.c b/gcc/varpool.c
index 458cdf1bf37..95d7844a398 100644
--- a/gcc/varpool.c
+++ b/gcc/varpool.c
@@ -540,8 +540,7 @@ varpool_node::assemble_aliases (void)
     {
       varpool_node *alias = dyn_cast <varpool_node *> (ref->referring);
       if (alias->symver)
-	do_assemble_symver (alias->decl,
-			    DECL_ASSEMBLER_NAME (decl));
+	do_assemble_symver (alias->decl, decl);
       else if (!alias->transparent_alias)
 	do_assemble_alias (alias->decl,
 			   DECL_ASSEMBLER_NAME (decl));

[PATCH 0/2] symver: extend functionality

[Martin Liska]

Hey.

Since the bintuils release 2.35, we can now support new .symver syntax added in:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6914be53bd662eefd979d0c82d2e20e108c4ee66

Patch survives bootstrap and regression tests.
Thoughts?
Martin

Martin Liska (2):
  IPA symver: allow multiple symvers for a definition
  IPA symver: support visibility and static symbols.

 gcc/cgraphunit.c                         | 134 +++++++++++------------
 gcc/config/elfos.h                       |  20 ++--
 gcc/doc/extend.texi                      |  13 +--
 gcc/symtab.c                             |  16 ---
 gcc/testsuite/gcc.dg/ipa/symver1.c       |  11 ++
 gcc/testsuite/gcc.dg/ipa/symver2.c       |   9 ++
 gcc/testsuite/gcc.dg/ipa/symver3.c       |  13 +++
 gcc/testsuite/lib/target-supports-dg.exp |  10 ++
 gcc/testsuite/lib/target-supports.exp    |  12 ++
 gcc/varasm.c                             |  12 +-
 gcc/varpool.c                            |   3 +-
 11 files changed, 144 insertions(+), 109 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver2.c
 create mode 100644 gcc/testsuite/gcc.dg/ipa/symver3.c

-- 
2.28.0

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Jan Hubicka]

> 
> gcc/ChangeLog:
> 
> 	* cgraphunit.c (process_symver_attribute): Remove checks that
> 	are not needed now.
> 	(cgraph_node::assemble_thunks_and_aliases): Change second
> 	argument to decl.
> 	* config/elfos.h (ASM_OUTPUT_SYMVER_DIRECTIVE): Add new
> 	VISIBILITY parameter.
> 	* doc/extend.texi: Document that .symver supports visibility.
> 	* symtab.c (symtab_node::verify_base): Remove checks that
> 	are not needed now.
> 	* varasm.c (do_assemble_symver): Detect visibility .symver
> 	directive argument.
> 	* varpool.c (varpool_node::assemble_aliases): Change second
> 	argument to decl.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/ipa/symver2.c: New test.
> 	* gcc.dg/ipa/symver3.c: New test.

Thanks for looking into this!
>  void
> -do_assemble_symver (tree decl, tree target)
> +do_assemble_symver (tree decl, tree origin_decl)
>  {
> +  tree target = DECL_ASSEMBLER_NAME (origin_decl);
>    tree id = DECL_ASSEMBLER_NAME (decl);
>    ultimate_transparent_alias_target (&id);
>    ultimate_transparent_alias_target (&target);
>  #ifdef ASM_OUTPUT_SYMVER_DIRECTIVE
> +  const char *visibility = NULL;
> +  if (!TREE_PUBLIC (origin_decl))
> +    visibility = "remove";
> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL)
> +    visibility = "local";
> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
> +    visibility = "hidden";

What will happen here with protected visibility?

Otherwise patch makes sense to me.
Honza
>    ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,
>  			       IDENTIFIER_POINTER (target),
> -			       IDENTIFIER_POINTER (id));
> +			       IDENTIFIER_POINTER (id), visibility);
>  #else
>    error ("symver is only supported on ELF platforms");
>  #endif
> diff --git a/gcc/varpool.c b/gcc/varpool.c
> index 458cdf1bf37..95d7844a398 100644
> --- a/gcc/varpool.c
> +++ b/gcc/varpool.c
> @@ -540,8 +540,7 @@ varpool_node::assemble_aliases (void)
>      {
>        varpool_node *alias = dyn_cast <varpool_node *> (ref->referring);
>        if (alias->symver)
> -	do_assemble_symver (alias->decl,
> -			    DECL_ASSEMBLER_NAME (decl));
> +	do_assemble_symver (alias->decl, decl);
>        else if (!alias->transparent_alias)
>  	do_assemble_alias (alias->decl,
>  			   DECL_ASSEMBLER_NAME (decl));

Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition

[Jan Hubicka]

> 
> gcc/ChangeLog:
> 
> 	* cgraphunit.c (process_symver_attribute): Allow multiple
> 	symver attributes for one symbol.
> 	* doc/extend.texi: Document the change.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* lib/target-supports-dg.exp: Add dg-require-symver.
> 	* lib/target-supports.exp: Likewise.
> 	* gcc.dg/ipa/symver1.c: New test.
> ---
>  gcc/cgraphunit.c                         | 143 ++++++++++++-----------
>  gcc/doc/extend.texi                      |  10 +-
>  gcc/testsuite/gcc.dg/ipa/symver1.c       |  11 ++
>  gcc/testsuite/lib/target-supports-dg.exp |  10 ++
>  gcc/testsuite/lib/target-supports.exp    |  12 ++
>  5 files changed, 110 insertions(+), 76 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c
> 

>  @smallexample
> -__attribute__ ((__symver__ ("foo@@VERS_2")))
> -__attribute__ ((alias ("foo_v1")))
> -int symver_foo_v1 (void);
> +__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3")))
> +int symver_foo_v1 (void)
So we can still write it as follows:
+__attribute__ ((__symver__ ("foo@@VERS_2"))))
+__attribute__ ((__symver__ ("foo@@VERS_3"))))
+int symver_foo_v1 (void)

I think we should support this syntax so the versions can be added
separately by macros mixed with other attributs....

Honza
> +@{
> +@}
>  @end smallexample
>  
>  This example creates an alias of @code{foo_v1} with symbol name
> diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c
> new file mode 100644
> index 00000000000..645de7ea259
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/symver1.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +
> +__attribute__ ((__symver__ ("foo@VER_2")))
> +__attribute__ ((__symver__ ("foo@VER_3")))
> +int foo()
> +{
> +  return 2;
> +}
> +
> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */
> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */
> diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp
> index 5bb99f4e8f9..4a03eaae9ce 100644
> --- a/gcc/testsuite/lib/target-supports-dg.exp
> +++ b/gcc/testsuite/lib/target-supports-dg.exp
> @@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } {
>  	return [dg-process-target-1 $selector]
>      }
>  }
> +
> +# If this target does not support the "symver" attribute, skip this
> +# test.
> +
> +proc dg-require-symver { args } {
> +    if { ![ check_symver_available ] } {
> +	upvar dg-do-what dg-do-what
> +	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
> +    }
> +}
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index c24330a27ab..f3fc5b80aea 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } {
>  
>      return 1
>  }
> +# Returns 1 if the target toolchain supports extended
> +# syntax of .symver directive, 0 otherwise.
> +
> +proc check_symver_available { } {
> +    return [check_no_compiler_messages symver_available object {
> +	    int foo(void) { return 0; }
> +	    int main (void) {
> +		asm volatile (".symver foo,foo@VER_1, local");
> +		return 0;
> +	    }
> +	}]
> +}

Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition

[Martin Liška]

On 8/25/20 8:48 PM, Jan Hubicka wrote:
>>
>> gcc/ChangeLog:
>>
>> 	* cgraphunit.c (process_symver_attribute): Allow multiple
>> 	symver attributes for one symbol.
>> 	* doc/extend.texi: Document the change.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* lib/target-supports-dg.exp: Add dg-require-symver.
>> 	* lib/target-supports.exp: Likewise.
>> 	* gcc.dg/ipa/symver1.c: New test.
>> ---
>>   gcc/cgraphunit.c                         | 143 ++++++++++++-----------
>>   gcc/doc/extend.texi                      |  10 +-
>>   gcc/testsuite/gcc.dg/ipa/symver1.c       |  11 ++
>>   gcc/testsuite/lib/target-supports-dg.exp |  10 ++
>>   gcc/testsuite/lib/target-supports.exp    |  12 ++
>>   5 files changed, 110 insertions(+), 76 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/ipa/symver1.c
>>
> 
>>   @smallexample
>> -__attribute__ ((__symver__ ("foo@@VERS_2")))
>> -__attribute__ ((alias ("foo_v1")))
>> -int symver_foo_v1 (void);
>> +__attribute__ ((__symver__ ("foo@@VERS_2"), ("foo@@VERS_3")))
>> +int symver_foo_v1 (void)
> So we can still write it as follows:
> +__attribute__ ((__symver__ ("foo@@VERS_2"))))
> +__attribute__ ((__symver__ ("foo@@VERS_3"))))
> +int symver_foo_v1 (void)
> 
> I think we should support this syntax so the versions can be added
> separately by macros mixed with other attributs....

Yes, the syntax is supported as well and I'm adding that to documentation.

I'm going to push this commit.

Thanks for review,
Martin

> 
> Honza
>> +@{
>> +@}
>>   @end smallexample
>>   
>>   This example creates an alias of @code{foo_v1} with symbol name
>> diff --git a/gcc/testsuite/gcc.dg/ipa/symver1.c b/gcc/testsuite/gcc.dg/ipa/symver1.c
>> new file mode 100644
>> index 00000000000..645de7ea259
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/ipa/symver1.c
>> @@ -0,0 +1,11 @@
>> +/* { dg-do compile } */
>> +
>> +__attribute__ ((__symver__ ("foo@VER_2")))
>> +__attribute__ ((__symver__ ("foo@VER_3")))
>> +int foo()
>> +{
>> +  return 2;
>> +}
>> +
>> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_2" } } */
>> +/* { dg-final { scan-assembler ".symver.*foo, foo@VER_3" } } */
>> diff --git a/gcc/testsuite/lib/target-supports-dg.exp b/gcc/testsuite/lib/target-supports-dg.exp
>> index 5bb99f4e8f9..4a03eaae9ce 100644
>> --- a/gcc/testsuite/lib/target-supports-dg.exp
>> +++ b/gcc/testsuite/lib/target-supports-dg.exp
>> @@ -665,3 +665,13 @@ if { [info procs saved-dg-process-target] == [list] } {
>>   	return [dg-process-target-1 $selector]
>>       }
>>   }
>> +
>> +# If this target does not support the "symver" attribute, skip this
>> +# test.
>> +
>> +proc dg-require-symver { args } {
>> +    if { ![ check_symver_available ] } {
>> +	upvar dg-do-what dg-do-what
>> +	set dg-do-what [list [lindex ${dg-do-what} 0] "N" "P"]
>> +    }
>> +}
>> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
>> index c24330a27ab..f3fc5b80aea 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -10445,3 +10445,15 @@ proc check_effective_target_large_initializer { } {
>>   
>>       return 1
>>   }
>> +# Returns 1 if the target toolchain supports extended
>> +# syntax of .symver directive, 0 otherwise.
>> +
>> +proc check_symver_available { } {
>> +    return [check_no_compiler_messages symver_available object {
>> +	    int foo(void) { return 0; }
>> +	    int main (void) {
>> +		asm volatile (".symver foo,foo@VER_1, local");
>> +		return 0;
>> +	    }
>> +	}]
>> +}
> 

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Martin Liška]

On 8/25/20 8:46 PM, Jan Hubicka wrote:
> What will happen here with protected visibility?

I forgot about it. Should it be mapped also to "local"?

+  const char *visibility = NULL;
+  if (!TREE_PUBLIC (origin_decl))
+    visibility = "remove";
+  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
+          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
+    visibility = "local";
+  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
+    visibility = "hidden";

Thanks,
Martin

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Jan Hubicka]

> On 8/25/20 8:46 PM, Jan Hubicka wrote:
> > What will happen here with protected visibility?
> 
> I forgot about it. Should it be mapped also to "local"?
> 
> +  const char *visibility = NULL;
> +  if (!TREE_PUBLIC (origin_decl))
> +    visibility = "remove";
> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
> +          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
> +    visibility = "local";
> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
> +    visibility = "hidden";

I have no idea (depends what gas will do), you need to check if the
resulting symbol will indeed have right visibility in all of the cases.
If some of them are not possible, I suppose we could just reject the
comination.

Honza
> 
> Thanks,
> Martin

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Martin Liška]

On 8/26/20 11:22 AM, Jan Hubicka wrote:
>> On 8/25/20 8:46 PM, Jan Hubicka wrote:
>>> What will happen here with protected visibility?
>>
>> I forgot about it. Should it be mapped also to "local"?
>>
>> +  const char *visibility = NULL;
>> +  if (!TREE_PUBLIC (origin_decl))
>> +    visibility = "remove";
>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
>> +          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
>> +    visibility = "local";
>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
>> +    visibility = "hidden";
> 
> I have no idea (depends what gas will do), you need to check if the
> resulting symbol will indeed have right visibility in all of the cases.
> If some of them are not possible, I suppose we could just reject the
> comination.

Ok, so I experimented a bit with the .symver attribute I don't see how
is the new syntax handy (I mean .symver foo, foo@VERS_2, hidden and
.symver foo, foo@VERS_1, local)?

For instance:

$ cat vi2.c
int
__attribute__((visibility ("hidden")))
hidden_object;

extern int
__attribute__ ((__symver__ ("foo@@VERS_2")))
__attribute__ ((alias ("hidden_object")))
symver_foo_v1;

$ gcc vi2.c -S
$ cat vi2.s | grep '\.symver'
	.symver	symver_foo_v1, foo@@VERS_2

$ readelf -s vi2.o -W
...
      8: 0000000000000000     4 OBJECT  GLOBAL HIDDEN     3 hidden_object
      9: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 symver_foo_v1
     10: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 foo@@VERS_2

Which seems fine to me. Similarly one can directly modify visibility of
symver_foo_v1 with:
.hidden	symver_foo_v1

Or do I miss something?

Martin

> 
> Honza
>>
>> Thanks,
>> Martin

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Jan Hubicka]

> On 8/26/20 11:22 AM, Jan Hubicka wrote:
> > > On 8/25/20 8:46 PM, Jan Hubicka wrote:
> > > > What will happen here with protected visibility?
> > > 
> > > I forgot about it. Should it be mapped also to "local"?
> > > 
> > > +  const char *visibility = NULL;
> > > +  if (!TREE_PUBLIC (origin_decl))
> > > +    visibility = "remove";
> > > +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
> > > +          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
> > > +    visibility = "local";
> > > +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
> > > +    visibility = "hidden";
> > 
> > I have no idea (depends what gas will do), you need to check if the
> > resulting symbol will indeed have right visibility in all of the cases.
> > If some of them are not possible, I suppose we could just reject the
> > comination.
> 
> Ok, so I experimented a bit with the .symver attribute I don't see how
> is the new syntax handy (I mean .symver foo, foo@VERS_2, hidden and
> .symver foo, foo@VERS_1, local)?
> 
> For instance:
> 
> $ cat vi2.c
> int
> __attribute__((visibility ("hidden")))
> hidden_object;
> 
> extern int
> __attribute__ ((__symver__ ("foo@@VERS_2")))
> __attribute__ ((alias ("hidden_object")))
> symver_foo_v1;
> 
> $ gcc vi2.c -S
> $ cat vi2.s | grep '\.symver'
> 	.symver	symver_foo_v1, foo@@VERS_2
> 
> $ readelf -s vi2.o -W
> ...
>      8: 0000000000000000     4 OBJECT  GLOBAL HIDDEN     3 hidden_object
>      9: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 symver_foo_v1
>     10: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 foo@@VERS_2
> 
> Which seems fine to me. Similarly one can directly modify visibility of
> symver_foo_v1 with:
> .hidden	symver_foo_v1
> 
> Or do I miss something?

At low-level (and how GCC symtab should understand it), symver is just
a symbol with funny name.  Alias is just another symbol pointing to same
place and in general we want to handle versions as aliases with funny
names.

But it is not how gas interpret its.
Writting .symver foo, foo@VERS_2 in my understnading does two things
1) it makes gas to turn all apperances of references to "foo" to "foo@VERS_2"
   this is necessary since gas syntax does not allow names with @ in it
   so otherwise we have no way to reffer to it
2) it either exports the foo symbol (and you can add visibility).
   or makes foo@VERS_2 disappear depending how you declare visibilities
   of foo.

With this we lose way to refernece to actul symbol foo (and not
foo@VERS_2) which may be needed in LTO if one symbol declares foo and
other declares foo with symtab attribute.

So I think we want symver attributes of symbol "foo" to produce a local
alias "foo.localalias" which will be renamed for GAS to "foo@VERS_2".
Symtab can understand this via syntactic aliases.  Then we have way to
refer to symbol "foo" if we want "foo" and "foo.localalias" if we want
"foo@VERS_2".  I had patch for that but I stopped on the situation that
there was no way to prevent gas from doing 2).

So I see reason for .symbol X,Y, local
I do not see much morivation for others, that is why I stopped on
updating the patch for new gas syntax - wanted to take some time to
understand it (and the time did not materialized).

So it seems to me that taking that old patch (or patch of yours) and add
the local alias machinery will do the trick, but I may be missing
something.
Honza

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Martin Liška]

On 8/26/20 1:27 PM, Jan Hubicka wrote:
>> On 8/26/20 11:22 AM, Jan Hubicka wrote:
>>>> On 8/25/20 8:46 PM, Jan Hubicka wrote:
>>>>> What will happen here with protected visibility?
>>>>
>>>> I forgot about it. Should it be mapped also to "local"?
>>>>
>>>> +  const char *visibility = NULL;
>>>> +  if (!TREE_PUBLIC (origin_decl))
>>>> +    visibility = "remove";
>>>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
>>>> +          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
>>>> +    visibility = "local";
>>>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
>>>> +    visibility = "hidden";
>>>
>>> I have no idea (depends what gas will do), you need to check if the
>>> resulting symbol will indeed have right visibility in all of the cases.
>>> If some of them are not possible, I suppose we could just reject the
>>> comination.
>>
>> Ok, so I experimented a bit with the .symver attribute I don't see how
>> is the new syntax handy (I mean .symver foo, foo@VERS_2, hidden and
>> .symver foo, foo@VERS_1, local)?
>>
>> For instance:
>>
>> $ cat vi2.c
>> int
>> __attribute__((visibility ("hidden")))
>> hidden_object;
>>
>> extern int
>> __attribute__ ((__symver__ ("foo@@VERS_2")))
>> __attribute__ ((alias ("hidden_object")))
>> symver_foo_v1;
>>
>> $ gcc vi2.c -S
>> $ cat vi2.s | grep '\.symver'
>> 	.symver	symver_foo_v1, foo@@VERS_2
>>
>> $ readelf -s vi2.o -W
>> ...
>>       8: 0000000000000000     4 OBJECT  GLOBAL HIDDEN     3 hidden_object
>>       9: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 symver_foo_v1
>>      10: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 foo@@VERS_2
>>
>> Which seems fine to me. Similarly one can directly modify visibility of
>> symver_foo_v1 with:
>> .hidden	symver_foo_v1
>>
>> Or do I miss something?
> 
> At low-level (and how GCC symtab should understand it), symver is just
> a symbol with funny name.  Alias is just another symbol pointing to same
> place and in general we want to handle versions as aliases with funny
> names.
> 
> But it is not how gas interpret its.
> Writting .symver foo, foo@VERS_2 in my understnading does two things
> 1) it makes gas to turn all apperances of references to "foo" to "foo@VERS_2"
>     this is necessary since gas syntax does not allow names with @ in it
>     so otherwise we have no way to reffer to it
> 2) it either exports the foo symbol (and you can add visibility).
>     or makes foo@VERS_2 disappear depending how you declare visibilities
>     of foo.
> 
> With this we lose way to refernece to actul symbol foo (and not
> foo@VERS_2) which may be needed in LTO if one symbol declares foo and
> other declares foo with symtab attribute.
> 
> So I think we want symver attributes of symbol "foo" to produce a local
> alias "foo.localalias" which will be renamed for GAS to "foo@VERS_2".
> Symtab can understand this via syntactic aliases.  Then we have way to
> refer to symbol "foo" if we want "foo" and "foo.localalias" if we want
> "foo@VERS_2".  I had patch for that but I stopped on the situation that
> there was no way to prevent gas from doing 2).

Now I see how is this more complicated :) Can you please send the patch
you have for this?

> 
> So I see reason for .symbol X,Y, local
> I do not see much morivation for others, that is why I stopped on
> updating the patch for new gas syntax - wanted to take some time to
> understand it (and the time did not materialized).
> 
> So it seems to me that taking that old patch (or patch of yours) and add
> the local alias machinery will do the trick, but I may be missing
> something.

Maybe H.J. can chime in what was motivation for implementation of the syntax?

MArtin

> Honza
> 

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[H.J. Lu]

On Wed, Aug 26, 2020 at 4:34 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 8/26/20 1:27 PM, Jan Hubicka wrote:
> >> On 8/26/20 11:22 AM, Jan Hubicka wrote:
> >>>> On 8/25/20 8:46 PM, Jan Hubicka wrote:
> >>>>> What will happen here with protected visibility?
> >>>>
> >>>> I forgot about it. Should it be mapped also to "local"?
> >>>>
> >>>> +  const char *visibility = NULL;
> >>>> +  if (!TREE_PUBLIC (origin_decl))
> >>>> +    visibility = "remove";
> >>>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_INTERNAL
> >>>> +          || DECL_VISIBILITY (origin_decl) == VISIBILITY_PROTECTED)
> >>>> +    visibility = "local";
> >>>> +  else if (DECL_VISIBILITY (origin_decl) == VISIBILITY_HIDDEN)
> >>>> +    visibility = "hidden";
> >>>
> >>> I have no idea (depends what gas will do), you need to check if the
> >>> resulting symbol will indeed have right visibility in all of the cases.
> >>> If some of them are not possible, I suppose we could just reject the
> >>> comination.
> >>
> >> Ok, so I experimented a bit with the .symver attribute I don't see how
> >> is the new syntax handy (I mean .symver foo, foo@VERS_2, hidden and
> >> .symver foo, foo@VERS_1, local)?
> >>
> >> For instance:
> >>
> >> $ cat vi2.c
> >> int
> >> __attribute__((visibility ("hidden")))
> >> hidden_object;
> >>
> >> extern int
> >> __attribute__ ((__symver__ ("foo@@VERS_2")))
> >> __attribute__ ((alias ("hidden_object")))
> >> symver_foo_v1;
> >>
> >> $ gcc vi2.c -S
> >> $ cat vi2.s | grep '\.symver'
> >>      .symver symver_foo_v1, foo@@VERS_2
> >>
> >> $ readelf -s vi2.o -W
> >> ...
> >>       8: 0000000000000000     4 OBJECT  GLOBAL HIDDEN     3 hidden_object
> >>       9: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 symver_foo_v1
> >>      10: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    3 foo@@VERS_2
> >>
> >> Which seems fine to me. Similarly one can directly modify visibility of
> >> symver_foo_v1 with:
> >> .hidden      symver_foo_v1
> >>
> >> Or do I miss something?
> >
> > At low-level (and how GCC symtab should understand it), symver is just
> > a symbol with funny name.  Alias is just another symbol pointing to same
> > place and in general we want to handle versions as aliases with funny
> > names.
> >
> > But it is not how gas interpret its.
> > Writting .symver foo, foo@VERS_2 in my understnading does two things
> > 1) it makes gas to turn all apperances of references to "foo" to "foo@VERS_2"
> >     this is necessary since gas syntax does not allow names with @ in it
> >     so otherwise we have no way to reffer to it
> > 2) it either exports the foo symbol (and you can add visibility).
> >     or makes foo@VERS_2 disappear depending how you declare visibilities
> >     of foo.
> >
> > With this we lose way to refernece to actul symbol foo (and not
> > foo@VERS_2) which may be needed in LTO if one symbol declares foo and
> > other declares foo with symtab attribute.
> >
> > So I think we want symver attributes of symbol "foo" to produce a local
> > alias "foo.localalias" which will be renamed for GAS to "foo@VERS_2".
> > Symtab can understand this via syntactic aliases.  Then we have way to
> > refer to symbol "foo" if we want "foo" and "foo.localalias" if we want
> > "foo@VERS_2".  I had patch for that but I stopped on the situation that
> > there was no way to prevent gas from doing 2).
>
> Now I see how is this more complicated :) Can you please send the patch
> you have for this?
>
> >
> > So I see reason for .symbol X,Y, local
> > I do not see much morivation for others, that is why I stopped on
> > updating the patch for new gas syntax - wanted to take some time to
> > understand it (and the time did not materialized).
> >
> > So it seems to me that taking that old patch (or patch of yours) and add
> > the local alias machinery will do the trick, but I may be missing
> > something.
>
> Maybe H.J. can chime in what was motivation for implementation of the syntax?

What is the question?

-- 
H.J.

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[Martin Liška]

On 8/26/20 2:22 PM, H.J. Lu wrote:
> What is the question?

What are the scenarios where the new syntax:

   .symver foo, foo@VERS_1, local    # Change foo to a local symbol.
   .symver foo, foo@VERS_2, hidden   # Change foo to a hidden symbol.

is useful?

Thanks,
Martin

Re: [PATCH 2/2] IPA symver: support visibility and static symbols.

[H.J. Lu]

On Wed, Aug 26, 2020 at 5:24 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 8/26/20 2:22 PM, H.J. Lu wrote:
> > What is the question?
>
> What are the scenarios where the new syntax:
>
>    .symver foo, foo@VERS_1, local    # Change foo to a local symbol.

foo is local to the file.  Use it there is no global reference to foo.  It is
usually used to define foo@VERS_1.

>    .symver foo, foo@VERS_2, hidden   # Change foo to a hidden symbol.
>

foo is local to shared object or executable.  It provides internal reference
to foo.

The difference is internal reference to foo.

-- 
H.J.

Re: [PATCH 1/2] IPA symver: allow multiple symvers for a definition

[Martin Liška]

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

Hi.

There's one obvious fix that I've just noticed.

I'm going to install it.
Martin

[-- Attachment #2: 0001-symver-fix-attribute-matching.patch --]
[-- Type: text/x-patch, Size: 853 bytes --]

From a5d075d595d35cd5d6607bbd9c8b2eb7707ca920 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Wed, 26 Aug 2020 13:18:14 +0200
Subject: [PATCH] symver: fix attribute matching.

gcc/ChangeLog:

	* cgraphunit.c (process_symver_attribute): Match only symver
	TREE_PURPOSE.
---
 gcc/cgraphunit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index fa3aec79a48..26d3995a0c0 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -727,6 +727,9 @@ process_symver_attribute (symtab_node *n)
 	  .symver foo, bar@V1
 	  .symver foo, baz@V2
       */
+      const char *purpose = IDENTIFIER_POINTER (TREE_PURPOSE (value));
+      if (strcmp (purpose, "symver") != 0)
+	continue;
 
       tree symver = get_identifier_with_length
 	(TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (value))),
-- 
2.28.0