From ba8222de1cdfdcadedd97896e43a416958d64160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 20 Sep 2017 22:41:36 +0200 Subject: [PATCH] ValueFlow: Put 'inconclusive' state in the ValueKind. A value can't be both known and inconclusive. --- lib/checkbufferoverrun.cpp | 4 ++-- lib/checkfunctions.cpp | 2 +- lib/checknullpointer.cpp | 16 ++++++++-------- lib/checkother.cpp | 6 +++--- lib/checkstl.cpp | 2 +- lib/checktype.cpp | 8 ++++---- lib/checkuninitvar.cpp | 2 +- lib/settings.cpp | 2 +- lib/token.cpp | 22 ++++++++++++---------- lib/valueflow.cpp | 21 ++++++++------------- lib/valueflow.h | 19 +++++++++++++------ 11 files changed, 54 insertions(+), 50 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index e8d42f35c..e55a2f176 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -86,7 +86,7 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra bool inconclusive = false; const Token *condition = nullptr; for (std::size_t i = 0; i < index.size(); ++i) { - inconclusive |= index[i].inconclusive; + inconclusive |= index[i].isInconclusive(); if (condition == nullptr) condition = index[i].condition; } @@ -1805,7 +1805,7 @@ void CheckBufferOverrun::negativeIndexError(const Token *tok, const ValueFlow::V << ", otherwise there is negative array index " << index.intvalue << "."; else errmsg << "Array index " << index.intvalue << " is out of bounds."; - reportError(errorPath, index.errorSeverity() ? Severity::error : Severity::warning, "negativeIndex", errmsg.str(), CWE786, index.inconclusive); + reportError(errorPath, index.errorSeverity() ? Severity::error : Severity::warning, "negativeIndex", errmsg.str(), CWE786, index.isInconclusive()); } CheckBufferOverrun::ArrayInfo::ArrayInfo() diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index 0cec4c369..8b5ee03c4 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -149,7 +149,7 @@ void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string "invalidFunctionArg", errmsg.str(), CWE628, - invalidValue->inconclusive); + invalidValue->isInconclusive()); else reportError(tok, Severity::error, diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 24c2a2ca5..fa541d0b6 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -334,7 +334,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec() if (!value) continue; - if (!printInconclusive && value->inconclusive) + if (!printInconclusive && value->isInconclusive()) continue; // Is pointer used as function parameter? @@ -350,7 +350,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec() std::list varlist; parseFunctionCall(*ftok->previous(), varlist, &_settings->library); if (std::find(varlist.begin(), varlist.end(), tok) != varlist.end()) { - nullPointerError(tok, tok->str(), value, value->inconclusive); + nullPointerError(tok, tok->str(), value, value->isInconclusive()); } continue; } @@ -363,7 +363,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec() continue; } - nullPointerError(tok, tok->str(), value, value->inconclusive); + nullPointerError(tok, tok->str(), value, value->isInconclusive()); } } @@ -485,9 +485,9 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var const ErrorPath errorPath = getErrorPath(tok, value, "Null pointer dereference"); if (value->condition) { - reportError(errorPath, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE476, inconclusive || value->inconclusive); + reportError(errorPath, Severity::warning, "nullPointerRedundantCheck", errmsgcond, CWE476, inconclusive || value->isInconclusive()); } else if (value->defaultArg) { - reportError(errorPath, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE476, inconclusive || value->inconclusive); + reportError(errorPath, Severity::warning, "nullPointerDefaultArg", errmsgdefarg, CWE476, inconclusive || value->isInconclusive()); } else { std::string errmsg; errmsg = std::string(value->isKnown() ? "Null" : "Possible null") + " pointer dereference"; @@ -498,7 +498,7 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var value->isKnown() ? Severity::error : Severity::warning, "nullPointer", errmsg, - CWE476, inconclusive || value->inconclusive); + CWE476, inconclusive || value->isInconclusive()); } } @@ -518,7 +518,7 @@ void CheckNullPointer::arithmetic() const ValueFlow::Value *value = tok->astOperand1()->getValue(0); if (!value) continue; - if (!_settings->inconclusive && value->inconclusive) + if (!_settings->inconclusive && value->isInconclusive()) continue; if (value->condition && !_settings->isEnabled(Settings::WARNING)) continue; @@ -545,6 +545,6 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value (value && value->condition) ? "nullPointerArithmeticRedundantCheck" : "nullPointerArithmetic", errmsg, CWE682, // unknown - pointer overflow - value && value->inconclusive); + value && value->isInconclusive()); } diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c2770954e..92e24e760 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1678,7 +1678,7 @@ void CheckOther::zerodivError(const Token *tok, const ValueFlow::Value *value) reportError(errorPath, value->errorSeverity() ? Severity::error : Severity::warning, value->condition ? "zerodivcond" : "zerodiv", - errmsg.str(), CWE369, value->inconclusive); + errmsg.str(), CWE369, value->isInconclusive()); } //--------------------------------------------------------------------------- @@ -2644,7 +2644,7 @@ void CheckOther::checkAccessOfMovedVariable() const ValueFlow::Value * movedValue = tok->getMovedValue(); if (!movedValue || movedValue->moveKind == ValueFlow::Value::NonMovedVariable) continue; - if (movedValue->inconclusive && !reportInconclusive) + if (movedValue->isInconclusive() && !reportInconclusive) continue; bool inconclusive = false; @@ -2664,7 +2664,7 @@ void CheckOther::checkAccessOfMovedVariable() } } if (accessOfMoved || (inconclusive && reportInconclusive)) - accessMovedError(tok, tok->str(), movedValue->moveKind, inconclusive || movedValue->inconclusive); + accessMovedError(tok, tok->str(), movedValue->moveKind, inconclusive || movedValue->isInconclusive()); } } } diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 5abd58efe..924c4d450 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -474,7 +474,7 @@ void CheckStl::negativeIndexError(const Token *tok, const ValueFlow::Value &inde << ", otherwise there is negative array index " << index.intvalue << "."; else errmsg << "Array index " << index.intvalue << " is out of bounds."; - reportError(errorPath, index.errorSeverity() ? Severity::error : Severity::warning, "negativeContainerIndex", errmsg.str(), CWE786, index.inconclusive); + reportError(errorPath, index.errorSeverity() ? Severity::error : Severity::warning, "negativeContainerIndex", errmsg.str(), CWE786, index.isInconclusive()); } void CheckStl::erase() diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 66f8b6ae3..902192a02 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -110,7 +110,7 @@ void CheckType::tooBigBitwiseShiftError(const Token *tok, int lhsbits, const Val if (rhsbits.condition) errmsg << ". See condition at line " << rhsbits.condition->linenr() << "."; - reportError(errorPath, rhsbits.errorSeverity() ? Severity::error : Severity::warning, id, errmsg.str(), CWE758, rhsbits.inconclusive); + reportError(errorPath, rhsbits.errorSeverity() ? Severity::error : Severity::warning, id, errmsg.str(), CWE758, rhsbits.isInconclusive()); } void CheckType::tooBigSignedBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits) @@ -129,7 +129,7 @@ void CheckType::tooBigSignedBitwiseShiftError(const Token *tok, int lhsbits, con if (rhsbits.condition) errmsg << ". See condition at line " << rhsbits.condition->linenr() << "."; - reportError(errorPath, rhsbits.errorSeverity() ? Severity::error : Severity::warning, id, errmsg.str(), CWE758, rhsbits.inconclusive); + reportError(errorPath, rhsbits.errorSeverity() ? Severity::error : Severity::warning, id, errmsg.str(), CWE758, rhsbits.isInconclusive()); } //--------------------------------------------------------------------------- @@ -198,7 +198,7 @@ void CheckType::integerOverflowError(const Token *tok, const ValueFlow::Value &v "integerOverflow", msg, CWE190, - value.inconclusive); + value.isInconclusive()); } //--------------------------------------------------------------------------- @@ -410,5 +410,5 @@ void CheckType::floatToIntegerOverflowError(const Token *tok, const ValueFlow::V reportError(getErrorPath(tok, &value, "float to integer conversion"), value.errorSeverity() ? Severity::error : Severity::warning, "floatConversionOverflow", - errmsg.str(), CWE190, value.inconclusive); + errmsg.str(), CWE190, value.isInconclusive()); } diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index e146a5673..d1e844712 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1238,7 +1238,7 @@ void CheckUninitVar::valueFlowUninit() if (!tok->variable() || tok->values().size() != 1U) continue; const ValueFlow::Value &v = tok->values().front(); - if (v.valueType != ValueFlow::Value::UNINIT || v.inconclusive) + if (v.valueType != ValueFlow::Value::UNINIT || v.isInconclusive()) continue; if (!isVariableUsage(tok, tok->variable()->isPointer(), NO_ALLOC)) continue; diff --git a/lib/settings.cpp b/lib/settings.cpp index f978d29e6..389e2106f 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -131,7 +131,7 @@ bool Settings::isEnabled(const ValueFlow::Value *value, bool inconclusiveCheck) { if (!isEnabled(Settings::WARNING) && (value->condition || value->defaultArg)) return false; - if (!inconclusive && (inconclusiveCheck || value->inconclusive)) + if (!inconclusive && (inconclusiveCheck || value->isInconclusive())) return false; return true; } diff --git a/lib/token.cpp b/lib/token.cpp index 9288c7900..f934b7eea 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1362,6 +1362,8 @@ void Token::printValueFlow(bool xml, std::ostream &out) const out << " known=\"true\""; else if (it->isPossible()) out << " possible=\"true\""; + else if (it->isInconclusive()) + out << " inconclusive=\"true\""; out << "/>" << std::endl; } @@ -1409,14 +1411,14 @@ const ValueFlow::Value * Token::getValueLE(const MathLib::bigint val, const Sett std::list::const_iterator it; for (it = _values->begin(); it != _values->end(); ++it) { if (it->isIntValue() && it->intvalue <= val) { - if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) + if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); - if (!ret->inconclusive && !ret->condition) + if (!ret->isInconclusive() && !ret->condition) break; } } if (settings && ret) { - if (ret->inconclusive && !settings->inconclusive) + if (ret->isInconclusive() && !settings->inconclusive) return nullptr; if (ret->condition && !settings->isEnabled(Settings::WARNING)) return nullptr; @@ -1432,14 +1434,14 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett std::list::const_iterator it; for (it = _values->begin(); it != _values->end(); ++it) { if (it->isIntValue() && it->intvalue >= val) { - if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) + if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); - if (!ret->inconclusive && !ret->condition) + if (!ret->isInconclusive() && !ret->condition) break; } } if (settings && ret) { - if (ret->inconclusive && !settings->inconclusive) + if (ret->isInconclusive() && !settings->inconclusive) return nullptr; if (ret->condition && !settings->isEnabled(Settings::WARNING)) return nullptr; @@ -1455,14 +1457,14 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, unsigned int std::list::const_iterator it; for (it = _values->begin(); it != _values->end(); ++it) { if (it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) { - if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) + if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); - if (!ret->inconclusive && !ret->condition) + if (!ret->isInconclusive() && !ret->condition) break; } } if (settings && ret) { - if (ret->inconclusive && !settings->inconclusive) + if (ret->isInconclusive() && !settings->inconclusive) return nullptr; if (ret->condition && !settings->isEnabled(Settings::WARNING)) return nullptr; @@ -1575,7 +1577,7 @@ bool Token::addValue(const ValueFlow::Value &value) continue; // same value, but old value is inconclusive so replace it - if (it->inconclusive && !value.inconclusive) { + if (it->isInconclusive() && !value.isInconclusive()) { *it = value; if (it->varId == 0) it->varId = _varId; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f8d4c58da..559ee5a08 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -382,7 +382,7 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti (value1->varId == value2->varId && value1->varvalue == value2->varvalue && value1->isIntValue() && value2->isIntValue())) { ValueFlow::Value result(0); result.condition = value1->condition ? value1->condition : value2->condition; - result.inconclusive = value1->inconclusive | value2->inconclusive; + result.setInconclusive(value1->isInconclusive() | value2->isInconclusive()); result.varId = (value1->varId != 0U) ? value1->varId : value2->varId; result.varvalue = (result.varId == value1->varId) ? value1->varvalue : value2->varvalue; result.errorPath = (value1->errorPath.empty() ? value2 : value1)->errorPath; @@ -593,7 +593,7 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti (value1->varId == value2->varId && value1->varvalue == value2->varvalue)) { ValueFlow::Value result(0); result.condition = value1->condition ? value1->condition : value2->condition; - result.inconclusive = value1->inconclusive | value2->inconclusive; + result.setInconclusive(value1->isInconclusive() | value2->isInconclusive()); result.varId = (value1->varId != 0U) ? value1->varId : value2->varId; result.varvalue = (result.varId == value1->varId) ? value1->intvalue : value2->intvalue; if (value1->valueKind == value2->valueKind) @@ -1080,8 +1080,8 @@ static void valueFlowReverse(TokenList *tokenlist, bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction"); break; } - val.inconclusive |= inconclusive; - val2.inconclusive |= inconclusive; + val.setInconclusive(inconclusive); + val2.setInconclusive(inconclusive); // skip if variable is conditionally used in ?: expression if (const Token *parent = skipValueInConditionalExpression(tok2)) { @@ -1989,18 +1989,14 @@ static bool valueFlowForward(Token * const startToken, } if (inconclusive) { std::list::iterator it; - for (it = values.begin(); it != values.end(); ++it) { - it->inconclusive = true; - it->changeKnownToPossible(); - } + for (it = values.begin(); it != values.end(); ++it) + it->setInconclusive(); } if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") { if (settings->inconclusive) { std::list::iterator it; - for (it = values.begin(); it != values.end(); ++it) { - it->inconclusive = true; - it->changeKnownToPossible(); - } + for (it = values.begin(); it != values.end(); ++it) + it->setInconclusive(); } else { if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by member function"); @@ -3176,7 +3172,6 @@ ValueFlow::Value::Value(const Token *c, long long val) condition(c), varId(0U), conditional(false), - inconclusive(false), defaultArg(false), valueKind(ValueKind::Possible) { diff --git a/lib/valueflow.h b/lib/valueflow.h index 57a57ec9c..8ad5926b3 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -39,7 +39,7 @@ namespace ValueFlow { typedef std::pair ErrorPathItem; typedef std::list ErrorPath; - explicit Value(long long val = 0) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(NonMovedVariable), varvalue(val), condition(nullptr), varId(0U), conditional(false), inconclusive(false), defaultArg(false), valueKind(ValueKind::Possible) {} + explicit Value(long long val = 0) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(NonMovedVariable), varvalue(val), condition(nullptr), varId(0U), conditional(false), defaultArg(false), valueKind(ValueKind::Possible) {} Value(const Token *c, long long val); bool operator==(const Value &rhs) const { @@ -71,7 +71,6 @@ namespace ValueFlow { condition == rhs.condition && varId == rhs.varId && conditional == rhs.conditional && - inconclusive == rhs.inconclusive && defaultArg == rhs.defaultArg && valueKind == rhs.valueKind; } @@ -121,9 +120,6 @@ namespace ValueFlow { /** Conditional value */ bool conditional; - /** Is this value inconclusive? */ - bool inconclusive; - /** Is this value passed as default parameter to the function? */ bool defaultArg; @@ -144,7 +140,9 @@ namespace ValueFlow { /** This value is possible, other unlisted values may also be possible */ Possible, /** Only listed values are possible */ - Known + Known, + /** Inconclusive */ + Inconclusive } valueKind; void setKnown() { @@ -163,6 +161,15 @@ namespace ValueFlow { return valueKind == ValueKind::Possible; } + void setInconclusive(bool inconclusive = true) { + if (inconclusive) + valueKind = ValueKind::Inconclusive; + } + + bool isInconclusive() const { + return valueKind == ValueKind::Inconclusive; + } + void changeKnownToPossible() { if (isKnown()) valueKind = ValueKind::Possible;