8

I was reading some code from a book, when I decided to make a change to see what the uninitialized value of sec would be before the while statement:

#include<stdio.h>

#define S_TO_M 60

int main(void)
{
    int sec,min,left;

    printf("This program converts seconds to minutes and ");
    printf("seconds. \n");
    printf("Just enter the number of seconds. \n");
    printf("Enter 0 to end the program. \n");
    printf("sec = %d\n",sec);
    while(sec > 0)
    {
        scanf("%d",&sec);
        min = sec/S_TO_M;
        left = sec % S_TO_M;
        printf("%d sec is %d min, %d sec. \n",sec,min,left);
        printf("Next input?\n");
    }
    printf("Bye!\n");

    return 0;
}

This compiles under GCC with no warnings, even though sec is uninitialized at that point, and I get a value of 32767:

$ gcc -Wall test.c
$ ./a.out 
This program converts seconds to minutes and seconds. 
Just enter the number of seconds. 
Enter 0 to end the program. 
sec = 32767

But when I comment out the while statement:

#include<stdio.h>

#define S_TO_M 60

int main(void)
{
    int sec;
    //min,left;

    printf("This program converts seconds to minutes and ");
    printf("seconds. \n");
    printf("Just enter the number of seconds. \n");
    printf("Enter 0 to end the program. \n");
    printf("sec = %d\n",sec);
    /*
    while(sec > 0)
    {
        scanf("%d",&sec);
        min = sec/S_TO_M;
        left = sec % S_TO_M;
        printf("%d sec is %d min, %d sec. \n",sec,min,left);
        printf("Next input?\n");
    }
    */
    printf("Bye!\n");

    return 0;
}

Now GCC issues a warning and sec ends up being zero:

$ gcc -Wall test.c
test.c: In function ‘main’:
test.c:12:8: warning: ‘sec’ is used uninitialized in this function[-Wuninitialized]
printf("sec = %d\n",sec);
    ^
$ ./a.out
This program converts seconds to mintutes and seconds. 
Just enter the number of seconds. 
Enter 0 to end the program. 
sec = 0
Bye!

Why did the warning show up the second time but not the first time?

Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
tiancantudou
  • 119
  • 3
  • If I am not wrong then emitting warnings is compiler's behavior (implementation defined) , may not be defined in language standard) – Grijesh Chauhan Mar 28 '15 at 05:04
  • Because the warning isn't perfect. – hobbs Mar 28 '15 at 05:04
  • You aren't initializing sec to anything. So it's just a random piece of memory. – Shawnic Hedgehog Mar 28 '15 at 05:05
  • 1
    You should add *gcc* version (as printed with the right command line switch). Also try with adding `-Wextra` – hyde Mar 28 '15 at 05:20
  • See also: http://stackoverflow.com/questions/17705880/gcc-failing-to-warn-of-uninitialized-variable – vanza Mar 28 '15 at 05:33
  • 1
    In my environment I get the warning for both posted codes when using `gcc -Wall `. I am curious since I cannot seem to test it, does GCC complain about the first code when `int sec,min,left;` is replaced with `int sec;`? – asimes Mar 28 '15 at 05:34
  • 1
    GCC 4.9.2 on Mac behaves as described in the question. – Jonathan Leffler Mar 28 '15 at 05:46
  • 1
    @JonathanLeffler, I should have mentioned the version, I am using GCC 4.2.1 on Mac **Edit**: although I was more curious if the declarations difference matters – asimes Mar 28 '15 at 05:48
  • 2
    Voting to reopen because this seems different than the other question in a small but important way. In the other question, there isn't a clear reference before the loop that requires deeper path analysis. Here, it seems particularly odd that deleting code _after_ the error helps the compiler find the error. – Adrian McCarthy Mar 28 '15 at 14:35

2 Answers2

8

Apparently gcc has a long running problem with false negatives with this option see this bug report: Bug 18501 - [4.8/4.9/5 Regression] Missing 'used uninitialized' warning (CCP) and the long list of duplicates:

