Add impossible values to ValueFlow (#2186)

* Add impossible category

* Replace values

* Try to adjust known values

* Add ! for impossible values

* Add impossible with possible values

* Remove contradictions

* Add values when the branch is not dead

* Only copy possible values

* Dont bail on while loops

* Load std lib in valueflow

* Check for function calls

* Fix stl errors

* Fix incorrect impossible check

* Fix heap-after-use error

* Remove impossible values when they are lowered

* Show the bound and remove overlaps

* Infer conditions

* Dont push pointer values through dynamic_cast

* Add test for dynamic_cast issue

* Add shifttoomanybits test

* Add test for div by zero

* Add a test for issue 9315

* Dont make impossible value inconclusive

* Fix FP with shift operator

* Improve handleKnownValuesInLoop for impossible values

* Fix cppcheck warning

* Fix impossible values for ctu

* Bailout for streams

* Check equality conditions

* Fix overflows

* Add regression test for 9332

* Remove duplicate conditions

* Skip impossible values for invalid value

* Check for null

* Rename bound to range

* Formatting
This commit is contained in:
Paul Fultz II 2019-09-20 08:06:37 -05:00 committed by Daniel Marjamäki
parent dc1cdd2b76
commit ad8abdb0c3
16 changed files with 675 additions and 150 deletions

View File

@ -168,10 +168,10 @@ bool ProjectFile::read(const QString &filename)
mClangTidy = tools.contains(CLANG_TIDY); mClangTidy = tools.contains(CLANG_TIDY);
} }
if (insideProject && xmlReader.name() == CppcheckXml::TagsElementName) if (xmlReader.name() == CppcheckXml::TagsElementName)
readStringList(mTags, xmlReader, CppcheckXml::TagElementName); readStringList(mTags, xmlReader, CppcheckXml::TagElementName);
if (insideProject && xmlReader.name() == CppcheckXml::MaxCtuDepthElementName) if (xmlReader.name() == CppcheckXml::MaxCtuDepthElementName)
mMaxCtuDepth = readInt(xmlReader, mMaxCtuDepth); mMaxCtuDepth = readInt(xmlReader, mMaxCtuDepth);
break; break;

View File

