Doesn't "always initialize variables" lead to important bugs being hidden?

Question

The C++ Core Guidelines have the rule ES.20: Always initialize an object.

Avoid used-before-set errors and their associated undefined behavior. Avoid problems with comprehension of complex initialization. Simplify refactoring.

But this rule doesn't help to find bugs, it only hides them.
Let's suppose that a program has an execution path where it uses an uninitialized variable. It is a bug. Undefined behavior aside, it also means that something went wrong, and the program probably doesn't meet its product requirements. When it will be deployed to production, there can be a money loss, or even worse.

How do we screen bugs? We write tests. But tests don't cover 100% of execution paths, and tests never cover 100% of program inputs. More than that, even a test covers a faulty execution path - it still can pass. It's undefined behavior after all, an uninitialized variable can have a somewhat valid value.

But in addition to our tests, we have the compilers which can write something like 0xCDCDCDCD to uninitialized variables. This slightly improves detection rate of the tests.
Even better - there are tools like Address Sanitizer, which will catch all the reads of uninitialized memory bytes.

And finally there are static analyzers, which can look at the program and tell that there is a read-before-set on that execution path.

So we have many powerful tools, but if we initialize the variable - sanitizers find nothing.

int bytes_read = 0;
my_read(buffer, &bytes_read); // err_t my_read(buffer_t, int*);
// bytes_read is not changed on read error.
// It's a bug of "my_read", but detection is suppressed by initialization.
buffer.shrink(bytes_read); // Uninitialized bytes_read could be detected here.

// Another bug: use empty buffer after read error.
use(buffer);

There is another rule - if program execution encounters a bug, program should die as soon as possible. No need to keep it alive, just crash, write a crashdump, give it to the engineers for investigation.
Initializing variables unnecessarily does the opposite - program is being kept alive, when it would already get a segmentation fault otherwise.

Answer 1

Your reasoning goes wrong on several accounts:

1. Segmentation faults are far from certain to occur. Using an uninitialized variable results in undefined behaviour. Segmentation faults are one way that such behaviour can manifest itself, but appearing to run normal is just as likely.
2. Compilers never fill the uninitialized memory with a defined pattern (like 0xCD). This is something that some debuggers do to assist you in finding places where uninitialized variables get used. If you run such a program outside a debugger, then the variable will contain completely random garbage. It is equally likely that a counter like the bytes_read has the value 10 as that it has the value 0xcdcdcdcd.
3. Even if you are running in a debugger that sets the uninitialized memory to a fixed pattern, they only do so at startup. This means that this mechanism only works reliably for static (and possibly heap-allocated) variables. For automatic variables, which get allocated on the stack or live only in a register, the chances are high that the variable is stored in a location that was used before, so the tell-tale memory pattern has already been overwritten.

The idea behind the guidance to always initialize variables is to enable these two situations

1. The variable contains a useful value right from the very beginning of its existence. If you combine that with the guidance to declare a variable only once you need it, you can avoid future maintenance programmers falling in the trap of starting to use a variable between its declaration and the first assignment, where the variable would exist but be uninitialized.

2. The variable contains a defined value that you can test for later, to tell if a function like my_read has updated the value. Without initialization, you can't tell if bytes_read actually has a valid value, because you can't know what value it started with.

Answer 2

You wrote "this rule doesn't help to find bugs, it only hides them" - well, the goal of the rule is not to help finding bugs, but to avoid them. And when a bug is avoided, there is nothing hidden.

Lets dicuss the issue in terms of your example: suppose the my_read function has the written contract to initialize bytes_read under all circumstances, but it does not in case of an error, so it is faulty, at least, for this case. Your intention is to use the run time environment to show that bug by not initializing the bytes_read parameter first. As long as you know for sure there is an address sanitizer in place, that is a indeed a possible way to detect such a bug. To fix the bug, one has to change the my_read function internally.

But there is a different point of view, which is at least equally valid: the faulty behaviour only emerges from the combination of not initializing bytes_read beforehand, and calling my_read afterwards (with the expectation bytes_read is initialized after that). This is a situation which will happen often in real world components when the written spec for a function like my_read is not 100% clear, or even wrong about the behaviour in case of an error. However, as long bytes_read is initialized to zero before the call, the program behaves the same way as if the initialization was done inside my_read, so it behaves correctly, in this combination there is no bug in the program.

