58

All C compilers I've tried won't detect uninitialized variables in the code snippet below. Yet the case is obvious here.

Don't bother about the functionality of this snippet. It's not real code, and I stripped it down for the investigation of this issue.

BOOL NearEqual (int tauxprecis, int max, int value)
{
  int tauxtrouve;      // Not initialized at this point
  int totaldiff;       // Not initialized at this point

  for (int i = 0; i < max; i++)
  {
    if (2 < totaldiff)  // At this point totaldiff is not initialized
    {
      totaldiff = 2;
      tauxtrouve = value;  // Commenting this line out will produce warning
    }
  }

  return tauxtrouve == tauxprecis ;  // At this point tauxtrouve is potentially
                                     // not initialized.
}

On the other hand, if I comment out tauxtrouve = value ;, I get the "local variable 'tauxtrouve' used without having been initialized" warning.

I tried these compilers:

  • GCC 4.9.2 with -Wall -WExtra
  • Microsoft Visual C++ 2013 with all warnings enabled
10
  • 2
    I have no idea but maybe its the compiler optimization? I'm also eager to know. Hope we'll get the answer soon. Nov 21, 2014 at 14:35
  • What happens if you add the -pedantic flag to gcc?
    – avgvstvs
    Nov 21, 2014 at 14:35
  • 1
    I'm not familiar with how initialization is tested, but check out gcc.gnu.org/wiki/Better_Uninitialized_Warnings which I found by googling. Especially CCP assumes a value for uninitialized variables. If I was sure of myself I would have made this comment and answer.
    – Peter M
    Nov 21, 2014 at 14:42
  • 5
    Visual Studio 2013 tells me "error C4700: uninitialized local variable 'totaldiff' used". Nov 21, 2014 at 15:07
  • 1
    stackoverflow.com/q/25151508/541686
    – user541686
    Nov 21, 2014 at 19:08

4 Answers 4

65

The obviousness with which this variable is not initialized is overstated. Path analysis costs time and your compiler vendors either didn't want to implement the feature or thought it would cost you too much time -- or you just didn't explicitly opt-in.

For example, with clang:

$ clang -Wall -Wextra -c obvious.c 
$ clang -Wall -Wextra --analyze -c obvious.c 
obvious.c:9:11: warning: The right operand of '<' is a garbage value
    if (2 < totaldiff)  // at this point totaldiff is not initialized
          ^ ~~~~~~~~~
obvious.c:16:21: warning: The left operand of '==' is a garbage value
  return tauxtrouve == tauxprecis ;  // at this point tauxtrouve is potentially
         ~~~~~~~~~~ ^
2 warnings generated.

The difference in execution time for these naïve examples is negligible. But imagine a translation unit with thousands of lines, tens of functions, each with loops and heavy nesting. The number of paths quickly compounds and becomes a large burden to analyze whether or not the first iteration through the loop whether the assignment will occur prior to that comparison.


EDIT: @Matthieu points out that with LLVM/clang, the path analysis required to find use-of-uninitialized value does not compound as nesting increases because of the SSA notation used by the IR.

It wasn't as simple as "-S -emit-llvm" like I'd hoped, but I found the SSA-notation output he described. I'll be honest, I'm not familiar enough with LLVM IR to be sure, but I'll take Matthieu's word for it.

Bottom line: use clang with --analyze, or convince someone to fix the gcc bug.

; Function Attrs: nounwind uwtable
define i32 @NearEqual(i32 %tauxprecis, i32 %max, i32 %value) #0 {
  br label %1

; <label>:1                                       ; preds = %7, %0
  %tauxtrouve.0 = phi i32 [ undef, %0 ], [ %tauxtrouve.1, %7 ]
  %i.0 = phi i32 [ 0, %0 ], [ %8, %7 ]
  %2 = icmp slt i32 %i.0, %max
  br i1 %2, label %3, label %9

; <label>:3                                       ; preds = %1
  %4 = icmp slt i32 2, 2
  br i1 %4, label %5, label %6

; <label>:5                                       ; preds = %3
  br label %6

; <label>:6                                       ; preds = %5, %3
  %tauxtrouve.1 = phi i32 [ %value, %5 ], [ %tauxtrouve.0, %3 ]
  br label %7

; <label>:7                                       ; preds = %6
  %8 = add nsw i32 %i.0, 1
  br label %1

