public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Dubious "'foo' might be used uninitialized in this function" message
@ 2004-12-14 15:23 Dmitry Antipov
  2004-12-14 16:40 ` E. Weddington
  2004-12-14 17:00 ` Nathan Sidwell
  0 siblings, 2 replies; 13+ messages in thread
From: Dmitry Antipov @ 2004-12-14 15:23 UTC (permalink / raw)
  To: gcc

When compiling the following program,

#include <unistd.h>

int f (int x, int y)
{
  int z;

  if (x)
    z = getppid ();
  y = getpid ();
  if (x)
    y += z;
  return x + y + z;
}

GCC (with '-Wall') always says:

w.c: In function `f':
w.c:5: warning: 'z' might be used uninitialized in this function

which is not true.

Here 'z' is initialized under 'if (x)' condition, and 'z' always used under
'if (x)' condition. Also, it's clear that 'x' isn't accessed between 'if 
(x)',
so it's impossible to access uninitialized 'z'.

Is it reasonable to learn GCC do more analysis in attempt to avoid
warning in this case ? How is it complex ?

Also, IMHO it would be better to point to the place where
uninitialized variable may be _used_ and not to the place where
it's _declared_ ('y += z' in this case). Or even to both places.

For example, for the following program:

#include <unistd.h>

int f (int x, int y)
{
  int z;

  if (x)
    z = getppid ();
  y = getpid ();
  y += z;
  return x + y + z;
}

it would be better to get:

w.c: In function `f':
w.c:10: warning: uninitialized 'z' might be used
w.c:5: warning: ('z' declared here)

Thanks,
Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-14 15:23 Dubious "'foo' might be used uninitialized in this function" message Dmitry Antipov
@ 2004-12-14 16:40 ` E. Weddington
  2004-12-14 17:00 ` Nathan Sidwell
  1 sibling, 0 replies; 13+ messages in thread
From: E. Weddington @ 2004-12-14 16:40 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: gcc

Dmitry Antipov wrote:

> When compiling the following program,
>
> #include <unistd.h>
>
> int f (int x, int y)
> {
>  int z;
>
>  if (x)
>    z = getppid ();
>  y = getpid ();
>  if (x)
>    y += z;
>  return x + y + z;
> }
>
> GCC (with '-Wall') always says:
>
> w.c: In function `f':
> w.c:5: warning: 'z' might be used uninitialized in this function
>
> which is not true.
>
> Here 'z' is initialized under 'if (x)' condition, and 'z' always used 
> under
> 'if (x)' condition. Also, it's clear that 'x' isn't accessed between 
> 'if (x)',
> so it's impossible to access uninitialized 'z'.
>
> Is it reasonable to learn GCC do more analysis in attempt to avoid
> warning in this case ? How is it complex ?
>

Even if you rearranged the code to remove your warning above:

int f (int x, int y)
{
   int z;

   y = getpid ();
   if (x)
  {
      z = getppid ();
      y += z;
   }

   return x + y + z;
}

You'll still get an uninitialized variable warning for your return 
statement, where you do use z uninitialized.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-14 15:23 Dubious "'foo' might be used uninitialized in this function" message Dmitry Antipov
  2004-12-14 16:40 ` E. Weddington
@ 2004-12-14 17:00 ` Nathan Sidwell
  2004-12-15  9:00   ` Dmitry Antipov
  1 sibling, 1 reply; 13+ messages in thread
From: Nathan Sidwell @ 2004-12-14 17:00 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: gcc

Dmitry Antipov wrote:

> w.c: In function `f':
> w.c:5: warning: 'z' might be used uninitialized in this function
> 
> which is not true.
'tis true too, just here.
    return x + y + z;

read the documentation for -Wuninitialized, which will explain
why it can't get all the cases correct.

> Is it reasonable to learn GCC do more analysis in attempt to avoid
> warning in this case ? How is it complex ?

it gets as complex as the halting problem :)

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-14 17:00 ` Nathan Sidwell
@ 2004-12-15  9:00   ` Dmitry Antipov
  2004-12-15 10:02     ` Nathan Sidwell
  2004-12-15 12:06     ` Robert Dewar
  0 siblings, 2 replies; 13+ messages in thread
From: Dmitry Antipov @ 2004-12-15  9:00 UTC (permalink / raw)
  To: gcc

Nathan Sidwell wrote:

> Dmitry Antipov wrote:
>
>> w.c: In function `f':
>> w.c:5: warning: 'z' might be used uninitialized in this function
>>
>> which is not true.
>
> 'tis true too, just here.
>    return x + y + z;

