diff mbox

[v2,RFC] Canonize names of attributes.

Message ID e3bb65b3-38f7-08d1-05ab-629fdd9affc3@suse.cz
State New
Headers show

Commit Message

Martin Liška July 13, 2017, 1:48 p.m. UTC
On 07/11/2017 05:52 PM, Jason Merrill wrote:
> On Tue, Jul 11, 2017 at 9:37 AM, Martin Liška <mliska@suse.cz> wrote:
>> On 07/03/2017 11:00 PM, Jason Merrill wrote:
>>> On Mon, Jul 3, 2017 at 5:52 AM, Martin Liška <mliska@suse.cz> wrote:
>>>> On 06/30/2017 09:34 PM, Jason Merrill wrote:
>>>>>
>>>>> On Fri, Jun 30, 2017 at 5:23 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>> This is v2 of the patch, where just names of attributes are
>>>>>> canonicalized.
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression
>>>>>> tests.
>>>>>
>>>>>
>>>>> What is the purpose of the new "strict" parameter to cmp_attribs* ?  I
>>>>> don't see any discussion of it.
>>>>
>>>>
>>>> It's needed for arguments of attribute names, like:
>>>>
>>>> /usr/include/stdio.h:391:62: internal compiler error: in cmp_attribs, at
>>>> tree.h:5523
>>>>        __THROWNL __attribute__ ((__format__ (__printf__, 3, 4)));
>>>>
>>>
>>> Mm.  Although we don't want to automatically canonicalize all
>>> identifier arguments to attributes in the parser, we could still do it
>>> for specific attributes, e.g. in handle_format_attribute or
>>> handle_mode_attribute.
>>
>> Yep, that was done in my previous version of the patch
>> (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00996.html).
>> Where only attribute that was preserved unchanged was 'cleanup':
>>
>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>> index 8f638785e0e..08b4db5e5bd 100644
>> --- a/gcc/cp/parser.c
>> +++ b/gcc/cp/parser.c
>> @@ -24765,7 +24765,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
>>                   tree tv;
>>                   if (arguments != NULL_TREE
>>                       && ((tv = TREE_VALUE (arguments)) != NULL_TREE)
>> -                     && TREE_CODE (tv) == IDENTIFIER_NODE)
>> +                     && TREE_CODE (tv) == IDENTIFIER_NODE
>> +                     && !id_equal (TREE_PURPOSE (attribute), "cleanup"))
>>                     TREE_VALUE (arguments) = canonize_attr_name (tv);
>>                   release_tree_vector (vec);
>>                 }
>>
>> Does it work for you to do it so?
> 
> This is canonicalizing arguments by default; I want the default to be
> not canonicalizing arguments.  I think we only want to canonicalize
> arguments for format and mode, and we can do that in their handle_*
> functions.

Yep, done that in v3. I decided to move couple of functions to attribs.h and
verified that it will not cause binary size increase of cc1 and cc1plus.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

> 
> Jason
>

Comments

Martin Liška July 27, 2017, 12:57 p.m. UTC | #1
PING^1

