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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 4DFFF3857C75 for ; Thu, 1 Apr 2021 05:47:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4DFFF3857C75 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-470-D_rmyaXZMzKkKhWlBe9OzQ-1; Thu, 01 Apr 2021 01:47:35 -0400 X-MC-Unique: D_rmyaXZMzKkKhWlBe9OzQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 69FA310866A3; Thu, 1 Apr 2021 05:47:34 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-95.ams2.redhat.com [10.36.112.95]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D4FB02AC82; Thu, 1 Apr 2021 05:47:33 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 1315lVmX3891122 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 1 Apr 2021 07:47:31 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 1315lUB73891121; Thu, 1 Apr 2021 07:47:30 +0200 Date: Thu, 1 Apr 2021 07:47:29 +0200 From: Jakub Jelinek To: Jason Merrill Cc: Richard Biener , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++, abi: Fix abi_tag attribute handling [PR98481] Message-ID: <20210401054729.GV1179226@tucnak> Reply-To: Jakub Jelinek References: <20210107164718.GP725145@tucnak> <07f1435c-44ce-4b00-5491-a9f07a547148@redhat.com> <20210108192905.GY725145@tucnak> <03eaf1db-d40d-5aa7-fc34-478bb97c0efa@redhat.com> MIME-Version: 1.0 In-Reply-To: <03eaf1db-d40d-5aa7-fc34-478bb97c0efa@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 01 Apr 2021 05:47:39 -0000 On Thu, Apr 01, 2021 at 12:52:20AM -0400, Jason Merrill wrote: > On 1/8/21 2:29 PM, Jakub Jelinek wrote: > > On Fri, Jan 08, 2021 at 02:22:59PM -0500, Jason Merrill wrote: > > > I like the idea to use *walk_subtrees to distinguish between walking > > > syntactic subtrees and walking type-identity subtrees. But it should be > > > more general; how does this look to you? > > > > LGTM, thanks. > > As discussed on IRC, we probably want to fix this in GCC 10 as well, but not > by default. The first patch adds ABI v15 and fixes the bug for !v14, so > -fabi-version=0 has the fix, but the default behavior does not. The second > patch adds ABI v15 on trunk. > > It would be nice to make -Wabi/-fabi-compat-version handle this case, but > that would require a bunch of new code unsuitable for this point in the > process. > > Does this make sense to you? LGTM, thanks. > commit 123f254cce416a4d50445465b88af6d8e754dc5e > Author: Jakub Jelinek > Date: Thu Jan 7 17:47:18 2021 +0100 > > c++, abi: Fix abi_tag attribute handling [PR98481] > > In GCC10 cp_walk_subtrees has been changed to walk template arguments. > As the following testcase, that changed the mangling of some functions. > I believe the previous behavior that find_abi_tags_r doesn't recurse into > template args has been the correct one, but setting *walk_subtrees = 0 > for the types and handling the types subtree walking manually in > find_abi_tags_r looks too hard, there are a lot of subtrees and details what > should and shouldn't be walked, both in tree.c (walk_type_fields there, > which is static) and in cp_walk_subtrees itself. > > The following patch abuses the fact that *walk_subtrees is an int to > tell cp_walk_subtrees it shouldn't walk the template args. > > But we don't want to introduce an ABI change in the middle of the GCC 10 > cycle, so the GCC 10 version of this patch introduces ABI v15 for the fix, > which will be available but not default in GCC 10.3. > > Co-authored-by: Jason Merrill > > gcc/cp/ChangeLog: > > PR c++/98481 > * class.c (find_abi_tags_r): Set *walk_subtrees to 2 instead of 1 > for types. > (mark_abi_tags_r): Likewise. > * tree.c (cp_walk_subtrees): If *walk_subtrees_p is 2, look through > typedefs. > > gcc/testsuite/ChangeLog: > > PR c++/98481 > * g++.dg/abi/abi-tag24.C: New test. > * g++.dg/abi/abi-tag24a.C: New test. > * g++.dg/abi/macro0.C: Adjust expected value. > > gcc/ChangeLog: > > PR c++/98481 > * common.opt (fabi-version): Default to 14. > > gcc/c-family/ChangeLog: > > PR c++/98481 > * c-opts.c (c_common_post_options): Bump latest_abi_version. > > diff --git a/gcc/common.opt b/gcc/common.opt > index 9cc47b16cac..ec5235c3a41 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -956,10 +956,13 @@ Driver Undocumented > ; 14: Corrects the mangling of nullptr expression. > ; Default in G++ 10. > ; > +; 15: Corrects G++ 10 ABI tag regression [PR98481]. > +; Available, but not default, in G++ 10.3. > +; > ; Additional positive integers will be assigned as new versions of > ; the ABI become the default version of the ABI. > fabi-version= > -Common Joined RejectNegative UInteger Var(flag_abi_version) Init(0) > +Common Joined RejectNegative UInteger Var(flag_abi_version) Init(14) > The version of the C++ ABI in use. > > faggressive-loop-optimizations > diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c > index 58ba0948e79..c51d6d34726 100644 > --- a/gcc/c-family/c-opts.c > +++ b/gcc/c-family/c-opts.c > @@ -943,7 +943,7 @@ c_common_post_options (const char **pfilename) > > /* Change flag_abi_version to be the actual current ABI level, for the > benefit of c_cpp_builtins, and to make comparison simpler. */ > - const int latest_abi_version = 14; > + const int latest_abi_version = 15; > /* Generate compatibility aliases for ABI v11 (7.1) by default. */ > const int abi_compat_default = 11; > > diff --git a/gcc/cp/class.c b/gcc/cp/class.c > index ed8f9527929..c0101130ba3 100644 > --- a/gcc/cp/class.c > +++ b/gcc/cp/class.c > @@ -1450,6 +1450,10 @@ mark_or_check_tags (tree t, tree *tp, abi_tag_data *p, bool val) > static tree > find_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > { > + if (TYPE_P (*tp) && *walk_subtrees == 1 && flag_abi_version != 14) > + /* Tell cp_walk_subtrees to look though typedefs. [PR98481] */ > + *walk_subtrees = 2; > + > if (!OVERLOAD_TYPE_P (*tp)) > return NULL_TREE; > > @@ -1470,6 +1474,10 @@ find_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > static tree > mark_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > { > + if (TYPE_P (*tp) && *walk_subtrees == 1 && flag_abi_version != 14) > + /* Tell cp_walk_subtrees to look though typedefs. */ > + *walk_subtrees = 2; > + > if (!OVERLOAD_TYPE_P (*tp)) > return NULL_TREE; > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index b36ca4eddc0..10b818d1370 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -5055,8 +5055,18 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func, > while (0) > > if (TYPE_P (*tp)) > - /* Walk into template args without looking through typedefs. */ > - if (tree ti = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (*tp)) > + /* If *WALK_SUBTREES_P is 1, we're interested in the syntactic form of > + the argument, so don't look through typedefs, but do walk into > + template arguments for alias templates (and non-typedefed classes). > + > + If *WALK_SUBTREES_P > 1, we're interested in type identity or > + equivalence, so look through typedefs, ignoring template arguments for > + alias templates, and walk into template args of classes. > + > + See find_abi_tags_r for an example of setting *WALK_SUBTREES_P to 2 > + when that's the behavior the walk_tree_fn wants. */ > + if (tree ti = (*walk_subtrees_p > 1 ? TYPE_TEMPLATE_INFO (*tp) > + : TYPE_TEMPLATE_INFO_MAYBE_ALIAS (*tp))) > WALK_SUBTREE (TI_ARGS (ti)); > > /* Not one of the easy cases. We must explicitly go through the > diff --git a/gcc/testsuite/g++.dg/abi/abi-tag24.C b/gcc/testsuite/g++.dg/abi/abi-tag24.C > new file mode 100644 > index 00000000000..2c5c542bfcd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/abi-tag24.C > @@ -0,0 +1,18 @@ > +// PR c++/98481 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options -fabi-version=0 } > +inline namespace N __attribute ((__abi_tag__ ("myabi"))) > +{ > + struct A {}; > +} > +template > +struct B { typedef int size_type; }; > +struct S1 { B::size_type foo () const { return 1; } }; > +struct S2 { B::size_type foo () const; }; > +int S2::foo () const { return 2; } > +int (S1::*f1) () const = &S1::foo; > +int (S2::*f2) () const = &S2::foo; > + > +// { dg-final { scan-assembler "_ZNK2S13fooEv" } } > +// { dg-final { scan-assembler "_ZNK2S23fooEv" } } > +// { dg-final { scan-assembler-not "_ZNK2S13fooB5myabiEv" } } > diff --git a/gcc/testsuite/g++.dg/abi/abi-tag24a.C b/gcc/testsuite/g++.dg/abi/abi-tag24a.C > new file mode 100644 > index 00000000000..83f930dfdde > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/abi-tag24a.C > @@ -0,0 +1,18 @@ > +// PR c++/98481 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options -fabi-version=14 } > +inline namespace N __attribute ((__abi_tag__ ("myabi"))) > +{ > + struct A {}; > +} > +template > +struct B { typedef int size_type; }; > +struct S1 { B::size_type foo () const { return 1; } }; > +struct S2 { B::size_type foo () const; }; > +int S2::foo () const { return 2; } > +int (S1::*f1) () const = &S1::foo; > +int (S2::*f2) () const = &S2::foo; > + > +// { dg-final { scan-assembler-not "_ZNK2S13fooEv" } } > +// { dg-final { scan-assembler "_ZNK2S23fooEv" } } > +// { dg-final { scan-assembler "_ZNK2S13fooB5myabiEv" } } > diff --git a/gcc/testsuite/g++.dg/abi/macro0.C b/gcc/testsuite/g++.dg/abi/macro0.C > index 08106004c4d..7c3c17051ed 100644 > --- a/gcc/testsuite/g++.dg/abi/macro0.C > +++ b/gcc/testsuite/g++.dg/abi/macro0.C > @@ -1,6 +1,6 @@ > // This testcase will need to be kept in sync with c_common_post_options. > // { dg-options "-fabi-version=0" } > > -#if __GXX_ABI_VERSION != 1014 > +#if __GXX_ABI_VERSION != 1015 > #error "Incorrect value of __GXX_ABI_VERSION" > #endif > commit 02ad075e2058c6c4f6cf05606653981c5efb4d0d > Author: Jason Merrill > Date: Wed Mar 31 17:48:50 2021 -0400 > > c++: Add ABI version for PR98481 fix > > The PR98481 fix corrects an ABI regression in GCC 10, but we don't want to > introduce an ABI change in the middle of the GCC 10 cycle. This patch > introduces ABI v15 for the fix, which will be available but not default in > GCC 10.3; the broken behavior remains in ABI v14. Compatibility aliases > will not be generated for this change. > > gcc/ChangeLog: > > PR c++/98481 > * common.opt: Document v15 and v16. > > gcc/c-family/ChangeLog: > > PR c++/98481 > * c-opts.c (c_common_post_options): Bump latest_abi_version. > > gcc/cp/ChangeLog: > > PR c++/98481 > * mangle.c (write_expression): Adjust. > * class.c (find_abi_tags_r): Disable PR98481 fix for ABI v14. > (mark_abi_tags_r): Likewise. > > gcc/testsuite/ChangeLog: > > PR c++/98481 > * g++.dg/abi/abi-tag24a.C: New test. > * g++.dg/abi/macro0.C: Adjust expected value. > > diff --git a/gcc/common.opt b/gcc/common.opt > index c75dd36843e..a75b44ee47e 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -960,7 +960,11 @@ Driver Undocumented > ; 14: Corrects the mangling of nullptr expression. > ; Default in G++ 10. > ; > -; 15: Changes the mangling of __alignof__ to be distinct from that of alignof. > +; 15: Corrects G++ 10 ABI tag regression [PR98481]. > +; Available, but not default, in G++ 10.3. > +; > +; 16: Changes the mangling of __alignof__ to be distinct from that of alignof. > +; Adds missing 'on' in mangling of operator names in some cases. > ; Default in G++ 11. > ; > ; Additional positive integers will be assigned as new versions of > diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c > index bd15b9cd902..89e05a4c551 100644 > --- a/gcc/c-family/c-opts.c > +++ b/gcc/c-family/c-opts.c > @@ -965,7 +965,7 @@ c_common_post_options (const char **pfilename) > > /* Change flag_abi_version to be the actual current ABI level, for the > benefit of c_cpp_builtins, and to make comparison simpler. */ > - const int latest_abi_version = 15; > + const int latest_abi_version = 16; > /* Generate compatibility aliases for ABI v11 (7.1) by default. */ > const int abi_compat_default = 11; > > diff --git a/gcc/cp/class.c b/gcc/cp/class.c > index 856e81e3d1a..4bffec4a707 100644 > --- a/gcc/cp/class.c > +++ b/gcc/cp/class.c > @@ -1494,8 +1494,8 @@ mark_or_check_tags (tree t, tree *tp, abi_tag_data *p, bool val) > static tree > find_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > { > - if (TYPE_P (*tp) && *walk_subtrees == 1) > - /* Tell cp_walk_subtrees to look though typedefs. */ > + if (TYPE_P (*tp) && *walk_subtrees == 1 && flag_abi_version != 14) > + /* Tell cp_walk_subtrees to look though typedefs. [PR98481] */ > *walk_subtrees = 2; > > if (!OVERLOAD_TYPE_P (*tp)) > @@ -1518,7 +1518,7 @@ find_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > static tree > mark_abi_tags_r (tree *tp, int *walk_subtrees, void *data) > { > - if (TYPE_P (*tp) && *walk_subtrees == 1) > + if (TYPE_P (*tp) && *walk_subtrees == 1 && flag_abi_version != 14) > /* Tell cp_walk_subtrees to look though typedefs. */ > *walk_subtrees = 2; > > diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c > index 57ce9a6710f..6c111342b97 100644 > --- a/gcc/cp/mangle.c > +++ b/gcc/cp/mangle.c > @@ -3119,9 +3119,9 @@ write_expression (tree expr) > { > if (!ALIGNOF_EXPR_STD_P (expr)) > { > - if (abi_warn_or_compat_version_crosses (15)) > + if (abi_warn_or_compat_version_crosses (16)) > G.need_abi_warning = true; > - if (abi_version_at_least (15)) > + if (abi_version_at_least (16)) > { > /* We used to mangle __alignof__ like alignof. */ > write_string ("u11__alignof__"); > @@ -3350,9 +3350,9 @@ write_expression (tree expr) > tree name = dependent_name (expr); > if (IDENTIFIER_ANY_OP_P (name)) > { > - if (abi_version_at_least (15)) > + if (abi_version_at_least (16)) > write_string ("on"); > - if (abi_warn_or_compat_version_crosses (15)) > + if (abi_warn_or_compat_version_crosses (16)) > G.need_abi_warning = 1; > } > write_unqualified_id (name); > diff --git a/gcc/testsuite/g++.dg/abi/abi-tag24a.C b/gcc/testsuite/g++.dg/abi/abi-tag24a.C > new file mode 100644 > index 00000000000..83f930dfdde > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/abi-tag24a.C > @@ -0,0 +1,18 @@ > +// PR c++/98481 > +// { dg-do compile { target c++11 } } > +// { dg-additional-options -fabi-version=14 } > +inline namespace N __attribute ((__abi_tag__ ("myabi"))) > +{ > + struct A {}; > +} > +template > +struct B { typedef int size_type; }; > +struct S1 { B::size_type foo () const { return 1; } }; > +struct S2 { B::size_type foo () const; }; > +int S2::foo () const { return 2; } > +int (S1::*f1) () const = &S1::foo; > +int (S2::*f2) () const = &S2::foo; > + > +// { dg-final { scan-assembler-not "_ZNK2S13fooEv" } } > +// { dg-final { scan-assembler "_ZNK2S23fooEv" } } > +// { dg-final { scan-assembler "_ZNK2S13fooB5myabiEv" } } > diff --git a/gcc/testsuite/g++.dg/abi/macro0.C b/gcc/testsuite/g++.dg/abi/macro0.C > index 7c3c17051ed..f25f291dba6 100644 > --- a/gcc/testsuite/g++.dg/abi/macro0.C > +++ b/gcc/testsuite/g++.dg/abi/macro0.C > @@ -1,6 +1,6 @@ > // This testcase will need to be kept in sync with c_common_post_options. > // { dg-options "-fabi-version=0" } > > -#if __GXX_ABI_VERSION != 1015 > +#if __GXX_ABI_VERSION != 1016 > #error "Incorrect value of __GXX_ABI_VERSION" > #endif Jakub