Skip to content

Commit 3e78e76

Browse files
pfultz2pfultz2
authored
Fix issue 10076: ValueFlow: False positive after address of var is taken 'T t = {{{&var}}};' (danmar#3283)
1 parent 548ec10 commit 3e78e76

File tree

6 files changed

+167
-20
lines changed

6 files changed

+167
-20
lines changed

‎lib/astutils.cpp‎

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,7 +1668,8 @@ const Token * getTokenArgumentFunction(const Token * tok, int& argn)
16681668
return nullptr;
16691669
if (!Token::Match(tok, "{|("))
16701670
return nullptr;
1671-
tok = tok->astOperand1();
1671+
if (tok->astOperand2())
1672+
tok = tok->astOperand1();
16721673
while (tok && (tok->isUnaryOp("*") || tok->str() == "["))
16731674
tok = tok->astOperand1();
16741675
while (Token::simpleMatch(tok, "."))
@@ -1690,21 +1691,31 @@ const Token * getTokenArgumentFunction(const Token * tok, int& argn)
16901691
return tok;
16911692
}
16921693

1693-
static std::vector<const Variable*> getArgumentVars(const Token* tok, int argnr)
1694+
std::vector<const Variable*> getArgumentVars(const Token* tok, int argnr)
16941695
{
16951696
std::vector<const Variable*> result;
16961697
if (!tok)
16971698
return result;
1698-
if (tok->function())
1699-
return {tok->function()->getArgumentVar(argnr)};
1700-
if (Token::Match(tok->previous(), "%type% (|{") || tok->variable()) {
1701-
const bool constructor = tok->variable() && tok->variable()->nameToken() == tok;
1699+
if (tok->function()) {
1700+
const Variable* argvar = tok->function()->getArgumentVar(argnr);
1701+
if (argvar)
1702+
return {argvar};
1703+
else
1704+
return result;
1705+
}
1706+
if (Token::Match(tok->previous(), "%type% (|{") || Token::simpleMatch(tok, "{") || tok->variable()) {
1707+
const bool constructor = Token::simpleMatch(tok, "{") || (tok->variable() && tok->variable()->nameToken() == tok);
17021708
const Type* type = Token::typeOf(tok);
17031709
if (!type)
17041710
return result;
17051711
const Scope* typeScope = type->classScope;
17061712
if (!typeScope)
17071713
return result;
1714+
// Aggregate constructor
1715+
if (Token::simpleMatch(tok, "{") && typeScope->numConstructors == 0 && argnr < typeScope->varlist.size()) {
1716+
auto it = std::next(typeScope->varlist.begin(), argnr);
1717+
return {&*it};
1718+
}
17081719
const int argCount = numberOfArguments(tok);
17091720
for (const Function &function : typeScope->functionList) {
17101721
if (function.argCount() < argCount)
@@ -1713,7 +1724,9 @@ static std::vector<const Variable*> getArgumentVars(const Token* tok, int argnr)
17131724
continue;
17141725
if (!constructor && !Token::simpleMatch(function.token, "operator()"))
17151726
continue;
1716-
result.push_back(function.getArgumentVar(argnr));
1727+
const Variable* argvar = function.getArgumentVar(argnr);
1728+
if (argvar)
1729+
result.push_back(argvar);
17171730
}
17181731
}
17191732
return result;
@@ -1726,6 +1739,17 @@ static bool isCPPCastKeyword(const Token* tok)
17261739
return endsWith(tok->str(), "_cast", 5);
17271740
}
17281741

1742+
static bool isTrivialConstructor(const Token* tok)
1743+
{
1744+
const Token* typeTok = nullptr;
1745+
const Type* t = Token::typeOf(tok, &typeTok);
1746+
if (t)
1747+
return false;
1748+
if (typeTok->valueType() && typeTok->valueType()->isPrimitive())
1749+
return true;
1750+
return false;
1751+
}
1752+
17291753
bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Settings *settings, bool *inconclusive)
17301754
{
17311755
if (!tok)
@@ -1743,6 +1767,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
17431767
tok = getTokenArgumentFunction(tok, argnr);
17441768
if (!tok)
17451769
return false; // not a function => variable not changed
1770+
if (Token::simpleMatch(tok, "{") && isTrivialConstructor(tok))
1771+
return false;
17461772
if (tok->isKeyword() && !isCPPCastKeyword(tok))
17471773
return false;
17481774
const Token * parenTok = tok->next();
@@ -2303,19 +2329,19 @@ std::vector<const Variable*> getLHSVariables(const Token* tok)
23032329
return result;
23042330
}
23052331