So my recommendation that follows from that is: use the non-initializing approach only if

• you want to test if a function or code block initializes a specific parameter
• you are 100% sure the function in stake has a contract where it is definitely wrong not to assign a value to that parameter
• you are 100% sure the environment can catch this

These are conditions you can typically arrange in test code, for a specific tooling environment.

In production code, however, better always initialize such a variable beforehand, it is the more defensive approach, which prevents bugs in case the contract is incomplete or wrong, or in case the address sanitizer or similar safety measures are not activated. And the "crash-early" rule applies, as you correctly wrote, if program execution encounters a bug. But when initializing a variable beforehand means there is nothing wrong, then there is no need to stop further execution.

Answer 3

Always initialize your variables

The difference between the situations you are considering is that the case without initialization results in undefined behavior, while the case where you took the time to initialize creates a well defined and deterministic bug. I cannot stress how extremely different these two cases are enough.

Consider a hypothetical example which may have happened to a hypothetical employee on a hypothetical simulations program. This hypothetical team was hypothetically trying to make a deterministic simulation to demonstrate that the product they were hypothetically selling met needs.

Okay, I'll stop with the word injections. I think you get the point ;-)

In this simulation, there were hundreds of uninitialized variables. One developer ran valgrind on the simulation and noticed there were several "branch on uninitialized value" errors. "Hmm, that looks like that could cause non-determinism, making it hard to repeat test runs when we need it most." The developer went to management, but management was on a very tight schedule, and couldn't spare resources to track down this issue. "We end up initializing all of our variables before we use them. We have good coding practices."

A few months before the final delivery, when the simulation is in full churn mode, and the entire team is sprinting to finish all the things management promised on a budget that, like every project ever funded, was too small. Someone noticed that they couldn't test an essential feature because, for some reason, the deterministic sim wasn't behaving deterministically to debug.

The entire team may have been halted and spent the better part of 2 month combing the entire simulation codebase fixing uninitialized value errors instead of implementing and testing features. Needless to say, the employee skipped the "I told you so's" and went straight into helping other developers understand what uninitialized values are. Strangely enough, the coding standards were changed shortly after this incident, encouraging developers to always initialize their variables.

And this is the warning shot. This is the bullet that grazed across your nose. The actual issue is far far far far far more insidious than you even imagine.

Using an uninitialized value is "undefined behavior" (except for a few corner cases such as char). Undefined behavior (or UB for short) is so insanely and completely bad for you, that you should never ever ever believe it is better than the alternative. Sometimes you can identify that your particular compiler defines the UB, and then its safe to use, but otherwise, undefined behavior is "any behavior the compiler feels like." It may do something you'd call "sane" like have an unspecified value. It may emit invalid opcodes, potentially causing your program to corrupt itself. It may trigger a warning at compile time, or the compiler may even consider it an error outright.

Or it may do nothing at all

My canary in the coal mine for UB is a case from a SQL engine that I read about. Forgive me for not linking it, I've failed to find the article again. There was a buffer overrun issue in the SQL engine when you passed a larger buffer size to a function, but only on a particular version of Debian. The bug got dutifully logged, and explored. The funny part was: the buffer overrun was checked. There was code to handle the buffer overrun in place. It looked something like this:

// move the pointers properly to copy data into a ring buffer.
char* putIntoRingBuffer(char* begin, char* end, char* get, char*put, char* newData, unsigned int dataLength)
{
    // If dataLength is very large, we might overflow the pointer
    // arithmetic, and end up with some very small pointer number,
    // causing us to fail to realize we were trying to write past the
    // end.  Check this before we continue
    if (put + dataLength < put)
    {
        RaiseError("Buffer overflow risk detected");
        return 0;
    }
    ...
    // typical ring-buffer pointer manipulation followed...
}

I've added more comments in my rendition, but the idea is the same. If put + dataLength wraps around, it will be smaller than the put pointer (they had compile time checks to make sure unsigned int was the size of a pointer, for the curious). If this happens, we know the standard ring buffer algorithms might get confused by this overflow, so we return 0. Or do we?