On 07/13/2017 03:48 PM, Martin Liška wrote:
> On 07/11/2017 05:52 PM, Jason Merrill wrote:
>> On Tue, Jul 11, 2017 at 9:37 AM, Martin Liška <mliska@suse.cz> wrote:
>>> On 07/03/2017 11:00 PM, Jason Merrill wrote:
>>>> On Mon, Jul 3, 2017 at 5:52 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>> On 06/30/2017 09:34 PM, Jason Merrill wrote:
>>>>>>
>>>>>> On Fri, Jun 30, 2017 at 5:23 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>>
>>>>>>> This is v2 of the patch, where just names of attributes are
>>>>>>> canonicalized.
>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression
>>>>>>> tests.
>>>>>>
>>>>>>
>>>>>> What is the purpose of the new "strict" parameter to cmp_attribs* ?  I
>>>>>> don't see any discussion of it.
>>>>>
>>>>>
>>>>> It's needed for arguments of attribute names, like:
>>>>>
>>>>> /usr/include/stdio.h:391:62: internal compiler error: in cmp_attribs, at
>>>>> tree.h:5523
>>>>>        __THROWNL __attribute__ ((__format__ (__printf__, 3, 4)));
>>>>>
>>>>
>>>> Mm.  Although we don't want to automatically canonicalize all
>>>> identifier arguments to attributes in the parser, we could still do it
>>>> for specific attributes, e.g. in handle_format_attribute or
>>>> handle_mode_attribute.
>>>
>>> Yep, that was done in my previous version of the patch
>>> (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00996.html).
>>> Where only attribute that was preserved unchanged was 'cleanup':
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 8f638785e0e..08b4db5e5bd 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -24765,7 +24765,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
>>>                   tree tv;
>>>                   if (arguments != NULL_TREE
>>>                       && ((tv = TREE_VALUE (arguments)) != NULL_TREE)
>>> -                     && TREE_CODE (tv) == IDENTIFIER_NODE)
>>> +                     && TREE_CODE (tv) == IDENTIFIER_NODE
>>> +                     && !id_equal (TREE_PURPOSE (attribute), "cleanup"))
>>>                     TREE_VALUE (arguments) = canonize_attr_name (tv);
>>>                   release_tree_vector (vec);
>>>                 }
>>>
>>> Does it work for you to do it so?
>>
>> This is canonicalizing arguments by default; I want the default to be
>> not canonicalizing arguments.  I think we only want to canonicalize
>> arguments for format and mode, and we can do that in their handle_*
>> functions.
> 
> Yep, done that in v3. I decided to move couple of functions to attribs.h and
> verified that it will not cause binary size increase of cc1 and cc1plus.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
>>
>> Jason
>>
>
Joseph Myers Aug. 2, 2017, 11:25 a.m. UTC | #2
On Thu, 13 Jul 2017, Martin Liška wrote:

> +/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
> +   so that we have a canonical form of attribute names.  */
> +
> +static inline tree
> +canonicalize_attr_name (tree attr_name)
> +{
> +  const size_t l = IDENTIFIER_LENGTH (attr_name);
> +  const char *s = IDENTIFIER_POINTER (attr_name);
> +
> +  if (l > 4 && s[0] == '_')
> +    {
> +      gcc_checking_assert (s[l - 2] == '_');
> +      return get_identifier_with_length (s + 2, l - 4);
> +    }
> +
> +  return attr_name;

For this to (a) be correct, (b) not trigger the assertion, there must be a 
precondition that attr_name either starts and ends with __, or does not 
start with _, or has length 4 or less.  I don't see anything in the 
callers to ensure this precondition holds, so that, for example, 
__attribute__ ((_foobar)) does not trigger the assertion, and 
__attribute__ ((_xformat__)) is not wrongly interpreted as a format 
attribute (and similarly for attribute arguments when those are 
canonicalized).

> +/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
> +   ATTR2_LEN.  */
> +
> +static inline bool
> +cmp_attribs (const char *attr1, size_t attr1_len,
> +	     const char *attr2, size_t attr2_len)
> +{
> +  gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
> +  gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');

Of course, once you only canonicalize attributes that both start and end 
with __, you have the possibility of a canonicalized attribute name that 
still starts with _, so can't assert on it here (unless this is only ever 
called with an attribute that is already known to be valid).  (And of 
course there are cases such as __attribute__((__)) that starts and ends 
with __ but is too short to canonicalize, etc., which arise even with the 
above canonicalize_attr_name.)
Jason Merrill Aug. 2, 2017, 7:42 p.m. UTC | #3
OK.

On Thu, Jul 13, 2017 at 9:48 AM, Martin Liška <mliska@suse.cz> wrote:
> On 07/11/2017 05:52 PM, Jason Merrill wrote:
>> On Tue, Jul 11, 2017 at 9:37 AM, Martin Liška <mliska@suse.cz> wrote:
>>> On 07/03/2017 11:00 PM, Jason Merrill wrote:
>>>> On Mon, Jul 3, 2017 at 5:52 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>> On 06/30/2017 09:34 PM, Jason Merrill wrote:
>>>>>>
>>>>>> On Fri, Jun 30, 2017 at 5:23 AM, Martin Liška <mliska@suse.cz> wrote:
>>>>>>>
>>>>>>> This is v2 of the patch, where just names of attributes are
>>>>>>> canonicalized.
>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression
>>>>>>> tests.
>>>>>>
>>>>>>
>>>>>> What is the purpose of the new "strict" parameter to cmp_attribs* ?  I
>>>>>> don't see any discussion of it.
>>>>>
>>>>>
>>>>> It's needed for arguments of attribute names, like:
>>>>>
>>>>> /usr/include/stdio.h:391:62: internal compiler error: in cmp_attribs, at
>>>>> tree.h:5523
>>>>>        __THROWNL __attribute__ ((__format__ (__printf__, 3, 4)));
>>>>>
>>>>
>>>> Mm.  Although we don't want to automatically canonicalize all
>>>> identifier arguments to attributes in the parser, we could still do it
>>>> for specific attributes, e.g. in handle_format_attribute or
>>>> handle_mode_attribute.
>>>
>>> Yep, that was done in my previous version of the patch
>>> (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00996.html).
>>> Where only attribute that was preserved unchanged was 'cleanup':
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 8f638785e0e..08b4db5e5bd 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -24765,7 +24765,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
>>>                   tree tv;
>>>                   if (arguments != NULL_TREE
>>>                       && ((tv = TREE_VALUE (arguments)) != NULL_TREE)
>>> -                     && TREE_CODE (tv) == IDENTIFIER_NODE)
>>> +                     && TREE_CODE (tv) == IDENTIFIER_NODE
>>> +                     && !id_equal (TREE_PURPOSE (attribute), "cleanup"))
>>>                     TREE_VALUE (arguments) = canonize_attr_name (tv);
>>>                   release_tree_vector (vec);
>>>                 }
>>>
>>> Does it work for you to do it so?
>>
>> This is canonicalizing arguments by default; I want the default to be
>> not canonicalizing arguments.  I think we only want to canonicalize
>> arguments for format and mode, and we can do that in their handle_*
>> functions.
>
> Yep, done that in v3. I decided to move couple of functions to attribs.h and
> verified that it will not cause binary size increase of cc1 and cc1plus.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
> Martin
>
>>
>> Jason
>>
>
diff mbox

Patch

From bd7bf58da6f0688fa10d3dfc72328f9313d7439c Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 8 Jun 2017 10:23:25 +0200
Subject: [PATCH 1/2] Canonicalize names of attributes.

gcc/ChangeLog:

2017-07-12  Martin Liska  <mliska@suse.cz>

	* attribs.h (canonicalize_attr_name): New function.
	(cmp_attribs): Move from c-format.c and adjusted.
	(is_attribute_p): Moved from tree.h.
	* tree-inline.c: Add new includes.
	* tree.c (cmp_attrib_identifiers): Use cmp_attribs.
	(private_is_attribute_p): Remove.
	(private_lookup_attribute): Likewise.
	(private_lookup_attribute_by_prefix): Simplify.
	(remove_attribute): Use is_attribute_p.
	* tree.h: Remove removed declarations.

gcc/c-family/ChangeLog:

2017-07-12  Martin Liska  <mliska@suse.cz>

	* array-notation-common.c: Add new includes.
	* c-format.c( handle_format_attribute): Canonicalize a format
	function name.
	* c-lex.c (c_common_has_attribute): Canonicalize name of an
	attribute.
	* c-pretty-print.c: Add new include.

gcc/cp/ChangeLog:

2017-07-12  Martin Liska  <mliska@suse.cz>

	* parser.c (cp_parser_gnu_attribute_list): Canonicalize name of an
	attribute.
	(cp_parser_std_attribute): Likewise.
	* tree.c: Add new include.

gcc/c/ChangeLog:

2017-07-12  Martin Liska  <mliska@suse.cz>

	* c-parser.c (c_parser_attributes): Canonicalize name of an
	attribute.
gcc/go/ChangeLog:

2017-06-29  Martin Liska  <mliska@suse.cz>

	* go-gcc.cc (Gcc_backend::function): Look up for no_split_stack
	and not __no_split_stack__.

gcc/testsuite/ChangeLog:

2017-06-29  Martin Liska  <mliska@suse.cz>

	* g++.dg/cpp0x/pr65558.C: Update scanned pattern.
	* gcc.dg/parm-impl-decl-1.c: Likewise.
	* gcc.dg/parm-impl-decl-3.c: Likewise.
---
 gcc/attribs.h                           |  49 +++++++++++++++
 gcc/c-family/array-notation-common.c    |   2 +
 gcc/c-family/c-format.c                 |  24 ++------
 gcc/c-family/c-lex.c                    |   1 +
 gcc/c-family/c-pretty-print.c           |   1 +
 gcc/c/c-parser.c                        |   3 +
 gcc/cp/parser.c                         |   6 +-
 gcc/cp/tree.c                           |   1 +
 gcc/go/go-gcc.cc                        |   2 +-
 gcc/testsuite/g++.dg/cpp0x/pr65558.C    |   2 +-
 gcc/testsuite/gcc.dg/parm-impl-decl-1.c |   2 +-
 gcc/testsuite/gcc.dg/parm-impl-decl-3.c |   2 +-
 gcc/tree-inline.c                       |   3 +-
 gcc/tree.c                              | 104 +++++---------------------------
 gcc/tree.h                              |  23 +------
 15 files changed, 88 insertions(+), 137 deletions(-)

diff --git a/gcc/attribs.h b/gcc/attribs.h
index 7f13332700e..a6b1ac1ff35 100644
--- a/gcc/attribs.h
+++ b/gcc/attribs.h
@@ -47,4 +47,53 @@  extern char *make_unique_name (tree, const char *, bool);
 extern tree make_dispatcher_decl (const tree);
 extern bool is_function_default_version (const tree);
 
+/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
+   so that we have a canonical form of attribute names.  */
+
+static inline tree
+canonicalize_attr_name (tree attr_name)
+{
+  const size_t l = IDENTIFIER_LENGTH (attr_name);
+  const char *s = IDENTIFIER_POINTER (attr_name);
+
+  if (l > 4 && s[0] == '_')
+    {
+      gcc_checking_assert (s[l - 2] == '_');
+      return get_identifier_with_length (s + 2, l - 4);
+    }
+
+  return attr_name;
+}
+
+/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
+   ATTR2_LEN.  */
+
+static inline bool
+cmp_attribs (const char *attr1, size_t attr1_len,
+	     const char *attr2, size_t attr2_len)
+{
+  gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
+  gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');
+
+  return attr1_len == attr2_len && strncmp (attr1, attr2, attr1_len) == 0;
+}
+
+/* Compare attribute identifiers ATTR1 and ATTR2.  */
+
+static inline bool
+cmp_attribs (const char *attr1, const char *attr2)
+{
+  return cmp_attribs (attr1, strlen (attr1), attr2, strlen (attr2));
+}
+
+/* Given an identifier node IDENT and a string ATTR_NAME, return true
+   if the identifier node is a valid attribute name for the string.  */
+
+static inline bool
+is_attribute_p (const char *attr_name, const_tree ident)
+{
+  return cmp_attribs (attr_name, strlen (attr_name),
+		      IDENTIFIER_POINTER (ident), IDENTIFIER_LENGTH (ident));
+}
+
 #endif // GCC_ATTRIBS_H
diff --git a/gcc/c-family/array-notation-common.c b/gcc/c-family/array-notation-common.c
index 3b95332adad..a4809948e20 100644
--- a/gcc/c-family/array-notation-common.c
+++ b/gcc/c-family/array-notation-common.c
@@ -27,6 +27,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "options.h"
 #include "c-family/c-common.h"
 #include "tree-iterator.h"
