Bug 20968

Summary: spurious "may be used uninitialized" warning (conditional PHIs)
Product: gcc Reporter: James Juran <James.Juran>
Component: middle-endAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: colin, gcc-bugs, law, manu, P.Schaffnit
Priority: P2    
Version: 4.0.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2009-02-07 16:27:01
Bug Depends on: 36550    
Bug Blocks: 24639    

Description James Juran 2005-04-12 14:04:16 UTC
struct {int count;} *v1;
int c;
int k;

extern void baz(int);

static int bar(int *j)
{
    if (k == 4)
    {
	*j = 1;
	return 1;
    }
    return 0;
}

void foo(void)
{
    int i;

    if (!bar(&i))
    {
	if (!c)
	    return;
	v1->count++;
    }
    if (!c)
    {
	baz(i);
    }
}

produces this warning when compiled with "-Wall -O2":

$ gcc-4.0.0-RC1 -Wall -O2 -c final.c
final.c: In function ‘foo’:
final.c:18: warning: ‘i’ may be used uninitialized in this function

GCC 3.4.3 and previous versions I tested did not generate this warning.

I believe this warning is improper because one of these two scenarios must be true:

1) k == 4, in which case i is initialized before it can be used
2) k != 4, in which case i is not used

Adding -fno-inline avoids the warning by preventing bar from being inlined into foo.

I was unable to reduce the test case further without making the warning
disappear.  In particular, removing the increment of v1->count makes the warning
disappear.

This is GCC 4.0.0 RC1:
$ gcc-4.0.0-RC1 -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.0.0-20050410/configure --program-suffix=-4.0.0-RC1
Thread model: posix
gcc version 4.0.0 20050410 (prerelease)

This warning still appears on mainline.
Comment 1 James Juran 2005-04-12 14:07:43 UTC
This bug appears to be related to, but not duplicates of, the following bugs:

PR 20644 -- is unreachable code.  This test case is all reachable.
PR 5035 -- is not a regression.  This test case is a regression.
PR 17506 -- is about wrong line number in the diagnostic.  The line number is
reported correctly for this test case.
Comment 2 Andrew Pinski 2005-04-12 14:49:16 UTC
Note this is not fully a regression but really a progression.
What is happening now is only partial optimizations is happen before the warning to happen.

>I was unable to reduce the test case further without making the warning
>disappear.  In particular, removing the increment of v1->count makes the warning
>disappear.
This is because we would then jump thread he jump.

Again this is because we are emitting the warning too soon, I might be able to come up with a testcase 
which shows that this is not really a regression but a progression in that we have warned in 3.4 and 
4.0:
struct {int count;} *v1;
int c;
int k;

extern void baz(int);
void foo(void)
{
    int i;
    int r;
    if (k == 4)
    {
        i = 1;
        r = 1;
    }
    else
        r = 0;

    if (!r)
    {
        if (!c)
            return;
        v1->count++;
    }
    if (!c)
    {
        baz(i);
    }
}

There is no different from the case above and the functions you gave below.

There has been some talking about moving where we warn about uninitialized variables but I feel that 
you can get around this in your code.
Comment 3 James Juran 2005-04-12 16:21:30 UTC
Thanks for the info.  Your testcase does warn in all versions I tested.  We can
certainly initialize the variable in our code to get around this issue, but it
would be nice to not have to do this since the initialization is unnecessary. 
The original code is obviously more complicated, so manually inlining it as your
test case does is not really a viable option.
Comment 4 Jeffrey A. Law 2005-11-08 17:47:20 UTC
Just an interesting tidbit.

This testcase exposes a much more difficult/interesting long term problem.  Namely, how should we handle uninitialized warnings for variables which are exposed by optimization.

ie, in this case (and probably others), we have a local variable which has had its address taken, which normally suppresses uninitialized warnings.  However, inlining followed by our SSA optimizers proves that we don't need the address of the local variable.  That in turn exposes the local variable and we want to be able to warn about it if its uninitialized.

Similar issues probably exist for SRA optimizations.  Whee fun.

jeff

Comment 5 Manuel López-Ibáñez 2007-08-22 17:14:51 UTC
(In reply to comment #4)
> Just an interesting tidbit.
> 
> This testcase exposes a much more difficult/interesting long term problem. 
> Namely, how should we handle uninitialized warnings for variables which are
> exposed by optimization.

I think the problem reduces to "what we do when we are unsure?" Currently we mostly warn except for CCP merging undefined values with constants and when the address of the variable is passed to a function. If the body is not taken into account (because it is not inlined) we assume that baz() initializes i. When inlined, we are not sure anymore, so we warn. This will happen whenever more optimisation turns the balance from "unsure but not warn" to "unsure but warn". In the general case, I don't think this can be solved unless we always warn or we always don't warn when unsure.

Therefore, the only thing we could do is handle the inlined version as well as Andrew's testcase in comment #2.
Comment 6 Manuel López-Ibáñez 2009-02-07 16:27:01 UTC
This is just another case that would require conditional PHIs. I am not marking it as a duplicate of bug 36550, because this case is harder than then typical:

if(q) p=1;
something()
if(q) use(p);

Therefore, it may be possible to fix bug 36550 and still not fix this.
Comment 7 Manuel López-Ibáñez 2009-12-30 16:59:18 UTC
*** Bug 42145 has been marked as a duplicate of this bug. ***
Comment 8 davidxl 2010-04-21 00:27:55 UTC
(In reply to comment #2)
> Note this is not fully a regression but really a progression.
> What is happening now is only partial optimizations is happen before the warning to happen.
> 
> >I was unable to reduce the test case further without making the warning
> >disappear.  In particular, removing the increment of v1->count makes the warning
> >disappear.
> This is because we would then jump thread he jump.
> 
> Again this is because we are emitting the warning too soon, I might be able to come up with a testcase 
> which shows that this is not really a regression but a progression in that we have warned in 3.4 and 
> 4.0:
> struct {int count;} *v1;
> int c;
> int k;
> 
> extern void baz(int);
> void foo(void)
> {
>     int i;
>     int r;
>     if (k == 4)
>     {
>         i = 1;
>         r = 1;
>     }
>     else
>         r = 0;
> 
>     if (!r)
>     {
>         if (!c)
>             return;
>         v1->count++;
>     }
>     if (!c)
>     {
>         baz(i);
>     }
> }
> 
> There is no different from the case above and the functions you gave below.
> 
> There has been some talking about moving where we warn about uninitialized variables but I feel that 
> you can get around this in your code.

To reproduce the problem -- -fno-tree-vrp  -fno-tree-dominator-opts -fno-tree-ccp are needed. This 
Comment 9 Jeffrey A. Law 2013-11-19 07:07:07 UTC
Fixed, not sure how long ago.  4.8.2 is clean.  I did double check that it's clean because we actually do all the right things in the optimizers to both eliminate the unwanted ADDRESSOF which then exposes "i" as a local scalar.  Then the optimizers thread the paths properly and as a result there's no uninitializes uses left (in fact, there are no uses of "i" left as the only one was determined to be a constant.