Fix false positive 9167 (#1904)

Skip returns from local class/struct definition in FwdAnalysis.
This commit is contained in:
Ken-Patrick 2019-07-03 08:17:06 +02:00 committed by Daniel Marjamäki
parent d74f8c678f
commit d6f6e68fa2
3 changed files with 38 additions and 11 deletions

View File

@ -1146,7 +1146,7 @@ static bool hasFunctionCall(const Token *tok)
return hasFunctionCall(tok->astOperand1()) || hasFunctionCall(tok->astOperand2());
}
struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<unsigned int> &exprVarIds, bool local)
struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<unsigned int> &exprVarIds, bool local, bool inInnerClass)
{
// Parse the given tokens
for (const Token *tok = startToken; tok != endToken; tok = tok->next()) {
@ -1162,13 +1162,21 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
if (Token::simpleMatch(tok, "goto"))
return Result(Result::Type::BAILOUT);
if (!inInnerClass && tok->str() == "{" && tok->scope()->isClassOrStruct()) {
// skip returns from local class definition
FwdAnalysis::Result result = checkRecursive(expr, tok, tok->link(), exprVarIds, local, true);
if (result.type != Result::Type::NONE)
return result;
tok=tok->link();
}
if (tok->str() == "continue")
// TODO
return Result(Result::Type::BAILOUT);
if (const Token *lambdaEndToken = findLambdaEndToken(tok)) {
tok = lambdaEndToken;
const Result lambdaResult = checkRecursive(expr, lambdaEndToken->link()->next(), lambdaEndToken, exprVarIds, local);
const Result lambdaResult = checkRecursive(expr, lambdaEndToken->link()->next(), lambdaEndToken, exprVarIds, local, inInnerClass);
if (lambdaResult.type == Result::Type::READ || lambdaResult.type == Result::Type::BAILOUT)
return lambdaResult;
}
@ -1184,10 +1192,13 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
return Result(Result::Type::READ);
}
if (!local && mWhat == What::Reassign)
return Result(Result::Type::BAILOUT);
// #9167: if the return is inside an inner class, it does not tell us anything
if (!inInnerClass) {
if (!local && mWhat == What::Reassign)
return Result(Result::Type::BAILOUT);
return Result(Result::Type::RETURN);
return Result(Result::Type::RETURN);
}
}
if (tok->str() == "}") {
@ -1218,7 +1229,7 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
}
// check loop body again..
const struct FwdAnalysis::Result &result = checkRecursive(expr, tok->link(), tok, exprVarIds, local);
const struct FwdAnalysis::Result &result = checkRecursive(expr, tok->link(), tok, exprVarIds, local, inInnerClass);
if (result.type == Result::Type::BAILOUT || result.type == Result::Type::READ)
return result;
}
@ -1297,14 +1308,14 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
if (Token::simpleMatch(tok->link()->previous(), "switch ("))
// TODO: parse switch
return Result(Result::Type::BAILOUT);
const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local);
const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local, inInnerClass);
if (result1.type == Result::Type::READ || result1.type == Result::Type::BAILOUT)
return result1;
if (mWhat == What::ValueFlow && result1.type == Result::Type::WRITE)
mValueFlowKnown = false;
if (Token::simpleMatch(tok->linkAt(1), "} else {")) {
const Token *elseStart = tok->linkAt(1)->tokAt(2);
const Result &result2 = checkRecursive(expr, elseStart, elseStart->link(), exprVarIds, local);
const Result &result2 = checkRecursive(expr, elseStart, elseStart->link(), exprVarIds, local, inInnerClass);
if (mWhat == What::ValueFlow && result2.type == Result::Type::WRITE)
mValueFlowKnown = false;
if (result2.type == Result::Type::READ || result2.type == Result::Type::BAILOUT)
@ -1426,7 +1437,7 @@ FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToke
if (mWhat == What::UnusedValue && isGlobalData(expr))
return Result(FwdAnalysis::Result::Type::BAILOUT);
Result result = checkRecursive(expr, startToken, endToken, exprVarIds, local);
Result result = checkRecursive(expr, startToken, endToken, exprVarIds, local, false);
// Break => continue checking in outer scope
while (mWhat!=What::ValueFlow && result.type == FwdAnalysis::Result::Type::BREAK) {
@ -1435,7 +1446,7 @@ FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToke
s = s->nestedIn;
if (s->type != Scope::eSwitch && s->type != Scope::eWhile && s->type != Scope::eFor)
break;
result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local);
result = checkRecursive(expr, s->bodyEnd->next(), endToken, exprVarIds, local, false);
}
return result;

View File

@ -219,7 +219,7 @@ private:
};
struct Result check(const Token *expr, const Token *startToken, const Token *endToken);
struct Result checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<unsigned int> &exprVarIds, bool local);
struct Result checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<unsigned int> &exprVarIds, bool local, bool inInnerClass);
// Is expression a l-value global data?
bool isGlobalData(const Token *expr) const;

View File

@ -144,6 +144,7 @@ private:
TEST_CASE(localvarstring2); // ticket #2929
TEST_CASE(localvarconst1);
TEST_CASE(localvarconst2);
TEST_CASE(localvarreturn); // ticket #9167
TEST_CASE(localvarthrow); // ticket #3687
@ -4086,6 +4087,21 @@ private:
ASSERT_EQUALS("", errout.str());
}
void localvarreturn() { // ticket #9167
functionVariableUsage("void foo() {\n"
" const int MyInt = 1;\n"
" class bar {\n"
" public:\n"
" bool operator()(const int &uIndexA, const int &uIndexB) const {\n"
" return true;\n"
" }\n"
" bar() {}\n"
" };\n"
" return MyInt;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void localvarthrow() { // ticket #3687
functionVariableUsage("void foo() {\n"
" try {}"