Fixed #2058 (Warn for unused variable when only doing malloc/free)

This commit is contained in:
Zachary Blair 2010-11-14 17:37:36 -08:00
parent 378e83e73d
commit 36d80d6eaa
3 changed files with 156 additions and 6 deletions

View File

@ -559,13 +559,15 @@ public:
Scope *scope = NULL,
bool read = false,
bool write = false,
bool modified = false) :
bool modified = false,
bool allocateMemory = false) :
_name(name),
_type(type),
_scope(scope),
_read(read),
_write(write),
_modified(modified)
_modified(modified),
_allocateMemory(allocateMemory)
{
}
@ -588,6 +590,7 @@ public:
bool _read;
bool _write;
bool _modified; // read/modify/write
bool _allocateMemory;
std::set<unsigned int> _aliases;
std::set<Scope *> _assignments;
};
@ -603,6 +606,7 @@ public:
return _varUsage;
}
void addVar(const Token *name, VariableType type, Scope *scope, bool write_);
void allocateMemory(unsigned int varid);
void read(unsigned int varid);
void readAliases(unsigned int varid);
void readAll(unsigned int varid);
@ -727,6 +731,14 @@ void Variables::addVar(const Token *name,
_varUsage.insert(std::make_pair(name->varId(), VariableUsage(name, type, scope, false, write_, false)));
}
void Variables::allocateMemory(unsigned int varid)
{
VariableUsage *usage = find(varid);
if (usage)
usage->_allocateMemory = true;
}
void Variables::read(unsigned int varid)
{
VariableUsage *usage = find(varid);
@ -1293,6 +1305,9 @@ void CheckOther::functionVariableUsage()
if (tok->str() == "const")
tok = tok->next();
if (tok->strAt(1) == "::")
tok = tok->tokAt(2);
if (tok->str() != "return" && tok->str() != "throw")
{
Variables::VariableType type;
@ -1495,7 +1510,36 @@ void CheckOther::functionVariableUsage()
tok = tok->tokAt(6);
}
else if (Token::Match(tok, "delete|return|throw %var%"))
// Freeing memory (not considered "using" the pointer if it was also allocated in this function)
else if (Token::Match(tok, "free|g_free|kfree|vfree ( %var% )") ||
Token::Match(tok, "delete %var% ;") ||
Token::Match(tok, "delete [ ] %var% ;"))
{
unsigned int varid = 0;
if (tok->str() != "delete")
{
varid = tok->tokAt(2)->varId();
tok = tok->tokAt(3);
}
else if (tok->strAt(1) == "[")
{
varid = tok->tokAt(3)->varId();
tok = tok->tokAt(4);
}
else
{
varid = tok->next()->varId();
tok = tok->tokAt(2);
}
Variables::VariableUsage *var = variables.find(varid);
if (var && !var->_allocateMemory)
{
variables.readAll(varid);
}
}
else if (Token::Match(tok, "return|throw %var%"))
variables.readAll(tok->next()->varId());
// assignment
@ -1544,8 +1588,16 @@ void CheckOther::functionVariableUsage()
variables.writeAliases(varid1);
variables.read(varid1);
}
// Consider allocating memory separately because allocating/freeing alone does not constitute using the variable
else if (var && var->_type == Variables::pointer &&
Token::Match(start, "%var% = new|malloc|calloc|g_malloc|kmalloc|vmalloc"))
{
variables.allocateMemory(varid1);
}
else
{
variables.write(varid1);
}
Variables::VariableUsage *var2 = variables.find(tok->varId());
if (var2 && var2->_type == Variables::reference)
@ -1580,7 +1632,13 @@ void CheckOther::functionVariableUsage()
if (var)
{
if (var->_type == Variables::pointer || var->_type == Variables::reference)
// Consider allocating memory separately because allocating/freeing alone does not constitute using the variable
if (var->_type == Variables::pointer &&
Token::Match(tok->next()->link(), "] = new|malloc|calloc|g_malloc|kmalloc|vmalloc"))
{
variables.allocateMemory(varid);
}
else if (var->_type == Variables::pointer || var->_type == Variables::reference)
{
variables.read(varid);
variables.writeAliases(varid);
@ -1653,8 +1711,13 @@ void CheckOther::functionVariableUsage()
usage._type == Variables::referenceArray)
continue;
// variable has had memory allocated for it, but hasn't done
// anything with that memory other than, perhaps, freeing it
if (usage.unused() && !usage._modified && usage._allocateMemory)
allocatedButUnusedVariableError(usage._name, varname);
// variable has not been written, read, or modified
if (usage.unused() && !usage._modified)
else if (usage.unused() && !usage._modified)
unusedVariableError(usage._name, varname);
// variable has not been written but has been modified
@ -1666,7 +1729,7 @@ void CheckOther::functionVariableUsage()
unreadVariableError(usage._name, varname);
// variable has been read but not written
else if (!usage._write)
else if (!usage._write && !usage._allocateMemory)
unassignedVariableError(usage._name, varname);
}
}
@ -1677,6 +1740,11 @@ void CheckOther::unusedVariableError(const Token *tok, const std::string &varnam
reportError(tok, Severity::style, "unusedVariable", "Unused variable: " + varname);
}
void CheckOther::allocatedButUnusedVariableError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::style, "unusedVariable", "Variable '" + varname + "' is allocated memory that is never used");
}
void CheckOther::unreadVariableError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::style, "unreadVariable", "Variable '" + varname + "' is assigned a value that is never used");

View File

@ -102,6 +102,7 @@ public:
/** @brief %Check for unused function variables */
void functionVariableUsage();
void unusedVariableError(const Token *tok, const std::string &varname);
void allocatedButUnusedVariableError(const Token *tok, const std::string &varname);
void unreadVariableError(const Token *tok, const std::string &varname);
void unassignedVariableError(const Token *tok, const std::string &varname);
@ -212,6 +213,7 @@ public:
invalidScanfError(0);
incorrectLogicOperatorError(0);
unusedVariableError(0, "varname");
allocatedButUnusedVariableError(0, "varname");
unreadVariableError(0, "varname");
unassignedVariableError(0, "varname");
}

View File

@ -86,6 +86,7 @@ private:
TEST_CASE(localvaralias10); // ticket #2004
TEST_CASE(localvarasm);
TEST_CASE(localvarstatic);
TEST_CASE(localvardynamic);
// Don't give false positives for variables in structs/unions
TEST_CASE(localvarStruct1);
@ -2422,6 +2423,85 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:4]: (style) Variable 'b' is assigned a value that is never used\n", errout.str());
}
void localvardynamic()
{
functionVariableUsage("void foo()\n"
"{\n"
" void* ptr = malloc(16);\n"
" free(ptr);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" void* ptr = g_malloc(16);\n"
" g_free(ptr);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" void* ptr = kmalloc(16, GFP_KERNEL);\n"
" kfree(ptr);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" void* ptr = vmalloc(16, GFP_KERNEL);\n"
" vfree(ptr);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" char* ptr = new char[16];\n"
" delete[] ptr;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" char* ptr = new char;\n"
" delete ptr;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'ptr' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" void* ptr = malloc(16);\n"
" ptr[0] = 123;\n"
" free(ptr);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" char* ptr = new char[16];\n"
" ptr[0] = 123;\n"
" delete[] ptr;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" string* txt = new string(\"test\");\n"
" std::cout << \"test\" << std::endl;\n"
" delete txt;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (style) Variable 'txt' is allocated memory that is never used\n", errout.str());
functionVariableUsage("void foo()\n"
"{\n"
" char* ptr = names[i];\n"
" delete[] ptr;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestUnusedVar)