Bug 19430 - taking address of a var causes missing uninitialized warning
taking address of a var causes missing uninitialized warning
Status: NEW
Product: gcc
Classification: Unclassified
Component: middle-end
3.4.2
: P3 enhancement
: ---
Assigned To: Not yet assigned to anyone
: diagnostic
: 23805 23844 29227 31284 42079 (view as bug list)
Depends on: 179
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2005-01-13 20:04 UTC by M Welinder
Modified: 2015-02-23 22:46 UTC (History)
10 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-05-03 18:04:44


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description M Welinder 2005-01-13 20:04:57 UTC
the program below should result in a warning that "j" might be used
uninitialized.  If the "baz" call is replaced with an explicit assignment
to "j" a warning is generated as expected.

> gcc-3.4.2 -c -Wall -O2 ~/foo.c
(stunning silence)


extern int bar (int);
extern void baz (int *);

int
foo (int i)
{
  int j;

  if (bar (i)) {
    // These should do the same with respect to `j':
    baz (&j);
    // j = 1;
  } else {
  }

  return j;
}
Comment 1 Diego Novillo 2005-01-13 20:14:30 UTC
Confirmed.  It doesn't work on mainline either.  The warning machinery is
getting confused with the first V_MAY_DEF to j in the first call to 'bar()'.

It would probably not be too hard to fix for 4.0.


<bb 0>:
  #   j_7 = V_MAY_DEF <j_3>;
  D.1124_2 = bar (i_1);
  if (D.1124_2 != 0) goto <L0>; else goto <L1>;

<L0>:;
  #   j_8 = V_MAY_DEF <j_7>;
  baz (&j);

  # j_6 = PHI <j_7(0), j_8(1)>;
<L1>:;
  #   VUSE <j_6>;
  D.1125_4 = j;
  return D.1125_4;
