| Message ID | 20210107164718.GP725145@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++, abi: Fix abi_tag attribute handling [PR98481] | expand |
On 1/7/21 11:47 AM, Jakub Jelinek wrote: > In GCC10 cp_walk_subtrees has been changed to walk template arguments. > As the following testcase, that changed the mangling of some functions. Argh. > 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. > > Another option would be to have two separate cp_walk_subtrees-like > callbacks, one that wouldn't walk into template args and the other > that would and then would tail call the other one, and > cp_walk_tree_without_duplicates but call walk_tree_1 directly or use > some other macro. 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? Jason
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. > diff --git a/gcc/cp/class.c b/gcc/cp/class.c > index c41ac7deefe..00c0dba0a55 100644 > --- a/gcc/cp/class.c > +++ b/gcc/cp/class.c > @@ -1507,6 +1507,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) > + /* Tell cp_walk_subtrees to look though typedefs. */ > + *walk_subtrees = 2; > + > if (!OVERLOAD_TYPE_P (*tp)) > return NULL_TREE; > > @@ -1527,6 +1531,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) > + /* Tell cp_walk_subtrees to look though typedefs. */ > + *walk_subtrees = 2; > + > if (!OVERLOAD_TYPE_P (*tp)) > return NULL_TREE; > > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c > index b10671091b5..b087753cfba 100644 > --- a/gcc/cp/decl2.c > +++ b/gcc/cp/decl2.c > @@ -2358,9 +2358,6 @@ min_vis_r (tree *tp, int *walk_subtrees, void *data) > int this_vis = VISIBILITY_DEFAULT; > if (! TYPE_P (*tp)) > *walk_subtrees = 0; > - else if (typedef_variant_p (*tp)) > - /* Look through typedefs despite cp_walk_subtrees. */ > - this_vis = type_visibility (DECL_ORIGINAL_TYPE (TYPE_NAME (*tp))); > else if (OVERLOAD_TYPE_P (*tp) > && !TREE_PUBLIC (TYPE_MAIN_DECL (*tp))) > { > @@ -2379,6 +2376,10 @@ min_vis_r (tree *tp, int *walk_subtrees, void *data) > if (this_vis > *vis_p) > *vis_p = this_vis; > > + /* Tell cp_walk_subtrees to look through typedefs. */ > + if (*walk_subtrees == 1) > + *walk_subtrees = 2; > + > return NULL; > } > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index 82027cc9abf..c536eb581a7 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -5146,16 +5146,26 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func, > > if (TYPE_P (*tp)) > { > - /* Walk into template args without looking through typedefs. */ > - if (tree ti = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (*tp)) > - WALK_SUBTREE (TI_ARGS (ti)); > - /* Don't look through typedefs; walk_tree_fns that want to look through > - typedefs (like min_vis_r) need to do that themselves. */ > - if (typedef_variant_p (*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 (*walk_subtrees_p == 1 && typedef_variant_p (*tp)) > { > + if (tree ti = TYPE_ALIAS_TEMPLATE_INFO (*tp)) > + WALK_SUBTREE (TI_ARGS (ti)); > *walk_subtrees_p = 0; > return NULL_TREE; > } > + > + if (tree ti = TYPE_TEMPLATE_INFO (*tp)) > + WALK_SUBTREE (TI_ARGS (ti)); > } > > /* Not one of the easy cases. We must explicitly go through the Jakub
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?
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 <jakub@redhat.com> > 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 <jason@redhat.com> > > 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 <typename T> > +struct B { typedef int size_type; }; > +struct S1 { B<A>::size_type foo () const { return 1; } }; > +struct S2 { B<A>::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 <typename T> > +struct B { typedef int size_type; }; > +struct S1 { B<A>::size_type foo () const { return 1; } }; > +struct S2 { B<A>::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 <jason@redhat.com> > 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 <typename T> > +struct B { typedef int size_type; }; > +struct S1 { B<A>::size_type foo () const { return 1; } }; > +struct S2 { B<A>::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
On Thu, 1 Apr 2021, Jakub Jelinek wrote: > 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. LGTM as well. I'd like to roll RC1 today around 2pm CEST, so I'm going to give this a round of testing myself (also noticing the corresponding trunk patch isn't in yet) So if you can manage to push it before this time that would be great, otherwise I guess I'm going to push it to 10. Richard. > > commit 123f254cce416a4d50445465b88af6d8e754dc5e > > Author: Jakub Jelinek <jakub@redhat.com> > > 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 <jason@redhat.com> > > > > 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 <typename T> > > +struct B { typedef int size_type; }; > > +struct S1 { B<A>::size_type foo () const { return 1; } }; > > +struct S2 { B<A>::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 <typename T> > > +struct B { typedef int size_type; }; > > +struct S1 { B<A>::size_type foo () const { return 1; } }; > > +struct S2 { B<A>::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 <jason@redhat.com> > > 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 <typename T> > > +struct B { typedef int size_type; }; > > +struct S1 { B<A>::size_type foo () const { return 1; } }; > > +struct S2 { B<A>::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 > >
--- gcc/cp/tree.c.jj 2021-01-04 10:25:49.102117545 +0100 +++ gcc/cp/tree.c 2021-01-07 12:43:17.674974823 +0100 @@ -5147,8 +5147,9 @@ cp_walk_subtrees (tree *tp, int *walk_su if (TYPE_P (*tp)) { /* Walk into template args without looking through typedefs. */ - if (tree ti = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (*tp)) - WALK_SUBTREE (TI_ARGS (ti)); + if (*walk_subtrees_p != 2) + if (tree ti = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (*tp)) + WALK_SUBTREE (TI_ARGS (ti)); /* Don't look through typedefs; walk_tree_fns that want to look through typedefs (like min_vis_r) need to do that themselves. */ if (typedef_variant_p (*tp)) --- gcc/cp/class.c.jj 2021-01-04 10:25:48.933119459 +0100 +++ gcc/cp/class.c 2021-01-07 12:50:51.723881933 +0100 @@ -1508,7 +1508,12 @@ static tree find_abi_tags_r (tree *tp, int *walk_subtrees, void *data) { if (!OVERLOAD_TYPE_P (*tp)) - return NULL_TREE; + { + if (TYPE_P (*tp) && *walk_subtrees == 1) + /* Tell cp_walk_subtrees not to walk into template args. */ + *walk_subtrees = 2; + return NULL_TREE; + } /* walk_tree shouldn't be walking into any subtrees of a RECORD_TYPE anyway, but let's make sure of it. */ --- gcc/testsuite/g++.dg/abi/abi-tag24.C.jj 2021-01-07 12:58:12.128942173 +0100 +++ gcc/testsuite/g++.dg/abi/abi-tag24.C 2021-01-07 12:58:47.995539911 +0100 @@ -0,0 +1,17 @@ +// PR c++/98481 +// { dg-do compile { target c++11 } } +inline namespace N __attribute ((__abi_tag__ ("myabi"))) +{ + struct A {}; +} +template <typename T> +struct B { typedef int size_type; }; +struct S1 { B<A>::size_type foo () const { return 1; } }; +struct S2 { B<A>::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" } }