2306-
static const Variable *getLHSVariableRecursive(const Token *tok)
2332+
static const Token* getLHSVariableRecursive(const Token* tok)
23072333
{
23082334
if (!tok)
23092335
return nullptr;
23102336
if (Token::Match(tok, "*|&|&&|[")) {
2311-
const Variable *var = getLHSVariableRecursive(tok->astOperand1());
2312-
if (var || Token::simpleMatch(tok, "["))
2313-
return var;
2337+
const Token* vartok = getLHSVariableRecursive(tok->astOperand1());
2338+
if ((vartok && vartok->variable()) || Token::simpleMatch(tok, "["))
2339+
return vartok;
23142340
return getLHSVariableRecursive(tok->astOperand2());
23152341
}
23162342
if (Token::Match(tok->previous(), "this . %var%"))
2317-
return tok->next()->variable();
2318-
return tok->variable();
2343+
return tok->next();
2344+
return tok;
23192345
}
23202346

23212347
const Variable *getLHSVariable(const Token *tok)
@@ -2326,7 +2352,24 @@ const Variable *getLHSVariable(const Token *tok)
23262352
return nullptr;
23272353
if (tok->astOperand1()->varId() > 0 && tok->astOperand1()->variable())
23282354
return tok->astOperand1()->variable();
2329-
return getLHSVariableRecursive(tok->astOperand1());
2355+
const Token* vartok = getLHSVariableRecursive(tok->astOperand1());
2356+
if (!vartok)
2357+
return nullptr;
2358+
return vartok->variable();
2359+
}
2360+
2361+
const Token* getLHSVariableToken(const Token* tok)
2362+
{
2363+
if (!Token::Match(tok, "%assign%"))
2364+
return nullptr;
2365+
if (!tok->astOperand1())
2366+
return nullptr;
2367+
if (tok->astOperand1()->varId() > 0)
2368+
return tok->astOperand1();
2369+
const Token* vartok = getLHSVariableRecursive(tok->astOperand1());
2370+
if (!vartok)
2371+
return tok->astOperand1();
2372+
return vartok;
23302373
}
23312374

23322375
const Token* findAllocFuncCallToken(const Token *expr, const Library &library)

