RFC: PATCH to add abi_tag attribute

RFC: PATCH to add abi_tag attribute

[Jason Merrill]

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

As discussed at the Cauldron in Prague, this patch introduces a C++ 
abi_tag attribute which can be attached to a function or class to modify 
its mangled name and avoid name collisions with earlier versions with a 
different ABI.  It also adds a -Wabi-tag warning option to make the 
compiler suggest adding ABI tags to classes with subobjects that have tags.

Any comments?

[-- Attachment #2: abi-tag.patch --]
[-- Type: text/x-patch, Size: 18894 bytes --]

commit 3c6c2650782f6ab21701c32a16fe787dd00d0e01
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Nov 5 22:44:20 2012 -0500

    	Add C++ attribute abi_tag and -Wabi-tag option.
    gcc/
    	* attribs.c (lookup_attribute_spec): Handle getting a TREE_LIST.
    gcc/c-family/
    	* c.opt (Wabi-tag): New.
    gcc/cp/
    	* tree.c (cxx_attribute_table): Add abi_tag attribute.
    	(check_abi_tag_redeclaration, handle_abi_tag_attribute): New.
    	* class.c (find_abi_tags_r, check_abi_tags): New.
    	(check_bases, check_field_decl): Call check_abi_tags.
    	* decl.c (redeclaration_error_message): Call
    	check_abi_tag_redeclaration.
    	* mangle.c (tree_string_cmp, write_abi_tags): New.
    	(write_unqualified_name): Call write_abi_tags.
    include/
    	* demangle.h (enum demangle_component_type): Add
    	DEMANGLE_COMPONENT_TAGGED_NAME.
    libiberty/
    	* cp-demangle.c (d_dump): Handle DEMANGLE_COMPONENT_TAGGED_NAME.
    	(d_make_comp, d_find_pack, d_print_comp): Likewise.
    	(d_abi_tags): New.
    	(d_name): Call it.

diff --git a/gcc/attribs.c b/gcc/attribs.c
index d167c1f..0425de9 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -325,12 +325,21 @@ lookup_scoped_attribute_spec (const_tree ns, const_tree name)
 			 substring_hash (attr.str, attr.length));
 }
 
-/* Return the spec for the attribute named NAME.  */
+/* Return the spec for the attribute named NAME.  If NAME is a TREE_LIST,
+   it also specifies the attribute namespace.  */
 
 const struct attribute_spec *
 lookup_attribute_spec (const_tree name)
 {
-  return lookup_scoped_attribute_spec (get_identifier ("gnu"), name);
+  tree ns;
+  if (TREE_CODE (name) == TREE_LIST)
+    {
+      ns = TREE_PURPOSE (name);
+      name = TREE_VALUE (name);
+    }
+  else
+    ns = get_identifier ("gnu");
+  return lookup_scoped_attribute_spec (ns, name);
 }
 
 \f
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 06d6e36..01d59f1 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -257,6 +257,10 @@ Wabi
 C ObjC C++ ObjC++ LTO Var(warn_abi) Warning
 Warn about things that will change when compiling with an ABI-compliant compiler
 
+Wabi-tag
+C++ ObjC++ Var(warn_abi_tag) Warning
+Warn if a subobject has an abi_tag attribute that the complete object type does not have
+
 Wpsabi
 C ObjC C++ ObjC++ LTO Var(warn_psabi) Init(1) Undocumented
 
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e55f1f9..ae16000 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1273,6 +1273,89 @@ handle_using_decl (tree using_decl, tree t)
     alter_access (t, decl, access);
 }
 \f
