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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 22FA83858024 for ; Thu, 1 Apr 2021 04:52:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 22FA83858024 Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-347-bLsXBDvnNjSrSP8mUhwsjw-1; Thu, 01 Apr 2021 00:52:23 -0400 X-MC-Unique: bLsXBDvnNjSrSP8mUhwsjw-1 Received: by mail-qt1-f198.google.com with SMTP id l11so2417670qtk.2 for ; Wed, 31 Mar 2021 21:52:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=aPTsdW9Hzwqk+DCoM+ViPly+1yMkHak+/SXogOTN7js=; b=CBq5Pv2uev55IjeBF7ZuZ3ggd8lRgMwEjabw96aDnV1s1TefQVEJt18oQxK3OzWhCd bBhEVUPMCBgpHBr96g6ZlykWjwncijZZ+c119Ud6HCVwfauec6p8NgDlQSse8GdBjj54 vwX+WJWQKF0LARlcPAAd6SbhOk4zhdq5SRuXZskHKSmAfs7/SJr7CSUSkQEvTok4HHvh 5cRVSJq3l624mTFT+QkWyfoEg9O1Msqna8rXdqsf6FzQx6IJ+m99hN8GjV407zq4vQTf b/pUCHLti9gtx+y4YmCob7+6p3EkTKgVNQKcI3mPNlK/qkua1YNoKn2+YUlF33Ww6zCR HUrA== X-Gm-Message-State: AOAM531KDt5zRnZ4U+ATniBz32kVkjD4Q/QU70dmzbWiYHJvWmqDQiEd DKsTO6EZLO3Q/9hoaa/S2vzk5rnuLmmAypfOROcrkB+zRejPj3HJQ5CFdW/0iBCJzwvxIKr6yF9 czEOt2wDrD7WUlMwaGokEA6Bjf2AgJzNEPm4TLWkC2+O7jFhDqsmSWFeJZi6UFnDSCw== X-Received: by 2002:ac8:6f59:: with SMTP id n25mr5605349qtv.307.1617252742647; Wed, 31 Mar 2021 21:52:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxX5IpZI0gQzwsXCnD7uK54+GVYoJ2jUFXFtH13jxDnsnoolU67v96u/edOivBcmmgYO3qBiA== X-Received: by 2002:ac8:6f59:: with SMTP id n25mr5605337qtv.307.1617252742329; Wed, 31 Mar 2021 21:52:22 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id e3sm2791067qtj.28.2021.03.31.21.52.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 31 Mar 2021 21:52:21 -0700 (PDT) Subject: Re: [PATCH] c++, abi: Fix abi_tag attribute handling [PR98481] To: Jakub Jelinek , Richard Biener Cc: gcc-patches@gcc.gnu.org References: <20210107164718.GP725145@tucnak> <07f1435c-44ce-4b00-5491-a9f07a547148@redhat.com> <20210108192905.GY725145@tucnak> From: Jason Merrill Message-ID: <03eaf1db-d40d-5aa7-fc34-478bb97c0efa@redhat.com> Date: Thu, 1 Apr 2021 00:52:20 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210108192905.GY725145@tucnak> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="------------5AE453A758D7F6811BB04D0A" Content-Language: en-US X-Spam-Status: No, score=-15.5 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_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 04:52:29 -0000 This is a multi-part message in MIME format. --------------5AE453A758D7F6811BB04D0A Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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? --------------5AE453A758D7F6811BB04D0A Content-Type: text/x-patch; charset=UTF-8; name="pr98481-gcc10.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr98481-gcc10.diff" 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 --------------5AE453A758D7F6811BB04D0A Content-Type: text/x-patch; charset=UTF-8; name="pr98481-abi15.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr98481-abi15.diff" 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 --------------5AE453A758D7F6811BB04D0A--