[Bug tree-optimization/97108] New: Wmaybe-uninitialized false positive

[Bug tree-optimization/97108] New: Wmaybe-uninitialized false positive

[clyon at gcc dot gnu.org]

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

            Bug ID: 97108
           Summary: Wmaybe-uninitialized false positive
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: clyon at gcc dot gnu.org
  Target Milestone: ---

This testcase causes a false positive 'may be used uninitialized' warning when
compiled with -O2 -Wall. It is derived from libiberty/pex-win32.c
=================================================
#define STDIN_FILENO 0
#define STDOUT_FILENO 1
#define STDERR_FILENO 2

int _close(int);

int
pex_win32_exec_child (int flags,
                      int in, int out, int errdes)
{
  int pid = flags;
  int orig_err;
  int separate_stderr = !(flags);

  if (separate_stderr)
    {
      orig_err = errdes;
    }

  if (pid != (int) -1)
    {
      if (in != STDIN_FILENO)
        _close (in);
      if (out != STDOUT_FILENO)
        _close (out);
      if (separate_stderr
          && orig_err != STDERR_FILENO)
        _close (orig_err);
    }

  return pid;
}
=================================================

../pex-win32.c: In function ‘pex_win32_exec_child’:
../pex-win32.c:27:23: warning: ‘orig_err’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
   27 |           && orig_err != STDERR_FILENO)
      |                       ^

[Bug tree-optimization/97108] Wmaybe-uninitialized false positive

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-11-06

--- Comment #1 from Jeffrey A. Law <law at redhat dot com> ---
So the key to this testcase is the merge points.  After some slight
simplifications to the test we have the following key blocks:

;;   basic block 2, loop depth 0, count 1073741823 (estimated locally), maybe
hot
;;    prev block 0, next block 3, flags: (NEW, VISITED)
;;    pred:       ENTRY [always]  count:1073741823 (estimated locally)
(FALLTHRU,EXECUTABLE)
  if (flags_8(D) == 0)
    goto <bb 5>; [50.00%]
  else
    goto <bb 3>; [50.00%]
;;    succ:       5 [50.0% (guessed)]  count:536870912 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                3 [50.0% (guessed)]  count:536870912 (estimated locally)
(FALSE_VALUE,EXECUTABLE)

;;   basic block 3, loop depth 0, count 536870912 (estimated locally), maybe
hot
;;    prev block 2, next block 4, flags: (NEW, VISITED)
;;    pred:       2 [50.0% (guessed)]  count:536870912 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
  if (flags_8(D) != -1)
    goto <bb 5>; [32.00%]
  else
    goto <bb 4>; [68.00%]
;;    succ:       5 [32.0% (guessed)]  count:171798688 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                4 [68.0% (guessed)]  count:365072224 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
[ ... ]
;;   basic block 5, loop depth 0, count 708669600 (estimated locally), maybe
hot
;;    prev block 4, next block 6, flags: (NEW, VISITED)
;;    pred:       2 [50.0% (guessed)]  count:536870912 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                3 [32.0% (guessed)]  count:171798688 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
  # orig_err_26 = PHI <errdes_10(D)(2), orig_err_9(D)(3)>
  if (in_12(D) != 0)
    goto <bb 6>; [33.00%]
  else
    goto <bb 7>; [67.00%]
;;    succ:       6 [33.0% (guessed)]  count:233860967 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                7 [67.0% (guessed)]  count:474808633 (estimated locally)
(FALSE_VALUE,EXECUTABLE)

;;   basic block 6, loop depth 0, count 233860966 (estimated locally), maybe
hot
;;    prev block 5, next block 7, flags: (NEW, VISITED)
;;    pred:       5 [33.0% (guessed)]  count:233860967 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
  _close (in_12(D));
;;    succ:       7 [always]  count:233860966 (estimated locally)
(FALLTHRU,EXECUTABLE)

;;   basic block 7, loop depth 0, count 708669601 (estimated locally), maybe
hot
;;    prev block 6, next block 8, flags: (NEW, VISITED)
;;    pred:       6 [always]  count:233860966 (estimated locally)
(FALLTHRU,EXECUTABLE)
;;                5 [67.0% (guessed)]  count:474808633 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
  if (out_14(D) != 1)
    goto <bb 8>; [48.88%]
  else
    goto <bb 9>; [51.12%]
;;    succ:       8 [48.9% (guessed)]  count:346397699 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;;                9 [51.1% (guessed)]  count:362271902 (estimated locally)
(FALSE_VALUE,EXECUTABLE)


Of particular note is that because BB2 and BB3 both reach BB5 we don't really
have any useful information about FLAGS.    The threader does know how to look
through a join point like that, but it then runs into another at BB7 at which
point it gives up.

The backwards threader probably could be made to detect this case though.  I'm
pretty sure we're not raising queries for COND_EXPRs  on the RHS of a
statement, just for GIMPLE_COND statements.  So this block:

