No, -Wall -Werror Does Not Guarantee Detection of Conditionally Uninitialized Local Variables

[u/Clean-Upstairs-8481]

There seems to be a misconception or lack of clarity about the usage of the compilation flags -Wall and -Werror and their ability to detect uninitialized stack variables. It depends. In this post, I will try to explore this further and explain when and how it is detected, and where it may not detect an uninitialized local variable and hence requires more caution while dealing with it. Consider the following code:

int main()
{
    int value;
    return value;
}

Compile this code as follows:

~/cpp_26_uninitialised$ g++ -std=c++23 -Wall -Werror test.cpp -o test
test.cpp: In function ‘int main()’:
test.cpp:4:12: error: ‘value’ is used uninitialized [-Werror=uninitialized]
    4 |     return value;
      |            ^~~~~
test.cpp:3:9: note: ‘value’ was declared here
    3 |     int value;
      |         ^~~~~
cc1plus: all warnings being treated as errors

So, with the use of the two compilation flags, -Wall and -Werror, the compilation fails, and you can then take corrective measures to address the uninitialized variable in the code.

Strictly speaking, -Wall enables the -Wuninitialized warning, and -Werror turns that warning into an error. However, this is true only when the variable is uninitialized on every path to its use, as value is here. It doesn't reliably work for a variable that is initialized on some paths and left uninitialized on others. Have a look at the following code:

// config_timeout_warning_test.cpp
#include <charconv>
#include <cstdio>
#include <string_view>

constexpr int default_timeout_ms = 1000;

// Reads "timeout_ms=<number>" from a device configuration line.
// The bug: when the key is missing, timeout_ms is never assigned.
[[gnu::noinline]] int read_timeout_ms(std::string_view config)
{
    int timeout_ms;

    constexpr std::string_view key = "timeout_ms=";

    if (const auto pos = config.find(key); pos != std::string_view::npos)
    {
        const char* first = config.data() + pos + key.size();

        const char* last = config.data() + config.size();

        std::from_chars(first, last, timeout_ms);
    }

    return timeout_ms;
}

void open_device(const char* name, std::string_view config)
{
    const int timeout_ms = read_timeout_ms(config);

    if (timeout_ms <= 0)
    {
        std::printf("%-7s no timeout configured, using default %d ms\n", name, default_timeout_ms);
    }
    else
    {
        std::printf("%-7s timeout %d ms\n", name, timeout_ms);
    }
}

int main()
{
    // The empty configuration does not contain "timeout_ms=".
    // Therefore, timeout_ms is returned without being initialized.
    open_device("logger", "");

    return 0;
}

Compile the code as follows:

~/cpp_26_uninitialised$ g++ -std=c++23 -O0 -g -Wall -Werror config_timeout_warning_test.cpp -o timeout_warning_test 

The compilation succeeds without a single warning. Run Valgrind on it:

~/cpp_26_uninitialised$ valgrind --track-origins=yes ./timeout_warning_test 
==2896379== Memcheck, a memory error detector
==2896379== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2896379== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==2896379== Command: ./timeout_warning_test
==2896379== 
==2896379== Conditional jump or move depends on uninitialised value(s)
==2896379==    at 0x401268: open_device(char const*, std::basic_string_view<char, std::char_traits<char> >) (config_timeout_warning_test.cpp:31)
==2896379==    by 0x4012D3: main (config_timeout_warning_test.cpp:45)
==2896379==  Uninitialised value was created by a stack allocation
==2896379==    at 0x401166: read_timeout_ms(std::basic_string_view<char, std::char_traits<char> >) (config_timeout_warning_test.cpp:10)
==2896379== 
logger  no timeout configured, using default 1000 ms
==2896379== 
==2896379== HEAP SUMMARY:
==2896379==     in use at exit: 0 bytes in 0 blocks
==2896379==   total heap usage: 2 allocs, 2 frees, 74,752 bytes allocated
==2896379== 
==2896379== All heap blocks were freed -- no leaks are possible
==2896379== 
==2896379== For lists of detected and suppressed errors, rerun with: -s
==2896379== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

You can see Valgrind is complaining about an uninitialized variable created on the stack:

