[Bug c++/104031] New: [12 regression] Global nested constructors generate invalid code.

[Bug c++/104031] New: [12 regression] Global nested constructors generate invalid code.

[slyfox at gcc dot gnu.org]

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

            Bug ID: 104031
           Summary: [12 regression] Global nested constructors generate
                    invalid code.
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

Extracted from original example of nix-2.4 where global constructors register
invalid primops. Single-file example:

// $ cat main.cc
#include <string>
#include <vector>

struct Info {
    std::vector<std::string> args;
    size_t arity = 0;
};

struct RegisterPrimOp
{
    RegisterPrimOp(Info && info) __attribute__((noipa, noinline)) {
        if (info.arity != 0)
            __builtin_trap();
    }
};

static RegisterPrimOp s_op({
    .args = {"path"},
    .arity = 0,
});

int main() {}

# ok:
$ g++-11.2.0 main.cc -o main -O2 && ./main
# bad:
$ g++-12.0.0 main.cc -o main -O2 && ./main
Illegal instruction (core dumped)

Using built-in specs.
COLLECT_GCC=/<<NIX>>/gcc-12.0.0/bin/g++
COLLECT_LTO_WRAPPER=/<<NIX>>/gcc-12.0.0/libexec/gcc/x86_64-unknown-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with:
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20220109 (experimental) (GCC)

Must be a recent regression. Possibly a close sibling of diagnostic false
positive: https://gcc.gnu.org/PR103984

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-01-14
     Ever confirmed|0                           |1
   Target Milestone|---                         |12.0
            Summary|[12 regression] Global      |[12 regression] Global
                   |nested constructors         |nested constructors
                   |generate invalid code.      |generate invalid code since
                   |                            |r12-6329-g4f6bc28fc7dd86bd
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P1
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org
           Keywords|                            |wrong-code

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Also started with r12-6329-g4f6bc28fc7dd86bd.

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I can remove std::string but not figure out how to remove std::vector yet:
#include <vector>
#include <stddef.h>

struct string
{
  string(int){}
};

struct allocator
{
  allocator(){}
};

struct vector
{
  vector(allocator t = allocator{}){}
  vector(string, allocator t = allocator{}){}
  vector(std::initializer_list<string>&, allocator t = allocator{}){}
};

#if 1
typedef std::vector<string> t;
#else
typedef vector t;
#endif

struct Info {
    t args;
    int arity;
};

struct RegisterPrimOp
{
    RegisterPrimOp(Info && info) __attribute__((noipa, noinline)) {
        if (info.arity != 0)
            __builtin_trap();
    }
};

static RegisterPrimOp s_op({
    .args = {1},
    .arity = 0,
});

int main() {}

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Reduced testcase without any headers:

struct vector
{
  vector(){}  ~vector(){}
};
struct Info {
    vector args;
    int arity = 0;
};
struct RegisterPrimOp
{
    [[gnu::noipa, gnu::noinline]]
    RegisterPrimOp(Info info) {
        if (info.arity != 0)
            __builtin_trap();
    }
};
static RegisterPrimOp s_op({
    .args = vector{},
    .arity = 0,
});
int main() {}

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[slyfox at gcc dot gnu.org]

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

--- Comment #4 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Great test, Andrew!

Something is completely dropped initialization of Info{} input argument to
s_op. As if it's lifetime ends before RegisterPrimOp{} enters:

--- main.s.good 2022-01-14 21:53:42.334571321 +0000
+++ main.s.bad  2022-01-14 21:53:51.275722971 +0000
@@ -43,26 +43,25 @@
        .p2align 4
        .type   _GLOBAL__sub_I_main, @function
 _GLOBAL__sub_I_main:
 .LFB14:
        .cfi_startproc
        subq    $24, %rsp       #,
        .cfi_def_cfa_offset 32
 # ../main.cc:21: });
        movl    $_ZL4s_op, %edi #,
        leaq    8(%rsp), %rsi   #, tmp84