; <label>:9                                       ; preds = %1
  %10 = icmp eq i32 %tauxtrouve.0, %tauxprecis
  %11 = zext i1 %10 to i32
  ret i32 %11
}
6
  • 7
    Do you have figures regarding the degradation? For your information, LLVM IR is based on SSA notation which, as the name implies, only allows a single assignment to any single variable (often, the variable name in the source code is reused suffixed by an index). As a result, the paths are necessarily materialized, and checking whether a variable is potentially used uninitialized is thus trivial: for each variable, when computing its assignment, check whether it is definitely initialized, potentially uninitialized or definitely uninitialized. Upon use, warn appropriately. Nov 21, 2014 at 17:42
  • 1
    @Matthieu Oh actually by far not as trivial as one might think. [See here](gist.github.com/voo42/293d3cafa820f8c86e54 for a simple example where your proposal would give spurious warnings. Certainly doable, but just using SSA form is not enough. I'm actually not sure if there's a general solution that doesn't involve checking all possible paths through a function.
    – Voo
    Nov 22, 2014 at 22:54
  • 1
    @Voo there isn't one which doesn't involve running into HP. Consider if (foo(0)) { i = 0 } if (bar(0)) { j = i }. But this requires that bar(0) => foo(0) so we need to know the values bar(0) and foo(0) evaluates to - which isn't possible without actually running a program (and hoping it terminates) due to HP. Nov 23, 2014 at 0:00
  • 1
    @Voo: Oh, I am not, AT ALL, implying that 100% accurate prediction is possible. That is why I have 3 states: the "potentially uninitialized" is exactly for the condition you describe. And that is why Clang has 2 warnings: -Wuninitialized (activated by default) only gives warnings for variables that are definitely uninitialized while -Wmaybe-uninitialized (not activated by default) only gives warnings for variables that are potentially uninitialized. So yes, the latter might give spurious warnings for convoluted code; I would argue you better fix the code, but you can ignore it. Nov 23, 2014 at 11:34
  • The IR is actually simple, if you ignore the noise :) For example, there are here 2 appearances of tauxtrouve: tauxtrouve.0 and tauxtrouve.1. The phi magic selects the value depending on the predecessor: phi i32 [ %value, %5 ], [ %tauxtrouve.0, %3 ] yields a value of type i32, if the predecessor was the label named 5 then it yields %value and if it was the one named 3 it yields tauxtrouve.0; undef is a special value to represent the lack of initialization, and br is a conditional goto... Nov 23, 2014 at 11:41
50

Yes, it should raise a warning about that uninitialized variable, but it's a GCC bug. The example given there is:

unsigned bmp_iter_set ();
int something (void);

void bitmap_print_value_set (void)
{
    unsigned first;

    for (; bmp_iter_set (); )
    {
        if (!first)
            something ();
        first = 0;
    }
}

And diagnosed with -O2 -W -Wall.

Unfortunately, this year is the 10 year anniversary of this bug!

10
  • 4
    Geez .. a ten year old bug, with a list of comments that reads like the embodiment of the worst fears of snark-iness in an OSS project
    – Peter M
    Nov 21, 2014 at 14:49
  • 3
    @PeterM; Unfortunately this 10 year old bug is not yet fixed!
    – haccks
    Nov 21, 2014 at 14:51
  • 4
    #WontFix #NotSexyEnoughToWorryAbout
    – Peter M
    Nov 21, 2014 at 14:53
  • 13
    @Lundin you write in your answer that you "can't expect" the compiler to detect bugs and that "the compiler only has two tasks". In fact, that was correct 20 years ago, not so correct nowadays. Compilers have been evolved enormously, and these days people rely on compilers to detect obvious bugs. Despite this having undefined behavior, it would be nice (and expected) that the compiler generates a warning. Not in order to make UB defined, but to let the programmer know he's doing it wrong. This may not be a technical bug, but it definitely is a user experience bug. Nov 21, 2014 at 15:12
  • 4
    I see myself reading this in the future: "..this bug was not solved due to historical reasons." Nov 26, 2014 at 13:17
12

This answer only addresses GCC.

After further investigation and comments, there is more going on than in my previous answer. This code snippet has two uninitialized variables, and each of them is undetected for a different reason.

First of all, the GCC documentation for the -Wuninitialized option says:

Because these warnings depend on optimization, the exact variables or elements for which there are warnings depends on the precise optimization options and version of GCC used.

Previous versions of the GCC manual worded this more explicitly. Here's an excerpt from the manual for GCC 3.3.6:

These warnings are possible only in optimizing compilation, because they require data flow information that is computed only when optimizing. If you don't specify -O, you simply won't get these warnings.

See also GCC - no warning about an uninitialized array with -O0.

It seems the current version may give some warnings without uninitialized variables without -O, but you still get much better results with it.

If I compile your example using gcc -std=c99 -Wall -O, I get:

foo.c: In function ‘NearEqual’:
foo.c:15:21: warning: ‘tauxtrouve’ is used uninitialized in this function [-Wuninitialized]
   return tauxtrouve == tauxprecis ;  // at this point tauxtrouve is potentially
                     ^

(Note this is with GCC 4.8.2 as I don't have 4.9.x installed, but the principle should be the same.)

So that detects the fact that tauxtrouve is uninitialized.

However, if we partially fix the code by adding an initializer for tauxtrouve (but not for totaldiff), then gcc -std=c99 -Wall -O accepts it without any warnings. This would appear to be an instance of the "bug" cited in haccks's answer.

There is some question as to whether this should really be considered a bug: GCC doesn't promise to catch every possible instance of an uninitialized variable. Indeed, it can't do so with perfect accuracy, because that's the halting problem. So warnings like this can be helpful when they work, but the absence of warnings does not prove that your code is free of uninitialized variables! They are really not a substitute for carefully checking your own code.

In the bug report linked by haccks, there is much discussion as to whether the bug is even fixable, or whether trying to detect this particular construct would result in an unacceptable false positive rate for other correct code.

2
  • 1
    I believe the OP is more concerned about detecting totaldiff not being initialized.
    – Peter M
    Nov 21, 2014 at 19:12
  • @PeterM: Good point. There are actually two separate issues here and I've rewritten my answer to address both. Nov 22, 2014 at 18:42
2

Michael, I don't know which version of Visual Studio 2013 you tried this on, but it is most certainly outdated. Visual Studio 2013 Update 4 correctly produces the following error message on the first use of totaldiff:

error C4700: uninitialized local variable 'totaldiff' used

You should consider updating your work environment.

By the way, here is what I see directly in the editor:

Visual Studio 2013 caught the error

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.