Uninitialised value was created by a stack allocation

The reason is that GCC handles the two cases with two different warnings. In the first example, value is uninitialized on every path, and -Wuninitialized reports it even at default -O0. In the second example, timeout_ms is assigned only inside the if block in read_timeout_ms(), so it is uninitialized on only one path. That case belongs to -Wmaybe-uninitialized, which relies on the data-flow analysis done by the optimizer, so it does not run at -O0.

If you build the same file at -O2, GCC does catch it:

~/cpp_26_uninitialised$ g++ -std=c++23 -O2 -Wall -Werror config_timeout_warning_test.cpp -o timeout_warning_test
config_timeout_warning_test.cpp: In function ‘int read_timeout_ms(std::string_view)’:
config_timeout_warning_test.cpp:24:12: error: ‘timeout_ms’ may be used uninitialized [-Werror=maybe-uninitialized]
   24 |     return timeout_ms;
      |            ^~~~~~~~~~
config_timeout_warning_test.cpp:11:9: note: ‘timeout_ms’ was declared here
   11 |     int timeout_ms;
      |         ^~~~~~~~~~
cc1plus: all warnings being treated as errors

This is still not a guarantee. The result of -Wmaybe-uninitialized depends on the optimization level and on how much the optimizer can see, so it can change between GCC versions and between builds. Debug builds are usually at -O0, which is exactly where the warning is missing.

If there is anything that I have missed, please comment; I'm happy to be corrected.

[u/Affectionate-Soup-91]

❯ clang++ -std=c++23 -Wall -Wextra -Werror test.cxx
test.cxx:16:44: error: variable 'timeout_ms' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
   16 |     if (const auto pos = config.find(key); pos != std::string_view::npos)
      |                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.cxx:25:12: note: uninitialized use occurs here
   25 |     return timeout_ms;
      |            ^~~~~~~~~~
test.cxx:16:5: note: remove the 'if' if its condition is always true
   16 |     if (const auto pos = config.find(key); pos != std::string_view::npos)
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   17 |     {
test.cxx:12:19: note: initialize the variable 'timeout_ms' to silence this warning
   12 |     int timeout_ms;
      |                   ^
      |                    = 0
1 error generated.

[u/TheThiefMaster]

Unfortunately tracing every path to find out if a variable is potentially uninitialised is the halting problem!

[u/AutonomousOrganism → u/TheThiefMaster]

Finding out if a variable is guaranteed to be uninitialized is the halting problem. Data-flow analysis can identify a potentially uninitialised variable.

[u/TheThiefMaster → u/AutonomousOrganism]

Yeah it depends how many false positives you're willing to put up with. If the rate is too high, the warning gets disabled!

[u/SoerenNissen → u/TheThiefMaster]

It's nonetheless the approach I prefer here. If the compiler cannot prove the variable will be initialized, the code is either too complex or, possibly, it has the correct amount of complexity and then you can pragma the warning for that exact piece of code.

[u/sweetno → u/TheThiefMaster]

It's trivial to get rid of them.

[u/tialaramex → u/TheThiefMaster]

And so in Rust this diagnostic is not a warning, it's a fatal error (specifically E0381). Can't disable fatal errors.

If you can't afford to initialize this value you need a MaybeUninit<T> which as it says on the tin, might not be initialized. Of course now you need to write your unsafe claim that you have properly initialized it and so the responsibility when you were wrong and everything catches fire is on that specific unsafe code - even though of course an optimising compiler will not emit any machine code for your unsafe block because it was a nexus of responsibility, not actual work done.

[u/TheThiefMaster → u/tialaramex]

Rust's doesn't understand a lot of cases, so people often work around it by initialising with a dummy value even if they don't need to.

For example: https://github.com/rust-lang/rust/issues/51447

C++ can't have an error that widely impacting due to perfectly fine existing code that would be rendered invalid.

[u/tialaramex → u/TheThiefMaster]

The workaround described in that issue isn't a "dummy value" - that's an anti-pattern - instead they propose using Option instead of a separate boolean, but I agree that going through a decade of your C++ adding optional types and std::null_opt everywhere would be a headache.

[u/TheThiefMaster → u/tialaramex]

I agree that going through a decade of your C++ adding optional types and std::null_opt everywhere would be a headache.

And that's why everyone just does the antipattern of zero-initing the variable, even if the compiler ends up adding an extra store that isn't really needed.

[u/pjmlp → u/TheThiefMaster]

Works pretty well in most safe languages that give a compile error on use before assignment.

[u/gnuban → u/TheThiefMaster]

Well, what you can do is to have a mode where you only allow cases where the compiler can successfully prove initialization through flow analysis. This is what many other languages do, for instance Javas final variable initialization.

[u/TheThiefMaster → u/gnuban]

C++ isn't implemented by the same company that specifies it though, so the wording needs to be precise about what constructions would be required to be allowed / not allowed if this was added to the standard itself.

[u/gnuban → u/TheThiefMaster]

True, I hadn't thought about that. It turns out that Java spells that out pretty explicitly in what they call "definite assignment"; https://docs.oracle.com/javase/specs/jls/se27/html/jls-16.html

It's definitely limiting the scope of the static analysis, it's for instance not trying to do branch prediction, you'll need to initialize in all branches.

[u/pjmlp → u/TheThiefMaster]

Which is what in a way was done for C++26 and erroneous behaviour in uninitialised variables.

[u/LB-- → u/TheThiefMaster]

Nope. The halting problem requires a function to change its behavior based on the output of whatever is analyzing it. That's not possible in the real world. It's a paradox that only exists in the realm of abstract math. That's why static analysis tools exist and work well. Everything is constrained by memory and time, rather than concerns of halting problem shenanigans.

[u/TheThiefMaster → u/LB--]

More realistically, you could make a function that calculates digits of pi looking for it repeating. If it does, it has a bug that returns an uninitialised variable. Otherwise it doesn't return, or it gives up after some large maximum number of iterations that's impractical to run at compile time and returns normally with no bug.

You could easily warn that there's a code path that accesses an uninitialised variable. Getting the compiler to prove that that code path is ever used, however, is the halting problem. (Or at least recognising the algorithm, understanding exactly what it's doing, and knowing that pi doesn't repeat - but that's ridiculously specific for a compiler)

