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 <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