+#include "stringpool.h"
+#include "attribs.h"
 
 /* Returns true if the function call in FNDECL is  __sec_implicit_index.  */
 
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 732339b9b5e..f6e65384b3f 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -33,6 +33,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "substring-locations.h"
 #include "selftest.h"
 #include "builtins.h"
+#include "attribs.h"
 
 /* Handle attributes associated with format checking.  */
 
@@ -67,7 +68,6 @@  static bool check_format_string (tree argument,
 static bool get_constant (tree expr, unsigned HOST_WIDE_INT *value,
 			  int validated_p);
 static const char *convert_format_name_to_system_name (const char *attr_name);
-static bool cmp_attribs (const char *tattr_name, const char *attr_name);
 
 static int first_target_format_type;
 static const char *format_name (int format_num);
@@ -3975,24 +3975,6 @@  convert_format_name_to_system_name (const char *attr_name)
   return attr_name;
 }
 
-/* Return true if TATTR_NAME and ATTR_NAME are the same format attribute,
-   counting "name" and "__name__" as the same, false otherwise.  */
-static bool
-cmp_attribs (const char *tattr_name, const char *attr_name)
-{
-  int alen = strlen (attr_name);
-  int slen = (tattr_name ? strlen (tattr_name) : 0);
-  if (alen > 4 && attr_name[0] == '_' && attr_name[1] == '_'
-      && attr_name[alen - 1] == '_' && attr_name[alen - 2] == '_')
-    {
-      attr_name += 2;
-      alen -= 4;
-    }
-  if (alen != slen || strncmp (tattr_name, attr_name, alen) != 0)
-    return false;
-  return true;
-}
-
 /* Handle a "format" attribute; arguments as in
    struct attribute_spec.handler.  */
 tree
@@ -4021,6 +4003,10 @@  handle_format_attribute (tree *node, tree ARG_UNUSED (name), tree args,
     }
 #endif
 
+  /* Canonicalize name of format function.  */
+  if (TREE_CODE (TREE_VALUE (args)) == IDENTIFIER_NODE)
+    TREE_VALUE (args) = canonicalize_attr_name (TREE_VALUE (args));
+
   if (!decode_format_attr (args, &info, 0))
     {
       *no_add_attrs = true;
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index e1c8bdff986..3765a800a57 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -316,6 +316,7 @@  c_common_has_attribute (cpp_reader *pfile)
     {
       attr_name = get_identifier ((const char *)
 				  cpp_token_as_text (pfile, token));
+      attr_name = canonicalize_attr_name (attr_name);
       if (c_dialect_cxx ())
 	{
 	  int idx = 0;
diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index fdb7b41f592..b8b8f665ef3 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -24,6 +24,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "c-pretty-print.h"
 #include "diagnostic.h"
 #include "stor-layout.h"
+#include "stringpool.h"
 #include "attribs.h"
 #include "intl.h"
 #include "tree-pretty-print.h"
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index f8fbc926172..e358b19a585 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4170,9 +4170,11 @@  c_parser_attributes (c_parser *parser)
 	  attr_name = c_parser_attribute_any_word (parser);
 	  if (attr_name == NULL)
 	    break;
+	  attr_name = canonicalize_attr_name (attr_name);
 	  if (is_cilkplus_vector_p (attr_name))
 	    {
 	      c_token *v_token = c_parser_peek_token (parser);
+	      v_token->value = canonicalize_attr_name (v_token->value);
 	      c_parser_cilk_simd_fn_vector_attrs (parser, *v_token);
 	      /* If the next token isn't a comma, we're done.  */
 	      if (!c_parser_next_token_is (parser, CPP_COMMA))
@@ -4236,6 +4238,7 @@  c_parser_attributes (c_parser *parser)
 		  release_tree_vector (expr_list);
 		}
 	    }
+
 	  attr = build_tree_list (attr_name, attr_args);
 	  if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
 	    c_parser_consume_token (parser);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c81b1a1f5c1..7e5900122f2 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -24777,7 +24777,8 @@  cp_parser_gnu_attribute_list (cp_parser* parser)
 	       parsed identifier.  */
 	    ? ridpointers[(int) token->keyword]
 	    : id_token->u.value;
-	  
+
+	  identifier = canonicalize_attr_name (identifier);
 	  attribute = build_tree_list (identifier, NULL_TREE);
 
 	  /* Peek at the next token.  */
@@ -24923,6 +24924,8 @@  cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
 		    "expected an identifier for the attribute name");
 	  return error_mark_node;
 	}