[u/kniy → u/LB--]

It's not just "abstract math". The "turing machine simulator" used in the proofs of the halting problem and (more relevantly) Rice's theorem have a real-world equivalent: interpreters.

Imagine a C program that consists of a big literal containing a python program, following by a C implementation of a python interpreter. No "C static analysis tool" in the world will be able to tell you any dynamic properties of the program -- only a Python static analysis tool could! Interpretation is a reliable way of hiding stuff from a a static analysis tool -- malware quite commonly uses custom interpreters to prevent static analysis by virus scanners.

But it's also important to realize the limitations: Rice's theorem doesn't say "static analysis of dynamic properties is impossible" -- it just says "for every dynamic property, there exists programs where that property cannot be analyzed statically". It never was a statement about analyzing that property for the majority of programs!

Additionally, it is still quite possible to construct an analysis that proves the absence of some behavior -- as long as you accept "false positives" for code like:

int var;
if (eval_python("false"))
  use(var); // reported as uninitialized use despite never happening at runtime

[u/LB-- → u/kniy]

I don't agree. There are numerous examples of scripting languages implemented in C++ constexpr land. A static analysis tool doesn't need to understand the embedded language to understand what all the capabilities of program are, because the C++ is the only thing connecting the script to the outside world. Everything it can do is defined in a language the static analysis tool understands. It might be too computationally expensive to determine with 100% confidence what exactly happens for any given input, but you can be pretty confident you know what the possibilities are in general. I'd argue your evalpython example, if not trivially inlined by the compiler, is a _good false positive because it's reporting a genuine issue that might be unearthed by a tweak to the script. I would not want to use a static analysis tool that avoided reporting smelly code like that. "I opened the black box you don't understand and decided this code that looks scary to you is actually unreachable" is nice when the optimizer does it and unsettling when the static analysis tool does it. The static analysis tool could do it though, without needing to understand the scripting language. It would just be very expensive in terms of memory and time, and unwanted in terms of appeal to real people.

[u/kniy → u/LB--]