As it turns out, overflow on pointers is undefined in C++. Because most compilers are treating pointers as integers, we end up with typical integer overflow behaviors, which happen to be the behavior we want. However, this is undefined behavior, meaning the compiler is allowed to do anything it wants.

In the case of this bug, Debian happened to choose to use a new version of gcc that none of the other major Linux flavors had updated to in their production releases. This new version of gcc had a more aggressive dead-code optimizer. The compiler saw the undefined behavior, and decided the result of the if statement would be "whatever makes the code optimization best," which was an absolutely legal translation of UB. Accordingly, it made the assumption that since ptr+dataLength can never be below ptr without a UB pointer overflow, the if statement would never trigger, and optimized out the buffer overrun check.

The use of "sane" UB actually caused a major SQL product to have a buffer overrun exploit that it had written code to avoid!

Never rely on undefined behavior. Ever.

Answer 4

I mostly work in a functional programming language where you aren't allowed to reassign variables. Ever. That completely eliminates this class of bugs. This seemed like a huge restriction at first, but it forces you to structure your code in a way that is consistent with the order you learn new data, which tends to simplify your code and make it easier to maintain.

Those habits can be carried over into imperative languages as well. It is nearly always possible to refactor your code to avoid initializing a variable with a dummy value. That's what those guidelines are telling you to do. They want you to put something meaningful in there, not something that will just make automated tools happy.

Your example with a C-style API is a little more tricky. In those cases, when I use the function I'll initialize to zero to keep the compiler from complaining, but one time in the my_read unit tests, I'll initialize to something else to make sure the error condition works properly. You don't need to test every possible error condition upon every use.

Answer 5

No, it doesn't hide bugs. Instead it makes behavior deterministic in a way such that if a user encounters an error, a developer can reproduce it.

Answer 6

TL;DR: There are two ways to making this program correct, initializing your variables and praying. Only one delivers results consistently.

---

Before I can answer your question, I will need to first explain what Undefined Behavior means. Actually, I'll let a compiler author do the bulk of the work:

• What Every C Programmer Should Know About Undefined Behavior #1/3
• What Every C Programmer Should Know About Undefined Behavior #2/3
• What Every C Programmer Should Know About Undefined Behavior #3/3

If you are unwilling to read those articles, a TL;DR is:

Undefined Behavior is a social contract between the developer and the compiler; the compiler assumes with blind faith that its user will never, ever, rely on Undefined Behavior.

The archetype of "Demons flying from your nose" has utterly failed to convey the implications of this fact, unfortunately. While meant to prove that anything could happen, it was so utterly unbelievable that it was mostly shrugged off.

The truth, however, is that Undefined Behavior affects the compilation itself, long before you even attempt to use the program (instrumented or not, within a debugger or not) and can utterly change its behavior.

I find the example in part 2 above striking:

void contains_null_check(int *P) {
  int dead = *P;
  if (P == 0)
    return;
  *P = 4;
}

is transformed into:

void contains_null_check(int *P) {
  *P = 4;
}

because it's obvious that P cannot be 0 since it's dereferenced before being checked.

---

How does this apply to your example?

int bytes_read = 0;
my_read(buffer, &bytes_read); // err_t my_read(buffer_t, int*);
// bytes_read is not changed on read error.
// It's a bug of "my_read", but detection is suppressed by initialization.
buffer.shrink(bytes_read); // Uninitialized bytes_read could be detected here.

Well, you have made the common mistake of assuming that Undefined Behavior would cause a run-time error. It may not.

Let us imagine that the definition of my_read is:

err_t my_read(buffer_t buffer, int* bytes_read) {
    err_t result = {};
    int blocks_read = 0;
    if (!(result = low_level_read(buffer, &blocks_read))) { return result; }
    *bytes_read = blocks_read * BLOCK_SIZE;
    return result;
}

and proceed as expected of a good compiler with inlining:

int bytes_read; // UNINITIALIZED

// start inlining my_read

