[Bug tree-optimization/107839] New: spurious "may be used uninitialized" warning while all uses are under "if (c)"

[Bug tree-optimization/107839] New: spurious "may be used uninitialized" warning while all uses are under "if (c)"

[vincent-gcc at vinc17 dot net]

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

            Bug ID: 107839
           Summary: spurious "may be used uninitialized" warning while all
                    uses are under "if (c)"
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vincent-gcc at vinc17 dot net
  Target Milestone: ---

Consider

int f (int);
void g (int c)
{
  int v;
  if (c)
    v = f(0);
  while (1)
    if (c)
      f(v + v);
}

$ gcc-test -O -Wmaybe-uninitialized -c tst2.c
tst2.c: In function ‘g’:
tst2.c:4:7: warning: ‘v’ may be used uninitialized [-Wmaybe-uninitialized]
    4 |   int v;
      |       ^

All uses of v are under "if (c)", so the warning is incorrect. Note that
replacing "v + v" by "v" makes the warning disappear.

This occurs with GCC 8.4.0 and above up to at least 13.0.0 20220906
(experimental) from the master branch. No warnings with GCC 6.5.0 and below.

Note to myself (to check once this bug is fixed): this testcase is derived from
tmd/binary32/hrcases.c (warning on variable t0).

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
   Last reconfirmed|                            |2022-11-23
             Blocks|                            |24639
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will have a look


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
[Bug 24639] [meta-bug] bug to track all Wuninitialized issues

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
We see

  <bb 2> [local count: 3508266]:
  if (c_4(D) != 0)
    goto <bb 3>; [33.00%]
  else
    goto <bb 10>; [67.00%]

  <bb 10> [local count: 2350538]:
  goto <bb 4>; [100.00%]

  <bb 4> [local count: 3508266]:
  # v_10 = PHI <v_5(D)(10), v_8(3)>
  _3 = (unsigned int) v_10;
  _12 = _3 * 2;
  _1 = (int) _12;

  <bb 5> [local count: 354334800]:
  if (c_4(D) != 0)
    goto <bb 7>; [66.33%]
  else
    goto <bb 8>; [33.67%]

it's loop invariant motion that hoists the v + v compute out of the loop
and thus outside of its controlling condition.  You can see it's careful
to not introduce undefined overflow that is possibly conditionally
executed only but it fails to consider the case of 'v' being conditionally
uninitialized.

It's very difficult to do the right thing here - it might be tempting to
hoist the compute as

  if (c)
    tem = v+v;
  while (1)
    if (c)
      f(tem);

but apart from the technical problems in invariant motion this would
cause it does introduce another variable that's only conditionally
initialized and thus might be prone to false positive diagnostics.
Not to mention the hoisted if (c) branch having a cost.

Maybe the simplest thing would be to never hoist v + v, or only
hoist it when the controlling branch is not loop invariant.

The original testcase is probably more "sensible", does it still have
a loop invariant controlling condition and a loop invariant computation
under that control?

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[vincent-gcc at vinc17 dot net]

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

