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

Optimizations to ValueFlow and ForwardAnalyzer: 

- Remove errorPath of a value on assignment (this fixes enormous memory consumption for code with many subsequent assignments)
- De-virtualized a simple get function that was virtual for no reason
- Cloned function isAliasOf() for single values to avoid instantiating unnecessary std::list objects (
- Replaced a couple of trivial Token::Match/simpleMatch expressions by direct comparison
- Treat enumerators as literal values
This commit is contained in:
PKEuS 2020-05-19 21:03:28 +02:00
parent e2ead44a2d
commit dc701276de
2 changed files with 62 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/forwardanalyzer.cpp
View File

@ -89,15 +89,15 @@ struct ForwardTraversal {
checkThen = true; checkThen = true;
checkElse = true; checkElse = true;
} }
if (Token::simpleMatch(childTok, ":")) { if (childTok->str() == ":") {
if (checkThen && traverseRecursive(childTok->astOperand1(), f, traverseUnknown) == Progress::Break) if (checkThen && traverseRecursive(childTok->astOperand1(), f, traverseUnknown) == Progress::Break)
return Progress::Break; return Progress::Break;
if (checkElse && traverseRecursive(childTok->astOperand2(), f, traverseUnknown) == Progress::Break) if (checkElse && traverseRecursive(childTok->astOperand2(), f, traverseUnknown) == Progress::Break)
return Progress::Break; return Progress::Break;
} else { } else {
if (!checkThen && Token::simpleMatch(tok, "&&")) if (!checkThen && tok->str() == "&&")
return Progress::Continue; return Progress::Continue;
if (!checkElse && Token::simpleMatch(tok, "||")) if (!checkElse && tok->str() == "||")
return Progress::Continue; return Progress::Continue;
if (traverseRecursive(childTok, f, traverseUnknown) == Progress::Break) if (traverseRecursive(childTok, f, traverseUnknown) == Progress::Break)
return Progress::Break; return Progress::Break;
@ -425,7 +425,7 @@ struct ForwardTraversal {
static Token* assignExpr(Token* tok) { static Token* assignExpr(Token* tok) {
while (tok->astParent() && astIsLHS(tok)) { while (tok->astParent() && astIsLHS(tok)) {
if (Token::Match(tok->astParent(), "%assign%")) if (tok->astParent()->isAssignmentOp())
return tok->astParent(); return tok->astParent();
tok = tok->astParent(); tok = tok->astParent();
} }
@ -461,7 +461,7 @@ struct ForwardTraversal {
return nullptr; return nullptr;
if (Token::Match(tok, "%name% (")) if (Token::Match(tok, "%name% ("))
return getInitTok(tok->next()); return getInitTok(tok->next());
if (!Token::simpleMatch(tok, "(")) if (tok->str() != "(")
return nullptr; return nullptr;
if (!Token::simpleMatch(tok->astOperand2(), ";")) if (!Token::simpleMatch(tok->astOperand2(), ";"))
return nullptr; return nullptr;
@ -475,7 +475,7 @@ struct ForwardTraversal {
return nullptr; return nullptr;
if (Token::Match(tok, "%name% (")) if (Token::Match(tok, "%name% ("))
return getStepTok(tok->next()); return getStepTok(tok->next());
if (!Token::simpleMatch(tok, "(")) if (tok->str() != "(")
return nullptr; return nullptr;
if (!Token::simpleMatch(tok->astOperand2(), ";")) if (!Token::simpleMatch(tok->astOperand2(), ";"))
return nullptr; return nullptr;

93
lib/valueflow.cpp
View File

@ -219,7 +219,7 @@ const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, Val
{ {
if (!tok->astOperand1() || !tok->astOperand2()) if (!tok->astOperand1() || !tok->astOperand2())
return nullptr; return nullptr;
if (Token::Match(tok, "%comp%")) { if (tok->isComparisonOp()) {
if (tok->astOperand1()->hasKnownIntValue()) { if (tok->astOperand1()->hasKnownIntValue()) {
MathLib::bigint value = tok->astOperand1()->values().front().intvalue; MathLib::bigint value = tok->astOperand1()->values().front().intvalue;
if (isSaturated(value)) if (isSaturated(value))
@ -2082,6 +2082,35 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign
return true; return true;
} }
// Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable* var, const Token* tok, nonneg int varid, const ValueFlow::Value& val)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (isAliasOf(tok, varid))
return true;
if (var && !var->isPointer())
return false;
// Search through non value aliases
if (!val.isNonValue())
return false;
if (val.isInconclusive())
return false;
if (val.isLifetimeValue() && !val.isLocalLifetimeValue())
return false;
if (val.isLifetimeValue() && val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
return false;
if (!Token::Match(val.tokvalue, ".|&|*|%var%"))
return false;
if (astHasVar(val.tokvalue, tok->varId()))
return true;
return false;
}
// Check if its an alias of the variable or is being aliased to this variable // Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values) static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values)
{ {
@ -2258,7 +2287,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
return Action::Read; return Action::Read;
Action read = Action::Read; Action read = Action::Read;
if (isWritableValue(tok) && Token::Match(parent, "%assign%") && astIsLHS(tok) && if (parent && isWritableValue(tok) && parent->isAssignmentOp() && astIsLHS(tok) &&
parent->astOperand2()->hasKnownValue()) { parent->astOperand2()->hasKnownValue()) {
const Token* rhs = parent->astOperand2(); const Token* rhs = parent->astOperand2();
const ValueFlow::Value* rhsValue = getKnownValue(rhs, ValueFlow::Value::ValueType::INT); const ValueFlow::Value* rhsValue = getKnownValue(rhs, ValueFlow::Value::ValueType::INT);
@ -2267,7 +2296,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
a = Action::Invalid; a = Action::Invalid;
else else
a = Action::Write; a = Action::Write;
if (!Token::simpleMatch(parent, "=")) if (parent->str() != "=")
a |= Action::Read; a |= Action::Read;
return a; return a;
} }
@ -2308,9 +2337,8 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
pms.assume(tok, state); pms.assume(tok, state);
const bool isAssert = Token::Match(at, "assert|ASSERT"); const bool isAssert = Token::Match(at, "assert|ASSERT");
const bool isEndScope = Token::simpleMatch(at, "}");
if (!isAssert && !isEndScope) { if (!isAssert && !Token::simpleMatch(at, "}")) {
std::string s = state ? "true" : "false"; std::string s = state ? "true" : "false";
addErrorPath(tok, "Assuming condition is " + s); addErrorPath(tok, "Assuming condition is " + s);
} }
@ -2326,22 +2354,23 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
setTokenValue(tok, *value, getSettings()); setTokenValue(tok, *value, getSettings());
if (a.isInconclusive()) if (a.isInconclusive())
lowerToInconclusive(); lowerToInconclusive();
if (a.isWrite()) { if (a.isWrite() && tok->astParent()) {
if (Token::Match(tok->astParent(), "%assign%")) { if (tok->astParent()->isAssignmentOp()) {
// TODO: Check result // TODO: Check result
if (evalAssignment(*value, if (evalAssignment(*value,
tok->astParent()->str(), tok->astParent()->str(),
*getKnownValue(tok->astParent()->astOperand2(), ValueFlow::Value::ValueType::INT))) { *getKnownValue(tok->astParent()->astOperand2(), ValueFlow::Value::ValueType::INT))) {
const std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " + const std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " +
value->infoString()); value->infoString());
if (tok->astParent()->str() == "=")
value->errorPath.clear();
value->errorPath.emplace_back(tok, info); value->errorPath.emplace_back(tok, info);
} else { } else {
// TODO: Don't set to zero // TODO: Don't set to zero
value->intvalue = 0; value->intvalue = 0;
} }
} } else if (tok->astParent()->tokType() == Token::eIncDecOp) {
if (Token::Match(tok->astParent(), "++|--")) { const bool inc = tok->astParent()->str() == "++";
const bool inc = Token::simpleMatch(tok->astParent(), "++");
value->intvalue += (inc ? 1 : -1); value->intvalue += (inc ? 1 : -1);
const std::string info(tok->str() + " is " + std::string(inc ? "incremented" : "decremented") + const std::string info(tok->str() + " is " + std::string(inc ? "incremented" : "decremented") +
"', new value is " + value->infoString()); "', new value is " + value->infoString());
@ -2352,6 +2381,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
}; };
struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
std::unordered_map<nonneg int, const Variable*> varids;
ValueFlow::Value value; ValueFlow::Value value;
SingleValueFlowForwardAnalyzer() SingleValueFlowForwardAnalyzer()
@ -2362,7 +2392,9 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
: ValueFlowForwardAnalyzer(t), value(v) : ValueFlowForwardAnalyzer(t), value(v)
{} {}
virtual const std::unordered_map<nonneg int, const Variable*>& getVars() const = 0; const std::unordered_map<nonneg int, const Variable*>& getVars() const {
return varids;
}
virtual const ValueFlow::Value* getValue(const Token*) const OVERRIDE { virtual const ValueFlow::Value* getValue(const Token*) const OVERRIDE {
return &value; return &value;
@ -2382,13 +2414,12 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
virtual bool isAlias(const Token* tok) const OVERRIDE { virtual bool isAlias(const Token* tok) const OVERRIDE {
if (value.isLifetimeValue()) if (value.isLifetimeValue())
return false; return false;
const std::list<ValueFlow::Value> vals{value};
for (const auto& p:getVars()) { for (const auto& p:getVars()) {
nonneg int varid = p.first; nonneg int varid = p.first;
const Variable* var = p.second; const Variable* var = p.second;
if (tok->varId() == varid) if (tok->varId() == varid)
return true; return true;
if (isAliasOf(var, tok, varid, vals)) if (isAliasOf(var, tok, varid, value))
return true; return true;
} }
return false; return false;
@ -2450,21 +2481,16 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
struct VariableForwardAnalyzer : SingleValueFlowForwardAnalyzer { struct VariableForwardAnalyzer : SingleValueFlowForwardAnalyzer {
const Variable* var; const Variable* var;
std::unordered_map<nonneg int, const Variable*> varids;
VariableForwardAnalyzer() VariableForwardAnalyzer()
: SingleValueFlowForwardAnalyzer(), var(nullptr), varids() : SingleValueFlowForwardAnalyzer(), var(nullptr)
{} {}
VariableForwardAnalyzer(const Variable* v, const ValueFlow::Value& val, const TokenList* t) VariableForwardAnalyzer(const Variable* v, const ValueFlow::Value& val, const TokenList* t)
: SingleValueFlowForwardAnalyzer(val, t), var(v), varids() { : SingleValueFlowForwardAnalyzer(val, t), var(v) {
varids[var->declarationId()] = var; varids[var->declarationId()] = var;
} }
virtual const std::unordered_map<nonneg int, const Variable*>& getVars() const OVERRIDE {
return varids;
}
virtual bool match(const Token* tok) const OVERRIDE { virtual bool match(const Token* tok) const OVERRIDE {
return tok->varId() == var->declarationId(); return tok->varId() == var->declarationId();
} }
@ -2496,16 +2522,15 @@ static bool valueFlowForwardVariable(Token* const startToken,
struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer { struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer {
const Token* expr; const Token* expr;
std::unordered_map<nonneg int, const Variable*> varids;
bool local; bool local;
bool unknown; bool unknown;
ExpressionForwardAnalyzer() ExpressionForwardAnalyzer()
: SingleValueFlowForwardAnalyzer(), expr(nullptr), varids(), local(true), unknown(false) : SingleValueFlowForwardAnalyzer(), expr(nullptr), local(true), unknown(false)
{} {}
ExpressionForwardAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t) ExpressionForwardAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t)
: SingleValueFlowForwardAnalyzer(val, t), expr(e), varids(), local(true), unknown(false) { : SingleValueFlowForwardAnalyzer(val, t), expr(e), local(true), unknown(false) {
setupExprVarIds(); setupExprVarIds();
} }
@ -2546,10 +2571,6 @@ struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer {
return std::vector<int> {}; return std::vector<int> {};
} }
virtual const std::unordered_map<nonneg int, const Variable*>& getVars() const OVERRIDE {
return varids;
}
virtual bool match(const Token* tok) const OVERRIDE { virtual bool match(const Token* tok) const OVERRIDE {
return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true);
} }
@ -2648,7 +2669,7 @@ static void valueFlowForward(Token* startToken,
const Settings* settings) const Settings* settings)
{ {
const Token* expr = solveExprValues(exprTok, values); const Token* expr = solveExprValues(exprTok, values);
if (Token::Match(expr, "%var%") && expr->variable()) { if (expr->variable()) {
valueFlowForwardVariable(startToken, valueFlowForwardVariable(startToken,
endToken, endToken,
expr->variable(), expr->variable(),
@ -2941,7 +2962,7 @@ bool isLifetimeBorrowed(const Token *tok, const Settings *settings)
{ {
if (!tok) if (!tok)
return true; return true;
if (Token::simpleMatch(tok, ",")) if (tok->str() == ",")
return true; return true;
if (!tok->astParent()) if (!tok->astParent())
return true; return true;
@ -3438,7 +3459,7 @@ static bool isDecayedPointer(const Token *tok)
return false; return false;
if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return")) if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return"))
return true; return true;
if (Token::Match(tok->astParent(), "%cop%")) if (tok->astParent()->isConstOp())
return true; return true;
if (!Token::simpleMatch(tok->astParent(), "return")) if (!Token::simpleMatch(tok->astParent(), "return"))
return false; return false;
@ -3808,7 +3829,7 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
static bool isLiteralNumber(const Token *tok, bool cpp) static bool isLiteralNumber(const Token *tok, bool cpp)
{ {
return tok->isNumber() || tok->str() == "NULL" || (cpp && Token::Match(tok, "false|true|nullptr")); return tok->isNumber() || tok->isEnumerator() || tok->str() == "NULL" || (cpp && Token::Match(tok, "false|true|nullptr"));
} }
static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
@ -4260,8 +4281,8 @@ static void valueFlowInferCondition(TokenList* tokenlist,
continue; continue;
if (tok->hasKnownValue()) if (tok->hasKnownValue())
continue; continue;
if (Token::Match(tok, "%var%") && (Token::Match(tok->astParent(), "?|&&|!|%oror%") || if (tok->variable() && (Token::Match(tok->astParent(), "?|&&|!|%oror%") ||
Token::Match(tok->astParent()->previous(), "if|while ("))) { Token::Match(tok->astParent()->previous(), "if|while ("))) {
const ValueFlow::Value* result = proveNotEqual(tok->values(), 0); const ValueFlow::Value* result = proveNotEqual(tok->values(), 0);
if (!result) if (!result)
continue; continue;
@ -4270,7 +4291,7 @@ static void valueFlowInferCondition(TokenList* tokenlist,
value.bound = ValueFlow::Value::Bound::Point; value.bound = ValueFlow::Value::Bound::Point;
value.setKnown(); value.setKnown();
setTokenValue(tok, value, settings); setTokenValue(tok, value, settings);
} else if (Token::Match(tok, "%comp%")) { } else if (tok->isComparisonOp()) {
MathLib::bigint val = 0; MathLib::bigint val = 0;
const Token* varTok = nullptr; const Token* varTok = nullptr;
if (tok->astOperand1()->hasKnownIntValue()) { if (tok->astOperand1()->hasKnownIntValue()) {
@ -5991,9 +6012,7 @@ static void valueFlowUnknownFunctionReturn(TokenList *tokenlist, const Settings
if (settings->checkUnknownFunctionReturn.empty()) if (settings->checkUnknownFunctionReturn.empty())
return; return;
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!tok->astParent() || tok->str() != "(") if (!tok->astParent() || tok->str() != "(" || !tok->previous()->isName())
continue;
if (!Token::Match(tok->previous(), "%name%"))
continue; continue;
if (settings->checkUnknownFunctionReturn.find(tok->previous()->str()) == settings->checkUnknownFunctionReturn.end()) if (settings->checkUnknownFunctionReturn.find(tok->previous()->str()) == settings->checkUnknownFunctionReturn.end())
continue; continue;