+/* walk_tree callback for check_abi_tags: if the type at *TP involves any
+   types with abi tags, add the corresponding identifiers to the VEC in
+   *DATA and set IDENTIFIER_MARKED.  */
+
+struct abi_tag_data
+{
+  tree t;
+  tree subob;
+};
+
+static tree
+find_abi_tags_r (tree *tp, int */*walk_subtrees*/, void *data)
+{
+  if (!TAGGED_TYPE_P (*tp))
+    return NULL_TREE;
+
+  if (tree attributes = lookup_attribute ("abi_tag", TYPE_ATTRIBUTES (*tp)))
+    {
+      struct abi_tag_data *p = static_cast<struct abi_tag_data*>(data);
+      for (tree list = TREE_VALUE (attributes); list;
+	   list = TREE_CHAIN (list))
+	{
+	  tree tag = TREE_VALUE (list);
+	  tree id = get_identifier (TREE_STRING_POINTER (tag));
+	  if (!IDENTIFIER_MARKED (id))
+	    {
+	      if (TYPE_P (p->subob))
+		{
+		  warning (OPT_Wabi_tag, "%qT does not have the %E abi tag "
+			   "that base %qT has", p->t, tag, p->subob);
+		  inform (location_of (p->subob), "%qT declared here",
+			  p->subob);
+		}
+	      else
+		{
+		  warning (OPT_Wabi_tag, "%qT does not have the %E abi tag "
+			   "that %qT (used in the type of %qD) has",
+			   p->t, tag, *tp, p->subob);
+		  inform (location_of (p->subob), "%qD declared here",
+			  p->subob);
+		  inform (location_of (*tp), "%qT declared here", *tp);
+		}
+	    }
+	}
+    }
+  return NULL_TREE;
+}
+
+/* Check that class T has all the abi tags that subobject SUBOB has, or
+   warn if not.  */
+
+static void
+check_abi_tags (tree t, tree subob)
+{
+  tree attributes = lookup_attribute ("abi_tag", TYPE_ATTRIBUTES (t));
+  if (attributes)
+    {
+      for (tree list = TREE_VALUE (attributes); list;
+	   list = TREE_CHAIN (list))
+	{
+	  tree tag = TREE_VALUE (list);
+	  tree id = get_identifier (TREE_STRING_POINTER (tag));
+	  IDENTIFIER_MARKED (id) = true;
+	}
+    }
+
+  tree subtype = TYPE_P (subob) ? subob : TREE_TYPE (subob);
+  struct abi_tag_data data = { t, subob };
+
+  cp_walk_tree_without_duplicates (&subtype, find_abi_tags_r, &data);
+
+  if (attributes)
+    {
+      for (tree list = TREE_VALUE (attributes); list;
+	   list = TREE_CHAIN (list))
+	{
+	  tree tag = TREE_VALUE (list);
+	  tree id = get_identifier (TREE_STRING_POINTER (tag));
+	  IDENTIFIER_MARKED (id) = false;
+	}
+    }
+}
+
 /* Run through the base classes of T, updating CANT_HAVE_CONST_CTOR_P,
    and NO_CONST_ASN_REF_P.  Also set flag bits in T based on
    properties of the bases.  */
@@ -1402,6 +1485,8 @@ check_bases (tree t,
 	  if (tm_attr)
 	    seen_tm_mask |= tm_attr_to_mask (tm_attr);
 	}
+
+      check_abi_tags (t, basetype);
     }
 
   /* If one of the base classes had TM attributes, and the current class
@@ -3118,6 +3203,9 @@ check_field_decl (tree field,
 	  && !TYPE_HAS_CONST_COPY_ASSIGN (type))
 	*no_const_asn_ref = 1;
     }
+
+  check_abi_tags (t, field);
+
   if (DECL_INITIAL (field) != NULL_TREE)
     {
       /* `build_class_init_list' does not recognize
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index d25aa80..e37f0c3 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2436,6 +2436,10 @@ redeclaration_error_message (tree newdecl, tree olddecl)
 	    }
 	}
 
+      check_abi_tag_redeclaration
+	(olddecl, lookup_attribute ("abi_tag", DECL_ATTRIBUTES (olddecl)),
+	 lookup_attribute ("abi_tag", DECL_ATTRIBUTES (newdecl)));
+
       return NULL;
     }
   else if (TREE_CODE (newdecl) == TEMPLATE_DECL)
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index f448932..54a4c9c 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -173,6 +173,7 @@ static void mangle_call_offset (const tree, const tree);
 static void write_mangled_name (const tree, bool);
 static void write_encoding (const tree);
 static void write_name (tree, const int);
+static void write_abi_tags (tree);
 static void write_unscoped_name (const tree);
 static void write_unscoped_template_name (const tree);
 static void write_nested_name (const tree);
@@ -1192,15 +1193,17 @@ write_unqualified_name (const tree decl)
       return;
     }
 
+  bool found = false;
+
   if (DECL_NAME (decl) == NULL_TREE)
     {
+      found = true;
       gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
       write_source_name (DECL_ASSEMBLER_NAME (decl));
-      return;
     }
   else if (DECL_DECLARES_FUNCTION_P (decl))
     {
-      bool found = true;
+      found = true;
       if (DECL_CONSTRUCTOR_P (decl))
 	write_special_name_constructor (decl);
       else if (DECL_DESTRUCTOR_P (decl))
@@ -1234,14 +1237,13 @@ write_unqualified_name (const tree decl)
 	write_literal_operator_name (DECL_NAME (decl));
       else
 	found = false;
-
-      if (found)
-	return;
     }
 
-  if (VAR_OR_FUNCTION_DECL_P (decl) && ! TREE_PUBLIC (decl)
-      && DECL_NAMESPACE_SCOPE_P (decl)
-      && decl_linkage (decl) == lk_internal)
+  if (found)
+    /* OK */;
+  else if (VAR_OR_FUNCTION_DECL_P (decl) && ! TREE_PUBLIC (decl)
+	   && DECL_NAMESPACE_SCOPE_P (decl)
+	   && decl_linkage (decl) == lk_internal)
     {
       MANGLE_TRACE_TREE ("local-source-name", decl);
       write_char ('L');
@@ -1262,6 +1264,11 @@ write_unqualified_name (const tree decl)
       else
         write_source_name (DECL_NAME (decl));
     }
+
+  tree attrs = (TREE_CODE (decl) == TYPE_DECL
+		? TYPE_ATTRIBUTES (TREE_TYPE (decl))
+		: DECL_ATTRIBUTES (decl));
+  write_abi_tags (lookup_attribute ("abi_tag", attrs));
 }
 
 /* Write the unqualified-name for a conversion operator to TYPE.  */
@@ -1291,6 +1298,51 @@ write_source_name (tree identifier)
   write_identifier (IDENTIFIER_POINTER (identifier));
 }
 
+/* Compare two TREE_STRINGs like strcmp.  */
+
+int
+tree_string_cmp (const void *p1, const void *p2)
+{
+  if (p1 == p2)
+    return 0;
+  tree s1 = *(const tree*)p1;
+  tree s2 = *(const tree*)p2;
+  return strcmp (TREE_STRING_POINTER (s1),
+		 TREE_STRING_POINTER (s2));
+}
+
+/* ID is the name of a function or type with abi_tags attribute TAGS.
+   Write out the name, suitably decorated.  */
+
+static void
+write_abi_tags (tree tags)
+{
+  if (tags == NULL_TREE)
+    return;
+
+  tags = TREE_VALUE (tags);
+
+  VEC(tree,gc)* vec = make_tree_vector();
+
+  for (tree t = tags; t; t = TREE_CHAIN (t))
+    {
+      tree str = TREE_VALUE (t);
+      VEC_safe_push (tree, gc, vec, str);
+    }
+
+  VEC_qsort (tree, vec, tree_string_cmp);
+
+  unsigned i; tree str;
+  FOR_EACH_VEC_ELT (tree, vec, i, str)
+    {
+      write_string ("B");
+      write_unsigned_number (TREE_STRING_LENGTH (str) - 1);
+      write_identifier (TREE_STRING_POINTER (str));
+    }
+
+  release_tree_vector (vec);
+}
+
 /* Write a user-defined literal operator.
           ::= li <source-name>    # "" <source-name>
    IDENTIFIER is an LITERAL_IDENTIFIER_NODE.  */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index bcc2779..5df7b6c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -49,6 +49,7 @@ static tree build_local_temp (tree);
 static tree handle_java_interface_attribute (tree *, tree, tree, int, bool *);
 static tree handle_com_interface_attribute (tree *, tree, tree, int, bool *);
 static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *);
+static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *);
 
 /* If REF is an lvalue, returns the kind of lvalue that REF is.
    Otherwise, returns clk_none.  */
@@ -3000,6 +3001,8 @@ const struct attribute_spec cxx_attribute_table[] =
     handle_com_interface_attribute, false },
   { "init_priority",  1, 1, true,  false, false,
     handle_init_priority_attribute, false },
+  { "abi_tag", 1, -1, false, false, false,
+    handle_abi_tag_attribute, true },
   { NULL,	      0, 0, false, false, false, NULL, false }
 };
 
@@ -3131,6 +3134,96 @@ handle_init_priority_attribute (tree* node,
     }
 }
 
