1 /*
2  * Cppcheck - A tool for static C/C++ code analysis
3  * Copyright (C) 2007-2021 Cppcheck team.
4  *
5  * This program is free software: you can redistribute it and/or modify
6  * it under the terms of the GNU General Public License as published by
7  * the Free Software Foundation, either version 3 of the License, or
8  * (at your option) any later version.
9  *
10  * This program is distributed in the hope that it will be useful,
11  * but WITHOUT ANY WARRANTY; without even the implied warranty of
12  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
13  * GNU General Public License for more details.
14  *
15  * You should have received a copy of the GNU General Public License
16  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
17  */
18 
19 
20 //---------------------------------------------------------------------------
21 #include "checkbool.h"
22 
23 #include "astutils.h"
24 #include "settings.h"
25 #include "symboldatabase.h"
26 #include "token.h"
27 #include "tokenize.h"
28 
29 #include <list>
30 //---------------------------------------------------------------------------
31 
32 // Register this check class (by creating a static instance of it)
33 namespace {
34     CheckBool instance;
35 }
36 
37 static const CWE CWE398(398U);  // Indicator of Poor Code Quality
38 static const CWE CWE571(571U);  // Expression is Always True
39 static const CWE CWE587(587U);  // Assignment of a Fixed Address to a Pointer
40 static const CWE CWE704(704U);  // Incorrect Type Conversion or Cast
41 
isBool(const Variable * var)42 static bool isBool(const Variable* var)
43 {
44     return (var && Token::Match(var->typeEndToken(), "bool|_Bool"));
45 }
46 
47 //---------------------------------------------------------------------------
checkIncrementBoolean()48 void CheckBool::checkIncrementBoolean()
49 {
50     if (!mSettings->severity.isEnabled(Severity::style))
51         return;
52 
53     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
54     for (const Scope * scope : symbolDatabase->functionScopes) {
55         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
56             if (astIsBool(tok) && tok->astParent() && tok->astParent()->str() == "++") {
57                 incrementBooleanError(tok);
58             }
59         }
60     }
61 }
62 
incrementBooleanError(const Token * tok)63 void CheckBool::incrementBooleanError(const Token *tok)
64 {
65     reportError(
66         tok,
67         Severity::style,
68         "incrementboolean",
69         "Incrementing a variable of type 'bool' with postfix operator++ is deprecated by the C++ Standard. You should assign it the value 'true' instead.\n"
70         "The operand of a postfix increment operator may be of type bool but it is deprecated by C++ Standard (Annex D-1) and the operand is always set to true. You should assign it the value 'true' instead.",
71         CWE398, Certainty::normal
72         );
73 }
74 
isConvertedToBool(const Token * tok)75 static bool isConvertedToBool(const Token* tok)
76 {
77     if (!tok->astParent())
78         return false;
79     return astIsBool(tok->astParent()) || Token::Match(tok->astParent()->previous(), "if|while (");
80 }
81 
82 //---------------------------------------------------------------------------
83 // if (bool & bool) -> if (bool && bool)
84 // if (bool | bool) -> if (bool || bool)
85 //---------------------------------------------------------------------------
checkBitwiseOnBoolean()86 void CheckBool::checkBitwiseOnBoolean()
87 {
88     if (!mSettings->severity.isEnabled(Severity::style))
89         return;
90 
91     // danmar: this is inconclusive because I don't like that there are
92     //         warnings for calculations. Example: set_flag(a & b);
93     if (!mSettings->certainty.isEnabled(Certainty::inconclusive))
94         return;
95 
96     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
97     for (const Scope * scope : symbolDatabase->functionScopes) {
98         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
99             if (tok->isBinaryOp() && (tok->str() == "&" || tok->str() == "|")) {
100                 if (!(astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2())))
101                     continue;
102                 if (tok->str() == "|" && !isConvertedToBool(tok) && !(astIsBool(tok->astOperand1()) && astIsBool(tok->astOperand2())))
103                     continue;
104                 if (!isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP()))
105                     continue;
106                 if (!isConstExpression(tok->astOperand2(), mSettings->library, true, mTokenizer->isCPP()))
107                     continue;
108                 if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2())
109                     continue;
110                 const std::string expression = astIsBool(tok->astOperand1()) ? tok->astOperand1()->expressionString()
111                                                                              : tok->astOperand2()->expressionString();
112                 bitwiseOnBooleanError(tok, expression, tok->str() == "&" ? "&&" : "||");
113             }
114         }
115     }
116 }
117 
bitwiseOnBooleanError(const Token * tok,const std::string & expression,const std::string & op)118 void CheckBool::bitwiseOnBooleanError(const Token* tok, const std::string& expression, const std::string& op)
119 {
120     reportError(tok,
121                 Severity::style,
122                 "bitwiseOnBoolean",
123                 "Boolean expression '" + expression + "' is used in bitwise operation. Did you mean '" + op + "'?",
124                 CWE398,
125                 Certainty::inconclusive);
126 }
127 
128 //---------------------------------------------------------------------------
129 //    if (!x==3) <- Probably meant to be "x!=3"
130 //---------------------------------------------------------------------------
131 
checkComparisonOfBoolWithInt()132 void CheckBool::checkComparisonOfBoolWithInt()
133 {
134     if (!mSettings->severity.isEnabled(Severity::warning) || !mTokenizer->isCPP())
135         return;
136 
137     const SymbolDatabase* const symbolDatabase = mTokenizer->getSymbolDatabase();
138     for (const Scope * scope : symbolDatabase->functionScopes) {
139         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
140             if (!tok->isComparisonOp() || !tok->isBinaryOp())
141                 continue;
142             const Token* const left = tok->astOperand1();
143             const Token* const right = tok->astOperand2();
144             if (left->isBoolean() && right->varId()) { // Comparing boolean constant with variable
145                 if (tok->str() != "==" && tok->str() != "!=") {
146                     comparisonOfBoolWithInvalidComparator(right, left->str());
147                 }
148             } else if (left->varId() && right->isBoolean()) { // Comparing variable with boolean constant
149                 if (tok->str() != "==" && tok->str() != "!=") {
150                     comparisonOfBoolWithInvalidComparator(right, left->str());
151                 }
152             }
153         }
154     }
155 }
156 
comparisonOfBoolWithInvalidComparator(const Token * tok,const std::string & expression)157 void CheckBool::comparisonOfBoolWithInvalidComparator(const Token *tok, const std::string &expression)
158 {
159     reportError(tok, Severity::warning, "comparisonOfBoolWithInvalidComparator",
160                 "Comparison of a boolean value using relational operator (<, >, <= or >=).\n"
161                 "The result of the expression '" + expression + "' is of type 'bool'. "
162                 "Comparing 'bool' value using relational (<, >, <= or >=)"
163                 " operator could cause unexpected results.");
164 }
165 
166 //-------------------------------------------------------------------------------
167 // Comparing functions which are returning value of type bool
168 //-------------------------------------------------------------------------------
169 
tokenIsFunctionReturningBool(const Token * tok)170 static bool tokenIsFunctionReturningBool(const Token* tok)
171 {
172     const Function* func = tok->function();
173     if (func && Token::Match(tok, "%name% (")) {
174         if (func->tokenDef && Token::Match(func->tokenDef->previous(), "bool|_Bool")) {
175             return true;
176         }
177     }
178     return false;
179 }
180 
checkComparisonOfFuncReturningBool()181 void CheckBool::checkComparisonOfFuncReturningBool()
182 {
183     if (!mSettings->severity.isEnabled(Severity::style))
184         return;
185 
186     if (!mTokenizer->isCPP())
187         return;
188 
189     const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase();
190 
191     for (const Scope * scope : symbolDatabase->functionScopes) {
192         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
193             if (!tok->isComparisonOp() || tok->str() == "==" || tok->str() == "!=")
194                 continue;
195             const Token *firstToken = tok->previous();
196             if (tok->strAt(-1) == ")") {
197                 firstToken = firstToken->link()->previous();
198             }
199             const Token *secondToken = tok->next();
200             while (secondToken->str() == "!") {
201                 secondToken = secondToken->next();
202             }
203             const bool firstIsFunctionReturningBool = tokenIsFunctionReturningBool(firstToken);
204             const bool secondIsFunctionReturningBool = tokenIsFunctionReturningBool(secondToken);
205             if (firstIsFunctionReturningBool && secondIsFunctionReturningBool) {
206                 comparisonOfTwoFuncsReturningBoolError(firstToken->next(), firstToken->str(), secondToken->str());
207             } else if (firstIsFunctionReturningBool) {
208                 comparisonOfFuncReturningBoolError(firstToken->next(), firstToken->str());
209             } else if (secondIsFunctionReturningBool) {
210                 comparisonOfFuncReturningBoolError(secondToken->previous(), secondToken->str());
211             }
212         }
213     }
214 }
215 
comparisonOfFuncReturningBoolError(const Token * tok,const std::string & expression)216 void CheckBool::comparisonOfFuncReturningBoolError(const Token *tok, const std::string &expression)
217 {
218     reportError(tok, Severity::style, "comparisonOfFuncReturningBoolError",
219                 "Comparison of a function returning boolean value using relational (<, >, <= or >=) operator.\n"
220                 "The return type of function '" + expression + "' is 'bool' "
221                 "and result is of type 'bool'. Comparing 'bool' value using relational (<, >, <= or >=)"
222                 " operator could cause unexpected results.", CWE398, Certainty::normal);
223 }
224 
comparisonOfTwoFuncsReturningBoolError(const Token * tok,const std::string & expression1,const std::string & expression2)225 void CheckBool::comparisonOfTwoFuncsReturningBoolError(const Token *tok, const std::string &expression1, const std::string &expression2)
226 {
227     reportError(tok, Severity::style, "comparisonOfTwoFuncsReturningBoolError",
228                 "Comparison of two functions returning boolean value using relational (<, >, <= or >=) operator.\n"
229                 "The return type of function '" + expression1 + "' and function '" + expression2 + "' is 'bool' "
230                 "and result is of type 'bool'. Comparing 'bool' value using relational (<, >, <= or >=)"
231                 " operator could cause unexpected results.", CWE398, Certainty::normal);
232 }
233 
234 //-------------------------------------------------------------------------------
235 // Comparison of bool with bool
236 //-------------------------------------------------------------------------------
237 
checkComparisonOfBoolWithBool()238 void CheckBool::checkComparisonOfBoolWithBool()
239 {
240     // FIXME: This checking is "experimental" because of the false positives
241     //        when self checking lib/tokenize.cpp (#2617)
242     if (!mSettings->certainty.isEnabled(Certainty::experimental))
243         return;
244 
245     if (!mSettings->severity.isEnabled(Severity::style))
246         return;
247 
248     if (!mTokenizer->isCPP())
249         return;
250 
251     const SymbolDatabase* const symbolDatabase = mTokenizer->getSymbolDatabase();
252 
253     for (const Scope * scope : symbolDatabase->functionScopes) {
254         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
255             if (!tok->isComparisonOp() || tok->str() == "==" || tok->str() == "!=")
256                 continue;
257             bool firstTokenBool = false;
258 
259             const Token *firstToken = tok->previous();
260             if (firstToken->varId()) {
261                 if (isBool(firstToken->variable())) {
262                     firstTokenBool = true;
263                 }
264             }
265             if (!firstTokenBool)
266                 continue;
267 
268             bool secondTokenBool = false;
269             const Token *secondToken = tok->next();
270             if (secondToken->varId()) {
271                 if (isBool(secondToken->variable())) {
272                     secondTokenBool = true;
273                 }
274             }
275             if (secondTokenBool) {
276                 comparisonOfBoolWithBoolError(firstToken->next(), secondToken->str());
277             }
278         }
279     }
280 }
281 
comparisonOfBoolWithBoolError(const Token * tok,const std::string & expression)282 void CheckBool::comparisonOfBoolWithBoolError(const Token *tok, const std::string &expression)
283 {
284     reportError(tok, Severity::style, "comparisonOfBoolWithBoolError",
285                 "Comparison of a variable having boolean value using relational (<, >, <= or >=) operator.\n"
286                 "The variable '" + expression + "' is of type 'bool' "
287                 "and comparing 'bool' value using relational (<, >, <= or >=)"
288                 " operator could cause unexpected results.", CWE398, Certainty::normal);
289 }
290 
291 //-----------------------------------------------------------------------------
checkAssignBoolToPointer()292 void CheckBool::checkAssignBoolToPointer()
293 {
294     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
295     for (const Scope * scope : symbolDatabase->functionScopes) {
296         for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
297             if (tok->str() == "=" && astIsPointer(tok->astOperand1()) && astIsBool(tok->astOperand2())) {
298                 assignBoolToPointerError(tok);
299             }
300         }
301     }
302 }
303 
assignBoolToPointerError(const Token * tok)304 void CheckBool::assignBoolToPointerError(const Token *tok)
305 {
306     reportError(tok, Severity::error, "assignBoolToPointer",
307                 "Boolean value assigned to pointer.", CWE587, Certainty::normal);
308 }
309 
310 //-----------------------------------------------------------------------------
311 //-----------------------------------------------------------------------------
checkComparisonOfBoolExpressionWithInt()312 void CheckBool::checkComparisonOfBoolExpressionWithInt()
313 {
314     if (!mSettings->severity.isEnabled(Severity::warning))
315         return;
316 
317     const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase();
318 
319     for (const Scope * scope : symbolDatabase->functionScopes) {
320         for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
321             if (!tok->isComparisonOp())
322                 continue;
323 
324             const Token* numTok = nullptr;
325             const Token* boolExpr = nullptr;
326             bool numInRhs;
327             if (astIsBool(tok->astOperand1())) {
328                 boolExpr = tok->astOperand1();
329                 numTok = tok->astOperand2();
330                 numInRhs = true;
331             } else if (astIsBool(tok->astOperand2())) {
332                 boolExpr = tok->astOperand2();
333                 numTok = tok->astOperand1();
334                 numInRhs = false;
335             } else {
336                 continue;
337             }
338 
339             if (!numTok || !boolExpr)
340                 continue;
341 
342             if (boolExpr->isOp() && numTok->isName() && Token::Match(tok, "==|!="))
343                 // there is weird code such as:  ((a<b)==c)
344                 // but it is probably written this way by design.
345                 continue;
346 
347             if (astIsBool(numTok))
348                 continue;
349 
350             const ValueFlow::Value *minval = numTok->getValueLE(0, mSettings);
351             if (minval && minval->intvalue == 0 &&
352                 (numInRhs ? Token::Match(tok, ">|==|!=")
353                  : Token::Match(tok, "<|==|!=")))
354                 minval = nullptr;
355 
356             const ValueFlow::Value *maxval = numTok->getValueGE(1, mSettings);
357             if (maxval && maxval->intvalue == 1 &&
358                 (numInRhs ? Token::Match(tok, "<|==|!=")
359                  : Token::Match(tok, ">|==|!=")))
360                 maxval = nullptr;
361 
362             if (minval || maxval) {
363                 bool not0or1 = (minval && minval->intvalue < 0) || (maxval && maxval->intvalue > 1);
364                 comparisonOfBoolExpressionWithIntError(tok, not0or1);
365             }
366         }
367     }
368 }
369 
comparisonOfBoolExpressionWithIntError(const Token * tok,bool not0or1)370 void CheckBool::comparisonOfBoolExpressionWithIntError(const Token *tok, bool not0or1)
371 {
372     if (not0or1)
373         reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
374                     "Comparison of a boolean expression with an integer other than 0 or 1.", CWE398, Certainty::normal);
375     else
376         reportError(tok, Severity::warning, "compareBoolExpressionWithInt",
377                     "Comparison of a boolean expression with an integer.", CWE398, Certainty::normal);
378 }
379 
380 
pointerArithBool()381 void CheckBool::pointerArithBool()
382 {
383     const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase();
384 
385     for (const Scope &scope : symbolDatabase->scopeList) {
386         if (scope.type != Scope::eIf && !scope.isLoopScope())
387             continue;
388         const Token* tok = scope.classDef->next()->astOperand2();
389         if (scope.type == Scope::eFor) {
390             tok = Token::findsimplematch(scope.classDef->tokAt(2), ";");
391             if (tok)
392                 tok = tok->astOperand2();
393             if (tok)
394                 tok = tok->astOperand1();
395         } else if (scope.type == Scope::eDo)
396             tok = (scope.bodyEnd->tokAt(2)) ? scope.bodyEnd->tokAt(2)->astOperand2() : nullptr;
397 
398         pointerArithBoolCond(tok);
399     }
400 }
401 
pointerArithBoolCond(const Token * tok)402 void CheckBool::pointerArithBoolCond(const Token *tok)
403 {
404     if (!tok)
405         return;
406     if (Token::Match(tok, "&&|%oror%")) {
407         pointerArithBoolCond(tok->astOperand1());
408         pointerArithBoolCond(tok->astOperand2());
409         return;
410     }
411     if (tok->str() != "+" && tok->str() != "-")
412         return;
413 
414     if (tok->isBinaryOp() &&
415         tok->astOperand1()->isName() &&
416         tok->astOperand1()->variable() &&
417         tok->astOperand1()->variable()->isPointer() &&
418         tok->astOperand2()->isNumber())
419         pointerArithBoolError(tok);
420 }
421 
pointerArithBoolError(const Token * tok)422 void CheckBool::pointerArithBoolError(const Token *tok)
423 {
424     reportError(tok,
425                 Severity::error,
426                 "pointerArithBool",
427                 "Converting pointer arithmetic result to bool. The bool is always true unless there is undefined behaviour.\n"
428                 "Converting pointer arithmetic result to bool. The boolean result is always true unless there is pointer arithmetic overflow, and overflow is undefined behaviour. Probably a dereference is forgotten.", CWE571, Certainty::normal);
429 }
430 
checkAssignBoolToFloat()431 void CheckBool::checkAssignBoolToFloat()
432 {
433     if (!mTokenizer->isCPP())
434         return;
435     if (!mSettings->severity.isEnabled(Severity::style))
436         return;
437     const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
438     for (const Scope * scope : symbolDatabase->functionScopes) {
439         for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
440             if (tok->str() == "=" && astIsFloat(tok->astOperand1(), false) && astIsBool(tok->astOperand2())) {
441                 assignBoolToFloatError(tok);
442             }
443         }
444     }
445 }
446 
assignBoolToFloatError(const Token * tok)447 void CheckBool::assignBoolToFloatError(const Token *tok)
448 {
449     reportError(tok, Severity::style, "assignBoolToFloat",
450                 "Boolean value assigned to floating point variable.", CWE704, Certainty::normal);
451 }
452 
returnValueOfFunctionReturningBool()453 void CheckBool::returnValueOfFunctionReturningBool()
454 {
455     if (!mSettings->severity.isEnabled(Severity::style))
456         return;
457 
458     const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase();
459 
460     for (const Scope * scope : symbolDatabase->functionScopes) {
461         if (!(scope->function && Token::Match(scope->function->retDef, "bool|_Bool")))
462             continue;
463 
464         for (const Token* tok = scope->bodyStart->next(); tok && (tok != scope->bodyEnd); tok = tok->next()) {
465             // Skip lambdas
466             const Token* tok2 = findLambdaEndToken(tok);
467             if (tok2)
468                 tok = tok2;
469             else if (tok->scope() && tok->scope()->isClassOrStruct())
470                 tok = tok->scope()->bodyEnd;
471             else if (Token::simpleMatch(tok, "return") && tok->astOperand1() &&
472                      (tok->astOperand1()->getValueGE(2, mSettings) || tok->astOperand1()->getValueLE(-1, mSettings)) &&
473                      !(tok->astOperand1()->astOperand1() && Token::Match(tok->astOperand1(), "&|%or%")))
474                 returnValueBoolError(tok);
475         }
476     }
477 }
478 
returnValueBoolError(const Token * tok)479 void CheckBool::returnValueBoolError(const Token *tok)
480 {
481     reportError(tok, Severity::style, "returnNonBoolInBooleanFunction", "Non-boolean value returned from function returning bool");
482 }
483