diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e1c8ff2d9..95785254e 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -136,11 +136,31 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp, return ret; } +static bool hasToken(const Token * startTok, const Token * stopTok, const Token * tok) +{ + for(const Token * tok2 = startTok;tok2 != stopTok;tok2 = tok2->next()) { + if(tok2 == tok) + return true; + } + return false; +} + const Token * nextAfterAstRightmostLeaf(const Token * tok) { - if (!tok || !tok->astOperand1()) + const Token * rightmostLeaf = tok; + if (!rightmostLeaf || !rightmostLeaf->astOperand1()) return nullptr; - return tok->findExpressionStartEndTokens().second->next(); + do { + if (rightmostLeaf->astOperand2()) + rightmostLeaf = rightmostLeaf->astOperand2(); + else + rightmostLeaf = rightmostLeaf->astOperand1(); + } while (rightmostLeaf->astOperand1()); + while(Token::Match(rightmostLeaf->next(), "]|)") && !hasToken(rightmostLeaf->next()->link(), rightmostLeaf->next(), tok)) + rightmostLeaf = rightmostLeaf->next(); + if(rightmostLeaf->str() == "{" && rightmostLeaf->link()) + rightmostLeaf = rightmostLeaf->link(); + return rightmostLeaf->next(); } static const Token * getVariableInitExpression(const Variable * var) @@ -943,6 +963,10 @@ const Token *findLambdaEndToken(const Token *first) { if (!first || first->str() != "[") return nullptr; + if(!Token::Match(first->link(), "] (|{")) + return nullptr; + if(first->astOperand1() != first->link()->next()) + return nullptr; const Token * tok = first; if (tok->astOperand1() && tok->astOperand1()->str() == "(") diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index e6c5bcc57..daf0baf1d 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -33,6 +33,7 @@ #include #include +#include //--------------------------------------------------------------------------- @@ -587,6 +588,120 @@ void CheckAutoVariables::returnReference() } } +static bool isInScope(const Token * tok, const Scope * scope) +{ + if(!tok) + return false; + if(!scope) + return false; + const Variable * var = tok->variable(); + if(var && (var->isGlobal() || var->isStatic() || var->isExtern())) + return false; + if(tok->scope() && tok->scope()->isNestedIn(scope)) + return true; + if(!var) + return false; + if(var->isArgument() && !var->isReference()) { + const Scope * tokScope = tok->scope(); + if(!tokScope) + return false; + for(const Scope * argScope:tokScope->nestedList) { + if(argScope && argScope->isNestedIn(scope)) + return true; + } + } + return false; +} + +void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end) +{ + if(!start) + return; + const Scope * scope = start->scope(); + if(!scope) + return; + // If the scope is not set correctly then skip checking it + if(scope->bodyStart != start) + return; + for (const Token *tok = start; tok && tok != end; tok = tok->next()) { + // Skip duplicate warning from dangling references + if(Token::Match(tok, "& %var%")) + continue; + if(tok->variable() && tok->variable()->isPointer()) + continue; + if(std::any_of(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isTokValue))) + continue; + + for(const ValueFlow::Value& val:tok->values()) { + if(!val.isLifetimeValue()) + continue; + if(Token::Match(tok->astParent(), "return|throw")) { + if (isInScope(val.tokvalue, scope)) { + errorReturnDanglingLifetime(tok, &val); + break; + } + } + } + const Token *lambdaEndToken = findLambdaEndToken(tok); + if(lambdaEndToken) { + checkVarLifetimeScope(lambdaEndToken->link(), lambdaEndToken); + tok = lambdaEndToken; + } + } +} + +void CheckAutoVariables::checkVarLifetime() +{ + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope * scope : symbolDatabase->functionScopes) { + if (!scope->function) + continue; + // Skip if returning a container + const Library::Container * container = mSettings->library.detectContainer(scope->function->retDef); + if (container) + continue; + checkVarLifetimeScope(scope->bodyStart, scope->bodyEnd); + } +} + +void CheckAutoVariables::errorReturnDanglingLifetime(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 = "Returning object"; + break; + case ValueFlow::Value::Lambda: + msg = "Returning lambda"; + break; + case ValueFlow::Value::Iterator: + msg = "Returning 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, "returnDanglingLifetime", msg + " that will be invalid when returning.", CWE562, false); +} + void CheckAutoVariables::errorReturnReference(const Token *tok) { reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false); diff --git a/lib/checkautovariables.h b/lib/checkautovariables.h index c6b309661..5aa3d278c 100644 --- a/lib/checkautovariables.h +++ b/lib/checkautovariables.h @@ -53,6 +53,7 @@ public: CheckAutoVariables checkAutoVariables(tokenizer, settings, errorLogger); checkAutoVariables.assignFunctionArg(); checkAutoVariables.returnReference(); + checkAutoVariables.checkVarLifetime(); } void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) override { @@ -73,6 +74,10 @@ public: /** Returning reference to local/temporary variable */ void returnReference(); + void checkVarLifetime(); + + void checkVarLifetimeScope(const Token * start, const Token * end); + private: /** * Returning a temporary object? @@ -87,6 +92,7 @@ private: void errorAssignAddressOfLocalVariableToGlobalPointer(const Token *pointer, const Token *variable); void errorReturnPointerToLocalArray(const Token *tok); void errorAutoVariableAssignment(const Token *tok, bool inconclusive); + void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val); void errorReturnReference(const Token *tok); void errorReturnTempReference(const Token *tok); void errorInvalidDeallocation(const Token *tok, const ValueFlow::Value *val); diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index cde12f7ee..1a35112d7 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -959,6 +959,20 @@ public: return nullptr; } + bool isNestedIn(const Scope * outer) const + { + if(!outer) + return false; + if(outer == this) + return true; + const Scope * parent = nestedIn; + while(outer != parent && parent) + parent = parent->nestedIn; + if(parent && parent == outer) + return true; + return false; + } + bool isClassOrStruct() const { return (type == eClass || type == eStruct); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 54d9eeef6..5e8521f2a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1872,7 +1872,8 @@ static bool valueFlowForward(Token * const startToken, // TODO: handle lambda functions if (Token::simpleMatch(tok2, "= [")) { Token *lambdaEndToken = const_cast(findLambdaEndToken(tok2->next())); - if (lambdaEndToken) { + // Dont skip lambdas for lifetime values + if (lambdaEndToken && !std::all_of(values.begin(), values.end(), std::mem_fn(&ValueFlow::Value::isLifetimeValue))) { tok2 = lambdaEndToken; continue; } @@ -2897,6 +2898,218 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol } } +static bool isNotLifetimeValue(const ValueFlow::Value& val) +{ + return !val.isLifetimeValue(); +} + +static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings) +{ + const Token * assignTok = tok->astParent(); + // Assignment + if (!assignTok || (assignTok->str() != "=") || assignTok->astParent()) + return; + + // Lhs should be a variable + if (!assignTok->astOperand1() || !assignTok->astOperand1()->varId()) + return; + const Variable *var = assignTok->astOperand1()->variable(); + if (!var || (!var->isLocal() && !var->isGlobal() && !var->isArgument())) + return; + + const Token * const endOfVarScope = var->typeStartToken()->scope()->bodyEnd; + + // Rhs values.. + if (!assignTok->astOperand2() || assignTok->astOperand2()->values().empty()) + return; + + std::list values = assignTok->astOperand2()->values(); + + // Static variable initialisation? + if (var->isStatic() && var->nameToken() == assignTok->astOperand1()) + changeKnownToPossible(values); + + // Skip RHS + const Token * nextExpression = nextAfterAstRightmostLeaf(assignTok); + + // Only forward lifetime values + values.remove_if(&isNotLifetimeValue); + valueFlowForward(const_cast(nextExpression), endOfVarScope, var, var->declarationId(), values, false, false, tokenlist, errorLogger, settings); +} + +static const Variable * getLifetimeVariable(const Token * tok, ErrorPath& errorPath) +{ + const Variable * var = tok->variable(); + if(!var) + return nullptr; + if(var->isReference() || var->isRValueReference()) { + for(const ValueFlow::Value& v:tok->values()) { + if(!v.isLifetimeValue() && !v.tokvalue) + continue; + errorPath.insert(errorPath.end(), v.errorPath.begin(), v.errorPath.end()); + const Variable * var2 = getLifetimeVariable(v.tokvalue, errorPath); + if(var2) + return var2; + } + return nullptr; + } + return var; +} + +struct Lambda +{ + explicit Lambda(const Token * tok) + : capture(nullptr), arguments(nullptr), returnTok(nullptr), bodyTok(nullptr) + { + if(!Token::simpleMatch(tok, "[") || !tok->link()) + return; + capture = tok; + + if(Token::simpleMatch(capture->link(), "] (")) { + arguments = capture->link()->next(); + } + const Token * afterArguments = arguments ? arguments->link()->next() : capture->link()->next(); + if(afterArguments && afterArguments->originalName() == "->") { + returnTok = afterArguments->next(); + bodyTok = Token::findsimplematch(returnTok, "{"); + } else if(Token::simpleMatch(afterArguments, "{")) { + bodyTok = afterArguments; + } + } + + const Token * capture; + const Token * arguments; + const Token * returnTok; + const Token * bodyTok; + + bool isLambda() const + { + return capture && bodyTok; + } +}; + +static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +{ + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { + Lambda lam(tok); + // Lamdas + if(lam.isLambda()) { + const Scope * bodyScope = lam.bodyTok->scope(); + + std::set scopes; + + // TODO: Handle explicit capture + bool captureByRef = Token::Match(lam.capture, "[ & ]"); + bool captureByValue = Token::Match(lam.capture, "[ = ]"); + + for(const Token * tok2 = lam.bodyTok;tok2 != lam.bodyTok->link();tok2 = tok2->next()) { + ErrorPath errorPath; + if(captureByRef) { + const Variable * var = getLifetimeVariable(tok2, errorPath); + if(!var) + continue; + const Scope * scope = var->scope(); + if(scopes.count(scope) > 0) + continue; + if(scope->isNestedIn(bodyScope)) + continue; + scopes.insert(scope); + errorPath.emplace_back(tok2, "Lambda captures variable by reference here."); + + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.tokvalue = var->nameToken(); + value.errorPath = errorPath; + value.lifetimeKind = ValueFlow::Value::Lambda; + setTokenValue(tok, value, tokenlist->getSettings()); + + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } else if(captureByValue) { + for(const ValueFlow::Value& v:tok2->values()) { + if(!v.isLifetimeValue() && !v.tokvalue) + continue; + const Token * tok3 = v.tokvalue; + errorPath = v.errorPath; + const Variable * var = getLifetimeVariable(tok3, errorPath); + if(!var) + continue; + const Scope * scope = var->scope(); + if(scopes.count(scope) > 0) + continue; + if(scope->isNestedIn(bodyScope)) + continue; + scopes.insert(scope); + errorPath.emplace_back(tok2, "Lambda captures variable by value here."); + + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.tokvalue = var->nameToken(); + value.errorPath = errorPath; + value.lifetimeKind = ValueFlow::Value::Lambda; + setTokenValue(tok, value, tokenlist->getSettings()); + + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } + } + } + } + // address of + else if (tok->isUnaryOp("&")) { + ErrorPath errorPath; + // child should be some buffer or variable + const Token *vartok = tok->astOperand1(); + while (vartok) { + if (vartok->str() == "[") + vartok = vartok->astOperand1(); + else if (vartok->str() == "." || vartok->str() == "::") + vartok = vartok->astOperand2(); + else + break; + } + + if(!vartok) + continue; + const Variable * var = getLifetimeVariable(vartok, errorPath); + if (!(var && !var->isPointer())) + continue; + + errorPath.emplace_back(tok, "Address of variable taken here."); + + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.tokvalue = var->nameToken(); + value.errorPath = errorPath; + setTokenValue(tok, value, tokenlist->getSettings()); + + valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings); + } + // container lifetimes + else if (tok->variable() && Token::Match(tok, "%var% . begin|cbegin|rbegin|crbegin|end|cend|rend|crend|data|c_str (")) { + ErrorPath errorPath; + const Library::Container * container = settings->library.detectContainer(tok->variable()->typeStartToken()); + if(!container) + continue; + const Variable * var = tok->variable(); + + bool isIterator = !Token::Match(tok->tokAt(2), "data|c_str"); + if(isIterator) + errorPath.emplace_back(tok, "Iterator to container is created here."); + else + errorPath.emplace_back(tok, "Pointer to container is created here."); + + ValueFlow::Value value; + value.valueType = ValueFlow::Value::LIFETIME; + value.tokvalue = var->nameToken(); + value.errorPath = errorPath; + value.lifetimeKind = isIterator ? ValueFlow::Value::Iterator : ValueFlow::Value::Object; + setTokenValue(tok->tokAt(3), value, tokenlist->getSettings()); + + valueFlowForwardLifetime(tok->tokAt(3), tokenlist, errorLogger, settings); + + } + } +} + static void execute(const Token *expr, ProgramMemory * const programMemory, MathLib::bigint *result, @@ -4050,6 +4263,7 @@ ValueFlow::Value::Value(const Token *c, long long val) varId(0U), conditional(false), defaultArg(false), + lifetimeKind(Object), valueKind(ValueKind::Possible) { errorPath.emplace_back(c, "Assuming that condition '" + c->expressionString() + "' is not redundant"); @@ -4070,6 +4284,8 @@ std::string ValueFlow::Value::infoString() const return ""; case CONTAINER_SIZE: return "size=" + MathLib::toString(intvalue); + case LIFETIME: + return "lifetime=" + tokvalue->str(); }; throw InternalError(nullptr, "Invalid ValueFlow Value type"); } @@ -4102,6 +4318,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowArray(tokenlist); valueFlowGlobalStaticVar(tokenlist, settings); valueFlowPointerAlias(tokenlist); + valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings); valueFlowFunctionReturn(tokenlist, errorLogger); valueFlowBitAnd(tokenlist); diff --git a/lib/valueflow.h b/lib/valueflow.h index f8320f00e..b10edf166 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -39,7 +39,7 @@ namespace ValueFlow { typedef std::pair ErrorPathItem; typedef std::list ErrorPath; - explicit Value(long long val = 0) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(NonMovedVariable), varvalue(val), condition(nullptr), varId(0U), conditional(false), defaultArg(false), valueKind(ValueKind::Possible) {} + explicit Value(long long val = 0) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(NonMovedVariable), varvalue(val), condition(nullptr), varId(0U), conditional(false), defaultArg(false), lifetimeKind(Object), valueKind(ValueKind::Possible) {} Value(const Token *c, long long val); bool operator==(const Value &rhs) const { @@ -68,6 +68,10 @@ namespace ValueFlow { case CONTAINER_SIZE: if (intvalue != rhs.intvalue) return false; + break; + case LIFETIME: + if (tokvalue != rhs.tokvalue) + return false; }; return varvalue == rhs.varvalue && @@ -80,7 +84,7 @@ namespace ValueFlow { std::string infoString() const; - enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE } valueType; + enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME } valueType; bool isIntValue() const { return valueType == INT; } @@ -99,6 +103,9 @@ namespace ValueFlow { bool isContainerSizeValue() const { return valueType == CONTAINER_SIZE; } + bool isLifetimeValue() const { + return valueType == LIFETIME; + } /** int value */ long long intvalue; @@ -128,6 +135,8 @@ namespace ValueFlow { /** Is this value passed as default parameter to the function? */ bool defaultArg; + + enum LifetimeKind {Object, Lambda, Iterator} lifetimeKind; static const char * toString(MoveKind moveKind) { switch (moveKind) { diff --git a/test/testastutils.cpp b/test/testastutils.cpp index 6ff882fbd..25200def1 100644 --- a/test/testastutils.cpp +++ b/test/testastutils.cpp @@ -37,6 +37,7 @@ private: TEST_CASE(isReturnScope); TEST_CASE(isVariableChanged); TEST_CASE(isVariableChangedByFunctionCall); + TEST_CASE(nextAfterAstRightmostLeaf); } bool findLambdaEndToken(const char code[]) { @@ -129,6 +130,29 @@ private: ASSERT_EQUALS(false, isVariableChangedByFunctionCall(code, "x ) ;", &inconclusive)); ASSERT_EQUALS(true, inconclusive); } + + bool nextAfterAstRightmostLeaf(const char code[], const char parentPattern[], const char rightPattern[]) { + Settings settings; + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + const Token * tok = Token::findsimplematch(tokenizer.tokens(), parentPattern); + return Token::simpleMatch(::nextAfterAstRightmostLeaf(tok), rightPattern); + } + + void nextAfterAstRightmostLeaf() { + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("void f(int a, int b) { int x = a + b; }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a); }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a)[b]; }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(g(a)[b]); }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(g(a)[b] + a); }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a)[b + 1]; }", "=", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("void f() { int a; int b; int x = [](int a){}; }", "=", "; }")); + + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = a + b; }", "+", "; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a)[b + 1]; }", "+", "] ; }")); + ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a + 1)[b]; }", "+", ") [")); + } }; REGISTER_TEST(TestAstUtils) diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 205f3fed0..82162235a 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -45,6 +45,7 @@ private: CheckAutoVariables checkAutoVariables(&tokenizer, &settings, this); checkAutoVariables.returnReference(); checkAutoVariables.assignFunctionArg(); + checkAutoVariables.checkVarLifetime(); if (runSimpleChecks) { tokenizer.simplifyTokenList2(); @@ -118,6 +119,10 @@ private: TEST_CASE(testconstructor); // ticket #5478 - crash TEST_CASE(variableIsUsedInScope); // ticket #5599 crash in variableIsUsedInScope() + + TEST_CASE(danglingLifetimeLambda); + TEST_CASE(danglingLifetimeContainer); + TEST_CASE(danglingLifetime); } @@ -1197,6 +1202,177 @@ private: "}"); } + void danglingLifetimeLambda() { + check("auto f() {\n" + " int a = 1;\n" + " auto l = [&](){ return a; };\n" + " return l;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto f() {\n" + " int a = 1;\n" + " return [&](){ return a; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto f(int a) {\n" + " return [&](){ return a; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1] -> [test.cpp:2]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto f(int a) {\n" + " auto p = &a;\n" + " return [=](){ return p; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto g(int& a) {\n" + " int p = a;\n" + " return [&](){ return p; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'p' that will be invalid when returning.\n", errout.str()); + + check("auto f() {\n" + " return [=](){\n" + " int a = 1;\n" + " return [&](){ return a; };\n" + " };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + // TODO: Variable is not set correctly for this case + check("auto f(int b) {\n" + " return [=](int a){\n" + " a += b;\n" + " return [&](){ return a; };\n" + " };\n" + "}\n"); + TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", "", errout.str()); + + check("auto g(int& a) {\n" + " return [&](){ return a; };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("auto g(int a) {\n" + " auto p = a;\n" + " return [=](){ return p; };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("auto g(int& a) {\n" + " auto p = a;\n" + " return [=](){ return p; };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("auto g(int& a) {\n" + " int& p = a;\n" + " return [&](){ return p; };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "void g(F);\n" + "auto f() {\n" + " int x;\n" + " return g([&]() { return x; });\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void danglingLifetimeContainer() { + check("auto f(const std::vector& x) {\n" + " auto it = x.begin();\n" + " return it;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("auto f() {\n" + " std::vector x;\n" + " auto it = x.begin();\n" + " return it;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str()); + + check("auto f() {\n" + " std::vector x;\n" + " auto p = x.data();\n" + " return p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning object that points to local variable 'x' that will be invalid when returning.\n", errout.str()); + + check("auto f(std::vector x) {\n" + " auto it = x.begin();\n" + " return it;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str()); + + check("auto f() {\n" + " static std::vector x;\n" + " return x.begin();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("std::string g() {\n" + " std::vector v;\n" + " return v.data();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("auto g() {\n" + " std::vector v;\n" + " return {v, [v]() { return v.data(); }};\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "void g(F);\n" + "auto f() {\n" + " std::vector v;\n" + " return g([&]() { return v.data(); });\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void danglingLifetime() { + check("auto f() {\n" + " std::vector a;\n" + " auto it = a.begin();\n" + " return [=](){ return it; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto f(std::vector a) {\n" + " auto it = a.begin();\n" + " return [=](){ return it; };\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'a' that will be invalid when returning.\n", errout.str()); + + check("auto f(std::vector& a) {\n" + " auto it = a.begin();\n" + " return [=](){ return it; };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // Make sure we dont hang + check("struct A;\n" + "void f() {\n" + " using T = A[3];\n" + " A &&a = T{1, 2, 3}[1];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + // Make sure we dont hang + check("struct A;\n" + "void f() {\n" + " using T = A[3];\n" + " A &&a = T{1, 2, 3}[1]();\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + }; REGISTER_TEST(TestAutoVariables) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 4af39b42e..421f1d766 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -53,6 +53,7 @@ private: TEST_CASE(valueFlowNumber); TEST_CASE(valueFlowString); TEST_CASE(valueFlowPointerAlias); + TEST_CASE(valueFlowLifetime); TEST_CASE(valueFlowArrayElement); TEST_CASE(valueFlowMove); @@ -193,7 +194,7 @@ private: return ""; } - bool testValueOfX(const char code[], unsigned int linenr, const char value[]) { + bool testValueOfX(const char code[], unsigned int linenr, const char value[], ValueFlow::Value::ValueType type) { // Tokenize.. Tokenizer tokenizer(&settings, this); std::istringstream istr(code); @@ -203,7 +204,7 @@ private: if (tok->str() == "x" && tok->linenr() == linenr) { std::list::const_iterator it; for (it = tok->values().begin(); it != tok->values().end(); ++it) { - if (it->isTokValue() && Token::simpleMatch(it->tokvalue, value)) + if (it->valueType == type && Token::simpleMatch(it->tokvalue, value)) return true; } } @@ -305,7 +306,7 @@ private: " if (a) x = \"123\";\n" " return x;\n" "}"; - ASSERT_EQUALS(true, testValueOfX(code, 4, "\"123\"")); + ASSERT_EQUALS(true, testValueOfX(code, 4, "\"123\"", ValueFlow::Value::TOK)); // valueFlowSubFunction code = "void dostuff(const char *x) {\n" @@ -313,7 +314,7 @@ private: "}\n" "\n" "void test() { dostuff(\"abc\"); }"; - ASSERT_EQUALS(true, testValueOfX(code, 2, "\"abc\"")); + ASSERT_EQUALS(true, testValueOfX(code, 2, "\"abc\"", ValueFlow::Value::TOK)); } void valueFlowPointerAlias() { @@ -325,7 +326,7 @@ private: " if (a) x = &ret[0];\n" " return x;\n" "}"; - ASSERT_EQUALS(true, testValueOfX(code, 5, "& ret [ 0 ]")); + ASSERT_EQUALS(true, testValueOfX(code, 5, "& ret [ 0 ]", ValueFlow::Value::TOK)); // dead pointer code = "void f() {\n" @@ -333,7 +334,7 @@ private: " if (cond) { int i; x = &i; }\n" " *x = 0;\n" // <- x can point at i "}"; - ASSERT_EQUALS(true, testValueOfX(code, 4, "& i")); + ASSERT_EQUALS(true, testValueOfX(code, 4, "& i", ValueFlow::Value::TOK)); code = "void f() {\n" " struct X *x;\n" @@ -342,6 +343,41 @@ private: ASSERT_EQUALS(true, tokenValues(code, "&").empty()); ASSERT_EQUALS(true, tokenValues(code, "x [").empty()); } + + void valueFlowLifetime() { + const char *code; + + LOAD_LIB_2(settings.library, "std.cfg"); + + code = "void f() {\n" + " int a = 1;\n" + " auto x = [&]() { return a + 1; };\n" + " auto b = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4, "a ;", ValueFlow::Value::LIFETIME)); + + code = "void f() {\n" + " int a = 1;\n" + " auto x = [=]() { return a + 1; };\n" + " auto b = x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 4, "a ;", ValueFlow::Value::LIFETIME)); + + code = "void f(int v) {\n" + " int a = v;\n" + " int * p = &a;\n" + " auto x = [=]() { return p + 1; };\n" + " auto b = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 5, "a ;", ValueFlow::Value::LIFETIME)); + + code = "void f() {\n" + " std::vector v;\n" + " auto x = v.begin();\n" + " auto it = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4, "v ;", ValueFlow::Value::LIFETIME)); + } void valueFlowArrayElement() { const char *code; @@ -350,26 +386,26 @@ private: " const int x[] = {43,23,12};\n" " return x;\n" "}"; - ASSERT_EQUALS(true, testValueOfX(code, 3U, "{ 43 , 23 , 12 }")); + ASSERT_EQUALS(true, testValueOfX(code, 3U, "{ 43 , 23 , 12 }", ValueFlow::Value::TOK)); code = "void f() {\n" " const char x[] = \"abcd\";\n" " return x;\n" "}"; - ASSERT_EQUALS(true, testValueOfX(code, 3U, "\"abcd\"")); + ASSERT_EQUALS(true, testValueOfX(code, 3U, "\"abcd\"", ValueFlow::Value::TOK)); code = "void f() {\n" " char x[32] = \"abcd\";\n" " return x;\n" "}"; - TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 3U, "\"abcd\"")); + TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 3U, "\"abcd\"", ValueFlow::Value::TOK)); code = "void f() {\n" " int a[10];\n" " int *x = a;\n" // <- a value is a " *x = 0;\n" // .. => x value is a "}"; - ASSERT_EQUALS(true, testValueOfX(code, 4, "a")); + ASSERT_EQUALS(true, testValueOfX(code, 4, "a", ValueFlow::Value::TOK)); code = "char f() {\n" " const char *x = \"abcd\";\n" @@ -1394,7 +1430,7 @@ private: " a = ((x[0] == 'U') ?\n" " x[1] : 0);\n" // <- x is not "" "}"; - ASSERT_EQUALS(false, testValueOfX(code, 4U, "\"\"")); + ASSERT_EQUALS(false, testValueOfX(code, 4U, "\"\"", ValueFlow::Value::TOK)); code = "void f() {\n" // #6973 " char *x = getenv (\"LC_ALL\");\n" @@ -1407,10 +1443,10 @@ private: " x[2] ))\n" // x can't be "" " {}\n" "}\n"; - ASSERT_EQUALS(true, testValueOfX(code, 6U, "\"\"")); - ASSERT_EQUALS(false, testValueOfX(code, 7U, "\"\"")); - ASSERT_EQUALS(false, testValueOfX(code, 8U, "\"\"")); - ASSERT_EQUALS(false, testValueOfX(code, 9U, "\"\"")); + ASSERT_EQUALS(true, testValueOfX(code, 6U, "\"\"", ValueFlow::Value::TOK)); + ASSERT_EQUALS(false, testValueOfX(code, 7U, "\"\"", ValueFlow::Value::TOK)); + ASSERT_EQUALS(false, testValueOfX(code, 8U, "\"\"", ValueFlow::Value::TOK)); + ASSERT_EQUALS(false, testValueOfX(code, 9U, "\"\"", ValueFlow::Value::TOK)); code = "void f() {\n" // #7599 " t *x = 0;\n"