Skip to content

Commit 3e1b34d

Browse files
pfultz2danmar
authored andcommitted
Fix FPs and crashes with byDerefCopy (#1503)
* Fix FP when inserting a range into a container * Formatting * Fix crash
1 parent ba56407 commit 3e1b34d

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

lib/valueflow.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2649,7 +2649,7 @@ struct LifetimeStore {
26492649
const Variable *var = getLifetimeVariable(tok2, errorPath);
26502650
if (!var)
26512651
continue;
2652-
for (const Token *tok3 = tok; tok != var->declEndToken(); tok3 = tok3->previous()) {
2652+
for (const Token *tok3 = tok; tok3 && tok3 != var->declEndToken(); tok3 = tok3->previous()) {
26532653
if (tok3->varId() == var->declarationId()) {
26542654
LifetimeStore{tok3, message, type} .byVal(tok, tokenlist, errorLogger, settings, pred);
26552655
break;
@@ -2699,8 +2699,10 @@ static void valueFlowLifetimeFunction(Token *tok, TokenList *tokenlist, ErrorLog
26992699
const bool isPointer = endTypeTok && Token::simpleMatch(endTypeTok->previous(), "*");
27002700
Token *vartok = tok->tokAt(-2);
27012701
std::vector<const Token *> args = getArguments(tok);
2702-
if (args.size() == 2 && astCanonicalType(args[0]) == astCanonicalType(args[1]) &&
2703-
(((astIsIterator(args[0]) && astIsIterator(args[1])) || (astIsPointer(args[0]) && astIsPointer(args[1]))))) {
2702+
std::size_t n = args.size();
2703+
if (n > 1 && astCanonicalType(args[n - 2]) == astCanonicalType(args[n - 1]) &&
2704+
(((astIsIterator(args[n - 2]) && astIsIterator(args[n - 1])) ||
2705+
(astIsPointer(args[n - 2]) && astIsPointer(args[n - 1]))))) {
27042706
LifetimeStore{args.back(), "Added to container '" + vartok->str() + "'.", ValueFlow::Value::Object} .byDerefCopy(
27052707
vartok, tokenlist, errorLogger, settings);
27062708
} else if (!args.empty() && astIsPointer(args.back()) == isPointer) {

test/testautovariables.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,6 +1420,19 @@ class TestAutoVariables : public TestFixture {
14201420
"[test.cpp:5] -> [test.cpp:6] -> [test.cpp:4] -> [test.cpp:6]: (error) Non-local variable 'v' will use object that points to local variable 'i'.\n",
14211421
errout.str());
14221422

1423+
check("struct A {\n"
1424+
" std::vector<int*> m;\n"
1425+
" void f() {\n"
1426+
" int x;\n"
1427+
" std::vector<int*> v;\n"
1428+
" v.push_back(&x);\n"
1429+
" m.insert(m.end(), v.begin(), v.end());\n"
1430+
" }\n"
1431+
"};\n");
1432+
ASSERT_EQUALS(
1433+
"[test.cpp:6] -> [test.cpp:6] -> [test.cpp:6] -> [test.cpp:4] -> [test.cpp:7]: (error) Non-local variable 'm' will use object that points to local variable 'x'.\n",
1434+
errout.str());
1435+
14231436
check("struct A {\n"
14241437
" std::vector<std::string> v;\n"
14251438
" void f() {\n"
@@ -1483,6 +1496,27 @@ class TestAutoVariables : public TestFixture {
14831496
" return g([&]() { return v.data(); });\n"
14841497
"}\n");
14851498
ASSERT_EQUALS("", errout.str());
1499+
1500+
check("std::vector<int> g();\n"
1501+
"struct A {\n"
1502+
" std::vector<int> m;\n"
1503+
" void f() {\n"
1504+
" std::vector<int> v = g();\n"
1505+
" m.insert(m.end(), v.begin(), v.end());\n"
1506+
" }\n"
1507+
"};\n");
1508+
ASSERT_EQUALS("", errout.str());
1509+
1510+
check("class A {\n"
1511+
" int f( P p ) {\n"
1512+
" std::vector< S > maps;\n"
1513+
" m2.insert( m1.begin(), m1.end() );\n"
1514+
" }\n"
1515+
" struct B {};\n"
1516+
" std::map< S, B > m1;\n"
1517+
" std::map< S, B > m2;\n"
1518+
"};\n");
1519+
ASSERT_EQUALS("", errout.str());
14861520
}
14871521

14881522
void danglingLifetime() {

0 commit comments

Comments
 (0)