+/* DECL is being redeclared; the old declaration had the abi tags in OLD,
+   and the new one has the tags in NEW_.  Give an error if there are tags
+   in NEW_ that weren't in OLD.  */
+
+bool
+check_abi_tag_redeclaration (const_tree decl, const_tree old, const_tree new_)
+{
+  if (old && TREE_CODE (TREE_VALUE (old)) == TREE_LIST)
+    old = TREE_VALUE (old);
+  if (new_ && TREE_CODE (TREE_VALUE (new_)) == TREE_LIST)
+    new_ = TREE_VALUE (new_);
+  bool err = false;
+  for (const_tree t = new_; t; t = TREE_CHAIN (t))
+    {
+      tree str = TREE_VALUE (t);
+      for (const_tree in = old; in; in = TREE_CHAIN (in))
+	{
+	  tree ostr = TREE_VALUE (in);
+	  if (cp_tree_equal (str, ostr))
+	    goto found;
+	}
+      error ("redeclaration of %qD adds abi tag %E", decl, str);
+      err = true;
+    found:;
+    }
+  if (err)
+    {
+      inform (DECL_SOURCE_LOCATION (decl), "previous declaration here");
+      return false;
+    }
+  return true;
+}
+
+/* Handle an "abi_tag" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_abi_tag_attribute (tree* node, tree name, tree args,
+			  int flags, bool* no_add_attrs)
+{
+  if (TYPE_P (*node))
+    {
+      if (!TAGGED_TYPE_P (*node))
+	{
+	  error ("%qE attribute applied to non-class, non-enum type %qT",
+		 name, *node);
+	  goto fail;
+	}
+      else if (!(flags & (int)ATTR_FLAG_TYPE_IN_PLACE))
+	{
+	  error ("%qE attribute applied to %qT after its definition",
+		 name, *node);
+	  goto fail;
+	}
+
+      tree attributes = TYPE_ATTRIBUTES (*node);
+      tree decl = TYPE_NAME (*node);
+
+      /* Make sure all declarations have the same abi tags.  */
+      if (DECL_SOURCE_LOCATION (decl) != input_location)
+	{
+	  if (!check_abi_tag_redeclaration (decl,
+					    lookup_attribute ("abi_tag",
+							      attributes),
+					    args))
+	    goto fail;
+	}
+    }
+  else
+    {
+      if (TREE_CODE (*node) != FUNCTION_DECL)
+	{
+	  error ("%qE attribute applied to non-function %qD", name, *node);
+	  goto fail;
+	}
+      else if (DECL_LANGUAGE (*node) == lang_c)
+	{
+	  error ("%qE attribute applied to extern \"C\" function %qD",
+		 name, *node);
+	  goto fail;
+	}
+    }
+
+  return NULL_TREE;
+
+ fail:
+  *no_add_attrs = true;
+  return NULL_TREE;
+}
+
 /* Return a new PTRMEM_CST of the indicated TYPE.  The MEMBER is the
    thing pointed to by the constant.  */
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 54fd548..fe09b85 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -15598,6 +15598,27 @@ You must specify @option{-Wno-pmf-conversions} to use this extension.
 Some attributes only make sense for C++ programs.
 
 @table @code
+@item abi_tag ("@var{tag}", ...)
+@cindex @code{abi_tag} attribute
+The @code{abi_tag} attribute can be applied to a function or class
+declaration.  It modifies the mangled name of the function or class to
+incorporate the tag name, in order to distinguish the function or
+class from an earlier version with a different ABI; perhaps the class
+has changed size, or the function has a different return type that is
+not encoded in the mangled name.
+
+The argument can be a list of strings of arbitrary length.  The
+strings are sorted on output, so the order of the list is
+unimportant.
+
+A redeclaration of a function or class must not add new ABI tags,
+since doing so would change the mangled name.
+
+The @option{-Wabi-tag} flag enables a warning about a class which does
+not have all the ABI tags used by its subobjects; for users with code
+that needs to coexist with an earlier ABI, using this option can help
+to find all affected types that need to be tagged.
+
 @item init_priority (@var{priority})
 @cindex @code{init_priority} attribute
 
diff --git a/gcc/testsuite/g++.dg/abi/abi-tag1.C b/gcc/testsuite/g++.dg/abi/abi-tag1.C
new file mode 100644
index 0000000..942929c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/abi-tag1.C
@@ -0,0 +1,19 @@
+// { dg-options "-Wabi-tag" }
+
+// { dg-final { scan-assembler "_Z1fB3barB3fooi" } }
+void f(int) __attribute ((abi_tag ("foo","bar")));
+
+struct __attribute ((abi_tag ("bar"))) A { };
+
+struct B: A { };		// { dg-warning "bar. abi tag" }
+struct D { A* ap; };		// { dg-warning "bar. abi tag" }
+
+// { dg-final { scan-assembler "_Z1gB3baz1AB3bar" } }
+void g(A) __attribute ((abi_tag ("baz")));
+void g(A) __attribute ((abi_tag ("baz")));
+
+int main()
+{
+  f(42);
+  g(A());
+}
diff --git a/gcc/testsuite/g++.dg/abi/abi-tag2.C b/gcc/testsuite/g++.dg/abi/abi-tag2.C
new file mode 100644
index 0000000..1550055
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/abi-tag2.C
@@ -0,0 +1,6 @@
+void f(int);
+void f(int) __attribute ((abi_tag ("foo"))); // { dg-error "adds abi tag" }
+
+struct C;
+struct __attribute ((abi_tag ("foo"))) C; // { dg-error "adds abi tag" }
+
diff --git a/include/demangle.h b/include/demangle.h
index 5da79d8..ed01950 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -420,6 +420,8 @@ enum demangle_component_type
   DEMANGLE_COMPONENT_NONTRANSACTION_CLONE,
   /* A pack expansion.  */
   DEMANGLE_COMPONENT_PACK_EXPANSION,