Oops. That's my mistake, both examples are incorrect :-(. It should be
(with line numbers, for referencing lines)

     1  #include <unistd.h>
     2
     3  int f (int x, int y)
     4  {
     5   int z;
     6
     7   if (x)
     8     z = getppid ();
     9   y = getpid ();
    10   if (x)
    11     y += z;
    12   return x + y;
    13  }

in the first case and

     1  #include <unistd.h>
     2
     3  int f (int x, int y)
     4  {
     5   int z;
     6
     7   if (x)
     8     z = getppid ();
     9   y = getpid ();
    10   y += z;
    11   return x + y;
    12  }

in the second, respectively.

> read the documentation for -Wuninitialized, which will explain
> why it can't get all the cases correct.

I've taken a look through 3.4.3. docs online. The docs describes
two different cases:

 >           {
 >             int x;
 >             switch (y)
 >               {
 >               case 1: x = 1;
 >                 break;
 >               case 2: x = 4;
 >                 break;
 >               case 3: x = 5;
 >               }
 >             foo (x);
 >          }
 >    
 > If the value of y is always 1, 2 or 3, then x is always initialized, 
but GCC doesn't know this.

This sample probably means that we have something like:

void bar (int y)
{
  int x;
  switch (y)
  {
    case 1: x = 1;
      break;
    case 2: x = 4;
      break;
    case 3: x = 5;
  }
  foo (x);
}

If 'bar()' has the global scope (can be called from another modules), it's
impossible to predict the value of 'y', and we should warn always. But, if
the bar is 'static', we can look through all calls of the 'bar()' and do
the following check for each call:

 if (argument of 'bar()' is a compile-time constant) {
   if (argument of bar is not 1, 2, or 3)
      warn ();
 } else
   warn ();

IMHO, this kind of check may be really useful.

The second example from documentation

 > {
 >  int save_y;
 >  if (change_y) save_y = y, y = new_y;
 >  ...
 >  if (change_y) y = save_y;
 > }

is similar to my sample #1 (except that this example implicitly assumes
that 'change_y' isn't changed in the middle code marked as '...').

Probably that's the case which is described with "GCC is not smart enough to
see all the reasons why the code might be correct" words in the docs :-).

>> Is it reasonable to learn GCC do more analysis in attempt to avoid
>> warning in this case ? How is it complex ?
>
>
> it gets as complex as the halting problem :)

Really ? Why ?

Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15  9:00   ` Dmitry Antipov
@ 2004-12-15 10:02     ` Nathan Sidwell
  2004-12-15 12:06     ` Robert Dewar
  1 sibling, 0 replies; 13+ messages in thread
From: Nathan Sidwell @ 2004-12-15 10:02 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: gcc

Dmitry Antipov wrote:
> Nathan Sidwell wrote:

>>
>> it gets as complex as the halting problem :)
> 
> 
> Really ? Why ?

you're asking the compiler to show that a piece of code X (the use) is only
executed whenever another piece of code Y (the set) is executed.  For arbitrary
intervening code.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15  9:00   ` Dmitry Antipov
  2004-12-15 10:02     ` Nathan Sidwell
@ 2004-12-15 12:06     ` Robert Dewar
  2004-12-15 17:33       ` Florian Weimer
  2004-12-15 17:33       ` Joe Buck
  1 sibling, 2 replies; 13+ messages in thread
From: Robert Dewar @ 2004-12-15 12:06 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: gcc

Dmitry Antipov wrote:

>> it gets as complex as the halting problem :)
> Really ? Why ?

Because the general evaluation of compile time contants
is clearly in this territory. Your example:

     1  #include <unistd.h>
     2
     3  int f (int x, int y)
     4  {
     5   int z;
     6
     7   if (x)
     8     z = getppid ();
     9   y = getpid ();
    10   if (x)
    11     y += z;
    12   return x + y;
    13  }

