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 <ja...@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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