err_t result = {};
int blocks_read = 0;
if (!(result = low_level_read(buffer, &blocks_read))) {
    // nothing
} else {
    bytes_read = blocks_reads * BLOCK_SIZE;
}

// end of inlining my_read

buffer.shrink(bytes_read);

Then, as expected of a good compiler, we optimize out useless branches:

1. No variable should be used uninitialized
2. bytes_read would be used uninitialized if result was not 0
3. The developer is promising that result will never be 0!

So result is never 0:

int bytes_read; // UNINITIALIZED
err_t result = {};
int blocks_read = 0;
result = low_level_read(buffer, &blocks_read);

bytes_read = blocks_reads * BLOCK_SIZE;
buffer.shrink(bytes_read);

Oh, result is never used:

int bytes_read; // UNINITIALIZED
int blocks_read = 0;
low_level_read(buffer, &blocks_read);

bytes_read = blocks_reads * BLOCK_SIZE;
buffer.shrink(bytes_read);

Oh, we can postpone the declaration of bytes_read:

int blocks_read = 0;
low_level_read(buffer, &blocks_read);

int bytes_read = blocks_reads * BLOCK_SIZE;
buffer.shrink(bytes_read);

And here we are, a strictly confirming transformation of the original, and no debugger will trap an uninitialized variable because there is none.

I've been down that road, understanding the issue when expected behavior and assembly do not match is really no fun.

Answer 7

Let’s take a closer look at your example code:

int bytes_read = 0;
my_read(buffer, &bytes_read); // err_t my_read(buffer_t, int*);
// bytes_read is not changed on read error.
// It's a bug of "my_read", but detection is suppressed by initialization.
buffer.shrink(bytes_read); // Uninitialized bytes_read could be detected here.

// Another bug: use empty buffer after read error.
use(buffer);

This is a good example. If we anticipate an error such as this, we can insert the line assert(bytes_read > 0); and catch this bug at runtime, which is not possible with an uninitialized variable.

But suppose we don’t, and we find an error inside the function use(buffer). We load the program up in the debugger, check the backtrace, and find out that it was called from this code. So we put a breakpoint at the top of this snippet, run again, and reproduce the bug. We single-step through trying to catch it.

If we haven’t initialized bytes_read, it contains garbage. It doesn’t necessarily contain the same garbage each time. We step past the line my_read(buffer, &bytes_read);. Now, if it’s a different value than before, we might not be able to reproduce our bug at all! It might work the next time, on the same input, by complete accident. If it’s consistently zero, we get consistent behavior.

We check the value, perhaps even on a backtrace in the same run. If it’s zero, we can see that something is wrong; bytes_read should not be zero on success. (Or if it can be, we might want to initialize it to -1.) We can probably catch the bug here. If bytes_read is a plausible value, though, that just happens to be wrong, would we spot it at a glance?

This is especially true of pointers: a NULL pointer will always be obvious in a debugger, can be tested for very easily, and should segfault on modern hardware if we try to dereference it. A garbage pointer can cause unreproducible memory-corruption bugs later, and these are almost impossible to debug.

Answer 8

The OP is not relying on undefined behavior, or at least not exactly. Indeed, relying on undefined behavior is bad. At the same time, the behavior of a program in an unexpected case is also undefined, but a different kind of undefined. If you set a variable to zero, but you didn't intend to have an execution path that uses that initial zero, will your program behave sanely when you have a bug and do have such a path? You're now in the weeds; you didn't plan to use that value, but you're using it anyway. Maybe it will be harmless, or maybe it will cause the program to crash, or maybe it will cause the program to silently corrupt data. You don't know.

What the OP is saying is that there are tools that will help you to find this bug, if you let them. If you don't initialize the value, but then you use it anyway, there are static and dynamic analyzers that will tell you that you have a bug. A static analyzer will tell you before you even start to test the program. If, on the other hand, you blindly initialize the value, the analyzers can't tell that you didn't plan to use that initial value, and so your bug goes undetected. If you're lucky it's harmless or merely crashes the program; if you're unlucky it silently corrupts data.

The only place I disagree with the OP is at the very end, where he says "when it would already get a segmentation fault otherwise." Indeed, an uninitialized variable will not reliably yield a segmentation fault. Instead, I would say that you should be using static analysis tools that won't let you get to the point of even attempting to execute the program.

