From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 851713858D28 for ; Fri, 17 Dec 2021 01:06:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 851713858D28 Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-33-omLB5odbOfaSvC1rwDd6TA-1; Thu, 16 Dec 2021 20:06:46 -0500 X-MC-Unique: omLB5odbOfaSvC1rwDd6TA-1 Received: by mail-qv1-f71.google.com with SMTP id q9-20020ad45749000000b003bdeb0612c5so1069523qvx.8 for ; Thu, 16 Dec 2021 17:06:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=kTqZmpRBHSzbSRujbVKW24OcFtXohAaOBDJuhDiudXg=; b=eE7/ylGazCJGagjGsfZDG4EMhW200bqd3L4Zyvuy2w7iqFqQ3mLSHF7PK3CNhRiB8p vhDFikfPlBwYMUXRn8e/9m5YqQZNIPranGeXvUAxL6Hnw43g2Lt+Geg1GWMYq3uf/4+e OLmeAaksqgD6YFuXYPXNd4BzuglKui8uCYw0cROhN17zpEbaAfXM++kFfjNT+2CUSQov CvGt0DVycYTVnQp0rcOVDz6AuCM7a3G64wmmdsHA8FLCCW2hHNnjhfWPBQTsFhJ8YyGj by7O3U3z8qCRi3l6VG7dJdpdcP/Q85kqT6OO/nm6BcZCjjzSfWwcmSRltT7+dp+qb5KC 0jLg== X-Gm-Message-State: AOAM53059Xy7QtU4ltksULPMO0Gg5cgqWW4NiyvTMYUH6ngHqG3AfPjv P7hv3ly8tiCDYnARpO6Nf337KgjrjbjvOlBzoYv2HMfMH2AMIramDEnRecJdcyMQinCEBeYJpQs aHUyb4E7KQJJJgQgeMg== X-Received: by 2002:a0c:c60e:: with SMTP id v14mr707803qvi.27.1639703205380; Thu, 16 Dec 2021 17:06:45 -0800 (PST) X-Google-Smtp-Source: ABdhPJzpLFdNzHfqQoKqZTR1cLYrdoOrR4TETgGeXseoGNlzZdPzamA214vW+qgMBuGsjiBq28lB4g== X-Received: by 2002:a0c:c60e:: with SMTP id v14mr707777qvi.27.1639703205041; Thu, 16 Dec 2021 17:06:45 -0800 (PST) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id p12sm4996078qtx.56.2021.12.16.17.06.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Dec 2021 17:06:44 -0800 (PST) Message-ID: <7d088a03-cddc-9287-aae1-e32532a21f12@redhat.com> Date: Thu, 16 Dec 2021 20:06:43 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Subject: Re: [PATCH v2] attribs: Fix wrong error with -Wno-attribute=A::b [PR103649] To: Marek Polacek , Jakub Jelinek Cc: GCC Patches , Joseph Myers References: <20211216223555.820800-1-polacek@redhat.com> <20211216225346.GA2646553@tucnak> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Dec 2021 01:06:49 -0000 On 12/16/21 19:52, Marek Polacek wrote: > On Thu, Dec 16, 2021 at 11:53:46PM +0100, Jakub Jelinek wrote: >> On Thu, Dec 16, 2021 at 05:35:55PM -0500, Marek Polacek wrote: >>> My patch to implement -Wno-attribute=A::b caused a bogus error when >>> parsing >>> >>> [[foo::bar(1, 2)]]; >>> >>> when -Wno-attributes=foo::bar was specified on the command line, because >>> when we create a fake foo::bar attribute and insert it into our attribute >>> table, it is created with max_length == 0 which doesn't allow any args. >>> That is wrong -- we know nothing about the attribute, so we shouldn't >>> require any specific number of arguments. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >>> >>> PR c/103649 >>> >>> gcc/ChangeLog: >>> >>> * attribs.c (handle_ignored_attributes_option): Create the fake >>> attribute with max_length == -1. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * c-c++-common/Wno-attributes-6.c: New test. >> >> I'm afraid this still changes behavior. 0, -1 range attribute arguments >> are parsed normally, while unknown attributes have the balanced token >> sequence skipped. >> E.g. the omp::{directive,sequence} attribute arguments are much more complex >> than what the normal parsing can handle. >> Can you make max -2 instead and special case it in the C and C++ FE >> attribute handling and in a testcase try something that is a balanced token >> sequence but not really valid when parsed as ordinary attributes' arguments? > > Ah I see what you mean now. Fixed here, thanks. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > My patch to implement -Wno-attribute=A::b caused a bogus error when > parsing > > [[foo::bar(1, 2)]]; > > when -Wno-attributes=foo::bar was specified on the command line, because > when we create a fake foo::bar attribute and insert it into our attribute > table, it is created with max_length == 0 which doesn't allow any args. > That is wrong -- we know nothing about the attribute, so we shouldn't > require any specific number of arguments. And since unknown attributes > can be rather complex (see for example omp::{directive,sequence}), we > must skip parsing their arguments. To that end, I'm using max_length > with value -2. > > PR c/103649 > > gcc/ChangeLog: > > * attribs.c (handle_ignored_attributes_option): Create the fake > attribute with max_length == -2. > * tree-core.h (struct attribute_spec): Document that max_length > can be -2. > > gcc/c/ChangeLog: > > * c-parser.c (c_parser_std_attribute): Skip parsing of the attribute > arguments when max_length == -2. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_std_attribute): Skip parsing of the attribute > arguments when max_length == -2. > > gcc/testsuite/ChangeLog: > > * c-c++-common/Wno-attributes-6.c: New test. > --- > gcc/attribs.c | 2 +- > gcc/c/c-parser.c | 4 +++- > gcc/cp/parser.c | 4 +++- > gcc/testsuite/c-c++-common/Wno-attributes-6.c | 14 ++++++++++++++ > gcc/tree-core.h | 4 +++- > 5 files changed, 24 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/Wno-attributes-6.c > > diff --git a/gcc/attribs.c b/gcc/attribs.c > index 29703e75fba..6af7f93e61c 100644 > --- a/gcc/attribs.c > +++ b/gcc/attribs.c > @@ -304,7 +304,7 @@ handle_ignored_attributes_option (vec *v) > We can't free it here, so squirrel away the pointers. */ > attribute_spec *table = new attribute_spec[2]; > ignored_attributes_table.safe_push (table); > - table[0] = { attr, 0, 0, false, false, false, false, nullptr, nullptr }; > + table[0] = { attr, 0, -2, false, false, false, false, nullptr, nullptr }; > table[1] = { nullptr, 0, 0, false, false, false, false, nullptr, > nullptr }; > register_scoped_attributes (table, IDENTIFIER_POINTER (vendor_id), !attr); > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c > index d7e5f051ac0..c9068bdbb8a 100644 > --- a/gcc/c/c-parser.c > +++ b/gcc/c/c-parser.c > @@ -4943,7 +4943,9 @@ c_parser_std_attribute (c_parser *parser, bool for_tm) > parens.skip_until_found_close (parser); > return error_mark_node; > } > - if (as) > + /* When MAX_LENGTH is -2, this is a fake attribute created to > + handle -Wno-attributes, and we must skip parsing the arguments. */ > + if (as && as->max_length != -2) > { > bool takes_identifier > = (ns != NULL_TREE > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 44eed7ea638..763d74eaf6c 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -28979,7 +28979,9 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) > /* A GNU attribute that takes an identifier in parameter. */ > attr_flag = id_attr; > > - if (as == NULL) > + /* When MAX_LENGTH is -2, this is a fake attribute created to > + handle -Wno-attributes, and we must skip parsing the arguments. */ > + if (as == NULL || as->max_length == -2) > { > if ((flag_openmp || flag_openmp_simd) && attr_ns == omp_identifier) > { > diff --git a/gcc/testsuite/c-c++-common/Wno-attributes-6.c b/gcc/testsuite/c-c++-common/Wno-attributes-6.c > new file mode 100644 > index 00000000000..0b2dd5aa290 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Wno-attributes-6.c > @@ -0,0 +1,14 @@ > +/* PR c/103649 */ > +/* { dg-do compile { target { c || c++11 } } } */ > +/* { dg-additional-options "-Wno-attributes=foo::bar" } */ > +/* { dg-additional-options "-Wno-attributes=baz::" } */ > +/* { dg-additional-options "-Wno-attributes=womp::womp" } */ > +/* { dg-additional-options "-Wno-attributes=qux::foo" } */ > + > +[[foo::bar(1, 2)]]; /* { dg-warning "attribute ignored" } */ > +[[baz::bar(1, 2)]]; /* { dg-warning "attribute ignored" } */ > +[[foo::bar(1, 2)]] void f1(); > +[[baz::bar(1, 2)]] void f2(); > +[[qux::foo({t})]] void f3(); > +[[womp::womp (another::directive (threadprivate (t)))]] void f4(); > +[[womp::womp (another::directive (threadprivate (t)))]]; /* { dg-warning "attribute ignored" } */ If we're ignoring these attributes, we should ignore them everywhere; perhaps using them for an attribute-declaration is meaningful in another compiler. > diff --git a/gcc/tree-core.h b/gcc/tree-core.h > index 91ae5237d7e..9b37a065d18 100644 > --- a/gcc/tree-core.h > +++ b/gcc/tree-core.h > @@ -2077,7 +2077,9 @@ struct attribute_spec { > /* The minimum length of the list of arguments of the attribute. */ > int min_length; > /* The maximum length of the list of arguments of the attribute > - (-1 for no maximum). */ > + (-1 for no maximum). It can also be -2 for fake attributes > + created for the sake of -Wno-attributes; in that case, we > + should skip the balanced token sequence when parsing the attribute. */ > int max_length; > /* Whether this attribute requires a DECL. If it does, it will be passed > from types of DECLs, function return types and array element types to > > base-commit: 840a22e0fee9e7369a2eb1c9e3c70dcae24a20e4