Fix issue 9496: False negative: Dereferencing returned smart null-pointer

lib/symboldatabase.cpp

@@ -21,6 +21,7 @@
 
 #include "astutils.h"
 #include "errorlogger.h"
+#include "library.h"
 #include "mathlib.h"
 #include "platform.h"
 #include "settings.h"
@@ -5968,7 +5969,8 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
                 valuetype.sign = ValueType::Sign::SIGNED;
             }
             setValueType(tok, valuetype);
-        } else if (tok->link() && tok->str() == "(") {
+        } else if (tok->link() && Token::Match(tok, "(|{")) {
+            const Token* start = tok->astOperand1() ? tok->astOperand1()->findExpressionStartEndTokens().first : nullptr;
             // cast
             if (tok->isCast() && !tok->astOperand2() && Token::Match(tok, "( %name%")) {
                 ValueType valuetype;
@@ -5983,6 +5985,16 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
                     setValueType(tok, valuetype);
             }
 
+            // Construct smart pointer
+            else if (mSettings->library.isSmartPointer(start)) {
+                ValueType valuetype;
+                if (parsedecl(start, &valuetype, mDefaultSignedness, mSettings)) {
+                    setValueType(tok, valuetype);
+                    setValueType(tok->astOperand1(), valuetype);
+                }
+
+            }
+
             // function
             else if (tok->previous() && tok->previous()->function() && tok->previous()->function()->retDef) {
                 ValueType valuetype;

lib/valueflow.cpp

@@ -5952,47 +5952,53 @@ static void valueFlowSmartPointer(TokenList *tokenlist, ErrorLogger * errorLogge
             continue;
         if (!tok->scope()->isExecutable())
             continue;
-        if (!tok->variable())
+        if (tok->variable()) {
-            continue;
+            const Variable * var = tok->variable();
-        const Variable * var = tok->variable();
+            if (!var->isSmartPointer())
-        if (!var->isSmartPointer())
+                continue;
-            continue;
+            if (var->nameToken() == tok) {
-        if (var->nameToken() == tok) {
+                if (Token::Match(tok, "%var% (|{") && tok->next()->astOperand2() && tok->next()->astOperand2()->str() != ",") {
-            if (Token::Match(tok, "%var% (|{") && tok->next()->astOperand2() && tok->next()->astOperand2()->str() != ",") {
+                    Token * inTok = tok->next()->astOperand2();
-                Token * inTok = tok->next()->astOperand2();
+                    std::list<ValueFlow::Value> values = inTok->values();
-                std::list<ValueFlow::Value> values = inTok->values();
+                    const bool constValue = inTok->isNumber();
-                const bool constValue = inTok->isNumber();
+                    valueFlowForwardAssign(inTok, var, values, constValue, true, tokenlist, errorLogger, settings);
-                valueFlowForwardAssign(inTok, var, values, constValue, true, tokenlist, errorLogger, settings);
 
-            } else if (Token::Match(tok, "%var% ;")) {
+                } else if (Token::Match(tok, "%var% ;")) {
-                std::list<ValueFlow::Value> values;
+                    std::list<ValueFlow::Value> values;
-                ValueFlow::Value v(0);
+                    ValueFlow::Value v(0);
-                v.setKnown();
+                    v.setKnown();
-                values.push_back(v);
+                    values.push_back(v);
-                valueFlowForwardAssign(tok, var, values, false, true, tokenlist, errorLogger, settings);
+                    valueFlowForwardAssign(tok, var, values, false, true, tokenlist, errorLogger, settings);
-            }
+                }
-        } else if (Token::Match(tok, "%var% . reset (") && tok->next()->originalName() != "->") {
+            } else if (Token::Match(tok, "%var% . reset (") && tok->next()->originalName() != "->") {
-            if (Token::simpleMatch(tok->tokAt(3), "( )")) {
+                if (Token::simpleMatch(tok->tokAt(3), "( )")) {
+                    std::list<ValueFlow::Value> values;
+                    ValueFlow::Value v(0);
+                    v.setKnown();
+                    values.push_back(v);
+                    valueFlowForwardAssign(tok->tokAt(4), var, values, false, false, tokenlist, errorLogger, settings);
+                } else {
+                    tok->removeValues(std::mem_fn(&ValueFlow::Value::isIntValue));
+                    Token * inTok = tok->tokAt(3)->astOperand2();
+                    if (!inTok)
+                        continue;
+                    std::list<ValueFlow::Value> values = inTok->values();
+                    const bool constValue = inTok->isNumber();
+                    valueFlowForwardAssign(inTok, var, values, constValue, false, tokenlist, errorLogger, settings);
+                }
+            } else if (Token::Match(tok, "%var% . release ( )") && tok->next()->originalName() != "->") {
                 std::list<ValueFlow::Value> values;
                 ValueFlow::Value v(0);
                 v.setKnown();
                 values.push_back(v);
                 valueFlowForwardAssign(tok->tokAt(4), var, values, false, false, tokenlist, errorLogger, settings);
-            } else {
-                tok->removeValues(std::mem_fn(&ValueFlow::Value::isIntValue));
-                Token * inTok = tok->tokAt(3)->astOperand2();
-                if (!inTok)
-                    continue;
-                std::list<ValueFlow::Value> values = inTok->values();
-                const bool constValue = inTok->isNumber();
-                valueFlowForwardAssign(inTok, var, values, constValue, false, tokenlist, errorLogger, settings);
             }
-        } else if (Token::Match(tok, "%var% . release ( )") && tok->next()->originalName() != "->") {
+        } else if (Token::Match(tok->previous(), "%name%|> (|{") && astIsSmartPointer(tok) && astIsSmartPointer(tok->astOperand1())) {
-            std::list<ValueFlow::Value> values;
+            std::vector<const Token*> args = getArguments(tok);
-            ValueFlow::Value v(0);
+            if (args.empty())
-            v.setKnown();
+                continue;
-            values.push_back(v);
+            for(const ValueFlow::Value& v:args.front()->values())
-            valueFlowForwardAssign(tok->tokAt(4), var, values, false, false, tokenlist, errorLogger, settings);
+                setTokenValue(tok, v, settings);
         }
     }
 }
@@ -6681,7 +6687,6 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
     valueFlowGlobalStaticVar(tokenlist, settings);
     valueFlowPointerAlias(tokenlist);
     valueFlowLifetime(tokenlist, symboldatabase, errorLogger, settings);
-    valueFlowFunctionReturn(tokenlist, errorLogger);
     valueFlowBitAnd(tokenlist);
     valueFlowSameExpressions(tokenlist);
     valueFlowFwdAnalysis(tokenlist, settings);
@@ -6703,6 +6708,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
         valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowSubFunction(tokenlist, symboldatabase, errorLogger, settings);
+        valueFlowFunctionReturn(tokenlist, errorLogger);
         valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
         valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
         if (tokenlist->isCPP()) {

test/testautovariables.cpp

@@ -1442,7 +1442,7 @@ private:
         check("auto& f() {\n"
               "    return std::vector<int>{1}.front();\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (error) Reference to temporary returned.\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (error) Reference to temporary returned.\n", errout.str());
 
         check("struct A { int foo; };\n"
               "int& f(std::vector<A> v) {\n"

test/testnullpointer.cpp

@@ -2859,6 +2859,15 @@ private:
               "    return x.release();\n"
               "}\n", true);
         ASSERT_EQUALS("", errout.str());
+
+        // #9496
+        check("std::shared_ptr<int> f() {\n"
+              "    return std::shared_ptr<int>(nullptr);\n"
+              "}\n"
+              "void g() {\n"
+              "    int a = *f();\n"
+              "}\n", true);
+        ASSERT_EQUALS("[test.cpp:5]: (error) Null pointer dereference: f()\n", errout.str());
     }
 
     void functioncall() {    // #3443 - function calls

test/testsimplifytypedef.cpp

@@ -1152,6 +1152,7 @@ private:
 
         ASSERT_EQUALS(expected, tok(code, false));
         ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+            "[test.cpp:31]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n"
             "[test.cpp:28]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n"
             , errout.str());
     }