Skip to content

Commit d3f0aa5

Browse files
pfultz2pfultz2
authored
Fix 10033: false negative: danglingTemporaryLifetime with usage of reference from nested object not detected (cppcheck-opensource#3542)
1 parent 7d7584b commit d3f0aa5

6 files changed

Lines changed: 58 additions & 23 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,8 +491,11 @@ const Token* getParentMember(const Token * tok)
491491
const Token * parent = tok->astParent();
492492
if (!Token::simpleMatch(parent, "."))
493493
return tok;
494-
if (tok == parent->astOperand2())
494+
if (astIsRHS(tok)) {
495+
if (Token::simpleMatch(parent->astOperand1(), "."))
496+
return parent->astOperand1()->astOperand2();
495497
return parent->astOperand1();
498+
}
496499
const Token * gparent = parent->astParent();
497500
if (!Token::simpleMatch(gparent, ".") || gparent->astOperand2() != parent)
498501
return tok;
@@ -2534,6 +2537,8 @@ static void getLHSVariablesRecursive(std::vector<const Variable*>& vars, const T
25342537
} else if (Token::simpleMatch(tok, ".")) {
25352538
getLHSVariablesRecursive(vars, tok->astOperand1());
25362539
getLHSVariablesRecursive(vars, tok->astOperand2());
2540+
} else if (Token::simpleMatch(tok, "::")) {
2541+
getLHSVariablesRecursive(vars, tok->astOperand2());
25372542
} else if (tok->variable()) {
25382543
vars.push_back(tok->variable());
25392544
}

‎lib/checkautovariables.cpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
597597
break;
598598
} else if (!tokvalue->variable() &&
599599
isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) {
600-
errorDanglingTemporaryLifetime(tok, &val);
600+
errorDanglingTemporaryLifetime(tok, &val, tokvalue);
601601
break;
602602
}
603603
}
@@ -679,13 +679,19 @@ void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow:
679679
reportError(errorPath, Severity::error, "invalidLifetime", msg + " that is out of scope.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
680680
}
681681

682-
void CheckAutoVariables::errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val)
682+
void CheckAutoVariables::errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val, const Token* tempTok)
683683
{
684684
const bool inconclusive = val ? val->isInconclusive() : false;
685685
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
686686
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
687+
errorPath.emplace_back(tempTok, "Temporary created here.");
687688
errorPath.emplace_back(tok, "");
688-
reportError(errorPath, Severity::error, "danglingTemporaryLifetime", msg + " to temporary.", CWE562, inconclusive ? Certainty::inconclusive : Certainty::normal);
689+
reportError(errorPath,
690+
Severity::error,
691+
"danglingTemporaryLifetime",
692+
msg + " that is a temporary.",
693+
CWE562,
694+
inconclusive ? Certainty::inconclusive : Certainty::normal);
689695
}
690696

691697
void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val)

‎lib/checkautovariables.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class CPPCHECKLIB CheckAutoVariables : public Check {
7979
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
8080
void errorInvalidLifetime(const Token *tok, const ValueFlow::Value* val);
8181
void errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val);
82-
void errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val);
82+
void errorDanglingTemporaryLifetime(const Token* tok, const ValueFlow::Value* val, const Token* tempTok);
8383
void errorReturnReference(const Token* tok, ErrorPath errorPath, bool inconclusive);
8484
void errorDanglingReference(const Token *tok, const Variable *var, ErrorPath errorPath);
8585
void errorDanglingTempReference(const Token* tok, ErrorPath errorPath, bool inconclusive);
@@ -106,7 +106,7 @@ class CPPCHECKLIB CheckAutoVariables : public Check {
106106
c.errorReturnDanglingLifetime(nullptr, nullptr);
107107
c.errorInvalidLifetime(nullptr, nullptr);
108108
c.errorDanglngLifetime(nullptr, nullptr);
109-
c.errorDanglingTemporaryLifetime(nullptr, nullptr);
109+
c.errorDanglingTemporaryLifetime(nullptr, nullptr, nullptr);
110110
}
111111