+
+      attr_id = canonicalize_attr_name (attr_id);
       attribute = build_tree_list (build_tree_list (attr_ns, attr_id),
 				   NULL_TREE);
       token = cp_lexer_peek_token (parser->lexer);
@@ -24932,6 +24935,7 @@  cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
 				 NULL_TREE);
   else
     {
+      attr_id = canonicalize_attr_name (attr_id);
       attribute = build_tree_list (build_tree_list (NULL_TREE, attr_id),
 				   NULL_TREE);
       /* C++11 noreturn attribute is equivalent to GNU's.  */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index c037b2c7440..900d2e13b18 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -32,6 +32,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "debug.h"
 #include "convert.h"
 #include "gimplify.h"
+#include "stringpool.h"
 #include "attribs.h"
 #include "flags.h"
 
diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
index a7977fe03c1..04912f0ed01 100644
--- a/gcc/go/go-gcc.cc
+++ b/gcc/go/go-gcc.cc
@@ -3046,7 +3046,7 @@  Gcc_backend::function(Btype* fntype, const std::string& name,
     DECL_UNINLINABLE(decl) = 1;
   if (disable_split_stack)
     {
-      tree attr = get_identifier("__no_split_stack__");
+      tree attr = get_identifier ("no_split_stack");
       DECL_ATTRIBUTES(decl) = tree_cons(attr, NULL_TREE, NULL_TREE);
     }
   if (in_unique_section)
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr65558.C b/gcc/testsuite/g++.dg/cpp0x/pr65558.C
index d294c95a657..12946b35eda 100644
--- a/gcc/testsuite/g++.dg/cpp0x/pr65558.C
+++ b/gcc/testsuite/g++.dg/cpp0x/pr65558.C
@@ -2,6 +2,6 @@ 
 // { dg-do compile { target c++11 } }
 
 inline namespace
-__attribute__((__abi_tag__)) // { dg-warning "ignoring .__abi_tag__. attribute on anonymous namespace" }
+__attribute__((__abi_tag__)) // { dg-warning "ignoring .abi_tag. attribute on anonymous namespace" }
 {
 }
diff --git a/gcc/testsuite/gcc.dg/parm-impl-decl-1.c b/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
index 5c7ddb0a259..c1219273c75 100644
--- a/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
+++ b/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
@@ -7,7 +7,7 @@ 
 /* Implicit function declaration in attribute in definition (testcase
    from bug).  */
 int
-foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'__mode__' attribute ignored" } */
+foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'mode' attribute ignored" } */
 {
   return (long long) i;
 }
diff --git a/gcc/testsuite/gcc.dg/parm-impl-decl-3.c b/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
index 904295258d7..20197b52402 100644
--- a/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
+++ b/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
@@ -4,7 +4,7 @@ 
 /* { dg-options "-g -std=gnu89" } */
 
 int
-foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'__mode__' attribute ignored" } */
+foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'mode' attribute ignored" } */
 {
   return (long long) i;
 }
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d4e4ef17247..affde64d2fd 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -57,7 +57,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "builtins.h"
 #include "tree-chkp.h"
-
+#include "stringpool.h"
+#include "attribs.h"
 
 /* I'm not real happy about this, but we need to handle gimple and
    non-gimple trees.  */
diff --git a/gcc/tree.c b/gcc/tree.c
index ca28afad0f2..13335a007b7 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -4928,9 +4928,8 @@  simple_cst_list_equal (const_tree l1, const_tree l2)
   return l1 == l2;
 }
 
-/* Compare two identifier nodes representing attributes.  Either one may
-   be in wrapped __ATTR__ form.  Return true if they are the same, false
-   otherwise.  */
+/* Compare two identifier nodes representing attributes.
+   Return true if they are the same, false otherwise.  */
 
 static bool
 cmp_attrib_identifiers (const_tree attr1, const_tree attr2)
@@ -4943,34 +4942,8 @@  cmp_attrib_identifiers (const_tree attr1, const_tree attr2)
   if (attr1 == attr2)
     return true;
 
-  /* If they are not equal, they may still be one in the form
-     'text' while the other one is in the form '__text__'.  TODO:
-     If we were storing attributes in normalized 'text' form, then
-     this could all go away and we could take full advantage of
-     the fact that we're comparing identifiers. :-)  */
-  const size_t attr1_len = IDENTIFIER_LENGTH (attr1);
-  const size_t attr2_len = IDENTIFIER_LENGTH (attr2);
-
-  if (attr2_len == attr1_len + 4)
-    {
-      const char *p = IDENTIFIER_POINTER (attr2);
-      const char *q = IDENTIFIER_POINTER (attr1);
-      if (p[0] == '_' && p[1] == '_'
-	  && p[attr2_len - 2] == '_' && p[attr2_len - 1] == '_'
-	  && strncmp (q, p + 2, attr1_len) == 0)
-	return true;;
-    }
-  else if (attr2_len + 4 == attr1_len)
-    {
-      const char *p = IDENTIFIER_POINTER (attr2);
-      const char *q = IDENTIFIER_POINTER (attr1);
-      if (q[0] == '_' && q[1] == '_'
-	  && q[attr1_len - 2] == '_' && q[attr1_len - 1] == '_'
-	  && strncmp (q + 2, p, attr2_len) == 0)
-	return true;
-    }
-
-  return false;
+  return cmp_attribs (IDENTIFIER_POINTER (attr1), IDENTIFIER_LENGTH (attr1),
+		      IDENTIFIER_POINTER (attr2), IDENTIFIER_LENGTH (attr2));
 }
 
 /* Compare two attributes for their value identity.  Return true if the
@@ -6043,32 +6016,6 @@  make_pass_ipa_free_lang_data (gcc::context *ctxt)
   return new pass_ipa_free_lang_data (ctxt);
 }
 
-/* The backbone of is_attribute_p().  ATTR_LEN is the string length of
-   ATTR_NAME.  Also used internally by remove_attribute().  */
-bool
-private_is_attribute_p (const char *attr_name, size_t attr_len, const_tree ident)
-{
-  size_t ident_len = IDENTIFIER_LENGTH (ident);
-
-  if (ident_len == attr_len)
-    {
-      if (id_equal (ident, attr_name))
-	return true;
-    }
-  else if (ident_len == attr_len + 4)
-    {
-      /* There is the possibility that ATTR is 'text' and IDENT is
-	 '__text__'.  */
-      const char *p = IDENTIFIER_POINTER (ident);      
-      if (p[0] == '_' && p[1] == '_'
-	  && p[ident_len - 2] == '_' && p[ident_len - 1] == '_'
-	  && strncmp (attr_name, p + 2, attr_len) == 0)
-	return true;
-    }
-
-  return false;
-}
-
 /* The backbone of lookup_attribute().  ATTR_LEN is the string length
    of ATTR_NAME, and LIST is not NULL_TREE.  */
 tree
