stumbling upon a non-trivial bool scenario in C++

Question

When Cppcheck runs over this code,^[1] it complains about an error:

void bool_express(bool aPre, bool aX, bool aPost)
{
    bool x;
    const bool pre = aPre;
    if (pre) {
        x = aX;
    }
    const bool post = aPost;

    // error checking passage of the original code:
    if ( !pre || !x || !post ) {
        if ( !pre ) {
            trace("pre failed");
        } else if ( !x ) {       // <-- HERE Cppcheck complains
            trace("x failed");
        } else {
            trace("post failed");
        }
    } else {
        // success passage of the original code:
        trace("ok");
    }
}

This is the message, that makes me nervous:

Id: uninitvar
Summary: Uninitialized variable: x
Message: Uninitialized variable: x

I think it's a false positive, but I admit that this may not be obvious. Nevertheless I don't want to touch that code, because it's old and a lot heavier than this extracted version.

Have you ever experienced situations like this? How to deal with it?

---

^[1] This code is reduced to its bones, the original code establishes a precondition (pre), does something (x), then forces a postcondition (post), after that, some error conditions are checked. I transformed the runtime-results of the functions that are called and then stored in pre, x, and post into the 3 arguments of my test case.

Termitarium

We're using Klockworks for static code analysis, and it shows the flow in which x is used uninitialized, if Cppcheck has this option post it and maybe they see something everyone else is missing, although it looks like a false positive to me – Occultation

Did you make sure the reduced version still produces the CppCheck warning? – Danette

How about you just initialize x to false? I cannot imagine a realistic scenario in which that would blow up. – Danette

@Danette Finally you are right ;) As to check if the bug has been fixed in Cppcheck, I have (after preparing the question) a test project, so I will probably better initialize may original "x" and schedule a meta check for Cppcheck 1.76. – Termitarium

@Danette concerning Did you make sure the reduced version still produces the CppCheck warning, yes, I did. – Termitarium

Answer 1

The static analysis appears to be complaining because if pre is false, then x is never set.

Your code is structured such that the value of x is never accessed if pre is false - I'd argue that the static analyser isn't giving a useful output in this case.

Enumerating the various cases we have (so we can be reasonably sure that it's cppcheck and not us!):

1. The first statement in which x is accessed is in the line if ( !pre || !x || !post ) - due to short-circuiting evaluation: if( A || B || C ) doesn't evaluate B or C if A is true; hence we never try to read an uninitialised x (since x is only uninitialised if pre is false, in which case we stopped evaluated the expression!)

2. The second usage is in

   if ( !pre ) { trace("pre failed"); } else if ( !x ) { // <-- HERE Cppcheck complains

   Again, we can only hit the offending line if pre was true (in which case x is properly initialised).

From this, we can conclude that either:

1. The actual code mistakenly tries to read x in some condition even is pre is false, and you've missed it when building the example (sometimes the logical flow of a program can be a bit obtuse)

2. The static analyser is lazy and spots the line else if( !x ) and can't determine if this line is reachable with an uninitialised value.

From the code you've provided, you shouldn't be concerned: the static analysis tool is technically correct that x can be uninitialised, but in those cases it's not used (and hence probably shouldn't be warning you).

I'd recommend assigning x a default value if you're not confident, or if the actual logic is exceedingly obtuse.

Piss

Better use the C/C++ version of or (||) instead of the binary or operator (|). – Termitarium

@Termitarium Whoops! Corrected! – Piss

Please don't refer to short-circuiting here, because it's simply wrong. Short-circuiting is used for boolean expressions and in the case shown, the only expression that is (most probably) short-circuit evaluated is ( !pre || !x || !post ), and it is passed in 7 of 8 cases, especially in all cases where !pre, the point is that x is only evaluated if !!pre, in other words if pre is true. – Termitarium

Please have a look on the second issue as well. Thanks :) – Termitarium

@Termitarium There's two uses of x in your example, one is ( !pre || !x || !post ), the other is in the else if block. In the former, !x isn't evaluated if !pre (and if !pre, then x is undefined) due to short-circuiting. I agree that it isn't the "whole story" (notable else if(!x) which is obviously not evaluated if !pre), but I disagree that it's wrong outright. Unless I missed something? – Piss

@Termitarium Edited to clarify that not every usage of !x is skipped due to short-circuiting. (I'm also aware your answer has pegged this as an issue with cppcheck in unreachable code; it's still an interesting question) – Piss

Let us continue this discussion in chat. – Termitarium

Following an extended chat discussion, I've edited the answer to hopefully be a bit more well-rounded, and not focussed so much on a singular issue (which wasn't all that important to the problem anyway). – Piss

Answer 2

It is a false positive in Cppcheck. I solved it by adding an inline suppression:^[1]

    if ( !pre ) {
        trace("pre failed");
    // cppcheck-suppress uninitvar
    } else if ( !x ) {
        trace("x failed");
    } else {
        trace("post failed");
    }

and I also brought it to the attention of the Cppcheck developers:

#7663 (False positive: uninitialised variable in unreachable code)

---

^[1] I decided not to initialize the variable. This is not for performance reasons, but to be informed about the bug being fixed in a future release, when Cppcheck will say

Id: unmatchedSuppression
Summary: Unmatched suppression: uninitvar
Message: Unmatched suppression: uninitvar

Termitarium

Answer 3

If your pre condition is false than x will be uninitialized. At line if ( !x ) CppCheck warns about usage of indeterminate value. To fix it initialize x variable.

Elouise

If pre is false then !x is not checked due to one of short circuiting or lazy evaluation, can't remember what it is called. – Danette

I think it's not even short-circuiting, but just impossible that x is uninitialized if pre is true. Well, initializing x will not harm, yes. – Termitarium

@Danette @Termitarium You right, !x doesn't checked in !pre || !x || !post in this case. But seems CppCheck doesn't have proper value propagation to understand that if ( !x ) is also not executed. Also it's better to warn about a possible problem here, because it can cause a real problem when outer condition !pre || !x || !post will be changed. – Elouise