Skip to content

Commit 0b4e08c

Browse files
danmardanmar
committed
Use FwdAnalysis in UnusedVar. This is still work-in-progress. Merging to master branch so it can be tested.
1 parent 02e1637 commit 0b4e08c

File tree

10 files changed

+396
-133
lines changed

10 files changed

+396
-133
lines changed

‎lib/astutils.cpp‎

Lines changed: 135 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ bool isLikelyStreamRead(bool cpp, const Token *op)
10401040

10411041
static bool nonLocal(const Variable* var)
10421042
{
1043-
return !var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
1043+
return !var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference() || var->isExtern();
10441044
}
10451045

10461046
static bool hasFunctionCall(const Token *tok)
@@ -1071,9 +1071,41 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
10711071
return Result(Result::Type::BREAK, tok);
10721072
}
10731073

1074-
if (Token::Match(tok, "continue|return|throw|goto")) {
1074+
if (Token::simpleMatch(tok, "goto"))
1075+
return Result(Result::Type::BAILOUT);
1076+
1077+
if (tok->str() == "continue")
1078+
// TODO
1079+
return Result(Result::Type::BAILOUT);
1080+
1081+
if (Token::Match(tok, "return|throw")) {
10751082
// TODO: Handle these better
1076-
return Result(Result::Type::RETURN);
1083+
switch (mWhat) {
1084+
case What::Reassign:
1085+
return Result(Result::Type::RETURN);
1086+
case What::UnusedValue:
1087+
// Is expr variable used in expression?
1088+
{
1089+
bool read = false;
1090+
visitAstNodes(tok->astOperand1(),
1091+
[&](const Token *tok2) {
1092+
if (!local && Token::Match(tok2, "%name% ("))
1093+
read = true;
1094+
if (tok2->varId() && exprVarIds.find(tok2->varId()) != exprVarIds.end())
1095+
read = true;
1096+
return read ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
1097+
});
1098+
1099+
return Result(read ? Result::Type::READ : Result::Type::RETURN);
1100+
}
1101+
}
1102+
}
1103+
1104+
if (tok->str() == "}") {
1105+
Scope::ScopeType scopeType = tok->scope()->type;
1106+
if (scopeType == Scope::eWhile || scopeType == Scope::eFor || scopeType == Scope::eDo)
1107+
// TODO handle loops better
1108+
return Result(Result::Type::BAILOUT);
10771109
}
10781110

10791111
if (Token::simpleMatch(tok, "else {"))
@@ -1088,6 +1120,10 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
10881120
return Result(Result::Type::BAILOUT);
10891121
}
10901122

1123+
if (expr->isName() && Token::Match(tok, "%name% (") && tok->str().find("<") != std::string::npos && tok->str().find(expr->str()) != std::string::npos)
1124+
return Result(Result::Type::BAILOUT);
1125+
1126+
10911127
if (exprVarIds.find(tok->varId()) != exprVarIds.end()) {
10921128
const Token *parent = tok;
10931129
while (Token::Match(parent->astParent(), ".|::|["))
@@ -1098,7 +1134,7 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
10981134
return Result(Result::Type::BAILOUT);
10991135
}
11001136
if (hasOperand(parent->astParent()->astOperand2(), expr)) {
1101-
if (mReassign)
1137+
if (mWhat == What::Reassign)
11021138
return Result(Result::Type::READ);
11031139
continue;
11041140
}
@@ -1113,6 +1149,9 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
11131149
}
11141150

11151151
if (Token::Match(tok, ") {")) {
1152+
if (Token::simpleMatch(tok->link()->previous(), "switch ("))
1153+
// TODO: parse switch
1154+
return Result(Result::Type::BAILOUT);
11161155
const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local);
11171156
if (result1.type == Result::Type::READ || result1.type == Result::Type::BAILOUT)
11181157
return result1;
@@ -1133,21 +1172,80 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const
11331172
return Result(Result::Type::NONE);
11341173
}
11351174

