Fix issue 10134: False positive: value is not known. Early return. (#3086)

This commit is contained in:
Paul Fultz II 2021-01-28 05:37:56 -06:00 committed by GitHub
parent dc230d18ef
commit e17d22eb87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 59 additions and 9 deletions

View File

@ -14,10 +14,10 @@ void ProgramMemory::setValue(nonneg int exprid, const ValueFlow::Value& value)
{ {
values[exprid] = value; values[exprid] = value;
} }
const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid) const const ValueFlow::Value* ProgramMemory::getValue(nonneg int exprid, bool impossible) const
{ {
const ProgramMemory::Map::const_iterator it = values.find(exprid); const ProgramMemory::Map::const_iterator it = values.find(exprid);
const bool found = it != values.end() && !it->second.isImpossible(); const bool found = it != values.end() && (impossible || !it->second.isImpossible());
if (found) if (found)
return &it->second; return &it->second;
else else
@ -58,6 +58,21 @@ bool ProgramMemory::getContainerSizeValue(nonneg int exprid, MathLib::bigint* re
} }
return false; return false;
} }
bool ProgramMemory::getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const
{
const ValueFlow::Value* value = getValue(exprid, true);
if (value && value->isContainerSizeValue()) {
if (value->isImpossible() && value->intvalue == 0) {
*result = false;
return true;
}
if (!value->isImpossible()) {
*result = (value->intvalue == 0);
return true;
}
}
return false;
}
void ProgramMemory::setUnknown(nonneg int exprid) void ProgramMemory::setUnknown(nonneg int exprid)
{ {
@ -563,10 +578,8 @@ void execute(const Token *expr,
if (!programMemory->getContainerSizeValue(containerTok->exprId(), result)) if (!programMemory->getContainerSizeValue(containerTok->exprId(), result))
*error = true; *error = true;
} else if (yield == Library::Container::Yield::EMPTY) { } else if (yield == Library::Container::Yield::EMPTY) {
MathLib::bigint size = 0; if (!programMemory->getContainerEmptyValue(containerTok->exprId(), result))
if (!programMemory->getContainerSizeValue(containerTok->exprId(), &size))
*error = true; *error = true;
*result = (size == 0);
} else { } else {
*error = true; *error = true;
} }

View File

@ -15,12 +15,13 @@ struct ProgramMemory {
Map values; Map values;
void setValue(nonneg int exprid, const ValueFlow::Value& value); void setValue(nonneg int exprid, const ValueFlow::Value& value);
const ValueFlow::Value* getValue(nonneg int exprid) const; const ValueFlow::Value* getValue(nonneg int exprid, bool impossible = false) const;
bool getIntValue(nonneg int exprid, MathLib::bigint* result) const; bool getIntValue(nonneg int exprid, MathLib::bigint* result) const;
void setIntValue(nonneg int exprid, MathLib::bigint value); void setIntValue(nonneg int exprid, MathLib::bigint value);
bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const; bool getContainerSizeValue(nonneg int exprid, MathLib::bigint* result) const;
bool getContainerEmptyValue(nonneg int exprid, MathLib::bigint* result) const;
void setUnknown(nonneg int exprid); void setUnknown(nonneg int exprid);

View File

@ -4421,13 +4421,17 @@ struct ConditionHandler {
std::mem_fn(&ValueFlow::Value::isPossible)); std::mem_fn(&ValueFlow::Value::isPossible));
} }
if (!values.empty()) { if (dead_if || dead_else) {
if ((dead_if || dead_else) && !Token::Match(tok->astParent(), "&&|&")) { if (Token::Match(tok->astParent(), "&&|&")) {
values.remove_if(std::mem_fn(&ValueFlow::Value::isImpossible));
changeKnownToPossible(values);
} else {
valueFlowSetConditionToKnown(tok, values, true); valueFlowSetConditionToKnown(tok, values, true);
valueFlowSetConditionToKnown(tok, values, false); valueFlowSetConditionToKnown(tok, values, false);
} }
forward(after, scope->bodyEnd, cond.vartok, values, tokenlist, settings);
} }
if (!values.empty())
forward(after, scope->bodyEnd, cond.vartok, values, tokenlist, settings);
} }
} }
}); });

View File

@ -3524,6 +3524,18 @@ private:
" return 0;\n" " return 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:11]: (style) Condition '!y' is always true\n", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:11]: (style) Condition '!y' is always true\n", errout.str());
// #10134
check("bool foo(bool b);\n"
"bool thud(const std::vector<std::wstring>& Arr, const std::wstring& Str) {\n"
" if (Arr.empty() && Str.empty())\n"
" return false;\n"
" bool OldFormat = Arr.empty() && !Str.empty();\n"
" if (OldFormat)\n"
" return foo(OldFormat);\n"
" return false;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
} }
void alwaysTrueInfer() { void alwaysTrueInfer() {

View File

@ -2616,6 +2616,26 @@ private:
" return x;\n" " return x;\n"
"}\n"; "}\n";
ASSERT_EQUALS(false, testValueOfX(code, 7U, 0)); ASSERT_EQUALS(false, testValueOfX(code, 7U, 0));
code = "void f(int x, int y) {\n"
" if (x && y)\n"
" return;\n"
" int a = x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 4U, 0));
ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 0));
ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 1));
code = "int f(std::vector<int> a, std::vector<int> b) {\n"
" if (a.empty() && b.empty())\n"
" return 0;\n"
" bool x = a.empty() && !b.empty();\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(false, testValueOfXKnown(code, 5U, 0));
ASSERT_EQUALS(false, testValueOfXKnown(code, 5U, 1));
ASSERT_EQUALS(false, testValueOfXImpossible(code, 5U, 0));
ASSERT_EQUALS(false, testValueOfXImpossible(code, 5U, 1));
} }
void valueFlowAfterConditionExpr() { void valueFlowAfterConditionExpr() {