@ -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) void CheckCondition::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result)
{ {
if (tok2 && diag(tok2->tokAt(2)))
return;
std::list<const Token *> locations = { tok1, tok2 }; std::list<const Token *> locations = { tok1, tok2 };
reportError(locations, reportError(locations,
Severity::style, Severity::style,
@ -520,6 +522,8 @@ void CheckCondition::multiCondition()
void CheckCondition::overlappingElseIfConditionError(const Token *tok, nonneg int line1) void CheckCondition::overlappingElseIfConditionError(const Token *tok, nonneg int line1)
{ {
if (diag(tok))
return;
std::ostringstream errmsg; std::ostringstream errmsg;
errmsg << "Expression is always false because 'else if' condition matches previous condition at line " errmsg << "Expression is always false because 'else if' condition matches previous condition at line "
<< line1 << "."; << line1 << ".";
@ -529,6 +533,8 @@ void CheckCondition::overlappingElseIfConditionError(const Token *tok, nonneg in
void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath) void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath)
{ {
if (diag(ifCond) & diag(elseIfCond))
return;
std::ostringstream errmsg; std::ostringstream errmsg;
errmsg << "Expression is always true because 'else if' condition is opposite to previous condition at line " errmsg << "Expression is always true because 'else if' condition is opposite to previous condition at line "
<< ifCond->linenr() << "."; << ifCond->linenr() << ".";

View File

@ -58,11 +58,11 @@ public:
checkCondition.multiCondition2(); checkCondition.multiCondition2();
checkCondition.checkIncorrectLogicOperator(); checkCondition.checkIncorrectLogicOperator();
checkCondition.checkInvalidTestForOverflow(); checkCondition.checkInvalidTestForOverflow();
checkCondition.alwaysTrueFalse();
checkCondition.duplicateCondition(); checkCondition.duplicateCondition();
checkCondition.checkPointerAdditionResultNotNull(); checkCondition.checkPointerAdditionResultNotNull();
checkCondition.checkDuplicateConditionalAssign(); checkCondition.checkDuplicateConditionalAssign();
checkCondition.assignIf(); checkCondition.assignIf();
checkCondition.alwaysTrueFalse();
checkCondition.checkBadBitmaskCheck(); checkCondition.checkBadBitmaskCheck();
checkCondition.comparison(); checkCondition.comparison();
checkCondition.checkModuloAlwaysTrueFalse(); checkCondition.checkModuloAlwaysTrueFalse();

View File

@ -62,6 +62,8 @@ void CheckStl::outOfBounds()
for (const ValueFlow::Value &value : tok->values()) { for (const ValueFlow::Value &value : tok->values()) {
if (!value.isContainerSizeValue()) if (!value.isContainerSizeValue())
continue; continue;
if (value.isImpossible())
continue;
if (value.isInconclusive() && !mSettings->inconclusive) if (value.isInconclusive() && !mSettings->inconclusive)
continue; continue;
if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING))

View File

@ -953,7 +953,7 @@ bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc al
if (!pointer) if (!pointer)
return false; return false;
if (pointer && alloc != CTOR_CALL && Token::Match(vartok, "%name% . %name% (")) if (alloc != CTOR_CALL && Token::Match(vartok, "%name% . %name% ("))
return true; return true;
bool assignment = false; bool assignment = false;

View File

@ -321,6 +321,9 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
for (const ValueFlow::Value &value : argtok->values()) { for (const ValueFlow::Value &value : argtok->values()) {
if ((!value.isIntValue() || value.intvalue != 0 || value.isInconclusive()) && !value.isBufferSizeValue()) if ((!value.isIntValue() || value.intvalue != 0 || value.isInconclusive()) && !value.isBufferSizeValue())
continue; continue;
// Skip impossible values since they cannot be represented
if (value.isImpossible())
continue;
FileInfo::FunctionCall functionCall; FileInfo::FunctionCall functionCall;
functionCall.callValueType = value.valueType; functionCall.callValueType = value.valueType;
functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function()); functionCall.callId = getFunctionId(tokenizer, tok->astOperand1()->function());

View File

@ -1592,6 +1592,8 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
out << " known=\"true\""; out << " known=\"true\"";
else if (value.isPossible()) else if (value.isPossible())
out << " possible=\"true\""; out << " possible=\"true\"";
else if (value.isImpossible())
out << " impossible=\"true\"";
else if (value.isInconclusive()) else if (value.isInconclusive())
out << " inconclusive=\"true\""; out << " inconclusive=\"true\"";
out << "/>" << std::endl; out << "/>" << std::endl;
@ -1600,6 +1602,12 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
else { else {
if (&value != &tok->mImpl->mValues->front()) if (&value != &tok->mImpl->mValues->front())
out << ","; out << ",";
if (value.isImpossible())
out << "!";
if (value.bound == ValueFlow::Value::Bound::Lower)
out << ">";
if (value.bound == ValueFlow::Value::Bound::Upper)
out << "<";
switch (value.valueType) { switch (value.valueType) {
case ValueFlow::Value::INT: case ValueFlow::Value::INT:
if (tok->valueType() && tok->valueType()->sign == ValueType::UNSIGNED) 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; const ValueFlow::Value *ret = nullptr;
std::list<ValueFlow::Value>::const_iterator it; std::list<ValueFlow::Value>::const_iterator it;
for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) { for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) {
if (it->isImpossible())
continue;
if (it->isIntValue() && it->intvalue <= val) { if (it->isIntValue() && it->intvalue <= val) {
if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive()))
ret = &(*it); ret = &(*it);
@ -1673,6 +1683,8 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett
const ValueFlow::Value *ret = nullptr; const ValueFlow::Value *ret = nullptr;
std::list<ValueFlow::Value>::const_iterator it; std::list<ValueFlow::Value>::const_iterator it;
for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) { for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++it) {
if (it->isImpossible())
continue;
if (it->isIntValue() && it->intvalue >= val) { if (it->isIntValue() && it->intvalue >= val) {
if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive()))
ret = &(*it); ret = &(*it);
@ -1696,6 +1708,8 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, nonneg int ar
const ValueFlow::Value *ret = nullptr; const ValueFlow::Value *ret = nullptr;
std::list<ValueFlow::Value>::const_iterator it; std::list<ValueFlow::Value>::const_iterator it;
for (it = mImpl->mValues->begin(); it != mImpl->mValues->end(); ++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)) || if ((it->isIntValue() && !settings->library.isIntArgValid(ftok, argnr, it->intvalue)) ||
(it->isFloatValue() && !settings->library.isFloatArgValid(ftok, argnr, it->floatValue))) { (it->isFloatValue() && !settings->library.isFloatArgValid(ftok, argnr, it->floatValue))) {
if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive()))
@ -1791,6 +1805,77 @@ const Token *Token::getValueTokenDeadPointer() const
return nullptr; return nullptr;
} }
static bool removeContradiction(std::list<ValueFlow::Value>& 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<ValueFlow::Value>& 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<ValueFlow::Value>& values)
{
for (int i = 0; i < 4; i++) {
if (!removeContradiction(values))
return;
removeOverlaps(values);
}
}
bool Token::addValue(const ValueFlow::Value &value) bool Token::addValue(const ValueFlow::Value &value)
{ {
if (value.isKnown() && mImpl->mValues) { if (value.isKnown() && mImpl->mValues) {
@ -1813,6 +1898,9 @@ bool Token::addValue(const ValueFlow::Value &value)
if (it->valueType != value.valueType) if (it->valueType != value.valueType)
continue; continue;
if (it->isImpossible() != value.isImpossible())
continue;
// different value => continue // different value => continue
bool differentValue = true; bool differentValue = true;
switch (it->valueType) { switch (it->valueType) {
@ -1843,7 +1931,7 @@ bool Token::addValue(const ValueFlow::Value &value)
continue; continue;
// same value, but old value is inconclusive so replace it // same value, but old value is inconclusive so replace it
if (it->isInconclusive() && !value.isInconclusive()) { if (it->isInconclusive() && !value.isInconclusive() && !value.isImpossible()) {
*it = value; *it = value;
if (it->varId == 0) if (it->varId == 0)
it->varId = mImpl->mVarId; it->varId = mImpl->mVarId;
@ -1871,6 +1959,8 @@ bool Token::addValue(const ValueFlow::Value &value)
mImpl->mValues = new std::list<ValueFlow::Value>(1, v); mImpl->mValues = new std::list<ValueFlow::Value>(1, v);
} }
removeContradictions(*mImpl->mValues);
return true; return true;
} }

View File

@ -988,8 +988,8 @@ public:
const ValueFlow::Value * getValue(const MathLib::bigint val) const { const ValueFlow::Value * getValue(const MathLib::bigint val) const {
if (!mImpl->mValues) if (!mImpl->mValues)
return nullptr; return nullptr;
const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value &value) { const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
return value.isIntValue() && value.intvalue == val; return value.isIntValue() && !value.isImpossible() && value.intvalue == val;
}); });
return it == mImpl->mValues->end() ? nullptr : &*it;; return it == mImpl->mValues->end() ? nullptr : &*it;;
} }
@ -1001,6 +1001,8 @@ public:
for (const ValueFlow::Value &value : *mImpl->mValues) { for (const ValueFlow::Value &value : *mImpl->mValues) {
if (!value.isIntValue()) if (!value.isIntValue())
continue; continue;
if (value.isImpossible())
continue;
if ((!ret || value.intvalue > ret->intvalue) && if ((!ret || value.intvalue > ret->intvalue) &&
((value.condition != nullptr) == condition)) ((value.condition != nullptr) == condition))
ret = &value; ret = &value;
@ -1011,8 +1013,9 @@ public:
const ValueFlow::Value * getMovedValue() const { const ValueFlow::Value * getMovedValue() const {
if (!mImpl->mValues) if (!mImpl->mValues)
return nullptr; return nullptr;
const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value &value) { 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; return value.isMovedValue() && !value.isImpossible() &&
value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable;
}); });
return it == mImpl->mValues->end() ? nullptr : &*it;; return it == mImpl->mValues->end() ? nullptr : &*it;;
} }
@ -1025,8 +1028,8 @@ public:
const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const { const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const {
if (!mImpl->mValues) if (!mImpl->mValues)
return nullptr; return nullptr;
const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value &value) { const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) {
return value.isContainerSizeValue() && value.intvalue == val; return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val;
}); });
return it == mImpl->mValues->end() ? nullptr : &*it; return it == mImpl->mValues->end() ? nullptr : &*it;
} }

View File

@ -189,6 +189,53 @@ static void changeKnownToPossible(std::list<ValueFlow::Value> &values, int indir
} }
} }
static void removeImpossible(std::list<ValueFlow::Value>& 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<ValueFlow::Value>& values, int indirect = -1)
{
changeKnownToPossible(values, indirect);
removeImpossible(values, indirect);
}
static void lowerToInconclusive(std::list<ValueFlow::Value>& 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<ValueFlow::Value>& 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? * Is condition always false when variable has given value?
* \param condition top ast token in condition * \param condition top ast token in condition
@ -229,6 +276,23 @@ static bool conditionIsTrue(const Token *condition, const ProgramMemory &program
return !error && result == 1; 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, static void setConditionalValues(const Token *tok,
bool invert, bool invert,
MathLib::bigint value, MathLib::bigint value,
@ -237,20 +301,37 @@ static void setConditionalValues(const Token *tok,
{ {
if (Token::Match(tok, "==|!=|>=|<=")) { if (Token::Match(tok, "==|!=|>=|<=")) {
true_value = ValueFlow::Value{tok, value}; true_value = ValueFlow::Value{tok, value};
false_value = ValueFlow::Value{tok, value}; const char* greaterThan = ">=";
return; const char* lessThan = "<=";
} if (invert)
const char *greaterThan = ">"; std::swap(greaterThan, lessThan);
const char *lessThan = "<"; if (Token::simpleMatch(tok, greaterThan)) {
if (invert) false_value = ValueFlow::Value{tok, value - 1};
std::swap(greaterThan, lessThan); } else if (Token::simpleMatch(tok, lessThan)) {
if (Token::simpleMatch(tok, greaterThan)) { false_value = ValueFlow::Value{tok, value + 1};
true_value = ValueFlow::Value{tok, value + 1}; } else {
false_value = ValueFlow::Value{tok, value}; false_value = ValueFlow::Value{tok, value};
} else if (Token::simpleMatch(tok, lessThan)) { }
true_value = ValueFlow::Value{tok, value - 1}; } else {
false_value = ValueFlow::Value{tok, value}; 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<MathLib::bigint>::max() || value == std::numeric_limits<MathLib::bigint>::min();
} }
static const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, ValueFlow::Value &false_value) 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; return nullptr;
if (Token::Match(tok, "%comp%")) { if (Token::Match(tok, "%comp%")) {
if (tok->astOperand1()->hasKnownIntValue()) { 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(); return tok->astOperand2();
} else if (tok->astOperand2()->hasKnownIntValue()) { } 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(); return tok->astOperand1();
} }
} }
@ -507,6 +594,8 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl
{ {
if (value1.isKnown() && value2.isKnown()) if (value1.isKnown() && value2.isKnown())
result->setKnown(); result->setKnown();
else if (value1.isImpossible() || value2.isImpossible())
result->setImpossible();
else if (value1.isInconclusive() || value2.isInconclusive()) else if (value1.isInconclusive() || value2.isInconclusive())
result->setInconclusive(); result->setInconclusive();
else else
@ -618,6 +707,9 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
// cast.. // cast..
if (const Token *castType = getCastTypeStartToken(parent)) { 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); const ValueType &valueType = ValueType::parseDecl(castType, settings);
setTokenValueCast(parent, valueType, value, settings); setTokenValueCast(parent, valueType, value, settings);
} }
@ -674,6 +766,10 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
parent->astOperand1() && parent->astOperand1() &&
parent->astOperand2()) { parent->astOperand2()) {
// Dont compare impossible values
if (parent->isComparisonOp() && value.isImpossible())
return;
// known result when a operand is 0. // known result when a operand is 0.
if (Token::Match(parent, "[&*]") && value.isKnown() && value.isIntValue() && value.intvalue==0) { if (Token::Match(parent, "[&*]") && value.isKnown() && value.isIntValue() && value.intvalue==0) {
setTokenValue(parent, value, settings); setTokenValue(parent, value, settings);
@ -1835,6 +1931,9 @@ static void valueFlowReverse(TokenList *tokenlist,
bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction"); bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by subfunction");
break; break;
} }
// Impossible values cant be inconclusive
if (val.isImpossible() || val2.isImpossible())
break;
val.setInconclusive(inconclusive); val.setInconclusive(inconclusive);
val2.setInconclusive(inconclusive); val2.setInconclusive(inconclusive);
@ -2048,14 +2147,8 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
val2.varId = varid; val2.varId = varid;
} }
} }
valueFlowReverse(tokenlist, Token* startTok = tok->astParent() ? tok->astParent() : tok->previous();
tok, valueFlowReverse(tokenlist, startTok, vartok, val, val2, errorLogger, settings);
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); const bool isChanged = isVariableChanged(startToken, endToken, varid, globalvar, settings, true);
if (!isChanged) if (!isChanged)
return false; return false;
for (std::list<ValueFlow::Value>::iterator it = values->begin(); it != values->end(); ++it) { lowerToPossible(*values);
if (it->isKnown()) {
it->setPossible();
}
}
return isChanged; return isChanged;
} }
@ -2224,6 +2313,8 @@ static bool valueFlowForward(Token * const startToken,
return true; return true;
for (Token *tok2 = startToken; tok2 && tok2 != endToken; tok2 = tok2->next()) { for (Token *tok2 = startToken; tok2 && tok2 != endToken; tok2 = tok2->next()) {
if (values.empty())
return true;
if (indentlevel >= 0 && tok2->str() == "{") if (indentlevel >= 0 && tok2->str() == "{")
++indentlevel; ++indentlevel;
else if (indentlevel >= 0 && tok2->str() == "}") { else if (indentlevel >= 0 && tok2->str() == "}") {
@ -2271,7 +2362,7 @@ static bool valueFlowForward(Token * const startToken,
Token::simpleMatch(tok2->link()->previous(), "else {") && Token::simpleMatch(tok2->link()->previous(), "else {") &&
!isReturnScope(tok2->link()->tokAt(-2), settings) && !isReturnScope(tok2->link()->tokAt(-2), settings) &&
isVariableChanged(tok2->link(), tok2, varid, var->isGlobal(), settings, tokenlist->isCPP())) { 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") { if (Token::Match(tok2, "[;{}] %name% :") || tok2->str() == "case") {
changeKnownToPossible(values); lowerToPossible(values);
tok2 = tok2->tokAt(2); tok2 = tok2->tokAt(2);
continue; continue;
} }
@ -2418,7 +2509,7 @@ static bool valueFlowForward(Token * const startToken,
if (!condAlwaysFalse && isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { if (!condAlwaysFalse && isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
removeValues(values, truevalues); removeValues(values, truevalues);
changeKnownToPossible(values); lowerToPossible(values);
} }
// goto '}' // goto '}'
@ -2446,7 +2537,7 @@ static bool valueFlowForward(Token * const startToken,
if (!condAlwaysTrue && isVariableChanged(startTokenElse, startTokenElse->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { if (!condAlwaysTrue && isVariableChanged(startTokenElse, startTokenElse->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
removeValues(values, falsevalues); removeValues(values, falsevalues);
changeKnownToPossible(values); lowerToPossible(values);
} }
// goto '}' // goto '}'
@ -2519,7 +2610,7 @@ static bool valueFlowForward(Token * const startToken,
// Remove conditional values // Remove conditional values
std::list<ValueFlow::Value>::iterator it; std::list<ValueFlow::Value>::iterator it;
for (it = values.begin(); it != values.end();) { for (it = values.begin(); it != values.end();) {
if (it->condition || it->conditional) if (it->condition || it->conditional || it->isImpossible())
values.erase(it++); values.erase(it++);
else { else {
it->changeKnownToPossible(); it->changeKnownToPossible();
@ -2607,6 +2698,7 @@ static bool valueFlowForward(Token * const startToken,
++number_of_if; ++number_of_if;
// Set "conditional" flag for all values // Set "conditional" flag for all values
removeImpossible(values);
std::list<ValueFlow::Value>::iterator it; std::list<ValueFlow::Value>::iterator it;
for (it = values.begin(); it != values.end(); ++it) { for (it = values.begin(); it != values.end(); ++it) {
it->conditional = true; it->conditional = true;
@ -2641,7 +2733,7 @@ static bool valueFlowForward(Token * const startToken,
if (tok2 == endToken) if (tok2 == endToken)
break; break;
--indentlevel; --indentlevel;
changeKnownToPossible(values); lowerToPossible(values);
continue; continue;
} }
} }
@ -2675,9 +2767,9 @@ static bool valueFlowForward(Token * const startToken,
for (const ValueFlow::Value &v : values) for (const ValueFlow::Value &v : values)
valueFlowAST(expr, varid, v, settings); valueFlowAST(expr, varid, v, settings);
if (isVariableChangedByFunctionCall(expr, 0, varid, settings, nullptr)) if (isVariableChangedByFunctionCall(expr, 0, varid, settings, nullptr))
changeKnownToPossible(values, 0); lowerToPossible(values, 0);
if (isVariableChangedByFunctionCall(expr, 1, varid, settings, nullptr)) if (isVariableChangedByFunctionCall(expr, 1, varid, settings, nullptr))
changeKnownToPossible(values, 1); lowerToPossible(values, 1);
} else { } else {
for (const ValueFlow::Value &v : values) { for (const ValueFlow::Value &v : values) {
const ProgramMemory programMemory(getProgramMemory(tok2, varid, v)); const ProgramMemory programMemory(getProgramMemory(tok2, varid, v));
@ -2704,7 +2796,7 @@ static bool valueFlowForward(Token * const startToken,
} }
if (changed0 || changed1) if (changed0 || changed1)
changeKnownToPossible(values); lowerToPossible(values);
} }
// Skip conditional expressions.. // Skip conditional expressions..
@ -2871,6 +2963,13 @@ static bool valueFlowForward(Token * const startToken,
return false; 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? // assigned by subfunction?
for (int i:getIndirections(values)) { for (int i:getIndirections(values)) {
bool inconclusive = false; bool inconclusive = false;
@ -2879,20 +2978,8 @@ static bool valueFlowForward(Token * const startToken,
return v.indirect <= i; return v.indirect <= i;
}); });
} }
if (inconclusive) { if (inconclusive)
if (settings->inconclusive) { lowerToInconclusive(values, settings, i);
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 (values.empty()) { if (values.empty()) {
if (settings->debugwarnings) if (settings->debugwarnings)
@ -2900,10 +2987,8 @@ static bool valueFlowForward(Token * const startToken,
return false; return false;
} }
if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") { if (tok2->strAt(1) == "." && tok2->next()->originalName() != "->") {
if (settings->inconclusive) { lowerToInconclusive(values, settings);
for (ValueFlow::Value &v : values) if (!settings->inconclusive) {
v.setInconclusive();
} else {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by member function"); bailout(tokenlist, errorLogger, tok2, "possible assignment of " + tok2->str() + " by member function");
return false; return false;
@ -3315,7 +3400,7 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
// Static variable initialisation? // Static variable initialisation?
if (var->isStatic() && var->nameToken() == parent->astOperand1()) if (var->isStatic() && var->nameToken() == parent->astOperand1())
changeKnownToPossible(values); lowerToPossible(values);
// Skip RHS // Skip RHS
const Token *nextExpression = nextAfterAstRightmostLeaf(parent); const Token *nextExpression = nextAfterAstRightmostLeaf(parent);
@ -4004,7 +4089,7 @@ static void valueFlowForwardAssign(Token * const tok,
// Static variable initialisation? // Static variable initialisation?
if (var->isStatic() && init) if (var->isStatic() && init)
changeKnownToPossible(values); lowerToPossible(values);
// Skip RHS // Skip RHS
const Token * nextExpression = tok->astParent() ? nextAfterAstRightmostLeaf(tok->astParent()) : tok->next(); 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<ValueFlow::Value>& 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 = "||"; const char * op = "||";
if (then) if (then)
op = "&&"; op = "&&";
const Token* parent = tok->astParent(); const Token* parent = tok->astParent();
while (parent && parent->str() == op) while (parent && parent->str() == op)
parent = parent->astParent(); parent = parent->astParent();
if (parent && parent->str() == "(") return (parent && parent->str() == "(");
values.front().setKnown(); }
static void valueFlowSetConditionToKnown(const Token* tok, std::list<ValueFlow::Value>& 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) 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); 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<ValueFlow::Value>& values, const std::list<ValueFlow::Value>& input)
{
std::transform(input.begin(), input.end(), std::back_inserter(values), &asImpossible);
}
struct ValueFlowConditionHandler { struct ValueFlowConditionHandler {
struct Condition { struct Condition {
const Token *vartok; const Token *vartok;
@ -4212,19 +4312,22 @@ struct ValueFlowConditionHandler {
continue; continue;
} }
std::list<ValueFlow::Value> thenValues;
std::list<ValueFlow::Value> 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 // start token of conditional code
Token *startTokens[] = {nullptr, nullptr}; 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;
// if astParent is "!" we need to invert codeblock // if astParent is "!" we need to invert codeblock
{ {
@ -4234,50 +4337,41 @@ struct ValueFlowConditionHandler {
while (parent && parent->str() == "&&") while (parent && parent->str() == "&&")
parent = parent->astParent(); parent = parent->astParent();
if (parent && (parent->str() == "!" || Token::simpleMatch(parent, "== false"))) { if (parent && (parent->str() == "!" || Token::simpleMatch(parent, "== false"))) {
check_if = !check_if; std::swap(thenValues, elseValues);
check_else = !check_else;
std::swap(cond.true_values, cond.false_values);
} }
tok2 = parent; tok2 = parent;
} }
} }
if (cond.true_values != cond.false_values) {
check_if = true;
check_else = true;
}
// determine startToken(s) // determine startToken(s)
if (check_if && Token::simpleMatch(top->link(), ") {")) if (Token::simpleMatch(top->link(), ") {"))
startTokens[0] = top->link()->next(); 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); startTokens[1] = top->link()->linkAt(1)->tokAt(2);
bool bail = false; int changeBlock = -1;
const bool bothCanBeKnown = check_if && check_else && !Token::Match(tok->astParent(), "&&|%oror%");
for (int i = 0; i < 2; i++) { for (int i = 0; i < 2; i++) {
const Token *const startToken = startTokens[i]; const Token *const startToken = startTokens[i];
if (!startToken) if (!startToken)
continue; continue;
std::list<ValueFlow::Value> &values = (i == 0 ? cond.true_values : cond.false_values); std::list<ValueFlow::Value>& values = (i == 0 ? thenValues : elseValues);
valueFlowSetConditionToKnown(tok, values, i == 0); valueFlowSetConditionToKnown(tok, values, i == 0);
if (bothCanBeKnown)
valueFlowSetConditionToKnown(tok, values, i != 0);
bool changed = forward(startTokens[i], startTokens[i]->link(), var, values, true); // TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call
values.front().setPossible(); if (forward(startTokens[i], startTokens[i]->link(), var, values, true))
if (changed) { changeBlock = i;
// TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call changeKnownToPossible(values);
if (settings->debugwarnings)
bailout(tokenlist,
errorLogger,
startTokens[i]->link(),
"valueFlowAfterCondition: " + var->name() + " is changed in conditional block");
bail = true;
}
} }
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; continue;
}
// After conditional code.. // After conditional code..
if (Token::simpleMatch(top->link(), ") {")) { if (Token::simpleMatch(top->link(), ") {")) {
@ -4304,21 +4398,34 @@ struct ValueFlowConditionHandler {
dead_else = isReturnScope(after, settings); dead_else = isReturnScope(after, settings);
} }
std::list<ValueFlow::Value> *values = nullptr; if (dead_if && dead_else)
if (!dead_if && check_if) continue;
values = &cond.true_values;
else if (!dead_else && check_else)
values = &cond.false_values;
if (values) { std::list<ValueFlow::Value> 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(), "&&|&")) { if ((dead_if || dead_else) && !Token::Match(tok->astParent(), "&&|&")) {
valueFlowSetConditionToKnown(tok, *values, true); valueFlowSetConditionToKnown(tok, values, true);
valueFlowSetConditionToKnown(tok, *values, false); valueFlowSetConditionToKnown(tok, values, false);
} }
// TODO: constValue could be true if there are no assignments in the conditional blocks and // 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 // perhaps if there are no && and no || in the condition
bool constValue = false; 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(); vartok = vartok->astOperand1();
if (!vartok->isName()) if (!vartok->isName())
return cond; 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.true_values.push_back(true_value);
cond.false_values.push_back(false_value); cond.false_values.push_back(false_value);
cond.vartok = vartok; cond.vartok = vartok;
return cond; return cond;
} }
long long falseIntValue = 0LL;
if (tok->str() == "!") { if (tok->str() == "!") {
vartok = tok->astOperand1(); vartok = tok->astOperand1();
if (astIsPointer(vartok))
falseIntValue = 1LL;
} else if (tok->isName() && (Token::Match(tok->astParent(), "%oror%|&&") || } else if (tok->isName() && (Token::Match(tok->astParent(), "%oror%|&&") ||
Token::Match(tok->tokAt(-2), "if|while ( %var% [)=]"))) { Token::Match(tok->tokAt(-2), "if|while ( %var% [)=]"))) {
@ -4379,7 +4476,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist,
if (!vartok || !vartok->isName()) if (!vartok || !vartok->isName())
return cond; return cond;
cond.true_values.emplace_back(tok, 0LL); cond.true_values.emplace_back(tok, 0LL);
cond.false_values.emplace_back(tok, falseIntValue); cond.false_values.emplace_back(tok, 0LL);
cond.vartok = vartok; cond.vartok = vartok;
return cond; return cond;
@ -4580,6 +4677,89 @@ static void execute(const Token *expr,
*error = true; *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<ValueFlow::Value>& 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) 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); tok = tok->tokAt(2);
@ -5163,7 +5343,7 @@ static void valueFlowSubFunction(TokenList* tokenlist, ErrorLogger* errorLogger,
} }
// passed values are not "known".. // passed values are not "known"..
changeKnownToPossible(argvalues); lowerToPossible(argvalues);
valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues);
// FIXME: We need to rewrite the valueflow analysis to better handle multiple arguments // 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), : valueType(INT),
bound(Bound::Point),
intvalue(val), intvalue(val),
tokvalue(nullptr), tokvalue(nullptr),
floatValue(0.0), floatValue(0.0),
@ -6130,6 +6311,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings);
valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings);
valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
valueFlowInferCondition(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings);
valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSubFunction(tokenlist, errorLogger, settings); valueFlowSubFunction(tokenlist, errorLogger, settings);

View File

@ -37,6 +37,20 @@ class TokenList;
class Variable; class Variable;
namespace ValueFlow { namespace ValueFlow {
struct increment {
template <class T>
void operator()(T& x) const
{
x++;
}
};
struct decrement {
template <class T>
void operator()(T& x) const
{
x--;
}
};
class CPPCHECKLIB Value { class CPPCHECKLIB Value {
public: public:
typedef std::pair<const Token *, std::string> ErrorPathItem; typedef std::pair<const Token *, std::string> ErrorPathItem;
@ -44,6 +58,7 @@ namespace ValueFlow {
explicit Value(long long val = 0) explicit Value(long long val = 0)
: valueType(ValueType::INT), : valueType(ValueType::INT),
bound(Bound::Point),
intvalue(val), intvalue(val),
tokvalue(nullptr), tokvalue(nullptr),
floatValue(0.0), floatValue(0.0),
@ -99,6 +114,28 @@ namespace ValueFlow {
return true; return true;
} }
template <class F>
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 { bool operator==(const Value &rhs) const {
if (!equalValue(rhs)) if (!equalValue(rhs))
return false; return false;
@ -116,6 +153,23 @@ namespace ValueFlow {
return !(*this == rhs); 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; std::string infoString() const;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE } valueType; enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE } valueType;
@ -156,6 +210,9 @@ namespace ValueFlow {
return isMovedValue() || isUninitValue() || isLifetimeValue(); return isMovedValue() || isUninitValue() || isLifetimeValue();
} }
/** The value bound */
enum class Bound { Upper, Lower, Point } bound;
/** int value */ /** int value */
long long intvalue; long long intvalue;
@ -213,7 +270,9 @@ namespace ValueFlow {
/** Only listed values are possible */ /** Only listed values are possible */
Known, Known,
/** Inconclusive */ /** Inconclusive */
Inconclusive Inconclusive,
/** Listed values are impossible */
Impossible
} valueKind; } valueKind;
void setKnown() { void setKnown() {
@ -232,6 +291,10 @@ namespace ValueFlow {
return valueKind == ValueKind::Possible; return valueKind == ValueKind::Possible;
} }
bool isImpossible() const { return valueKind == ValueKind::Impossible; }
void setImpossible() { valueKind = ValueKind::Impossible; }
void setInconclusive(bool inconclusive = true) { void setInconclusive(bool inconclusive = true) {
if (inconclusive) if (inconclusive)
valueKind = ValueKind::Inconclusive; valueKind = ValueKind::Inconclusive;

View File

@ -244,7 +244,9 @@ private:
" while (y != 0) g(y);\n" " while (y != 0) g(y);\n"
" }\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" check("void g(int &x);\n"
"void f(int x) {\n" "void f(int x) {\n"
@ -538,14 +540,14 @@ private:
" else { if (a == 2) { b = 2; }\n" " else { if (a == 2) { b = 2; }\n"
" else { if (a == 1) { b = 3; } } }\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" check("void f(int a, int &b) {\n"
" if (a == 1) { b = 1; }\n" " if (a == 1) { b = 1; }\n"
" else { if (a == 2) { b = 2; }\n" " else { if (a == 2) { b = 2; }\n"
" else { if (a == 2) { b = 3; } } }\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" check("void f(int a, int &b) {\n"
" if (a++) { b = 1; }\n" " if (a++) { b = 1; }\n"
@ -2062,10 +2064,10 @@ private:
check("void f(int x) {\n" check("void f(int x) {\n"
"\n" "\n"
" if (x<4) {\n" " if (x<4) {\n"
" if (x!=5) {}\n" // <- TODO " if (x!=5) {}\n"
" }\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" check("void f(int x) {\n"
"\n" "\n"
" if (x<4) {\n" " if (x<4) {\n"
@ -3229,6 +3231,62 @@ private:
" if (x) {}\n" " if (x) {}\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); 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<B*>(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() { void alwaysTrueContainer() {

View File

@ -76,6 +76,8 @@ private:
TEST_CASE(nullpointer34); TEST_CASE(nullpointer34);
TEST_CASE(nullpointer35); TEST_CASE(nullpointer35);
TEST_CASE(nullpointer36); // #9264 TEST_CASE(nullpointer36); // #9264
TEST_CASE(nullpointer37); // #9315
TEST_CASE(nullpointer38);
TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointer_addressOf); // address of
TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointerSwitch); // #2626
TEST_CASE(nullpointer_cast); // #4692 TEST_CASE(nullpointer_cast); // #4692
@ -92,6 +94,7 @@ private:
TEST_CASE(nullpointer_in_typeid); TEST_CASE(nullpointer_in_typeid);
TEST_CASE(nullpointer_in_for_loop); TEST_CASE(nullpointer_in_for_loop);
TEST_CASE(nullpointerDelete); TEST_CASE(nullpointerDelete);
TEST_CASE(nullpointerSubFunction);
TEST_CASE(nullpointerExit); TEST_CASE(nullpointerExit);
TEST_CASE(nullpointerStdString); TEST_CASE(nullpointerStdString);
TEST_CASE(nullpointerStdStream); TEST_CASE(nullpointerStdStream);
@ -1440,6 +1443,39 @@ private:
ASSERT_EQUALS("", errout.str()); 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<int*> v;\n"
" if (x) {\n"
" v.push_back(x);\n"
" *x;\n"
" }\n"
"}\n",
true);
ASSERT_EQUALS("", errout.str());
}
void nullpointer_addressOf() { // address of void nullpointer_addressOf() { // address of
check("void f() {\n" check("void f() {\n"
" struct X *x = 0;\n" " struct X *x = 0;\n"
@ -2107,6 +2143,16 @@ private:
ASSERT_EQUALS("", errout.str()); 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() { void nullpointerExit() {
check("void f() {\n" check("void f() {\n"
" K *k = getK();\n" " K *k = getK();\n"
@ -2915,6 +2961,13 @@ private:
" dostuff(0, 0);\n" " dostuff(0, 0);\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); 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());
} }
}; };

View File

@ -660,6 +660,16 @@ private:
" if (a == 0) {}\n" " if (a == 0) {}\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); 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() { void nanInArithmeticExpression() {

View File

@ -1152,7 +1152,6 @@ private:
ASSERT_EQUALS(expected, tok(code, false)); ASSERT_EQUALS(expected, tok(code, false));
ASSERT_EQUALS_WITHOUT_LINENUMBERS( ASSERT_EQUALS_WITHOUT_LINENUMBERS(
"[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n" "[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()); , errout.str());
} }

View File

@ -143,6 +143,47 @@ private:
" UINFO(x << 1234);\n" " UINFO(x << 1234);\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); 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() { void checkIntegerOverflow() {

View File

@ -50,6 +50,7 @@ private:
" <function name=\"abort\"> <noreturn>true</noreturn> </function>\n" // abort is a noreturn function " <function name=\"abort\"> <noreturn>true</noreturn> </function>\n" // abort is a noreturn function
"</def>"; "</def>";
settings.library.loadxmldata(cfg, sizeof(cfg)); settings.library.loadxmldata(cfg, sizeof(cfg));
LOAD_LIB_2(settings.library, "std.cfg");
TEST_CASE(valueFlowNumber); TEST_CASE(valueFlowNumber);
TEST_CASE(valueFlowString); TEST_CASE(valueFlowString);
@ -158,7 +159,7 @@ private:
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) { if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value &v : tok->values()) { for (const ValueFlow::Value &v : tok->values()) {
if (v.isIntValue() && v.intvalue == value) if (v.isIntValue() && !v.isImpossible() && v.intvalue == value)
return true; return true;
} }
} }
@ -176,7 +177,8 @@ private:
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) { if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value &v : tok->values()) { 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; return true;
} }
} }
@ -3714,6 +3716,19 @@ private:
return ""; return "";
} }
static std::string isImpossibleContainerSizeValue(const std::list<ValueFlow::Value>& 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<ValueFlow::Value> &values, MathLib::bigint i) { static std::string isKnownContainerSizeValue(const std::list<ValueFlow::Value> &values, MathLib::bigint i) {
if (values.size() != 1) if (values.size() != 1)
return "values.size():" + std::to_string(values.size()); return "values.size():" + std::to_string(values.size());
@ -3788,7 +3803,7 @@ private:
" if (ints.empty()) { continue; }\n" " if (ints.empty()) { continue; }\n"
" ints.front();\n" // <- no container size " 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<int> &ints) {\n" code = "void f(const std::list<int> &ints) {\n"
" if (ints.empty()) { ints.push_back(0); }\n" " if (ints.empty()) { ints.push_back(0); }\n"
@ -3912,7 +3927,7 @@ private:
" if (s != \"hello\")\n" " if (s != \"hello\")\n"
" s[40] = c;\n" " s[40] = c;\n"
"}"; "}";
ASSERT(tokenValues(code, "s [").empty()); ASSERT_EQUALS("", isImpossibleContainerSizeValue(tokenValues(code, "s ["), 5));
// valueFlowContainerForward, loop // valueFlowContainerForward, loop
code = "void f() {\n" code = "void f() {\n"