requires the compiler to do symbolic evaluation of the
tests in lines 7 and 10 and determine that they always
have the same truth value. Then you expect the compiler
to do an analysis of the flow of the program for the two
cases, and determine that everything is OK in both cases.

Yes, it is conceivable, though really *quite* difficult
to do this in a simple case like this, but obviously
impractical/impossible in more complex cases.

The compiler is complaining about line 11, because as far
as it is concerned, z has not been unconditionally assigned
previously. It is simply not in the business of saying "ah,
but it's OK, we would never have got to line 11 unless the
condition in line 10 was true, in which case the condition
in line 7 would have been true, which means that z would
have been assigned at line 8".

Warnings are warnings, not errors. Usually they are warnings
precisely because it is impossible to precisely separate the
bad cases from the good cases. The art of warning design is
to give as many useful warnings as possible without giving
too many false positives, and without requiring any extensive
analysis that the compiler is not doing anyway.

Here is an example from Ada:

      1.        generic
      2.           type R is private;
                        |
         >>> warning: type "R" is not referenced

      3.        package Q is
      4.           X : Integer;
      5.        end Q;

One of our customers recently complained that this was a false
positive, because there might be a child unit of Q that references
R. This is true. However, after quite a bit of dicussion, we
decided that it was useful to retain the warning, since it is
rather unusual (though certainly legal) to have entities that
are referenced ONLY in a child unit, and if we removed the
warning on this basis, we would lose a lot of useful warnings.
child unit

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 12:06     ` Robert Dewar
  2004-12-15 17:33       ` Florian Weimer
@ 2004-12-15 17:33       ` Joe Buck
  2004-12-15 18:03         ` Dave Korn
  1 sibling, 1 reply; 13+ messages in thread
From: Joe Buck @ 2004-12-15 17:33 UTC (permalink / raw)
  To: Robert Dewar; +Cc: Dmitry Antipov, gcc

On Wed, Dec 15, 2004 at 07:05:48AM -0500, Robert Dewar wrote:
> Dmitry Antipov wrote:
> 
> >> it gets as complex as the halting problem :)
> > Really ? Why ?
> 
> Because the general evaluation of compile time contants
> is clearly in this territory. Your example:
> [ deleted ]
> 
> requires the compiler to do symbolic evaluation of the
> tests in lines 7 and 10 and determine that they always
> have the same truth value.

This case and many other common cases that gcc gives false reports
on could be solved by using a gated SSA approach (which would, as you
suggest, determine that the two tests have the same truth value).
However, using such an approach goes against the desire by many people
that the warnings they get at -O0 be the same as those for higher levels.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 12:06     ` Robert Dewar
@ 2004-12-15 17:33       ` Florian Weimer
  2004-12-15 17:34         ` Robert Dewar
  2004-12-15 17:33       ` Joe Buck
  1 sibling, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2004-12-15 17:33 UTC (permalink / raw)
  To: Robert Dewar; +Cc: Dmitry Antipov, gcc

* Robert Dewar:

> Here is an example from Ada:
>
>      1.        generic
>      2.           type R is private;
>                        |
>         >>> warning: type "R" is not referenced
>
>      3.        package Q is
>      4.           X : Integer;
>      5.        end Q;
>
> One of our customers recently complained that this was a false
> positive, because there might be a child unit of Q that references
> R. This is true. However, after quite a bit of dicussion, we
> decided that it was useful to retain the warning, since it is
> rather unusual (though certainly legal) to have entities that
> are referenced ONLY in a child unit, and if we removed the
> warning on this basis, we would lose a lot of useful warnings.

