From 45dcfad9f9190ffcc21fc3c98b5174981ccee713 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 15 Dec 2018 17:58:45 +0100 Subject: [PATCH] Fix issue 8899: False positive returnDanglingLifetime when returning by value This fixes the FP from: ```cpp #include class MyString { public: MyString(char* source) { length = strlen( source ); buffer = new char[length+1]; if( buffer ) { strcpy( buffer, source ); } } char* buffer; int length; }; MyString Foo() { char arr[20]; sprintf(arr, "hello world"); return arr; } void main() { MyString str = Foo(); printf(str.buffer); } ``` --- lib/valueflow.cpp | 26 +++++++++++++++++++++-- test/testautovariables.cpp | 42 +++++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index eba71bb64..d2716a46e 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2743,6 +2743,27 @@ struct Lambda { } }; +static bool isDecayedPointer(const Token *tok, const Settings *settings) +{ + if (!tok) + return false; + if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return")) + return true; + if (!Token::simpleMatch(tok->astParent(), "return")) + return false; + if (!tok->scope()) + return false; + if (!tok->scope()->function) + return false; + if (!tok->scope()->function->retDef) + return false; + // TODO: Add valuetypes to return types of functions + ValueType vt = ValueType::parseDecl(tok->scope()->function->retDef, settings); + if (vt.pointer > 0) + return true; + return false; +} + static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger *errorLogger, const Settings *settings) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { @@ -2846,8 +2867,9 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger const Variable * var = getLifetimeVariable(tok, errorPath); if (!var) continue; - if (var->isArray() && !var->isStlType() && !var->isArgument() && tok->astParent() && - (astIsPointer(tok->astParent()) || Token::Match(tok->astParent(), "%assign%|return"))) { + if (var->nameToken() == tok) + continue; + if (var->isArray() && !var->isStlType() && !var->isArgument() && isDecayedPointer(tok, settings)) { errorPath.emplace_back(tok, "Array decayed to pointer here."); ValueFlow::Value value; diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp index 296ffbe4d..841a56c68 100644 --- a/test/testautovariables.cpp +++ b/test/testautovariables.cpp @@ -717,7 +717,7 @@ private: " return str;\n" "}"); ASSERT_EQUALS( - "[test.cpp:3] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", + "[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str()); check("char *foo()\n" // use ValueFlow @@ -727,7 +727,7 @@ private: " return p;\n" "}"); ASSERT_EQUALS( - "[test.cpp:3] -> [test.cpp:3] -> [test.cpp:5]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", + "[test.cpp:4] -> [test.cpp:3] -> [test.cpp:5]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str()); check("class Fred {\n" @@ -739,7 +739,7 @@ private: " return str;\n" "}"); ASSERT_EQUALS( - "[test.cpp:6] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", + "[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning pointer to local variable 'str' that will be invalid when returning.\n", errout.str()); check("char * format_reg(char *outbuffer_start) {\n" @@ -795,7 +795,7 @@ private: " return x+5;\n" "}"); ASSERT_EQUALS( - "[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str()); check("char *foo(int y) {\n" @@ -803,7 +803,7 @@ private: " return (x+8)-y;\n" "}"); ASSERT_EQUALS( - "[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str()); } @@ -813,7 +813,7 @@ private: " return (char *)x;\n" "}"); ASSERT_EQUALS( - "[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", + "[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning pointer to local variable 'x' that will be invalid when returning.\n", errout.str()); } @@ -1533,6 +1533,15 @@ private: "}\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("struct e {};\n" + "e * j() {\n" + " e c[20];\n" + " return c;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning pointer to local variable 'c' 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" @@ -1605,6 +1614,25 @@ private: " }\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + check("struct a {\n" + " a(char* b) {}\n" + "};\n" + "a f() {\n" + " char c[20];\n" + " return c;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct a {\n" + " a(char* b) {}\n" + "};\n" + "a g() {\n" + " char c[20];\n" + " a d = c;\n" + " return d;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void danglingLifetimeFunction() { @@ -1652,7 +1680,7 @@ private: " x[3];\n" "}\n"); ASSERT_EQUALS( - "[test.cpp:4] -> [test.cpp:4] -> [test.cpp:7]: (error) Using pointer to local variable 'y' that is out of scope.\n", + "[test.cpp:5] -> [test.cpp:4] -> [test.cpp:7]: (error) Using pointer to local variable 'y' that is out of scope.\n", errout.str()); check("void foo(int a) {\n"