Skip to content

Commit 3656f1a

Browse files
danmardanmar
committed
Auto variables: Fix false negatives for normal tokens
1 parent 7911684 commit 3656f1a

3 files changed

Lines changed: 41 additions & 62 deletions

File tree

‎lib/checkautovariables.cpp‎

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,14 @@ static bool checkRvalueExpression(const Token * const vartok)
167167
return ((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != ".");
168168
}
169169

170-
static bool isAddressOfLocalVariableRecursive(const Token *expr)
170+
static bool isAddressOfLocalVariable(const Token *expr)
171171
{
172172
if (!expr)
173173
return false;
174174
if (Token::Match(expr, "+|-"))
175-
return isAddressOfLocalVariableRecursive(expr->astOperand1()) || isAddressOfLocalVariableRecursive(expr->astOperand2());
175+
return isAddressOfLocalVariable(expr->astOperand1()) || isAddressOfLocalVariable(expr->astOperand2());
176176
if (expr->isCast())
177-
return isAddressOfLocalVariableRecursive(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1());
177+
return isAddressOfLocalVariable(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1());
178178
if (expr->isUnaryOp("&")) {
179179
const Token *op = expr->astOperand1();
180180
bool deref = false;
@@ -190,13 +190,6 @@ static bool isAddressOfLocalVariableRecursive(const Token *expr)
190190
return false;
191191
}
192192

193-
static bool isAddressOfLocalVariable(const Token *expr)
194-
{
195-
if (!expr || !expr->valueType() || expr->valueType()->pointer == 0)
196-
return false;
197-
return isAddressOfLocalVariableRecursive(expr);
198-
}
199-
200193
static bool variableIsUsedInScope(const Token* start, unsigned int varId, const Scope *scope)
201194
{
202195
if (!start) // Ticket #5024
@@ -271,6 +264,8 @@ void CheckAutoVariables::autoVariables()
271264
errorAutoVariableAssignment(tok->next(), false);
272265
} else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) {
273266
errorAutoVariableAssignment(tok->next(), false);
267+
} else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) {
268+
errorAutoVariableAssignment(tok->next(), false);
274269
} else if (mSettings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = &| %var% ;") && isGlobalPtr(tok->next())) {
275270
const Token * const pointer = tok->next();
276271
if (isAutoVarArray(tok->tokAt(3))) {
@@ -282,14 +277,6 @@ void CheckAutoVariables::autoVariables()
282277
if (!reassignedGlobalPointer(variable, pointer->varId(), mSettings, mTokenizer->isCPP()))
283278
errorAssignAddressOfLocalVariableToGlobalPointer(pointer, variable);
284279
}
285-
} else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) {
286-
// TODO: check if the parameter is only changed temporarily (#2969)
287-
if (printInconclusive && isPtrArg(tok->next())) {
288-
const Token * const var2tok = tok->tokAt(6);
289-
if (isAutoVar(var2tok) && checkRvalueExpression(var2tok))
290-
errorAutoVariableAssignment(tok->next(), true);
291-
}
292-
tok = tok->tokAt(6);
293280
} else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) {
294281
// TODO: check if the parameter is only changed temporarily (#2969)
295282
if (printInconclusive && isPtrArg(tok->next())) {
@@ -304,11 +291,9 @@ void CheckAutoVariables::autoVariables()
304291
errorAutoVariableAssignment(tok->next(), false);
305292
}
306293
tok = tok->tokAt(4);
307-
} else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") &&
308-
(isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAutoVar(tok->linkAt(2)->tokAt(3))) {
309-
const Token* const varTok = tok->linkAt(2)->tokAt(3);
310-
if (checkRvalueExpression(varTok))
311-
errorAutoVariableAssignment(tok->next(), false);
294+
} else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") &&
295+
(isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAddressOfLocalVariable(tok->linkAt(2)->next()->astOperand2())) {
296+
errorAutoVariableAssignment(tok->next(), false);
312297
}
313298
// Invalid pointer deallocation
314299
else if ((Token::Match(tok, "%name% ( %var% ) ;") && mSettings->library.dealloc(tok)) ||
@@ -354,14 +339,14 @@ void CheckAutoVariables::errorReturnPointerToLocalArray(const Token *tok)
354339
void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive)
355340
{
356341
if (!inconclusive) {
357-
reportError(tok, Severity::error, "autoVariables",
342+
reportError(tok, Severity::warning, "autoVariables",
358343
"Address of local auto-variable assigned to a function parameter.\n"
359344
"Dangerous assignment - the function parameter is assigned the address of a local "
360345
"auto-variable. Local auto-variables are reserved from the stack which "
361346
"is freed when the function ends. So the pointer to a local variable "
362347
"is invalid after the function ends.", CWE562, false);
363348
} else {
364-
reportError(tok, Severity::error, "autoVariables",
349+
reportError(tok, Severity::warning, "autoVariables",
365350
"Address of local auto-variable assigned to a function parameter.\n"
366351
"Function parameter is assigned the address of a local auto-variable. "
367352
"Local auto-variables are reserved from the stack which is freed when "

‎samples/autoVariables/out.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[samples\autoVariables\bad.c:4]: (error) Address of local auto-variable assigned to a function parameter.
1+
[samples\autoVariables\bad.c:4]: (warning) Address of local auto-variable assigned to a function parameter.

‎test/testautovariables.cpp‎

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class TestAutoVariables : public TestFixture {
3131
private:
3232
Settings settings;
3333

34-
void check(const char code[], bool inconclusive = false, bool runSimpleChecks = true, const char* filename = "test.cpp") {
34+
void check(const char code[], bool inconclusive = false, const char* filename = "test.cpp") {
3535
// Clear the error buffer..
3636
errout.str("");
3737

@@ -42,17 +42,8 @@ class TestAutoVariables : public TestFixture {
4242
std::istringstream istr(code);
4343
tokenizer.tokenize(istr, filename);
4444

45-
CheckAutoVariables checkAutoVariables(&tokenizer, &settings, this);
46-
checkAutoVariables.returnReference();
47-
checkAutoVariables.assignFunctionArg();
48-
checkAutoVariables.checkVarLifetime();
49-
50-
if (runSimpleChecks) {
51-
tokenizer.simplifyTokenList2();
52-
53-
// Check auto variables
54-
checkAutoVariables.autoVariables();
55-
}
45+
CheckAutoVariables checkAutoVariables;
46+
checkAutoVariables.runChecks(&tokenizer, &settings, this);
5647
}
5748

5849
void run() OVERRIDE {
@@ -78,6 +69,7 @@ class TestAutoVariables : public TestFixture {
7869
TEST_CASE(testautovar16); // ticket #8114
7970
TEST_CASE(testautovar_array1);
8071
TEST_CASE(testautovar_array2);
72+
TEST_CASE(testautovar_normal); // "normal" token list that does not remove casts etc
8173
TEST_CASE(testautovar_ptrptr); // ticket #6956
8274
TEST_CASE(testautovar_return1);
8375
TEST_CASE(testautovar_return2);
@@ -141,7 +133,7 @@ class TestAutoVariables : public TestFixture {
141133
" int num = 2;\n"
142134
" *res = #\n"
143135
"}");
144-
ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
136+
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
145137

146138
check("void func1(int **res)\n"
147139
"{\n"
@@ -167,7 +159,7 @@ class TestAutoVariables : public TestFixture {
167159
" int num = 2;\n"
168160
" *res = #\n"
169161
"}");
170-
ASSERT_EQUALS("[test.cpp:7]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
162+
ASSERT_EQUALS("[test.cpp:7]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
171163

172164
check("class Fred {\n"
173165
" void func1(int **res);\n"
@@ -196,7 +188,7 @@ class TestAutoVariables : public TestFixture {
196188
" int x[100];\n"
197189
" *p = x;\n"
198190
"}");
199-
ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
191+
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
200192
}
201193

202194
void testautovar4() { // ticket #2928
@@ -213,15 +205,8 @@ class TestAutoVariables : public TestFixture {
213205
"{\n"
214206
" char a;\n"
215207
" ab->a = &a;\n"
216-
"}", false);
217-
ASSERT_EQUALS("", errout.str());
218-
219-
check("void foo(struct AB *ab)\n"
220-
"{\n"
221-
" char a;\n"
222-
" ab->a = &a;\n"
223-
"}", true);
224-
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
208+
"}");
209+
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
225210
}
226211

227212
void testautovar6() { // ticket #2931
@@ -237,7 +222,7 @@ class TestAutoVariables : public TestFixture {
237222
" char a[10];\n"
238223
" x->str = a;\n"
239224
"}", true);
240-
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
225+
ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
241226
}
242227

243228
void testautovar7() { // ticket #3066
@@ -255,7 +240,7 @@ class TestAutoVariables : public TestFixture {
255240
" int i = 0;\n"
256241
" p = &i;\n"
257242
"}", false);
258-
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
243+
ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
259244

260245
check("void foo(std::string& s) {\n"
261246
" s = foo;\n"
@@ -273,7 +258,7 @@ class TestAutoVariables : public TestFixture {
273258
" p = &p_fp->i;\n"
274259
" p = &fp.f->i;\n"
275260
"}", false);
276-
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
261+
ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
277262
}
278263

279264
void testautovar10() { // #2930 - assignment of function parameter
@@ -350,7 +335,7 @@ class TestAutoVariables : public TestFixture {
350335
" int i = d;\n"
351336
" d = i;\n"
352337
" return d;"
353-
"}",false,false);
338+
"}",false);
354339
ASSERT_EQUALS("", errout.str());
355340

356341
check("void foo(int* ptr) {\n" // #4793
@@ -386,7 +371,7 @@ class TestAutoVariables : public TestFixture {
386371
" struct A a = bar();\n"
387372
" *p = &a.data[0];\n"
388373
"}");
389-
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
374+
ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
390375

391376
check("void f(char **out) {\n"
392377
" struct S *p = glob;\n"
@@ -405,7 +390,7 @@ class TestAutoVariables : public TestFixture {
405390
" s8 p[10];\n" // <- p is array => error
406391
" *out = &p[1];\n"
407392
"}");
408-
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
393+
ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
409394
}
410395

411396
void testautovar12() { // Ticket #5024, #5050 - Crash on invalid input
@@ -447,7 +432,7 @@ class TestAutoVariables : public TestFixture {
447432
" if (lumdiff > 5.0f)\n"
448433
" return &darkOutline;\n"
449434
" return 0;\n"
450-
"}", false, false);
435+
"}", false);
451436
ASSERT_EQUALS("", errout.str());
452437
}
453438

@@ -465,7 +450,7 @@ class TestAutoVariables : public TestFixture {
465450
" int num=2;"
466451
" arr[0]=&num;\n"
467452
"}");
468-
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
453+
ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
469454
}
470455

471456
void testautovar_array2() {
@@ -477,15 +462,24 @@ class TestAutoVariables : public TestFixture {
477462
" int num=2;"
478463
" arr[0]=&num;\n"
479464
"}");
480-
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
465+
ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
466+
}
467+
468+
void testautovar_normal() {
469+
check("void f(XmDestinationCallbackStruct *ds)\n"
470+
"{\n"
471+
" XPoint DropPoint;\n"
472+
" ds->location_data = (XtPointer *)&DropPoint;\n"
473+
"}");
474+
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
481475
}
482476

483477
void testautovar_ptrptr() { // #6596
484478
check("void remove_duplicate_matches (char **matches) {\n"
485479
" char dead_slot;\n"
486480
" matches[0] = (char *)&dead_slot;\n"
487481
"}");
488-
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str());
482+
ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
489483
}
490484

491485
void testautovar_return1() {
@@ -677,7 +671,7 @@ class TestAutoVariables : public TestFixture {
677671
check("void svn_repos_dir_delta2() {\n"
678672
" struct context c;\n"
679673
" SVN_ERR(delete(&c, root_baton, src_entry, pool));\n"
680-
"}\n", false, true, "test.c");
674+
"}\n", false, "test.c");
681675
ASSERT_EQUALS("", errout.str());
682676
}
683677

@@ -1055,7 +1049,7 @@ class TestAutoVariables : public TestFixture {
10551049
" double ret = getValue();\n"
10561050
" rd = ret;\n"
10571051
" return rd;\n"
1058-
"}", false, false);
1052+
"}", false);
10591053
ASSERT_EQUALS("", errout.str());
10601054
}
10611055

0 commit comments

Comments
 (0)