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

Add symbolic values to ValueFlow (#3367)

This commit is contained in:
Paul Fultz II 2021-07-30 14:29:35 -05:00 committed by GitHub
parent f85f3c28e1
commit 3a7ba3cd29
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 271 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/astutils.cpp
View File

@ -878,7 +878,7 @@ static bool compareKnownValue(const Token * const tok1, const Token * const tok2
if (v1 == tok1->values().end()) {
return false;
}
if (v1->isNonValue() || v1->isContainerSizeValue())
if (v1->isNonValue() || v1->isContainerSizeValue() || v1->isSymbolicValue())
return false;
const auto v2 = std::find_if(tok2->values().begin(), tok2->values().end(), isKnownFn);
if (v2 == tok2->values().end()) {

2
lib/reverseanalyzer.cpp
View File

@ -172,7 +172,7 @@ struct ReverseTraversal {
settings);
}
// Assignment to
} else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownValue() &&
} else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownIntValue() &&
isConstExpression(assignTok->astOperand2(), settings->library, true, true)) {
const std::string info = "Assignment to '" + assignTok->expressionString() + "'";
ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand2(), info);

50
lib/token.cpp
View File

@ -1292,8 +1292,7 @@ std::string Token::stringifyList(const stringifyOptions& options, const std::vec
if (options.linenumbers) {
ret += std::to_string(lineNumber);
ret += ':';
if (lineNumber == tok->linenr())
ret += ' ';
ret += ' ';
}
} else {
while (lineNumber < tok->linenr()) {
@ -1742,6 +1741,10 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
case ValueFlow::Value::ValueType::LIFETIME:
out << "lifetime=\"" << value.tokvalue << '\"';
break;
case ValueFlow::Value::ValueType::SYMBOLIC:
out << "tokvalue=\"" << value.tokvalue << '\"';
out << " intvalue=\"" << value.intvalue << '\"';
break;
}
if (value.condition)
out << " condition-line=\"" << value.condition->linenr() << '\"';
@ -1795,6 +1798,14 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
out << "lifetime[" << ValueFlow::Value::toString(value.lifetimeKind) << "]=("
<< value.tokvalue->expressionString() << ")";
break;
case ValueFlow::Value::ValueType::SYMBOLIC:
out << "symbolic=(" << value.tokvalue->expressionString();
if (value.intvalue > 0)
out << "+" << value.intvalue;
else if (value.intvalue < 0)
out << "-" << -value.intvalue;
out << ")";
break;
}
if (value.indirect > 0)
for (int i=0; i<value.indirect; i++)
@ -2124,8 +2135,13 @@ bool Token::addValue(const ValueFlow::Value &value)
{
if (value.isKnown() && mImpl->mValues) {
// Clear all other values of the same type since value is known
mImpl->mValues->remove_if([&](const ValueFlow::Value & x) {
return x.valueType == value.valueType;
mImpl->mValues->remove_if([&](const ValueFlow::Value& x) {
if (x.valueType != value.valueType)
return false;
// Allow multiple known symbolic values
if (x.isSymbolicValue())
return !x.isKnown();
return true;
});
}
@ -2146,31 +2162,7 @@ bool Token::addValue(const ValueFlow::Value &value)
continue;
// different value => continue
bool differentValue = true;
switch (it->valueType) {
case ValueFlow::Value::ValueType::INT:
case ValueFlow::Value::ValueType::CONTAINER_SIZE:
case ValueFlow::Value::ValueType::BUFFER_SIZE:
case ValueFlow::Value::ValueType::ITERATOR_START:
case ValueFlow::Value::ValueType::ITERATOR_END:
differentValue = (it->intvalue != value.intvalue);
break;
case ValueFlow::Value::ValueType::TOK:
case ValueFlow::Value::ValueType::LIFETIME:
differentValue = (it->tokvalue != value.tokvalue);
break;
case ValueFlow::Value::ValueType::FLOAT:
// TODO: Write some better comparison
differentValue = (it->floatValue > value.floatValue || it->floatValue < value.floatValue);
break;
case ValueFlow::Value::ValueType::MOVED:
differentValue = (it->moveKind != value.moveKind);
break;
case ValueFlow::Value::ValueType::UNINIT:
differentValue = false;
break;
}
if (differentValue)
if (!it->equalValue(value))
continue;
if ((value.isTokValue() || value.isLifetimeValue()) && (it->tokvalue != value.tokvalue) && (it->tokvalue->str() != value.tokvalue->str()))

133
lib/valueflow.cpp
View File

@ -1387,7 +1387,7 @@ static void valueFlowBitAnd(TokenList *tokenlist)
static void valueFlowSameExpressions(TokenList *tokenlist)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (tok->hasKnownValue())
if (tok->hasKnownIntValue())
continue;
if (!tok->astOperand1() || !tok->astOperand2())
@ -2032,7 +2032,7 @@ struct ValueFlowAnalyzer : Analyzer {
const ValueFlow::Value* value = getValue(tok);
if (!value)
return Action::None;
if (!(value->isIntValue() || value->isFloatValue()))
if (!(value->isIntValue() || value->isFloatValue() || value->isSymbolicValue()))
return Action::None;
const Token* parent = tok->astParent();
@ -2409,6 +2409,8 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
dependOnThis = exprDependsOnThis(expr);
setupExprVarIds(expr);
if (val.isSymbolicValue())
setupExprVarIds(val.tokvalue);
}
virtual const ValueType* getValueType(const Token*) const OVERRIDE {
@ -3992,6 +3994,100 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
}
}
static ValueFlow::Value makeSymbolic(const Token* tok, MathLib::bigint delta = 0)
{
ValueFlow::Value value;
value.setKnown();
value.valueType = ValueFlow::Value::ValueType::SYMBOLIC;
value.tokvalue = tok;
value.intvalue = delta;
return value;
}
static void valueFlowSymbolic(TokenList* tokenlist, SymbolDatabase* symboldatabase)
{
for (const Scope* scope : symboldatabase->functionScopes) {
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (!Token::simpleMatch(tok, "="))
continue;
if (!tok->astOperand1())
continue;
if (!tok->astOperand2())
continue;
if (tok->astOperand1()->hasKnownIntValue())
continue;
if (tok->astOperand2()->hasKnownIntValue())
continue;
Token* start = nextAfterAstRightmostLeaf(tok);
const Token* end = scope->bodyEnd;
ValueFlow::Value rhs = makeSymbolic(tok->astOperand2());
rhs.errorPath.emplace_back(tok,
tok->astOperand1()->expressionString() + " is assigned '" +
tok->astOperand2()->expressionString() + "' here.");
valueFlowForward(start, end, tok->astOperand1(), {rhs}, tokenlist, tokenlist->getSettings());
ValueFlow::Value lhs = makeSymbolic(tok->astOperand1());
lhs.errorPath.emplace_back(tok,
tok->astOperand1()->expressionString() + " is assigned '" +
tok->astOperand2()->expressionString() + "' here.");
valueFlowForward(start, end, tok->astOperand2(), {lhs}, tokenlist, tokenlist->getSettings());
}
}
}
static void valueFlowSymbolicInfer(TokenList* tokenlist, SymbolDatabase* symboldatabase)
{
for (const Scope* scope : symboldatabase->functionScopes) {
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (!Token::Match(tok, "-|%comp%"))
continue;
if (tok->hasKnownIntValue())
continue;
if (!tok->astOperand1())
continue;
if (!tok->astOperand2())
continue;
if (tok->astOperand1()->hasKnownIntValue())
continue;
if (tok->astOperand2()->hasKnownIntValue())
continue;
MathLib::bigint rhsvalue = 0;
const ValueFlow::Value* rhs =
ValueFlow::findValue(tok->astOperand2()->values(), nullptr, [&](const ValueFlow::Value& v) {
return v.isSymbolicValue() && v.tokvalue && v.tokvalue->exprId() == tok->astOperand1()->exprId();
});
if (rhs)
rhsvalue = rhs->intvalue;
MathLib::bigint lhsvalue = 0;
const ValueFlow::Value* lhs =
ValueFlow::findValue(tok->astOperand1()->values(), nullptr, [&](const ValueFlow::Value& v) {
return v.isSymbolicValue() && v.tokvalue && v.tokvalue->exprId() == tok->astOperand2()->exprId();
});
if (lhs)
lhsvalue = lhs->intvalue;
if (!lhs && !rhs)
continue;
ValueFlow::Value value{calculate(tok->str(), lhsvalue, rhsvalue)};
if (lhs && rhs) {
if (lhs->isImpossible() != rhs->isImpossible())
continue;
combineValueProperties(*lhs, *rhs, &value);
} else if (lhs) {
value.valueKind = lhs->valueKind;
value.errorPath = lhs->errorPath;
} else if (rhs) {
value.valueKind = rhs->valueKind;
value.errorPath = rhs->errorPath;
}
setTokenValue(tok, value, tokenlist->getSettings());
}
}
}
static void valueFlowForwardAssign(Token* const tok,
const Token* expr,
std::vector<const Variable*> vars,
@ -4044,17 +4140,13 @@ static void valueFlowForwardAssign(Token* const tok,
// Skip RHS
const Token * nextExpression = tok->astParent() ? nextAfterAstRightmostLeaf(tok->astParent()) : tok->next();
if (std::any_of(values.begin(), values.end(), std::mem_fn(&ValueFlow::Value::isTokValue))) {
std::list<ValueFlow::Value> tokvalues;
std::copy_if(values.begin(),
values.end(),
std::back_inserter(tokvalues),
std::mem_fn(&ValueFlow::Value::isTokValue));
valueFlowForward(const_cast<Token*>(nextExpression), endOfVarScope, expr, values, tokenlist, settings);
values.remove_if(std::mem_fn(&ValueFlow::Value::isTokValue));
}
for (ValueFlow::Value& value:values)
for (ValueFlow::Value& value : values) {
if (value.isSymbolicValue())
continue;
if (value.isTokValue())
continue;
value.tokvalue = tok;
}
valueFlowForward(const_cast<Token*>(nextExpression), endOfVarScope, expr, values, tokenlist, settings);
}
@ -4163,7 +4255,7 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat
std::set<ValueFlow::Value::ValueType> types;
if (tok->astOperand1()->hasKnownValue()) {
for (const ValueFlow::Value& value:tok->astOperand1()->values()) {
if (value.isKnown())
if (value.isKnown() && !value.isSymbolicValue())
types.insert(value.valueType);
}
}
@ -4719,7 +4811,7 @@ struct SimpleConditionHandler : ConditionHandler {
ValueFlow::Value false_value;
const Token *vartok = parseCompareInt(tok, true_value, false_value);
if (vartok) {
if (vartok->hasKnownValue())
if (vartok->hasKnownIntValue())
return {};
if (vartok->str() == "=" && vartok->astOperand1() && vartok->astOperand2())
vartok = vartok->astOperand1();
@ -4887,7 +4979,7 @@ static void valueFlowInferCondition(TokenList* tokenlist,
for (Token* tok = tokenlist->front(); tok; tok = tok->next()) {
if (!tok->astParent())
continue;
if (tok->hasKnownValue())
if (tok->hasKnownIntValue())
continue;
if (tok->variable() && (Token::Match(tok->astParent(), "?|&&|!|%oror%") ||
Token::Match(tok->astParent()->previous(), "if|while ("))) {
@ -6365,7 +6457,7 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold
continue;
if (!astIsContainer(var->nameToken()))
continue;
if (var->nameToken()->hasKnownValue())
if (var->nameToken()->hasKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
continue;
if (!Token::Match(var->nameToken(), "%name% ;") &&
!(Token::Match(var->nameToken(), "%name% {") && Token::simpleMatch(var->nameToken()->next()->link(), "} ;")))
@ -6824,6 +6916,13 @@ std::string ValueFlow::Value::infoString() const
return "end=" + MathLib::toString(intvalue);
case ValueType::LIFETIME:
return "lifetime=" + tokvalue->str();
case ValueType::SYMBOLIC:
std::string result = "symbolic=" + tokvalue->expressionString();
if (intvalue > 0)
result += "+" + MathLib::toString(intvalue);
else if (intvalue < 0)
result += "-" + MathLib::toString(-intvalue);
return result;
}
throw InternalError(nullptr, "Invalid ValueFlow Value type");
}
@ -6892,6 +6991,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowGlobalStaticVar(tokenlist, settings);
valueFlowPointerAlias(tokenlist);
valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSymbolic(tokenlist, symboldatabase);
valueFlowBitAnd(tokenlist);
valueFlowSameExpressions(tokenlist);
valueFlowConditionExpressions(tokenlist, symboldatabase, errorLogger, settings);
@ -6901,6 +7001,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
while (n > 0 && values < getTotalValues(tokenlist)) {
values = getTotalValues(tokenlist);
valueFlowImpossibleValues(tokenlist, settings);
valueFlowSymbolicInfer(tokenlist, symboldatabase);
valueFlowArrayBool(tokenlist);
valueFlowRightShift(tokenlist, settings);
valueFlowAfterMove(tokenlist, symboldatabase, settings);

26
lib/valueflow.h
View File

@ -26,6 +26,7 @@
#include "mathlib.h"
#include "utils.h"
#include <cassert>
#include <functional>
#include <list>
#include <string>
@ -132,6 +133,13 @@ namespace ValueFlow {
case ValueType::LIFETIME:
if (tokvalue != rhs.tokvalue)
return false;
break;
case ValueType::SYMBOLIC:
if (tokvalue != rhs.tokvalue)
return false;
if (intvalue != rhs.intvalue)
return false;
break;
}
return true;
}
@ -140,6 +148,7 @@ namespace ValueFlow {
static void visitValue(T& self, F f) {
switch (self.valueType) {
case ValueType::INT:
case ValueType::SYMBOLIC:
case ValueType::BUFFER_SIZE:
case ValueType::CONTAINER_SIZE:
case ValueType::ITERATOR_START:
@ -175,6 +184,8 @@ namespace ValueFlow {
template <class Compare>
bool compareValue(const Value& rhs, Compare compare) const {
assert((!this->isSymbolicValue() && !rhs.isSymbolicValue()) ||
(this->valueType == rhs.valueType && this->tokvalue == rhs.tokvalue));
bool result = false;
visitValue(
*this,
@ -229,7 +240,19 @@ namespace ValueFlow {
std::string infoString() const;
enum class ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE, ITERATOR_START, ITERATOR_END } valueType;
enum class ValueType {
INT,
TOK,
FLOAT,
MOVED,
UNINIT,
CONTAINER_SIZE,
LIFETIME,
BUFFER_SIZE,
ITERATOR_START,
ITERATOR_END,
SYMBOLIC
} valueType;
bool isIntValue() const {
return valueType == ValueType::INT;
}
@ -263,6 +286,7 @@ namespace ValueFlow {
bool isIteratorEndValue() const {
return valueType == ValueType::ITERATOR_END;
}
bool isSymbolicValue() const { return valueType == ValueType::SYMBOLIC; }
bool isLocalLifetimeValue() const {
return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Local;

111
test/testvalueflow.cpp
View File

@ -24,16 +24,17 @@
#include "tokenize.h"
#include "valueflow.h"
#include <simplecpp.h>
#include <algorithm>
#include <cmath>
#include <cstdint>
#include <cstring>
#include <functional>
#include <list>
#include <map>
#include <simplecpp.h>
#include <string>
#include <utility>
#include <vector>
#include <cstdint>
#include <cstring>
class TestValueFlow : public TestFixture {
public:
@ -142,6 +143,7 @@ private:
TEST_CASE(valueFlowIdempotent);
TEST_CASE(valueFlowUnsigned);
TEST_CASE(valueFlowMod);
TEST_CASE(valueFlowSymbolic);
}
static bool isNotTokValue(const ValueFlow::Value &val) {
@ -152,6 +154,8 @@ private:
return !val.isLifetimeValue();
}
static bool isNotUninitValue(const ValueFlow::Value& val) { return !val.isUninitValue(); }
static bool isNotPossible(const ValueFlow::Value& val) {
return !val.isPossible();
}
@ -177,6 +181,8 @@ private:
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value& val:tok->values()) {
if (val.isSymbolicValue())
continue;
if (val.isKnown() && val.intvalue == value)
return true;
}
@ -186,6 +192,26 @@ private:
return false;
}
bool testValueOfXKnown(const char code[], unsigned int linenr, const std::string& expr, int value) {
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value& val : tok->values()) {
if (!val.isSymbolicValue())
continue;
if (val.isKnown() && val.intvalue == value && val.tokvalue->expressionString() == expr)
return true;
}
}
}
return false;
}
bool testValueOfXImpossible(const char code[], unsigned int linenr, int value) {
// Tokenize..
Tokenizer tokenizer(&settings, this);
@ -195,6 +221,8 @@ private:
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value& val:tok->values()) {
if (val.isSymbolicValue())
continue;
if (val.isImpossible() && val.intvalue == value)
return true;
}
@ -213,6 +241,8 @@ private:
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value& val:tok->values()) {
if (val.isSymbolicValue())
continue;
if (val.isInconclusive() && val.intvalue == value)
return true;
}
@ -1152,7 +1182,8 @@ private:
" y += 12;\n"
" if (y == 32) {}"
"}\n";
ASSERT_EQUALS("5,Assuming that condition 'y==32' is not redundant\n"
ASSERT_EQUALS("2,x is assigned 'y' here.\n"
"5,Assuming that condition 'y==32' is not redundant\n"
"4,Compound assignment '+=', assigned value is 20\n"
"2,Assignment 'x=y', assigned value is 20\n",
getErrorPathForX(code, 3U));
@ -1173,7 +1204,10 @@ private:
" for (x = a; x < 50; x++) {}\n"
" b = x;\n"
"}\n";
ASSERT_EQUALS("3,After for loop, x has value 50\n",
ASSERT_EQUALS("3,x is assigned 'a' here.\n"
"3,x is incremented', new value is symbolic=a+1\n"
"3,x is incremented', new value is symbolic=a+2\n"
"3,After for loop, x has value 50\n",
getErrorPathForX(code, 4U));
}
@ -4298,6 +4332,7 @@ private:
" a = x + 1;\n"
"}\n";
values = tokenValues(code, "x +");
values.remove_if(&isNotUninitValue);
ASSERT_EQUALS(true, values.empty());
// #8494 - overloaded operator &
@ -4682,6 +4717,7 @@ private:
static std::string isPossibleContainerSizeValue(std::list<ValueFlow::Value> values,
MathLib::bigint i,
bool unique = true) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isSymbolicValue));
if (!unique)
values.remove_if(&isNotPossible);
if (values.size() != 1)
@ -4698,6 +4734,7 @@ private:
static std::string isImpossibleContainerSizeValue(std::list<ValueFlow::Value> values,
MathLib::bigint i,
bool unique = true) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isSymbolicValue));
if (!unique)
values.remove_if(&isNotImpossible);
if (values.size() != 1)
@ -4714,6 +4751,7 @@ private:
static std::string isInconclusiveContainerSizeValue(std::list<ValueFlow::Value> values,
MathLib::bigint i,
bool unique = true) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isSymbolicValue));
if (!unique)
values.remove_if(&isNotInconclusive);
if (values.size() != 1)
@ -4728,6 +4766,7 @@ private:
}
static std::string isKnownContainerSizeValue(std::list<ValueFlow::Value> values, MathLib::bigint i, bool unique = true) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isSymbolicValue));
if (!unique)
values.remove_if(&isNotKnown);
if (values.size() != 1)
@ -5898,6 +5937,68 @@ private:
ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 0));
ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1));
}
void valueFlowSymbolic() {
const char* code;
code = "int f(int i) {\n"
" int j = i;\n"
" int x = i;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, "j", 0));
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, "i", 0));
code = "int f(int i) {\n"
" int j = i;\n"
" int x = j;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, "i", 0));
ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, "j", 0));
code = "void g(int&);\n"
"int f(int i) {\n"
" int j = i;\n"
" g(i);\n"
" int x = i;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, "i", 0));
ASSERT_EQUALS(false, testValueOfXKnown(code, 6U, "j", 0));
code = "int f(int i) {\n"
" int j = i;\n"
" j++;\n"
" int x = i == j;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 0));
code = "int f(int i) {\n"
" int j = i;\n"
" i++;\n"
" int x = i - j;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 1));
code = "int f(int i) {\n"
" int j = i;\n"
" i++;\n"
" int x = i > j;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 1));
code = "int f(int i) {\n"
" int j = i;\n"
" j++;\n"
" int x = j > i;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 1));
}
};
REGISTER_TEST(TestValueFlow)