diff --git a/lib/astutils.cpp b/lib/astutils.cpp index d4917fdd2..1a6e57bb3 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -523,17 +523,22 @@ bool precedes(const Token * tok1, const Token * tok2) return tok1->index() < tok2->index(); } -bool isAliasOf(const Token *tok, nonneg int varid) +bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive) { if (tok->varId() == varid) return false; for (const ValueFlow::Value &val : tok->values()) { if (!val.isLocalLifetimeValue()) continue; - if (val.isInconclusive()) - continue; - if (val.tokvalue->varId() == varid) + if (val.tokvalue->varId() == varid) { + if (val.isInconclusive()) { + if (inconclusive) + *inconclusive = true; + else + continue; + } return true; + } } return false; } diff --git a/lib/astutils.h b/lib/astutils.h index 0479d462b..3171d65fd 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -202,7 +202,7 @@ const Token* findVariableChanged(const Token *start, const Token *end, int indir Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, 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 isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr); bool isAliased(const Variable *var); diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index a1d99f1d7..78075b44e 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -494,6 +494,7 @@ static bool isDanglingSubFunction(const Token* tokvalue, const Token* tok) void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end) { + const bool printInconclusive = (mSettings->inconclusive); if (!start) return; const Scope * scope = start->scope(); @@ -507,6 +508,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token // Return reference from function if (returnRef && Token::simpleMatch(tok->astParent(), "return")) { for (const LifetimeToken& lt : getLifetimeTokens(tok, true)) { + if (!printInconclusive && lt.inconclusive) + continue; const Variable* var = lt.token->variable(); if (var && !var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() && isInScope(var->nameToken(), tok->scope())) { @@ -531,6 +534,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token // Reference to temporary } else if (tok->variable() && (tok->variable()->isReference() || tok->variable()->isRValueReference())) { for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(tok))) { + if (!printInconclusive && lt.inconclusive) + continue; const Token * tokvalue = lt.token; if (isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) { errorDanglingTempReference(tok, lt.errorPath, lt.inconclusive); @@ -542,6 +547,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token for (const ValueFlow::Value& val:tok->values()) { if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue()) continue; + if (!printInconclusive && val.isInconclusive()) + continue; const bool escape = Token::Match(tok->astParent(), "return|throw"); for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(val.tokvalue), escape)) { const Token * tokvalue = lt.token; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 925441a9d..c127944a1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2193,42 +2193,13 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign } // 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 ValueFlow::Value& val) +static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list& values, bool* inconclusive = nullptr) { if (tok->varId() == varid) return false; if (tok->varId() == 0) return false; - if (isAliasOf(tok, varid)) - return true; - if (var && !var->isPointer()) - return false; - // Search through non value aliases - - if (!val.isNonValue()) - return false; - if (val.isInconclusive()) - return false; - if (val.isLifetimeValue() && !val.isLocalLifetimeValue()) - return false; - if (val.isLifetimeValue() && val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address) - return false; - if (!Token::Match(val.tokvalue, ".|&|*|%var%")) - return false; - if (astHasVar(val.tokvalue, tok->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) -{ - if (tok->varId() == varid) - return false; - if (tok->varId() == 0) - return false; - if (isAliasOf(tok, varid)) + if (isAliasOf(tok, varid, inconclusive)) return true; if (var && !var->isPointer()) return false; @@ -2328,7 +2299,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer { virtual bool match(const Token* tok) const = 0; - virtual bool isAlias(const Token* tok) const = 0; + virtual bool isAlias(const Token* tok, bool& inconclusive) const = 0; using ProgramState = std::unordered_map; @@ -2448,6 +2419,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer { virtual Action analyze(const Token* tok) const OVERRIDE { if (invalid()) return Action::Invalid; + bool inconclusive = false; if (match(tok)) { const Token* parent = tok->astParent(); if (astIsPointer(tok) && (Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && getIndirect(tok) <= 0) @@ -2479,8 +2451,12 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer { } return Action::None; - } else if (isAlias(tok)) { - return isAliasModified(tok); + } else if (isAlias(tok, inconclusive)) { + Action a = isAliasModified(tok); + if (inconclusive && a.isModified()) + return Action::Inconclusive; + else + return a; } else if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) { // bailout: global non-const variables if (isGlobal()) { @@ -2568,7 +2544,7 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { value.errorPath.emplace_back(tok, s); } - virtual bool isAlias(const Token* tok) const OVERRIDE { + virtual bool isAlias(const Token* tok, bool& inconclusive) const OVERRIDE { if (value.isLifetimeValue()) return false; for (const auto& m: { @@ -2579,7 +2555,7 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { const Variable* var = p.second; if (tok->varId() == varid) return true; - if (isAliasOf(var, tok, varid, value)) + if (isAliasOf(var, tok, varid, {value}, &inconclusive)) return true; } } @@ -3293,8 +3269,32 @@ static void valueFlowLifetimeConstructor(Token *tok, ErrorLogger *errorLogger, const Settings *settings); +static const Token* getEndOfVarScope(const Token* tok, const std::vector& vars) +{ + const Token* endOfVarScope = nullptr; + for (const Variable* var : vars) { + if (var && var->isLocal()) + endOfVarScope = var->typeStartToken()->scope()->bodyEnd; + else if (!endOfVarScope) + endOfVarScope = tok->scope()->bodyEnd; + } + return endOfVarScope; +} + static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) { + // Forward lifetimes to constructed variable + if (Token::Match(tok->previous(), "%var% {")) { + std::list values = tok->values(); + values.remove_if(&isNotLifetimeValue); + valueFlowForward(nextAfterAstRightmostLeaf(tok), + getEndOfVarScope(tok, {tok->variable()}), + tok->previous(), + values, + tokenlist, + settings); + return; + } Token *parent = tok->astParent(); while (parent && (parent->isArithmeticalOp() || parent->str() == ",")) parent = parent->astParent(); @@ -3311,13 +3311,7 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog std::vector vars = getLHSVariables(parent); - const Token* endOfVarScope = nullptr; - for (const Variable* var : vars) { - if (var && var->isLocal()) - endOfVarScope = var->typeStartToken()->scope()->bodyEnd; - else if (!endOfVarScope) - endOfVarScope = tok->scope()->bodyEnd; - } + const Token* endOfVarScope = getEndOfVarScope(tok, vars); // Only forward lifetime values std::list values = parent->astOperand2()->values(); @@ -3427,8 +3421,6 @@ 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; for (const LifetimeToken& lt : getLifetimeTokens(argtok)) { @@ -3465,8 +3457,6 @@ 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()) { @@ -3647,10 +3637,21 @@ static void valueFlowLifetimeConstructor(Token* tok, ErrorLogger* errorLogger, const Settings* settings) { - if (!t) - return; if (!Token::Match(tok, "(|{")) return; + if (!t) { + if (tok->valueType() && tok->valueType()->type != ValueType::RECORD) + return; + // If the type is unknown then assume it captures by value in the + // constructor, but make each lifetime inconclusive + std::vector args = getArguments(tok); + for (const Token *argtok : args) { + LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object}; + ls.inconclusive = true; + ls.byVal(tok, tokenlist, errorLogger, settings); + } + return; + } const Scope* scope = t->classScope; if (!scope) return; @@ -3710,8 +3711,8 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error ls.byVal(tok, tokenlist, errorLogger, settings); } } - } else if (const Type* t = Token::typeOf(tok->previous())) { - valueFlowLifetimeConstructor(tok, t, tokenlist, errorLogger, settings); + } else { + valueFlowLifetimeConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings); } } @@ -5037,7 +5038,7 @@ struct MultiValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { } } - virtual bool isAlias(const Token* tok) const OVERRIDE { + virtual bool isAlias(const Token* tok, bool& inconclusive) const OVERRIDE { std::list vals; std::transform(values.begin(), values.end(), std::back_inserter(vals), SelectMapValues{}); @@ -5046,7 +5047,7 @@ struct MultiValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer { const Variable* var = p.second; if (tok->varId() == varid) return true; - if (isAliasOf(var, tok, varid, vals)) + if (isAliasOf(var, tok, varid, vals, &inconclusive)) return true; } return false; diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index fd4113554..481d99adb 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -2611,7 +2611,6 @@ private: "[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n", errout.str()); - // TODO: Ast is missing for this case check("struct A {\n" " const int& x;\n" " int y;\n" @@ -2621,9 +2620,8 @@ private: " A r{i, i};\n" " return r;\n" "}\n"); - TODO_ASSERT_EQUALS( - "[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n", - "", + ASSERT_EQUALS( + "[test.cpp:7] -> [test.cpp:6] -> [test.cpp:8]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n", errout.str()); check("struct A {\n" diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 4f7a6c10f..b9106094d 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3441,6 +3441,18 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + // #9711 + check("int main(int argc, char* argv[]) {\n" + " int foo = 0;\n" + " struct option options[] = {\n" + " {\"foo\", no_argument, &foo, \'f\'},\n" + " {NULL, 0, NULL, 0},\n" + " };\n" + " getopt_long(argc, argv, \"f\", options, NULL);\n" + " if (foo) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } void alwaysTrueInfer() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 7b5bdbe07..12ab967a2 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -182,6 +182,24 @@ private: return false; } + bool testValueOfXInconclusive(const char code[], unsigned int linenr, int value) { + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { + if (tok->str() == "x" && tok->linenr() == linenr) { + for (const ValueFlow::Value& val:tok->values()) { + if (val.isInconclusive() && val.intvalue == value) + return true; + } + } + } + + return false; + } + bool testValueOfX(const char code[], unsigned int linenr, int value) { // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -2106,6 +2124,28 @@ private: "Fred::Fred(std::unique_ptr wilma)\n" " : mWilma(std::move(wilma)) {}\n"; ASSERT_EQUALS(0, tokenValues(code, "mWilma (").size()); + + code = "void g(unknown*);\n" + "int f() {\n" + " int a = 1;\n" + " unknown c[] = {{&a}};\n" + " g(c);\n" + " int x = a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, 1)); + ASSERT_EQUALS(true, testValueOfXInconclusive(code, 7U, 1)); + + code = "void g(unknown&);\n" + "int f() {\n" + " int a = 1;\n" + " unknown c{&a};\n" + " g(c);\n" + " int x = a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, 1)); + ASSERT_EQUALS(true, testValueOfXInconclusive(code, 7U, 1)); } void valueFlowAfterCondition() {