Improve Fix #6180 (Access of moved variable still allowed until function is called)

Author: zingsheim

lib/token.h

@@ -779,7 +779,7 @@ class CPPCHECKLIB Token {
     const ValueFlow::Value * getMovedValue() const {
         std::list<ValueFlow::Value>::const_iterator it;
         for (it = values.begin(); it != values.end(); ++it) {
-            if (it->isMovedValue())
+            if (it->isMovedValue() && it->moveKind != ValueFlow::Value::NonMovedVariable)
                 return &(*it);
         }
         return nullptr;

lib/valueflow.cpp

@@ -1742,6 +1742,40 @@ static bool isOpenParenthesisMemberFunctionCallOfVarId(const Token * openParenth
            openParenthesisToken->tokAt(-2)->originalName() == emptyString;
 }
 
+static const Token * nextAfterAstRightmostLeaf(Token const * tok)
+{
+    const Token * rightmostLeaf = tok;
+    if (!rightmostLeaf || !rightmostLeaf->astOperand1())
+        return nullptr;
+    do {
+        if (rightmostLeaf->astOperand2())
+            rightmostLeaf = rightmostLeaf->astOperand2();
+        else
+            rightmostLeaf = rightmostLeaf->astOperand1();
+    } while (rightmostLeaf->astOperand1());
+    return rightmostLeaf->next();
+}
+
+static const Token * findOpenParentesisOfMove(const Token * moveVarTok)
+{
+    const Token * tok = moveVarTok;
+    while (tok && tok->str() != "(")
+        tok = tok->previous();
+    return tok;
+}
+
+static const Token * findEndOfFunctionCallForParameter(const Token * parameterToken)
+{
+    if (!parameterToken)
+        return nullptr;
+    const Token * parent = parameterToken->astParent();
+    while(parent && !parent->isOp() && parent->str() != "(")
+        parent = parent->astParent();
+    if (!parent)
+        return nullptr;
+    return nextAfterAstRightmostLeaf(parent);
+}
+
 static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
 {
     if (!tokenlist->isCPP() || settings->standards.cpp < Standards::CPP11)
@@ -1804,26 +1838,14 @@ static void valueFlowAfterMove(TokenList *tokenlist, SymbolDatabase* symboldatab
             value.setKnown();
             std::list<ValueFlow::Value> values;
             values.push_back(value);
-
-            valueFlowForward(varTok->next(), endOfVarScope, var, varId, values, false, tokenlist, errorLogger, settings);
+            const Token * openParentesisOfMove = findOpenParentesisOfMove(varTok);
+            const Token * endOfFunctionCall = findEndOfFunctionCallForParameter(openParentesisOfMove);
+            if (endOfFunctionCall)
+                valueFlowForward(const_cast<Token *>(endOfFunctionCall), endOfVarScope, var, varId, values, false, tokenlist, errorLogger, settings);
         }
     }
 }
 
-static const Token * nextAfterAstRightmostLeaf(Token const * tok)
-{
-    const Token * rightmostLeaf = tok;
-    if (!rightmostLeaf || !rightmostLeaf->astOperand1())
-        return nullptr;
-    do {
-        if (rightmostLeaf->astOperand2())
-            rightmostLeaf = rightmostLeaf->astOperand2();
-        else
-            rightmostLeaf = rightmostLeaf->astOperand1();
-    } while (rightmostLeaf->astOperand1());
-    return rightmostLeaf->next();
-}
-
 static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
 {
     const std::size_t functions = symboldatabase->functionScopes.size();

lib/valueflow.h

@@ -33,8 +33,8 @@ class Settings;
 namespace ValueFlow {
     class CPPCHECKLIB Value {
     public:
-        explicit Value(long long val = 0) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(MovedVariable), varvalue(val), condition(0), varId(0U), conditional(false), inconclusive(false), defaultArg(false), valueKind(ValueKind::Possible) {}
-        Value(const Token *c, long long val) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(MovedVariable), varvalue(val), condition(c), varId(0U), conditional(false), inconclusive(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(0), varId(0U), conditional(false), inconclusive(false), defaultArg(false), valueKind(ValueKind::Possible) {}
+        Value(const Token *c, long long val) : valueType(INT), intvalue(val), tokvalue(nullptr), floatValue(0.0), moveKind(NonMovedVariable), varvalue(val), condition(c), varId(0U), conditional(false), inconclusive(false), defaultArg(false), valueKind(ValueKind::Possible) {}
 
         bool operator==(const Value &rhs) const {
             if (valueType != rhs.valueType)

test/testvalueflow.cpp

@@ -338,6 +338,13 @@ class TestValueFlow : public TestFixture {
                "}";
         ASSERT_EQUALS(true, testValueOfX(code, 4U, ValueFlow::Value::MovedVariable));
 
+        code = "void f(int i) {\n"
+               "    X x;\n"
+               "    y = g(std::move(x), \n"
+               "          x.size());\n"
+               "}";
+        ASSERT_EQUALS(false, testValueOfX(code, 4U, ValueFlow::Value::MovedVariable));
+
         code = "void f(int i) {\n"
                "    X x;\n"
                "    x = g(std::move(x));\n"
@@ -352,7 +359,6 @@ class TestValueFlow : public TestFixture {
                "    return h(std::move(x));\n"
                "}";
         ASSERT_EQUALS(false, testValueOfX(code, 5U, ValueFlow::Value::MovedVariable));
-
     }
 
     void valueFlowCalculations() {