Answer 9

An answer to your question needs to be broken down into the different types of variables that appear inside a program:

---

Local variables

Usually the declaration should be right at the spot where the variable first gets its value. Do not predeclare variables like in old style C:

//Bad: predeclared variables
int foo = 0;
double bar = 0.0;
long* baz = NULL;

bar = getBar();
foo = (int)bar;
baz = malloc(foo);


//Correct: declaration and initialization at the same place
double bar = getBar();
int foo = (int)bar;
long* baz = malloc(foo);

This removes 99% of the need for initialization, the variables have their final value right from the off. The few exceptions are where initialization depends on some condition:

Base* ptr;
if(foo()) {
    ptr = new Derived1();
} else {
    ptr = new Derived2();
}

I believe that it's a good idea to write these cases like this:

Base* ptr = nullptr;
if(foo()) {
    ptr = new Derived1();
} else {
    ptr = new Derived2();
}
assert(ptr);

I. e. explicitly assert that some sensible initialization of your variable is performed.

---

Member variables

Here I agree with what the other answerers said: These should always be initialized by the constructors/initializer lists. Otherwise you are hard put to ensure consistency between your members. And if you have a set of members that does not seem to need initialization in all cases, refactor your class, adding those members in a derived class where they are always needed.

---

Buffers

This is where I disagree with the other answers. When people go religious about initializing variables, they frequently end up initializing buffers like this:

char buffer[30];
memset(buffer, 0, sizeof(buffer));

char* buffer2 = calloc(30);

I believe this is almost always harmfull: The only effect of these initializations is that they render tools like valgrind powerless. Any code that reads more from the initialized buffers than it should is very likely a bug. But with the initialization, that bug cannot be exposed by valgrind. So don't use them unless you really rely on the memory being filled with zeros (and in that case, drop a comment saying what you need the zero's for).

I would also strongly recommend adding a target to your build system that runs the entire testsuite under valgrind or a similar tool to expose use-before-initialization bugs and memory leaks. This is more valuable than all preinitializations of variables. That valgrind target should be executed on a regular basis, most importantly before any code goes public.

---

Global Variables

You can't have global variables that are not initialized (at least in C/C++ etc.), so make sure that this initialization is what you want.

Answer 10

A decent C, C++ or Objective-C compiler with the right compiler options set will tell you at compile time if a variable is used before its value is set. Since in these languages using the value of an uninitialised variable is undefined behavior, "set a value before you use" is not a hint, or a guideline, or good practice, it is a 100% requirement; otherwise your program is absolutely broken. In other languages, like Java and Swift, the compiler will never allow you to use a variable before it is initialised.

There is a logical difference between "initialise" and "set a value". If I want to find the conversion rate between dollars and euros, and write "double rate = 0.0;" then the variable has a value set, but it isn't initialised. The 0.0 stored here has nothing whatsoever to do with the correct result. In this situation, if because of a bug you never store the correct conversion rate, the compiler doesn't have a chance to tell you. If you just wrote "double rate;" and never stored a meaningful conversion rate, the compiler would tell you.

So: Don't initialise a variable just because the compiler tells you it is used without being initialised. That is hiding a bug. The real problem is that you are using a variable that you shouldn't be using, or that on one code path you didn't set a value. Fix the problem, don't hide it.

Don't initialise a variable just because the compiler might tell you it is used without being initialised. Again, you are hiding problems.

Declare variables close to use. This improves the chances that you can initialise it with a meaningful value at the point of declaration.

Avoid reusing variables. When you reuse a variable, it is most likely initialised to a useless value when you use it for the second purpose.

It has been commented that some compilers have false negatives, and that checking for initialisation is equivalent to the halting problem. Both are in practice irrelevant. If a compiler, as quoted, cannot find the use of an uninitialized variable ten years after the bug is reported, then it is time to look for an alternative compiler. Java implements this twice; once in the compiler, once in the verifier, without any problems. The easy way to get around the halting problem is not to require that a variable is initialised before used, but that it is initialised before use in a way that can be checked by a simple and fast algorithm.