On 11/15/19 9:04 AM, Jan Hubicka wrote:
Hi,
thanks for feedback. Here is updated patch that incorporates Martin's
changes, formatting corrections and makes symvers of aliases work.
Just a couple of questions and a few minor nits, mostly having to
do with my favorite subject of quoting things in diagnostics ;-)
Honza
* c-family/c-attribs.c (handle_symver_attribute): New.
(c_common_attribytes): Add symver.
* cgraph.h (symtab_node): Add symver flag.
* cgraphunit.c (process_symver_attribute): New.
(process_common_attributes): Use it.
(cgraph_node::assemble_thunks_and_aliases): Assemble symvers.
* config/elfos.h (ASM_OUTPUT_SYMVER_DIRECTIVE): New.
* lto-cgraph.c (lto_output_node, lto_output_varpool_node,
input_overwrite_node, input_varpool_node): Stream symver attributes.
* output.h (do_assemble_symver): Declare.
* symtab.c (symtab_node::dump_base): Dump symver flag.
(symtab_node::verify_base): Sanity check symver.
(symtab_node::resolve_alias): Add support for symvers.
* varasm.c (do_assemble_symver): New.
* varpool.c (varpool_node::assemble_aliases): Output symver.
* doc/extend.texi (symver): Document new attribute.
Index: c-family/c-attribs.c
===================================================================
--- c-family/c-attribs.c (revision 278293)
+++ c-family/c-attribs.c (working copy)
@@ -146,6 +146,7 @@ static tree handle_omp_declare_target_at
static tree handle_designated_init_attribute (tree *, tree, tree, int, bool
*);
static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
int, bool *);
+static tree handle_symver_attribute (tree *, tree, tree, int, bool *);
static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
/* Helper to define attribute exclusions. */
@@ -473,6 +474,8 @@ const struct attribute_spec c_common_att
NULL },
{ "nocf_check", 0, 0, false, true, true, true,
handle_nocf_check_attribute, NULL },
+ { "symver", 1, -1, true, false, false, false,
+ handle_symver_attribute, NULL},
{ "copy", 1, 1, false, false, false, false,
handle_copy_attribute, NULL },
{ "noinit", 0, 0, true, false, false, false,
@@ -2329,6 +2332,59 @@ handle_noplt_attribute (tree *node, tree
return NULL_TREE;
}
+static tree
+handle_symver_attribute (tree *node, tree ARG_UNUSED (name), tree args,
+ int ARG_UNUSED (flags), bool *no_add_attrs)
+{
+ tree symver;
+ const char *symver_str;
+
+ if (TREE_CODE (*node) != FUNCTION_DECL && TREE_CODE (*node) != VAR_DECL)
Should the attribute apply to member functions as well? If not,
FWIW, I think VAR_OR_FUNCTION_DECL_P() tests for just functions
and variables.
+ {
+ warning (OPT_Wattributes,
+ "%<symver%> attribute is only applicable on functions and "
+ "variables");
"applicable to functions" as below (though I think the far more
common phrase among GCC messages is "applies to functions").
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ if (!decl_in_symtab_p (*node))
+ {
+ warning (OPT_Wattributes,
+ "%<symver%> attribute is only applicable to symbols");
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ for (; args; args = TREE_CHAIN (args))
+ {
+ symver = TREE_VALUE (args);
+ if (TREE_CODE (symver) != STRING_CST)
+ {
+ error ("%<symver%> attribute argument not a string constant");
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ symver_str = TREE_STRING_POINTER (symver);
+
+ int ats = 0;
+ for (int n = 0; n < TREE_STRING_LENGTH (symver); n++)
+ if (symver_str[n] == '@')
+ ats++;
+
+ if (ats != 1 && ats != 2)
+ {
+ error ("symver attribute argument must have format "
The "symver" part should be quoted same as above.
+ "%<name@nodename%>");
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+ }
+
+ return NULL_TREE;
+}
+
/* Handle an "alias" or "ifunc" attribute; arguments as in
struct attribute_spec.handler, except that IS_ALIAS tells us
whether this is an alias as opposed to ifunc attribute. */
Index: cgraph.h
===================================================================
--- cgraph.h (revision 278293)
+++ cgraph.h (working copy)
@@ -497,6 +497,8 @@ public:
and their visibility needs to be copied from their "masters" at
the end of parsing. */
unsigned cpp_implicit_alias : 1;
+ /* The alias is a symbol version. */
+ unsigned symver : 1;
/* Set once the definition was analyzed. The list of references and
other properties are built during analysis. */
unsigned analyzed : 1;
Index: cgraphunit.c
===================================================================
--- cgraphunit.c (revision 278293)
+++ cgraphunit.c (working copy)
@@ -711,6 +711,89 @@ symbol_table::process_same_body_aliases
cpp_implicit_aliases_done = true;
}
+/* Process a symver attribute. */
+
+static void
+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)))
+ {
+ 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);
+
+ 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 can't be versioned");
The "can't" should trigger -Wformat-diag warnings suggesting to use
"cannot" instead. There are a few remaining warnings like that we
haven't cleaned up yet but I'm hoping to get to it at some point and
make the warning an error.
+ return;
+ }
+ if (DECL_COMDAT (n->decl))
+ {
+ error_at (DECL_SOURCE_LOCATION (n->decl),
+ "comdat symbol can't be versioned");
+ return;
+ }
+ if (n->weakref)
+ {
+ error_at (DECL_SOURCE_LOCATION (n->decl),
+ "weakref can't be versioned");
Same here. If weakref refers to attribute weakref it should also
be quoted (-Wformat-diag should suggest that; if it doesn't I'll
enhance it).
+ 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);
+
+ 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. */
static void
@@ -730,6 +813,7 @@ process_common_attributes (symtab_node *
if (lookup_attribute ("no_reorder", DECL_ATTRIBUTES (decl)))
node->no_reorder = 1;
+ process_symver_attribute (node);
}
/* Look for externally_visible and used attributes and mark cgraph nodes
@@ -2137,8 +2221,12 @@ cgraph_node::assemble_thunks_and_aliases
/* Force assemble_alias to really output the alias this time instead
of buffering it in same alias pairs. */
TREE_ASM_WRITTEN (decl) = 1;
- do_assemble_alias (alias->decl,
- DECL_ASSEMBLER_NAME (decl));
+ if (alias->symver)
+ do_assemble_symver (alias->decl,
+ DECL_ASSEMBLER_NAME (decl));
+ else
+ do_assemble_alias (alias->decl,
+ DECL_ASSEMBLER_NAME (decl));
alias->assemble_thunks_and_aliases ();
TREE_ASM_WRITTEN (decl) = saved_written;
}
Index: config/elfos.h
===================================================================
--- config/elfos.h (revision 278293)
+++ config/elfos.h (working copy)
@@ -248,6 +248,17 @@ see the files COPYING3 and COPYING.RUNTI
} \
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)); \
+ } \
+ while (0)
+
/* The following macro defines the format used to output the second
operand of the .type assembler directive. Different svr4 assemblers
expect various different forms for this operand. The one given here
Index: doc/extend.texi
===================================================================
--- doc/extend.texi (revision 278293)
+++ doc/extend.texi (working copy)
@@ -3640,6 +3640,34 @@ Function Attributes}, @ref{PowerPC Funct
@ref{Nios II Function Attributes}, and @ref{S/390 Function Attributes}
for details.
+@item symver ("@var{name2}@@@var{nodename}")
+On ELF targets this attribute creates symbol version.
Just a few minor spelling nits here that jumped out at me about
missing articles:
"creates <ins>a </ins>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 of the alias 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.
Just for clarity, the text refers to "the alias" without having ever
mentioned an alias. What alias is it talking about? If a symbol
version is a kind of an alias the text should probably introduce
that notion before referring to it.
Missing article:
"building <ins>a </ins> shared library."
and
"Versioned symbol<ins>s</ins>..."
+
+@smallexample
+__attribute__ ((__symver__ ("foo@@VERS_1"))) int
+foo_v1 (void)
+@{
+@}
+@end smallexample
+
+Will produce @code{.symver foo_v1, foo@@VERS_1} directive in the assembler
"produce <ins>a </ins>@code{.symver...} directive..."
+output. It is not allowed to make multiple versions out of one symbol, however
+versioned symbol may also be an alias.
I'm not sure I understand what the above restriction means. That
"it's an error to define multiple versions of the same symbol?"
(That doesn't make sense to me if this is all about making it
possible to do just that so I must be misunderstanding it. If
that's the problem it may be worth clarifying a bit.)
+
+@smallexample
+__attribute__ ((__symver__ ("foo@VERS_2")))
+__attribute__ ((alias ("foo_v1")))
+int symver_foo_v1 (void);
+@end smallexample
+
+This example creates alias an of @code{foo_v1} with symbol name
"crates <ins>an </ins>alias."
+@code{symver_foo_v1} which will be version @code{VERS_2} of @code{foo}.
+
@item target_clones (@var{options})
@cindex @code{target_clones} function attribute
The @code{target_clones} attribute is used to specify that a function
Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c (revision 278293)
+++ lto-cgraph.c (working copy)
@@ -526,6 +526,7 @@ lto_output_node (struct lto_simple_outpu
bp_pack_value (&bp, node->alias, 1);
bp_pack_value (&bp, node->transparent_alias, 1);
bp_pack_value (&bp, node->weakref, 1);
+ bp_pack_value (&bp, node->symver, 1);
bp_pack_value (&bp, node->frequency, 2);
bp_pack_value (&bp, node->only_called_at_startup, 1);
bp_pack_value (&bp, node->only_called_at_exit, 1);
@@ -606,6 +607,7 @@ lto_output_varpool_node (struct lto_simp
bp_pack_value (&bp, node->alias, 1);
bp_pack_value (&bp, node->transparent_alias, 1);
bp_pack_value (&bp, node->weakref, 1);
+ bp_pack_value (&bp, node->symver, 1);
bp_pack_value (&bp, node->analyzed && (!boundary_p || node->alias), 1);
gcc_assert (node->definition || !node->analyzed);
/* Constant pool initializers can be de-unified into individual ltrans
units.
@@ -1170,6 +1172,7 @@ input_overwrite_node (struct lto_file_de
node->alias = bp_unpack_value (bp, 1);
node->transparent_alias = bp_unpack_value (bp, 1);
node->weakref = bp_unpack_value (bp, 1);
+ node->symver = bp_unpack_value (bp, 1);
node->frequency = (enum node_frequency)bp_unpack_value (bp, 2);
node->only_called_at_startup = bp_unpack_value (bp, 1);
node->only_called_at_exit = bp_unpack_value (bp, 1);
@@ -1363,6 +1366,7 @@ input_varpool_node (struct lto_file_decl
node->alias = bp_unpack_value (&bp, 1);
node->transparent_alias = bp_unpack_value (&bp, 1);
node->weakref = bp_unpack_value (&bp, 1);
+ node->symver = bp_unpack_value (&bp, 1);
node->analyzed = bp_unpack_value (&bp, 1);
node->used_from_other_partition = bp_unpack_value (&bp, 1);
node->in_other_partition = bp_unpack_value (&bp, 1);
Index: output.h
===================================================================
--- output.h (revision 278293)
+++ output.h (working copy)
@@ -167,6 +167,7 @@ extern int decode_reg_name (const char *
extern int decode_reg_name_and_count (const char *, int *);
extern void do_assemble_alias (tree, tree);
+extern void do_assemble_symver (tree, tree);
extern void default_assemble_visibility (tree, int);
Index: symtab.c
===================================================================
--- symtab.c (revision 278293)
+++ symtab.c (working copy)
@@ -848,6 +848,8 @@ symtab_node::dump_base (FILE *f)
fprintf (f, " transparent_alias");
if (weakref)
fprintf (f, " weakref");
+ if (symver)
+ fprintf (f, " symver");
if (cpp_implicit_alias)
fprintf (f, " cpp_implicit_alias");
if (alias_target)
@@ -1145,6 +1147,11 @@ symtab_node::verify_base (void)
error ("node is transparent_alias but not an alias");
error_found = true;
}
+ if (symver && !alias)
+ {
+ error ("node is symver but not alias");
symver should be quoted: %<symver%>.
Martin
