From 8b26ecbcdd349a659998fec8080abd0ebdec3013 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 21 Jan 2021 12:49:37 -0600 Subject: [PATCH] Extend ProgramMemory to handle expressions (#3069) --- lib/programmemory.cpp | 160 +++++++++++++++++++-------------------- lib/programmemory.h | 22 +++--- lib/symboldatabase.h | 2 + lib/valueflow.cpp | 17 ++--- test/testnullpointer.cpp | 13 +++- 5 files changed, 111 insertions(+), 103 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index e15428e74..7cb974aa6 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -1,21 +1,19 @@ #include "programmemory.h" -#include "mathlib.h" -#include "token.h" #include "astutils.h" +#include "mathlib.h" #include "symboldatabase.h" +#include "token.h" #include #include +#include #include #include -void ProgramMemory::setValue(nonneg int varid, const ValueFlow::Value &value) +void ProgramMemory::setValue(MathLib::bigint exprid, const ValueFlow::Value& value) { values[exprid] = value; } +const ValueFlow::Value* ProgramMemory::getValue(MathLib::bigint exprid) const { - values[varid] = value; -} -const ValueFlow::Value* ProgramMemory::getValue(nonneg int varid) const -{ - const ProgramMemory::Map::const_iterator it = values.find(varid); + const ProgramMemory::Map::const_iterator it = values.find(exprid); const bool found = it != values.end() && !it->second.isImpossible(); if (found) return &it->second; @@ -23,9 +21,9 @@ const ValueFlow::Value* ProgramMemory::getValue(nonneg int varid) const return nullptr; } -bool ProgramMemory::getIntValue(nonneg int varid, MathLib::bigint* result) const +bool ProgramMemory::getIntValue(MathLib::bigint exprid, MathLib::bigint* result) const { - const ValueFlow::Value* value = getValue(varid); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isIntValue()) { *result = value->intvalue; return true; @@ -33,14 +31,14 @@ bool ProgramMemory::getIntValue(nonneg int varid, MathLib::bigint* result) const return false; } -void ProgramMemory::setIntValue(nonneg int varid, MathLib::bigint value) +void ProgramMemory::setIntValue(MathLib::bigint exprid, MathLib::bigint value) { - values[varid] = ValueFlow::Value(value); + values[exprid] = ValueFlow::Value(value); } -bool ProgramMemory::getTokValue(nonneg int varid, const Token** result) const +bool ProgramMemory::getTokValue(MathLib::bigint exprid, const Token** result) const { - const ValueFlow::Value* value = getValue(varid); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isTokValue()) { *result = value->tokvalue; return true; @@ -48,9 +46,9 @@ bool ProgramMemory::getTokValue(nonneg int varid, const Token** result) const return false; } -bool ProgramMemory::getContainerSizeValue(nonneg int varid, MathLib::bigint* result) const +bool ProgramMemory::getContainerSizeValue(MathLib::bigint exprid, MathLib::bigint* result) const { - const ValueFlow::Value* value = getValue(varid); + const ValueFlow::Value* value = getValue(exprid); if (value && value->isContainerSizeValue()) { *result = value->intvalue; return true; @@ -58,15 +56,12 @@ bool ProgramMemory::getContainerSizeValue(nonneg int varid, MathLib::bigint* res return false; } -void ProgramMemory::setUnknown(nonneg int varid) +void ProgramMemory::setUnknown(MathLib::bigint exprid) { - values[varid].valueType = ValueFlow::Value::ValueType::UNINIT; + values[exprid].valueType = ValueFlow::Value::ValueType::UNINIT; } -bool ProgramMemory::hasValue(nonneg int varid) -{ - return values.find(varid) != values.end(); -} +bool ProgramMemory::hasValue(MathLib::bigint exprid) { return values.find(exprid) != values.end(); } void ProgramMemory::swap(ProgramMemory &pm) { @@ -85,8 +80,9 @@ bool ProgramMemory::empty() const void ProgramMemory::replace(const ProgramMemory &pm) { - for (auto&& p:pm.values) + for (auto&& p : pm.values) { values[p.first] = p.second; + } } void ProgramMemory::insert(const ProgramMemory &pm) @@ -146,21 +142,13 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke }); if (!vartok) return; - if (vartok->varId() == 0) + if (vartok->exprId() == 0) return; if (!truevalue.isIntValue()) return; - if (endTok && isVariableChanged(tok->next(), endTok, vartok->varId(), false, settings, true)) + if (endTok && isExpressionChanged(vartok, tok->next(), endTok, settings, true)) return; - pm.setIntValue(vartok->varId(), then ? truevalue.intvalue : falsevalue.intvalue); - } else if (Token::Match(tok, "%var%")) { - if (tok->varId() == 0) - return; - if (then && !astIsPointer(tok) && !astIsBool(tok)) - return; - if (endTok && isVariableChanged(tok->next(), endTok, tok->varId(), false, settings, true)) - return; - pm.setIntValue(tok->varId(), then); + pm.setIntValue(vartok->exprId(), then ? truevalue.intvalue : falsevalue.intvalue); } else if (Token::simpleMatch(tok, "!")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, !then); } else if (then && Token::simpleMatch(tok, "&&")) { @@ -169,6 +157,12 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke } else if (!then && Token::simpleMatch(tok, "||")) { programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then); programMemoryParseCondition(pm, tok->astOperand2(), endTok, settings, then); + } else if (tok->exprId() > 0) { + if (then && !astIsPointer(tok) && !astIsBool(tok)) + return; + if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true)) + return; + pm.setIntValue(tok->exprId(), then); } } @@ -197,36 +191,33 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok { int indentlevel = 0; for (const Token *tok2 = tok; tok2; tok2 = tok2->previous()) { - bool setvar = false; - if (Token::Match(tok2, "[;{}] %var% = %var% ;")) { + if ((Token::simpleMatch(tok2, "=") || Token::Match(tok2->previous(), "%var% (|{")) && tok2->astOperand1() && + tok2->astOperand2()) { + bool setvar = false; + const Token* vartok = tok2->astOperand1(); + const Token* valuetok = tok2->astOperand2(); for (const auto& p:vars) { - if (p.first != tok2->next()->varId()) + if (p.first != vartok->exprId()) continue; - const Token *vartok = tok2->tokAt(3); if (vartok == tok) continue; - pm.setValue(vartok->varId(), p.second); + pm.setValue(vartok->exprId(), p.second); setvar = true; } - } - if (!setvar && (Token::Match(tok2, ";|{|}|%type% %var% =") || Token::Match(tok2, "[;{}] const| %type% %var% (") || - Token::Match(tok2->previous(), "for ( %var% ="))) { - const Token *vartok = tok2->next(); - while (vartok->next()->isName()) - vartok = vartok->next(); - if (!pm.hasValue(vartok->varId())) { - MathLib::bigint result = 0; - bool error = false; - execute(vartok->next()->astOperand2(), &pm, &result, &error); - if (!error) - pm.setIntValue(vartok->varId(), result); - else - pm.setUnknown(vartok->varId()); + if (!setvar) { + if (!pm.hasValue(vartok->exprId())) { + MathLib::bigint result = 0; + bool error = false; + execute(valuetok, &pm, &result, &error); + if (!error) + pm.setIntValue(vartok->exprId(), result); + else + pm.setUnknown(vartok->exprId()); + } } - } else if (!setvar && Token::Match(tok2, "%var% !!=") && isVariableChanged(tok2, 0, nullptr, true)) { - const Token *vartok = tok2; - if (!pm.hasValue(vartok->varId())) - pm.setUnknown(vartok->varId()); + } else if (tok2->exprId() > 0 && Token::Match(tok2, ".|(|[|*|%var%") && !pm.hasValue(tok2->exprId()) && + isVariableChanged(tok2, 0, nullptr, true)) { + pm.setUnknown(tok2->exprId()); } if (tok2->str() == "{") { @@ -291,12 +282,12 @@ void ProgramMemoryState::replace(const ProgramMemory &pm, const Token* origin) void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& vars) { - ProgramMemory pm; + ProgramMemory pm = state; fillProgramMemoryFromConditions(pm, tok, nullptr); for (const auto& p:vars) { - nonneg int varid = p.first; + MathLib::bigint exprid = p.first; const ValueFlow::Value &value = p.second; - pm.setValue(varid, value); + pm.setValue(exprid, value); if (value.varId) pm.setIntValue(value.varId, value.varvalue); } @@ -343,9 +334,9 @@ ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars) fillProgramMemoryFromConditions(programMemory, tok, nullptr); ProgramMemory state; for (const auto& p:vars) { - nonneg int varid = p.first; + MathLib::bigint exprid = p.first; const ValueFlow::Value &value = p.second; - programMemory.setValue(varid, value); + programMemory.setValue(exprid, value); if (value.varId) programMemory.setIntValue(value.varId, value.varvalue); } @@ -354,17 +345,17 @@ ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars) return programMemory; } -ProgramMemory getProgramMemory(const Token *tok, nonneg int varid, const ValueFlow::Value &value) +ProgramMemory getProgramMemory(const Token* tok, MathLib::bigint exprid, const ValueFlow::Value& value) { ProgramMemory programMemory; programMemory.replace(getInitialProgramState(tok, value.tokvalue)); programMemory.replace(getInitialProgramState(tok, value.condition)); fillProgramMemoryFromConditions(programMemory, tok, nullptr); - programMemory.setValue(varid, value); + programMemory.setValue(exprid, value); if (value.varId) programMemory.setIntValue(value.varId, value.varvalue); const ProgramMemory state = programMemory; - fillProgramMemoryFromAssignments(programMemory, tok, state, {{varid, value}}); + fillProgramMemoryFromAssignments(programMemory, tok, state, {{exprid, value}}); return programMemory; } @@ -391,6 +382,11 @@ void execute(const Token *expr, *error = true; } + else if (expr->exprId() > 0 && programMemory->hasValue(expr->exprId())) { + if (!programMemory->getIntValue(expr->exprId(), result)) + *error = true; + } + else if (expr->isComparisonOp()) { MathLib::bigint result1(0), result2(0); execute(expr->astOperand1(), programMemory, &result1, error); @@ -411,45 +407,45 @@ void execute(const Token *expr, else if (expr->isAssignmentOp()) { execute(expr->astOperand2(), programMemory, result, error); - if (!expr->astOperand1() || !expr->astOperand1()->varId()) + if (!expr->astOperand1() || !expr->astOperand1()->exprId()) *error = true; if (*error) return; if (expr->str() == "=") { - programMemory->setIntValue(expr->astOperand1()->varId(), *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), *result); return; } long long intValue; - if (!programMemory->getIntValue(expr->astOperand1()->varId(), &intValue)) { + if (!programMemory->getIntValue(expr->astOperand1()->exprId(), &intValue)) { *error = true; return; } if (expr->str() == "+=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue + *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue + *result); else if (expr->str() == "-=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue - *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue - *result); else if (expr->str() == "*=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue * *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue * *result); else if (expr->str() == "/=" && *result != 0) - programMemory->setIntValue(expr->astOperand1()->varId(), intValue / *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue / *result); else if (expr->str() == "%=" && *result != 0) - programMemory->setIntValue(expr->astOperand1()->varId(), intValue % *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue % *result); else if (expr->str() == "&=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue & *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue & *result); else if (expr->str() == "|=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue | *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue | *result); else if (expr->str() == "^=") - programMemory->setIntValue(expr->astOperand1()->varId(), intValue ^ *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), intValue ^ *result); } else if (Token::Match(expr, "++|--")) { - if (!expr->astOperand1() || expr->astOperand1()->varId() == 0U) + if (!expr->astOperand1() || expr->astOperand1()->exprId() == 0U) *error = true; else { long long intValue; - if (!programMemory->getIntValue(expr->astOperand1()->varId(), &intValue)) + if (!programMemory->getIntValue(expr->astOperand1()->exprId(), &intValue)) *error = true; else { if (intValue == 0 && @@ -458,7 +454,7 @@ void execute(const Token *expr, expr->astOperand1()->variable()->isUnsigned()) *error = true; // overflow *result = intValue + (expr->str() == "++" ? 1 : -1); - programMemory->setIntValue(expr->astOperand1()->varId(), *result); + programMemory->setIntValue(expr->astOperand1()->exprId(), *result); } } } @@ -530,7 +526,7 @@ void execute(const Token *expr, else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) { const Token *tokvalue = nullptr; - if (!programMemory->getTokValue(expr->astOperand1()->varId(), &tokvalue)) { + if (!programMemory->getTokValue(expr->astOperand1()->exprId(), &tokvalue)) { auto tokvalue_it = std::find_if(expr->astOperand1()->values().begin(), expr->astOperand1()->values().end(), std::mem_fn(&ValueFlow::Value::isTokValue)); @@ -558,11 +554,11 @@ void execute(const Token *expr, if (astIsContainer(containerTok)) { Library::Container::Yield yield = containerTok->valueType()->container->getYield(expr->strAt(-1)); if (yield == Library::Container::Yield::SIZE) { - if (!programMemory->getContainerSizeValue(containerTok->varId(), result)) + if (!programMemory->getContainerSizeValue(containerTok->exprId(), result)) *error = true; } else if (yield == Library::Container::Yield::EMPTY) { MathLib::bigint size = 0; - if (!programMemory->getContainerSizeValue(containerTok->varId(), &size)) + if (!programMemory->getContainerSizeValue(containerTok->exprId(), &size)) *error = true; *result = (size == 0); } else { diff --git a/lib/programmemory.h b/lib/programmemory.h index 6982fbc5e..93a5587cd 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -11,21 +11,21 @@ class Token; struct ProgramMemory { - using Map = std::unordered_map; + using Map = std::unordered_map; Map values; - void setValue(nonneg int varid, const ValueFlow::Value &value); - const ValueFlow::Value* getValue(nonneg int varid) const; + void setValue(MathLib::bigint exprid, const ValueFlow::Value& value); + const ValueFlow::Value* getValue(MathLib::bigint exprid) const; - bool getIntValue(nonneg int varid, MathLib::bigint* result) const; - void setIntValue(nonneg int varid, MathLib::bigint value); + bool getIntValue(MathLib::bigint exprid, MathLib::bigint* result) const; + void setIntValue(MathLib::bigint exprid, MathLib::bigint value); - bool getContainerSizeValue(nonneg int varid, MathLib::bigint* result) const; + bool getContainerSizeValue(MathLib::bigint exprid, MathLib::bigint* result) const; - void setUnknown(nonneg int varid); + void setUnknown(MathLib::bigint exprid); - bool getTokValue(nonneg int varid, const Token** result) const; - bool hasValue(nonneg int varid); + bool getTokValue(MathLib::bigint exprid, const Token** result) const; + bool hasValue(MathLib::bigint exprid); void swap(ProgramMemory &pm); @@ -42,7 +42,7 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke struct ProgramMemoryState { ProgramMemory state; - std::map origins; + std::map origins; void insert(const ProgramMemory &pm, const Token* origin = nullptr); void replace(const ProgramMemory &pm, const Token* origin = nullptr); @@ -79,7 +79,7 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory) /** * Get program memory by looking backwards from given token. */ -ProgramMemory getProgramMemory(const Token *tok, nonneg int varid, const ValueFlow::Value &value); +ProgramMemory getProgramMemory(const Token* tok, MathLib::bigint exprid, const ValueFlow::Value& value); ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars); diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index 604eed176..b2563b296 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -1310,6 +1310,8 @@ public: return const_cast(this->findScope(tok, const_cast(startScope))); } + bool isVarId(nonneg int varid) const { return varid < mVariableList.size(); } + const Variable *getVariableFromVarId(nonneg int varId) const { return mVariableList.at(varId); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2c195456b..14f465b7b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1802,7 +1802,7 @@ struct ValueFlowAnalyzer : Analyzer { virtual bool isAlias(const Token* tok, bool& inconclusive) const = 0; - using ProgramState = std::unordered_map; + using ProgramState = std::unordered_map; virtual ProgramState getProgramState() const = 0; @@ -2311,20 +2311,17 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer { 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 ProgramState getProgramState() const OVERRIDE + { + ProgramState ps; + ps[expr->exprId()] = value; + return ps; } 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; } @@ -5005,6 +5002,8 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { // ProgramMemory pm = pms.get(endBlock->link()->next(), getProgramState()); for (const auto& p:pm.values) { int varid = p.first; + if (!symboldatabase->isVarId(varid)) + continue; ValueFlow::Value value = p.second; if (vars.count(varid) != 0) continue; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 12cad0f8c..d7dbf9c01 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1797,10 +1797,21 @@ private: " c.x = nullptr;\n" " if(b) c.x = b;\n" " bool d = !c.x;\n" - " if (!d) c.x = &a;\n" + " if (d) 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("[test.cpp:8]: (warning) Possible null pointer dereference: c.x\n", errout.str()); } void nullpointer53() {