walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 8871: improve check: mismatching container size conditions (#2988)

This commit is contained in:
Paul Fultz II 2021-01-10 06:30:00 -06:00 committed by GitHub
parent cdc0ba32e4
commit bc3f5554a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 161 additions and 149 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/checkcondition.cpp
View File

@ -1385,10 +1385,15 @@ void CheckCondition::alwaysTrueFalse()
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) { for (const Scope * scope : symbolDatabase->functionScopes) {
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (tok->link()) // don't write false positives when templates are used if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used
continue; continue;
if (!tok->hasKnownIntValue()) if (!tok->hasKnownIntValue())
continue; continue;
if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) {
const Function* f = tok->previous()->function();
if (f->functionScope && Token::Match(f->functionScope->bodyStart, "{ return true|false ;"))
continue;
}
{ {
// is this a condition.. // is this a condition..
const Token *parent = tok->astParent(); const Token *parent = tok->astParent();

8
lib/checkuninitvar.cpp
View File

@ -266,14 +266,6 @@ static void conditionAlwaysTrueOrFalse(const Token *tok, const std::map<int, Var
} }
else if (tok->isComparisonOp()) { else if (tok->isComparisonOp()) {
if (tok->hasKnownIntValue()) {
if (tok->values().front().intvalue)
*alwaysTrue = true;
else
*alwaysFalse = true;
return;
}
if (variableValue.empty()) { if (variableValue.empty()) {
return; return;
} }

2
lib/clangimport.cpp
View File

@ -392,7 +392,7 @@ std::string clangimport::AstNode::getSpelling() const
if (typeIndex <= 0) if (typeIndex <= 0)
return ""; return "";
} }
const std::string &str = mExtTokens[typeIndex - 1]; const std::string &str = mExtTokens[std::max(typeIndex - 1, 0)];
if (str.compare(0,4,"col:") == 0) if (str.compare(0,4,"col:") == 0)
return ""; return "";
if (str.compare(0,8,"<invalid") == 0) if (str.compare(0,8,"<invalid") == 0)

2
lib/mathlib.cpp
View File

@ -346,7 +346,7 @@ MathLib::biguint MathLib::toULongNumber(const std::string & str)
return ret; return ret;
} }
static unsigned int encodeMultiChar(const std::string& str) unsigned int MathLib::encodeMultiChar(const std::string& str)
{ {
unsigned int retval = 0; unsigned int retval = 0;
for (char it : str) { for (char it : str) {

2
lib/mathlib.h
View File

@ -125,6 +125,8 @@ public:
*/ */
static bool isOctalDigit(char c); static bool isOctalDigit(char c);
static unsigned int encodeMultiChar(const std::string& str);
/** /**
* \param[in] str character literal * \param[in] str character literal
* @return Number of internal representation of the character literal * @return Number of internal representation of the character literal

240
lib/valueflow.cpp
View File

@ -80,6 +80,7 @@
#include "analyzer.h" #include "analyzer.h"
#include "astutils.h" #include "astutils.h"
#include "errorlogger.h" #include "errorlogger.h"
#include "errortypes.h"
#include "forwardanalyzer.h" #include "forwardanalyzer.h"
#include "library.h" #include "library.h"
#include "mathlib.h" #include "mathlib.h"
@ -106,6 +107,7 @@
#include <map> #include <map>
#include <set> #include <set>
#include <stack> #include <stack>
#include <stdexcept>
#include <string> #include <string>
#include <tuple> #include <tuple>
#include <vector> #include <vector>
@ -341,6 +343,60 @@ static bool isComputableValue(const Token* parent, const ValueFlow::Value& value
return true; return true;
} }
template <class T>
T asInt(T x)
{
return x;
}
MathLib::bigint asInt(float x) { return x; }
MathLib::bigint asInt(double x) { return x; }
template <class T, class U>
static T calculate(const std::string& s, T x, U y)
{
switch (MathLib::encodeMultiChar(s)) {
case '+':
return x + y;
case '-':
return x - y;
case '*':
return x * y;
case '/':
return x / y;
case '%':
return asInt(x) % asInt(y);
case '&':
return asInt(x) & asInt(y);
case '|':
return asInt(x) | asInt(y);
case '^':
return asInt(x) ^ asInt(y);
case '>':
return x > y;
case '<':
return x < y;
case '<<':
return asInt(x) << asInt(y);
case '>>':
return asInt(x) >> asInt(y);
case '&&':
return x && y;
case '||':
return x || y;
case '==':
return x == y;
case '!=':
return x != y;
case '>=':
return x >= y;
case '<=':
return x <= y;
}
throw InternalError(nullptr, "Unknown operator: " + s);
}
/** Set token value for cast */ /** Set token value for cast */
static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings); static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings);
@ -359,31 +415,40 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
if (value.isContainerSizeValue()) { if (value.isContainerSizeValue()) {
// .empty, .size, +"abc", +'a' // .empty, .size, +"abc", +'a'
if (parent->str() == "+" && parent->astOperand1() && parent->astOperand2()) { if (Token::Match(parent, "+|==|!=") && parent->astOperand1() && parent->astOperand2()) {
for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) { for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) {
for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) { for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) {
if (value1.path != value2.path) if (value1.path != value2.path)
continue; continue;
ValueFlow::Value result; ValueFlow::Value result;
if (Token::Match(parent, "%comp%"))
result.valueType = ValueFlow::Value::ValueType::INT;
else
result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
if (value1.isContainerSizeValue() && value2.isContainerSizeValue()) if (value1.isContainerSizeValue() && value2.isContainerSizeValue())
result.intvalue = value1.intvalue + value2.intvalue; result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
else if (value1.isContainerSizeValue() && value2.isTokValue() && value2.tokvalue->tokType() == Token::eString) else if (value1.isContainerSizeValue() && value2.isTokValue() && value2.tokvalue->tokType() == Token::eString)
result.intvalue = value1.intvalue + Token::getStrLength(value2.tokvalue); result.intvalue = calculate(parent->str(), value1.intvalue, Token::getStrLength(value2.tokvalue));
else if (value2.isContainerSizeValue() && value1.isTokValue() && value1.tokvalue->tokType() == Token::eString) else if (value2.isContainerSizeValue() && value1.isTokValue() && value1.tokvalue->tokType() == Token::eString)
result.intvalue = Token::getStrLength(value1.tokvalue) + value2.intvalue; result.intvalue = calculate(parent->str(), Token::getStrLength(value1.tokvalue), value2.intvalue);
else else
continue; continue;
combineValueProperties(value1, value2, &result); combineValueProperties(value1, value2, &result);
if (Token::simpleMatch(parent, "==") && result.intvalue)
continue;
if (Token::simpleMatch(parent, "!=") && !result.intvalue)
continue;
setTokenValue(parent, result, settings); setTokenValue(parent, result, settings);
} }
} }
} }
else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) &&
else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && parent->astOperand1() && parent->astOperand1()->valueType()) { parent->astOperand1() && parent->astOperand1()->valueType()) {
const Library::Container *c = parent->astOperand1()->valueType()->container; const Library::Container *c = parent->astOperand1()->valueType()->container;
const Library::Container::Yield yields = c ? c->getYield(parent->strAt(1)) : Library::Container::Yield::NO_YIELD; const Library::Container::Yield yields = c ? c->getYield(parent->strAt(1)) : Library::Container::Yield::NO_YIELD;
if (yields == Library::Container::Yield::SIZE) { if (yields == Library::Container::Yield::SIZE) {
@ -545,147 +610,48 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
combineValueProperties(value1, value2, &result); combineValueProperties(value1, value2, &result);
const double floatValue1 = value1.isIntValue() ? value1.intvalue : value1.floatValue; const double floatValue1 = value1.isIntValue() ? value1.intvalue : value1.floatValue;
const double floatValue2 = value2.isIntValue() ? value2.intvalue : value2.floatValue; const double floatValue2 = value2.isIntValue() ? value2.intvalue : value2.floatValue;
switch (parent->str()[0]) { const bool isFloat = value1.isFloatValue() || value2.isFloatValue();
case '+': if (isFloat && Token::Match(parent, "&|^|%|<<|>>|%or%"))
if (value1.isTokValue() || value2.isTokValue()) continue;
break; if (Token::Match(parent, "<<|>>") &&
if (value1.isFloatValue() || value2.isFloatValue()) { !(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits))
result.valueType = ValueFlow::Value::ValueType::FLOAT; continue;
result.floatValue = floatValue1 + floatValue2; if (Token::Match(parent, "/|%") && floatValue2 == 0)
continue;
if (Token::Match(parent, "==|!=")) {
if ((value1.isIntValue() && value2.isTokValue()) || (value1.isTokValue() && value2.isIntValue())) {
if (parent->str() == "==")
result.intvalue = 0;
else if (parent->str() == "!=")
result.intvalue = 1;
} else if (value1.isIntValue() && value2.isIntValue()) {
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
} else { } else {
result.intvalue = value1.intvalue + value2.intvalue; continue;
} }
setTokenValue(parent, result, settings); setTokenValue(parent, result, settings);
break; } else if (Token::Match(parent, "%comp%")) {
case '-': if (!isFloat && !value1.isIntValue() && !value2.isIntValue())
continue;
if (isFloat)
result.intvalue = calculate(parent->str(), floatValue1, floatValue2);
else
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
setTokenValue(parent, result, settings);
} else if (Token::Match(parent, "%op%")) {
if (value1.isTokValue() || value2.isTokValue()) if (value1.isTokValue() || value2.isTokValue())
break; break;
if (value1.isFloatValue() || value2.isFloatValue()) { if (isFloat) {
result.valueType = ValueFlow::Value::ValueType::FLOAT; result.valueType = ValueFlow::Value::ValueType::FLOAT;
result.floatValue = floatValue1 - floatValue2; result.floatValue = calculate(parent->str(), floatValue1, floatValue2);
} else { } else {
// Avoid overflow result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
if (value1.intvalue < 0 && value2.intvalue > value1.intvalue - LLONG_MIN)
break;
result.intvalue = value1.intvalue - value2.intvalue;
} }
// If the bound comes from the second value then invert the bound // If the bound comes from the second value then invert the bound when subtracting
if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) if (Token::simpleMatch(parent, "-") && value2.bound == result.bound &&
value2.bound != ValueFlow::Value::Bound::Point)
result.invertBound(); result.invertBound();
setTokenValue(parent, result, settings); setTokenValue(parent, result, settings);
break;
case '*':
if (value1.isTokValue() || value2.isTokValue())
break;
if (value1.isFloatValue() || value2.isFloatValue()) {
result.valueType = ValueFlow::Value::ValueType::FLOAT;
result.floatValue = floatValue1 * floatValue2;
} else {
result.intvalue = value1.intvalue * value2.intvalue;
}
setTokenValue(parent, result, settings);
break;
case '/':
if (value1.isTokValue() || value2.isTokValue() || value2.intvalue == 0)
break;
if (value1.isFloatValue() || value2.isFloatValue()) {
result.valueType = ValueFlow::Value::ValueType::FLOAT;
result.floatValue = floatValue1 / floatValue2;
} else {
result.intvalue = value1.intvalue / value2.intvalue;
}
setTokenValue(parent, result, settings);
break;
case '%':
if (!value1.isIntValue() || !value2.isIntValue())
break;
if (value2.intvalue == 0)
break;
result.intvalue = value1.intvalue % value2.intvalue;
setTokenValue(parent, result, settings);
break;
case '=':
if (parent->str() == "==") {
if ((value1.isIntValue() && value2.isTokValue()) ||
(value1.isTokValue() && value2.isIntValue())) {
result.intvalue = 0;
setTokenValue(parent, result, settings);
} else if (value1.isIntValue() && value2.isIntValue()) {
result.intvalue = value1.intvalue == value2.intvalue;
setTokenValue(parent, result, settings);
}
}
break;
case '!':
if (parent->str() == "!=") {
if ((value1.isIntValue() && value2.isTokValue()) ||
(value1.isTokValue() && value2.isIntValue())) {
result.intvalue = 1;
setTokenValue(parent, result, settings);
} else if (value1.isIntValue() && value2.isIntValue()) {
result.intvalue = value1.intvalue != value2.intvalue;
setTokenValue(parent, result, settings);
}
}
break;
case '>': {
const bool f = value1.isFloatValue() || value2.isFloatValue();
if (!f && !value1.isIntValue() && !value2.isIntValue())
break;
if (parent->str() == ">")
result.intvalue = f ? (floatValue1 > floatValue2) : (value1.intvalue > value2.intvalue);
else if (parent->str() == ">=")
result.intvalue = f ? (floatValue1 >= floatValue2) : (value1.intvalue >= value2.intvalue);
else if (!f && parent->str() == ">>" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)
result.intvalue = value1.intvalue >> value2.intvalue;
else
break;
setTokenValue(parent, result, settings);
break;
}
case '<': {
const bool f = value1.isFloatValue() || value2.isFloatValue();
if (!f && !value1.isIntValue() && !value2.isIntValue())
break;
if (parent->str() == "<")
result.intvalue = f ? (floatValue1 < floatValue2) : (value1.intvalue < value2.intvalue);
else if (parent->str() == "<=")
result.intvalue = f ? (floatValue1 <= floatValue2) : (value1.intvalue <= value2.intvalue);
else if (!f && parent->str() == "<<" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)
result.intvalue = value1.intvalue << value2.intvalue;
else
break;
setTokenValue(parent, result, settings);
break;
}
case '&':
if (!value1.isIntValue() || !value2.isIntValue())
break;
if (parent->str() == "&")
result.intvalue = value1.intvalue & value2.intvalue;
else
result.intvalue = value1.intvalue && value2.intvalue;
setTokenValue(parent, result, settings);
break;
case '|':
if (!value1.isIntValue() || !value2.isIntValue())
break;
if (parent->str() == "|")
result.intvalue = value1.intvalue | value2.intvalue;
else
result.intvalue = value1.intvalue || value2.intvalue;
setTokenValue(parent, result, settings);
break;
case '^':
if (!value1.isIntValue() || !value2.isIntValue())
break;
result.intvalue = value1.intvalue ^ value2.intvalue;
setTokenValue(parent, result, settings);
break;
default:
// unhandled operator, do nothing
break;
} }
} }
} }