-       movl    $0, 12(%rsp)    #, D.2496.arityD.2380
        call    _ZN14RegisterPrimOpC1E4Info     #
 # ../main.cc:22: int main() {}
        addq    $24, %rsp       #,
        .cfi_def_cfa_offset 8
        ret
        .cfi_endproc
 .LFE14:
        .size   _GLOBAL__sub_I_main, .-_GLOBAL__sub_I_main
        .section        .init_array,"aw"
        .align 8
        .quad   _GLOBAL__sub_I_main
        .local  _ZL4s_op
        .comm   _ZL4s_op,1,1
-       .ident  "GCC: (GNU) 11.2.0"
+       .ident  "GCC: (GNU) 12.0.0 20220109 (experimental)"
        .section        .note.GNU-stack,"",@progbits

Initial gimple looks valid to me: main.cc.006t.gimple:

void __static_initialization_and_destruction_0 (int __initialize_p, int
__priority)
{
  struct InfoD.2399 D.2453 = {.arityD.2402=0};
  ...
  _ZN6vectorC1EvD.2377 (&D.2453.argsD.2401);
  ...
  try { try { try {
    # USE = anything
    # CLB = anything
    _ZN14RegisterPrimOpC1E4InfoD.2413 (&s_opD.2424, &D.2453);
                  } finally {
      # USE = anything
      # CLB = anything
      _ZN4InfoD1EvD.2457 (&D.2453);
       } } finally {
    D.2453 = {CLOBBER};
       }

Final tree also looks ok: main.cc.250t.optimized:

voidD.48 _GLOBAL__sub_I_mainD.2486 ()
{
  struct InfoD.2399 D.2522 = {.arityD.2402=0};
  # USE = nonlocal escaped { D.2424 D.2522 } (nonlocal, escaped)
  # CLB = nonlocal escaped { D.2424 D.2522 } (nonlocal, escaped)
  _ZN14RegisterPrimOpC1E4InfoD.2413 (&_ZL4s_opD.2424, &D.2522);
  D.2522 ={v} {CLOBBER};

First RTL seems to lack the assignment: main.cc.252r.expand:

;; Function _GLOBAL__sub_I_main (_GLOBAL__sub_I_main, funcdef_no=14,
decl_uid=2486, cgraph_uid=15, symbol_order=15) (executed once)
...
;;
;; Full RTL generated for this function:
;;
      1: NOTE_INSN_DELETED
      3: NOTE_INSN_BASIC_BLOCK 2
      2: NOTE_INSN_FUNCTION_BEG
    5: {r82:DI=r77:DI-0x8;clobber flags:CC;}
    6: si:DI=r82:DI
    7: di:DI=`_ZL4s_op'
    8: call [`_ZN14RegisterPrimOpC1E4Info'] argc:0
      REG_CALL_DECL `_ZN14RegisterPrimOpC1E4Info'
      REG_EH_REGION 0

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[slyfox at gcc dot gnu.org]

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

--- Comment #5 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
gcc-11 for comparison did not seem to have `struct InfoD.2399 D.2453 =
{.arityD.2402=0};` style nodes and encoded stores explicitly:
main.cc.244t.optimized:

voidD.48 _GLOBAL__sub_I_main ()
{
  struct InfoD.2377 D.2496;
  ...
  D.2496.arityD.2380 = 0;

and produced expected RTL: main.cc.245r.expand:

;; Function _GLOBAL__sub_I_main (_GLOBAL__sub_I_main, funcdef_no=14,
decl_uid=2463, cgraph_uid=15, symbol_order=15) (executed once)
...
;;
;; Full RTL generated for this function:
;;
      1: NOTE_INSN_DELETED
      3: NOTE_INSN_BASIC_BLOCK 2
      2: NOTE_INSN_FUNCTION_BEG

    5: [r77:DI-0x4]=0 // <<<--- our lost store

    6: {r82:DI=r77:DI-0x8;clobber flags:CC;}
    7: si:DI=r82:DI
    8: di:DI=`_ZL4s_op'
    9: call [`_ZN14RegisterPrimOpC1E4Info'] argc:0
      REG_CALL_DECL `_ZN14RegisterPrimOpC1E4Info'
      REG_EH_REGION 0

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[slyfox at gcc dot gnu.org]

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

--- Comment #6 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
> void __static_initialization_and_destruction_0 (int __initialize_p, int
> __priority)
> {
>   struct InfoD.2399 D.2453 = {.arityD.2402=0};

Having poked at -fdump-tree-all-raw I now think `= {.arityD.2402=0};` is not a
variable initialization itself, but a reference to `DECL_INITIAL` that would do
an initialization.

Perhaps something was supposed to expand this `DECL_INITIAL` to an actual
initializing statement, but did not for this case.

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
My guess is that the problem is that when the TARGET_EXPR_SLOT is created using
build_local_temp, current_function_decl is NULL, it is in s_op global namespace
var initializer.
Eventually we add it to ssdf and call in the new r12-6329 spot
split_nonconstant_init, which has:
743           if (VAR_P (dest) && !is_local_temp (dest))
744             {
745               DECL_INITIAL (dest) = init;
746               TREE_READONLY (dest) = 0;
747             }
but due to the still unchanged DECL_CONTEXT being NULL is_local_temp returns
false and we set DECL_INITIAL instead of what the code expects.
The temporary gets DECL_CONTEXT set only during gimplification.
So, I think either we should fill in TARGET_EXPR_SLOT's DECL_CONTEXT when we
are adding those exprs into ssdf, or the r12-6329 code should ensure the slot
has non-NULL DECL_CONTEXT if current_function_decl or something similar.

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That last testcase isn't very good for the testsuite, because 0 is pretty
common value on the stack, so even without the store the chances that it will
be already zero are high.

42 is less likely...

// PR c++/104031
// { dg-do run { target c++14 } }
// { dg-options "-O2" }

struct A {
  A () {}
  ~A () {}
};
struct B {
  A a;
  int b = 0;
};
struct C
{
  [[gnu::noipa]]
  C (B x) { if (x.b != 42) __builtin_abort (); }
};
static C c ({ .a = A{}, .b = 42 });

int
main ()
{
}

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[jakub at gcc dot gnu.org]

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

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

This seems to work for the testcase, but dunno if there aren't better fixes.

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> That last testcase isn't very good for the testsuite, because 0 is pretty
> common value on the stack, so even without the store the chances that it
> will be already zero are high.

Yes I didn't think of that while reducing it further. But that explains why it
would pass at -O0 really :)

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:aeca44768d54b089243004d1ef00d34dfa9f6530

commit r12-6643-gaeca44768d54b089243004d1ef00d34dfa9f6530
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Jan 17 18:10:34 2022 +0100

    c++: Fix cp_genericize_target_expr for TARGET_EXPRs created for global
initialization [PR104031]

    The following patch is miscompiled, cp_genericize_target_expr expects
    that for the constant part split_nonconstant_init will emit an INIT_EXPR
    that will initialize it, but that doesn't happen and instead we get
    DECL_INITIAL on the TARGET_EXPR_SLOT that isn't initialized anywhere
    in the IL.

    The problem is that the TARGET_EXPR has been created while
    current_function_decl was NULL, it is inside a global var initializer.
    That means the build_local_temp created VAR_DECL has NULL DECL_CONTEXT.
    Later on when genericizing the ssdf (current_function_decl is already
    non-NULL), the new cp_genericize_target_expr is called and during
    split_nonconstant_init it checks is_local_temp, but that due to the NULL
    DECL_CONTEXT returns false.  DECL_CONTEXT is set only later on during
    gimplification.

    The following patch fixes it by setting DECL_CONTEXT also inside of
    cp_genericize_target_expr, which fixes the testcase.  But if there are
    better spots to do that, please let me know...

    2022-01-17  Jakub Jelinek  <jakub@redhat.com>

            PR c++/104031
            * cp-gimplify.c (cp_genericize_target_expr): Set DECL_CONTEXT of
            TARGET_EXPR_SLOT to current_function_decl if it was NULL.

            * g++.dg/cpp1y/pr104031.C: New test.

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[jakub at gcc dot gnu.org]

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

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

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

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

[Bug c++/104031] [12 regression] Global nested constructors generate invalid code since r12-6329-g4f6bc28fc7dd86bd

[slyfox at gcc dot gnu.org]

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

--- Comment #13 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
The change also fixed original nix-2.4 test failure. Thank you!