Diagnose invalid lifetimes (#1475)

* Add check for invalid lifetimes

* Fix FP with member variables

* Dont forward lifetime values in subfunction

* Update message to use out of scope
This commit is contained in:
Paul Fultz II 2018-11-11 09:43:54 -06:00 committed by Daniel Marjamäki
parent bdd4623124
commit 68d6b96878
6 changed files with 102 additions and 1 deletions

View File

@ -178,7 +178,7 @@ static bool isInLoopCondition(const Token * tok)
} }
/// If tok2 comes after tok1 /// If tok2 comes after tok1
static bool precedes(const Token * tok1, const Token * tok2) bool precedes(const Token * tok1, const Token * tok2)
{ {
if (!tok1) if (!tok1)
return false; return false;

View File

@ -59,6 +59,8 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
const Token * nextAfterAstRightmostLeaf(const Token * tok); const Token * nextAfterAstRightmostLeaf(const Token * tok);
bool precedes(const Token * tok1, const Token * tok2);
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr); bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);
bool isEqualKnownValue(const Token * const tok1, const Token * const tok2); bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);

View File

@ -618,6 +618,20 @@ static bool isInScope(const Token * tok, const Scope * scope)
return false; return false;
} }
static bool isDeadScope(const Token * tok, const Scope * scope)
{
if (!tok)
return false;
if (!scope)
return false;
const Variable * var = tok->variable();
if (var && (!var->isLocal() || var->isStatic() || var->isExtern()))
return false;
if (tok->scope() && tok->scope()->bodyEnd != scope->bodyEnd && precedes(tok->scope()->bodyEnd, scope->bodyEnd))
return true;
return false;
}
void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end) void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end)
{ {
if (!start) if (!start)
@ -645,6 +659,9 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
errorReturnDanglingLifetime(tok, &val); errorReturnDanglingLifetime(tok, &val);
break; break;
} }
} else if (isDeadScope(val.tokvalue, tok->scope())) {
errorInvalidLifetime(tok, &val);
break;
} }
} }
const Token *lambdaEndToken = findLambdaEndToken(tok); const Token *lambdaEndToken = findLambdaEndToken(tok);
@ -707,6 +724,44 @@ void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const Val
reportError(errorPath, Severity::error, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, false); reportError(errorPath, Severity::error, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, false);
} }
void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val)
{
const Token *vartok = val->tokvalue;
ErrorPath errorPath = val->errorPath;
std::string msg = "";
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
msg = "Using object";
break;
case ValueFlow::Value::Lambda:
msg = "Using lambda";
break;
case ValueFlow::Value::Iterator:
msg = "Using iterator";
break;
}
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
if (var) {
switch (val->lifetimeKind) {
case ValueFlow::Value::Object:
msg += " that points to local variable";
break;
case ValueFlow::Value::Lambda:
msg += " that captures local variable";
break;
case ValueFlow::Value::Iterator:
msg += " to local container";
break;
}
msg += " '" + var->name() + "'";
}
}
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, false);
}
void CheckAutoVariables::errorReturnReference(const Token *tok) void CheckAutoVariables::errorReturnReference(const Token *tok)
{ {
reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false); reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false);

View File

@ -93,6 +93,7 @@ private:
void errorReturnPointerToLocalArray(const Token *tok); void errorReturnPointerToLocalArray(const Token *tok);
void errorAutoVariableAssignment(const Token *tok, bool inconclusive); void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val); void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
void errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val);
void errorReturnReference(const Token *tok); void errorReturnReference(const Token *tok);
void errorReturnTempReference(const Token *tok); void errorReturnTempReference(const Token *tok);
void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val); void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);

View File

@ -3837,6 +3837,9 @@ static void valueFlowSubFunction(TokenList *tokenlist, ErrorLogger *errorLogger,
// passing value(s) to function // passing value(s) to function
std::list<ValueFlow::Value> argvalues(getFunctionArgumentValues(argtok)); std::list<ValueFlow::Value> argvalues(getFunctionArgumentValues(argtok));
// Dont forward lifetime values
argvalues.remove_if(std::mem_fn(&ValueFlow::Value::isLifetimeValue));
if (argvalues.empty()) if (argvalues.empty())
continue; continue;

View File

@ -124,6 +124,7 @@ private:
TEST_CASE(danglingLifetimeLambda); TEST_CASE(danglingLifetimeLambda);
TEST_CASE(danglingLifetimeContainer); TEST_CASE(danglingLifetimeContainer);
TEST_CASE(danglingLifetime); TEST_CASE(danglingLifetime);
TEST_CASE(invalidLifetime);
} }
@ -1383,6 +1384,45 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void invalidLifetime() {
check("void foo(int a) {\n"
" std::function<void()> f;\n"
" if (a > 0) {\n"
" int b = a + 1;\n"
" f = [&]{ return b; };\n"
" }\n"
" f();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4] -> [test.cpp:7]: (error) Using lambda that captures local variable 'b' that is out of scope.\n", errout.str());
check("void foo(int a) {\n"
" std::function<void()> f;\n"
" if (a > 0) {\n"
" int b = a + 1;\n"
" f = [&]{ return b; };\n"
" f();\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("struct a {\n"
" b();\n"
" std::list<int> c;\n"
"};\n"
"void a::b() {\n"
" c.end()\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void b(char f[], char c[]) {\n"
" std::string d(c); {\n"
" std::string e;\n"
" b(f, e.c_str())\n"
" }\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
}; };
REGISTER_TEST(TestAutoVariables) REGISTER_TEST(TestAutoVariables)