By Jonathan Corbet
December 19, 2012
Compiler warnings can be life savers for kernel developers; often a
well-placed warning will help to avert a bug that, otherwise, could have
been painful to track down. But developers quickly tire of warnings that
appear when the relevant code is, in fact, correct. It does not take too
many spurious warnings to cause a developer to tune out compiler warnings
altogether. So developers will often try to suppress warnings for correct
code — a practice which can have undesirable effects in the longer term.
GCC will, when run with suitable options, emit a warning if it believes
that the value of a variable might be used before that variable is set.
This warning is based on the compiler's analysis of the paths through a
function; if it believes it can find a path where the variable is not
initialized, an "uninitialized variable" warning will result. The problem
is that the compiler is not always smart enough to know that a specific
path will never be taken. As a simple example, consider
uhid_hid_get_raw() in drivers/hid/uhid.c:
size_t len;
/* ... */
return ret ? ret : len;
A look at the surrounding code makes it clear that, in the case where
ret is set to zero, the value of len has been set
accordingly. But the compiler is unable to figure that out and warns that
len might be used in an uninitialized state.
The obvious response to such a warning is to simply change the declaration
of len so that the variable starts out initialized:
size_t len = 0;
Over the years, though, this practice has been discouraged on the kernel
mailing lists. The unneeded initialization results in larger code and a
(slightly) longer run time. And, besides, it is most irritating to be
pushed around by a compiler that is not smart enough to figure out that the
code is correct; Real Kernel Hackers don't put up with that kind of thing.
So, instead, a special macro was added to the kernel:
/* <linux/compiler-gcc.h> */
#define uninitialized_var(x) x = x
It is used in declarations in this manner:
size_t uninitialized_var(len);
This macro has the effect of suppressing the warning, but it doesn't cause
any additional code to be generated by the compiler. This macro has proved
reasonably popular; a quick grep shows over 280 instances in the 3.7+
mainline repository. That popularity is not surprising: it allows a kernel
developer to
turn off a spurious warning and to document the fact that the use of the
variable is, indeed, correct.
Unfortunately, there are a couple of problems with
uninitialized_var(). One is that, at the same time that it is
fooling GCC into thinking that the variable is initialized, it is also
fooling it into thinking that the variable is used. If the variable is
never referenced again, the compiler will still not issue an "unused
variable" warning. So, chances are, there are a number of excess variables
that have not been removed because nobody has noticed that they are not
actually used. That is a minor irritation, but one could easily decide
that it is tolerable if it were the only problem.
The other problem, of course, is that the compiler might just be right.
During the 3.7 merge window, a
patch was merged that moved some extended attribute handling code from
the tmpfs filesystem into common code. In the process of moving that code,
the developer noticed that one variable initialization could be removed,
since, it seemed, it would pick up a value in any actual path through the
function. GCC disagreed, issuing a warning, so, when this developer wrote
a
second patch to remove the initialization, he also suppressed the
warning with uninitialized_var().
Unfortunately, GCC knew what it was talking about in this case; that code
had just
picked up a bug where, in a specific set of circumstances, an uninitialized
value would be passed to kfree() with predictably pyrotechnic
results. That bug had to be tracked down by
other developers; it was fixed by David
Rientjes on October 17. At that time, Hugh Dickins commented that it was a good example of how
uninitialized_var() can go wrong.
And, of course, this kind of problem need not be there from the outset.
The code for a given function might indeed be correct when
uninitialized_var() is employed to silence a warning. Future
changes could introduce a bug that the compiler would ordinarily warn
about, except that the warning will have been suppressed. So, in a sense,
every uninitialized_var() instance is a trap for the unwary.
That is why Linus threatened to remove it
later in October, calling it "an abomination" and saying:
The thing is moronic. The whole thing is almost entirely due to
compiler bugs (*stupid* gcc behavior), and we would have been
better off with an explicit (unnecessary) initialization that at
least doesn't cause random crashes etc if it turns out to be wrong.
In response, Ingo Molnar put together a
patch removing uninitialized_var() outright. Every use is
replaced with an actual initialization appropriate to the type of the
variable in question. A special comment
("/* GCC */") is added as well to make the
purpose of the initialization clear.
The patch was generally well received and appears to be ready to go. In
October, Ingo said that he would keep it
out of linux-next (to avoid creating countless merge conflicts), but would
post it for merging right at the end of the 3.8 merge window. As of this
writing, that posting has not occurred, but there have been no signs that
the plans have changed. So, most likely, the 3.8 kernel will lack the
uninitialized_var() macro and developers will have to silence
warnings the old-fashioned (and obviously correct) way.
(
Log in to post comments)