Duplicates: 30542 30575 30856 33327 36814 37148 38945 39113 40469 42724 42884 45493 46684 46853 47623 48414 48643 49971 56972 57629 58323 58890 59225 60444

Note from comment towards the end, we can see this has been an ongoing issue for over ten years:

This year will be the 10 year anniversary of this bug. We should order cake!

Note, if you remove the:

scanf("%d",&sec);

you will also receive a warning, Marc Glisse also points out removing the first four printf also works as well (see it live). I don't see a similar example in the duplicates but not sure it is work adding yet another bug report for this.

Also see Better Uninitialized Warnings which says:

GCC has the ability to warn the user about using the value of a uninitialized variable. Such value is undefined and it is never useful. It is not even useful as a random value, since it rarely is a random value. Unfortunately, detecting when the use of an uninitialized variable is equivalent, in the general case, to solving the halting problem. GCC tries to detect some instances by using the information gathered by optimisers and warns about them when the option -Wuninitialized is given in the command line. There are a number of perceived shortcomings in current implementation. First, it only works when optimisation is enabled through -O1, -O2 or -O3. Second, the set of false positives or negatives varies according to the optimisations enabled. This also causes high variability of the warnings reported when optimisations are added or modified between releases.

A side note, clang seems to catch this case just fine (see live example):

 warning: variable 'sec' is uninitialized when used here [-Wuninitialized]
printf("sec = %d\n",sec);
                    ^~~

As I note in a comment below at least this case seems to be fixed in gcc 5.0.

peterh
  • 11,875
  • 18
  • 85
  • 108
Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
  • It does not look related to that PR. If you replace the whole loop with just one `scanf("%d",&sec);` (no condition/loop left anywhere), the warning is still missing. The reason is that the address of `sec` is taken, which inhibits the optimization that usually triggers the warning. – Marc Glisse Mar 28 '15 at 06:34
  • @MarcGlisse they seemed similar but since no rationale is given then perhaps you are correct, I have removed that reference. – Shafik Yaghmour Mar 28 '15 at 06:42
  • It also warns if you remove the first 4 printf (they make it slightly harder for the compiler to notice that nothing writes to `sec` beforehand). – Marc Glisse Mar 28 '15 at 09:33
  • @MarcGlisse it looks like at least this case is [fixed in 5.0](http://melpon.org/wandbox/permlink/KSq9YKFRuDwX1A8D) – Shafik Yaghmour Mar 28 '15 at 12:22
5

With this line:

scanf("%d",&sec);

GCC knows that you initialize sec somewhere in your function.

As a separate step, GCC can do flow analysis to figure out whether you initialize sec before using it. Unfortunately, GCC will not always do the flow analysis you want, or even do flow analysis at all. Sometimes the flow analyis is done at a stage where the other information is not available. So, you cannot count on GCC to figure it out.

Other compilers will figure it out. For example,

~ $ clang-3.7 -c -Wall -Wextra -O2 ex.c                      
ex.c:11:25: warning: variable 'sec' is uninitialized when used here
      [-Wuninitialized]
    printf("sec = %d\n",sec);
                        ^~~
ex.c:5:12: note: initialize the variable 'sec' to silence this warning
    int sec,min,left;
           ^
            = 0
1 warning generated.

It just so happens that GCC is very bad at detecting these errors.

Dietrich Epp
  • 205,541
  • 37
  • 345
  • 415
  • 3
    Indeed, `scanf` prevents from treating `sec` as a value (i.e. forget its address), and tree-ssa-uninit.c contains a comment saying that it should use `walk_aliased_vdefs` (do some flow/alias analysis) but doesn't, so it thinks the early `printf` may write to `sec`. – Marc Glisse Mar 28 '15 at 09:46
  • Perhaps more importantly, reading an uninitialized variable is only Undefined Behavior if either (1) the type supports trap representations, or (2) the address of the variable is not taken anywhere (the wording of the rule makes it clear that it doesn't matter whether the code which takes the address is actually executed). – supercat Nov 05 '16 at 22:57