47
test/testvalueflow.cpp
View File

@ -4775,6 +4775,53 @@ private:
"}"; "}";
ASSERT_EQUALS("", ASSERT_EQUALS("",
isKnownContainerSizeValue(tokenValues(code, "ints . front", ValueFlow::Value::ValueType::CONTAINER_SIZE), 1)); isKnownContainerSizeValue(tokenValues(code, "ints . front", ValueFlow::Value::ValueType::CONTAINER_SIZE), 1));
code = "void f(std::string str) {\n"
" if (str == \"123\")\n"
" bool x = str.empty();\n"
"}\n";
ASSERT_EQUALS("",
isKnownContainerSizeValue(tokenValues(code, "str . empty", ValueFlow::Value::ValueType::CONTAINER_SIZE), 3));
code = "void f(std::string str) {\n"
" if (str == \"123\") {\n"
" bool x = (str == \"\");\n"
" bool a = x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0));
code = "void f(std::string str) {\n"
" if (str == \"123\") {\n"
" bool x = (str != \"\");\n"
" bool a = x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 1));
code = "void f(std::string str) {\n"
" if (str == \"123\") {\n"
" bool x = (str == \"321\");\n"
" bool a = x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 1));
code = "void f(std::string str) {\n"
" if (str == \"123\") {\n"
" bool x = (str != \"321\");\n"
" bool a = x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 0));
code = "void f(std::string str) {\n"
" if (str.size() == 1) {\n"
" bool x = (str == \"123\");\n"
" bool a = x;\n"
" }\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0));
} }
void valueFlowDynamicBufferSize() { void valueFlowDynamicBufferSize() {