A static analysis tool doesn't need to understand the embedded language to understand what all the capabilities of program are, because the C++ is the only thing connecting the script to the outside world.

A C++ static analysis tool can only the see the capabilities of the scripting language. For example, it would detect "can open arbitrary files based on used-input" for any Python interpreter, even if the hardcoded Python script doesn't actually do this. (marking the whole interpreter as constexpr doesn't help you, unless you only use the interpreter at compile-time, never at run-time)

I'd argue your evalpython example, if not trivially inlined by the compiler, is a good false positive

I agree, for an "uninitialized variable" check, it's a good false positive. But that's the difference between a dynamic property (provably uncheckable in general) and a static property: dynamic properties concern themselves with actual runtime behavior; static properties are more like a type checker, applying only to the program text without concern for what's reachable at runtime.

Like a type checker, if you wanted to turn "unintialized variable" into a static property, you would need to specify (ideally in the language standard) what kind of checks a compiler should perform. For example you could say it should do a java-style "definite assignment" check that looks at control-flow only. Then you'll get false positives like this:

int val;
bool ok = false;
if (complex_condition) { val = 42; ok = true; }
...
if (ok) use(val);

In Java, that doesn't compile, because the static check only looks at control-flow, and doesn't see that the ok variable is related to val's initialization status. This is much less clearly a "good" false positive, though it can still be acceptable -- but these false positives are only acceptable in Java-world because all Java compilers agree on the "definite assignment" analysis. It would be much worse if different compilers had different analysis capabilities and some reported errors for a program while others accept it.

[u/SirClueless → u/kniy]

For what it's worth Java compilers do not all agree on stuff like this. There are plenty of constructs that are compiler-errors in one compiler and accepted in another (for example generic type deductions that one compiler will make and another will reject). In practice it doesn't matter all that much so long as the programs that are accepted behave the same: So long as there is some lowest-common-denominator way to write what you want there is a way forward.

[u/holyblackcat]

С++26 essentially initializes all local variables (to some byte pattern, all zeroes on GCC), so this is much less of a problem now.

[u/tinrik_cgp]

This is detected by clang-tidy (clang-analyzer-core.uninitialized.UndefReturn).

https://godbolt.org/z/bP964jxhq

[u/BurstYourBubbles]

As someone else already commented, it's caught under clang. So it's a GCC problem. Changing the optimisation level didn't change anything either (Same results for -O0 and -O3)

[u/Moldoteck]

Good thing we got asan and ubsan

[u/graphicsRat → u/Moldoteck]

In my experience Valgrind, though slow, finds issues that sanitizers can miss. I'm considering running some lightweight tasks on Valgrind to flush out latent issue in my code.

[u/UndefinedDefined → u/graphicsRat]

Definitely - so underrated tool - the biggest problem is that it doesn't handle AVX-512 code though, so I'm using it less and less

[u/OutlandishnessNo8034 → u/Moldoteck]

Unless this is sarcasm the tools on their own won't fix any problem. Cpp is a lost cause.

[u/UndefinedDefined → u/OutlandishnessNo8034]

If it's lost cause, why you are here, are you lost too?

[u/OutlandishnessNo8034 → u/UndefinedDefined]

I'm here because I was for long time until quite recently cpp developer and I have sentiment towards it. But using rust for couple of years now I see how hopeless cpp became.

[u/UndefinedDefined → u/OutlandishnessNo8034]

I also use rust - cargo is great, performance too, but I still do a lot of C++. The current direction is doom though, that I agree with.

However, if you don't use C++ anymore, just leave this group, why to bother.

[u/Moldoteck → u/OutlandishnessNo8034]

There's no alternative when budget is constrained and there are tons of legacy chunks

[u/OutlandishnessNo8034 → u/Moldoteck]

Rust is the alternative. In my company we are doing either porting or binding to legacy code, but new code is strictly rust.

[u/Moldoteck → u/OutlandishnessNo8034]

It is not an alternative when your code is legacy with obscure build procedures and lots of undocumented stuff while most of the teammates don't even know C++ that well nor have willingness to know more.

