walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Auto variables: Fix false negatives for normal tokens

This commit is contained in:
Daniel Marjamäki 2019-03-14 13:51:35 +01:00
parent 7911684399
commit 3656f1ae4f
3 changed files with 41 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
lib/checkautovariables.cpp
View File

@ -167,14 +167,14 @@ static bool checkRvalueExpression(const Token * const vartok)
return ((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != "."); return ((next->str() != "." || (!var->isPointer() && (!var->isClass() || var->type()))) && next->strAt(2) != ".");
} }
static bool isAddressOfLocalVariableRecursive(const Token *expr) static bool isAddressOfLocalVariable(const Token *expr)
{ {
if (!expr) if (!expr)
return false; return false;
if (Token::Match(expr, "+|-")) if (Token::Match(expr, "+|-"))
return isAddressOfLocalVariableRecursive(expr->astOperand1()) || isAddressOfLocalVariableRecursive(expr->astOperand2()); return isAddressOfLocalVariable(expr->astOperand1()) || isAddressOfLocalVariable(expr->astOperand2());
if (expr->isCast()) if (expr->isCast())
return isAddressOfLocalVariableRecursive(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1()); return isAddressOfLocalVariable(expr->astOperand2() ? expr->astOperand2() : expr->astOperand1());
if (expr->isUnaryOp("&")) { if (expr->isUnaryOp("&")) {
const Token *op = expr->astOperand1(); const Token *op = expr->astOperand1();
bool deref = false; bool deref = false;
@ -190,13 +190,6 @@ static bool isAddressOfLocalVariableRecursive(const Token *expr)
return false; return false;
} }
static bool isAddressOfLocalVariable(const Token *expr)
{
if (!expr || !expr->valueType() || expr->valueType()->pointer == 0)
return false;
return isAddressOfLocalVariableRecursive(expr);
}
static bool variableIsUsedInScope(const Token* start, unsigned int varId, const Scope *scope) static bool variableIsUsedInScope(const Token* start, unsigned int varId, const Scope *scope)
{ {
if (!start) // Ticket #5024 if (!start) // Ticket #5024
@ -271,6 +264,8 @@ void CheckAutoVariables::autoVariables()
errorAutoVariableAssignment(tok->next(), false); errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) { } else if (Token::Match(tok, "[;{}] * %var% =") && isPtrArg(tok->tokAt(2)) && isAddressOfLocalVariable(tok->tokAt(3)->astOperand2())) {
errorAutoVariableAssignment(tok->next(), false); errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) {
errorAutoVariableAssignment(tok->next(), false);
} else if (mSettings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = &| %var% ;") && isGlobalPtr(tok->next())) { } else if (mSettings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = &| %var% ;") && isGlobalPtr(tok->next())) {
const Token * const pointer = tok->next(); const Token * const pointer = tok->next();
if (isAutoVarArray(tok->tokAt(3))) { if (isAutoVarArray(tok->tokAt(3))) {
@ -282,14 +277,6 @@ void CheckAutoVariables::autoVariables()
if (!reassignedGlobalPointer(variable, pointer->varId(), mSettings, mTokenizer->isCPP())) if (!reassignedGlobalPointer(variable, pointer->varId(), mSettings, mTokenizer->isCPP()))
errorAssignAddressOfLocalVariableToGlobalPointer(pointer, variable); errorAssignAddressOfLocalVariableToGlobalPointer(pointer, variable);
} }
} else if (Token::Match(tok, "[;{}] %var% . %var% = & %var%")) {
// TODO: check if the parameter is only changed temporarily (#2969)
if (printInconclusive && isPtrArg(tok->next())) {
const Token * const var2tok = tok->tokAt(6);
if (isAutoVar(var2tok) && checkRvalueExpression(var2tok))
errorAutoVariableAssignment(tok->next(), true);
}
tok = tok->tokAt(6);
} else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) { } else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) {
// TODO: check if the parameter is only changed temporarily (#2969) // TODO: check if the parameter is only changed temporarily (#2969)
if (printInconclusive && isPtrArg(tok->next())) { if (printInconclusive && isPtrArg(tok->next())) {
@ -304,10 +291,8 @@ void CheckAutoVariables::autoVariables()
errorAutoVariableAssignment(tok->next(), false); errorAutoVariableAssignment(tok->next(), false);
} }
tok = tok->tokAt(4); tok = tok->tokAt(4);
} else if (Token::Match(tok, "[;{}] %var% [") && Token::Match(tok->linkAt(2), "] = & %var%") && } else if (Token::Match(tok, "[;{}] %var% [") && Token::simpleMatch(tok->linkAt(2), "] =") &&
(isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAutoVar(tok->linkAt(2)->tokAt(3))) { (isPtrArg(tok->next()) || isArrayArg(tok->next())) && isAddressOfLocalVariable(tok->linkAt(2)->next()->astOperand2())) {
const Token* const varTok = tok->linkAt(2)->tokAt(3);
if (checkRvalueExpression(varTok))
errorAutoVariableAssignment(tok->next(), false); errorAutoVariableAssignment(tok->next(), false);
} }
// Invalid pointer deallocation // Invalid pointer deallocation
@ -354,14 +339,14 @@ void CheckAutoVariables::errorReturnPointerToLocalArray(const Token *tok)
void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive) void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive)
{ {
if (!inconclusive) { if (!inconclusive) {
reportError(tok, Severity::error, "autoVariables", reportError(tok, Severity::warning, "autoVariables",
"Address of local auto-variable assigned to a function parameter.\n" "Address of local auto-variable assigned to a function parameter.\n"
"Dangerous assignment - the function parameter is assigned the address of a local " "Dangerous assignment - the function parameter is assigned the address of a local "
"auto-variable. Local auto-variables are reserved from the stack which " "auto-variable. Local auto-variables are reserved from the stack which "
"is freed when the function ends. So the pointer to a local variable " "is freed when the function ends. So the pointer to a local variable "
"is invalid after the function ends.", CWE562, false); "is invalid after the function ends.", CWE562, false);
} else { } else {
reportError(tok, Severity::error, "autoVariables", reportError(tok, Severity::warning, "autoVariables",
"Address of local auto-variable assigned to a function parameter.\n" "Address of local auto-variable assigned to a function parameter.\n"
"Function parameter is assigned the address of a local auto-variable. " "Function parameter is assigned the address of a local auto-variable. "
"Local auto-variables are reserved from the stack which is freed when " "Local auto-variables are reserved from the stack which is freed when "

2
samples/autoVariables/out.txt
View File

@ -1 +1 @@
[samples\autoVariables\bad.c:4]: (error) Address of local auto-variable assigned to a function parameter. [samples\autoVariables\bad.c:4]: (warning) Address of local auto-variable assigned to a function parameter.

66
test/testautovariables.cpp
View File

@ -31,7 +31,7 @@ public:
private: private:
Settings settings; Settings settings;
void check(const char code[], bool inconclusive = false, bool runSimpleChecks = true, const char* filename = "test.cpp") { void check(const char code[], bool inconclusive = false, const char* filename = "test.cpp") {
// Clear the error buffer.. // Clear the error buffer..
errout.str(""); errout.str("");
@ -42,17 +42,8 @@ private:
std::istringstream istr(code); std::istringstream istr(code);
tokenizer.tokenize(istr, filename); tokenizer.tokenize(istr, filename);
CheckAutoVariables checkAutoVariables(&tokenizer, &settings, this); CheckAutoVariables checkAutoVariables;
checkAutoVariables.returnReference(); checkAutoVariables.runChecks(&tokenizer, &settings, this);
checkAutoVariables.assignFunctionArg();
checkAutoVariables.checkVarLifetime();
if (runSimpleChecks) {
tokenizer.simplifyTokenList2();
// Check auto variables
checkAutoVariables.autoVariables();
}
} }
void run() OVERRIDE { void run() OVERRIDE {
@ -78,6 +69,7 @@ private:
TEST_CASE(testautovar16); // ticket #8114 TEST_CASE(testautovar16); // ticket #8114
TEST_CASE(testautovar_array1); TEST_CASE(testautovar_array1);
TEST_CASE(testautovar_array2); TEST_CASE(testautovar_array2);
TEST_CASE(testautovar_normal); // "normal" token list that does not remove casts etc
TEST_CASE(testautovar_ptrptr); // ticket #6956 TEST_CASE(testautovar_ptrptr); // ticket #6956
TEST_CASE(testautovar_return1); TEST_CASE(testautovar_return1);
TEST_CASE(testautovar_return2); TEST_CASE(testautovar_return2);
@ -141,7 +133,7 @@ private:
" int num = 2;\n" " int num = 2;\n"
" *res = #\n" " *res = #\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("void func1(int **res)\n" check("void func1(int **res)\n"
"{\n" "{\n"
@ -167,7 +159,7 @@ private:
" int num = 2;\n" " int num = 2;\n"
" *res = #\n" " *res = #\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:7]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:7]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("class Fred {\n" check("class Fred {\n"
" void func1(int **res);\n" " void func1(int **res);\n"
@ -196,7 +188,7 @@ private:
" int x[100];\n" " int x[100];\n"
" *p = x;\n" " *p = x;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar4() { // ticket #2928 void testautovar4() { // ticket #2928
@ -213,15 +205,8 @@ private:
"{\n" "{\n"
" char a;\n" " char a;\n"
" ab->a = &a;\n" " ab->a = &a;\n"
"}", false); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("void foo(struct AB *ab)\n"
"{\n"
" char a;\n"
" ab->a = &a;\n"
"}", true);
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar6() { // ticket #2931 void testautovar6() { // ticket #2931
@ -237,7 +222,7 @@ private:
" char a[10];\n" " char a[10];\n"
" x->str = a;\n" " x->str = a;\n"
"}", true); "}", true);
ASSERT_EQUALS("[test.cpp:4]: (error, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar7() { // ticket #3066 void testautovar7() { // ticket #3066
@ -255,7 +240,7 @@ private:
" int i = 0;\n" " int i = 0;\n"
" p = &i;\n" " p = &i;\n"
"}", false); "}", false);
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("void foo(std::string& s) {\n" check("void foo(std::string& s) {\n"
" s = foo;\n" " s = foo;\n"
@ -273,7 +258,7 @@ private:
" p = &p_fp->i;\n" " p = &p_fp->i;\n"
" p = &fp.f->i;\n" " p = &fp.f->i;\n"
"}", false); "}", false);
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar10() { // #2930 - assignment of function parameter void testautovar10() { // #2930 - assignment of function parameter
@ -350,7 +335,7 @@ private:
" int i = d;\n" " int i = d;\n"
" d = i;\n" " d = i;\n"
" return d;" " return d;"
"}",false,false); "}",false);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void foo(int* ptr) {\n" // #4793 check("void foo(int* ptr) {\n" // #4793
@ -386,7 +371,7 @@ private:
" struct A a = bar();\n" " struct A a = bar();\n"
" *p = &a.data[0];\n" " *p = &a.data[0];\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
check("void f(char **out) {\n" check("void f(char **out) {\n"
" struct S *p = glob;\n" " struct S *p = glob;\n"
@ -405,7 +390,7 @@ private:
" s8 p[10];\n" // <- p is array => error " s8 p[10];\n" // <- p is array => error
" *out = &p[1];\n" " *out = &p[1];\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar12() { // Ticket #5024, #5050 - Crash on invalid input void testautovar12() { // Ticket #5024, #5050 - Crash on invalid input
@ -447,7 +432,7 @@ private:
" if (lumdiff > 5.0f)\n" " if (lumdiff > 5.0f)\n"
" return &darkOutline;\n" " return &darkOutline;\n"
" return 0;\n" " return 0;\n"
"}", false, false); "}", false);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -465,7 +450,7 @@ private:
" int num=2;" " int num=2;"
" arr[0]=&num;\n" " arr[0]=&num;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar_array2() { void testautovar_array2() {
@ -477,7 +462,16 @@ private:
" int num=2;" " int num=2;"
" arr[0]=&num;\n" " arr[0]=&num;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:6]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
}
void testautovar_normal() {
check("void f(XmDestinationCallbackStruct *ds)\n"
"{\n"
" XPoint DropPoint;\n"
" ds->location_data = (XtPointer *)&DropPoint;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar_ptrptr() { // #6596 void testautovar_ptrptr() { // #6596
@ -485,7 +479,7 @@ private:
" char dead_slot;\n" " char dead_slot;\n"
" matches[0] = (char *)&dead_slot;\n" " matches[0] = (char *)&dead_slot;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Address of local auto-variable assigned to a function parameter.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (warning) Address of local auto-variable assigned to a function parameter.\n", errout.str());
} }
void testautovar_return1() { void testautovar_return1() {
@ -677,7 +671,7 @@ private:
check("void svn_repos_dir_delta2() {\n" check("void svn_repos_dir_delta2() {\n"
" struct context c;\n" " struct context c;\n"
" SVN_ERR(delete(&c, root_baton, src_entry, pool));\n" " SVN_ERR(delete(&c, root_baton, src_entry, pool));\n"
"}\n", false, true, "test.c"); "}\n", false, "test.c");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -1055,7 +1049,7 @@ private:
" double ret = getValue();\n" " double ret = getValue();\n"
" rd = ret;\n" " rd = ret;\n"
" return rd;\n" " return rd;\n"
"}", false, false); "}", false);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }