[Bug c++/99456] New: [11 regression] ABI breakage with some static initialization

[Bug c++/99456] New: [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

            Bug ID: 99456
           Summary: [11 regression] ABI breakage with some static
                    initialization
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nathan at gcc dot gnu.org
  Target Milestone: ---

Created attachment 50326
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50326&action=edit
testcase

I've noticed some poorer behaviour in trunk than gcc10 with emitting inline
vars.  The example when compiled with gcc-10 -std=c++17 emits only those vars
that are reachable from the externally visible vars.  I.e. Var1...Var4 are NOT
present.  Further all vars are statically initialized with no global
initializer function.
(This is regardless of optimization level).

With trunk, all the inline vars are emitted, even when not referenced. 
Further, there is a gloabl initializer function emitted, that only tests and
sets their respective guard variables, with two exceptions.

Var3 and Var13 are dynamically initialized. This is an abi breakage.

I attach the two assembly files generated (the source presumes LP32 or LP64
ABI)

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #1 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
Created attachment 50327
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50327&action=edit
gcc-10 output (unoptimized)

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #2 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
Created attachment 50328
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50328&action=edit
trunk output (unoptimized)

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI
   Target Milestone|---                         |11.0

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
           Priority|P3                          |P1

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This changed with my r11-893-geeb54a14c48f543857f561556ab1fc49dc21af26
Though, it is unclear why, because REINTERPRET_CAST_P isn't set on those
NOP_EXPRs.
I don't understand why it is an ABI issue though.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #4 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
It's an ABI issue, because all compilers must agree on which parts of an inline
object are dynamically initialized.  (gcc-11 does not agree with gcc-10).

consider:

inline Type Var = Expr;

That'll be emitted as comdat in every TU that needs it.  Assume some of those
TUs generate a static init and others generate a dynamic init.  Let's say it's
a dynamic one that gets into the executable, but one of the static TUs accesses
Var before that dyn init has run.  Boom, zero-initialized entity observed.

Or, let's say a static one wins, but then a dynmic initializer runs
concurrently to a (static TU's) access.  Oops, we could observe (weird) partial
writes.  (this would require Var to be a function-scope static because global
inits run in a single-thread environment, and it's less likelu to be a
problem).

In case it's not clear, this example shows both the above happening, and poor
code generation with (a) unused inline vars emitting code and (b) guard
variables for inline vars with no dynamic init.

Hope that helps.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50330
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50330&action=edit
gcc11-pr99456.patch

On one side, we have still accepts-invalid issue, e.g. in your testcase:
constexpr inline Container Var1 {reinterpret_cast<uintptr_t> (&descRef)};
should be rejected because reinterpret_cast is not allowed in constant
expression, but we accept it, because we have REINTERPRET_CAST_P flag only on
NOP_EXPRs and not CONVERT_EXPRs that are emitted in this case.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Another thing is that perhaps we should be rejecting reinterpret_cast only in
the
pedantic constant expression evaluation mode, not when we allow extensions and
fold as much as we can.  So something like (incremental):
--- gcc/cp/constexpr.c  2021-03-08 13:08:49.944229227 +0100
+++ gcc/cp/constexpr.c  2021-03-08 14:23:01.263716186 +0100
@@ -6629,11 +6629,9 @@

     case NOP_EXPR:
     case CONVERT_EXPR:
-      if (REINTERPRET_CAST_P (t))
+      if (REINTERPRET_CAST_P (t) && !ctx->quiet)
        {
-         if (!ctx->quiet)
-           error_at (loc,
-                     "%<reinterpret_cast%> is not a constant expression");
+         error_at (loc, "%<reinterpret_cast%> is not a constant expression");
          *non_constant_p = true;
          return t;
        }
@@ -8103,13 +8101,6 @@

     case NOP_EXPR:
     case CONVERT_EXPR:
-      if (REINTERPRET_CAST_P (t))
-       {
-         if (flags & tf_error)
-           error_at (loc, "%<reinterpret_cast%> is not a constant
expression");
-         return false;
-       }
-      /* FALLTHRU */
     case VIEW_CONVERT_EXPR:
       /* -- a reinterpret_cast.  FIXME not implemented, and this rule
         may change to something more specific to type-punning (DR 1312).  */

But that doesn't make these inline vars statically initialized either, really
confused on what's going on.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #7 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
You're misreading this particular testcase.  I don't believe it contains
anything invalid -- the only constexpr is on:

static constexpr unsigned &descRef = desc;

(and I don't think it's necessary there, but that's unimportant)

the inline vars are explicitly not constinit/constexpr, but one would expect
static initialization for them as they all boil down to symbol refs.


it is 99452 showing the inconsistent handling of constinit

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'm not misreading the testcase, I'm just reacting on the fact that my patch
changed the behavior of it and looking into related issues when analyzing why.
It is up to the compiler to decide if it can or can't optimize it.
The strange thing is that before my patch, it was actually triggerring the
now removed cxx_eval_outermost_constant_expression code:
  /* Technically we should check this for all subexpressions, but that
     runs into problems with our internal representation of pointer
     subtraction and the 5.19 rules are still in flux.  */
  if (CONVERT_EXPR_CODE_P (TREE_CODE (r))
      && ARITHMETIC_TYPE_P (TREE_TYPE (r))
      && TREE_CODE (TREE_OPERAND (r, 0)) == ADDR_EXPR)
    {
      if (!allow_non_constant)
       error ("conversion from pointer type %qT "
              "to arithmetic type %qT in a constant expression",
              TREE_TYPE (TREE_OPERAND (r, 0)), TREE_TYPE (r));
      non_constant_p = true;
    }
but with allow_non_constant false.  Which means the call didn't return
(uintptr_t) &desc (the folded expression), but
(uintptr_t) (unsigned int *) descRef
(an unfolded one).  And strangely that one then led to no dynamic
initialization,
while the folded one doesn't.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-03-08
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50331
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50331&action=edit
gcc11-pr99456.patch

Actually, this patch fixes this.  For GCC12 we can play with the other patches.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50330|0                           |1
        is obsolete|                            |

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50332
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50332&action=edit
gcc12-pr99456.patch

Untested patch for GCC 12.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[nathan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #11 from Nathan Sidwell <nathan at gcc dot gnu.org> ---
gcc11-pr99456.patch looks good, can we add a scan-not for the _ZGV guard
variables too?  If the optimizer's turned on, I think __static_init... gets
inlined into the global constructor, might be good to make sure that's not
there either.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Unfortunately the #c9 patch caused
+FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++14  (test for errors, line 9)
+FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++17  (test for errors, line 9)
+FAIL: g++.dg/cpp1y/constexpr-82304.C  -std=c++2a  (test for errors, line 9)
+FAIL: g++.dg/ext/vla22.C  -std=gnu++11 (test for excess errors)
+FAIL: g++.dg/ext/vla22.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/ext/vla22.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/ext/vla22.C  -std=gnu++2a (test for excess errors)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++11  (test for errors, line 15)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++11  (test for errors, line 17)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++14  (test for errors, line 15)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++14  (test for errors, line 17)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++17  (test for errors, line 15)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++17  (test for errors, line 17)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++2a  (test for errors, line 15)
+FAIL: g++.dg/template/pr79650.C  -std=gnu++2a  (test for errors, line 17)
regressions.

I think the first one is about the non-strict optimizations being applied even
to initializers of variables of automatic variables in constexpr functions.
I guess we need to avoid doing that before we copy the body for constexpr
evaluation purposes.

vla22.C emits extra -Wnarrowing warning on the invalid code, dunno, perhaps it
is ok to adjust the testcase?

And pr79650.C is yet another thing, in that case it is about non-type template
argument and the expected error disappearing there.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50331|0                           |1
        is obsolete|                            |

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50337
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50337&action=edit
gcc11-pr99456.patch

Actually checking ctx->manifestly_const_eval instead of !ctx->quiet fixes two
of the three FAILs and perhaps the narrowing conversion warning is acceptable.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:82bb66730bc42b8694fdebef607ea6e49e8496bf

commit r11-7740-g82bb66730bc42b8694fdebef607ea6e49e8496bf
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 18:36:56 2021 +0100

    c++: Only reject reinterpret casts from pointers to integers for
manifestly_const_eval evaluation [PR99456]

    My PR82304/PR95307 fix moved reinterpret cast from pointer to integer
    diagnostics from cxx_eval_outermost_constant_expr where it caught
    invalid code only at the outermost level down into
    cxx_eval_constant_expression.
    Unfortunately, it regressed following testcase, we emit worse code
    including dynamic initialization of some vars.
    While the initializers are not constant expressions due to the
    reinterpret_cast in there, there is no reason not to fold them as an
    optimization.

    I've tried to make this dependent on !ctx->quiet, but that regressed
    two further tests, and on ctx->strict, which regressed other tests,
    so this patch bases that on manifestly_const_eval.

    The new testcase is now optimized as much as it used to be in GCC 10
    and the only regression it causes is an extra -Wnarrowing warning
    on vla22.C test on invalid code (which the patch adjusts).

    2021-03-19  Jakub Jelinek  <jakub@redhat.com>

            PR c++/99456
            * constexpr.c (cxx_eval_constant_expression): For CONVERT_EXPR from
            INDIRECT_TYPE_P to ARITHMETIC_TYPE_P, when
!ctx->manifestly_const_eval
            don't diagnose it, set *non_constant_p nor return t.

            * g++.dg/opt/pr99456.C: New test.
            * g++.dg/ext/vla22.C: Expect a -Wnarrowing warning for c++11 and
            later.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug c++/99456] [11 regression] ABI breakage with some static initialization

[unlvsur at live dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99456

cqwrteur <unlvsur at live dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |unlvsur at live dot com

--- Comment #16 from cqwrteur <unlvsur at live dot com> ---
(In reply to Jakub Jelinek from comment #15)
> Fixed.

Hi Jakub. Is this a related issue?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99677