Buried in warnings [LWN.net]

Buried in warnings

Posted Nov 2, 2006 5:20 UTC (Thu) by JoeBuck (subscriber, #2330)
Parent article: Buried in warnings

A typical example of a false uninitialized variable warning gcc will generate is the following:

bool flag = false;
some_type* pointer;

if (some_condition_is_true()) {
   flag = true;
   pointer = expensive_allocation_function();
}
do_something_else();
if (flag) {
   use_the_fine(pointer);
}

GCC will report that pointer might be used uninitialized, because it does not track the association between the flag variable and the state of pointer. Doing checks of this kind would require rather sophisticated analysis; gated static single assignment would work, but even then, there are cases that a human being can immediately see that the compiler will not. However, use of an uninitialized object can be such a disaster that most consider false positives better than false negatives.

Sometimes suppressing the warning will require a minor time and space penalty: say, an instruction to set some object to zero. Unless you're dealing with the most time-critical of inner loops, I suggest that you're better off trying for a clean compilation with -Wall, even if there is a minor cost, because a microscopically faster but more buggy program isn't worth the cost.

to post comments

Buried in warnings

Posted Nov 2, 2006 6:26 UTC (Thu) by viro (subscriber, #7872) [Link] (4 responses)

There is such thing as bogusly initialized variable. And I would
argue that it's worse than even genuine uninitialized one; the latter
at least gets you a warning. The former is hidden...

Buried in warnings

Posted Nov 2, 2006 7:26 UTC (Thu) by avik (guest, #704) [Link] (3 responses)

The latter gets you a warning and data corruption. The former is hidden,
and maybe wastes a cycle when run.

I think that a bogus initialization is better than a genuine uninitialized
variable. It's only worse if you never run the kernel in question.

Buried in warnings

Posted Nov 2, 2006 8:48 UTC (Thu) by viro (subscriber, #7872) [Link] (2 responses)

Bogus as in "with value that doesn't make sense". Suppose you used
to have a declaration, then several places assigning to variable,
then several places using it. All paths to the latter actually
pass through the former, so we are fine. gcc is too dumb to prove
that, so it gives a warning. Fine, some kind idio^Wsoul slaps = 0
into declaration. Everything's fine. Until a modification of code
creates a path that *really* does use without assignment. Suddenly
(and without any warning from gcc whatsoever) we get a case that
gets us to use of variable when utter crap is stored in it; the crap
in question is that 0 supplied by helpful idiot several months ago.

Worse yet, code review finding a code path that leads to use without
assignment => OK, we've definitely found a bug. Code review finding
a code path that leads to use of variable explicitly initialized with
something that doesn't make much sense in that place => scratching
head for a long time and trying to figure out whether it's a bug or
not and WTBleedingF was supposed to be done in that place.

Buried in warnings

Posted Nov 2, 2006 15:16 UTC (Thu) by evgeny (subscriber, #774) [Link]

I'd vote for the bogus initialization. If it does cause a bug later on, it is at least reproducible. Tracking down uninitialized vars typically takes much longer. YMMV, of course.

Buried in warnings

Posted Nov 2, 2006 16:58 UTC (Thu) by nevyn (guest, #33129) [Link]

You are saying that the uninitialized vars aren't hidden, but this entire article proves otherwise. There are so many warnings when you compile the kernel that noone is looking at the ones that are being output.

Personally I think there is a huge amount of middle ground, for instance the example code Joe posted could declare the pointer as NULL allocate to it in the if and then just check if the pointer is not NULL later on (Ie. pointer also takes on the job of the seperate boolean).

Finally with decent usage of ASSERT/nonnull you can _very_ easily detect when pointers are still NULL from declaration time.

Buried in warnings

Posted Nov 2, 2006 10:46 UTC (Thu) by Asebe8zu (subscriber, #24600) [Link] (3 responses)

Couldn't you use the following to make it correct?

some_type* pointer;

if (some_condition_is_true()) {
   pointer = expensive_allocation_function();
   do_something_else();
   use_the_fine(pointer);
}
else
{
   do_something_else();
}

Buried in warnings

Posted Nov 2, 2006 15:05 UTC (Thu) by etienne_lorrain@yahoo.fr (guest, #38022) [Link] (2 responses)

That is just what you did not want to do, for instance when do_something_else() will be inlined if it is called once...

Either you would replace:
{
if (some_condition_is_true()) {
some_type* pointer = expensive_allocation_function();
do_something_else();
use_the_fine(pointer);
} else {
do_something_else();
}
}

By (definning a new GCC extern_local keyword):
{
if (some_condition_is_true()) {
some_type* pointer = expensive_allocation_function();
}
do_something_else();
if (some_condition_is_true()) {
extern_local some_type* pointer;
use_the_fine(pointer);
}
}

Or by a new GCC attribute:
{
some_type* pointer __attribute__((used_if(some_condition_is_true())));

if (some_condition_is_true())
pointer = expensive_allocation_function();
do_something_else();
if (some_condition_is_true())
use_the_fine(pointer);
}

By the way, to kill "used if not initialised" warning, you do it by
the recognised: "variable = variable;" feature, not by "variable = 0;".

Buried in warnings

Posted Nov 2, 2006 16:44 UTC (Thu) by NAR (subscriber, #1313) [Link]

By the way, to kill "used if not initialised" warning, you do it by the recognised: "variable = variable;" feature, not by "variable = 0;".

I think this still hides the case when a new execution path is added and the variable is failed to get initialized in the new path. I guess the real solution is to initialize at declaration (i.e. with a constructor) and do whatever case handling needed in that constructor - altough I'm not sure it can be done easily in C.

Bye,NAR

Buried in warnings

Posted Nov 3, 2006 1:07 UTC (Fri) by nix (subscriber, #2304) [Link]

I hope that's an assertion. you can't verify that attribute in the general
case without not just solving the halting problem but foretelling the
future. :)

And in that case you've now moved the problem from bogus initializations
to bogus assertions. The problem remains.

(And it's insoluble in the general case, anyway. Hence GCC's use of the
word `may', to emphasise that FPs from this warning are a
quality-of-implementation issue, sure, but not a compiler bug.)

Buried in warnings

Posted Nov 2, 2006 16:10 UTC (Thu) by NAR (subscriber, #1313) [Link] (3 responses)

I know it's just a made-up code, but if the pointer is really not used in do_something_else(), couldn't you just write:

do_something_else();
if (some_condition_is_true()) {
   some_type* pointer = expensive_allocation_function();
   use_the_fine(pointer);
}

I guess if the pointer is not used in do_something_else(), the function can't really use the things allocated there. Or does this syntax only work in C++? In this case it's too bad...

Anyway, just to reflect to the questioning of code quality in closed software (in a separate thread), we've had quarter-million lines of C++ code compiling without a single warning...

Bye,NAR

Buried in warnings

Posted Nov 3, 2006 1:09 UTC (Fri) by nix (subscriber, #2304) [Link] (2 responses)

C99 supports this. C90 does not.

C90 vs C99

Posted Nov 3, 2006 1:24 UTC (Fri) by xoddam (subscriber, #2322) [Link] (1 responses)

Not so. Declarations with initialisations have always been legal at the
beginning of a block.

C99 also permits declarations to follow statements, like C++.

C90 vs C99

Posted Nov 14, 2006 20:52 UTC (Tue) by nix (subscriber, #2304) [Link]

In C90 the initializations must be literals: you can't put an arbitrary
function call in there and expect it to work. (That's a GNU C extension,
copied from C++ and also found in C99: look up `Non-Constant Initializers'
in the manual.)

Reconvergent fanout

Posted Nov 2, 2006 16:19 UTC (Thu) by jreiser (subscriber, #11027) [Link] (2 responses)

The situation JoeBuck describes is a kind of "re-convergent fanout" which the people who do static timing analysis for digital circuit design figured out how to handle 20 years ago.

Reconvergent fanout

Posted Nov 3, 2006 2:59 UTC (Fri) by pimlott (guest, #1535) [Link] (1 responses)

A few more minutes of writing might have made that post a whole lot more educational for the rest of us. :-)

Reconvergent fanout

Posted Nov 3, 2006 19:40 UTC (Fri) by bronson (subscriber, #4806) [Link]

I don't think so... Reconvergent fanout is similar to this problem much like a banana is similar to a school bus because they are yellow.

Reconvergent fanout is a way to make VLSI static timing analysis less pessimistic. Each time a signal passes thorugh a node, a bit of uncertainty is added to the timing delay. However, if two (ore more) signals pass through the same node, follow separate paths, and then return to the same point, you can remove a fair amount of uncertainty because you know that one point they shared a common path. Whatever the uncertainty was, it was identical for both.

So, yes, both techniques trace signals through a system. Beyond that, though, it seems to me that they're totally different. I hope somebody will clue me in if I'm missing something; it's been many years since I was a VLSI hack.