On 8/6/25 3:34 PM, Patrick Palka wrote:
On Tue, 5 Aug 2025, Jason Merrill wrote:

On 7/31/25 10:51 AM, Patrick Palka wrote:
On Thu, 31 Jul 2025, Jason Merrill wrote:

On 7/30/25 5:48 PM, Patrick Palka wrote:
On Wed, 30 Jul 2025, Patrick Palka wrote:

On Wed, 30 Jul 2025, Patrick Palka wrote:

On Wed, 30 Jul 2025, Patrick Palka wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

-- >8 --

Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are
each
represented as a single-element CONSTRUCTOR with
CONSTRUCTOR_NO_CLEARING
cleared, and we end up mangling them all as A{1}, i.e. eliding
both
the
implicit initial and trailing zeros.  Mangling them all the same
seems
clearly wrong since they're logically different values.

Just realized that A::make(1, 0, 1) is also mangled incorrectly --
as
A{1, 1} instead of A{1, 0, 1}.  So we need to consider intermediate
implicit zeroes as well, not just initial zeroes...

Here's an updated patch that also considers intermediate implicit
zeros.

Bootstrap and regtest on x86_64-pc-linux-gnu in progress, does this
look
OK for
trunk if successful?

-- >8 --

Subject: [PATCH] c++: mangling cNTTP object w/ implicit non-trailing
zeros
    [PR121231]

Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are each
represented as a single-element CONSTRUCTOR with
CONSTRUCTOR_NO_CLEARING
cleared, and we end up mangling them all as A{1}, i.e. eliding both
the
implicit initial and trailing zeros.  Mangling them all the same seems
clearly wrong since they're logically different values.

It turns out we also omit intermediate zeros, e.g. A::make(1, 0, 1) is
mangled as A{1, 1} same as A::make(1, 1, 0).

It seems we can't omit both trailing and non-trailing implicit zeros
without introducing mangling ambiguities.  This patch makes us include
non-trailing zeros in these manglings, while continuing to omit
trailing zeros, which matches e.g. Clang.

        PR c++/121231

gcc/ChangeLog:

        * common.opt: Document additional ABI version 21 change.
        * doc/invoke.texi: Likewise.

gcc/cp/ChangeLog:

        * mangle.cc (write_expression): Write out implicit
non-trailing
        zeroes of a CONSTRUCTOR when the ABI version is at least 21.

gcc/testsuite/ChangeLog:

        * g++.dg/abi/mangle82.C: New test.
---
    gcc/common.opt                      |  2 +
    gcc/cp/mangle.cc                    | 49 ++++++++++++++++
    gcc/doc/invoke.texi                 |  5 +-
    gcc/testsuite/g++.dg/abi/mangle82.C | 86
+++++++++++++++++++++++++++++
    4 files changed, 140 insertions(+), 2 deletions(-)
    create mode 100644 gcc/testsuite/g++.dg/abi/mangle82.C

diff --git a/gcc/common.opt b/gcc/common.opt
index 70659fabebd5..bf38f60d194b 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1067,6 +1067,8 @@ Driver Undocumented
    ;
    ; 21: Fix noexcept lambda capture pruning.
    ;     Fix C++20 layout of base with all explicitly defaulted
constructors.
+;     Fix mangling of class and array objects with implicitly
+;     zero-initialized non-trailing subojects.
    ;     Default in G++ 16.
    ;
    ; Additional positive integers will be assigned as new versions of
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 13d5dedebd29..296e0f34552e 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3745,9 +3745,55 @@ write_expression (tree expr)
                      || !zero_init_expr_p (ce->value))
                    last_nonzero = i;
    +         tree prev_field = NULL_TREE;
+             if (TREE_CODE (TREE_TYPE (expr)) == RECORD_TYPE)
+               prev_field = first_field (etype);
+             else if (TREE_CODE (TREE_TYPE (expr)) == ARRAY_TYPE)
+               prev_field = size_int (0);

On second thought I think the logic is a little clearer if prev_field is
NULL_TREE on the first iteration, and we adjust the i == 0 logic
accordingly.  Like so (passes initial testisg, full testing in
progress):

