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

[Andrew Pinski]

On Wed, Nov 24, 2021 at 12:48 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> 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?
>
> 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.

Only one comment on the new testcases, you might want to add a
testcase for the option on the command line too.

Thanks,
Andrew Pinski

>
> --- 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
>