1175+
bool FwdAnalysis::isGlobalData(const Token *expr) const
1176+
{
1177+
bool globalData = false;
1178+
visitAstNodes(expr,
1179+
[&](const Token *tok) {
1180+
if (tok->originalName() == "->") {
1181+
// TODO check if pointer points at local data
1182+
globalData = true;
1183+
return ChildrenToVisit::none;
1184+
} else if (Token::Match(tok, "[*[]") && tok->astOperand1() && tok->astOperand1()->variable()) {
1185+
// TODO check if pointer points at local data
1186+
const Variable *lhsvar = tok->astOperand1()->variable();
1187+
const ValueType *lhstype = tok->astOperand1()->valueType();
1188+
if (lhsvar->isPointer()) {
1189+
globalData = true;
1190+
return ChildrenToVisit::none;
1191+
} else if (lhsvar->isArgument() && (!lhstype || (lhstype->type <= ValueType::Type::VOID && !lhstype->container))) {
1192+
globalData = true;
1193+
return ChildrenToVisit::none;
1194+
}
1195+
}
1196+
if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".") {
1197+
globalData = true;
1198+
return ChildrenToVisit::none;
1199+
}
1200+
if (tok->variable()) {
1201+
// TODO : Check references
1202+
if (tok->variable()->isReference() && tok != tok->variable()->nameToken()) {
1203+
globalData = true;
1204+
return ChildrenToVisit::none;
1205+
}
1206+
if ((tok->previous()->str() != "." && (!tok->variable()->isLocal() && !tok->variable()->isArgument())) || tok->variable()->isExtern()) {
1207+
globalData = true;
1208+
return ChildrenToVisit::none;
1209+
}
1210+
if (tok->variable()->isArgument() && tok->variable()->isPointer() && tok != expr) {
1211+
globalData = true;
1212+
return ChildrenToVisit::none;
1213+
}
1214+
}
1215+
if (Token::Match(tok, ".|["))
1216+
return ChildrenToVisit::op1;
1217+
return ChildrenToVisit::op1_and_op2;
1218+
});
1219+
return globalData;
1220+
}
1221+
11361222
FwdAnalysis::Result FwdAnalysis::check(const Token *expr, const Token *startToken, const Token *endToken)
11371223
{
11381224
// all variable ids in expr.
11391225
std::set<unsigned int> exprVarIds;
11401226
bool local = true;
11411227
visitAstNodes(expr,
11421228
[&](const Token *tok) {
1229+
if (tok->varId() == 0 && tok->isName() && tok->previous()->str() != ".")
1230+
// unknown variables are not local
1231+
local = false;
11431232
if (tok->varId() > 0) {
11441233
exprVarIds.insert(tok->varId());
1145-
if (!Token::simpleMatch(tok->previous(), "."))
1234+
if (!Token::simpleMatch(tok->previous(), ".")) {
1235+
const Variable *var = tok->variable();
1236+
if (var && var->isReference() && var->isLocal() && Token::Match(var->nameToken(), "%var% [=(]") && !isGlobalData(var->nameToken()->next()->astOperand2()))
1237+
return ChildrenToVisit::none;
11461238
local &= !nonLocal(tok->variable());
1239+
}
11471240
}
11481241
return ChildrenToVisit::op1_and_op2;
11491242
});
11501243

1244+
// In unused values checking we do not want to check assignments to
1245+
// global data.
1246+
if (mWhat == What::UnusedValue && isGlobalData(expr))
1247+
return Result(FwdAnalysis::Result::Type::BAILOUT);
1248+
11511249
Result result = checkRecursive(expr, startToken, endToken, exprVarIds, local);
11521250

11531251
// Break => continue checking in outer scope
@@ -1174,7 +1272,38 @@ bool FwdAnalysis::hasOperand(const Token *tok, const Token *lhs) const
11741272

11751273
const Token *FwdAnalysis::reassign(const Token *expr, const Token *startToken, const Token *endToken)
11761274
{
1177-
mReassign = true;
1275+
mWhat = What::Reassign;
11781276
Result result = check(expr, startToken, endToken);
11791277
return result.type == FwdAnalysis::Result::Type::WRITE ? result.token : nullptr;
11801278
}
1279+
1280+
bool FwdAnalysis::unusedValue(const Token *expr, const Token *startToken, const Token *endToken)
1281+
{
1282+
mWhat = What::UnusedValue;
1283+
Result result = check(expr, startToken, endToken);
1284+
return (result.type == FwdAnalysis::Result::Type::NONE || result.type == FwdAnalysis::Result::Type::RETURN) && !possiblyAliased(expr, startToken);
1285+
}
1286+
1287+
bool FwdAnalysis::possiblyAliased(const Token *expr, const Token *startToken) const
1288+
{
1289+
if (expr->isUnaryOp("*"))
1290+
return true;
1291+
1292+
const bool macro = false;
1293+
const bool pure = false;
1294+
const bool followVar = false;
1295+
for (const Token *tok = startToken; tok; tok = tok->previous()) {
1296+
if (tok->str() == "{" && tok->scope()->type == Scope::eFunction)
1297+
continue;
1298+
if (isSameExpression(mCpp, macro, expr, tok, mLibrary, pure, followVar)) {
1299+
const Token *parent = tok->astParent();
1300+
if (parent && parent->isUnaryOp("&"))
1301+
return true;
1302+
if (parent && Token::Match(parent->tokAt(-2), "& %name% ="))
1303+
return true;
1304+
if (parent && Token::simpleMatch(parent->tokAt(-3), "std :: ref ("))
1305+
return true;
1306+
}
1307+
}
1308+
return false;
1309+
}

‎lib/astutils.h‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ bool isLikelyStreamRead(bool cpp, const Token *op);
162162

163163
class FwdAnalysis {
164164
public:
165-
FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mReassign(false) {}
165+
FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mWhat(What::Reassign) {}
166166

167167
bool hasOperand(const Token *tok, const Token *lhs) const;
168168

@@ -175,6 +175,18 @@ class FwdAnalysis {
175175
*/
176176
const Token *reassign(const Token *expr, const Token *startToken, const Token *endToken);
177177

178+
/**
179+
* Check if "expr" is used. The "expr" can be a tree (x.y[12]).
180+
* @param expr Symbolic expression to perform forward analysis for
181+
* @param startToken First token in forward analysis
182+
* @param endToken Last token in forward analysis
183+
* @return true if expr is used.
184+
*/
185+
bool unusedValue(const Token *expr, const Token *startToken, const Token *endToken);
186+
187+
/** Is there some possible alias for given expression */
188+
bool possiblyAliased(const Token *expr, const Token *startToken) const;
189+
178190
private:
179191
/** Result of forward analysis */
180192
struct Result {
@@ -187,10 +199,12 @@ class FwdAnalysis {
187199
struct Result check(const Token *expr, const Token *startToken, const Token *endToken);
188200
struct Result checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<unsigned int> &exprVarIds, bool local);
189201

202+
// Is expression a l-value global data?
203+
bool isGlobalData(const Token *expr) const;
204+
190205
const bool mCpp;
191206
const Library &mLibrary;
192-
bool mReassign;
193-
std::vector<const Token *> mReads;
207+
enum class What { Reassign, UnusedValue } mWhat;
194208
};
195209

196210
#endif // astutilsH

‎lib/checkunusedvar.cpp‎

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,6 +1229,60 @@ void CheckUnusedVar::checkFunctionVariableUsage()
12291229
if (scope->hasInlineOrLambdaFunction())
12301230
continue;
12311231

1232+
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
1233+
if (Token::simpleMatch(tok, "] ("))
1234+
// todo: handle lambdas
1235+
break;
1236+
if (Token::simpleMatch(tok, "try {"))
1237+
// todo: check try blocks
1238+
tok = tok->linkAt(1);
1239+
// not assignment/initialization => continue
1240+
if ((!tok->isAssignmentOp() || !tok->astOperand1()) && !(Token::Match(tok, "%var% (") && tok->variable() && tok->variable()->nameToken() == tok))
1241+
continue;
1242+
if (tok->isName()) {
1243+
if (!tok->valueType() || !tok->valueType()->isIntegral())
1244+
continue;
1245+
tok = tok->next();
1246+
}
1247+
if (tok->astParent() && tok->str() != "(") {
1248+
const Token *parent = tok->astParent();
1249+
while (Token::Match(parent, "%oror%|%comp%|!|&&"))
1250+
parent = parent->astParent();
1251+
if (!parent)
1252+
continue;
1253+
if (!Token::simpleMatch(parent->previous(), "if ("))
1254+
continue;
1255+
}
1256+
// Do not warn about assignment with NULL
1257+
if (tok->astOperand1() && tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer && Token::Match(tok->astOperand2(), "0|NULL|nullptr"))
1258+
continue;
1259+
1260+
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference() && tok->astOperand1()->variable()->nameToken() != tok->astOperand1())
1261+
// todo: check references
1262+
continue;
1263+
1264+
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isStatic())
1265+
// todo: check static variables
1266+
continue;
1267+
1268+
if (tok->astOperand1()->variable() && tok->astOperand1()->variable()->nameToken()->isAttributeUnused())
1269+
continue;
1270+
1271+
// Is there a redundant assignment?
1272+
const Token *start = tok->findExpressionStartEndTokens().second->next();
1273+
1274+
const Token *expr = tok->astOperand1();
1275+
if (Token::Match(expr->previous(), "%var% [") && expr->previous()->variable() && expr->previous()->variable()->nameToken() == expr->previous())
1276+
expr = expr->previous();
1277+
else if (Token::Match(expr, "& %var% ="))
1278+
expr = expr->next();
1279+
1280+
FwdAnalysis fwdAnalysis(mTokenizer->isCPP(), mSettings->library);
1281+
if (fwdAnalysis.unusedValue(expr, start, scope->bodyEnd))
1282+
// warn
1283+
unreadVariableError(tok, expr->expressionString(), false);
1284+
}
1285+
12321286
// varId, usage {read, write, modified}
12331287
Variables variables;
12341288