--- Comment #3 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> ---
(In reply to Richard Biener from comment #2)
> it's loop invariant motion that hoists the v + v compute out of the loop
> and thus outside of its controlling condition.  You can see it's careful
> to not introduce undefined overflow that is possibly conditionally
> executed only but it fails to consider the case of 'v' being conditionally
> uninitialized.
> 
> It's very difficult to do the right thing here - it might be tempting to
> hoist the compute as
> 
>   if (c)
>     tem = v+v;
>   while (1)
>     if (c)
>       f(tem);

Couldn't the -Wmaybe-uninitialized warning be disabled on hoisted code, so that
the controlling condition wouldn't be needed?

To make sure not to disable potential warnings, the information that v was used
for tem should be kept together with tem in the loop. Something like
((void)v,tem), though GCC doesn't currently warn on that if v is uninitialized
(but that's another issue that should be solved).

However...

> Maybe the simplest thing would be to never hoist v + v, or only
> hoist it when the controlling branch is not loop invariant.
> 
> The original testcase is probably more "sensible", does it still have
> a loop invariant controlling condition and a loop invariant computation
> under that control?

In my tmd/binary32/hrcases.c file, there doesn't seem to be a loop invariant,
so I'm wondering what is the real cause. The code looks like the following:

static inline double cldiff (clock_t t1, clock_t t0)
{
  return (double) (t1 - t0) / CLOCKS_PER_SEC;
}

and in a function hrsearch() where its mprog argument (named c above) is an
integer that enables progress output when it is nonzero:

  if (mprog)
    {
      mctr = 0;
      nctr = 0;
      t0 = ti = clock ();
    }

  do
    {
[...]
      if (mprog && ++mctr == mprog)
        {
          mctr = 0;
          tj = clock ();
          mpfr_fprintf (stderr, "[exponent %ld: %8.2fs %8.2fs  %5lu / %lu]\n",
                        e, cldiff (tj, ti), cldiff (tj, t0), ++nctr, nprog);
          ti = tj;
        }
[...]
    }
  while (mpfr_get_exp (x) < e + 2);

The warning I get is

In function ‘cldiff’,
    inlined from ‘hrsearch’ at hrcases.c:298:11,
    inlined from ‘main’ at hrcases.c:520:9:
hrcases.c:46:23: warning: ‘t0’ may be used uninitialized
[-Wmaybe-uninitialized]
   46 |   return (double) (t1 - t0) / CLOCKS_PER_SEC;
      |                   ~~~~^~~~~
hrcases.c: In function ‘main’:
hrcases.c:128:11: note: ‘t0’ was declared here
  128 |   clock_t t0, ti, tj;
      |           ^~

So the operation on t0 is tj - t0, and as tj is set just before, I don't see
how it can be used in a loop invariant.

This can be simplified as follows:

int f (int);
void g (int mprog)
{
  int t0, ti, tj;

  if (mprog)
    t0 = ti = f(0);

  do
    if (mprog)
      {
        tj = f(0);
        f(tj - ti);
        f(tj - t0);
        ti = tj;
      }
  while (f(0));
}

and I get

tst.c: In function ‘g’:
tst.c:13:9: warning: ‘t0’ may be used uninitialized [-Wmaybe-uninitialized]
   13 |         f(tj - ti);
      |         ^~~~~~~~~~
tst.c:4:7: note: ‘t0’ was declared here
    4 |   int t0, ti, tj;
      |       ^~

BTW, the warning is incorrect: I can't see t0 in "f(tj - ti);".

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[cvs-commit at gcc dot gnu.org]

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

--- 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:44c8402d35160515b3c09fd2bc239587e0c32a2b

commit r13-4491-g44c8402d35160515b3c09fd2bc239587e0c32a2b
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Dec 2 14:52:20 2022 +0100

    tree-optimization/107833 - invariant motion of uninit uses

    The following fixes a wrong-code bug caused by loop invariant motion
    hoisting an expression using an uninitialized value outside of its
    controlling condition causing IVOPTs to use that to rewrite a defined
    value.  PR107839 is a similar case involving a bogus uninit diagnostic.

            PR tree-optimization/107833
            PR tree-optimization/107839
            * cfghooks.cc: Include tree.h.
            * tree-ssa-loop-im.cc (movement_possibility): Wrap and
            make stmts using any ssa_name_maybe_undef_p operand
            to preserve execution.
            (loop_invariant_motion_in_fun): Call mark_ssa_maybe_undefs
            to init maybe-undefined status.
            * tree-ssa-loop-ivopts.cc (ssa_name_maybe_undef_p,
            ssa_name_set_maybe_undef, ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Move ...
            * tree-ssa.cc: ... here.
            * tree-ssa.h (ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Declare.
            (ssa_name_maybe_undef_p, ssa_name_set_maybe_undef): Define.

            * gcc.dg/torture/pr107833.c: New testcase.
            * gcc.dg/uninit-pr107839.c: Likewise.

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |13.0

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk.

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:812847a9d12c0b65695cbe1a23959b69a7e62355

commit r12-8977-g812847a9d12c0b65695cbe1a23959b69a7e62355
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Dec 2 14:52:20 2022 +0100

    tree-optimization/107833 - invariant motion of uninit uses

    The following fixes a wrong-code bug caused by loop invariant motion
    hoisting an expression using an uninitialized value outside of its
    controlling condition causing IVOPTs to use that to rewrite a defined
    value.  PR107839 is a similar case involving a bogus uninit diagnostic.

            PR tree-optimization/107833
            PR tree-optimization/107839
            * cfghooks.cc: Include tree.h.
            * tree-ssa-loop-im.cc (movement_possibility): Wrap and
            make stmts using any ssa_name_maybe_undef_p operand
            to preserve execution.
            (loop_invariant_motion_in_fun): Call mark_ssa_maybe_undefs
            to init maybe-undefined status.
            * tree-ssa-loop-ivopts.cc (ssa_name_maybe_undef_p,
            ssa_name_set_maybe_undef, ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Move ...
            * tree-ssa.cc: ... here.
            * tree-ssa.h (ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Declare.
            (ssa_name_maybe_undef_p, ssa_name_set_maybe_undef): Define.

            * gcc.dg/torture/pr107833.c: New testcase.
            * gcc.dg/uninit-pr107839.c: Likewise.

    (cherry picked from commit 44c8402d35160515b3c09fd2bc239587e0c32a2b)

[Bug tree-optimization/107839] spurious "may be used uninitialized" warning while all uses are under "if (c)"

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.3