diff --git a/gui/projectfile.cpp b/gui/projectfile.cpp index 1666c7132..aad1a3d79 100644 --- a/gui/projectfile.cpp +++ b/gui/projectfile.cpp @@ -168,10 +168,10 @@ bool ProjectFile::read(const QString &filename) mClangTidy = tools.contains(CLANG_TIDY); } - if (insideProject && xmlReader.name() == CppcheckXml::TagsElementName) + if (xmlReader.name() == CppcheckXml::TagsElementName) readStringList(mTags, xmlReader, CppcheckXml::TagElementName); - if (insideProject && xmlReader.name() == CppcheckXml::MaxCtuDepthElementName) + if (xmlReader.name() == CppcheckXml::MaxCtuDepthElementName) mMaxCtuDepth = readInt(xmlReader, mMaxCtuDepth); break; diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index fb66631e0..718ef45b6 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -223,6 +223,8 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok, void CheckCondition::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result) { + if (tok2 && diag(tok2->tokAt(2))) + return; std::list locations = { tok1, tok2 }; reportError(locations, Severity::style, @@ -520,6 +522,8 @@ void CheckCondition::multiCondition() void CheckCondition::overlappingElseIfConditionError(const Token *tok, nonneg int line1) { + if (diag(tok)) + return; std::ostringstream errmsg; errmsg << "Expression is always false because 'else if' condition matches previous condition at line " << line1 << "."; @@ -529,6 +533,8 @@ void CheckCondition::overlappingElseIfConditionError(const Token *tok, nonneg in void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath) { + if (diag(ifCond) & diag(elseIfCond)) + return; std::ostringstream errmsg; errmsg << "Expression is always true because 'else if' condition is opposite to previous condition at line " << ifCond->linenr() << "."; diff --git a/lib/checkcondition.h b/lib/checkcondition.h index e3cc888ce..bcadceb20 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -58,11 +58,11 @@ public: checkCondition.multiCondition2(); checkCondition.checkIncorrectLogicOperator(); checkCondition.checkInvalidTestForOverflow(); - checkCondition.alwaysTrueFalse(); checkCondition.duplicateCondition(); checkCondition.checkPointerAdditionResultNotNull(); checkCondition.checkDuplicateConditionalAssign(); checkCondition.assignIf(); + checkCondition.alwaysTrueFalse(); checkCondition.checkBadBitmaskCheck(); checkCondition.comparison(); checkCondition.checkModuloAlwaysTrueFalse(); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 2689dc48a..5bbbf70a0 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -62,6 +62,8 @@ void CheckStl::outOfBounds() for (const ValueFlow::Value &value : tok->values()) { if (!value.isContainerSizeValue()) continue; + if (value.isImpossible()) + continue; if (value.isInconclusive() && !mSettings->inconclusive) continue; if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 29c8d57cc..2b6491164 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -953,7 +953,7 @@ bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc al if (!pointer) return false; - if (pointer && alloc != CTOR_CALL && Token::Match(vartok, "%name% . %name% (")) + if (alloc != CTOR_CALL && Token::Match(vartok, "%name% . %name% (")) return true; bool assignment = false; diff --git a/lib/ctu.cpp b/lib/ctu.cpp index 6fd3c1d0c..8313cb66a 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -321,6 +321,9 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) for (const ValueFlow::Value &value : argtok->values()) { if ((!value.isIntValue() || value.intvalue != 0 || value.isInconclusive()) && !value.isBufferSizeValue()) continue; + // Skip impossible values since they cannot be represented + if (value.isImpossible()) + continue; FileInfo::FunctionCall functionCall; functionCall.callValueType = value.valueType; functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); diff --git a/lib/token.cpp b/lib/token.cpp index fdc77059b..fe0656361 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1592,6 +1592,8 @@ void Token::printValueFlow(bool xml, std::ostream &out) const out << " known=\"true\""; else if (value.isPossible()) out << " possible=\"true\""; + else if (value.isImpossible()) + out << " impossible=\"true\""; else if (value.isInconclusive()) out << " inconclusive=\"true\""; out << "/>" << std::endl; @@ -1600,6 +1602,12 @@ void Token::printValueFlow(bool xml, std::ostream &out) const else { if (&value != &tok->mImpl->mValues->front()) out << ","; + if (value.isImpossible()) + out << "!"; + if (value.bound == ValueFlow::Value::Bound::Lower) + out << ">"; + if (value.bound == ValueFlow::Value::Bound::Upper) + out << "<"; switch (value.valueType) { case ValueFlow::Value::INT: if (tok->valueType() && tok->valueType()->sign == ValueType::UNSIGNED) @@ -1650,6 +1658,8 @@ const ValueFlow::Value * Token::getValueLE(const MathLib::bigint val, const Sett const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) { + if (it->isImpossible()) + continue; if (it->isIntValue() && it->intvalue <= val) { if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); @@ -1673,6 +1683,8 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) { + if (it->isImpossible()) + continue; if (it->isIntValue() && it->intvalue >= val) { if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); @@ -1696,6 +1708,8 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, nonneg int ar const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) { + if (it->isImpossible()) + continue; if ((it->isIntValue() && !settings->library.isIntArgValid(ftok, argnr, it->intvalue)) || (it->isFloatValue() && !settings->library.isFloatArgValid(ftok, argnr, it->floatValue))) { if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) @@ -1791,6 +1805,77 @@ const Token *Token::getValueTokenDeadPointer() const return nullptr; } +static bool removeContradiction(std::list& values) +{ + bool result = false; + for (ValueFlow::Value& x : values) { + if (x.isNonValue()) + continue; + for (ValueFlow::Value& y : values) { + if (y.isNonValue()) + continue; + if (x == y) + continue; + if (x.valueType != y.valueType) + continue; + if (x.isImpossible() == y.isImpossible()) + continue; + if (!x.equalValue(y)) + continue; + if (x.bound == y.bound || + (x.bound != ValueFlow::Value::Bound::Point && y.bound != ValueFlow::Value::Bound::Point)) { + const bool removex = !x.isImpossible() || y.isKnown(); + const bool removey = !y.isImpossible() || x.isKnown(); + if (removex) + values.remove(x); + if (removey) + values.remove(y); + return true; + } else if (x.bound == ValueFlow::Value::Bound::Point) { + y.decreaseRange(); + result = true; + } + } + } + return result; +} + +static void removeOverlaps(std::list& values) +{ + for (ValueFlow::Value& x : values) { + if (x.isNonValue()) + continue; + values.remove_if([&](ValueFlow::Value& y) { + if (y.isNonValue()) + return false; + if (&x == &y) + return false; + if (x.valueType != y.valueType) + return false; + if (x.valueKind != y.valueKind) + return false; + // TODO: Remove points coverd in a lower or upper bound + // TODO: Remove lower or upper bound already covered by a lower and upper bound + if (!x.equalValue(y)) + return false; + if (x.bound != y.bound) + return false; + return true; + }); + } +} + +// Removing contradictions is an NP-hard problem. Instead we run multiple +// passes to try to catch most contradictions +static void removeContradictions(std::list& values) +{ + for (int i = 0; i < 4; i++) { + if (!removeContradiction(values)) + return; + removeOverlaps(values); + } +} + bool Token::addValue(const ValueFlow::Value &value) { if (value.isKnown() && mImpl->mValues) { @@ -1813,6 +1898,9 @@ bool Token::addValue(const ValueFlow::Value &value) if (it->valueType != value.valueType) continue; + if (it->isImpossible() != value.isImpossible()) + continue; + // different value => continue bool differentValue = true; switch (it->valueType) { @@ -1843,7 +1931,7 @@ bool Token::addValue(const ValueFlow::Value &value) continue; // same value, but old value is inconclusive so replace it - if (it->isInconclusive() && !value.isInconclusive()) { + if (it->isInconclusive() && !value.isInconclusive() && !value.isImpossible()) { *it = value; if (it->varId == 0) it->varId = mImpl->mVarId; @@ -1871,6 +1959,8 @@ bool Token::addValue(const ValueFlow::Value &value) mImpl->mValues = new std::list(1, v); } + removeContradictions(*mImpl->mValues); + return true; } diff --git a/lib/token.h b/lib/token.h index bd1efee8b..917457639 100644 --- a/lib/token.h +++ b/lib/token.h @@ -988,8 +988,8 @@ public: const ValueFlow::Value * getValue(const MathLib::bigint val) const { if (!mImpl->mValues) return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value &value) { - return value.isIntValue() && value.intvalue == val; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { + return value.isIntValue() && !value.isImpossible() && value.intvalue == val; }); return it == mImpl->mValues->end() ? nullptr : &*it;; } @@ -1001,6 +1001,8 @@ public: for (const ValueFlow::Value &value : *mImpl->mValues) { if (!value.isIntValue()) continue; + if (value.isImpossible()) + continue; if ((!ret || value.intvalue > ret->intvalue) && ((value.condition != nullptr) == condition)) ret = &value; @@ -1011,8 +1013,9 @@ public: const ValueFlow::Value * getMovedValue() const { if (!mImpl->mValues) return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value &value) { - return value.isMovedValue() && value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) { + return value.isMovedValue() && !value.isImpossible() && + value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable; }); return it == mImpl->mValues->end() ? nullptr : &*it;; } @@ -1025,8 +1028,8 @@ public: const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const { if (!mImpl->mValues) return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value &value) { - return value.isContainerSizeValue() && value.intvalue == val; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { + return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val; }); return it == mImpl->mValues->end() ? nullptr : &*it; } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 02d8e8c17..a42c329aa 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -189,6 +189,53 @@ static void changeKnownToPossible(std::list &values, int indir } } +static void removeImpossible(std::list& values, int indirect = -1) +{ + values.remove_if([&](const ValueFlow::Value& v) { + if (indirect >= 0 && v.indirect != indirect) + return false; + return v.isImpossible(); + }); +} + +static void lowerToPossible(std::list& values, int indirect = -1) +{ + changeKnownToPossible(values, indirect); + removeImpossible(values, indirect); +} + +static void lowerToInconclusive(std::list& values, const Settings* settings, int indirect = -1) +{ + if (settings->inconclusive) { + removeImpossible(values, indirect); + for (ValueFlow::Value& v : values) { + if (indirect >= 0 && v.indirect != indirect) + continue; + v.setInconclusive(); + } + } else { + // Remove all values if the inconclusive flags is not set + values.remove_if([&](const ValueFlow::Value& v) { + if (indirect >= 0 && v.indirect != indirect) + return false; + return true; + }); + } +} + +static void changePossibleToKnown(std::list& values, int indirect = -1) +{ + for (ValueFlow::Value& v : values) { + if (indirect >= 0 && v.indirect != indirect) + continue; + if (!v.isPossible()) + continue; + if (v.bound != ValueFlow::Value::Bound::Point) + continue; + v.setKnown(); + } +} + /** * Is condition always false when variable has given value? * \param condition top ast token in condition @@ -229,6 +276,23 @@ static bool conditionIsTrue(const Token *condition, const ProgramMemory &program return !error && result == 1; } +void setValueUpperBound(ValueFlow::Value& value, bool upper) +{ + if (upper) + value.bound = ValueFlow::Value::Bound::Upper; + else + value.bound = ValueFlow::Value::Bound::Lower; +} + +void setValueBound(ValueFlow::Value& value, const Token* tok, bool invert) +{ + if (Token::Match(tok, "<|<=")) { + setValueUpperBound(value, !invert); + } else if (Token::Match(tok, ">|>=")) { + setValueUpperBound(value, invert); + } +} + static void setConditionalValues(const Token *tok, bool invert, MathLib::bigint value, @@ -237,20 +301,37 @@ static void setConditionalValues(const Token *tok, { if (Token::Match(tok, "==|!=|>=|<=")) { true_value = ValueFlow::Value{tok, value}; - false_value = ValueFlow::Value{tok, value}; - return; - } - const char *greaterThan = ">"; - const char *lessThan = "<"; - if (invert) - std::swap(greaterThan, lessThan); - if (Token::simpleMatch(tok, greaterThan)) { - true_value = ValueFlow::Value{tok, value + 1}; - false_value = ValueFlow::Value{tok, value}; - } else if (Token::simpleMatch(tok, lessThan)) { - true_value = ValueFlow::Value{tok, value - 1}; - false_value = ValueFlow::Value{tok, value}; + const char* greaterThan = ">="; + const char* lessThan = "<="; + if (invert) + std::swap(greaterThan, lessThan); + if (Token::simpleMatch(tok, greaterThan)) { + false_value = ValueFlow::Value{tok, value - 1}; + } else if (Token::simpleMatch(tok, lessThan)) { + false_value = ValueFlow::Value{tok, value + 1}; + } else { + false_value = ValueFlow::Value{tok, value}; + } + } else { + const char* greaterThan = ">"; + const char* lessThan = "<"; + if (invert) + std::swap(greaterThan, lessThan); + if (Token::simpleMatch(tok, greaterThan)) { + true_value = ValueFlow::Value{tok, value + 1}; + false_value = ValueFlow::Value{tok, value}; + } else if (Token::simpleMatch(tok, lessThan)) { + true_value = ValueFlow::Value{tok, value - 1}; + false_value = ValueFlow::Value{tok, value}; + } } + setValueBound(true_value, tok, invert); + setValueBound(false_value, tok, !invert); +} + +static bool isSaturated(MathLib::bigint value) +{ + return value == std::numeric_limits::max() || value == std::numeric_limits::min(); } static const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value) @@ -259,10 +340,16 @@ static const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_val return nullptr; if (Token::Match(tok, "%comp%")) { if (tok->astOperand1()->hasKnownIntValue()) { - setConditionalValues(tok, true, tok->astOperand1()->values().front().intvalue, true_value, false_value); + MathLib::bigint value = tok->astOperand1()->values().front().intvalue; + if (isSaturated(value)) + return nullptr; + setConditionalValues(tok, true, value, true_value, false_value); return tok->astOperand2(); } else if (tok->astOperand2()->hasKnownIntValue()) { - setConditionalValues(tok, false, tok->astOperand2()->values().front().intvalue, true_value, false_value); + MathLib::bigint value = tok->astOperand2()->values().front().intvalue; + if (isSaturated(value)) + return nullptr; + setConditionalValues(tok, false, value, true_value, false_value); return tok->astOperand1(); } } @@ -507,6 +594,8 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl { if (value1.isKnown() && value2.isKnown()) result->setKnown(); + else if (value1.isImpossible() || value2.isImpossible()) + result->setImpossible(); else if (value1.isInconclusive() || value2.isInconclusive()) result->setInconclusive(); else @@ -618,6 +707,9 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti // cast.. if (const Token *castType = getCastTypeStartToken(parent)) { + if (astIsPointer(tok) && value.valueType == ValueFlow::Value::INT && + Token::simpleMatch(parent->astOperand1(), "dynamic_cast")) + return; const ValueType &valueType = ValueType::parseDecl(castType, settings); setTokenValueCast(parent, valueType, value, settings); } @@ -674,6 +766,10 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti parent->astOperand1() && parent->astOperand2()) { + // Dont compare impossible values + if (parent->isComparisonOp() && value.isImpossible()) + return; + // known result when a operand is 0. if (Token::Match(parent, "[&*]") && value.isKnown() && value.isIntValue() && value.intvalue==0) { setTokenValue(parent, value, settings); @@ -1835,6 +1931,9 @@ static void valueFlowReverse(TokenList *tokenlist, bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction"); break; } + // Impossible values cant be inconclusive + if (val.isImpossible() || val2.isImpossible()) + break; val.setInconclusive(inconclusive); val2.setInconclusive(inconclusive); @@ -2048,14 +2147,8 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo val2.varId = varid; } } - valueFlowReverse(tokenlist, - tok, - vartok, - val, - val2, - errorLogger, - settings); - + Token* startTok = tok->astParent() ? tok->astParent() : tok->previous(); + valueFlowReverse(tokenlist, startTok, vartok, val, val2, errorLogger, settings); } } } @@ -2113,11 +2206,7 @@ static bool handleKnownValuesInLoop(const Token *startToken, const bool isChanged = isVariableChanged(startToken, endToken, varid, globalvar, settings, true); if (!isChanged) return false; - for (std::list::iterator it = values->begin(); it != values->end(); ++it) { - if (it->isKnown()) { - it->setPossible(); - } - } + lowerToPossible(*values); return isChanged; } @@ -2224,6 +2313,8 @@ static bool valueFlowForward(Token * const startToken, return true; for (Token *tok2 = startToken; tok2 && tok2 != endToken; tok2 = tok2->next()) { + if (values.empty()) + return true; if (indentlevel >= 0 && tok2->str() == "{") ++indentlevel; else if (indentlevel >= 0 && tok2->str() == "}") { @@ -2271,7 +2362,7 @@ static bool valueFlowForward(Token * const startToken, Token::simpleMatch(tok2->link()->previous(), "else {") && !isReturnScope(tok2->link()->tokAt(-2), settings) && isVariableChanged(tok2->link(), tok2, varid, var->isGlobal(), settings, tokenlist->isCPP())) { - changeKnownToPossible(values); + lowerToPossible(values); } } @@ -2289,7 +2380,7 @@ static bool valueFlowForward(Token * const startToken, } if (Token::Match(tok2, "[;{}] %name% :") || tok2->str() == "case") { - changeKnownToPossible(values); + lowerToPossible(values); tok2 = tok2->tokAt(2); continue; } @@ -2418,7 +2509,7 @@ static bool valueFlowForward(Token * const startToken, if (!condAlwaysFalse && isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { removeValues(values, truevalues); - changeKnownToPossible(values); + lowerToPossible(values); } // goto '}' @@ -2446,7 +2537,7 @@ static bool valueFlowForward(Token * const startToken, if (!condAlwaysTrue && isVariableChanged(startTokenElse, startTokenElse->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { removeValues(values, falsevalues); - changeKnownToPossible(values); + lowerToPossible(values); } // goto '}' @@ -2519,7 +2610,7 @@ static bool valueFlowForward(Token * const startToken, // Remove conditional values std::list::iterator it; for (it = values.begin(); it != values.end();) { - if (it->condition || it->conditional) + if (it->condition || it->conditional || it->isImpossible()) values.erase(it++); else { it->changeKnownToPossible(); @@ -2607,6 +2698,7 @@ static bool valueFlowForward(Token * const startToken, ++number_of_if; // Set "conditional" flag for all values + removeImpossible(values); std::list::iterator it; for (it = values.begin(); it != values.end(); ++it) { it->conditional = true; @@ -2641,7 +2733,7 @@ static bool valueFlowForward(Token * const startToken, if (tok2 == endToken) break; --indentlevel; - changeKnownToPossible(values); + lowerToPossible(values); continue; } } @@ -2675,9 +2767,9 @@ static bool valueFlowForward(Token * const startToken, for (const ValueFlow::Value &v : values) valueFlowAST(expr, varid, v, settings); if (isVariableChangedByFunctionCall(expr, 0, varid, settings, nullptr)) - changeKnownToPossible(values, 0); + lowerToPossible(values, 0); if (isVariableChangedByFunctionCall(expr, 1, varid, settings, nullptr)) - changeKnownToPossible(values, 1); + lowerToPossible(values, 1); } else { for (const ValueFlow::Value &v : values) { const ProgramMemory programMemory(getProgramMemory(tok2, varid, v)); @@ -2704,7 +2796,7 @@ static bool valueFlowForward(Token * const startToken, } if (changed0 || changed1) - changeKnownToPossible(values); + lowerToPossible(values); } // Skip conditional expressions.. @@ -2871,6 +2963,13 @@ static bool valueFlowForward(Token * const startToken, return false; } + // bailout if its stream.. + if (isLikelyStream(tokenlist->isCPP(), tok2)) { + if (settings->debugwarnings) + bailout(tokenlist, errorLogger, tok2, "Stream used: " + tok2->str()); + return false; + } + // assigned by subfunction? for (int i:getIndirections(values)) { bool inconclusive = false; @@ -2879,20 +2978,8 @@ static bool valueFlowForward(Token * const startToken, return v.indirect <= i; }); } - if (inconclusive) { - if (settings->inconclusive) { - for (ValueFlow::Value &v : values) { - if (v.indirect != i) - continue; - v.setInconclusive(); - } - } else { - // If inconclusive flag not enable then remove the values - values.remove_if([&](const ValueFlow::Value &v) { - return v.indirect == i; - }); - } - } + if (inconclusive) + lowerToInconclusive(values, settings, i); } if (values.empty()) { if (settings->debugwarnings) @@ -2900,10 +2987,8 @@ static bool valueFlowForward(Token * const startToken, return false; } if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") { - if (settings->inconclusive) { - for (ValueFlow::Value &v : values) - v.setInconclusive(); - } else { + lowerToInconclusive(values, settings); + if (!settings->inconclusive) { if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by member function"); return false; @@ -3315,7 +3400,7 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog // Static variable initialisation? if (var->isStatic() && var->nameToken() == parent->astOperand1()) - changeKnownToPossible(values); + lowerToPossible(values); // Skip RHS const Token *nextExpression = nextAfterAstRightmostLeaf(parent); @@ -4004,7 +4089,7 @@ static void valueFlowForwardAssign(Token * const tok, // Static variable initialisation? if (var->isStatic() && init) - changeKnownToPossible(values); + lowerToPossible(values); // Skip RHS const Token * nextExpression = tok->astParent() ? nextAfterAstRightmostLeaf(tok->astParent()) : tok->next(); @@ -4098,24 +4183,27 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat } } -static void valueFlowSetConditionToKnown(const Token* tok, std::list& values, bool then) +static bool isConditionKnown(const Token* tok, bool then) { - if (values.size() != 1U) - return; - if (values.front().isKnown()) - return; - if (then && !Token::Match(tok, "==|!|(")) - return; - if (!then && !Token::Match(tok, "!=|%var%|(")) - return; const char * op = "||"; if (then) op = "&&"; const Token* parent = tok->astParent(); while (parent && parent->str() == op) parent = parent->astParent(); - if (parent && parent->str() == "(") - values.front().setKnown(); + return (parent && parent->str() == "("); +} + +static void valueFlowSetConditionToKnown(const Token* tok, std::list& values, bool then) +{ + if (values.empty()) + return; + if (then && !Token::Match(tok, "==|!|(")) + return; + if (!then && !Token::Match(tok, "!=|%var%|(")) + return; + if (isConditionKnown(tok, then)) + changePossibleToKnown(values); } static bool isBreakScope(const Token* const endToken) @@ -4127,6 +4215,18 @@ static bool isBreakScope(const Token* const endToken) return Token::findmatch(endToken->link(), "break|goto", endToken); } +static ValueFlow::Value asImpossible(ValueFlow::Value v) +{ + v.invertRange(); + v.setImpossible(); + return v; +} + +void insertImpossible(std::list& values, const std::list& input) +{ + std::transform(input.begin(), input.end(), std::back_inserter(values), &asImpossible); +} + struct ValueFlowConditionHandler { struct Condition { const Token *vartok; @@ -4212,19 +4312,22 @@ struct ValueFlowConditionHandler { continue; } + std::list thenValues; + std::list elseValues; + + if (!Token::Match(tok, "!=|%var%")) { + thenValues.insert(thenValues.end(), cond.true_values.begin(), cond.true_values.end()); + if (isConditionKnown(tok, false)) + insertImpossible(elseValues, cond.false_values); + } + if (!Token::Match(tok, "==|!") && !Token::Match(tok->previous(), "%name% (")) { + elseValues.insert(elseValues.end(), cond.false_values.begin(), cond.false_values.end()); + if (isConditionKnown(tok, true)) + insertImpossible(thenValues, cond.true_values); + } + // start token of conditional code - Token *startTokens[] = {nullptr, nullptr}; - - // based on the comparison, should we check the if or while? - bool check_if = false; - bool check_else = false; - if (Token::Match(tok, "==|>=|<=|!|>|<|(")) - check_if = true; - if (Token::Match(tok, "%name%|!=|>|<")) - check_else = true; - - if (!check_if && !check_else) - continue; + Token* startTokens[] = {nullptr, nullptr}; // if astParent is "!" we need to invert codeblock { @@ -4234,50 +4337,41 @@ struct ValueFlowConditionHandler { while (parent && parent->str() == "&&") parent = parent->astParent(); if (parent && (parent->str() == "!" || Token::simpleMatch(parent, "== false"))) { - check_if = !check_if; - check_else = !check_else; - std::swap(cond.true_values, cond.false_values); + std::swap(thenValues, elseValues); } tok2 = parent; } } - if (cond.true_values != cond.false_values) { - check_if = true; - check_else = true; - } // determine startToken(s) - if (check_if && Token::simpleMatch(top->link(), ") {")) + if (Token::simpleMatch(top->link(), ") {")) startTokens[0] = top->link()->next(); - if (check_else && Token::simpleMatch(top->link()->linkAt(1), "} else {")) + if (Token::simpleMatch(top->link()->linkAt(1), "} else {")) startTokens[1] = top->link()->linkAt(1)->tokAt(2); - bool bail = false; - const bool bothCanBeKnown = check_if && check_else && !Token::Match(tok->astParent(), "&&|%oror%"); + int changeBlock = -1; for (int i = 0; i < 2; i++) { const Token *const startToken = startTokens[i]; if (!startToken) continue; - std::list &values = (i == 0 ? cond.true_values : cond.false_values); + std::list& values = (i == 0 ? thenValues : elseValues); valueFlowSetConditionToKnown(tok, values, i == 0); - if (bothCanBeKnown) - valueFlowSetConditionToKnown(tok, values, i != 0); - bool changed = forward(startTokens[i], startTokens[i]->link(), var, values, true); - values.front().setPossible(); - if (changed) { - // TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call - if (settings->debugwarnings) - bailout(tokenlist, - errorLogger, - startTokens[i]->link(), - "valueFlowAfterCondition: " + var->name() + " is changed in conditional block"); - bail = true; - } + // TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call + if (forward(startTokens[i], startTokens[i]->link(), var, values, true)) + changeBlock = i; + changeKnownToPossible(values); } - if (bail) + // TODO: Values changed in noreturn blocks should not bail + if (changeBlock >= 0 && !Token::simpleMatch(top->previous(), "while (")) { + if (settings->debugwarnings) + bailout(tokenlist, + errorLogger, + startTokens[changeBlock]->link(), + "valueFlowAfterCondition: " + var->name() + " is changed in conditional block"); continue; + } // After conditional code.. if (Token::simpleMatch(top->link(), ") {")) { @@ -4304,21 +4398,34 @@ struct ValueFlowConditionHandler { dead_else = isReturnScope(after, settings); } - std::list *values = nullptr; - if (!dead_if && check_if) - values = &cond.true_values; - else if (!dead_else && check_else) - values = &cond.false_values; + if (dead_if && dead_else) + continue; - if (values) { + std::list values; + if (dead_if) { + values = elseValues; + } else if (dead_else) { + values = thenValues; + } else { + std::copy_if(thenValues.begin(), + thenValues.end(), + std::back_inserter(values), + std::mem_fn(&ValueFlow::Value::isPossible)); + std::copy_if(elseValues.begin(), + elseValues.end(), + std::back_inserter(values), + std::mem_fn(&ValueFlow::Value::isPossible)); + } + + if (!values.empty()) { if ((dead_if || dead_else) && !Token::Match(tok->astParent(), "&&|&")) { - valueFlowSetConditionToKnown(tok, *values, true); - valueFlowSetConditionToKnown(tok, *values, false); + valueFlowSetConditionToKnown(tok, values, true); + valueFlowSetConditionToKnown(tok, values, false); } // TODO: constValue could be true if there are no assignments in the conditional blocks and // perhaps if there are no && and no || in the condition bool constValue = false; - forward(after, top->scope()->bodyEnd, var, *values, constValue); + forward(after, top->scope()->bodyEnd, var, values, constValue); } } } @@ -4352,24 +4459,14 @@ static void valueFlowAfterCondition(TokenList *tokenlist, vartok = vartok->astOperand1(); if (!vartok->isName()) return cond; - if (astIsPointer(vartok) && true_value.intvalue == 0) { - if (Token::simpleMatch(tok, "==")) - false_value.intvalue = 1; - if (Token::simpleMatch(tok, "!=")) - true_value.intvalue = 1; - } - cond.true_values.push_back(true_value); cond.false_values.push_back(false_value); cond.vartok = vartok; return cond; } - long long falseIntValue = 0LL; if (tok->str() == "!") { vartok = tok->astOperand1(); - if (astIsPointer(vartok)) - falseIntValue = 1LL; } else if (tok->isName() && (Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->tokAt(-2), "if|while ( %var% [)=]"))) { @@ -4379,7 +4476,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, if (!vartok || !vartok->isName()) return cond; cond.true_values.emplace_back(tok, 0LL); - cond.false_values.emplace_back(tok, falseIntValue); + cond.false_values.emplace_back(tok, 0LL); cond.vartok = vartok; return cond; @@ -4580,6 +4677,89 @@ static void execute(const Token *expr, *error = true; } +static bool isInBounds(const ValueFlow::Value& value, MathLib::bigint x) +{ + if (value.intvalue == x) + return true; + if (value.bound == ValueFlow::Value::Bound::Lower && value.intvalue > x) + return false; + if (value.bound == ValueFlow::Value::Bound::Upper && value.intvalue < x) + return false; + // Checking for equality is not necessary since we already know the value is not equal + if (value.bound == ValueFlow::Value::Bound::Point) + return false; + return true; +} + +static const ValueFlow::Value* proveNotEqual(const std::list& values, MathLib::bigint x) +{ + const ValueFlow::Value* result = nullptr; + for (const ValueFlow::Value& value : values) { + if (value.valueType != ValueFlow::Value::INT) + continue; + if (result && !isInBounds(value, result->intvalue)) + continue; + if (value.isImpossible()) { + if (value.intvalue == x) + return &value; + if (!isInBounds(value, x)) + continue; + result = &value; + } else { + if (value.intvalue == x) + return nullptr; + if (!isInBounds(value, x)) + continue; + result = nullptr; + } + } + return result; +} + +static void valueFlowInferCondition(TokenList* tokenlist, + SymbolDatabase* symboldatabase, + ErrorLogger* errorLogger, + const Settings* settings) +{ + for (Token* tok = tokenlist->front(); tok; tok = tok->next()) { + if (!tok->astParent()) + continue; + if (tok->hasKnownValue()) + continue; + if (Token::Match(tok, "%var%") && (Token::Match(tok->astParent(), "&&|!|%oror%") || + Token::Match(tok->astParent()->previous(), "if|while ("))) { + const ValueFlow::Value* result = proveNotEqual(tok->values(), 0); + if (!result) + continue; + ValueFlow::Value value = *result; + value.intvalue = 1; + value.setKnown(); + setTokenValue(tok, value, settings); + } else if (Token::Match(tok, "==|!=")) { + MathLib::bigint val = 0; + const Token* varTok = nullptr; + if (tok->astOperand1()->hasKnownIntValue()) { + val = tok->astOperand1()->values().front().intvalue; + varTok = tok->astOperand2(); + } else if (tok->astOperand2()->hasKnownIntValue()) { + val = tok->astOperand2()->values().front().intvalue; + varTok = tok->astOperand1(); + } + if (!varTok) + continue; + if (varTok->hasKnownIntValue()) + continue; + const ValueFlow::Value* result = proveNotEqual(varTok->values(), val); + if (!result) + continue; + ValueFlow::Value value = *result; + value.intvalue = tok->str() == "!="; + value.setKnown(); + setTokenValue(tok, value, settings); + } + } +} + static bool valueFlowForLoop1(const Token *tok, int * const varid, MathLib::bigint * const num1, MathLib::bigint * const num2, MathLib::bigint * const numAfter) { tok = tok->tokAt(2); @@ -5163,7 +5343,7 @@ static void valueFlowSubFunction(TokenList* tokenlist, ErrorLogger* errorLogger, } // passed values are not "known".. - changeKnownToPossible(argvalues); + lowerToPossible(argvalues); valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); // FIXME: We need to rewrite the valueflow analysis to better handle multiple arguments @@ -6038,8 +6218,9 @@ static void valueFlowUnknownFunctionReturn(TokenList *tokenlist, const Settings } } -ValueFlow::Value::Value(const Token *c, long long val) +ValueFlow::Value::Value(const Token* c, long long val) : valueType(INT), + bound(Bound::Point), intvalue(val), tokvalue(nullptr), floatValue(0.0), @@ -6130,6 +6311,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings); + valueFlowInferCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); valueFlowSubFunction(tokenlist, errorLogger, settings); diff --git a/lib/valueflow.h b/lib/valueflow.h index f164ac4da..124215549 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -37,6 +37,20 @@ class TokenList; class Variable; namespace ValueFlow { +struct increment { + template + void operator()(T& x) const + { + x++; + } +}; +struct decrement { + template + void operator()(T& x) const + { + x--; + } +}; class CPPCHECKLIB Value { public: typedef std::pair ErrorPathItem; @@ -44,6 +58,7 @@ namespace ValueFlow { explicit Value(long long val = 0) : valueType(ValueType::INT), + bound(Bound::Point), intvalue(val), tokvalue(nullptr), floatValue(0.0), @@ -99,6 +114,28 @@ namespace ValueFlow { return true; } + template + void visitValue(F f) + { + switch (valueType) { + case ValueType::INT: + case ValueType::BUFFER_SIZE: + case ValueType::CONTAINER_SIZE: { + f(intvalue); + break; + } + case ValueType::FLOAT: { + f(floatValue); + break; + } + case ValueType::UNINIT: + case ValueType::TOK: + case ValueType::LIFETIME: + case ValueType::MOVED: + break; + } + } + bool operator==(const Value &rhs) const { if (!equalValue(rhs)) return false; @@ -116,6 +153,23 @@ namespace ValueFlow { return !(*this == rhs); } + void decreaseRange() + { + if (bound == Bound::Lower) + visitValue(increment{}); + else if (bound == Bound::Upper) + visitValue(decrement{}); + } + + void invertRange() + { + if (bound == Bound::Lower) + bound = Bound::Upper; + else if (bound == Bound::Upper) + bound = Bound::Lower; + decreaseRange(); + } + std::string infoString() const; enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE } valueType; @@ -156,6 +210,9 @@ namespace ValueFlow { return isMovedValue() || isUninitValue() || isLifetimeValue(); } + /** The value bound */ + enum class Bound { Upper, Lower, Point } bound; + /** int value */ long long intvalue; @@ -213,7 +270,9 @@ namespace ValueFlow { /** Only listed values are possible */ Known, /** Inconclusive */ - Inconclusive + Inconclusive, + /** Listed values are impossible */ + Impossible } valueKind; void setKnown() { @@ -232,6 +291,10 @@ namespace ValueFlow { return valueKind == ValueKind::Possible; } + bool isImpossible() const { return valueKind == ValueKind::Impossible; } + + void setImpossible() { valueKind = ValueKind::Impossible; } + void setInconclusive(bool inconclusive = true) { if (inconclusive) valueKind = ValueKind::Inconclusive; diff --git a/test/testcondition.cpp b/test/testcondition.cpp index c00615784..7876219bf 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -244,7 +244,9 @@ private: " while (y != 0) g(y);\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (style) Condition 'y!=0' is always true\n[test.cpp:5] -> [test.cpp:6]: (style) Mismatching assignment and comparison, comparison 'y!=0' is always true.\n", errout.str()); + ASSERT_EQUALS( + "[test.cpp:5] -> [test.cpp:6]: (style) Mismatching assignment and comparison, comparison 'y!=0' is always true.\n", + errout.str()); check("void g(int &x);\n" "void f(int x) {\n" @@ -538,14 +540,14 @@ private: " else { if (a == 2) { b = 2; }\n" " else { if (a == 1) { b = 3; } } }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'a==1' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a == 1) { b = 1; }\n" " else { if (a == 2) { b = 2; }\n" " else { if (a == 2) { b = 3; } } }\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'a==2' is always false\n", errout.str()); check("void f(int a, int &b) {\n" " if (a++) { b = 1; }\n" @@ -2062,10 +2064,10 @@ private: check("void f(int x) {\n" "\n" " if (x<4) {\n" - " if (x!=5) {}\n" // <- TODO + " if (x!=5) {}\n" " }\n" "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x!=5' is always true\n", errout.str()); check("void f(int x) {\n" "\n" " if (x<4) {\n" @@ -3229,6 +3231,62 @@ private: " if (x) {}\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #9318 + check("class A {};\n" + "class B : public A {};\n" + "void f(A* x) {\n" + " if (!x)\n" + " return;\n" + " auto b = dynamic_cast(x);\n" + " if (b) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // handleKnownValuesInLoop + check("bool g();\n" + "void f(bool x) {\n" + " if (x) while(x) x = g();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // isLikelyStream + check("void f(std::istringstream& iss) {\n" + " std::string x;\n" + " while (iss) {\n" + " iss >> x;\n" + " if (!iss) break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int x) {\n" + " if (x > 5) {\n" + " x++;\n" + " if (x == 1) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'x==1' is always false\n", errout.str()); + + check("void f(int x) {\n" + " if (x > 5) {\n" + " x++;\n" + " if (x != 1) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'x!=1' is always true\n", errout.str()); + + // #9332 + check("struct A { void* g(); };\n" + "void f() {\n" + " A a;\n" + " void* b = a.g();\n" + " if (!b) return;\n" + " void* c = a.g();\n" + " if (!c) return;\n" + " bool compare = c == b;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void alwaysTrueContainer() { diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f2037be13..cfcfc9ed7 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -76,6 +76,8 @@ private: TEST_CASE(nullpointer34); TEST_CASE(nullpointer35); TEST_CASE(nullpointer36); // #9264 + TEST_CASE(nullpointer37); // #9315 + TEST_CASE(nullpointer38); TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -92,6 +94,7 @@ private: TEST_CASE(nullpointer_in_typeid); TEST_CASE(nullpointer_in_for_loop); TEST_CASE(nullpointerDelete); + TEST_CASE(nullpointerSubFunction); TEST_CASE(nullpointerExit); TEST_CASE(nullpointerStdString); TEST_CASE(nullpointerStdStream); @@ -1440,6 +1443,39 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer37() + { + check("void f(int value, char *string) {\n" + " char *ptr1 = NULL, *ptr2 = NULL;\n" + " unsigned long count = 0;\n" + " if(!string)\n" + " return;\n" + " ptr1 = string;\n" + " ptr2 = strrchr(string, 'a');\n" + " if(ptr2 == NULL)\n" + " return;\n" + " while(ptr1 < ptr2) {\n" + " count++;\n" + " ptr1++;\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + + void nullpointer38() + { + check("void f(int * x) {\n" + " std::vector v;\n" + " if (x) {\n" + " v.push_back(x);\n" + " *x;\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" @@ -2107,6 +2143,16 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointerSubFunction() + { + check("void g(int* x) { *x; }\n" + "void f(int* x) {\n" + " if (x)\n" + " g(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void nullpointerExit() { check("void f() {\n" " K *k = getK();\n" @@ -2915,6 +2961,13 @@ private: " dostuff(0, 0);\n" "}"); ASSERT_EQUALS("", errout.str()); + + ctu("void g(int* x) { *x; }\n" + "void f(int* x) {\n" + " if (x)\n" + " g(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } }; diff --git a/test/testother.cpp b/test/testother.cpp index 389c39b8f..2fa8271a0 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -660,6 +660,16 @@ private: " if (a == 0) {}\n" "}"); ASSERT_EQUALS("", errout.str()); + + check("int g();\n" + "void f(int b) {\n" + " int x = g();\n" + " if (x == 0) {}\n" + " else if (x > 0) {}\n" + " else\n" + " a = b / -x;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void nanInArithmeticExpression() { diff --git a/test/testsimplifytypedef.cpp b/test/testsimplifytypedef.cpp index 9420d1753..47c0c3266 100644 --- a/test/testsimplifytypedef.cpp +++ b/test/testsimplifytypedef.cpp @@ -1152,7 +1152,6 @@ private: ASSERT_EQUALS(expected, tok(code, false)); ASSERT_EQUALS_WITHOUT_LINENUMBERS( "[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n" - "[test.cpp:26]: (debug) valueflow.cpp::valueFlowForward bailout: possible assignment of s by subfunction\n" , errout.str()); } diff --git a/test/testtype.cpp b/test/testtype.cpp index b85211dbe..0ac40e945 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -143,6 +143,47 @@ private: " UINFO(x << 1234);\n" "}"); ASSERT_EQUALS("", errout.str()); + + // #8885 + check("int f(int k, int rm) {\n" + " if (k == 32)\n" + " return 0;\n" + " if (k > 32)\n" + " return 0;\n" + " return rm>> k;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:6]: (warning) Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 4.\n", + errout.str()); + + check("int f(int k, int rm) {\n" + " if (k == 0 || k == 32)\n" + " return 0;\n" + " else if (k > 32)\n" + " return 0;\n" + " else\n" + " return rm>> k;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:7]: (warning) Shifting signed 32-bit value by 31 bits is undefined behaviour. See condition at line 4.\n", + errout.str()); + + check("int f(int k, int rm) {\n" + " if (k == 0 || k == 32 || k == 31)\n" + " return 0;\n" + " else if (k > 32)\n" + " return 0;\n" + " else\n" + " return rm>> k;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("static long long f(int x, long long y) {\n" + " if (x >= 64)\n" + " return 0;\n" + " return -(y << (x-1));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void checkIntegerOverflow() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 87c0955af..8b4eedc32 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -50,6 +50,7 @@ private: " true \n" // abort is a noreturn function ""; settings.library.loadxmldata(cfg, sizeof(cfg)); + LOAD_LIB_2(settings.library, "std.cfg"); TEST_CASE(valueFlowNumber); TEST_CASE(valueFlowString); @@ -158,7 +159,7 @@ private: for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { for (const ValueFlow::Value &v : tok->values()) { - if (v.isIntValue() && v.intvalue == value) + if (v.isIntValue() && !v.isImpossible() && v.intvalue == value) return true; } } @@ -176,7 +177,8 @@ private: for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { if (tok->str() == "x" && tok->linenr() == linenr) { for (const ValueFlow::Value &v : tok->values()) { - if (v.isFloatValue() && v.floatValue >= value - diff && v.floatValue <= value + diff) + if (v.isFloatValue() && !v.isImpossible() && v.floatValue >= value - diff && + v.floatValue <= value + diff) return true; } } @@ -3714,6 +3716,19 @@ private: return ""; } + static std::string isImpossibleContainerSizeValue(const std::list& values, MathLib::bigint i) + { + if (values.size() != 1) + return "values.size():" + std::to_string(values.size()); + if (!values.front().isContainerSizeValue()) + return "ContainerSizeValue"; + if (!values.front().isImpossible()) + return "Impossible"; + if (values.front().intvalue != i) + return "intvalue:" + std::to_string(values.front().intvalue); + return ""; + } + static std::string isKnownContainerSizeValue(const std::list &values, MathLib::bigint i) { if (values.size() != 1) return "values.size():" + std::to_string(values.size()); @@ -3788,7 +3803,7 @@ private: " if (ints.empty()) { continue; }\n" " ints.front();\n" // <- no container size "}"; - ASSERT(tokenValues(code, "ints . front").empty()); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "ints . front"), 0)); code = "void f(const std::list &ints) {\n" " if (ints.empty()) { ints.push_back(0); }\n" @@ -3912,7 +3927,7 @@ private: " if (s != \"hello\")\n" " s[40] = c;\n" "}"; - ASSERT(tokenValues(code, "s [").empty()); + ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "s ["), 5)); // valueFlowContainerForward, loop code = "void f() {\n"