From cb5a5e6a25f05c82392f8fe6b72166d9f7ff7a20 Mon Sep 17 00:00:00 2001 From: Frank Zingsheim Date: Mon, 12 Dec 2016 21:46:05 +0100 Subject: [PATCH] Improve Fix #6180 (Access of moved variable still allowed until function is called) --- lib/token.h | 2 +- lib/valueflow.cpp | 54 +++++++++++++++++++++++++++++------------- lib/valueflow.h | 4 ++-- test/testvalueflow.cpp | 8 ++++++- 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/lib/token.h b/lib/token.h index 653b9728b..a00940c77 100644 --- a/lib/token.h +++ b/lib/token.h @@ -779,7 +779,7 @@ public: const ValueFlow::Value * getMovedValue() const { std::list::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; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 888b9c53b..334d9f9c1 100644 --- a/lib/valueflow.cpp +++ b/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 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(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(); diff --git a/lib/valueflow.h b/lib/valueflow.h index 6368e70be..d8516b56d 100644 --- a/lib/valueflow.h +++ b/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) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 328b3ecd8..4db727485 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -338,6 +338,13 @@ private: "}"; 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 @@ private: " return h(std::move(x));\n" "}"; ASSERT_EQUALS(false, testValueOfX(code, 5U, ValueFlow::Value::MovedVariable)); - } void valueFlowCalculations() {