+  /* A name with an ABI tag.  */
+  DEMANGLE_COMPONENT_TAGGED_NAME,
   /* A cloned function.  */
   DEMANGLE_COMPONENT_CLONE
 };
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 32df38c..86c7747 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -508,6 +508,11 @@ d_dump (struct demangle_component *dc, int indent)
     case DEMANGLE_COMPONENT_NAME:
       printf ("name '%.*s'\n", dc->u.s_name.len, dc->u.s_name.s);
       return;
+    case DEMANGLE_COMPONENT_TAGGED_NAME:
+      printf ("tagged name\n");
+      d_dump (dc->u.s_binary.left, indent + 2);
+      d_dump (dc->u.s_binary.right, indent + 2);
+      return;
     case DEMANGLE_COMPONENT_TEMPLATE_PARAM:
       printf ("template parameter %ld\n", dc->u.s_number.number);
       return;
@@ -809,6 +814,7 @@ d_make_comp (struct d_info *di, enum demangle_component_type type,
     case DEMANGLE_COMPONENT_QUAL_NAME:
     case DEMANGLE_COMPONENT_LOCAL_NAME:
     case DEMANGLE_COMPONENT_TYPED_NAME:
+    case DEMANGLE_COMPONENT_TAGGED_NAME:
     case DEMANGLE_COMPONENT_TEMPLATE:
     case DEMANGLE_COMPONENT_CONSTRUCTION_VTABLE:
     case DEMANGLE_COMPONENT_VENDOR_TYPE_QUAL:
@@ -1202,6 +1208,23 @@ d_encoding (struct d_info *di, int top_level)
     }
 }
 
+/* <tagged-name> ::= <name> B <source-name> */
+
+static struct demangle_component *
+d_abi_tags (struct d_info *di, struct demangle_component *dc)
+{
+  char peek;
+  while (peek = d_peek_char (di),
+	 peek == 'B')
+    {
+      struct demangle_component *tag;
+      d_advance (di, 1);
+      tag = d_source_name (di);
+      dc = d_make_comp (di, DEMANGLE_COMPONENT_TAGGED_NAME, dc, tag);
+    }
+  return dc;
+}
+
 /* <name> ::= <nested-name>
           ::= <unscoped-name>
           ::= <unscoped-template-name> <template-args>
@@ -1223,14 +1246,17 @@ d_name (struct d_info *di)
   switch (peek)
     {
     case 'N':
-      return d_nested_name (di);
+      dc = d_nested_name (di);
+      break;
 
     case 'Z':
-      return d_local_name (di);
+      dc = d_local_name (di);
+      break;
 
     case 'L':
     case 'U':
-      return d_unqualified_name (di);
+      dc = d_unqualified_name (di);
+      break;
 
     case 'S':
       {
@@ -1272,7 +1298,7 @@ d_name (struct d_info *di)
 			      d_template_args (di));
 	  }
 
-	return dc;
+	break;
       }
 
     default:
@@ -1287,8 +1313,12 @@ d_name (struct d_info *di)
 	  dc = d_make_comp (di, DEMANGLE_COMPONENT_TEMPLATE, dc,
 			    d_template_args (di));
 	}
-      return dc;
+      break;
     }
+
+  if (d_peek_char (di) == 'B')
+    dc = d_abi_tags (di, dc);
+  return dc;
 }
 
 /* <nested-name> ::= N [<CV-qualifiers>] <prefix> <unqualified-name> E
@@ -3745,6 +3775,7 @@ d_find_pack (struct d_print_info *dpi,
       
     case DEMANGLE_COMPONENT_LAMBDA:
     case DEMANGLE_COMPONENT_NAME:
+    case DEMANGLE_COMPONENT_TAGGED_NAME:
     case DEMANGLE_COMPONENT_OPERATOR:
     case DEMANGLE_COMPONENT_BUILTIN_TYPE:
     case DEMANGLE_COMPONENT_SUB_STD:
@@ -3830,6 +3861,13 @@ d_print_comp (struct d_print_info *dpi, int options,
 	d_print_java_identifier (dpi, dc->u.s_name.s, dc->u.s_name.len);
       return;
 
+    case DEMANGLE_COMPONENT_TAGGED_NAME:
+      d_print_comp (dpi, options, d_left (dc));
+      d_append_string (dpi, "[abi:");
+      d_print_comp (dpi, options, d_right (dc));
+      d_append_char (dpi, ']');
+      return;
+
     case DEMANGLE_COMPONENT_QUAL_NAME:
     case DEMANGLE_COMPONENT_LOCAL_NAME:
       d_print_comp (dpi, options, d_left (dc));

Re: RFC: PATCH to add abi_tag attribute

[Jakub Jelinek]

On Mon, Nov 05, 2012 at 11:03:37PM -0500, Jason Merrill wrote:
> As discussed at the Cauldron in Prague, this patch introduces a C++
> abi_tag attribute which can be attached to a function or class to
> modify its mangled name and avoid name collisions with earlier
> versions with a different ABI.  It also adds a -Wabi-tag warning
> option to make the compiler suggest adding ABI tags to classes with
> subobjects that have tags.

Couldn't there be auto-propagation at least for classes that aren't forward
declared first?

Also perhaps the documentation should perhaps reserve some names for the
implementation or uses compatible with that (say starting with underscore
or whatever), so that we could in libstdc++ use abi tag names without
a fear that it is used already by others for something else.

	Jakub

Re: RFC: PATCH to add abi_tag attribute

[Jason Merrill]

On 11/06/2012 06:20 AM, Jakub Jelinek wrote:
> On Mon, Nov 05, 2012 at 11:03:37PM -0500, Jason Merrill wrote:
>> As discussed at the Cauldron in Prague, this patch introduces a C++
>> abi_tag attribute which can be attached to a function or class to
>> modify its mangled name and avoid name collisions with earlier
>> versions with a different ABI.  It also adds a -Wabi-tag warning
>> option to make the compiler suggest adding ABI tags to classes with
>> subobjects that have tags.
>
> Couldn't there be auto-propagation at least for classes that aren't forward
> declared first?

There could, but then there would be a silent difference in behavior 
based on whether or not a class has a forward declaration.  It would 
also mean we would need to instantiate templates in more situations in 
order to collect tags.  I think this is the best solution, even though 
it isn't as comprehensive as we would like.

> Also perhaps the documentation should perhaps reserve some names for the
> implementation or uses compatible with that (say starting with underscore
> or whatever), so that we could in libstdc++ use abi tag names without
> a fear that it is used already by others for something else.

Sure, that makes sense.

Jason

Re: RFC: PATCH to add abi_tag attribute

[Jason Merrill]

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

The demangler change was handling the tags in the wrong place; I'm 
writing them out with unqualified names, so the demangler should expect 
them in the same place.

Tested x86_64-pc-linux-gnu, applied to trunk.


[-- Attachment #2: dem-tag.patch --]
[-- Type: text/x-patch, Size: 3256 bytes --]

commit 75eef303e5494f27a6d9bbef68aaf3200978a8f1
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Nov 10 13:10:24 2012 -0500

    	* cp-demangle.c (d_unqualified_name): Handle abi tags here.
    	(d_name): Not here.

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 86c7747..913d4bf 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -1246,17 +1246,14 @@ d_name (struct d_info *di)
   switch (peek)
     {
     case 'N':
-      dc = d_nested_name (di);
-      break;
+      return d_nested_name (di);
 
     case 'Z':
-      dc = d_local_name (di);
-      break;
+      return d_local_name (di);
 
     case 'L':
     case 'U':
-      dc = d_unqualified_name (di);
-      break;
+      return d_unqualified_name (di);
 
     case 'S':
       {
@@ -1298,7 +1295,7 @@ d_name (struct d_info *di)
 			      d_template_args (di));
 	  }
 
-	break;
+	return dc;
       }
 
     default:
@@ -1313,12 +1310,8 @@ d_name (struct d_info *di)
 	  dc = d_make_comp (di, DEMANGLE_COMPONENT_TEMPLATE, dc,
 			    d_template_args (di));
 	}
-      break;
+      return dc;
     }
-
-  if (d_peek_char (di) == 'B')
-    dc = d_abi_tags (di, dc);
-  return dc;
 }
 
 /* <nested-name> ::= N [<CV-qualifiers>] <prefix> <unqualified-name> E
@@ -1446,15 +1439,14 @@ d_prefix (struct d_info *di)
 static struct demangle_component *
 d_unqualified_name (struct d_info *di)
 {
+  struct demangle_component *ret;
   char peek;
 
   peek = d_peek_char (di);
   if (IS_DIGIT (peek))
-    return d_source_name (di);
+    ret = d_source_name (di);
   else if (IS_LOWER (peek))
     {
-      struct demangle_component *ret;
-
       ret = d_operator_name (di);
       if (ret != NULL && ret->type == DEMANGLE_COMPONENT_OPERATOR)
 	{
@@ -1463,14 +1455,11 @@ d_unqualified_name (struct d_info *di)
 	    ret = d_make_comp (di, DEMANGLE_COMPONENT_UNARY, ret,
 			       d_source_name (di));
 	}
-      return ret;
     }
   else if (peek == 'C' || peek == 'D')
-    return d_ctor_dtor_name (di);
+    ret = d_ctor_dtor_name (di);
   else if (peek == 'L')
     {
-      struct demangle_component * ret;
-
       d_advance (di, 1);
 
       ret = d_source_name (di);
@@ -1478,22 +1467,27 @@ d_unqualified_name (struct d_info *di)
 	return NULL;
       if (! d_discriminator (di))
 	return NULL;
-      return ret;
     }
   else if (peek == 'U')
     {
       switch (d_peek_next_char (di))
 	{
 	case 'l':
-	  return d_lambda (di);
+	  ret = d_lambda (di);
+	  break;
 	case 't':
-	  return d_unnamed_type (di);
+	  ret = d_unnamed_type (di);
+	  break;
 	default:
 	  return NULL;
 	}
     }
   else
     return NULL;
+
+  if (d_peek_char (di) == 'B')
+    ret = d_abi_tags (di, ret);
+  return ret;
 }
 
 /* <source-name> ::= <(positive length) number> <identifier>  */
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 6b55d30..5b41b03 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4084,6 +4084,9 @@ auto& f<int>(int const&, int)
 --format=gnu-v3
 _Z1gILi1EEvR1AIXT_EER1BIXscbT_EE
 void g<1>(A<1>&, B<static_cast<bool>(1)>&)
+--format=gnu-v3
+_ZNKSt7complexIiE4realB5cxx11Ev
+std::complex<int>::real[abi:cxx11]() const
 #
 # Ada (GNAT) tests.
 #

Re: RFC: PATCH to add abi_tag attribute

[Florian Weimer]

On 11/06/2012 05:03 AM, Jason Merrill wrote:
> +The @code{abi_tag} attribute can be applied to a function or class
> +declaration.  It modifies the mangled name of the function or class to
> +incorporate the tag name, in order to distinguish the function or
> +class from an earlier version with a different ABI; perhaps the class
> +has changed size, or the function has a different return type that is
> +not encoded in the mangled name.

> +A redeclaration of a function or class must not add new ABI tags,
> +since doing so would change the mangled name.

I'm not sure if this sufficiently far-reaching.  It seems that this 
doesn't allow me to implement a virtual function which takes a 
std::string parameter in new-ABI-mode when the base class is also used 
in old-ABI-mode.

I really see no way to deal with this with either old-ABI-by-default 
(without explicit annotations), annotations on legacy headers, or some 
form of automated cross-translation-unit feedback.

-- 
Florian Weimer / Red Hat Product Security Team

Re: RFC: PATCH to add abi_tag attribute

[Jason Merrill]

On 11/11/2012 08:01 AM, Florian Weimer wrote:
> I'm not sure if this sufficiently far-reaching.  It seems that this
> doesn't allow me to implement a virtual function which takes a
> std::string parameter in new-ABI-mode when the base class is also used
> in old-ABI-mode.

Ah, yes; since virtual functions are called through the vtable the 
signatures of virtual functions are also part of the class ABI, so the 
base class also needs to be tagged.  I guess I should add that to -Wabi-tag.

Jason

Re: RFC: PATCH to add abi_tag attribute

[Jason Merrill]

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

On 11/11/2012 11:58 PM, Jason Merrill wrote:
> On 11/11/2012 08:01 AM, Florian Weimer wrote:
>> I'm not sure if this sufficiently far-reaching.  It seems that this
>> doesn't allow me to implement a virtual function which takes a
>> std::string parameter in new-ABI-mode when the base class is also used
>> in old-ABI-mode.
>
> Ah, yes; since virtual functions are called through the vtable the
> signatures of virtual functions are also part of the class ABI, so the
> base class also needs to be tagged.  I guess I should add that to
> -Wabi-tag.

