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