;;   basic block 9, loop depth 0, count 708669601 (estimated locally), maybe
hot
;;    prev block 8, next block 10, flags: (NEW, VISITED)
;;    pred:       8 [always]  count:346397698 (estimated locally)
(FALLTHRU,EXECUTABLE)
;;                7 [51.1% (guessed)]  count:362271902 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
  _2 = orig_err_26 != 2;
  _1 = flags_8(D) == 0;
  _3 = _2 & _1;
  if (_3 != 0)
    goto <bb 10>; [33.00%]
  else
    goto <bb 4>; [67.00%]

We don't raise a query for flags_8.  The backwards threader also doesn't know
how to imply a range from anything other than assignments.  So the conditionals
involving flags_8 aren't helpful.  This is something we're going to want to
tackle with ranger integration into the backwards threader.

SO think we keep this as an open BZ until that's addressed.

[Bug tree-optimization/97108] Wmaybe-uninitialized false positive

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2020-11-06 00:00:00         |2021-4-9
                 CC|                            |msebor at gcc dot gnu.org
      Known to fail|                            |10.2.0, 11.0, 4.7.0, 4.8.4,
                   |                            |4.9.4, 5.5.0, 6.4.0, 7.2.0,
                   |                            |8.3.0, 9.1.0
           Keywords|                            |diagnostic

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
Reconfirming with a slightly more simplified test case below.

$ cat pr97108.c && gcc -O2 -S -Wall -fdump-tree-uninit-details=/dev/stdout
pr97108.c
int z;

void f (int flags, int i, int j)
{
  int x;
  _Bool f = !flags;

  if (f)
    x = j;

  if (i != 0)
    z = i;

  if (f && x != 2)
    z = 0;
}

;; Function f (f, funcdef_no=0, decl_uid=1946, cgraph_uid=1, symbol_order=1)

[WORKLIST]: add to initial list: x_18 = PHI <j_9(D)(10), x_8(D)(13)>
[CHECK]: examining phi: x_18 = PHI <j_9(D)(10), x_8(D)(13)>
[CHECK]: Found unguarded use: x_19 = PHI <j_9(D)(11), x_18(4)>
[WORKLIST]: Update worklist with phi: x_19 = PHI <j_9(D)(11), x_18(4)>
[CHECK]: examining phi: x_19 = PHI <j_9(D)(11), x_18(4)>
[CHECK]: Found unguarded use: _1 = x_19 != 2;
pr97108.c: In function ‘f’:
pr97108.c:14:14: warning: ‘x’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
   14 |   if (f && x != 2)
      |            ~~^~~~
pr97108.c:5:7: note: ‘x’ was declared here
    5 |   int x;
      |       ^
