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

Track lifetime across multiple returns 

This will now warn when doing something like this:

```cpp
template <class T, class K, class V>
const V& get_default(const T& t, const K& k, const V& v) {
    auto it = t.find(k);
    if (it == t.end()) return v;
    return it->second;
}
const int& bar(const std::unordered_map<int, int>& m, int k) {
    auto x = 0;
    return get_default(m, k, x);
}
```

The lifetime warning is considered inconclusive in this case.

I also updated valueflow to no tinject inconclusive values unless `--inconclusive` flag is passed. This creates some false negatives because library functions are not configured to not modify their input parameters, and there are some checks that do not check if the value is inconclusive or not.
This commit is contained in:
Paul Fultz II 2019-09-11 19:25:09 +02:00 committed by Daniel Marjamäki
parent a56bc006b7
commit ba037837c9
15 changed files with 324 additions and 192 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/astutils.cpp
View File

@ -292,6 +292,23 @@ bool precedes(const Token * tok1, const Token * tok2)
return tok1->index() < tok2->index();
}
bool isAliasOf(const Token *tok, nonneg int varid)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
for (const ValueFlow::Value &val : tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (val.isInconclusive())
continue;
if (val.tokvalue->varId() == varid)
return true;
}
return false;
}
static bool isAliased(const Token *startTok, const Token *endTok, nonneg int varid)
{
if (!precedes(startTok, endTok))
@ -299,19 +316,9 @@ static bool isAliased(const Token *startTok, const Token *endTok, nonneg int var
for (const Token *tok = startTok; tok != endTok; tok = tok->next()) {
if (Token::Match(tok, "= & %varid% ;", varid))
return true;
if (tok->varId() == varid)
continue;
if (tok->varId() == 0)
continue;
if (!astIsPointer(tok))
continue;
for (const ValueFlow::Value &val : tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (val.tokvalue->varId() == varid)
if (isAliasOf(tok, varid))
return true;
}
}
return false;
}

3
lib/astutils.h
View File

@ -154,6 +154,9 @@ bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp,
const Token* findVariableChanged(const Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
/// If token is an alias if another variable
bool isAliasOf(const Token *tok, nonneg int varid);
bool isAliased(const Variable *var);
/** Determines the number of arguments - if token is a function call or macro

27
lib/checkautovariables.cpp
View File

@ -548,12 +548,15 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
for (const Token *tok = start; tok && tok != end; tok = tok->next()) {
// Return reference from function
if (returnRef && Token::simpleMatch(tok->astParent(), "return")) {
ErrorPath errorPath;
const Variable *var = getLifetimeVariable(tok, errorPath);
if (var && !var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() &&
isInScope(var->nameToken(), tok->scope())) {
errorReturnReference(tok, errorPath);
for (const LifetimeToken& lt : getLifetimeTokens(tok)) {
const Variable* var = lt.token->variable();
if (!var)
continue;
if (!var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() &&
isInScope(var->nameToken(), tok->scope())) {
errorReturnReference(tok, lt.errorPath, lt.inconclusive);
break;
}
}
// Assign reference to non-local variable
} else if (Token::Match(tok->previous(), "&|&& %var% =") && tok->astParent() == tok->next() &&
@ -635,33 +638,37 @@ void CheckAutoVariables::checkVarLifetime()
void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value *val)
{
const bool inconclusive = val ? val->isInconclusive() : false;
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string msg = "Returning " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
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, inconclusive);
}
void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val)
{
const bool inconclusive = val ? val->isInconclusive() : false;
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, false);
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, inconclusive);
}
void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val)
{
const bool inconclusive = val ? val->isInconclusive() : false;
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string tokName = tok ? tok->expressionString() : "x";
std::string msg = "Non-local variable '" + tokName + "' will use " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, false);
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, inconclusive);
}
void CheckAutoVariables::errorReturnReference(const Token *tok, ErrorPath errorPath)
void CheckAutoVariables::errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive)
{
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "returnReference", "Reference to local variable returned.", CWE562, false);
reportError(
errorPath, Severity::error, "returnReference", "Reference to local variable returned.", CWE562, inconclusive);
}
void CheckAutoVariables::errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath)

4
lib/checkautovariables.h
View File

@ -85,7 +85,7 @@ private:
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
void errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val);
void errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val);
void errorReturnReference(const Token *tok, ErrorPath errorPath);
void errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive);
void errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath);
void errorReturnTempReference(const Token *tok);
void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val);
@ -99,7 +99,7 @@ private:
c.errorAutoVariableAssignment(nullptr, false);
c.errorReturnAddressToAutoVariable(nullptr);
c.errorReturnPointerToLocalArray(nullptr);
c.errorReturnReference(nullptr, errorPath);
c.errorReturnReference(nullptr, errorPath, false);
c.errorDanglingReference(nullptr, nullptr, errorPath);
c.errorReturnTempReference(nullptr);
c.errorInvalidDeallocation(nullptr, nullptr);

3
lib/checkstl.cpp
View File

@ -881,13 +881,14 @@ void CheckStl::invalidContainer()
void CheckStl::invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath)
{
const bool inconclusive = val ? val->isInconclusive() : false;
std::string method = contTok ? contTok->strAt(2) : "erase";
errorPath.emplace_back(contTok, "After calling '" + method + "', iterators or references to the container's data may be invalid .");
if (val)
errorPath.insert(errorPath.begin(), val->errorPath.begin(), val->errorPath.end());
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidContainer", msg + " that may be invalid.", CWE664, false);
reportError(errorPath, Severity::error, "invalidContainer", msg + " that may be invalid.", CWE664, inconclusive);
}
void CheckStl::invalidContainerReferenceError(const Token* tok, const Token* contTok, ErrorPath errorPath)

2
lib/symboldatabase.cpp
View File

@ -5912,7 +5912,7 @@ ValueType::MatchResult ValueType::matchParameter(const ValueType *call, const Va
if (call->type == ValueType::Type::VOID || func->type == ValueType::Type::VOID)
return ValueType::MatchResult::FALLBACK1;
if (call->pointer > 0 && func->pointer > 0)
return ValueType::MatchResult::NOMATCH;
return func->type == ValueType::UNKNOWN_TYPE ? ValueType::MatchResult::UNKNOWN : ValueType::MatchResult::NOMATCH;
if (call->isIntegral() && func->isIntegral())
return call->type < func->type ?
ValueType::MatchResult::FALLBACK1 :

209
lib/valueflow.cpp
View File

@ -2165,25 +2165,6 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign
return true;
}
static bool isAliasOf(const Token *tok, nonneg int varid)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (!astIsPointer(tok))
return false;
for (const ValueFlow::Value &val : tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
continue;
if (val.tokvalue->varId() == varid)
return true;
}
return false;
}
// Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values)
{
@ -2199,6 +2180,8 @@ static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid,
for (const ValueFlow::Value &val : values) {
if (!val.isNonValue())
continue;
if (val.isInconclusive())
continue;
if (val.isLifetimeValue() && !val.isLocalLifetimeValue())
continue;
if (val.isLifetimeValue() && val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
@ -2897,11 +2880,18 @@ static bool valueFlowForward(Token * const startToken,
});
}
if (inconclusive) {
if (settings->inconclusive) {
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()) {
@ -2981,6 +2971,30 @@ static const Token *findSimpleReturn(const Function *f)
return returnTok;
}
static std::vector<const Token*> findReturns(const Function* f)
{
std::vector<const Token*> result;
const Scope* scope = f->functionScope;
if (!scope)
return result;
for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) {
if (tok->str() == "{" && tok->scope() &&
(tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) {
tok = tok->link();
continue;
}
if (Token::simpleMatch(tok->astParent(), "return")) {
result.push_back(tok);
}
// Skip lambda functions since the scope may not be set correctly
const Token* lambdaEndToken = findLambdaEndToken(tok);
if (lambdaEndToken) {
tok = lambdaEndToken;
}
}
return result;
}
static int getArgumentPos(const Variable *var, const Function *f)
{
auto arg_it = std::find_if(f->argumentList.begin(), f->argumentList.end(), [&](const Variable &v) {
@ -3052,6 +3066,8 @@ ValueFlow::Value getLifetimeObjValue(const Token *tok)
auto pred = [](const ValueFlow::Value &v) {
if (!v.isLocalLifetimeValue())
return false;
if (v.isInconclusive())
return false;
if (!v.tokvalue->variable())
return false;
return true;
@ -3066,69 +3082,67 @@ ValueFlow::Value getLifetimeObjValue(const Token *tok)
return result;
}
static const Token* getLifetimeToken(const Token* tok,
ValueFlow::Value::ErrorPath& errorPath,
bool* addressOf = nullptr,
int depth = 20)
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value::ErrorPath errorPath, int depth)
{
if (!tok)
return nullptr;
return std::vector<LifetimeToken> {};
const Variable *var = tok->variable();
if (depth < 0)
return tok;
return {{tok, std::move(errorPath)}};
if (var && var->declarationId() == tok->varId()) {
if (var->isReference() || var->isRValueReference()) {
if (addressOf)
*addressOf = true;
if (!var->declEndToken())
return tok;
return {{tok, true, std::move(errorPath)}};
if (var->isArgument()) {
errorPath.emplace_back(var->declEndToken(), "Passed to reference.");
return tok;
return {{tok, true, std::move(errorPath)}};
} else if (Token::simpleMatch(var->declEndToken(), "=")) {
errorPath.emplace_back(var->declEndToken(), "Assigned to reference.");
const Token *vartok = var->declEndToken()->astOperand2();
if (vartok == tok)
return tok;
return {{tok, true, std::move(errorPath)}};
if (vartok)
return getLifetimeToken(vartok, errorPath, addressOf, depth - 1);
return getLifetimeTokens(vartok, std::move(errorPath), depth - 1);
} else {
return nullptr;
return std::vector<LifetimeToken> {};
}
}
} else if (Token::Match(tok->previous(), "%name% (")) {
const Function *f = tok->previous()->function();
if (f) {
if (!Function::returnsReference(f))
return tok;
const Token* returnTok = findSimpleReturn(f);
if (!returnTok)
return tok;
return {{tok, std::move(errorPath)}};
std::vector<LifetimeToken> result;
std::vector<const Token*> returns = findReturns(f);
for (const Token* returnTok : returns) {
if (returnTok == tok)
return tok;
const Token* argvarTok = getLifetimeToken(returnTok, errorPath, addressOf, depth - 1);
if (!argvarTok)
return tok;
continue;
for (LifetimeToken& lt : getLifetimeTokens(returnTok, std::move(errorPath), depth - 1)) {
const Token* argvarTok = lt.token;
const Variable* argvar = argvarTok->variable();
if (!argvar)
return tok;
continue;
if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) {
int n = getArgumentPos(argvar, f);
if (n < 0)
return nullptr;
return std::vector<LifetimeToken> {};
const Token* argTok = getArguments(tok->previous()).at(n);
errorPath.emplace_back(returnTok, "Return reference.");
errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'.");
return getLifetimeToken(argTok, errorPath, addressOf, depth - 1);
lt.errorPath.emplace_back(returnTok, "Return reference.");
lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'.");
std::vector<LifetimeToken> arglts = LifetimeToken::setInconclusive(
getLifetimeTokens(argTok, std::move(lt.errorPath), depth - 1), returns.size() > 1);
result.insert(result.end(), arglts.begin(), arglts.end());
}
}
}
return result;
} else if (Token::Match(tok->tokAt(-2), ". %name% (") && astIsContainer(tok->tokAt(-2)->astOperand1())) {
const Library::Container* library = getLibraryContainer(tok->tokAt(-2)->astOperand1());
Library::Container::Yield y = library->getYield(tok->previous()->str());
if (y == Library::Container::Yield::AT_INDEX || y == Library::Container::Yield::ITEM) {
errorPath.emplace_back(tok->previous(), "Accessing container.");
if (addressOf)
*addressOf = false;
return getLifetimeToken(tok->tokAt(-2)->astOperand1(), errorPath, nullptr, depth - 1);
return LifetimeToken::setAddressOf(
getLifetimeTokens(tok->tokAt(-2)->astOperand1(), std::move(errorPath), depth - 1), false);
}
}
} else if (Token::Match(tok, ".|::|[")) {
@ -3143,7 +3157,7 @@ static const Token* getLifetimeToken(const Token* tok,
}
if (!vartok)
return tok;
return {{tok, std::move(errorPath)}};
const Variable *tokvar = vartok->variable();
if (!astIsContainer(vartok) && !(tokvar && tokvar->isArray()) &&
(Token::Match(vartok->astParent(), "[|*") || vartok->astParent()->originalName() == "->")) {
@ -3151,17 +3165,27 @@ static const Token* getLifetimeToken(const Token* tok,
if (!v.isLocalLifetimeValue())
continue;
errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end());
return getLifetimeToken(v.tokvalue, errorPath, addressOf);
return getLifetimeTokens(v.tokvalue, std::move(errorPath));
}
} else {
if (addressOf && astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "[")) {
*addressOf = false;
addressOf = nullptr;
}
return getLifetimeToken(vartok, errorPath, addressOf);
return LifetimeToken::setAddressOf(getLifetimeTokens(vartok, std::move(errorPath)),
!(astIsContainer(vartok) && Token::simpleMatch(vartok->astParent(), "[")));
}
}
return tok;
return {{tok, std::move(errorPath)}};
}
static const Token* getLifetimeToken(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr)
{
std::vector<LifetimeToken> lts = getLifetimeTokens(tok);
if (lts.size() != 1)
return nullptr;
if (lts.front().inconclusive)
return nullptr;
if (addressOf)
*addressOf = lts.front().addressOf;
errorPath.insert(errorPath.end(), lts.front().errorPath.begin(), lts.front().errorPath.end());
return lts.front().token;
}
const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf)
@ -3360,15 +3384,17 @@ struct LifetimeStore {
std::string message;
ValueFlow::Value::LifetimeKind type;
ErrorPath errorPath;
bool inconclusive;
LifetimeStore()
: argtok(nullptr), message(), type(), errorPath()
: argtok(nullptr), message(), type(), errorPath(), inconclusive(false)
{}
LifetimeStore(const Token *argtok,
const std::string &message,
ValueFlow::Value::LifetimeKind type = ValueFlow::Value::LifetimeKind::Object)
: argtok(argtok), message(message), type(type), errorPath()
ValueFlow::Value::LifetimeKind type = ValueFlow::Value::LifetimeKind::Object,
bool inconclusive = false)
: argtok(argtok), message(message), type(type), errorPath(), inconclusive(inconclusive)
{}
static LifetimeStore fromFunctionArg(const Function * f, Token *tok, const Variable *var, TokenList *tokenlist, ErrorLogger *errorLogger) {
@ -3396,28 +3422,35 @@ struct LifetimeStore {
template <class Predicate>
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
if (!settings->inconclusive && inconclusive)
return;
if (!argtok)
return;
for (const LifetimeToken& lt : getLifetimeTokens(argtok)) {
if (!settings->inconclusive && lt.inconclusive)
continue;
ErrorPath er = errorPath;
const Token *lifeTok = getLifetimeToken(argtok, er);
if (!lifeTok)
er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end());
if (!lt.token)
return;
if (!pred(lifeTok))
if (!pred(lt.token))
return;
er.emplace_back(argtok, message);
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = ValueFlow::Value::LifetimeScope::Local;
value.tokvalue = lifeTok;
value.errorPath = er;
value.tokvalue = lt.token;
value.errorPath = std::move(er);
value.lifetimeKind = type;
value.setInconclusive(lt.inconclusive || inconclusive);
// Don't add the value a second time
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
return;
setTokenValue(tok, value, tokenlist->getSettings());
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
}
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byRef(tok, tokenlist, errorLogger, settings, [](const Token *) {
@ -3427,6 +3460,8 @@ struct LifetimeStore {
template <class Predicate>
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
if (!settings->inconclusive && inconclusive)
return;
if (!argtok)
return;
if (argtok->values().empty()) {
@ -3440,6 +3475,7 @@ struct LifetimeStore {
value.tokvalue = var->nameToken();
value.errorPath = er;
value.lifetimeKind = type;
value.setInconclusive(inconclusive);
// Don't add the value a second time
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
return;
@ -3451,11 +3487,14 @@ struct LifetimeStore {
if (!v.isLifetimeValue())
continue;
const Token *tok3 = v.tokvalue;
for (const LifetimeToken& lt : getLifetimeTokens(tok3)) {
if (!settings->inconclusive && lt.inconclusive)
continue;
ErrorPath er = v.errorPath;
const Token *lifeTok = getLifetimeToken(tok3, er);
if (!lifeTok)
er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end());
if (!lt.token)
return;
if (!pred(lifeTok))
if (!pred(lt.token))
return;
er.emplace_back(argtok, message);
er.insert(er.end(), errorPath.begin(), errorPath.end());
@ -3463,9 +3502,10 @@ struct LifetimeStore {
ValueFlow::Value value;
value.valueType = ValueFlow::Value::LIFETIME;
value.lifetimeScope = v.lifetimeScope;
value.tokvalue = lifeTok;
value.errorPath = er;
value.tokvalue = lt.token;
value.errorPath = std::move(er);
value.lifetimeKind = type;
value.setInconclusive(lt.inconclusive || v.isInconclusive() || inconclusive);
// Don't add the value a second time
if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end())
continue;
@ -3473,6 +3513,7 @@ struct LifetimeStore {
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
}
}
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) const {
byVal(tok, tokenlist, errorLogger, settings, [](const Token *) {
@ -3482,6 +3523,8 @@ struct LifetimeStore {
template <class Predicate>
void byDerefCopy(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
if (!settings->inconclusive && inconclusive)
return;
if (!argtok)
return;
for (const ValueFlow::Value &v : argtok->values()) {
@ -3495,7 +3538,7 @@ struct LifetimeStore {
continue;
for (const Token *tok3 = tok; tok3 && tok3 != var->declEndToken(); tok3 = tok3->previous()) {
if (tok3->varId() == var->declarationId()) {
LifetimeStore{tok3, message, type} .byVal(tok, tokenlist, errorLogger, settings, pred);
LifetimeStore{tok3, message, type, inconclusive} .byVal(tok, tokenlist, errorLogger, settings, pred);
break;
}
}
@ -3541,12 +3584,15 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
const Function *f = tok->function();
if (Function::returnsReference(f))
return;
const Token *returnTok = findSimpleReturn(f);
if (!returnTok)
return;
std::vector<const Token*> returns = findReturns(f);
const bool inconclusive = returns.size() > 1;
for (const Token* returnTok : returns) {
if (returnTok == tok)
continue;
const Variable *returnVar = returnTok->variable();
if (returnVar && returnVar->isArgument() && (returnVar->isConst() || !isVariableChanged(returnVar, settings, tokenlist->isCPP()))) {
LifetimeStore ls = LifetimeStore::fromFunctionArg(f, tok, returnVar, tokenlist, errorLogger);
ls.inconclusive = inconclusive;
ls.byVal(tok->next(), tokenlist, errorLogger, settings);
}
for (const ValueFlow::Value &v : returnTok->values()) {
@ -3558,6 +3604,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
LifetimeStore ls = LifetimeStore::fromFunctionArg(f, tok, var, tokenlist, errorLogger);
if (!ls.argtok)
continue;
ls.inconclusive = inconclusive;
ls.errorPath = v.errorPath;
ls.errorPath.emplace_front(returnTok, "Return " + lifetimeType(returnTok, &v) + ".");
if (var->isReference() || var->isRValueReference()) {
@ -3568,6 +3615,7 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
}
}
}
}
static void valueFlowLifetimeConstructor(Token* tok,
const Type* t,
@ -3719,24 +3767,25 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
}
// address of
else if (tok->isUnaryOp("&")) {
ErrorPath errorPath;
const Token *lifeTok = getLifetimeToken(tok->astOperand1(), errorPath);
if (!lifeTok)
for (const LifetimeToken& lt : getLifetimeTokens(tok->astOperand1())) {
if (!settings->inconclusive && lt.inconclusive)
continue;
ErrorPath errorPath = lt.errorPath;
errorPath.emplace_back(tok, "Address of variable taken here.");
ValueFlow::Value value;
value.valueType = ValueFlow::Value::ValueType::LIFETIME;
value.lifetimeScope = ValueFlow::Value::LifetimeScope::Local;
value.tokvalue = lifeTok;
value.errorPath = errorPath;
if (astIsPointer(lifeTok) || !Token::Match(lifeTok->astParent(), ".|["))
value.tokvalue = lt.token;
value.errorPath = std::move(errorPath);
if (astIsPointer(lt.token) || !Token::Match(lt.token->astParent(), ".|["))
value.lifetimeKind = ValueFlow::Value::LifetimeKind::Address;
value.setInconclusive(lt.inconclusive);
setTokenValue(tok, value, tokenlist->getSettings());
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
}
// container lifetimes
else if (astIsContainer(tok)) {
Token * parent = astParentSkipParens(tok);

32
lib/valueflow.h
View File

@ -27,6 +27,7 @@
#include <list>
#include <string>
#include <utility>
#include <vector>
class ErrorLogger;
class Settings;
@ -259,6 +260,37 @@ namespace ValueFlow {
std::string eitherTheConditionIsRedundant(const Token *condition);
}
struct LifetimeToken {
const Token* token;
bool addressOf;
ValueFlow::Value::ErrorPath errorPath;
bool inconclusive;
LifetimeToken() : token(nullptr), addressOf(false), errorPath(), inconclusive(false) {}
LifetimeToken(const Token* token, ValueFlow::Value::ErrorPath errorPath)
: token(token), addressOf(false), errorPath(std::move(errorPath)), inconclusive(false)
{}
LifetimeToken(const Token* token, bool addressOf, ValueFlow::Value::ErrorPath errorPath)
: token(token), addressOf(addressOf), errorPath(std::move(errorPath)), inconclusive(false)
{}
static std::vector<LifetimeToken> setAddressOf(std::vector<LifetimeToken> v, bool b) {
for (LifetimeToken& x : v)
x.addressOf = b;
return v;
}
static std::vector<LifetimeToken> setInconclusive(std::vector<LifetimeToken> v, bool b) {
for (LifetimeToken& x : v)
x.inconclusive = b;
return v;
}
};
std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{}, int depth = 20);
const Variable* getLifetimeVariable(const Token* tok, ValueFlow::Value::ErrorPath& errorPath, bool* addressOf = nullptr);
bool isLifetimeBorrowed(const Token *tok, const Settings *settings);

19
test/CMakeLists.txt
View File

@ -59,13 +59,6 @@ if (BUILD_TESTS)
endif()
endforeach()
# Set cost of the more expensive tests to help improve parallel scheduling
# of tests
fixture_cost(TestIO 20)
fixture_cost(TestThreadExecutor 5)
fixture_cost(TestLeakAutoVar 4)
fixture_cost(TestTokenizer 4)
function(add_cfg CFG_TEST)
set(options INCONCLUSIVE)
set(oneValueArgs PLATFORM NAME)
@ -118,10 +111,18 @@ if (BUILD_TESTS)
add_cfg(python.c)
add_cfg(qt.cpp)
add_cfg(sqlite3.c INCONCLUSIVE)
add_cfg(std.c)
add_cfg(std.cpp)
add_cfg(std.c INCONCLUSIVE)
add_cfg(std.cpp INCONCLUSIVE)
add_cfg(windows.cpp INCONCLUSIVE NAME windows32A PLATFORM win32A)
add_cfg(windows.cpp INCONCLUSIVE NAME windows32W PLATFORM win32W)
add_cfg(windows.cpp INCONCLUSIVE NAME windows64 PLATFORM win64)
add_cfg(wxwidgets.cpp INCONCLUSIVE)
# Set cost of the more expensive tests to help improve parallel scheduling
# of tests
fixture_cost(TestIO 20)
fixture_cost(cfg-std_c 8)
fixture_cost(TestThreadExecutor 5)
fixture_cost(TestLeakAutoVar 4)
fixture_cost(TestTokenizer 4)
endif()

4
test/cfg/runtests.sh
View File

@ -63,9 +63,9 @@ ${CPPCHECK} ${CPPCHECK_OPT} --library=bsd ${DIR}bsd.c
# std.c
${CC} ${CC_OPT} ${DIR}std.c
${CPPCHECK} ${CPPCHECK_OPT} ${DIR}std.c
${CPPCHECK} ${CPPCHECK_OPT} --inconclusive ${DIR}std.c
${CXX} ${CXX_OPT} ${DIR}std.cpp
${CPPCHECK} ${CPPCHECK_OPT} ${DIR}std.cpp
${CPPCHECK} ${CPPCHECK_OPT} --inconclusive ${DIR}std.cpp
# windows.cpp
# Syntax check via g++ does not work because it can not find a valid windows.h

1
test/cfg/std.c
View File

@ -47,6 +47,7 @@ void bufferAccessOutOfBounds(void)
// cppcheck-suppress bufferAccessOutOfBounds
strcpy_s(a, 10, "abcdefghij");
// TODO cppcheck-suppress redundantCopy
// cppcheck-suppress terminateStrncpy
strncpy(a,"abcde",5);
// cppcheck-suppress bufferAccessOutOfBounds
// TODO cppcheck-suppress redundantCopy

1
test/cfg/std.cpp
View File

@ -78,6 +78,7 @@ void bufferAccessOutOfBounds(void)
// TODO cppcheck-suppress redundantCopy
std::strcpy(a, "abcde");
// TODO cppcheck-suppress redundantCopy
// cppcheck-suppress terminateStrncpy
std::strncpy(a,"abcde",5);
// cppcheck-suppress bufferAccessOutOfBounds
// TODO cppcheck-suppress redundantCopy

30
test/testautovariables.cpp
View File

@ -1228,6 +1228,21 @@ private:
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Reference to local variable returned.\n", errout.str());
check("template <class T, class K, class V>\n"
"const V& get_default(const T& t, const K& k, const V& v) {\n"
" auto it = t.find(k);\n"
" if (it == t.end()) return v;\n"
" return it->second;\n"
"}\n"
"const int& bar(const std::unordered_map<int, int>& m, int k) {\n"
" auto x = 0;\n"
" return get_default(m, k, x);\n"
"}\n",
true);
ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:4] -> [test.cpp:9] -> [test.cpp:9]: (error, inconclusive) Reference to local variable returned.\n",
errout.str());
check("struct A { int foo; };\n"
"int& f(std::vector<A>& v) {\n"
" auto it = v.begin();\n"
@ -1666,6 +1681,21 @@ private:
"[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'ba' that will be invalid when returning.\n",
errout.str());
check("template <class T, class K, class V>\n"
"const V* get_default(const T& t, const K& k, const V* v) {\n"
" auto it = t.find(k);\n"
" if (it == t.end()) return v;\n"
" return &it->second;\n"
"}\n"
"const int* bar(const std::unordered_map<int, int>& m, int k) {\n"
" auto x = 0;\n"
" return get_default(m, k, &x);\n"
"}\n",
true);
ASSERT_EQUALS(
"[test.cpp:9] -> [test.cpp:9] -> [test.cpp:8] -> [test.cpp:9]: (error, inconclusive) Returning pointer to local variable 'x' that will be invalid when returning.\n",
errout.str());
check("struct A {\n"
" std::vector<std::string> v;\n"
" void f() {\n"

2
test/testio.cpp
View File

@ -4454,7 +4454,7 @@ private:
" sprintf_s(lineBuffer, 100, format1, \"type\", \"sum\", \"avg\", \"min\", i, 0);\n"
" sprintf_s(lineBuffer, 100, format2, \"type\", \"sum\", \"avg\", \"min\", i, 0);\n"
" sprintf_s(lineBuffer, 100, format3, \"type\", \"sum\", \"avg\", \"min\", i, 0);\n"
"}\n", false, false, Settings::Win32A);
"}\n", true, false, Settings::Win32A);
ASSERT_EQUALS("[test.cpp:6]: (warning) %s in format string (no. 5) requires 'char *' but the argument type is 'signed int'.\n"
"[test.cpp:6]: (warning) sprintf_s format string requires 5 parameters but 6 are given.\n"
"[test.cpp:7]: (warning) %s in format string (no. 5) requires 'char *' but the argument type is 'signed int'.\n"

18
test/teststl.cpp
View File

@ -1188,8 +1188,8 @@ private:
" iter = ints.begin() + 2;\n"
" ints.erase(iter);\n"
" std::cout << (*iter) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:7]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
"}", true);
TODO_ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:7]: (error) Using iterator to local container 'ints' that may be invalid.\n", "[test.cpp:5] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:7]: (error, inconclusive) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
// #6554 "False positive eraseDereference - erase in while() loop"
check("typedef std::map<Packet> packetMap;\n"
@ -1259,8 +1259,8 @@ private:
" auto iter = ints.begin() + 2;\n"
" ints.erase(iter);\n"
" std::cout << (*iter) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
"}", true);
TODO_ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", "[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error, inconclusive) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
check("void f() {\n"
" auto x = *myList.begin();\n"
@ -1766,8 +1766,8 @@ private:
" iter = ints.begin() + 2;\n"
" ints.erase(iter);\n"
" ints.erase(iter);\n"
"}");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:4] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
"}", true);
TODO_ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:4] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", "[test.cpp:1] -> [test.cpp:4] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6]: (error, inconclusive) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
}
void eraseByValue() {
@ -2053,7 +2053,7 @@ private:
" *it;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error) Using iterator to local container 'vec' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:4]: (error) Using iterator to local container 'vec' that may be invalid.\n", errout.str());
}
void pushback13() {
@ -2089,8 +2089,8 @@ private:
" std::vector<int>::iterator iter = ints.begin();\n"
" ints.insert(iter, 1);\n"
" ints.insert(iter, 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
"}", true);
TODO_ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", "[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error, inconclusive) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
check("void* f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"