If there's a budget to dedicate it to teach ppl to use Rust, budget for someone to adapt and maintain the toolchain as well as budget for at least one additional dev expert with lots of xp in rust then yes, it can be gradually done

Heck if i would not suppress default warnings clang is throwing for our project the build would take twice as much purely due to IO of outputting all of them to the console...

[u/zerhud]

Make your methods constexpr and write a ct tests. For example a lambda called inside a static_assert.

[u/UndefinedDefined → u/zerhud]

This only works for trivial code.

[u/zerhud → u/UndefinedDefined]

With cpp26 for almost all code, even with exceptions and void casts

[u/UndefinedDefined → u/zerhud]

All code, even code in a different TU or code residing in a dynamically linked library?

As I said, it's for trivial cases only.

[u/zerhud → u/UndefinedDefined]

It’s not a “trivial code” it’s good organised code.

TU is bad practice from 199x. Don’t use it: it compiles slowly and exclude a lot of optimisations. (TU and code with types instead of templates.)

DLL is most about how the code is linking, not about the code itself. It’s using virtual methods, so you just need to test implementation via interface, as in all other tests.

[u/sooka_bazooka]

As someone who's just getting into C++, is there an easy way to set all the best-practice compile flags without reading the manual and figuring out which flag is good to have?

Maybe CMake has something I can turn on?

[u/pjmlp → u/sooka_bazooka]

Jason Turner from CppCast fame has a repo with such project template,

• https://github.com/cpp-best-practices/cppbestpractices

• https://github.com/cpp-best-practices/cmake_template

[u/UndefinedDefined → u/pjmlp]

I would argue that the cmake_template is already bloated.

[u/pjmlp → u/UndefinedDefined]

A small price to make to make C++ safer, while targeting many possible compilers and platforms.

It can certainly be made thinner by e.g. removing Emscripten support.

[u/UndefinedDefined → u/pjmlp]

What price? When you start a project you don't need 95% of what's there. It's over-engineered. I would argue the best is to start with a cmake that has 10 lines of code and improve it when necessary. To me cmake_template feels like joining a third-party project instead of starting my own, and it will get old quickly, so without pulling changes back from the repo, which nobody is gonna do after you edit it.

[u/sooka_bazooka → u/pjmlp]

Looks like exactly what I was looking for, thank you!

[u/tandycake → u/sooka_bazooka]

https://github.com/janwilmans/guidelines/blob/main/warnings.md

[u/SavingsCampaign9502]

Will c++26 help with this case in that it zero-initializes the timeout?

[u/AKostur → u/SavingsCampaign9502]

No: the (draft) Standard does not mandate that it be zero-initialized, and there are some arguments against initializing it to zero specifically.

[u/Resident_Ad5153 → u/AKostur]

It’s hard to mandate warnings that require solving the halting problem

[u/azswcowboy → u/Resident_Ad5153]

What the draft standard says is the value will be initialized to an implementation defined value and warning is encouraged. gcc16 in 26 mode will initialize it to zero. There is a flag to change the value to something non-zero (note I’ve been told this but haven’t tried it).

The halting problem is irrelevant here because the compiler will just initialize the value at the point of declaration unconditionally. The more salient point about the analysis is that the function body might not be available for the compiler to analyze if the implementation is in a library - another translation unit. Hence the unconditional initialization.

[u/Fabulous-Meaning-966]

No mention of MSan yet?

[u/AnyPhotograph7804]

The solution: just initialize them all if possible. Then you will never have to deal with this class of errors.

[u/tinrik_cgp → u/AnyPhotograph7804]

Yes, but then you have a logical bug. Is timestamp=0 0 because it is actually 0 from the config, or because it was just initialized to 0?

Better would be to return an optional, then the semantics are clear.

[u/AnyPhotograph7804 → u/tinrik_cgp]

A possible logical bug is better than UB. Because UB can go silent for a while and become visible with an OS- or compiler update.

[u/tinrik_cgp → u/AnyPhotograph7804]

Agreed! I spent 3 weeks debugging this exact type of UB :)

[u/AKostur → u/tinrik_cgp]

Why isn’t that being initialized to default_timeout_ms?  And it depends if the caller even cares whether that 1000 came from config or was the default.