walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 6890: ValueFlow: min/max value for variable, after condition (#2460) 

* Set bounds when combining values

* Adust bounds when they are negated

* Try to infer conditional values

* Switch false and true

* Fix checking of conditions

* Fix compare

* Fix remaining tests

* Fix overflows
This commit is contained in:
Paul Fultz II 2019-12-26 08:47:53 -06:00 committed by Daniel Marjamäki
parent 8c652afd6e
commit ce1fc56e96
3 changed files with 219 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
lib/valueflow.cpp
View File

@ -369,9 +369,14 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl
result->varvalue = (result->varId == value1.varId) ? value1.varvalue : value2.varvalue;
result->errorPath = (value1.errorPath.empty() ? value2 : value1).errorPath;
result->safe = value1.safe || value2.safe;
if (value1.bound == ValueFlow::Value::Bound::Point || value2.bound == ValueFlow::Value::Bound::Point) {
if (value1.bound == ValueFlow::Value::Bound::Upper || value2.bound == ValueFlow::Value::Bound::Upper)
result->bound = ValueFlow::Value::Bound::Upper;
if (value1.bound == ValueFlow::Value::Bound::Lower || value2.bound == ValueFlow::Value::Bound::Lower)
result->bound = ValueFlow::Value::Bound::Lower;
}
}
static const Token *getCastTypeStartToken(const Token *parent)
{
// TODO: This might be a generic utility function?
@ -598,6 +603,9 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
} else {
result.intvalue = value1.intvalue - value2.intvalue;
}
// If the bound comes from the second value then invert the bound
if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point)
result.invertBound();
setTokenValue(parent, result, settings);
break;
case '*':
@ -761,6 +769,7 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
v.intvalue = -v.intvalue;
else
v.floatValue = -v.floatValue;
v.invertBound();
setTokenValue(parent, v, settings);
}
}
@ -4532,6 +4541,45 @@ static bool isInBounds(const ValueFlow::Value& value, MathLib::bigint x)
return true;
}
template<class Compare>
static const ValueFlow::Value* getCompareIntValue(const std::list<ValueFlow::Value>& values, Compare compare)
{
const ValueFlow::Value* result = nullptr;
for (const ValueFlow::Value& value : values) {
if (!value.isIntValue())
continue;
if (result)
result = &std::min(value, *result, [&](const ValueFlow::Value& x, const ValueFlow::Value& y) {
return compare(x.intvalue, y.intvalue);
});
else
result = &value;
}
return result;
}
static const ValueFlow::Value* proveLessThan(const std::list<ValueFlow::Value>& values, MathLib::bigint x)
{
const ValueFlow::Value* result = nullptr;
const ValueFlow::Value* maxValue = getCompareIntValue(values, std::greater<MathLib::bigint>{});
if (maxValue && maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower) {
if (maxValue->intvalue <= x)
result = maxValue;
}
return result;
}
static const ValueFlow::Value* proveGreaterThan(const std::list<ValueFlow::Value>& values, MathLib::bigint x)
{
const ValueFlow::Value* result = nullptr;
const ValueFlow::Value* minValue = getCompareIntValue(values, std::less<MathLib::bigint>{});
if (minValue && minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper) {
if (minValue->intvalue >= x)
result = minValue;
}
return result;
}
static const ValueFlow::Value* proveNotEqual(const std::list<ValueFlow::Value>& values, MathLib::bigint x)
{
const ValueFlow::Value* result = nullptr;
@ -4572,9 +4620,10 @@ static void valueFlowInferCondition(TokenList* tokenlist,
continue;
ValueFlow::Value value = *result;
value.intvalue = 1;
value.bound = ValueFlow::Value::Bound::Point;
value.setKnown();
setTokenValue(tok, value, settings);
} else if (Token::Match(tok, "==|!=")) {
} else if (Token::Match(tok, "%comp%")) {
MathLib::bigint val = 0;
const Token* varTok = nullptr;
if (tok->astOperand1()->hasKnownIntValue()) {
@ -4588,11 +4637,33 @@ static void valueFlowInferCondition(TokenList* tokenlist,
continue;
if (varTok->hasKnownIntValue())
continue;
const ValueFlow::Value* result = proveNotEqual(varTok->values(), val);
if (varTok->values().empty())
continue;
const ValueFlow::Value* result = nullptr;
bool known = false;
if (Token::Match(tok, "==|!=")) {
result = proveNotEqual(varTok->values(), val);
known = tok->str() == "!=";
} else if (Token::Match(tok, "<|>=")) {
result = proveLessThan(varTok->values(), val);
known = tok->str() == "<";
if (!result && !isSaturated(val)) {
result = proveGreaterThan(varTok->values(), val - 1);
known = tok->str() == ">=";
}
} else if (Token::Match(tok, ">|<=")) {
result = proveGreaterThan(varTok->values(), val);
known = tok->str() == ">";
if (!result && !isSaturated(val)) {
result = proveLessThan(varTok->values(), val + 1);
known = tok->str() == "<=";
}
}
if (!result)
continue;
ValueFlow::Value value = *result;
value.intvalue = tok->str() == "!=";
value.intvalue = known;
value.bound = ValueFlow::Value::Bound::Point;
value.setKnown();
setTokenValue(tok, value, settings);
}

6
lib/valueflow.h
View File

@ -158,11 +158,15 @@ namespace ValueFlow {
visitValue(decrement{});
}
void invertRange() {
void invertBound() {
if (bound == Bound::Lower)
bound = Bound::Upper;
else if (bound == Bound::Upper)
bound = Bound::Lower;
}
void invertRange() {
invertBound();
decreaseRange();
}

168
test/testcondition.cpp
View File

@ -109,6 +109,7 @@ private:
TEST_CASE(clarifyCondition8);
TEST_CASE(alwaysTrue);
TEST_CASE(alwaysTrueInfer);
TEST_CASE(multiConditionAlwaysTrue);
TEST_CASE(duplicateCondition);
@ -2090,14 +2091,14 @@ private:
" 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"
" 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"
@ -2116,10 +2117,17 @@ private:
check("void f(int x) {\n"
"\n"
" if (x<5) {\n"
" if (x>4) {}\n" // <- TODO
" if (x!=6) {}\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x!=6' is always true\n", errout.str());
check("void f(int x) {\n"
"\n"
" if (x<5) {\n"
" if (x>4) {}\n" // <- Warning
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x>4' is always false\n", errout.str());
check("void f(int x) {\n"
"\n"
" if (x<5) {\n"
@ -2140,7 +2148,7 @@ private:
" if (x<=4) {}\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x<=4' is always true\n", errout.str());
// first comparison: >
check("void f(int x) {\n"
@ -2160,17 +2168,17 @@ private:
check("void f(int x) {\n"
"\n"
" if (x>4) {\n"
" if (x>=5) {}\n" // <- TODO
" if (x>=5) {}\n" // <- Warning
" }\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"
" if (x<5) {}\n" // <- TODO
" if (x<5) {}\n" // <- Warning
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x<5' is always false\n", errout.str());
check("void f(int x) {\n"
"\n"
" if (x>4) {\n"
@ -2190,17 +2198,17 @@ private:
check("void f(int x) {\n"
"\n"
" if (x>5) {\n"
" if (x>4) {}\n" // <- TODO
" if (x>4) {}\n" // <- Warning
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x>4' is always true\n", errout.str());
check("void f(int x) {\n"
"\n"
" if (x>5) {\n"
" if (x>=4) {}\n" // <- TODO
" if (x>=4) {}\n" // <- Warning
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Condition 'x>=4' is always true\n", errout.str());
check("void f(int x) {\n"
"\n"
" if (x>5) {\n"
@ -3276,22 +3284,6 @@ private:
"}\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"
@ -3339,6 +3331,124 @@ private:
ASSERT_EQUALS("", errout.str());
}
void alwaysTrueInfer() {
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());
// #6890
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x == -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x==-1' is always false\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x != -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x!=-1' is always true\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x >= -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x>=-1' is always true\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x > -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x>-1' is always true\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x < -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x<-1' is always false\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x <= -1) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x<=-1' is always false\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x > 7) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Condition 'x>7' is always true\n", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x > 9) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f(int i) {\n"
" int x = i;\n"
" if (x >= 1) {}\n"
" else {\n"
" x = 8 - x; \n"
" if (x > 10) {}\n"
" else {}\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void alwaysTrueContainer() {
// #9329
check("void c1(std::vector<double>&);\n"