Like so:

Tested x86_64-pc-linux-gnu, applying to trunk.



[-- Attachment #2: virt-tag.patch --]
[-- Type: text/x-patch, Size: 1843 bytes --]

commit 0c1d53def870aef5f785278eb447fceb2974df09
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Nov 13 12:23:52 2012 -0500

    	* class.c (finish_struct_1): Check virtual functions
    	for missing ABI tags.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 0665e90..cdc02ae 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6275,6 +6275,12 @@ finish_struct_1 (tree t)
 	/* Here we know enough to change the type of our virtual
 	   function table, but we will wait until later this function.  */
 	build_primary_vtable (CLASSTYPE_PRIMARY_BINFO (t), t);
+
+      /* If we're warning about ABI tags, check the types of the new
+	 virtual functions.  */
+      if (warn_abi_tag)
+	for (tree v = virtuals; v; v = TREE_CHAIN (v))
+	  check_abi_tags (t, TREE_VALUE (v));
     }
 
   if (TYPE_CONTAINS_VPTR_P (t))
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 25dd685..43b21c6 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -15622,7 +15622,7 @@ A redeclaration of a function or class must not add new ABI tags,
 since doing so would change the mangled name.
 
 The @option{-Wabi-tag} flag enables a warning about a class which does
-not have all the ABI tags used by its subobjects; for users with code
+not have all the ABI tags used by its subobjects and virtual functions; for users with code
 that needs to coexist with an earlier ABI, using this option can help
 to find all affected types that need to be tagged.
 
diff --git a/gcc/testsuite/g++.dg/abi/abi-tag4.C b/gcc/testsuite/g++.dg/abi/abi-tag4.C
new file mode 100644
index 0000000..3f8d7bf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/abi-tag4.C
@@ -0,0 +1,8 @@
+// { dg-options "-Wabi-tag" }
+
+struct __attribute ((abi_tag ("X"))) A { };
+
+struct B			// { dg-warning "abi tag" }
+{
+  virtual void f(A);		// { dg-message "declared here" }
+};

Re: RFC: PATCH to add abi_tag attribute

[Florian Weimer]

On 11/15/2012 02:51 AM, Jason Merrill wrote:
> On 11/11/2012 11:58 PM, Jason Merrill wrote:
>> On 11/11/2012 08:01 AM, Florian Weimer wrote:
>>> I'm not sure if this sufficiently far-reaching.  It seems that this
>>> doesn't allow me to implement a virtual function which takes a
>>> std::string parameter in new-ABI-mode when the base class is also used
>>> in old-ABI-mode.
>>
>> Ah, yes; since virtual functions are called through the vtable the
>> signatures of virtual functions are also part of the class ABI, so the
>> base class also needs to be tagged.  I guess I should add that to
>> -Wabi-tag.
>
> Like so:

Okay, this might work in the sense that it flags the relevant cases. 
I'm still not convinced that this actually helps programmers that much 
because it pretty much separates the two worlds.  If this is the intend, 
surely there are simpler approaches (such as a command-line flag which 
changes the mangling for everything).

-- 
Florian Weimer / Red Hat Product Security Team

Re: RFC: PATCH to add abi_tag attribute

[Jason Merrill]

On 11/23/2012 04:58 AM, Florian Weimer wrote:
> Okay, this might work in the sense that it flags the relevant cases. I'm
> still not convinced that this actually helps programmers that much
> because it pretty much separates the two worlds.  If this is the intend,
> surely there are simpler approaches (such as a command-line flag which
> changes the mangling for everything).

It only separates affected code; the idea is that code unaffected by ABI 
changes should be able to continue to interoperate normally.

Jason

Re: RFC: PATCH to add abi_tag attribute

[Florian Weimer]

On 11/23/2012 03:24 PM, Jason Merrill wrote:
> On 11/23/2012 04:58 AM, Florian Weimer wrote:
>> Okay, this might work in the sense that it flags the relevant cases. I'm
>> still not convinced that this actually helps programmers that much
>> because it pretty much separates the two worlds.  If this is the intend,
>> surely there are simpler approaches (such as a command-line flag which
>> changes the mangling for everything).
>
> It only separates affected code; the idea is that code unaffected by ABI
> changes should be able to continue to interoperate normally.

What about code like this?

struct __attribute ((abi_tag ("foo"))) A { };

inline int foo()
{
   return sizeof(A);
}

Couldn't this lead to ODR violations?  Wouldn't we have to warn about 
this case as well?

-- 
Florian Weimer / Red Hat Product Security Team