[PATCH] attribs: Fix ICEs on attributes starting with _ [PR103365]

Richard Biener rguenther@suse.de
Wed Nov 24 09:03:03 GMT 2021


On Wed, 24 Nov 2021, Jakub Jelinek wrote:

> Hi!
> 
> As the patch shows, we have quite a few asserts that we don't call
> lookup_attribute etc. with attr_name that starts with an underscore,
> to make sure nobody is trying to call it with non-canonicalized
> attribute name like "__cold__" instead of "cold".
> We canonicalize only attributes that start with 2 underscores and end
> with 2 underscores though.
> Before Marek's patch, that wasn't an issue, we had no attributes like
> "_foo" or "__bar_" etc., so lookup_scoped_attribute_spec would
> always return NULL for those and we wouldn't try to register them,
> look them up etc., just with -Wattributes would warn about them.
> But now, as the new testcase shows, users can actually request such
> attributes to be ignored, and we ICE for those during
> register_scoped_attribute and when that is fixed, ICE later on when
> somebody uses those attributes because they will be looked up
> to find out that they should be ignored.
> 
> So, the following patch instead of or in addition to, depending on
> how performance sensitive a particular spot is, checking that
> attribute doesn't start with underscore allows attribute
> names that start with underscore as long as it doesn't canonicalize
> (i.e. doesn't start and end with 2 underscores).
> In addition to that, I've noticed lookup_attribute_by_prefix
> was calling get_attribute_name twice unnecessarily, and 2 tests
> were running in c++98 mode with -std=c++98 -std=c++11 which IMHO
> isn't useful because -std=c++11 testing is done too when testing
> all language versions.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2021-11-24  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/103365
> 	* attribs.h (lookup_attribute): Allow attr_name to start with
> 	underscore, as long as canonicalize_attr_name returns false.
> 	(lookup_attribute_by_prefix): Don't call get_attribute_name twice.
> 	* attribs.c (extract_attribute_substring): Reimplement using
> 	canonicalize_attr_name.
> 	(register_scoped_attribute): Change gcc_assert into
> 	gcc_checking_assert, verify !canonicalize_attr_name rather than
> 	that str.str doesn't start with '_'.
> 
> 	* c-c++-common/Wno-attributes-1.c: Require effective target
> 	c || c++11 and drop dg-additional-options.
> 	* c-c++-common/Wno-attributes-2.c: Likewise.
> 	* c-c++-common/Wno-attributes-4.c: New test.
> 
> --- gcc/attribs.h.jj	2021-11-22 10:06:42.173498383 +0100
> +++ gcc/attribs.h	2021-11-23 23:35:13.757972934 +0100
> @@ -188,7 +188,11 @@ is_attribute_p (const char *attr_name, c
>  static inline tree
>  lookup_attribute (const char *attr_name, tree list)
>  {
> -  gcc_checking_assert (attr_name[0] != '_');
> +  if (CHECKING_P && attr_name[0] != '_')
> +    {
> +      size_t attr_len = strlen (attr_name);
> +      gcc_checking_assert (!canonicalize_attr_name (attr_name, attr_len));
> +    }
>    /* In most cases, list is NULL_TREE.  */
>    if (list == NULL_TREE)
>      return NULL_TREE;
> @@ -219,7 +223,8 @@ lookup_attribute_by_prefix (const char *
>        size_t attr_len = strlen (attr_name);
>        while (list)
>  	{
> -	  size_t ident_len = IDENTIFIER_LENGTH (get_attribute_name (list));
> +	  tree name = get_attribute_name (list);
> +	  size_t ident_len = IDENTIFIER_LENGTH (name);
>  
>  	  if (attr_len > ident_len)
>  	    {
> @@ -227,7 +232,7 @@ lookup_attribute_by_prefix (const char *
>  	      continue;
>  	    }
>  
> -	  const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
> +	  const char *p = IDENTIFIER_POINTER (name);
>  	  gcc_checking_assert (attr_len == 0 || p[0] != '_');
>  
>  	  if (strncmp (attr_name, p, attr_len) == 0)
> --- gcc/attribs.c.jj	2021-11-22 10:06:42.172498397 +0100
> +++ gcc/attribs.c	2021-11-23 14:58:25.076233815 +0100
> @@ -115,12 +115,7 @@ static const struct attribute_spec empty
>  static void
>  extract_attribute_substring (struct substring *str)
>  {
> -  if (str->length > 4 && str->str[0] == '_' && str->str[1] == '_'
> -      && str->str[str->length - 1] == '_' && str->str[str->length - 2] == '_')
> -    {
> -      str->length -= 4;
> -      str->str += 2;
> -    }
> +  canonicalize_attr_name (str->str, str->length);
>  }
>  
>  /* Insert an array of attributes ATTRIBUTES into a namespace.  This
> @@ -387,7 +382,7 @@ register_scoped_attribute (const struct
>  
>    /* Attribute names in the table must be in the form 'text' and not
>       in the form '__text__'.  */
> -  gcc_assert (str.length > 0 && str.str[0] != '_');
> +  gcc_checking_assert (!canonicalize_attr_name (str.str, str.length));
>  
>    slot = name_space->attribute_hash
>  	 ->find_slot_with_hash (&str, substring_hash (str.str, str.length),
> --- gcc/testsuite/c-c++-common/Wno-attributes-1.c.jj	2021-11-11 14:35:37.637348034 +0100
> +++ gcc/testsuite/c-c++-common/Wno-attributes-1.c	2021-11-23 15:03:05.426198652 +0100
> @@ -1,6 +1,5 @@
>  /* PR c++/101940 */
> -/* { dg-do compile } */
> -/* { dg-additional-options "-std=c++11" { target c++ } } */
> +/* { dg-do compile { target { c || c++11 } } } */
>  /* { dg-additional-options "-Wno-attributes=company::,yoyodyne::attr" } */
>  /* { dg-additional-options "-Wno-attributes=c1::attr,c1::attr,c1::__attr__" } */
>  /* { dg-additional-options "-Wno-attributes=c2::,c2::attr" } */
> --- gcc/testsuite/c-c++-common/Wno-attributes-2.c.jj	2021-11-11 14:35:37.637348034 +0100
> +++ gcc/testsuite/c-c++-common/Wno-attributes-2.c	2021-11-23 15:03:14.637066079 +0100
> @@ -1,6 +1,5 @@
>  /* PR c++/101940 */
> -/* { dg-do compile } */
> -/* { dg-additional-options "-std=c++11" { target c++ } } */
> +/* { dg-do compile { target { c || c++11 } } } */
>  
>  #pragma GCC diagnostic ignored_attributes "company::,yoyodyne::attr"
>  #pragma GCC diagnostic ignored_attributes "c1::attr,c1::attr,c1::__attr__"
> --- gcc/testsuite/c-c++-common/Wno-attributes-4.c.jj	2021-11-23 15:00:47.584182655 +0100
> +++ gcc/testsuite/c-c++-common/Wno-attributes-4.c	2021-11-23 15:02:23.348804286 +0100
> @@ -0,0 +1,8 @@
> +/* PR middle-end/103365 */
> +/* { dg-do compile { target { c || c++11 } } } */
> +
> +#pragma GCC diagnostic ignored_attributes "foo::_bar"
> +#pragma GCC diagnostic ignored_attributes "_foo::bar"
> +
> +[[foo::_bar]] void foo (void);
> +[[_foo::bar]] void bar (void);
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)


More information about the Gcc-patches mailing list