In this case, pragma Unreferenced could be specified in the body of
the parant unit, and it could do the right thing.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 17:33       ` Florian Weimer
@ 2004-12-15 17:34         ` Robert Dewar
  2004-12-15 17:52           ` Florian Weimer
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Dewar @ 2004-12-15 17:34 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Dmitry Antipov, gcc

Florian Weimer wrote:
> * Robert Dewar:
> 
> 
>>Here is an example from Ada:
>>
>>     1.        generic
>>     2.           type R is private;
>>                       |
>>        >>> warning: type "R" is not referenced
>>
>>     3.        package Q is
>>     4.           X : Integer;
>>     5.        end Q;
>>
>>One of our customers recently complained that this was a false
>>positive, because there might be a child unit of Q that references
>>R. This is true. However, after quite a bit of dicussion, we
>>decided that it was useful to retain the warning, since it is
>>rather unusual (though certainly legal) to have entities that
>>are referenced ONLY in a child unit, and if we removed the
>>warning on this basis, we would lose a lot of useful warnings.
> 
> 
> In this case, pragma Unreferenced could be specified in the body of
> the parant unit, and it could do the right thing.

first of all, there is no body of the parent unit in this case. Second
of all, pragma Unreferenced means you have something that is not
referenced anywhere. So this does not help in this case. Indeed the
"bug" report was precisely that pragma unreferenced does not work :-)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 17:34         ` Robert Dewar
@ 2004-12-15 17:52           ` Florian Weimer
  2004-12-15 18:00             ` Robert Dewar
  0 siblings, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2004-12-15 17:52 UTC (permalink / raw)
  To: Robert Dewar; +Cc: Dmitry Antipov, gcc

* Robert Dewar:

> first of all, there is no body of the parent unit in this case.

pragma Elaborate_All? *runs away*

> Second of all, pragma Unreferenced means you have something that is
> not referenced anywhere.

But warnings for the spec are only generated when it is compiled (or
its body).  A pragma Unreferenced in the body would suppress it this
warning.

Oh, wait, it's a generic unit.  I think warnings are issued in
multiple places. 8-(

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 17:52           ` Florian Weimer
@ 2004-12-15 18:00             ` Robert Dewar
  0 siblings, 0 replies; 13+ messages in thread
From: Robert Dewar @ 2004-12-15 18:00 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Dmitry Antipov, gcc

Florian Weimer wrote:

> Oh, wait, it's a generic unit.  I think warnings are issued in
> multiple places. 8-

Yes, it is fundamental to this example that it is a generic. This
case does not arise for non-generic packages. The point is that
the generic formals can ONLY be referenced in the parent generic
or one of its children, they cannot be referenced by clients.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 17:33       ` Joe Buck
@ 2004-12-15 18:03         ` Dave Korn
  2004-12-15 18:09           ` Robert Dewar
  0 siblings, 1 reply; 13+ messages in thread
From: Dave Korn @ 2004-12-15 18:03 UTC (permalink / raw)
  To: 'Joe Buck', 'Robert Dewar'; +Cc: 'Dmitry Antipov', gcc

> -----Original Message-----
> From: gcc-owner On Behalf Of Joe Buck
> Sent: 15 December 2004 17:22

> This case and many other common cases that gcc gives false reports
> on could be solved by using a gated SSA approach (which would, as you
> suggest, determine that the two tests have the same truth value).
> However, using such an approach goes against the desire by many people
> that the warnings they get at -O0 be the same as those for 
> higher levels.

  Ummmm.. I thought the 'may be used uninited' warning could only occur at > O0
anyway?


    cheers, 
      DaveK
-- 
Can't think of a witty .sigline today....
 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Dubious "'foo' might be used uninitialized in this function" message
  2004-12-15 18:03         ` Dave Korn
@ 2004-12-15 18:09           ` Robert Dewar
  0 siblings, 0 replies; 13+ messages in thread
From: Robert Dewar @ 2004-12-15 18:09 UTC (permalink / raw)
  To: Dave Korn; +Cc: 'Joe Buck', 'Dmitry Antipov', gcc

I also think it is fine if optimization *suppresses* warnings if
it can tell they are not needed. It is problematic if warnings
appear when you optimize, but if the alternative is not getting
the warnings at all, how is that better ???

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2004-12-15 18:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-12-14 15:23 Dubious "'foo' might be used uninitialized in this function" message Dmitry Antipov
2004-12-14 16:40 ` E. Weddington
2004-12-14 17:00 ` Nathan Sidwell
2004-12-15  9:00   ` Dmitry Antipov
2004-12-15 10:02     ` Nathan Sidwell
2004-12-15 12:06     ` Robert Dewar
2004-12-15 17:33       ` Florian Weimer
2004-12-15 17:34         ` Robert Dewar
2004-12-15 17:52           ` Florian Weimer
2004-12-15 18:00             ` Robert Dewar
2004-12-15 17:33       ` Joe Buck
2004-12-15 18:03         ` Dave Korn
2004-12-15 18:09           ` Robert Dewar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).