@@ -1267,10 +1321,6 @@ void CheckUnusedVar::checkFunctionVariableUsage()
12671321
else if (usage._modified && !usage._write && !usage._allocateMemory && var && !var->isStlType())
12681322
unassignedVariableError(usage._var->nameToken(), varname);
12691323

1270-
// variable has been written but not read
1271-
else if (!usage._read)
1272-
unreadVariableError(usage._lastAccess, varname, usage._modified);
1273-
12741324
// variable has been read but not written
12751325
else if (!usage._write && !usage._allocateMemory && var && !var->isStlType() && !isEmptyType(var->type()))
12761326
unassignedVariableError(usage._var->nameToken(), varname);

‎samples/arrayIndexOutOfBounds/bad.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
int a[2];
2+
13
int main()
24
{
3-
int a[2];
45
a[0] = 0;
56
a[1] = 0;
67
a[2] = 0;

‎samples/arrayIndexOutOfBounds/good.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
int a[3];
2+
13
int main()
24
{
3-
int a[3];
45
a[0] = 0;
56
a[1] = 0;
67
a[2] = 0;
7-
return a[0];
8+
return 0;
89
}

‎samples/arrayIndexOutOfBounds/out.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[samples\arrayIndexOutOfBounds\bad.c:6]: (error) Array 'a[2]' accessed at index 2, which is out of bounds.
1+
[samples\arrayIndexOutOfBounds\bad.c:7]: (error) Array 'a[2]' accessed at index 2, which is out of bounds.

‎test/cfg/std.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,14 @@ void nullpointerMemchr1(char *p, char *s)
251251
{
252252
// cppcheck-suppress uselessAssignmentPtrArg
253253
p = memchr(s, 'p', strlen(s));
254+
(void)p;
254255
}
255256

256257
void nullpointerMemchr2(char *p, char *s)
257258
{
258259
// cppcheck-suppress uselessAssignmentPtrArg
259260
p = memchr(s, 0, strlen(s));
261+
(void)p;
260262
}
261263

262264
void nullPointer_memchr(char *p)
@@ -265,6 +267,7 @@ void nullPointer_memchr(char *p)
265267
// cppcheck-suppress nullPointer
266268
// cppcheck-suppress uselessAssignmentPtrArg
267269
p = memchr(s, 0, strlen(s));
270+
(void)p;
268271
}
269272

270273
void nullPointer_memcmp(char *p)

‎test/cfg/windows.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ void nullPointer()
257257
//Incorrect: 1. parameter, must not be null
258258
// cppcheck-suppress nullPointer
259259
FARPROC pAddr = GetProcAddress(NULL, "name");
260+
(void)pAddr;
260261
HMODULE * phModule = NULL;
261262
// cppcheck-suppress nullPointer
262263
GetModuleHandleEx(0, NULL, phModule);

‎test/cfg/wxwidgets.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@
2626
void validCode()
2727
{
2828
wxString str = wxGetCwd();
29+
(void)str;
2930

3031
wxLogGeneric(wxLOG_Message, "test %d", 0);
3132
wxLogMessage("test %s", "str");
3233

3334
wxString translation1 = _("text");
3435
wxString translation2 = wxGetTranslation("text");
3536
wxString translation3 = wxGetTranslation("string", "domain");
37+
(void)translation1;
38+
(void)translation2;
39+
(void)translation3;
3640
}
3741

3842
#if wxUSE_GUI==1

0 commit comments

Comments
 (0)