LGTM but I'd like Marek to approve it. On Fri, Nov 3, 2023, 3:12 PM Jakub Jelinek <ja...@redhat.com> wrote:
> Hi! > > The following testcase ICEs, because with -Wno-attributes=foo::no_sanitize > (but generally any other non-gnu namespace and some gnu well known > attribute > name within that other namespace) the FEs don't really parse attribute > arguments of such attribute, but lookup_attribute_spec is non-NULL with > NULL handler and such attributes are added to DECL_ATTRIBUTES or > TYPE_ATTRIBUTES and then when e.g. middle-end does lookup_attribute > on a particular attribute and expects the attribute to mean something > and/or have a particular verified arguments, it can crash when seeing > the foreign attribute in there instead. > > The following patch fixes that by never adding ignored attributes > to DECL_ATTRIBUTES/TYPE_ATTRIBUTES, previously that was the case just > for attributes in ignored namespace (where lookup_attribute_space > returned NULL). We don't really know anything about those attributes, > so shouldn't pretend we know something about them, especially when > the arguments are error_mark_node or NULL instead of something that > would have been parsed. And it would be really weird if we normally > ignore say [[clang::unused]] attribute, but when people use > -Wno-attributes=clang::unused we actually treated it as gnu::unused. > All the user asked for is suppress warnings about that attribute being > unknown. > > The first hunk is just playing safe, I'm worried people could > -Wno-attributes=gnu:: > and get various crashes with known GNU attributes not being actually > parsed and recorded (or worse e.g. when we tweak standard attributes > into GNU attributes and we wouldn't add those). > The -Wno-attributes= documentation says that it suppresses warning about > unknown attributes, so I think -Wno-attributes=gnu:: should prevent > warning about say [[gnu::foobarbaz]] attribute, but not about > [[gnu::unused]] because the latter is a known attribute. > The routine would return true for any scoped attribute in the ignored > namespace, with the change it ignores only unknown attributes in ignored > namespace, known ones in there will be ignored only if they have > max_length of -2 (e.g.. with > -Wno-attributes=gnu:: -Wno-attributes=gnu::foobarbaz). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2023-11-03 Jakub Jelinek <ja...@redhat.com> > > PR c/112339 > * attribs.cc (attribute_ignored_p): Only return true for > attr_namespace_ignored_p if as is NULL. > (decl_attributes): Never add ignored attributes. > > * c-c++-common/ubsan/Wno-attributes-1.c: New test. > > --- gcc/attribs.cc.jj 2023-11-02 20:22:02.017016371 +0100 > +++ gcc/attribs.cc 2023-11-03 08:45:32.688726738 +0100 > @@ -579,9 +579,9 @@ attribute_ignored_p (tree attr) > return false; > if (tree ns = get_attribute_namespace (attr)) > { > - if (attr_namespace_ignored_p (ns)) > - return true; > const attribute_spec *as = lookup_attribute_spec (TREE_PURPOSE > (attr)); > + if (as == NULL && attr_namespace_ignored_p (ns)) > + return true; > if (as && as->max_length == -2) > return true; > } > @@ -862,7 +862,10 @@ decl_attributes (tree *node, tree attrib > } > } > > - if (no_add_attrs) > + if (no_add_attrs > + /* Don't add attributes registered just for > -Wno-attributes=foo::bar > + purposes. */ > + || attribute_ignored_p (attr)) > continue; > > if (spec->handler != NULL) > --- gcc/testsuite/c-c++-common/ubsan/Wno-attributes-1.c.jj 2023-11-03 > 08:44:13.752837201 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/Wno-attributes-1.c 2023-11-03 > 08:44:13.751837215 +0100 > @@ -0,0 +1,9 @@ > +/* PR c/112339 */ > +/* { dg-do compile { target { c++11 || c } } } */ > +/* { dg-options "-Wno-attributes=foo::no_sanitize -fsanitize=undefined" } > */ > +/* { dg-additional-options "-std=c2x" { target c } } */ > + > +[[foo::no_sanitize("bounds")]] void > +foo (void) > +{ > +} > > Jakub > >