diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 08b251322..9ff7d4884 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -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,18 +316,8 @@ 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) - return true; - } + if (isAliasOf(tok, varid)) + return true; } return false; } diff --git a/lib/astutils.h b/lib/astutils.h index 29619b945..5786fa35b 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -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 diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index e72294783..0b3e845db 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -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); - continue; + 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) diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index 32471260f..9bdb1900e 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -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); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index a587ad801..2689dc48a 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -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) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 63fe4d965..dbdca2da5 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -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 : diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b5c12d9bc..02d8e8c17 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -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& 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,10 +2880,17 @@ static bool valueFlowForward(Token * const startToken, }); } if (inconclusive) { - for (ValueFlow::Value &v : values) { - if (v.indirect != i) - continue; - v.setInconclusive(); + 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; + }); } } } @@ -2981,6 +2971,30 @@ static const Token *findSimpleReturn(const Function *f) return returnTok; } +static std::vector findReturns(const Function* f) +{ + std::vector 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 getLifetimeTokens(const Token* tok, ValueFlow::Value::ErrorPath errorPath, int depth) { if (!tok) - return nullptr; + return std::vector {}; 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 {}; } } } 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; - if (returnTok == tok) - return tok; - const Token* argvarTok = getLifetimeToken(returnTok, errorPath, addressOf, depth - 1); - if (!argvarTok) - return tok; - const Variable* argvar = argvarTok->variable(); - if (!argvar) - return tok; - if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) { - int n = getArgumentPos(argvar, f); - if (n < 0) - return nullptr; - 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); + return {{tok, std::move(errorPath)}}; + std::vector result; + std::vector returns = findReturns(f); + for (const Token* returnTok : returns) { + if (returnTok == tok) + continue; + for (LifetimeToken& lt : getLifetimeTokens(returnTok, std::move(errorPath), depth - 1)) { + const Token* argvarTok = lt.token; + const Variable* argvar = argvarTok->variable(); + if (!argvar) + continue; + if (argvar->isArgument() && (argvar->isReference() || argvar->isRValueReference())) { + int n = getArgumentPos(argvar, f); + if (n < 0) + return std::vector {}; + const Token* argTok = getArguments(tok->previous()).at(n); + lt.errorPath.emplace_back(returnTok, "Return reference."); + lt.errorPath.emplace_back(tok->previous(), "Called function passing '" + argTok->str() + "'."); + std::vector 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 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,27 +3422,34 @@ struct LifetimeStore { template void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const { + if (!settings->inconclusive && inconclusive) + return; if (!argtok) return; - ErrorPath er = errorPath; - const Token *lifeTok = getLifetimeToken(argtok, er); - if (!lifeTok) - return; - if (!pred(lifeTok)) - return; - er.emplace_back(argtok, message); + for (const LifetimeToken& lt : getLifetimeTokens(argtok)) { + if (!settings->inconclusive && lt.inconclusive) + continue; + ErrorPath er = errorPath; + er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end()); + if (!lt.token) + return; + 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.lifetimeKind = type; - // 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); + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.lifetimeScope = ValueFlow::Value::LifetimeScope::Local; + 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 { @@ -3427,6 +3460,8 @@ struct LifetimeStore { template 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,26 +3487,31 @@ struct LifetimeStore { if (!v.isLifetimeValue()) continue; const Token *tok3 = v.tokvalue; - ErrorPath er = v.errorPath; - const Token *lifeTok = getLifetimeToken(tok3, er); - if (!lifeTok) - return; - if (!pred(lifeTok)) - return; - er.emplace_back(argtok, message); - er.insert(er.end(), errorPath.begin(), errorPath.end()); + for (const LifetimeToken& lt : getLifetimeTokens(tok3)) { + if (!settings->inconclusive && lt.inconclusive) + continue; + ErrorPath er = v.errorPath; + er.insert(er.end(), lt.errorPath.begin(), lt.errorPath.end()); + if (!lt.token) + return; + if (!pred(lt.token)) + return; + er.emplace_back(argtok, message); + er.insert(er.end(), errorPath.begin(), errorPath.end()); - ValueFlow::Value value; - value.valueType = ValueFlow::Value::LIFETIME; - value.lifetimeScope = v.lifetimeScope; - value.tokvalue = lifeTok; - value.errorPath = er; - value.lifetimeKind = type; - // Don't add the value a second time - if (std::find(tok->values().begin(), tok->values().end(), value) != tok->values().end()) - continue; - setTokenValue(tok, value, tokenlist->getSettings()); - valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.lifetimeScope = v.lifetimeScope; + 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; + setTokenValue(tok, value, tokenlist->getSettings()); + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } } } @@ -3482,6 +3523,8 @@ struct LifetimeStore { template 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,30 +3584,35 @@ 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; - 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.byVal(tok->next(), tokenlist, errorLogger, settings); - } - for (const ValueFlow::Value &v : returnTok->values()) { - if (!v.isLifetimeValue()) + std::vector returns = findReturns(f); + const bool inconclusive = returns.size() > 1; + for (const Token* returnTok : returns) { + if (returnTok == tok) continue; - if (!v.tokvalue) - continue; - const Variable *var = v.tokvalue->variable(); - LifetimeStore ls = LifetimeStore::fromFunctionArg(f, tok, var, tokenlist, errorLogger); - if (!ls.argtok) - continue; - ls.errorPath = v.errorPath; - ls.errorPath.emplace_front(returnTok, "Return " + lifetimeType(returnTok, &v) + "."); - if (var->isReference() || var->isRValueReference()) { - ls.byRef(tok->next(), tokenlist, errorLogger, settings); - } else if (v.isArgumentLifetimeValue()) { + 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()) { + if (!v.isLifetimeValue()) + continue; + if (!v.tokvalue) + continue; + const Variable *var = v.tokvalue->variable(); + 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()) { + ls.byRef(tok->next(), tokenlist, errorLogger, settings); + } else if (v.isArgumentLifetimeValue()) { + ls.byVal(tok->next(), tokenlist, errorLogger, settings); + } + } } } } @@ -3719,23 +3767,24 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger } // address of else if (tok->isUnaryOp("&")) { - ErrorPath errorPath; - const Token *lifeTok = getLifetimeToken(tok->astOperand1(), errorPath); - if (!lifeTok) - continue; + 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."); - 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 = 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()); - 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.lifetimeKind = ValueFlow::Value::LifetimeKind::Address; - setTokenValue(tok, value, tokenlist->getSettings()); - - valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } } // container lifetimes else if (astIsContainer(tok)) { diff --git a/lib/valueflow.h b/lib/valueflow.h index d9fc94dc1..f164ac4da 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -27,6 +27,7 @@ #include #include #include +#include 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 setAddressOf(std::vector v, bool b) { + for (LifetimeToken& x : v) + x.addressOf = b; + return v; + } + + static std::vector setInconclusive(std::vector v, bool b) { + for (LifetimeToken& x : v) + x.inconclusive = b; + return v; + } +}; + +std::vector 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); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7748e52dd..ab38e6154 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -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() diff --git a/test/cfg/runtests.sh b/test/cfg/runtests.sh index 3118a6f3f..a870090d1 100755 --- a/test/cfg/runtests.sh +++ b/test/cfg/runtests.sh @@ -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 diff --git a/test/cfg/std.c b/test/cfg/std.c index 3dce3c204..6992358a2 100644 --- a/test/cfg/std.c +++ b/test/cfg/std.c @@ -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 diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index d9cd7b261..409e6c9ef 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -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 diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index c0d0e96e1..e41f7cd44 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -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 \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& 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& 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 \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& 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 v;\n" " void f() {\n" diff --git a/test/testio.cpp b/test/testio.cpp index 33a7d7c6a..989e77759 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -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" diff --git a/test/teststl.cpp b/test/teststl.cpp index eb007543f..99e861144 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -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 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::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& bars) {\n" " std::vector::iterator i = bars.begin();\n"