[Bug c++/104373] New: [12 regression] bogus -Wmaybe-uninitialized warning with array new

[Bug c++/104373] New: [12 regression] bogus -Wmaybe-uninitialized warning with array new

[sss@li-snyder.org]

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

            Bug ID: 104373
           Summary: [12 regression] bogus -Wmaybe-uninitialized warning
                    with array new
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sss@li-snyder.org
  Target Milestone: ---

hi -

With a recent checkout of gcc12 (20220203), on a x86_64-pc-linux-gnu host,
the following source gives bogus -Wmaybe-uninitialized warnings
with -Wall:

--------------------------------------------------------------
void* operator new[](unsigned long, void* __p);

struct allocator
{
  ~allocator();
};


void *foo (void *p)
{
  return p ? new(p) allocator[1] : new allocator[1];
}
--------------------------------------------------------------

$ g++ -Wall -c gccbug.cc
gccbug.cc: In function ‘void* foo(void*)’:
gccbug.cc:11:51: warning: ‘<anonymous>’ may be used uninitialized
[-Wmaybe-uninitialized]
   11 |   return p ? new(p) allocator[1] : new allocator[1];
      |                                                   ^
gccbug.cc:11:51: note: ‘<anonymous>’ was declared here
   11 |   return p ? new(p) allocator[1] : new allocator[1];
      |                                                   ^
gccbug.cc:11:32: warning: ‘<anonymous>’ may be used uninitialized
[-Wmaybe-uninitialized]
   11 |   return p ? new(p) allocator[1] : new allocator[1];
      |                                ^
gccbug.cc:11:32: note: ‘<anonymous>’ was declared here
   11 |   return p ? new(p) allocator[1] : new allocator[1];
      |                                ^


>From git bisect, this appears to have been introduced by this commit:

commit beaee0a871b6485d20573fe050b1fd425581e56a (HEAD)
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Jan 1 16:00:22 2022 -0500

    c++: temporary lifetime with array aggr init [PR94041]

    The previous patch fixed temporary lifetime for aggregate initialization of
    classes; this one extends that fix to arrays.  This specifically reverses
my
    r74790, the patch for PR12253, which was made wrong when these semantics
    were specified in DR201.

    Since the array cleanup region encloses the regions for any temporaries, we
    don't need to add an additional region for the array object itself in
either
    initialize_local_var or split_nonconstant_init; we do, however, need to
tell
    split_nonconstant_init how to disable the cleanup once an enclosing object
    is fully constructed, at which point we want to run that destructor
instead.


FWIW, the warning goes away if the conditional expression in foo()
is rewritten as an explicit if statement.

[Bug c++/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
           Keywords|                            |diagnostic

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c++                         |tree-optimization
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-02-04

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed. Though I am not 100% sure how to get this fixed. The warning is 100%
bogus as the basic block where it is being warned about can never be reached.



  if (cleanup.5_20 != 0)
    goto <bb 16>; [INV]
  else
    goto <bb 20>; [INV]

  <bb 16> :
  if (_51(D) != 0B)
    goto <bb 17>; [INV]
  else
    goto <bb 20>; [INV]

Only path which is bb 16 is reached is when cleanup.5_20 is non-zero and
cleanup.5_20 is defined as:
  cleanup.5_20 = 0;

So in theory the uninitialize warning pass should have detected that and not
warned.

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
When not optimizing we intentionally warn about only conditionally executed
cases early - and not optimizing means we do not detect trivially unreachable
paths like this.

I don't know whether we have good enough infrastructure now to have a
way to determine that after discovering a possible uninit but it would
be possible to fix this particular instance by walking BBs in RPO order
and keeping a very simple const/copy lattice to avoid traversing along
unreachable edges.

The non-iterative VN algorithm does this for example.  In theory you could
also use that, you can specify eliminate == false so it doesn't modify the
IL and if you then add a callback per reachable stmt (or BB) you'd get
even more fancy cases handled (at compile-time cost of course).

The other early diagnostic passes might also benefit from that.  Alternatively
you could just initialize EDGE_EXECUTABLE/BB_REACHABLE from it before
the early diagnostic passes and then rely on that during those.

Note it's all non-IPA though and the early diagnostic passes should maybe
run from the local optimization pipeline, before early_inline (maybe
after inlining always-inline though?), so they can benefit from local IPA
analysis done on called functions.

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 52347
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52347&action=edit
patch

Like this, just convering uninit diagnostics and not exercising the other idea
of local IPA or always-inline.

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:0f58ba4dd6b25b16d25494ae18d15dfa681f9b65

commit r12-7175-g0f58ba4dd6b25b16d25494ae18d15dfa681f9b65
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Feb 4 09:46:43 2022 +0100

    tree-optimization/104373 - early diagnostic on unreachable code

    The following improves early uninit diagnostics by computing edge
    reachability using VN and ignoring unreachable blocks when looking
    for uninitialized uses.  To not ICE with -fdump-tree-all the
    early uninit pass needs a dumpfile since VN tries to dump statistics.

    2022-02-04  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/104373
            * tree-ssa-sccvn.h (do_rpo_vn): New export exposing the
            walk kind.
            * tree-ssa-sccvn.cc (do_rpo_vn): Export, get the default
            walk kind as argument.
            (run_rpo_vn): Adjust.
            (pass_fre::execute): Likewise.
            * tree-ssa-uninit.cc (warn_uninitialized_vars): Skip
            blocks not reachable.
            (execute_late_warn_uninitialized): Mark all edges as
            executable.
            (execute_early_warn_uninitialized): Use VN to compute
            executable edges.
            (pass_data_early_warn_uninitialized): Enable a dump file,
            change dump name to warn_uninit.

            * g++.dg/warn/Wuninitialized-32.C: New testcase.
            * gcc.dg/uninit-pr20644-O0.c: Remove XFAIL.

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
This particular case should be fixed now.

[Bug tree-optimization/104373] [12 regression] bogus -Wmaybe-uninitialized warning with array new

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:50243f4918c2ed7f1ddbf0e8df97a37aee73ebf2

commit r12-7188-g50243f4918c2ed7f1ddbf0e8df97a37aee73ebf2
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Feb 11 00:27:11 2022 +0100

    testsuite: Fix up g++.dg/warn/Wuninitialized-32.C test for ilp32 [PR104373]

    The testcase FAILs whenever size_t is not unsigned long:
    FAIL: g++.dg/warn/Wuninitialized-32.C  -std=c++98 (test for excess errors)
    Excess errors:
    .../gcc/testsuite/g++.dg/warn/Wuninitialized-32.C:4:7: error: 'operator
new' takes type 'size_t' ('unsigned int') as first parameter [-fpermissive]

    Fixed by using __SIZE_TYPE__ instead of unsigned long.

    2022-02-11  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/104373
            * g++.dg/warn/Wuninitialized-32.C (operator new[]): Use
__SIZE_TYPE__
            as type of the first argument instead of unsigned long.