‎lib/astutils.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,8 @@ bool isReturnScope(const Token* const endToken,
188188
/// Return the token to the function and the argument number
189189
const Token * getTokenArgumentFunction(const Token * tok, int& argn);
190190

191+
std::vector<const Variable*> getArgumentVars(const Token* tok, int argnr);
192+
191193
/** Is variable changed by function call?
192194
* In case the answer of the question is inconclusive, e.g. because the function declaration is not known
193195
* the return value is false and the output parameter inconclusive is set to true
@@ -279,6 +281,8 @@ bool isConstVarExpression(const Token *tok, const char * skipMatch = nullptr);
279281

280282
const Variable *getLHSVariable(const Token *tok);
281283

284+
const Token* getLHSVariableToken(const Token* tok);
285+
282286
std::vector<const Variable*> getLHSVariables(const Token* tok);
283287

284288
/** Find a allocation function call in expression, so result of expression is allocated memory/resource. */

‎lib/symboldatabase.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,8 @@ class CPPCHECKLIB ValueType {
12731273
static MatchResult matchParameter(const ValueType *call, const ValueType *func);
12741274
static MatchResult matchParameter(const ValueType *call, const Variable *callVar, const Variable *funcVar);
12751275

1276+
bool isPrimitive() const { return (type >= ValueType::Type::BOOL); }
1277+
12761278
bool isIntegral() const {
12771279
return (type >= ValueType::Type::BOOL && type <= ValueType::Type::UNKNOWN_INT);
12781280
}

‎lib/token.cpp‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,10 +2257,12 @@ void Token::type(const ::Type *t)
22572257
tokType(eName);
22582258
}
22592259

2260-
const ::Type *Token::typeOf(const Token *tok)
2260+
const ::Type* Token::typeOf(const Token* tok, const Token** typeTok)
22612261
{
22622262
if (!tok)
22632263
return nullptr;
2264+
if (typeTok != nullptr)
2265+
*typeTok = tok;
22642266
if (Token::simpleMatch(tok, "return")) {
22652267
const Scope *scope = tok->scope();
22662268
if (!scope)
@@ -2282,14 +2284,30 @@ const ::Type *Token::typeOf(const Token *tok)
22822284
return nullptr;
22832285
return function->retType;
22842286
} else if (Token::Match(tok->previous(), "%type%|= (|{")) {
2285-
return typeOf(tok->previous());
2287+
return typeOf(tok->previous(), typeTok);
22862288
} else if (Token::simpleMatch(tok, "=")) {
2287-
return Token::typeOf(tok->astOperand1());
2289+
return Token::typeOf(getLHSVariableToken(tok), typeTok);
22882290
} else if (Token::simpleMatch(tok, ".")) {
2289-
return Token::typeOf(tok->astOperand2());
2291+
return Token::typeOf(tok->astOperand2(), typeTok);
22902292
} else if (Token::simpleMatch(tok, "[")) {
2291-
return Token::typeOf(tok->astOperand1());
2293+
return Token::typeOf(tok->astOperand1(), typeTok);
2294+
} else if (Token::simpleMatch(tok, "{")) {
2295+
int argnr;
2296+
const Token* ftok = getTokenArgumentFunction(tok, argnr);
2297+
if (argnr < 0)
2298+
return nullptr;
2299+
if (!ftok)
2300+
return nullptr;
2301+
if (ftok == tok)
2302+
return nullptr;
2303+
std::vector<const Variable*> vars = getArgumentVars(ftok, argnr);
2304+
if (vars.empty())
2305+
return nullptr;
2306+
if (std::all_of(
2307+
vars.begin(), vars.end(), [&](const Variable* var) { return var->type() == vars.front()->type(); }))
2308+
return vars.front()->type();
22922309
}
2310+
22932311
return nullptr;
22942312
}
22952313

‎lib/token.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ class CPPCHECKLIB Token {
10101010
return mTokType == eType ? mImpl->mType : nullptr;
10111011
}
10121012

1013-
static const ::Type *typeOf(const Token *tok);
1013+
static const ::Type* typeOf(const Token* tok, const Token** typeTok = nullptr);
10141014

10151015
static std::pair<const Token*, const Token*> typeDecl(const Token * tok);
10161016

‎test/teststl.cpp‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,86 @@ class TestStl : public TestFixture {
381381
"}\n", true);
382382
ASSERT_EQUALS("", errout.str());
383383

384+
check("struct T {\n"
385+
" std::vector<int>* v;\n"
386+
"};\n"
387+
"struct S {\n"
388+
" std::vector<T> t;\n"
389+
"};\n"
390+
"long g(S& s);\n"
391+
"int f() {\n"
392+
" std::vector<int> ArrS;\n"
393+
" S s = { { { &ArrS } } };\n"
394+
" g(s);\n"
395+
" return ArrS[0];\n"
396+
"}\n",
397+
true);
398+
ASSERT_EQUALS("", errout.str());
399+
400+
check("struct T {\n"
401+
" std::vector<int>* v;\n"
402+
"};\n"
403+
"struct S {\n"
404+
" std::vector<std::vector<T>> t;\n"
405+
"};\n"
406+
"long g(S& s);\n"
407+
"int f() {\n"
408+
" std::vector<int> ArrS;\n"
409+
" S s = { { { { &ArrS } } } };\n"
410+
" g(s);\n"
411+
" return ArrS[0];\n"
412+
"}\n",
413+
true);
414+
ASSERT_EQUALS("", errout.str());
415+
416+
check("struct T {\n"
417+
" std::vector<int>* v;\n"
418+
"};\n"
419+
"struct S {\n"
420+
" T t;\n"
421+
"};\n"
422+
"long g(S& s);\n"
423+
"int f() {\n"
424+
" std::vector<int> ArrS;\n"
425+
" S s { { &ArrS } };\n"
426+
" g(s);\n"
427+
" return ArrS[0];\n"
428+
"}\n",
429+
true);
430+
ASSERT_EQUALS("", errout.str());
431+
432+
check("struct T {\n"
433+
" std::vector<int>* v;\n"
434+
"};\n"
435+
"struct S {\n"
436+
" std::vector<T> t;\n"
437+
"};\n"
438+
"long g(S& s);\n"
439+
"int f() {\n"
440+
" std::vector<int> ArrS;\n"
441+
" S s { { { &ArrS } } };\n"
442+
" g(s);\n"
443+
" return ArrS[0];\n"
444+
"}\n",
445+
true);
446+
ASSERT_EQUALS("", errout.str());
447+
448+
check("struct T {\n"
449+
" std::vector<int>* v;\n"
450+
"};\n"
451+
"struct S {\n"
452+
" std::vector<std::vector<T>> t;\n"
453+
"};\n"
454+
"long g(S& s);\n"
455+
"int f() {\n"
456+
" std::vector<int> ArrS;\n"
457+
" S s { { { { &ArrS } } } };\n"
458+
" g(s);\n"
459+
" return ArrS[0];\n"
460+
"}\n",
461+
true);
462+
ASSERT_EQUALS("", errout.str());
463+
384464
checkNormal("extern void Bar(const double, const double);\n"
385465
"void f(std::vector<double> &r, const double ) {\n"
386466
" std::vector<double> result;\n"

0 commit comments

Comments
 (0)