Comment 2 Andrew Pinski 2005-09-10 02:16:29 UTC
*** Bug 23805 has been marked as a duplicate of this bug. ***
Comment 3 Andrew Pinski 2005-09-10 02:17:12 UTC
(In reply to comment #1)
> Confirmed.  It doesn't work on mainline either.  The warning machinery is
> getting confused with the first V_MAY_DEF to j in the first call to 'bar()'.
Actually the warning machinery does not work with VOPs at all.
Comment 4 Andrew Pinski 2005-09-10 02:19:17 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Confirmed.  It doesn't work on mainline either.  The warning machinery is
> > getting confused with the first V_MAY_DEF to j in the first call to 'bar()'.
> Actually the warning machinery does not work with VOPs at all.
I should say see PR 23805 for an example of that.
Also the extra V_MAY_DEF is coming from that fact the clobber list is not flow sensitive, see PR 23384 
for that bug.
Comment 5 Mike McCormack 2005-09-10 08:07:47 UTC
Simplifying things a bit:

void foo(int*);
void bar(void) { int x; if(x) foo(&x); }

gcc -c foo.c -Wall -O2 -Wuninitialized

(More stunning silence.)
Comment 6 Andrew Pinski 2005-09-12 21:55:42 UTC
*** Bug 23844 has been marked as a duplicate of this bug. ***
Comment 7 giuseppe ciaccio 2006-03-10 15:25:23 UTC
An even simpler test case of this bug (tested with gcc 3.4.5):

int main( void ) {
        int rc;
        return rc;
        *&rc = 0;
}

> gcc -O -Wall program.c
(more stunning silence)

NOTE: in this example, there is no function invocation inside main().
NOTE: remove the last statement in the program, and the warning will show up.

g.
Comment 8 Andrew Pinski 2006-03-10 15:28:16 UTC
(In reply to comment #7)
> An even simpler test case of this bug (tested with gcc 3.4.5):
That works correctly in 4.0.0 and above.
Comment 9 Diego Novillo 2006-03-10 15:31:53 UTC
Not going to work on this problem any time soon.
Comment 10 Mike McCormack 2006-03-10 15:37:41 UTC
Can I bribe you to work on it by fixing a Wine bug in exchange? :)
Comment 11 Andrew Pinski 2006-09-25 22:44:12 UTC
*** Bug 29227 has been marked as a duplicate of this bug. ***
Comment 12 Daniel Richard G. 2007-03-02 23:36:19 UTC
Here's my minimal test case. Compile with "-O3 -Wall -c":

#include <stdio.h>

void frob(int *pi);

int main(void)
{
    int i;
    printf("i = %d\n", i);
    frob(&i);

    return 0;
}

No warning from 4.0.3 nor 4.1.2....
Comment 13 Andrew Pinski 2007-03-20 17:43:00 UTC
*** Bug 31284 has been marked as a duplicate of this bug. ***
Comment 14 Manuel López-Ibáñez 2007-08-15 15:36:20 UTC
Trying to improve the summary to help spot duplicates.
Comment 15 Manuel López-Ibáñez 2007-08-17 09:17:30 UTC
This seems to me a duplicate of PR179. I am going to add a dependency to remember to check this PR when PR179 gets fixed.
Comment 16 Manuel López-Ibáñez 2008-08-20 22:31:23 UTC
All testcases except the one in the original description were actually duplicates of PR179 and are thusly fixed. 

The original testcase deals with PHI ops which is a completely different beast. I added it XFAILED as gcc.dg/uninit-pr19430.c but I doubt it will get magically fixed, so suggestions/patches welcome.
Comment 17 Manuel López-Ibáñez 2009-11-19 12:00:51 UTC
*** Bug 42079 has been marked as a duplicate of this bug. ***
Comment 18 Vincent Lefèvre 2013-11-21 03:11:26 UTC
This seems to be fixed in the trunk.
Comment 19 Manuel López-Ibáñez 2013-11-21 03:31:55 UTC
(In reply to Vincent Lefèvre from comment #18)
> This seems to be fixed in the trunk.

Is there an XPASS for gcc.dg/uninit-pr19430.c ?

Also, the testcase from bug 42079?
Comment 20 Manuel López-Ibáñez 2013-11-21 04:22:12 UTC
(In reply to Manuel López-Ibáñez from comment #19)
> (In reply to Vincent Lefèvre from comment #18)
> > This seems to be fixed in the trunk.
> 
> Is there an XPASS for gcc.dg/uninit-pr19430.c ?
> 
> Also, the testcase from bug 42079?

Clang warns for this in the FE without any optimization:

gcc.dg/uninit-pr19430.c:10:7: warning: variable 'j' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
  if (bar (i)) { 
      ^~~~~~~
gcc.dg/uninit-pr19430.c:15:10: note: uninitialized use occurs here
  return j;
         ^
gcc.dg/uninit-pr19430.c:10:3: note: remove the 'if' if its condition is always true
  if (bar (i)) { 
  ^~~~~~~~~~~~~
gcc.dg/uninit-pr19430.c:8:8: note: initialize the variable 'j' to silence this warning
  int j; /* { dg-warning "'j' may be used uninitialized in this function" "uninitialized" { xfail *-*-* } 8 } */
       ^
        = 0
Comment 21 Manuel López-Ibáñez 2013-11-21 04:39:04 UTC
$ ~/test1/205036M/build/gcc/cc1 -O1  -Wuninitialized  test.c -fdump-tree-all-all-lineno
$ cat test.c.139t.uninit1

foo (intD.6 iD.1789)
{
  intD.6 jD.1792;
  intD.6 _5;
  intD.6 _7;

;;   basic block 2, loop depth 0, count 0, freq 10000, maybe hot
;;    prev block 0, next block 5, flags: (NEW, REACHABLE)
;;    pred:       ENTRY [100.0%]  (FALLTHRU,EXECUTABLE)
;;   starting at line 10
  [test.c : 10:11] # .MEM_4 = VDEF <.MEM_2(D)>
  # USE = nonlocal { D.1792 } (escaped)
  # CLB = nonlocal { D.1792 } (escaped)
  _5 = barD.1786 (i_3(D));
  [test.c : 10:6] if (_5 != 0)
    goto <bb 3>;
  else
    goto <bb 5>;
;;    succ:       3 [39.0%]  (TRUE_VALUE,EXECUTABLE)
;;                5 [61.0%]  (FALSE_VALUE,EXECUTABLE)

;;   basic block 5, loop depth 0, count 0, freq 6102, maybe hot
;;    prev block 2, next block 3, flags: (NEW)
;;    pred:       2 [61.0%]  (FALSE_VALUE,EXECUTABLE)
;;
  goto <bb 4>;
;;    succ:       4 [100.0%]  (FALLTHRU)

;;   basic block 3, loop depth 0, count 0, freq 3898, maybe hot
;;    prev block 5, next block 4, flags: (NEW, REACHABLE)
;;    pred:       2 [39.0%]  (TRUE_VALUE,EXECUTABLE)
;;   starting at line 11
  [test.c : 11:9] # .MEM_6 = VDEF <.MEM_4>
  # USE = nonlocal { D.1792 } (escaped)
  # CLB = nonlocal { D.1792 } (escaped)
  bazD.1788 ([test.c : 11] &jD.1792);
;;    succ:       4 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 4, loop depth 0, count 0, freq 10000, maybe hot
;;    prev block 3, next block 1, flags: (NEW, REACHABLE)
;;    pred:       5 [100.0%]  (FALLTHRU)
;;                3 [100.0%]  (FALLTHRU,EXECUTABLE)
;;   starting at line 14
  # .MEM_1 = PHI <.MEM_4(5), .MEM_6(3)>
  [test.c : 14:3] # VUSE <.MEM_1>
  _7 = jD.1792;
  # .MEM_8 = VDEF <.MEM_1>
  jD.1792 ={v} {CLOBBER};
  [test.c : 14:3] # VUSE <.MEM_8>
  return _7;
;;    succ:       EXIT [100.0%]

}

It seems one could follow every operand of the MEM_1 PHI node until you find the MEM_4 = VDEF <.MEM_2(D)>. But perhaps this may produce a lot of false positives.

The uninit pass avoids virtual operands on purpose, so it cannot be that trivial.
Comment 22 Vincent Lefèvre 2013-11-21 10:33:02 UTC
(In reply to Manuel López-Ibáñez from comment #19)
> (In reply to Vincent Lefèvre from comment #18)
> > This seems to be fixed in the trunk.
> 
> Is there an XPASS for gcc.dg/uninit-pr19430.c ?

Actually, with trunk revision 203899, only the last 3 tests now get the warning (those corresponding to -Wuninitialized). The first one (which should be a "may be uninitialized") is still silent.
Comment 23 Vincent Lefèvre 2013-11-21 10:39:34 UTC
BTW, I suppose that in this test, -Wuninitialized should be changed to "-Wuninitialized -Wmaybe-uninitialized" in case it is decided later that -Wuninitialized no longer enables -Wmaybe-uninitialized (see PR59223 about that).
Comment 24 Richard Biener 2013-11-21 12:16:30 UTC
Well, fact is we still don't have the machinery that warns for the use of
uninitialized memory - and we never had.  The cases that work by
accident are for loads that alias-analysis can prove we can move to
the function entry (well, a little less than that).
Comment 25 Manuel López-Ibáñez 2013-11-21 18:31:31 UTC
(In reply to Vincent Lefèvre from comment #23)
> BTW, I suppose that in this test, -Wuninitialized should be changed to
> "-Wuninitialized -Wmaybe-uninitialized" in case it is decided later that
> -Wuninitialized no longer enables -Wmaybe-uninitialized (see PR59223 about
> that).

I don't see any reason for -Wuninitialized to not enable -Wmaybe-uninitialized.
Comment 26 Vincent Lefèvre 2013-11-21 23:57:18 UTC
(In reply to Manuel López-Ibáñez from comment #25)
> I don't see any reason for -Wuninitialized to not enable
> -Wmaybe-uninitialized.

I can see 3 kinds of use:

1. Users who are interested in neither: they just don't use these options (if they want to use -Wall, they need the -Wno- version).

2. Users interested in -Wuninitialized but not in -Wmaybe-uninitialized (to avoid potential many false positives). Because -Wuninitialized enables -Wmaybe-uninitialized, they need 2 options: -Wuninitialized -Wno-maybe-uninitialized. If -Wuninitialized did not enable -Wmaybe-uninitialized, only one option would be needed: -Wuninitialized.

3. Users interested in both. I think that -Wmaybe-uninitialized should enable -Wuninitialized because it makes no sense to have -Wmaybe-uninitialized but not -Wuninitialized. Indeed, if some variable is uninitialized, then it may be uninitialized. So, only one option should be needed in this case: -Wmaybe-uninitialized.
Comment 27 Manuel López-Ibáñez 2013-11-22 02:26:47 UTC
(In reply to Vincent Lefèvre from comment #26)
> (In reply to Manuel López-Ibáñez from comment #25)
> > I don't see any reason for -Wuninitialized to not enable
> > -Wmaybe-uninitialized.
> 
> I can see 3 kinds of use:
> 
> 1. Users who are interested in neither: they just don't use these options
> (if they want to use -Wall, they need the -Wno- version).

-Wall -Wno-uninitialized suppresses both.


> 2. Users interested in -Wuninitialized but not in -Wmaybe-uninitialized (to
> avoid potential many false positives). Because -Wuninitialized enables
> -Wmaybe-uninitialized, they need 2 options: -Wuninitialized
> -Wno-maybe-uninitialized. If -Wuninitialized did not enable
> -Wmaybe-uninitialized, only one option would be needed: -Wuninitialized.

This argument goes both ways: if you are interested in both warnings, you would need both options.

> 3. Users interested in both. I think that -Wmaybe-uninitialized should
> enable -Wuninitialized because it makes no sense to have
> -Wmaybe-uninitialized but not -Wuninitialized. Indeed, if some variable is
> uninitialized, then it may be uninitialized. So, only one option should be
> needed in this case: -Wmaybe-uninitialized.

Aha! This is a really good point on which I can agree. However, it will seriously break backwards compatibility for people that use either -Wuninitialized or -Wno-uninitialized explicitly. I am not sure if the pain is worth the beauty.

I don't think it is worth to discuss this in Bugzilla (specially not on this PR). If you are convinced that this is the right thing to do, you could propose a patch to gcc-patches and see how people receive it.
Comment 28 Manuel López-Ibáñez 2015-02-23 22:46:38 UTC
*** Bug 65182 has been marked as a duplicate of this bug. ***