From 921887a28122124e0f0b28130d682bfb7b5c487d Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 16 Feb 2020 09:02:22 -0600 Subject: [PATCH] Use valueFlowGeneric for valueFlowForwardExpression (#2537) --- lib/astutils.cpp | 135 +++++++------- lib/astutils.h | 2 + lib/exprengine.cpp | 2 +- lib/forwardanalyzer.cpp | 7 +- lib/programmemory.cpp | 22 +++ lib/programmemory.h | 3 + lib/symboldatabase.cpp | 6 +- lib/tokenize.cpp | 2 +- lib/tokenlist.cpp | 4 +- lib/valueflow.cpp | 374 +++++++++++++++++++++++++++++---------- test/testnullpointer.cpp | 41 +++++ test/testuninitvar.cpp | 2 +- test/testvalueflow.cpp | 19 +- 13 files changed, 441 insertions(+), 178 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index b7c1183e4..80a752fb2 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1321,7 +1321,7 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings, if (tok->variable() && Token::Match(tok2->astParent(), ". %name%") && isFunctionCall(tok2->astParent()->next()) && tok2->astParent()->astOperand1() == tok2) { const Variable * var = tok->variable(); bool isConst = var && var->isConst(); - if (!isConst && var) { + if (!isConst) { const ValueType * valueType = var->valueType(); isConst = (valueType && valueType->pointer == 1 && valueType->constness == 1); } @@ -1710,6 +1710,74 @@ bool isNullOperand(const Token *expr) return Token::Match(castOp, "NULL|nullptr") || (MathLib::isInt(castOp->str()) && MathLib::isNullValue(castOp->str())); } +bool isGlobalData(const Token *expr, bool cpp) +{ + bool globalData = false; + visitAstNodes(expr, + [&](const Token *tok) { + if (tok->varId() && !tok->variable()) { + // Bailout, this is probably global + globalData = true; + return ChildrenToVisit::none; + } + if (tok->originalName() == "->") { + // TODO check if pointer points at local data + globalData = true; + return ChildrenToVisit::none; + } else if (Token::Match(tok, "[*[]") && tok->astOperand1() && tok->astOperand1()->variable()) { + // TODO check if pointer points at local data + const Variable *lhsvar = tok->astOperand1()->variable(); + const ValueType *lhstype = tok->astOperand1()->valueType(); + if (lhsvar->isPointer()) { + globalData = true; + return ChildrenToVisit::none; + } else if (lhsvar->isArgument() && lhsvar->isArray()) { + globalData = true; + return ChildrenToVisit::none; + } else if (lhsvar->isArgument() && (!lhstype || (lhstype->type <= ValueType::Type::VOID && !lhstype->container))) { + globalData = true; + return ChildrenToVisit::none; + } + } + if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { + globalData = true; + return ChildrenToVisit::none; + } + if (tok->variable()) { + // TODO : Check references + if (tok->variable()->isReference() && tok != tok->variable()->nameToken()) { + globalData = true; + return ChildrenToVisit::none; + } + if (tok->variable()->isExtern()) { + globalData = true; + return ChildrenToVisit::none; + } + if (tok->previous()->str() != "." && !tok->variable()->isLocal() && !tok->variable()->isArgument()) { + globalData = true; + return ChildrenToVisit::none; + } + if (tok->variable()->isArgument() && tok->variable()->isPointer() && tok != expr) { + globalData = true; + return ChildrenToVisit::none; + } + if (tok->variable()->isPointerArray()) { + globalData = true; + return ChildrenToVisit::none; + } + } + // Unknown argument type => it might be some reference type.. + if (cpp && tok->str() == "." && tok->astOperand1() && tok->astOperand1()->variable() && !tok->astOperand1()->valueType()) { + globalData = true; + return ChildrenToVisit::none; + } + if (Token::Match(tok, ".|[")) + return ChildrenToVisit::op1; + return ChildrenToVisit::op1_and_op2; + }); + return globalData; +} + struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set &exprVarIds, bool local, bool inInnerClass, int depth) { // Parse the given tokens @@ -1984,70 +2052,7 @@ static bool hasVolatileCast(const Token *expr) bool FwdAnalysis::isGlobalData(const Token *expr) const { - bool globalData = false; - visitAstNodes(expr, - [&](const Token *tok) { - if (tok->varId() && !tok->variable()) { - // Bailout, this is probably global - globalData = true; - return ChildrenToVisit::none; - } - if (tok->originalName() == "->") { - // TODO check if pointer points at local data - globalData = true; - return ChildrenToVisit::none; - } else if (Token::Match(tok, "[*[]") && tok->astOperand1() && tok->astOperand1()->variable()) { - // TODO check if pointer points at local data - const Variable *lhsvar = tok->astOperand1()->variable(); - const ValueType *lhstype = tok->astOperand1()->valueType(); - if (lhsvar->isPointer()) { - globalData = true; - return ChildrenToVisit::none; - } else if (lhsvar->isArgument() && lhsvar->isArray()) { - globalData = true; - return ChildrenToVisit::none; - } else if (lhsvar->isArgument() && (!lhstype || (lhstype->type <= ValueType::Type::VOID && !lhstype->container))) { - globalData = true; - return ChildrenToVisit::none; - } - } - if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { - globalData = true; - return ChildrenToVisit::none; - } - if (tok->variable()) { - // TODO : Check references - if (tok->variable()->isReference() && tok != tok->variable()->nameToken()) { - globalData = true; - return ChildrenToVisit::none; - } - if (tok->variable()->isExtern()) { - globalData = true; - return ChildrenToVisit::none; - } - if (tok->previous()->str() != "." && !tok->variable()->isLocal() && !tok->variable()->isArgument()) { - globalData = true; - return ChildrenToVisit::none; - } - if (tok->variable()->isArgument() && tok->variable()->isPointer() && tok != expr) { - globalData = true; - return ChildrenToVisit::none; - } - if (tok->variable()->isPointerArray()) { - globalData = true; - return ChildrenToVisit::none; - } - } - // Unknown argument type => it might be some reference type.. - if (mCpp && tok->str() == "." && tok->astOperand1() && tok->astOperand1()->variable() && !tok->astOperand1()->valueType()) { - globalData = true; - return ChildrenToVisit::none; - } - if (Token::Match(tok, ".|[")) - return ChildrenToVisit::op1; - return ChildrenToVisit::op1_and_op2; - }); - return globalData; + return ::isGlobalData(expr, mCpp); } std::set FwdAnalysis::getExprVarIds(const Token* expr, bool* localOut, bool* unknownVarIdOut) const diff --git a/lib/astutils.h b/lib/astutils.h index 387badbd7..396b773f3 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -230,6 +230,8 @@ std::vector getLHSVariables(const Token* tok); bool isScopeBracket(const Token* tok); bool isNullOperand(const Token *expr); + +bool isGlobalData(const Token *expr, bool cpp); /** * Forward data flow analysis for checks * - unused value diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index e3c300ae4..44616403c 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -1573,7 +1573,7 @@ static void execute(const Token *start, const Token *end, Data &data) if (Token::Match(tok2, "%assign%")) { if (Token::Match(tok2->astOperand1(), ". %name% =") && tok2->astOperand1()->astOperand1() && tok2->astOperand1()->astOperand1()->valueType()) { const Token *structToken = tok2->astOperand1()->astOperand1(); - if (!structToken || !structToken->valueType() || !structToken->varId()) + if (!structToken->valueType() || !structToken->varId()) throw VerifyException(tok2, "Unhandled assignment in loop"); const Scope *structScope = structToken->valueType()->typeScope; if (!structScope) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 2d6d77441..032a99eb7 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -293,7 +293,7 @@ struct ForwardTraversal { std::tie(checkThen, checkElse) = evalCond(condTok); ForwardAnalyzer::Action thenAction = ForwardAnalyzer::Action::None; ForwardAnalyzer::Action elseAction = ForwardAnalyzer::Action::None; - bool hasElse = false; + bool hasElse = Token::simpleMatch(endBlock, "} else {"); bool bail = false; // Traverse then block @@ -308,8 +308,7 @@ struct ForwardTraversal { bail = true; } // Traverse else block - if (Token::simpleMatch(endBlock, "} else {")) { - hasElse = true; + if (hasElse) { returnElse = isEscapeScope(endBlock->linkAt(2), true); if (checkElse) { Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2)); @@ -346,6 +345,8 @@ struct ForwardTraversal { if (!analyzer->lowerToInconclusive()) return Progress::Break; } else if (thenAction.isModified() || elseAction.isModified()) { + if (!hasElse && analyzer->isConditional()) + return Progress::Break; if (!analyzer->lowerToPossible()) return Progress::Break; analyzer->assume(condTok, elseAction.isModified()); diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index aeedea4f0..98d1fc228 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -246,6 +246,28 @@ static ProgramMemory getInitialProgramState(const Token* tok, return pm; } +ProgramMemory getProgramMemory(const Token *tok, const std::unordered_map& vars) +{ + ProgramMemory programMemory; + for(const auto& p:vars) { + const ValueFlow::Value &value = p.second; + programMemory.replace(getInitialProgramState(tok, value.tokvalue)); + programMemory.replace(getInitialProgramState(tok, value.condition)); + } + fillProgramMemoryFromConditions(programMemory, tok, nullptr); + ProgramMemory state; + for(const auto& p:vars) { + nonneg int varid = p.first; + const ValueFlow::Value &value = p.second; + programMemory.setValue(varid, value); + if (value.varId) + programMemory.setIntValue(value.varId, value.varvalue); + state = programMemory; + } + fillProgramMemoryFromAssignments(programMemory, tok, state, vars); + return programMemory; +} + ProgramMemory getProgramMemory(const Token *tok, nonneg int varid, const ValueFlow::Value &value) { ProgramMemory programMemory; diff --git a/lib/programmemory.h b/lib/programmemory.h index 0c079af79..928431853 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -6,6 +6,7 @@ #include "valueflow.h" #include "mathlib.h" #include +#include struct ProgramMemory { std::map values; @@ -55,6 +56,8 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory) */ ProgramMemory getProgramMemory(const Token *tok, nonneg int varid, const ValueFlow::Value &value); +ProgramMemory getProgramMemory(const Token *tok, const std::unordered_map& vars); + #endif diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 14f825ae1..0ac322125 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1184,14 +1184,14 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers() if (membertok) { const Variable *var = tok->variable(); - if (var && var->typeScope()) { + if (var->typeScope()) { const Variable *membervar = var->typeScope()->getVariable(membertok->str()); if (membervar) { membertok->variable(membervar); if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr) fixVarId(varIds, tok, const_cast(membertok), membervar); } - } else if (const ::Type *type = var ? var->smartPointerType() : nullptr) { + } else if (const ::Type *type = var->smartPointerType()) { const Scope *classScope = type->classScope; const Variable *membervar = classScope ? classScope->getVariable(membertok->str()) : nullptr; if (membervar) { @@ -1199,7 +1199,7 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers() if (membertok->varId() == 0 || mVariableList[membertok->varId()] == nullptr) fixVarId(varIds, tok, const_cast(membertok), membervar); } - } else if (var && tok->valueType() && tok->valueType()->type == ValueType::CONTAINER) { + } else if (tok->valueType() && tok->valueType()->type == ValueType::CONTAINER) { if (Token::Match(var->typeStartToken(), "std :: %type% < %type% *| *| >")) { const Type * type = var->typeStartToken()->tokAt(4)->type(); if (type && type->classScope && type->classScope->definedType) { diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 391459eb4..6c2101628 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -1067,7 +1067,7 @@ void Tokenizer::simplifyTypedef() const Token *func = temp->link()->previous(); if (temp->str() != ")") continue; - if (!func || !func->previous()) // Ticket #4239 + if (!func->previous()) // Ticket #4239 continue; /** @todo add support for multi-token operators */ diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp index b8ac02292..2a695d503 100644 --- a/lib/tokenlist.cpp +++ b/lib/tokenlist.cpp @@ -1226,7 +1226,7 @@ static bool isLambdaCaptureList(const Token * tok) if (!tok->astOperand1() || tok->astOperand1()->str() != "(") return false; const Token * params = tok->astOperand1(); - if (!params || !params->astOperand1() || params->astOperand1()->str() != "{") + if (!params->astOperand1() || params->astOperand1()->str() != "{") return false; return true; } @@ -1324,8 +1324,6 @@ static Token * createAstAtToken(Token *tok, bool cpp) while (tok2 && tok2 != endPar && tok2->str() != ";") { if (tok2->str() == "<" && tok2->link()) { tok2 = tok2->link(); - if (!tok2) - break; } else if (Token::Match(tok2, "%name% %op%|(|[|.|:|::") || Token::Match(tok2->previous(), "[(;{}] %cop%|(")) { init1 = tok2; AST_state state1(cpp); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index bb46b5a37..ab60d67f0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2081,7 +2081,7 @@ static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, return false; if (isAliasOf(tok, varid)) return true; - if (!var->isPointer()) + if (var && !var->isPointer()) return false; // Search through non value aliases for (const ValueFlow::Value &val : values) { @@ -2101,31 +2101,6 @@ static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, return false; } -static void valueFlowForwardExpression(Token* startToken, - const Token* endToken, - const Token* exprTok, - const std::list& values, - const TokenList* const tokenlist, - const Settings* settings) -{ - FwdAnalysis fwdAnalysis(tokenlist->isCPP(), settings->library); - for (const FwdAnalysis::KnownAndToken read : fwdAnalysis.valueFlow(exprTok, startToken, endToken)) { - for (const ValueFlow::Value& value : values) { - // Don't set inconclusive values - if (value.isInconclusive()) - continue; - ValueFlow::Value v = value; - if (v.isImpossible()) { - if (read.known) - continue; - } else if (!read.known) { - v.valueKind = ValueFlow::Value::ValueKind::Possible; - } - setTokenValue(const_cast(read.token), v, settings); - } - } -} - static const ValueFlow::Value* getKnownValue(const Token* tok, ValueFlow::Value::ValueType type) { if (!tok) @@ -2169,24 +2144,86 @@ static bool bifurcate(const Token* tok, const std::set& varids, cons return false; } -struct VariableForwardAnalyzer : ForwardAnalyzer { - Token* start; - const Variable* var; - nonneg int varid; - ValueFlow::Value value; - const Settings* settings; +struct SelectMapKeys +{ + template + typename Pair::first_type operator()(const Pair& p) const + { + return p.first; + } +}; - VariableForwardAnalyzer() = default; +struct ValueFlowForwardAnalyzer : ForwardAnalyzer { + const TokenList* tokenlist; - bool isWritableValue() const { - return value.isIntValue() || value.isFloatValue(); + ValueFlowForwardAnalyzer() + : tokenlist(nullptr) + {} + + ValueFlowForwardAnalyzer(const TokenList* t) + : tokenlist(t) + {} + + virtual int getIndirect() const = 0; + + virtual bool match(const Token* tok) const = 0; + + virtual bool isAlias(const Token* tok) const = 0; + + using ProgramState = std::unordered_map; + + virtual ProgramState getProgramState() const = 0; + + virtual bool isWritableValue() const { + return false; + } + + virtual bool isGlobal() const { + return false; + } + + virtual bool invalid() const { + return false; + } + + bool isCPP() const { + return tokenlist->isCPP(); + } + + const Settings* getSettings() const { + return tokenlist->getSettings(); + } + + virtual Action isModified(const Token* tok) const { + Action read = Action::Read; + bool inconclusive = false; + if (isVariableChangedByFunctionCall(tok, getIndirect(), getSettings(), &inconclusive)) + return read | Action::Invalid; + if (inconclusive) + return read | Action::Inconclusive; + if (isVariableChanged(tok, getIndirect(), getSettings(), isCPP())) { + if (Token::Match(tok->astParent(), "*|[|.|++|--")) + return read | Action::Invalid; + return Action::Invalid; + } + return read; + } + + virtual Action isAliasModified(const Token* tok) const { + int indirect = 0; + if (tok->valueType()) + indirect = tok->valueType()->pointer; + if (isVariableChanged(tok, indirect, getSettings(), isCPP())) + return Action::Invalid; + return Action::None; } virtual Action analyze(const Token* tok) const OVERRIDE { - bool cpp = true; - if (tok->varId() == varid) { + if (invalid()) + return Action::Invalid; + if (match(tok)) { const Token* parent = tok->astParent(); - if ((Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && value.indirect <= 0) + if ((Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && getIndirect() <= 0) return Action::Read; Action read = Action::Read; @@ -2205,74 +2242,80 @@ struct VariableForwardAnalyzer : ForwardAnalyzer { } // increment/decrement - if (value.valueType == ValueFlow::Value::ValueType::INT && (Token::Match(tok->previous(), "++|-- %name%") || Token::Match(tok, "%name% ++|--"))) { + if (isWritableValue() && (Token::Match(tok->previous(), "++|-- %name%") || Token::Match(tok, "%name% ++|--"))) { return read | Action::Write; } // Check for modifications by function calls - bool inconclusive = false; - if (isVariableChangedByFunctionCall(tok, value.indirect, settings, &inconclusive)) - return read | Action::Invalid; - if (inconclusive) - return read | Action::Inconclusive; - if (isVariableChanged(tok, value.indirect, settings, cpp)) { - if (Token::Match(parent, "*|[|.|++|--")) - return read | Action::Invalid; - return Action::Invalid; - } - return read; - } else if (!value.isLifetimeValue() && isAliasOf(var, tok, varid, {value})) { - int indirect = 0; - if (tok->valueType()) - indirect = tok->valueType()->pointer; - if (isVariableChanged(tok, indirect, settings, cpp)) - return Action::Invalid; + return isModified(tok); + } else if (isAlias(tok)) { + return isAliasModified(tok); } else if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { // bailout: global non-const variables - if (var->isGlobal() && !var->isConst()) { + if (isGlobal()) { return Action::Invalid; } } return Action::None; } - virtual void update(Token* tok, Action a) OVERRIDE { - if (a.isRead()) - setTokenValue(tok, value, settings); - if (a.isInconclusive()) - lowerToInconclusive(); - if (a.isWrite()) { - if (Token::Match(tok->astParent(), "%assign%")) { - // TODO: Check result - if (evalAssignment(value, - tok->astParent()->str(), - *getKnownValue(tok->astParent()->astOperand2(), ValueFlow::Value::ValueType::INT))) { - const std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " + - value.infoString()); - value.errorPath.emplace_back(tok, info); - } else { - // TODO: Dont set to zero - value.intvalue = 0; - } - } - if (Token::Match(tok->astParent(), "++|--")) { - const bool inc = Token::simpleMatch(tok->astParent(), "++"); - value.intvalue += (inc ? 1 : -1); - const std::string info(tok->str() + " is " + std::string(inc ? "incremented" : "decremented") + - "', new value is " + value.infoString()); - value.errorPath.emplace_back(tok, info); - } - } - } + virtual std::vector evaluate(const Token* tok) const OVERRIDE { if (tok->hasKnownIntValue()) return {static_cast(tok->values().front().intvalue)}; std::vector result; - ProgramMemory programMemory = getProgramMemory(tok, varid, value); + ProgramState ps = getProgramState(); + ProgramMemory programMemory = getProgramMemory(tok, ps); if (conditionIsTrue(tok, programMemory)) result.push_back(1); if (conditionIsFalse(tok, programMemory)) result.push_back(0); return result; } +}; + +struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { + ValueFlow::Value value; + + SingleValueFlowForwardAnalyzer() + : ValueFlowForwardAnalyzer() + {} + + SingleValueFlowForwardAnalyzer(const ValueFlow::Value& v, const TokenList* t) + : ValueFlowForwardAnalyzer(t), value(v) + {} + + virtual const std::unordered_map& getVars() const = 0; + + virtual int getIndirect() const OVERRIDE { + return value.indirect; + } + + virtual bool isAlias(const Token* tok) const OVERRIDE { + if (value.isLifetimeValue()) + return false; + for(const auto& p:getVars()) { + nonneg int varid = p.first; + const Variable* var = p.second; + if (tok->varId() == varid) + return true; + if (isAliasOf(var, tok, varid, {value})) + return true; + } + return false; + } + + virtual bool isWritableValue() const OVERRIDE { + return value.isIntValue() || value.isFloatValue(); + } + + virtual bool isGlobal() const OVERRIDE { + for(const auto&p:getVars()) { + const Variable* var = p.second; + if (var->isGlobal() && !var->isConst()) + return true; + } + return false; + } + virtual bool lowerToPossible() OVERRIDE { if (value.isImpossible()) return false; @@ -2299,6 +2342,35 @@ struct VariableForwardAnalyzer : ForwardAnalyzer { return false; } + virtual void update(Token* tok, Action a) OVERRIDE { + if (a.isRead()) + setTokenValue(tok, value, getSettings()); + if (a.isInconclusive()) + lowerToInconclusive(); + if (a.isWrite()) { + if (Token::Match(tok->astParent(), "%assign%")) { + // TODO: Check result + if (evalAssignment(value, + tok->astParent()->str(), + *getKnownValue(tok->astParent()->astOperand2(), ValueFlow::Value::ValueType::INT))) { + const std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " + + value.infoString()); + value.errorPath.emplace_back(tok, info); + } else { + // TODO: Dont set to zero + value.intvalue = 0; + } + } + if (Token::Match(tok->astParent(), "++|--")) { + const bool inc = Token::simpleMatch(tok->astParent(), "++"); + value.intvalue += (inc ? 1 : -1); + const std::string info(tok->str() + " is " + std::string(inc ? "incremented" : "decremented") + + "', new value is " + value.infoString()); + value.errorPath.emplace_back(tok, info); + } + } + } + virtual bool updateScope(const Token* endBlock, bool) const OVERRIDE { const Scope* scope = endBlock->scope(); if (!scope) @@ -2314,36 +2386,145 @@ struct VariableForwardAnalyzer : ForwardAnalyzer { if (isConditional()) return false; const Token* condTok = getCondTokFromEnd(endBlock); - return bifurcate(condTok, {varid}, settings); + std::set varids; + std::transform(getVars().begin(), getVars().end(), std::inserter(varids, varids.begin()), SelectMapKeys{}); + return bifurcate(condTok, varids, getSettings()); } return false; } }; +struct VariableForwardAnalyzer : SingleValueFlowForwardAnalyzer { + const Variable* var; + std::unordered_map varids; + + VariableForwardAnalyzer() + : SingleValueFlowForwardAnalyzer(), var(nullptr), varids() + {} + + VariableForwardAnalyzer(const Variable* v, const ValueFlow::Value& val, const TokenList* t) + : SingleValueFlowForwardAnalyzer(val, t), var(v), varids() { + varids[var->declarationId()] = var; + } + + virtual const std::unordered_map& getVars() const OVERRIDE { + return varids; + } + + virtual bool match(const Token* tok) const OVERRIDE { + return tok->varId() == var->declarationId(); + } + + virtual ProgramState getProgramState() const OVERRIDE { + ProgramState ps; + ps[var->declarationId()] = value; + return ps; + } +}; + static bool valueFlowForwardVariable(Token* const startToken, const Token* const endToken, const Variable* const var, - const nonneg int varid, + const nonneg int, std::list values, const bool, const bool, - TokenList* const, + TokenList* const tokenlist, ErrorLogger* const, const Settings* const settings) { - VariableForwardAnalyzer a; - a.start = startToken; - a.var = var; - a.varid = varid; - a.settings = settings; for (ValueFlow::Value& v : values) { - a.value = v; + VariableForwardAnalyzer a(var, v, tokenlist); valueFlowGenericForward(startToken, endToken, a, settings); } return true; } +struct ExpressionForwardAnalyzer : SingleValueFlowForwardAnalyzer { + const Token* expr; + std::unordered_map varids; + bool local; + bool unknown; + + ExpressionForwardAnalyzer() + : SingleValueFlowForwardAnalyzer(), expr(nullptr), varids(), local(true), unknown(false) + {} + + ExpressionForwardAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t) + : SingleValueFlowForwardAnalyzer(val, t), expr(e), varids(), local(true), unknown(false) { + + setupExprVarIds(); + } + + static bool nonLocal(const Variable* var, bool deref) + { + return !var || (!var->isLocal() && !var->isArgument()) || (deref && var->isArgument() && var->isPointer()) || var->isStatic() || var->isReference() || var->isExtern(); + } + + void setupExprVarIds() + { + visitAstNodes(expr, + [&](const Token *tok) { + if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") { + // unknown variable + unknown = true; + return ChildrenToVisit::none; + } + if (tok->varId() > 0) { + varids[tok->varId()] = tok->variable(); + if (!Token::simpleMatch(tok->previous(), ".")) { + const Variable *var = tok->variable(); + if (var && var->isReference() && var->isLocal() && Token::Match(var->nameToken(), "%var% [=(]") && !isGlobalData(var->nameToken()->next()->astOperand2(), isCPP())) + return ChildrenToVisit::none; + const bool deref = tok->astParent() && (tok->astParent()->isUnaryOp("*") || (tok->astParent()->str() == "[" && tok == tok->astParent()->astOperand1())); + local &= !nonLocal(tok->variable(), deref); + } + } + return ChildrenToVisit::op1_and_op2; + }); + } + + virtual bool invalid() const OVERRIDE { + return unknown; + } + + virtual std::vector evaluate(const Token* tok) const OVERRIDE { + if (tok->hasKnownIntValue()) + return {static_cast(tok->values().front().intvalue)}; + return std::vector{}; + } + + virtual const std::unordered_map& getVars() const OVERRIDE { + return varids; + } + + virtual bool match(const Token* tok) const OVERRIDE { + return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true); + } + + virtual ProgramState getProgramState() const OVERRIDE { + return ProgramState{}; + } + + virtual bool isGlobal() const OVERRIDE { + return !local; + } +}; + +static void valueFlowForwardExpression(Token* startToken, + const Token* endToken, + const Token* exprTok, + const std::list& values, + const TokenList* const tokenlist, + const Settings* settings) +{ + for (const ValueFlow::Value& v : values) { + ExpressionForwardAnalyzer a(exprTok, v, tokenlist); + valueFlowGenericForward(startToken, endToken, a, settings); + } +} + static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known) { if (!expr) @@ -5269,6 +5450,9 @@ static void valueFlowFwdAnalysis(const TokenList *tokenlist, const Settings *set tok = tok->linkAt(1); if (tok->str() != "=" || !tok->astOperand1() || !tok->astOperand2()) continue; + // Skip variables + if (tok->astOperand1()->variable()) + continue; if (!tok->scope()->isExecutable()) continue; if (!tok->astOperand2()->hasKnownIntValue()) diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 44ca71a11..abf62dca0 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -91,6 +91,7 @@ private: TEST_CASE(nullpointer49); // #7804 TEST_CASE(nullpointer50); // #6462 TEST_CASE(nullpointer51); + TEST_CASE(nullpointer52); TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -1707,6 +1708,46 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer52() { + check("int f(int a, int* b) {\n" + " int* c = nullptr;\n" + " if(b) c = b;\n" + " if (!c) c = &a;\n" + " return *c;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(int a, int* b) {\n" + " int* c = nullptr;\n" + " if(b) c = b;\n" + " bool d = !c;\n" + " if (d) c = &a;\n" + " return *c;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int* x; };\n" + "int f(int a, int* b) {\n" + " A c;\n" + " c.x = nullptr;\n" + " if(b) c.x = b;\n" + " if (!c.x) c.x = &a;\n" + " return *c.x;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int* x; };\n" + "int f(int a, int* b) {\n" + " A c;\n" + " c.x = nullptr;\n" + " if(b) c.x = b;\n" + " bool d = !c.x;\n" + " if (!d) c.x = &a;\n" + " return *c.x;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index fb9c7367b..05f47cf77 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4346,7 +4346,7 @@ private: " if (b == 'x') {}\n" " if (a) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:8]: (error) Uninitialized variable: a\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Uninitialized variable: a\n", "", errout.str()); valueFlowUninit("void h() {\n" " int i;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index afac6ecd6..6ff89e20f 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1665,7 +1665,7 @@ private: " if (condition2) x = 789;\n" " a = 2 + x;\n" // <- either assignment "x=123" is redundant or x can be 123 here. "}"; - ASSERT_EQUALS(true, testValueOfX(code, 5U, 123)); + TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 5U, 123)); code = "void f(int a) {\n" " int x = 123;\n" @@ -2717,7 +2717,9 @@ private: " s.x++;\n" "}"; values = tokenValues(code, "<"); - ASSERT_EQUALS(true, values.empty()); + ASSERT_EQUALS(1, values.size()); + ASSERT(values.front().isPossible()); + ASSERT_EQUALS(true, values.front().intvalue); code = "void f() {\n" " S s;\n" @@ -2755,7 +2757,9 @@ private: " }\n" "}"; values = tokenValues(code, ">"); - ASSERT_EQUALS(true, values.empty()); + ASSERT_EQUALS(1, values.size()); + ASSERT(values.front().isPossible()); + ASSERT_EQUALS(true, values.front().intvalue); code = "void foo() {\n" " struct ISO_PVD_s pvd;\n" @@ -3647,7 +3651,11 @@ private: " int x;\n" " f(x=3), return x+3;\n" "}"; - ASSERT_EQUALS(0U, tokenValues(code, "x +").size()); + values = tokenValues(code, "x +"); + ASSERT_EQUALS(true, values.empty()); + // ASSERT_EQUALS(1U, values.size()); + // ASSERT(values.front().isIntValue()); + // ASSERT_EQUALS(3, values.front().intvalue); // #8195 code = "void foo(std::istream &is) {\n" @@ -3677,8 +3685,7 @@ private: " x = 1;\n" " if (x>1) {}\n" "}"; - values = tokenValues(code, "x >"); - ASSERT_EQUALS(true, values.size() == 1U && values.front().isIntValue()); + ASSERT_EQUALS(true, testValueOfXKnown(code, 8U, 1)); // #8348 - noreturn else code = "int test_input_int(int a, int b) {\n"