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 <ja...@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 <ja...@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 <ja...@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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
