Bug 30856

Summary: missing uninitialized variable warning (CCP)
Product: gcc Reporter: Christoph Mallon <christoph.mallon>
Component: middle-endAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED DUPLICATE    
Severity: minor CC: christoph.mallon, david.cuthbert, dberlin, dhill, fang, fredrik, gcc-bugs, jellegeerts, jens.mueller, manu, mark, mueller, muntyan, P.Schaffnit, pinskia, satyam, thutt
Priority: P3 Keywords: diagnostic
Version: 4.1.2   
Target Milestone: 4.1.3   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2007-02-19 10:05:28
Bug Depends on: 18501    
Bug Blocks:    

Description Christoph Mallon 2007-02-19 09:24:07 UTC
int f(int x)
{
  int y;
  if (x == 42) y = 23;
  return y;
}

This simple function results in no warning about the possibly uninitialized local variable 'y' when using the following compiler versions (compiler flags: -O -Wall):
GCC 4.1.2 20070205
GCC 4.2.0 20070214
GCC 4.3.0 20070209

The warning is correctly reported by GCC 2.95.3, GCC 3.4.6 20060305 and GCC 4.0.4.
Comment 1 Richard Biener 2007-02-19 10:05:28 UTC
This is because we don't warn for uninitialized PHIs in early_warn_uninitialized
and later the code is removed because it is dead.
Comment 2 Richard Biener 2007-02-19 10:08:53 UTC
One possibility is to have an argument to -Wuninitialized to specify if we want
to have the "may be uninitialized" warnings early as well.
Comment 3 Andrew Pinski 2007-02-19 10:12:28 UTC

*** This bug has been marked as a duplicate of 22456 ***

