Bug 106155 - [12/13/14 Regression] spurious "may be used uninitialized" warning
Summary: [12/13/14 Regression] spurious "may be used uninitialized" warning
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P2 normal
Target Milestone: 12.4
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, missed-optimization
Depends on:
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2022-07-01 01:49 UTC by Vincent Lefèvre
Modified: 2023-08-08 05:04 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-07-01 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Lefèvre 2022-07-01 01:49:32 UTC
With "-O -Wmaybe-uninitialized", I get a spurious "may be used uninitialized" on the following code on an x86_64 Debian/unstable machine:

int *e;
int f1 (void);
void f2 (int);
long f3 (void *, long, int *);
void f4 (void *);
int *fh;

void tst (void)
{
  int status;
  unsigned char badData[3][3] = { { 7 }, { 16 }, { 23 } };
  int badDataSize[3] = { 1, 1, 1 };
  int i;

  for (i = 0; i < 3; i++)
    {
      int emax;
      if (i == 2)
        emax = f1 ();
      status = f3 (&badData[i][0], badDataSize[i], fh);
      if (status)
        {
          f1 ();
          f1 ();
          f1 ();
        }
      f4 (fh);
      *e = 0;
      f1 ();
      if (i == 2)
        f2 (emax);
    }
}

Note that even a small change such as changing "long" to "int" as the second parameter of f3 makes the warning disappear.

$ gcc-12 -O -Wmaybe-uninitialized -c -o tfpif.o tfpif.c
tfpif.c: In function ‘tst’:
tfpif.c:31:9: warning: ‘emax’ may be used uninitialized [-Wmaybe-uninitialized]
   31 |         f2 (emax);
      |         ^~~~~~~~~
tfpif.c:17:11: note: ‘emax’ was declared here
   17 |       int emax;
      |           ^~~~
$ gcc-12 --version
gcc-12 (Debian 12.1.0-5) 12.1.0
[...]

$ gcc-snapshot -O -Wmaybe-uninitialized -c -o tfpif.o tfpif.c
tfpif.c: In function 'tst':
tfpif.c:31:9: warning: 'emax' may be used uninitialized [-Wmaybe-uninitialized]
   31 |         f2 (emax);
      |         ^~~~~~~~~
tfpif.c:17:11: note: 'emax' was declared here
   17 |       int emax;
      |           ^~~~
$ gcc-snapshot --version
gcc (Debian 20220630-1) 13.0.0 20220630 (experimental) [master r13-1359-gaa1ae74711b]
[...]

No such issue with:
  gcc-9 (Debian 9.5.0-1) 9.5.0
  gcc-10 (Debian 10.4.0-1) 10.4.0
  gcc-11 (Debian 11.3.0-4) 11.3.0

I detected this issue by testing GNU MPFR. The above code is derived from "tests/tfpif.c", function check_bad.
Comment 1 Vincent Lefèvre 2022-07-01 02:05:51 UTC
I detected the issue on tests/tfpif.c with the upgrade of Debian's package gcc-snapshot from 1:20220126-1 to 1:20220630-1 (it doesn't occur on tests/tfpif.c with gcc-snapshot 1:20220126-1). However, the simplified testcase I've provided fails with gcc-snapshot 1:20220126-1.
Comment 2 Andrew Pinski 2022-07-01 02:32:45 UTC
init_from_control_deps {{0 -> 3}}:
	i_44 != 2 (expanded)
	NOT (i_44 == 2)
After normalization [DEF]:
	i_44 != 2 (expanded)
	NOT (i_44 == 2)
After normalization [USE]:
	f2 (emax_8);
  is conditional on:
	ivtmp.7_37 == 2 (expanded)
	ivtmp.7_37 == 2

Which is funny we get:

  # ivtmp.7_37 = PHI <ivtmp.7_38(9), 0(2)>
  i_44 = (int) ivtmp.7_37;
  if (i_44 == 2)
    goto <bb 4>; [12.49%]
  else
    goto <bb 10>; [87.51%]
....
  if (ivtmp.7_37 == 2)
    goto <bb 8>; [12.49%]
  else
    goto <bb 9>; [87.51%]

  <bb 8> [local count: 268435456]:
  f2 (emax_8);

I don't know why ivopts only changed the second if and not the first.

Oh because the second is the exit check:
Replacing exit test: if (ivtmp_39 != 0)
Replacing exit test: if (i_40 == 2)

If we change the size to 4 and keep the 2 as 2, there would be no warning.
And if then change 2 to 3, the warning is back.