@@ -6076,25 +6023,11 @@  private_lookup_attribute (const char *attr_name, size_t attr_len, tree list)
 {
   while (list)
     {
-      size_t ident_len = IDENTIFIER_LENGTH (get_attribute_name (list));
-
-      if (ident_len == attr_len)
-	{
-	  if (!strcmp (attr_name,
-		       IDENTIFIER_POINTER (get_attribute_name (list))))
-	    break;
-	}
-      /* TODO: If we made sure that attributes were stored in the
-	 canonical form without '__...__' (ie, as in 'text' as opposed
-	 to '__text__') then we could avoid the following case.  */
-      else if (ident_len == attr_len + 4)
-	{
-	  const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
-	  if (p[0] == '_' && p[1] == '_'
-	      && p[ident_len - 2] == '_' && p[ident_len - 1] == '_'
-	      && strncmp (attr_name, p + 2, attr_len) == 0)
-	    break;
-	}
+      tree attr = get_attribute_name (list);
+      size_t ident_len = IDENTIFIER_LENGTH (attr);
+      if (cmp_attribs (attr_name, attr_len, IDENTIFIER_POINTER (attr),
+		       ident_len))
+	break;
       list = TREE_CHAIN (list);
     }
 
@@ -6103,8 +6036,7 @@  private_lookup_attribute (const char *attr_name, size_t attr_len, tree list)
 
 /* Given an attribute name ATTR_NAME and a list of attributes LIST,
    return a pointer to the attribute's list first element if the attribute
-   starts with ATTR_NAME. ATTR_NAME must be in the form 'text' (not
-   '__text__').  */
+   starts with ATTR_NAME.  */
 
 tree
 private_lookup_attribute_by_prefix (const char *attr_name, size_t attr_len,
@@ -6121,17 +6053,11 @@  private_lookup_attribute_by_prefix (const char *attr_name, size_t attr_len,
 	}
 
       const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
+      gcc_checking_assert (attr_len == 0 || p[0] != '_');
 
       if (strncmp (attr_name, p, attr_len) == 0)
 	break;
 
-      /* TODO: If we made sure that attributes were stored in the
-	 canonical form without '__...__' (ie, as in 'text' as opposed
-	 to '__text__') then we could avoid the following case.  */
-      if (p[0] == '_' && p[1] == '_' &&
-	  strncmp (attr_name, p + 2, attr_len) == 0)
-	break;
-
       list = TREE_CHAIN (list);
     }
 
@@ -6177,16 +6103,14 @@  tree
 remove_attribute (const char *attr_name, tree list)
 {
   tree *p;
-  size_t attr_len = strlen (attr_name);
-
   gcc_checking_assert (attr_name[0] != '_');
 
   for (p = &list; *p; )
     {
       tree l = *p;
-      /* TODO: If we were storing attributes in normalized form, here
-	 we could use a simple strcmp().  */
-      if (private_is_attribute_p (attr_name, attr_len, get_attribute_name (l)))
+
+      tree attr = get_attribute_name (l);
+      if (is_attribute_p (attr_name, attr))
 	*p = TREE_CHAIN (l);
       else
 	p = &TREE_CHAIN (l);
diff --git a/gcc/tree.h b/gcc/tree.h
index 91cf253dee5..fab566b6684 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4231,29 +4231,8 @@  lookup_attribute_by_prefix (const char *attr_name, tree list)
 					       list);
 }
 
-
-/* This function is a private implementation detail of
-   is_attribute_p() and you should never call it directly.  */
-extern bool private_is_attribute_p (const char *, size_t, const_tree);
-
-/* Given an identifier node IDENT and a string ATTR_NAME, return true
-   if the identifier node is a valid attribute name for the string.
-   ATTR_NAME must be in the form 'text' (not '__text__').  IDENT could
-   be the identifier for 'text' or for '__text__'.  */
-
-static inline bool
-is_attribute_p (const char *attr_name, const_tree ident)
-{
-  gcc_checking_assert (attr_name[0] != '_');
-  /* Do the strlen() before calling the out-of-line implementation.
-     In most cases attr_name is a string constant, and the compiler
-     will optimize the strlen() away.  */
-  return private_is_attribute_p (attr_name, strlen (attr_name), ident);
-}
-
 /* Remove any instances of attribute ATTR_NAME in LIST and return the
-   modified list.  ATTR_NAME must be in the form 'text' (not
-   '__text__').  */
+   modified list.  */
 
 extern tree remove_attribute (const char *, tree);
 
-- 
2.13.2