*** This bug has been marked as a duplicate of 22456 ***
Comment 4 Christoph Mallon 2007-02-19 13:57:09 UTC
(In reply to comment #1)
> This is because we don't warn for uninitialized PHIs in
> early_warn_uninitialized
> and later the code is removed because it is dead.

The code is certainly not dead.
The local variable 'y' is used as return value of the function.
I don't know how exactly the internal representation of GCC is, but it should be something like:
t = phi(42, UNDEFINED)
ret t

I doubt this is a duplicate of Bug 22456 because the code there is dead.
It looks more like Bug 30542 and Bug 30575 which both are mentioned in Bug 22456. These do not look like duplicates of Bug 22456 either because the code there is not dead.


Ok, I investigated a bit more on this:
int f(int x)
{
  int y;
  if (x == 42) y = 23;
  else if (x == 23) y = 42;
  return y;
}

This function triggers the expected warning message.
This is because the Phi in this case has three inputs.
In the other example (or if you remove the else in the code above) you have a Phi with two inputs. One of these is undefined and phase 033t.ccp optimises this Phi away (which per se is ok, the value is undefined after all) and only its one defined input remains. If the Phi has more (different) inputs this optimisation cannot happen.
The problem is the warning about possibly undefined values definitely should be generated _before_ this phase.
I think it is a major bug if this warning simply vanishes in some cases.
Comment 5 Yevgen Muntyan 2007-02-19 14:07:02 UTC
(In reply to comment #4)
> I doubt this is a duplicate of Bug 22456 because the code there is dead.
> It looks more like Bug 30542 and Bug 30575 which both are mentioned in Bug
> 22456. These do not look like duplicates of Bug 22456 either because the code
> there is not dead.

It's exactly the same as Bug 30542 and Bug 30575. I'd guess it is really a duplicate of Bug 22456, and the "dead" code is indeed dead but not in the sense we think it is. A single comment from gcc developers would clarify it, something like "We are indeed aware of this serious bug and #22456 is just an umbreall bug which happened to have bogus test case" or "No, it's not a serious bug, and the code there and here is the same". Or something.
Comment 6 Christoph Mallon 2007-02-19 14:25:49 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > I doubt this is a duplicate of Bug 22456 because the code there is dead.
> > It looks more like Bug 30542 and Bug 30575 which both are mentioned in Bug
> > 22456. These do not look like duplicates of Bug 22456 either because the code
> > there is not dead.
> 
> It's exactly the same as Bug 30542 and Bug 30575. I'd guess it is really a
> duplicate of Bug 22456, and the "dead" code is indeed dead but not in the sense
> we think it is. A single comment from gcc developers would clarify it,
> something like "We are indeed aware of this serious bug and #22456 is just an
> umbreall bug which happened to have bogus test case" or "No, it's not a serious
> bug, and the code there and here is the same". Or something.

In Bug 22456 all calculated values are unused from the start, so it could be argued that it is not necessary to generate a warning.

In this case (and Bug 30542 and Bug 30575) the values are _not_ unused and only a later optimisation phase, which reduces phis, which only have undefined and arbitrarily often one value as input to exactly this input, or a bit math speak:
  Phi(x_1, ..., x_n) --> X where x_1, ..., x_n are element of { UNDEFINED, X }
So when finally the uninitialised variable warning is generated there is no more uninitialised variable.
The warning should simply be generated _before_ this optimisation.
Comment 7 Richard Biener 2007-02-19 15:56:52 UTC
Constant propagation propagates through the PHI in this case:

<bb 2>:
  if (x_2(D) == 42) goto <L0>; else goto <L1>;

<L0>:;
  y_4 = 23;

  # y_1 = PHI <y_3(D)(2), 23(3)>
<L1>:;
  y_5 = 23;
  return 23;

so this would be the place where a warning could trigger.  But it's likely
this will introduce more false positives (so the "may" form is only done
very late).  The above could be warned as

"Warning: uninitialized value assumed to be 23."

it's like -Wstrict-overflow -- warn about valid optimizations of undefined
code (that is here, make it "dead").  In that sense it's a dup of the
mentioned PRs (it's treated as dead code).

So, let's make this a request for a warning for an optimization based on
the undefinedness of a value instead ;)
Comment 8 Christoph Mallon 2007-02-19 16:28:15 UTC
(In reply to comment #7)
> Constant propagation propagates through the PHI in this case:

I basically wrote this in comment #4 and #6 (though I thought it was more general, see the sidenote below)

> [...]
> 
> so this would be the place where a warning could trigger.  But it's likely
> this will introduce more false positives (so the "may" form is only done
> very late).

An optimisation - which is not stricly necessary and could just be left out - removes the cause for the warning, therefore i doubt this generates false positives. Also this warning was correctly reported before GCC 4.1, why should it suddenly trigger false positives?

> it's like -Wstrict-overflow -- warn about valid optimizations of undefined
> code (that is here, make it "dead").  In that sense it's a dup of the
> mentioned PRs (it's treated as dead code).
> 
> So, let's make this a request for a warning for an optimization based on
> the undefinedness of a value instead ;)

Why should this be a request? I still think it is a regression, because all GCCs before 4.1 correctly reported this warning (which i stated in the original posting).
Further I think this warning is highly valuable. I often write code where a local variable gets declared and later gets values assigned in different code paths and I want that my compiler tells me if i forgot to do so on one path.


I also expanded this test case a little:
int f(int x, int z)
{
  int y;
  if (x == 42) y = z;
  return y;
}

The only difference is that I don't assign a constant, but another local variable to 'y'. Now the optimisation does not trigger (because it stricly operates on constants). So this is very confusing: If the code operates on constants the warning is missing (if the Phi is simple enough). On the other hand it gets shown if non-constants or Phis with multiple non-undefined values are involved.

Sidenote: In the non-constant case this could also be counted as missed optimisation. If the value gets calculated in a dominating block, the Phi could also be replaced by this value.


IAACC - I Am A Compiler Constructor. My student research project was about if-conversion on SSA, so I think I know a bit about this stuff in general. (;
Comment 9 Andrew Pinski 2007-02-19 18:44:41 UTC
Note SSA CCP usually (this is how it is done in the papers and almost always done in all comericial compilers too) does PHI<1, UNDEFINED> as just being 1.

This is still a dup because the loop in that bug is exactly that case.

*** This bug has been marked as a duplicate of 22456 ***
Comment 10 Christoph Mallon 2007-02-20 06:02:19 UTC
(In reply to comment #9)
> Note SSA CCP usually (this is how it is done in the papers and almost always
> done in all comericial compilers too) does PHI<1, UNDEFINED> as just being 1.
> 
> This is still a dup because the loop in that bug is exactly that case.
> 
> *** This bug has been marked as a duplicate of 22456 ***

I checked the dumps of Bug #22456 and this one. There is one major difference: In 22456 CCP does exactly nothing (which is not astonishing, because the phi there does not has any constants as arguments).
Just the user perceivable result is the same: A missing warning.

Which commercial compilers are you referring to? Sun HotSpot comes to mind, but they certainly don't do this optimisation, because undefined values are not allowed in Java and should cause an abort of the compilation. Another popular compiler would be ICC, but its EDG frontend faithfully generates the warning, of course, so there is no problem there.

Here is the relevant dump of the test case in 22456, i.e. 033t.ccp (produced by GCC 4.2):
foo ()
{
  int i;

<bb 2>:
  goto <bb 4> (<L1>);

<L0>:;
  i_3 = i_1 + 1;

  # i_1 = PHI <i_2(2), i_3(3)>;
<L1>:;
  if (i_1 != 0) goto <L0>; else goto <L2>;

<L2>:;
  return;

}

As you can see, the phi with one undefined argument (i_2) is still used in the following if. To get the warning about the use of the uninitialised variable it is sufficient to use -fno-tree-dce. This way a part of the loop survives long enough so the warning gets generated (Which seems to happen somewhere before 092t.cddce where the rest of the loop gets eliminated).

This does _not_ work with the test case provided here.
To get this valuable warning you have to use -fno-tree-ccp (This obviously does not work with the case provided in #22456, because CCP does nothing there).
So as a workaround for this common case i suggest compiling with -fno-tree-ccp. 

IMO the long term fix for this problem is to generate the warning about uninitialised variables much earlier. Most probably this point would be just after (or during) 029t.ssa where the SSA form gets built. Thanks to the explicit def-use-chains of SSA determining if a undefined value gets used on some code path is easy as pie.
Until then i strongly recommend to use -fno-tree-ccp, otherwise the warning behaviour is erratic (as stated earlier) and you miss valuable information.

(I leave this as closed because I don't want to cause an edit war, but I like to see this reopened as separate bug from 22456 because it has a different cause - it just maybe have the same fix)
Comment 11 Manuel López-Ibáñez 2009-02-07 21:21:29 UTC
This wasn't a duplicate of bug 22456
Comment 12 Manuel López-Ibáñez 2009-02-07 21:29:45 UTC
This is a duplicate of 18501. CCP assumes y is always 23. 

Reordering passes just changes the set of false negatives/positives. For example, if you move the warning before CCP, then you get a warning for this case:

int f(int x)
{
  int y;
  int x = 42;
  if (x == 42) y = 23;
  return y;
}

A more complete analysis is here:

http://gcc.gnu.org/ml/gcc/2005-11/msg00028.html

And some random ideas are here:

http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings#problem_ccp

How other compilers get this right? I don't know, but ideas and patches are welcome.

*** This bug has been marked as a duplicate of 18501 ***