void f (int flags, int i, int j)
{
  _Bool f;
  int x;
  _Bool _1;
  _Bool _2;

  <bb 2> [local count: 1073741824]:
  f_7 = flags_6(D) == 0;
  if (flags_6(D) == 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 9>; [50.00%]

  <bb 3> [local count: 536870912]:
  if (i_10(D) != 0)
    goto <bb 10>; [50.00%]
  else
    goto <bb 11>; [50.00%]

  <bb 11> [local count: 268435456]:
  goto <bb 5>; [100.00%]

  <bb 10> [local count: 268435456]:

  <bb 4> [local count: 536870913]:
  # x_18 = PHI <j_9(D)(10), x_8(D)(13)>
  z = i_10(D);

  <bb 5> [local count: 805306368]:
  # x_19 = PHI <j_9(D)(11), x_18(4)>
  _1 = x_19 != 2;
  _2 = _1 & f_7;
  if (_2 != 0)
    goto <bb 7>; [66.67%]
  else
    goto <bb 12>; [33.33%]

  <bb 12> [local count: 268435458]:

  <bb 6> [local count: 536870914]:
  goto <bb 8>; [100.00%]

  <bb 7> [local count: 536870913]:
  z = 0;

  <bb 8> [local count: 1073741824]:
  return;

  <bb 9> [local count: 536870912]:
  if (i_10(D) != 0)
    goto <bb 13>; [50.00%]
  else
    goto <bb 14>; [50.00%]

  <bb 14> [local count: 268435456]:
  goto <bb 6>; [100.00%]

  <bb 13> [local count: 268435456]:
  goto <bb 4>; [100.00%]

}


Bisection indicates either r180295 or r180298 introduced the warning.  Before
then the dump was:

;; Function f (f, funcdef_no=0, decl_uid=1598, cgraph_uid=0)

[WORKLIST]: add to initial list: x_1 = PHI <x_4(D)(3), j_5(D)(2)>
[CHECK]: examining phi: x_1 = PHI <x_4(D)(3), j_5(D)(2)>

Use in stmt if (x_1 != 2)
is guarded by :
if (f_3 != 0)

[CHECK] Found def edge 1 in x_1 = PHI <x_4(D)(3), j_5(D)(2)>
Operand defs of phi x_1 = PHI <x_4(D)(3), j_5(D)(2)>
is guarded by :
if (f_3 != 0)
f (int flags, int i, int j)
{
  _Bool f;
  int x;

<bb 2>:
  f_3 = flags_2(D) == 0;
  if (f_3 != 0)
    goto <bb 4>;
  else
    goto <bb 3>;

<bb 3>:

<bb 4>:
  # x_1 = PHI <x_4(D)(3), j_5(D)(2)>
  if (i_7(D) != 0)
    goto <bb 5>;
  else
    goto <bb 6>;

<bb 5>:
  z = i_7(D);

<bb 6>:
  if (f_3 != 0)
    goto <bb 7>;
  else
    goto <bb 9>;

<bb 7>:
  if (x_1 != 2)
    goto <bb 8>;
  else
    goto <bb 9>;

<bb 8>:
  z = 0;

<bb 9>:
  return;

}

[Bug tree-optimization/97108] [9/10/11 Regression] -Wmaybe-uninitialized false positive

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Wmaybe-uninitialized false  |[9/10/11 Regression]
                   |positive                    |-Wmaybe-uninitialized false
                   |                            |positive

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
It's a regression introduced in GCC 4.7.

[Bug tree-optimization/97108] [9/10/11 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.4
           Priority|P3                          |P2

[Bug tree-optimization/97108] [9/10/11/12 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug tree-optimization/97108] [9/10/11/12 Regression] -Wmaybe-uninitialized false positive

[aldyh at gcc dot gnu.org]

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

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |101912
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |amacleod at redhat dot com,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #5 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Jeffrey A. Law from comment #1)

> The backwards threader probably could be made to detect this case though. 
> I'm pretty sure we're not raising queries for COND_EXPRs  on the RHS of a
> statement, just for GIMPLE_COND statements.  So this block:
> 
> ;;   basic block 9, loop depth 0, count 708669601 (estimated locally), maybe
> hot
> ;;    prev block 8, next block 10, flags: (NEW, VISITED)
> ;;    pred:       8 [always]  count:346397698 (estimated locally)
> (FALLTHRU,EXECUTABLE)
> ;;                7 [51.1% (guessed)]  count:362271902 (estimated locally)
> (FALSE_VALUE,EXECUTABLE)
>   _2 = orig_err_26 != 2;
>   _1 = flags_8(D) == 0;
>   _3 = _2 & _1;
>   if (_3 != 0)
>     goto <bb 10>; [33.00%]
>   else
>     goto <bb 4>; [67.00%]

The original code gates the read from orig_err with !separate_stderr:

      if (separate_stderr
          && orig_err != STDERR_FILENO)
        _close (orig_err);

which is somehow lost in BB9 above, since the reads from both orig_err_26 and
separate_stderr (!flags) are occuring in the same block.

I've made a similar observation in PR101912.  I assume this eliding of the gate
is allowable per language rules?

However, even if the gate were there, I think we'd start to run into path
length issues.  For example, I see some of the paths starting at the 3->5 edge
(which contains the undefined read):

Checking profitability of path (backwards):  bb:9 (4 insns) bb:7 (2 insns) bb:6
(2 insns) bb:5 (3 insns) bb:3
  Control statement insns: 2
  Overall: 9 insns
  FAIL: Did not thread around loop and would copy too many statements.

So I'm not even sure we could depend on the threader catching this since it's
handcuffed from attempting such long paths.

Crazy idea, but I wonder if we could plug the path solver to the uninit code. 
For instance, if we're going to warn on a use of orig_err_26, first query all
the paths from DEF to USE and see if any are reachable.  This would be cheaper
than a full blown threader, since we've only be chasing paths for places we're
already going to warn on.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101912
[Bug 101912] -Wmaybe-uninitialized false alarm in tzdb localtime.c

[Bug tree-optimization/97108] [10/11/12/13 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug tree-optimization/97108] [10/11/12/13 Regression] -Wmaybe-uninitialized false positive

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug tree-optimization/97108] [10/11/12/13 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97108
Bug 97108 depends on bug 101912, which changed state.

Bug 101912 Summary: -Wmaybe-uninitialized false alarm in tzdb localtime.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101912

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

[Bug tree-optimization/97108] [10/11/12/13 Regression] -Wmaybe-uninitialized false positive

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97108
Bug 97108 depends on bug 101912, which changed state.

Bug 101912 Summary: -Wmaybe-uninitialized false alarm in tzdb localtime.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101912

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

[Bug tree-optimization/97108] [10/11/12/13/14 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97108
Bug 97108 depends on bug 101912, which changed state.

Bug 101912 Summary: -Wmaybe-uninitialized false alarm in tzdb localtime.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101912

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

[Bug tree-optimization/97108] [11/12/13/14 Regression] -Wmaybe-uninitialized false positive

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.