The logical AND and logical OR operators (&&
and ||
, respectively) exhibit "short-circuit" operation. That is, the second operand is not evaluated if the result can be deduced solely by evaluating the first operand.
Programmers should exercise caution if the second operand contains side effects because it may not be apparent whether the side effects actually occur.
In the following code, the value of i
is incremented only when i >= 0
:
enum { max = 15 }; int i = /* Initialize to user-supplied value */; if ( (i >= 0) && ( (i++) <= max) ) { /* Code */ }
Although the behavior is well defined, it is not immediately obvious whether or not i
gets incremented.
Noncompliant Code Example
In this noncompliant code example, the second operand of the logical OR operator invokes a function that results in side effects:
char *p = /* Initialize; may or may not be NULL */ if (p || (p = (char *) malloc(BUF_SIZE)) ) { /* Perform some computation based on p */ free(p); p = NULL; } else { /* Handle malloc() error */ return; }
Because malloc()
is called only if p
is NULL
when entering the if
clause, free()
might be called with a pointer to local data not allocated by malloc()
. (See MEM34-C. Only free memory allocated dynamically.) This behavior is partially due to the uncertainty of whether or not malloc()
is actually called.
Compliant Solution
In this compliant solution, a second pointer, q
, is used to indicate whether malloc()
is called; if not, q
remains set to NULL
. Passing NULL
to free()
is guaranteed to safely do nothing.
char *p = /* Initialize; may or may not be NULL */ char *q = NULL; if (p == NULL) { q = (char *) malloc(BUF_SIZE); p = q; } if (p == NULL) { /* Handle malloc() error */ return; } /* Perform some computation based on p */ free(q); q = NULL;
Risk Assessment
Failing to understand the short-circuit behavior of the logical OR or AND operator may cause unintended program behavior.
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
EXP02-C | Low | Unlikely | Medium | P2 | L3 |
Automated Detection
Tool | Version | Checker | Description |
---|---|---|---|
Astrée | 24.04 | logop-side-effect | Fully checked |
Axivion Bauhaus Suite | 7.2.0 | CertC-EXP02 | Fully implemented |
Compass/ROSE | Could detect possible violations of this recommendation by reporting expressions with side effects, including function calls, that appear on the right-hand side of an | ||
Helix QAC | 2024.3 | C3415 | |
Klocwork | 2024.3 | MISRA.LOGIC.SIDEEFF | |
LDRA tool suite | 9.7.1 | 35 D | Fully implemented |
Parasoft C/C++test | 2023.1 | CERT_C-EXP02-a | The right-hand operand of a logical && or || operator shall not contain side effects |
PC-lint Plus | 1.4 | 9007 | Fully supported |
RuleChecker | 24.04 | logop-side-effect | Fully checked |
SonarQube C/C++ Plugin | 3.11 | SideEffectInRightHandSideOfLogical |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Related Guidelines
SEI CERT C++ Coding Standard | VOID EXP02-CPP. Be aware of the short-circuit behavior of the logical AND and OR operators |
MITRE CWE | CWE-768, Incorrect short circuit evaluation |
11 Comments
Stephen Friedl
I'm not sure I buy this universally, because though it can certainly be confusing, the rules are rock-solid clear on what's done when.
I am pretty sure I've done plenty of things like this that are readable and reliable.
Jonathan Leffler
Seconded. I have also seen code like:
Again, perfectly safe because of the sequence points, and if the same error will be generated regardless of which of the functions failed. (We can debate the error reporting in this function - perhaps the called functions reported the error themselves.)
Robert Seacord
Both these comments suggest that it should be OK to have side effects in the second operands of the logical AND and OR operators, if the side effects are caused by a function invocation.
Still, I think there are clearer ways to write each example that doesn't require violating this recommendation. Also, this is only a recommendation (that is, a suggestion as to how code can be written more clearly).
We've also been contemplating a recommendation not to allow assignment in a conditional, as this construction has clearly been a cause of program error, and disallowing it would allow static analysis tools to check for the problems caused by using '=' instead of '=='.
Robert Seacord
Would this be clearer as:
If so, then maybe we shouldn't make an exception to this recommendation for this reason.
Douglas A. Gwyn
I disagree with this as a general rule. It is okay to caution about this, but I've written and seen a lot of code where the best way to express something involves a side effect in the second operand, and not always as a function call. E.g.
while (i < MAX && ++i < j) ...
David Svoboda
I'm afraid I must concur with the naysayers on this rule. Unlike our other rules that also 'verge on coding style', this rule is condemning properties of C behavior that are well-defined, well-behaved, and well-understood. So this rule exists not because of ambiguity in compiler implementations or the C standards but because of psychology...we are scared that programmers don't understand the short-circuiting of && and ||.
I agree that the first non-compliant code example is bad, but then I don't like ++ inside if-clauses in general, due to the vaguries of sequence points. Why was the NCCE written that way in the first place? It seems contrived and artificial.
The 2nd NCCE is bad, but mainly because you can't separate the cases of p already being defined, and malloc failing...both dump you into the else clause. (Also p isn't initialized, but that's easily fixed.)
There are several reasonable-looking counterexamples in the comments. They are all fairly abstract; we would need to flesh one out to cite it in the rule.
I think the basic problem with this rule is, what constitutes a 'side effect'? I know 'side effect' has a precise definition in C, but that definition diverges from the English definition of something changing outside our focus. Saying
i++
in the if clause in the first NCCE may be a side effect in the C sense, but not in the English sense, since the if clause is all about the value of i, so a programmer will be focused on i and be aware of its change.In summary, this rule belongs in a style guideline, not a secure coding standard. I could be convinced that this rule is valid by a sufficient 'in the wild' NCCE, esp. if it led to a vul.
David Svoboda
This rec is now weaker, it no longer prohibits side effects in && or ||, just warns the user about their short-circuit behavior.
George Romanski
Code structures should not be unrolled into nested conditions. For verification (e.g. at the highest levels of criticality, DO-178B Level A) we need to show that every condition has an effect on its outcome. Unrolling the conditions makes it more difficult to see the effect each individual condition has on the outcome, and some combinations are difficult to unroll. (try recoding: if (a || b) && c { ..., and another construct if (a && b) || c {...
Substituting arithmetic operators for logical ones compounds the problem because the order of evaluation is not fixed and with side effects the results depend on compiler choices. These choices are effected by code instrumentation typically used for measurement of code coverage while testing. What you test may not be what finally runs.
By the way, don't assume that the compiler will evaluate your conditions in short circuit order just because you have used short circuit forms. Often compilers will change logical operators to arithmetic operators because it reduces the number of branches in the code and this reduces the number of times the pipeline is stalled. Thankfully these optimizations are turned off, if possible side effects are detected.
Robert Seacord
Are you recommending a change to this rule, or the creation of one or more additional rules? If so, could you flesh these out a little?
I'm not concerned about the optimizations of the short circuit order... sounds like a reasonable application of the "as if" rule provided their are no side effects.
David Svoboda
According to C99 6.5.13:
and 6.5.14 has a similar paragraph regarding ||.
Reordering of ||/&& expressions is only permitted as long as they don't change the outcome; the 'as if' rule Rob cites.
Jerome Dern
This is covered by MISRA rule 13.5 (2012 edition), information shall be updated