Skip to content

Commit 1a5449c

Browse files
danmardanmar
committed
Fixed #10327 (ValueFlow; Wrong Uninit value in called function)
1 parent 869eac5 commit 1a5449c

4 files changed

Lines changed: 53 additions & 11 deletions

File tree

‎lib/checkuninitvar.cpp‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,7 @@ static bool isVoidCast(const Token *tok)
10931093
return Token::simpleMatch(tok, "(") && tok->isCast() && tok->valueType() && tok->valueType()->type == ValueType::Type::VOID && tok->valueType()->pointer == 0;
10941094
}
10951095

1096-
const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect) const
1096+
const Token* CheckUninitVar::isVariableUsage(bool cpp, const Token *vartok, const Library& library, bool pointer, Alloc alloc, int indirect)
10971097
{
10981098
const Token *valueExpr = vartok; // non-dereferenced , no address of value as variable
10991099
while (Token::Match(valueExpr->astParent(), ".|::") && astIsRhs(valueExpr))
@@ -1197,17 +1197,17 @@ const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer,
11971197
parent = parent->astParent();
11981198
if (Token::simpleMatch(parent, "{"))
11991199
return valueExpr;
1200-
const int use = isFunctionParUsage(valueExpr, pointer, alloc, indirect);
1200+
const int use = isFunctionParUsage(valueExpr, library, pointer, alloc, indirect);
12011201
return (use>0) ? valueExpr : nullptr;
12021202
}
12031203
if (derefValue && Token::Match(derefValue->astParent(), "[(,]") && (derefValue->astParent()->str() == "," || astIsRhs(derefValue))) {
1204-
const int use = isFunctionParUsage(derefValue, false, NO_ALLOC, indirect);
1204+
const int use = isFunctionParUsage(derefValue, library, false, NO_ALLOC, indirect);
12051205
return (use>0) ? derefValue : nullptr;
12061206
}
12071207
if (valueExpr->astParent()->isUnaryOp("&")) {
12081208
const Token *parent = valueExpr->astParent();
12091209
if (Token::Match(parent->astParent(), "[(,]") && (parent->astParent()->str() == "," || astIsRhs(parent))) {
1210-
const int use = isFunctionParUsage(valueExpr, pointer, alloc, indirect);
1210+
const int use = isFunctionParUsage(valueExpr, library, pointer, alloc, indirect);
12111211
return (use>0) ? valueExpr : nullptr;
12121212
}
12131213
return nullptr;
@@ -1253,18 +1253,18 @@ const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer,
12531253

12541254
// Stream read/write
12551255
// FIXME this code is a hack!!
1256-
if (mTokenizer->isCPP() && Token::Match(valueExpr->astParent(), "<<|>>")) {
1257-
if (isLikelyStreamRead(mTokenizer->isCPP(), vartok->previous()))
1256+
if (cpp && Token::Match(valueExpr->astParent(), "<<|>>")) {
1257+
if (isLikelyStreamRead(cpp, vartok->previous()))
12581258
return nullptr;
12591259

12601260
if (valueExpr->valueType() && valueExpr->valueType()->type == ValueType::Type::VOID)
12611261
return nullptr;
12621262
}
1263-
if (astIsRhs(derefValue) && isLikelyStreamRead(mTokenizer->isCPP(), derefValue->astParent()))
1263+
if (astIsRhs(derefValue) && isLikelyStreamRead(cpp, derefValue->astParent()))
12641264
return nullptr;
12651265

12661266
// Assignment with overloaded &
1267-
if (mTokenizer->isCPP() && Token::simpleMatch(valueExpr->astParent(), "&") && astIsRhs(valueExpr)) {
1267+
if (cpp && Token::simpleMatch(valueExpr->astParent(), "&") && astIsRhs(valueExpr)) {
12681268
const Token *parent = valueExpr->astParent();
12691269
while (Token::simpleMatch(parent, "&") && parent->isBinaryOp())
12701270
parent = parent->astParent();
@@ -1280,13 +1280,18 @@ const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer,
12801280
return derefValue ? derefValue : valueExpr;
12811281
}
12821282

1283+
const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect) const
1284+
{
1285+
return CheckUninitVar::isVariableUsage(mTokenizer->isCPP(), vartok, mSettings->library, pointer, alloc, indirect);
1286+
}
1287+
12831288
/***
12841289
* Is function parameter "used" so a "usage of uninitialized variable" can
12851290
* be written? If parameter is passed "by value" then it is "used". If it
12861291
* is passed "by reference" then it is not necessarily "used".
12871292
* @return -1 => unknown 0 => not used 1 => used
12881293
*/
1289-
int CheckUninitVar::isFunctionParUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect) const
1294+
int CheckUninitVar::isFunctionParUsage(const Token *vartok, const Library& library, bool pointer, Alloc alloc, int indirect)
12901295
{
12911296
bool unknown = false;
12921297
const Token *parent = getAstParentSkipPossibleCastAndAddressOf(vartok, &unknown);
@@ -1339,11 +1344,11 @@ int CheckUninitVar::isFunctionParUsage(const Token *vartok, bool pointer, Alloc
13391344
// control-flow statement reading the variable "by value"
13401345
return alloc == NO_ALLOC;
13411346
} else {
1342-
const bool isnullbad = mSettings->library.isnullargbad(start->previous(), argumentNumber + 1);
1347+
const bool isnullbad = library.isnullargbad(start->previous(), argumentNumber + 1);
13431348
if (indirect == 0 && pointer && !address && isnullbad && alloc == NO_ALLOC)
13441349
return 1;
13451350
bool hasIndirect = false;
1346-
const bool isuninitbad = mSettings->library.isuninitargbad(start->previous(), argumentNumber + 1, indirect, &hasIndirect);
1351+
const bool isuninitbad = library.isuninitargbad(start->previous(), argumentNumber + 1, indirect, &hasIndirect);
13471352
if (alloc != NO_ALLOC)
13481353
return (isnullbad || hasIndirect) && isuninitbad;
13491354
return isuninitbad && (!address || isnullbad);
@@ -1354,6 +1359,11 @@ int CheckUninitVar::isFunctionParUsage(const Token *vartok, bool pointer, Alloc
13541359
return -1;
13551360
}
13561361

1362+
int CheckUninitVar::isFunctionParUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect) const
1363+
{
1364+
return CheckUninitVar::isFunctionParUsage(vartok, mSettings->library, pointer, alloc, indirect);
1365+
}
1366+
13571367
bool CheckUninitVar::isMemberVariableAssignment(const Token *tok, const std::string &membervar) const
13581368
{
13591369
if (Token::Match(tok, "%name% . %name%") && tok->strAt(2) == membervar) {

‎lib/checkuninitvar.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ class CPPCHECKLIB CheckUninitVar : public Check {
8484
bool checkLoopBody(const Token *tok, const Variable& var, const Alloc alloc, const std::string &membervar, const bool suppressErrors);
8585
const Token* checkLoopBodyRecursive(const Token *start, const Variable& var, const Alloc alloc, const std::string &membervar, bool &bailout) const;
8686
void checkRhs(const Token *tok, const Variable &var, Alloc alloc, nonneg int number_of_if, const std::string &membervar);
87+
static const Token *isVariableUsage(bool cpp, const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0);
8788
const Token *isVariableUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect = 0) const;
89+
static int isFunctionParUsage(const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0);
8890
int isFunctionParUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect = 0) const;
8991
bool isMemberVariableAssignment(const Token *tok, const std::string &membervar) const;
9092
bool isMemberVariableUsage(const Token *tok, bool isPointer, Alloc alloc, const std::string &membervar) const;

‎lib/valueflow.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979

8080
#include "analyzer.h"
8181
#include "astutils.h"
82+
#include "checkuninitvar.h"
8283
#include "errorlogger.h"
8384
#include "errortypes.h"
8485
#include "forwardanalyzer.h"
@@ -4255,6 +4256,12 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat
42554256
values.remove_if([&](const ValueFlow::Value& value) {
42564257
return value.valueType == ValueFlow::Value::ValueType::CONTAINER_SIZE;
42574258
});
4259+
// If assignment copy by value, remove Uninit values..
4260+
if ((tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer == 0) ||
4261+
(tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference() && tok->astOperand1()->variable()->nameToken() == tok->astOperand1()))
4262+
values.remove_if([&](const ValueFlow::Value& value) {
4263+
return value.isUninitValue();
4264+
});
42584265
if (values.empty())
42594266
continue;
42604267
const bool init = vars.size() == 1 && vars.front()->nameToken() == tok->astOperand1();
@@ -5777,6 +5784,11 @@ static void valueFlowSubFunction(TokenList* tokenlist, SymbolDatabase* symboldat
57775784
});
57785785
// Don't forward container sizes for now since programmemory can't evaluate conditions
57795786
argvalues.remove_if(std::mem_fn(&ValueFlow::Value::isContainerSizeValue));
5787+
// Remove uninit values if argument is passed by value
5788+
if (argtok->variable() && !argtok->variable()->isPointer() && argvalues.size() == 1 && argvalues.front().isUninitValue()) {
5789+
if (CheckUninitVar::isVariableUsage(tokenlist->isCPP(), argtok, settings->library, false, CheckUninitVar::Alloc::NO_ALLOC, 0))
5790+
continue;
5791+
}
57805792

57815793
if (argvalues.empty())
57825794
continue;

‎test/testuninitvar.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4844,6 +4844,24 @@ class TestUninitVar : public TestFixture {
48444844
" return f.i;\n"
48454845
"}\n");
48464846
ASSERT_EQUALS("", errout.str());
4847+
4848+
// #10327 - avoid extra warnings for uninitialized variable
4849+
valueFlowUninit("void dowork( int me ) {\n"
4850+
" if ( me == 0 ) {}\n" // <- not uninitialized
4851+
"}\n"
4852+
"\n"
4853+
"int main() {\n"
4854+
" int me;\n"
4855+
" dowork(me);\n" // <- me is uninitialized
4856+
"}");
4857+
ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: me\n", errout.str());
4858+
4859+
valueFlowUninit("int foo() {\n"
4860+
" int x;\n"
4861+
" int a = x;\n" // <- x is uninitialized
4862+
" return a;\n" // <- a has been initialized
4863+
"}");
4864+
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str());
48474865
}
48484866

48494867
void uninitvar_ipa() {

0 commit comments

Comments
 (0)