Skip to content

Commit bc3f555

Browse files
pfultz2pfultz2
authored
Fix issue 8871: improve check: mismatching container size conditions (#2988)
1 parent cdc0ba3 commit bc3f555

7 files changed

Lines changed: 161 additions & 149 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1385,10 +1385,15 @@ void CheckCondition::alwaysTrueFalse()
13851385
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
13861386
for (const Scope * scope : symbolDatabase->functionScopes) {
13871387
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
1388-
if (tok->link()) // don't write false positives when templates are used
1388+
if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used
13891389
continue;
13901390
if (!tok->hasKnownIntValue())
13911391
continue;
1392+
if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) {
1393+
const Function* f = tok->previous()->function();
1394+
if (f->functionScope && Token::Match(f->functionScope->bodyStart, "{ return true|false ;"))
1395+
continue;
1396+
}
13921397
{
13931398
// is this a condition..
13941399
const Token *parent = tok->astParent();

‎lib/checkuninitvar.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,14 +266,6 @@ static void conditionAlwaysTrueOrFalse(const Token *tok, const std::map<int, Var
266266
}
267267

268268
else if (tok->isComparisonOp()) {
269-
if (tok->hasKnownIntValue()) {
270-
if (tok->values().front().intvalue)
271-
*alwaysTrue = true;
272-
else
273-
*alwaysFalse = true;
274-
return;
275-
}
276-
277269
if (variableValue.empty()) {
278270
return;
279271
}

‎lib/clangimport.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ std::string clangimport::AstNode::getSpelling() const
392392
if (typeIndex <= 0)
393393
return "";
394394
}
395-
const std::string &str = mExtTokens[typeIndex - 1];
395+
const std::string &str = mExtTokens[std::max(typeIndex - 1, 0)];
396396
if (str.compare(0,4,"col:") == 0)
397397
return "";
398398
if (str.compare(0,8,"<invalid") == 0)

‎lib/mathlib.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ MathLib::biguint MathLib::toULongNumber(const std::string & str)
346346
return ret;
347347
}
348348

349-
static unsigned int encodeMultiChar(const std::string& str)
349+
unsigned int MathLib::encodeMultiChar(const std::string& str)
350350
{
351351
unsigned int retval = 0;
352352
for (char it : str) {

‎lib/mathlib.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ class CPPCHECKLIB MathLib {
125125
*/
126126
static bool isOctalDigit(char c);
127127

128+
static unsigned int encodeMultiChar(const std::string& str);
129+
128130
/**
129131
* \param[in] str character literal
130132
* @return Number of internal representation of the character literal

‎lib/valueflow.cpp‎

Lines changed: 104 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
#include "analyzer.h"
8181
#include "astutils.h"
8282
#include "errorlogger.h"
83+
#include "errortypes.h"
8384
#include "forwardanalyzer.h"
8485
#include "library.h"
8586
#include "mathlib.h"
@@ -106,6 +107,7 @@
106107
#include <map>
107108
#include <set>
108109
#include <stack>
110+
#include <stdexcept>
109111
#include <string>
110112
#include <tuple>
111113
#include <vector>
@@ -341,6 +343,60 @@ static bool isComputableValue(const Token* parent, const ValueFlow::Value& value
341343
return true;
342344
}
343345

346+
template <class T>
347+
T asInt(T x)
348+
{
349+
return x;
350+
}
351+
352+
MathLib::bigint asInt(float x) { return x; }
353+
354+
MathLib::bigint asInt(double x) { return x; }
355+
356+
template <class T, class U>
357+
static T calculate(const std::string& s, T x, U y)
358+
{
359+
switch (MathLib::encodeMultiChar(s)) {
360+
case '+':
361+
return x + y;
362+
case '-':
363+
return x - y;
364+
case '*':
365+
return x * y;
366+
case '/':
367+
return x / y;
368+
case '%':
369+
return asInt(x) % asInt(y);
370+
case '&':
371+
return asInt(x) & asInt(y);
372+
case '|':
373+
return asInt(x) | asInt(y);
374+
case '^':
375+
return asInt(x) ^ asInt(y);
376+
case '>':
377+
return x > y;
378+
case '<':
379+
return x < y;
380+
case '<<':
381+
return asInt(x) << asInt(y);
382+
case '>>':
383+
return asInt(x) >> asInt(y);
384+
case '&&':
385+
return x && y;
386+
case '||':
387+
return x || y;
388+
case '==':
389+
return x == y;
390+
case '!=':
391+
return x != y;
392+
case '>=':
393+
return x >= y;
394+
case '<=':
395+
return x <= y;
396+
}
397+
throw InternalError(nullptr, "Unknown operator: " + s);
398+
}
399+
344400
/** Set token value for cast */
345401
static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings);
346402

@@ -359,31 +415,40 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
359415

360416
if (value.isContainerSizeValue()) {
361417
// .empty, .size, +"abc", +'a'
362-
if (parent->str() == "+" && parent->astOperand1() && parent->astOperand2()) {
418+
if (Token::Match(parent, "+|==|!=") && parent->astOperand1() && parent->astOperand2()) {
363419
for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) {
364420
for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) {
365421
if (value1.path != value2.path)
366422
continue;
367423
ValueFlow::Value result;
368-
result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
424+
if (Token::Match(parent, "%comp%"))
425+
result.valueType = ValueFlow::Value::ValueType::INT;
426+
else
427+
result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
428+
369429
if (value1.isContainerSizeValue() && value2.isContainerSizeValue())
370-
result.intvalue = value1.intvalue + value2.intvalue;
430+
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
371431
else if (value1.isContainerSizeValue() && value2.isTokValue() && value2.tokvalue->tokType() == Token::eString)
372-
result.intvalue = value1.intvalue + Token::getStrLength(value2.tokvalue);
432+
result.intvalue = calculate(parent->str(), value1.intvalue, Token::getStrLength(value2.tokvalue));
373433
else if (value2.isContainerSizeValue() && value1.isTokValue() && value1.tokvalue->tokType() == Token::eString)
374-
result.intvalue = Token::getStrLength(value1.tokvalue) + value2.intvalue;
434+
result.intvalue = calculate(parent->str(), Token::getStrLength(value1.tokvalue), value2.intvalue);
375435
else
376436
continue;
377437

378438
combineValueProperties(value1, value2, &result);
379439

440+
if (Token::simpleMatch(parent, "==") && result.intvalue)
441+
continue;
442+
if (Token::simpleMatch(parent, "!=") && !result.intvalue)
443+
continue;
444+
380445
setTokenValue(parent, result, settings);
381446
}
382447
}
383448
}
384449

385-
386-
else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && parent->astOperand1() && parent->astOperand1()->valueType()) {
450+
else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) &&
451+
parent->astOperand1() && parent->astOperand1()->valueType()) {
387452
const Library::Container *c = parent->astOperand1()->valueType()->container;
388453
const Library::Container::Yield yields = c ? c->getYield(parent->strAt(1)) : Library::Container::Yield::NO_YIELD;
389454
if (yields == Library::Container::Yield::SIZE) {
@@ -545,147 +610,48 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
545610
combineValueProperties(value1, value2, &result);
546611
const double floatValue1 = value1.isIntValue() ? value1.intvalue : value1.floatValue;
547612
const double floatValue2 = value2.isIntValue() ? value2.intvalue : value2.floatValue;
548-
switch (parent->str()[0]) {
549-
case '+':
550-
if (value1.isTokValue() || value2.isTokValue())
551-
break;
552-
if (value1.isFloatValue() || value2.isFloatValue()) {
553-
result.valueType = ValueFlow::Value::ValueType::FLOAT;
554-
result.floatValue = floatValue1 + floatValue2;
613+
const bool isFloat = value1.isFloatValue() || value2.isFloatValue();
614+
if (isFloat && Token::Match(parent, "&|^|%|<<|>>|%or%"))
615+
continue;
616+
if (Token::Match(parent, "<<|>>") &&
617+
!(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits))
618+
continue;
619+
if (Token::Match(parent, "/|%") && floatValue2 == 0)
620+
continue;
621+
if (Token::Match(parent, "==|!=")) {
622+
if ((value1.isIntValue() && value2.isTokValue()) || (value1.isTokValue() && value2.isIntValue())) {
623+
if (parent->str() == "==")
624+
result.intvalue = 0;
625+
else if (parent->str() == "!=")
626+
result.intvalue = 1;
627+
} else if (value1.isIntValue() && value2.isIntValue()) {
628+
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
555629
} else {
556-
result.intvalue = value1.intvalue + value2.intvalue;
630+
continue;
557631
}
558632
setTokenValue(parent, result, settings);
559-
break;
560-
case '-':
561-
if (value1.isTokValue() || value2.isTokValue())
562-
break;
563-
if (value1.isFloatValue() || value2.isFloatValue()) {
564-
result.valueType = ValueFlow::Value::ValueType::FLOAT;
565-
result.floatValue = floatValue1 - floatValue2;
566-
} else {
567-
// Avoid overflow
568-
if (value1.intvalue < 0 && value2.intvalue > value1.intvalue - LLONG_MIN)
569-
break;
570-
571-
result.intvalue = value1.intvalue - value2.intvalue;
572-
}
573-
// If the bound comes from the second value then invert the bound
574-
if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point)
575-
result.invertBound();
633+
} else if (Token::Match(parent, "%comp%")) {
634+
if (!isFloat && !value1.isIntValue() && !value2.isIntValue())
635+
continue;
636+
if (isFloat)
637+
result.intvalue = calculate(parent->str(), floatValue1, floatValue2);
638+
else
639+
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
576640
setTokenValue(parent, result, settings);
577-
break;
578-
case '*':
641+
} else if (Token::Match(parent, "%op%")) {
579642
if (value1.isTokValue() || value2.isTokValue())
580643
break;
581-
if (value1.isFloatValue() || value2.isFloatValue()) {
582-
result.valueType = ValueFlow::Value::ValueType::FLOAT;
583-
result.floatValue = floatValue1 * floatValue2;
584-
} else {
585-
result.intvalue = value1.intvalue * value2.intvalue;
586-
}
587-
setTokenValue(parent, result, settings);
588-
break;
589-
case '/':
590-
if (value1.isTokValue() || value2.isTokValue() || value2.intvalue == 0)
591-
break;
592-
if (value1.isFloatValue() || value2.isFloatValue()) {
644+
if (isFloat) {
593645
result.valueType = ValueFlow::Value::ValueType::FLOAT;
594-
result.floatValue = floatValue1 / floatValue2;
646+
result.floatValue = calculate(parent->str(), floatValue1, floatValue2);
595647
} else {
596-
result.intvalue = value1.intvalue / value2.intvalue;
648+
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
597649
}
650+
// If the bound comes from the second value then invert the bound when subtracting
651+
if (Token::simpleMatch(parent, "-") && value2.bound == result.bound &&
652+
value2.bound != ValueFlow::Value::Bound::Point)
653+
result.invertBound();
598654
setTokenValue(parent, result, settings);
599-
break;
600-
case '%':
601-
if (!value1.isIntValue() || !value2.isIntValue())
602-
break;
603-
if (value2.intvalue == 0)
604-
break;
605-
result.intvalue = value1.intvalue % value2.intvalue;
606-
setTokenValue(parent, result, settings);
607-
break;
608-
case '=':
609-
if (parent->str() == "==") {
610-
if ((value1.isIntValue() && value2.isTokValue()) ||
611-
(value1.isTokValue() && value2.isIntValue())) {
612-
result.intvalue = 0;
613-
setTokenValue(parent, result, settings);
614-
} else if (value1.isIntValue() && value2.isIntValue()) {
615-
result.intvalue = value1.intvalue == value2.intvalue;
616-
setTokenValue(parent, result, settings);
617-
}
618-
}
619-
break;
620-
case '!':
621-
if (parent->str() == "!=") {
622-
if ((value1.isIntValue() && value2.isTokValue()) ||
623-
(value1.isTokValue() && value2.isIntValue())) {
624-
result.intvalue = 1;
625-
setTokenValue(parent, result, settings);
626-
} else if (value1.isIntValue() && value2.isIntValue()) {
627-
result.intvalue = value1.intvalue != value2.intvalue;
628-
setTokenValue(parent, result, settings);
629-
}
630-
}
631-
break;
632-
case '>': {
633-
const bool f = value1.isFloatValue() || value2.isFloatValue();
634-
if (!f && !value1.isIntValue() && !value2.isIntValue())
635-
break;
636-
if (parent->str() == ">")
637-
result.intvalue = f ? (floatValue1 > floatValue2) : (value1.intvalue > value2.intvalue);
638-
else if (parent->str() == ">=")
639-
result.intvalue = f ? (floatValue1 >= floatValue2) : (value1.intvalue >= value2.intvalue);
640-
else if (!f && parent->str() == ">>" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)
641-
result.intvalue = value1.intvalue >> value2.intvalue;
642-
else
643-
break;
644-
setTokenValue(parent, result, settings);
645-
break;
646-
}
647-
case '<': {
648-
const bool f = value1.isFloatValue() || value2.isFloatValue();
649-
if (!f && !value1.isIntValue() && !value2.isIntValue())
650-
break;
651-
if (parent->str() == "<")
652-
result.intvalue = f ? (floatValue1 < floatValue2) : (value1.intvalue < value2.intvalue);
653-
else if (parent->str() == "<=")
654-
result.intvalue = f ? (floatValue1 <= floatValue2) : (value1.intvalue <= value2.intvalue);
655-
else if (!f && parent->str() == "<<" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)
656-
result.intvalue = value1.intvalue << value2.intvalue;
657-
else
658-
break;
659-
setTokenValue(parent, result, settings);
660-
break;
661-
}
662-
case '&':
663-
if (!value1.isIntValue() || !value2.isIntValue())
664-
break;
665-
if (parent->str() == "&")
666-
result.intvalue = value1.intvalue & value2.intvalue;
667-
else
668-
result.intvalue = value1.intvalue && value2.intvalue;
669-
setTokenValue(parent, result, settings);
670-
break;
671-
case '|':
672-
if (!value1.isIntValue() || !value2.isIntValue())
673-
break;
674-
if (parent->str() == "|")
675-
result.intvalue = value1.intvalue | value2.intvalue;
676-
else
677-
result.intvalue = value1.intvalue || value2.intvalue;
678-
setTokenValue(parent, result, settings);
679-
break;
680-
case '^':
681-
if (!value1.isIntValue() || !value2.isIntValue())
682-
break;
683-
result.intvalue = value1.intvalue ^ value2.intvalue;
684-
setTokenValue(parent, result, settings);
685-
break;
686-
default:
687-
// unhandled operator, do nothing
688-
break;
689655
}
690656
}
691657
}

0 commit comments

Comments
 (0)