So there is a missed optimization of changing the internal comparison for the IV to the new type if the right side is a constant.
Comment 3 Andrew Pinski 2022-07-01 02:43:14 UTC
I can't figure out why 11.x didn't warn but 12+ does though. The IR looks the same to me.
Comment 4 Martin Liška 2022-07-04 12:09:07 UTC
Started with r12-4526-gd8edfadfc7a9795b.
Comment 5 Richard Biener 2022-08-19 08:26:55 UTC
GCC 12.2 is being released, retargeting bugs to GCC 12.3.
Comment 6 Richard Biener 2022-08-31 14:04:11 UTC
The issue here is that we see

<bb 3> [local count: 805306369]:
# emax_41 = PHI <emax_9(9), emax_22(D)(2)>
# ivtmp.7_44 = PHI <ivtmp.7_45(9), 0(2)>
i_46 = (int) ivtmp.7_44;
if (i_46 == 2)
  goto <bb 4>; [12.49%] // init
else
  goto <bb 10>; [87.51%]

...


<bb 7> [local count: 805306369]:
fh.1_6 = fh;
f4 (fh.1_6);
e.2_7 = e;
*e.2_7 = 0;
f1 ();
if (ivtmp.7_44 == 2)
  goto <bb 8>; [12.49%] // use
else
  goto <bb 9>; [87.51%]

and the predicates i_46 == 2 and ivtmp.7_44 == 2 do not match up.

Oh, and my r13-2149-g200baf7698a100 change made us run into

  if (dominated_by_p (CDI_POST_DOMINATORS, def_bb, use_bb))
    /* The use is not guarded.  */
    return false;

for this use on the loop exit because post-dominance doesn't cover
loop exit conditions.

I will think about this a bit more.

I'll also note it is a missed optimization to not replace the i_46 == 2
check.  -fno-ivopts makes the warning disappear (when the above is fixed).
For some reason we do not optimize the

  i_46 = (int) ivtmp.7_44;
  if (i_46 == 2)

stmt, likely because ivtmp is 64bit while i is 32bit and we don't check
ivtmps value range here.
Comment 7 Richard Biener 2022-09-06 12:57:04 UTC
This should now be fixed up to the confusion due to IVOPTs.  As said the narrowing is problematic here :/
Comment 8 Richard Biener 2022-09-06 13:17:07 UTC
The following variant warns with all GCC releases I tested (and -fno-ivopts fixes it), not exposing a mitigating jump threading opportunity:

int *e;
int f1 (void);
void f2 (int);
long f3 (void *, long, int *);
void f4 (void *);
int *fh;

void tst (void)
{
  int status;
  unsigned char badData[5][5] = { { 7 }, { 16 }, { 23 } };
  int badDataSize[5] = { 1, 1, 1 };
  int i;

  for (i = 0; i < 5; i++)
    {
      int emax;
      if (i == 2)
        emax = f1 ();
      status = f3 (&badData[i][0], badDataSize[i], fh);
      if (status)
        {
          f1 ();
          f1 ();
          f1 ();
        }
      f4 (fh);
      *e = 0;
      f1 ();
      if (i >= 2)
        f2 (emax);
    }
}
Comment 9 Richard Biener 2022-09-06 13:21:10 UTC
This is another example where when doing PHI chaining, we fail to consider the use predicate of the PHI operand in the final PHI which has now no further
guard before the use.
Comment 10 Vincent Lefèvre 2022-11-23 16:44:33 UTC
A similar bug (all uses of the variable are under some condition) with a simpler testcase I've just reported: PR107839.
Comment 11 Richard Biener 2023-05-08 12:24:58 UTC
GCC 12.3 is being released, retargeting bugs to GCC 12.4.
Comment 12 Vincent Lefèvre 2023-06-13 02:03:02 UTC
Here's a similar, simpler testcase:

int f1 (void);
void f2 (int);
long f3 (long);

void tst (void)
{
  int badDataSize[3] = { 1, 1, 1 };

  for (int i = 0; i < 3; i++)
    {
      int emax;
      if (i == 2)
        emax = f1 ();
      int status = f3 (badDataSize[i]);
      if (f1 ())
        f1 ();
      if (status)
        f1 ();
      if (i == 2)
        f2 (emax);
    }
}

gcc-12 (Debian 12.2.0-14) 12.2.0 warns at -O1, but not at -O2.
gcc-13 (Debian 13.1.0-5) 13.1.0 is worse, as it warns at both -O1 and -O2.