112112
static std::string myName() {

‎lib/valueflow.cpp‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,8 +2341,12 @@ struct ValueFlowAnalyzer : Analyzer {
23412341
// Follow references
23422342
std::vector<ReferenceToken> refs = followAllReferences(tok);
23432343
const bool inconclusiveRefs = refs.size() != 1;
2344+
if (std::none_of(refs.begin(), refs.end(), [&](const ReferenceToken& ref) {
2345+
return tok == ref.token;
2346+
}))
2347+
refs.push_back(ReferenceToken{tok, {}});
23442348
for (const ReferenceToken& ref:refs) {
2345-
Action a = analyzeToken(ref.token, tok, d, inconclusiveRefs);
2349+
Action a = analyzeToken(ref.token, tok, d, inconclusiveRefs && ref.token != tok);
23462350
if (internalMatch(ref.token))
23472351
a |= Action::Internal;
23482352
if (a != Action::None)
@@ -2944,29 +2948,34 @@ std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, Error
29442948
const Token *tokvalue = val ? val->tokvalue : nullptr;
29452949
const Variable *tokvar = tokvalue ? tokvalue->variable() : nullptr;
29462950
const Token *vartok = tokvar ? tokvar->nameToken() : nullptr;
2951+
const bool classVar = tokvar ? (!tokvar->isLocal() && !tokvar->isArgument() && !tokvar->isGlobal()) : false;
29472952
std::string type = lifetimeType(tok, val);
29482953
std::string msg = type;
29492954
if (vartok) {
2950-
errorPath.emplace_back(vartok, "Variable created here.");
2955+
if (!classVar)
2956+
errorPath.emplace_back(vartok, "Variable created here.");
29512957
const Variable * var = vartok->variable();
2958+
std::string submessage;
29522959
if (var) {
29532960
switch (val->lifetimeKind) {
29542961
case ValueFlow::Value::LifetimeKind::SubObject:
29552962
case ValueFlow::Value::LifetimeKind::Object:
29562963
case ValueFlow::Value::LifetimeKind::Address:
29572964
if (type == "pointer")
2958-
msg += " to local variable";
2965+
submessage = " to local variable";
29592966
else
2960-
msg += " that points to local variable";
2967+
submessage = " that points to local variable";
29612968
break;
29622969
case ValueFlow::Value::LifetimeKind::Lambda:
2963-
msg += " that captures local variable";
2970+
submessage = " that captures local variable";
29642971
break;
29652972
case ValueFlow::Value::LifetimeKind::Iterator:
2966-
msg += " to local container";
2973+
submessage = " to local container";
29672974
break;
29682975
}
2969-
msg += " '" + var->name() + "'";
2976+
if (classVar)
2977+
submessage.replace(submessage.find("local"), 5, "member");
2978+
msg += submessage + " '" + var->name() + "'";
29702979
}
29712980
}
29722981
return msg;

‎test/testautovariables.cpp‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,8 +1767,10 @@ class TestAutoVariables : public TestFixture {
17671767
" int& x = h();\n"
17681768
" g(&x);\n"
17691769
"}");
1770-
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:5]: (error) Using pointer to temporary.\n"
1771-
"[test.cpp:4] -> [test.cpp:5]: (error) Using reference to dangling temporary.\n", errout.str());
1770+
ASSERT_EQUALS(
1771+
"[test.cpp:4] -> [test.cpp:5] -> [test.cpp:4] -> [test.cpp:5]: (error) Using pointer that is a temporary.\n"
1772+
"[test.cpp:4] -> [test.cpp:5]: (error) Using reference to dangling temporary.\n",
1773+
errout.str());
17721774

17731775
check("void g(int*);\n"
17741776
"int h();\n"
@@ -3120,7 +3122,7 @@ class TestAutoVariables : public TestFixture {
31203122
" i += *x;\n"
31213123
"}");
31223124
ASSERT_EQUALS(
3123-
"[test.cpp:1] -> [test.cpp:2] -> [test.cpp:5] -> [test.cpp:5] -> [test.cpp:6]: (error) Using pointer to temporary.\n",
3125+
"[test.cpp:1] -> [test.cpp:2] -> [test.cpp:5] -> [test.cpp:5] -> [test.cpp:5] -> [test.cpp:6]: (error) Using pointer that is a temporary.\n",
31243126
errout.str());
31253127

31263128
check("QString f() {\n"
@@ -3129,18 +3131,16 @@ class TestAutoVariables : public TestFixture {
31293131
" QString c = b;\n"
31303132
" return c;\n"
31313133
"}");
3132-
ASSERT_EQUALS(
3133-
"[test.cpp:3] -> [test.cpp:4]: (error) Using pointer to temporary.\n",
3134-
errout.str());
3134+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3] -> [test.cpp:4]: (error) Using pointer that is a temporary.\n",
3135+
errout.str());
31353136

31363137
check("auto f(std::string s) {\n"
31373138
" const char *x = s.substr(1,2).c_str();\n"
31383139
" auto i = s.substr(4,5).begin();\n"
31393140
" return *i;\n"
31403141
"}");
3141-
ASSERT_EQUALS(
3142-
"[test.cpp:3] -> [test.cpp:4]: (error) Using iterator to temporary.\n",
3143-
errout.str());
3142+
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3] -> [test.cpp:4]: (error) Using iterator that is a temporary.\n",
3143+
errout.str());
31443144

31453145
check("std::string f() {\n"
31463146
" std::stringstream tmp;\n"
@@ -3183,6 +3183,21 @@ class TestAutoVariables : public TestFixture {
31833183
" a->fun();\n"
31843184
"}\n");
31853185
ASSERT_EQUALS("", errout.str());
3186+
3187+
check("struct A {\n"
3188+
" std::map<int, int> m_;\n"
3189+
"};\n"
3190+
"struct B {\n"
3191+
" A a_;\n"
3192+
"};\n"
3193+
"B func();\n"
3194+
"void f() {\n"
3195+
" const std::map<int, int>::iterator& m = func().a_.m_.begin();\n"
3196+
" (void)m->first;\n"
3197+
"}\n");
3198+
ASSERT_EQUALS(
3199+
"[test.cpp:9] -> [test.cpp:9] -> [test.cpp:10]: (error) Using object that points to member variable 'm_' that is a temporary.\n",
3200+
errout.str());
31863201
}
31873202

31883203
void invalidLifetime() {

‎test/teststl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4833,7 +4833,7 @@ class TestStl : public TestFixture {
48334833
"}\n",
48344834
true);
48354835
ASSERT_EQUALS(
4836-
"[test.cpp:7] -> [test.cpp:8] -> [test.cpp:12] -> [test.cpp:2] -> [test.cpp:9]: (error) Using iterator to local container 'v' that may be invalid.\n",
4836+
"[test.cpp:7] -> [test.cpp:8] -> [test.cpp:12] -> [test.cpp:9]: (error) Using iterator to member container 'v' that may be invalid.\n",
48374837
errout.str());
48384838

48394839
check("void foo(std::vector<int>& v) {\n"

0 commit comments

Comments
 (0)