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

Improved nullpointer check: Detect dereferences by streams (#410) 

Refactorizations:
- Replaced || by %oror% in Token::Match patterns
- Replaced some simple patterns by direct comparisions, replaced Match call with simpleMatch
- Increased data encapsulation by making more members private in CheckNullpointer
This commit is contained in:
PKEuS 2012-03-16 17:24:03 +01:00
parent 4587a1a06c
commit 0340764726
3 changed files with 94 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
lib/checknullpointer.cpp
View File

@ -254,6 +254,25 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Sym
return true;
}
// streams dereference nullpointers
if (Token::Match(tok->previous(), "<<|>> %var%")) {
const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && Token::Match(var->typeStartToken(), "const| char const| *")) { // Only outputing or reading to char* can cause problems
const Token* tok2 = tok->previous(); // Find start of statement
for (; tok2; tok2 = tok2->previous()) {
if (Token::Match(tok2->previous(), ";|{|}|:"))
break;
}
if (Token::Match(tok2, "std :: cout|cin|cerr"))
return true;
if (tok2 && tok2->varId() != 0) {
const Variable* var2 = symbolDatabase->getVariableFromVarId(tok2->varId());
if (var2 && Token::Match(var2->typeStartToken(), "const| std :: istream|ifstream|istringstream|ostream|ofstream|ostringstream|stringstream|fstream|iostream"))
return true;
}
}
}
unsigned int ovarid = 0;
if (Token::Match(tok, "%var% ==|!= %var%"))
ovarid = tok->tokAt(2)->varId();
@ -505,7 +524,7 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec()
tok1 = tok1->next();
skipvar.insert(tok1->varId());
continue;
} else if (Token::Match(tok1, "( ! %var% ||") ||
} else if (Token::Match(tok1, "( ! %var% %oror%") ||
Token::Match(tok1, "( %var% &&")) {
// TODO: there are false negatives caused by this. The
// variable should be removed from skipvar after the
@ -724,7 +743,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
if (Token::Match(tok1->link()->previous(), "while ( %varid%", varid))
break;
if (Token::Match(tok1->link(), "( ! %varid% ||", varid) ||
if (Token::Match(tok1->link(), "( ! %varid% %oror%", varid) ||
Token::Match(tok1->link(), "( %varid% &&", varid)) {
tok1 = tok1->link();
continue;
@ -797,9 +816,9 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
} else if (CheckNullPointer::isPointerDeRef(tok1, unknown, symbolDatabase)) {
nullPointerError(tok1, varname, tok->linenr(), inconclusive);
break;
} else if (Token::simpleMatch(tok1->previous(), "&")) {
} else if (tok1->strAt(-1) == "&") {
break;
} else if (Token::simpleMatch(tok1->next(), "=")) {
} else if (tok1->strAt(1) == "=") {
break;
}
}
@ -847,7 +866,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
vartok = tok->next();
} else if (Token::Match(tok, "( ! ( %var% =")) {
vartok = tok->tokAt(3);
if (Token::Match(tok->tokAt(2)->link(), ") &&"))
if (Token::simpleMatch(tok->tokAt(2)->link(), ") &&"))
checkConditionStart = tok->tokAt(2)->link();
} else
continue;
@ -877,7 +896,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
const Token * const conditionEnd = tok->link();
for (const Token *tok2 = checkConditionStart; tok2 != conditionEnd; tok2 = tok2->next()) {
// If we hit a || operator, abort
if (Token::Match(tok2, "%oror%", varid)) {
if (tok2->str() == "||") {
break;
}
// Pointer is used
@ -1052,7 +1071,7 @@ void CheckNullPointer::nullConstantDereference()
nullPointerError(tok);
else if (Token::Match(tok->previous(), "!!. %var% (") && (tok->previous()->str() != "::" || tok->strAt(-2) == "std")) {
if (Token::Match(tok->tokAt(2), "0 )")) {
if (Token::simpleMatch(tok->tokAt(2), "0 )")) {
const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId());
if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::"))
nullPointerError(tok);
@ -1070,6 +1089,21 @@ void CheckNullPointer::nullConstantDereference()
} else if (Token::simpleMatch(tok, "std :: string ( 0 )"))
nullPointerError(tok);
else if (Token::simpleMatch(tok->previous(), ">> 0")) { // Only checking input stream operations is safe here, because otherwise 0 can be an integer as well
const Token* tok2 = tok->previous(); // Find start of statement
for (; tok2; tok2 = tok2->previous()) {
if (Token::Match(tok2->previous(), ";|{|}|:"))
break;
}
if (Token::simpleMatch(tok2, "std :: cin"))
nullPointerError(tok);
if (tok2 && tok2->varId() != 0) {
const Variable* var = symbolDatabase->getVariableFromVarId(tok2->varId());
if (var && Token::Match(var->typeStartToken(), "const| std :: istream|ifstream|istringstream|stringstream|fstream|iostream"))
nullPointerError(tok);
}
}
unsigned int ovarid = 0;
if (Token::Match(tok, "0 ==|!= %var%"))
ovarid = tok->tokAt(2)->varId();

4
lib/checknullpointer.h
View File

@ -102,6 +102,8 @@ public:
void nullPointerError(const Token *tok, const std::string &varname);
void nullPointerError(const Token *tok, const std::string &varname, const unsigned int line, bool inconclusive = false);
private:
/** Get error messages. Used by --errorlist */
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckNullPointer c(0, settings, errorLogger);
@ -119,8 +121,6 @@ public:
"* null pointer dereferencing\n";
}
private:
/**
* @brief Does one part of the check for nullPointer().
* Locate insufficient null-pointer handling after loop

51
test/testnullpointer.cpp
View File

@ -64,6 +64,7 @@ private:
TEST_CASE(nullpointerDelete);
TEST_CASE(nullpointerExit);
TEST_CASE(nullpointerStdString);
TEST_CASE(nullpointerStdStream);
TEST_CASE(functioncall);
TEST_CASE(crash1);
@ -1905,6 +1906,56 @@ private:
"[test.cpp:3]: (error) Null pointer dereference\n", errout.str());
}
void nullpointerStdStream() {
check("void f(std::ifstream& is) {\n"
" char* p = 0;\n"
" is >> p;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p\n", errout.str());
check("void f(const std::ostringstream& oss, char* q) {\n"
" char const* p = 0;\n" // Simplification makes detection of bug difficult
" oss << p;\n"
" oss << foo << p;\n"
" if(q == 0)\n"
" oss << foo << q;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p\n"
"[test.cpp:4]: (error) Possible null pointer dereference: p\n"
"[test.cpp:6]: (error) Possible null pointer dereference: q - otherwise it is redundant to check if q is null at line 5\n",
"[test.cpp:6]: (error) Possible null pointer dereference: q - otherwise it is redundant to check if q is null at line 5\n", errout.str());
check("void f(const char* p) {\n"
" if(p == 0) {\n"
" std::cout << p;\n"
" std::cerr << p;\n"
" std::cin >> p;\n"
" std::cout << abc << p;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 2\n"
"[test.cpp:4]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 2\n"
"[test.cpp:5]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 2\n"
"[test.cpp:6]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 2\n", errout.str());
check("void f() {\n"
" void* p1 = 0;\n"
" std::cout << p1;\n" // No char*
" char* p2 = 0;\n"
" std::cin >> (int)p;\n" // result casted
" std::cout << (int)p;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(const std::string& str) {\n"
" long long ret = 0;\n"
" std::istringstream istr(str);\n"
" istr >> std::hex >> ret;\n" // Read integer
" return ret;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void functioncall() { // #3443 - function calls
// dereference pointer and then check if it's null
{