-- >8 --

Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are each
represented as a single-element CONSTRUCTOR with CONSTRUCTOR_NO_CLEARING
cleared, and we end up mangling them all as A{1}, i.e. eliding both the
implicit initial and trailing zeros.  Mangling them all the same seems
clearly wrong since they're logically different values.

It turns out we also omit intermediate zeros, e.g. A::make(1, 0, 1) is
mangled as A{1, 1}, the same as A::make(1, 1, 0).

It seems we can't omit both trailing and non-trailing implicit zeros
without introducing mangling ambiguities.  This patch makes us include
non-trailing zeros in these manglings, while continuing to omit
trailing zeros, which matches e.g. Clang.

        PR c++/121231

gcc/ChangeLog:

        * common.opt: Document additional ABI version 21 change.
        * doc/invoke.texi: Likewise.

gcc/cp/ChangeLog:

        * mangle.cc (write_expression): Write out implicit non-trailing
        zeroes of a CONSTRUCTOR when the ABI version is at least 21.

gcc/testsuite/ChangeLog:

        * g++.dg/abi/mangle82.C: New test.
---
    gcc/common.opt                      |  2 +
    gcc/cp/mangle.cc                    | 50 +++++++++++++++++
    gcc/doc/invoke.texi                 |  5 +-
    gcc/testsuite/g++.dg/abi/mangle82.C | 86
+++++++++++++++++++++++++++++
    4 files changed, 141 insertions(+), 2 deletions(-)
    create mode 100644 gcc/testsuite/g++.dg/abi/mangle82.C

diff --git a/gcc/common.opt b/gcc/common.opt
index 70659fabebd5..bf38f60d194b 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1067,6 +1067,8 @@ Driver Undocumented
    ;
    ; 21: Fix noexcept lambda capture pruning.
    ;     Fix C++20 layout of base with all explicitly defaulted
constructors.
+;     Fix mangling of class and array objects with implicitly
+;     zero-initialized non-trailing subojects.
    ;     Default in G++ 16.
    ;
    ; Additional positive integers will be assigned as new versions of
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 13d5dedebd29..23b21c0a5b7a 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3745,11 +3745,58 @@ write_expression (tree expr)
                      || !zero_init_expr_p (ce->value))
                    last_nonzero = i;
    +         tree prev_field = NULL_TREE;
              if (undigested || last_nonzero != UINT_MAX)
                for (HOST_WIDE_INT i = 0; vec_safe_iterate (elts, i,
&ce);
++i)
                  {
                    if (i > last_nonzero)
                      break;
+                   if (!undigested && !CONSTRUCTOR_NO_CLEARING (expr)
+                       && (TREE_CODE (etype) == RECORD_TYPE
+                           || TREE_CODE (etype) == ARRAY_TYPE))
+                     {
+                       /* Write out any implicit non-trailing zeros
+                          (which we neglected to do before v21).  */
+                       if (TREE_CODE (etype) == RECORD_TYPE)
+                         {
+                           tree field;
+                           if (i == 0)
+                             field = first_field (etype);
+                           else
+                             field = DECL_CHAIN (prev_field);
+                           for (; field; field = DECL_CHAIN (field))
+                             {
+                               field = next_subobject_field (field);
+                               if (!field || field == ce->index)
+                                 break;
+                               if (abi_check (21))
+                                 write_expression (build_zero_cst
+                                                   (TREE_TYPE (field)));

This seems to assume that field will have scalar type, surely that isn't
necessarily true?

build_zero_cst also handles aggregate types by returning an empty
CONSTRUCTOR, which we in turn mangle as T{}.

Ah, indeed.  The patch is OK.

Thanks, pushed as r16-3046-g0a7eae02dea519 after noting a couple of
other PRs that are fixed by this.

I wonder on second thought whether this fix should be unconditional
instead of controlled by -fabi-version?  Mainly because the bug is
wrong-code, not just wrong mangling, since the identity of class NTTP
arguments is tied to their mangling (and so we can end up conflating
different specializations involving different class NTTP arguments).
If we fixed it unconditionally then we could backport the fix to 15.
I don't feel strongly about it but I thought I'd raise the idea.

I think let's keep it conditional.

Jason

Reply via email to