Skip to content

Commit 42a7569

Browse files
PKEuSPKEuS
committed
Improved nullpointer check:
- More accurate checking for dereferences and non-dereferences - improved checking for nullpointer dereferences after return statement - Supports pointer dereferences by std::string - Code optimization/refactorization
1 parent 589a246 commit 42a7569

File tree

4 files changed

+163
-99
lines changed

4 files changed

+163
-99
lines changed

‎lib/checknullpointer.cpp‎

Lines changed: 108 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
129129
}
130130

131131
// 2nd parameter..
132-
if (Token::Match(&tok, "%var% ( %any%")) {
132+
if (Token::Match(&tok, "%var% ( !!)")) {
133133
const Token* secondParameter = tok.tokAt(2)->nextArgument();
134134
if (secondParameter && ((value == 0 && secondParameter->str() == "0") || (Token::Match(secondParameter, "%var%") && secondParameter->varId() > 0))) {
135135
if (functionNames2_all.find(tok.str()) != functionNames2_all.end())
@@ -216,14 +216,14 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
216216
* @param unknown it is not known if there is a pointer dereference (could be reported as a debug message)
217217
* @return true => there is a dereference
218218
*/
219-
bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
219+
bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const SymbolDatabase* symbolDatabase)
220220
{
221221
const bool inconclusive = unknown;
222222

223223
unknown = false;
224224

225225
// Dereferencing pointer..
226-
if (Token::Match(tok->tokAt(-3), "!!sizeof [;{}=+-/(,] * %var%") && Token::Match(tok->tokAt(-3), "!!decltype [;{}=+-/(,] * %var%"))
226+
if (tok->strAt(-1) == "*" && (Token::Match(tok->tokAt(-2), "return|throw|;|{|}|:|[|(|,") || tok->tokAt(-2)->isOp() || tok->tokAt(-2)->isAssignmentOp()) && !Token::Match(tok->tokAt(-3), "sizeof|decltype"))
227227
return true;
228228

229229
// read/write member variable
@@ -234,7 +234,7 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
234234
return false;
235235
}
236236

237-
if (Token::Match(tok->previous(), "!!& %var% ["))
237+
if (Token::Match(tok, "%var% [") && (tok->previous()->str() != "&" || Token::Match(tok->next()->link()->next(), "[.(]")))
238238
return true;
239239

240240
if (Token::Match(tok, "%var% ("))
@@ -245,6 +245,23 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
245245
tok->varId() == tok->tokAt(2)->varId())
246246
return true;
247247

248+
// std::string dereferences nullpointers
249+
if (Token::Match(tok->tokAt(-4), "std :: string ( %var% )"))
250+
return true;
251+
if (Token::Match(tok->tokAt(-5), "std :: string %var% ( %var% )"))
252+
return true;
253+
254+
unsigned int ovarid = 0;
255+
if (Token::Match(tok, "%var% ==|!= %var%"))
256+
ovarid = tok->tokAt(2)->varId();
257+
else if (Token::Match(tok->tokAt(-2), "%var% ==|!=|=|+= %var%"))
258+
ovarid = tok->tokAt(-2)->varId();
259+
if (ovarid) {
260+
const Variable* var = symbolDatabase->getVariableFromVarId(ovarid);
261+
if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::"))
262+
return true;
263+
}
264+
248265
// Check if it's NOT a pointer dereference.
249266
// This is most useful in inconclusive checking
250267
if (inconclusive) {
@@ -382,7 +399,7 @@ void CheckNullPointer::nullPointerAfterLoop()
382399
bool unknown = _settings->inconclusive;
383400

384401
// Is the loop variable dereferenced?
385-
if (CheckNullPointer::isPointerDeRef(tok2, unknown)) {
402+
if (CheckNullPointer::isPointerDeRef(tok2, unknown, symbolDatabase)) {
386403
nullPointerError(tok2, varname, tok->linenr(), inconclusive);
387404
}
388405

@@ -422,31 +439,33 @@ void CheckNullPointer::nullPointerLinkedList()
422439
if (varid == 0)
423440
continue;
424441

425-
if (Token::Match(tok2->tokAt(-2), "%varid% ?", varid))
442+
// Is this variable a pointer?
443+
const Variable* var = symbolDatabase->getVariableFromVarId(varid);
444+
if (!var || !var->isPointer())
426445
continue;
427446

428-
// Variable name of dereferenced variable
429-
const std::string varname(tok2->str());
447+
if (Token::Match(tok2->tokAt(-2), "%varid% ?", varid))
448+
continue;
430449

431450
// Check usage of dereferenced variable in the loop..
432-
for (const Token *tok3 = i->classStart->next(); tok3 && tok3 != i->classEnd; tok3 = tok3->next()) {
451+
for (std::list<Scope*>::const_iterator j = i->nestedList.begin(); j != i->nestedList.end(); ++j) {
452+
Scope* scope = *j;
453+
if (scope->type != Scope::eWhile)
454+
continue;
455+
433456
// TODO: are there false negatives for "while ( %varid% ||"
434-
if (Token::Match(tok3, "while ( %varid% &&|)", varid)) {
457+
if (Token::Match(scope->classDef->next(), "( %varid% &&|)", varid)) {
435458
// Make sure there is a "break" or "return" inside the loop.
436459
// Without the "break" a null pointer could be dereferenced in the
437460
// for statement.
438461
// indentlevel4 is a counter for { and }. When scanning the code with tok4
439462
unsigned int indentlevel4 = 1;
440-
for (const Token *tok4 = tok3->next()->link(); tok4; tok4 = tok4->next()) {
463+
for (const Token *tok4 = scope->classStart; tok4; tok4 = tok4->next()) {
441464
if (tok4->str() == "{")
442465
++indentlevel4;
443466
else if (tok4->str() == "}") {
444467
if (indentlevel4 <= 1) {
445-
const Variable* var = symbolDatabase->getVariableFromVarId(varid);
446-
// Is this variable a pointer?
447-
if (var && var->isPointer())
448-
nullPointerError(tok1, varname, tok3->linenr());
449-
468+
nullPointerError(tok1, var->name(), scope->classDef->linenr());
450469
break;
451470
}
452471
--indentlevel4;
@@ -758,7 +777,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
758777

759778
// unknown : this is set by isPointerDeRef if it is
760779
// uncertain
761-
bool unknown = false;
780+
bool unknown = _settings->inconclusive;
762781

763782
if (Token::Match(tok1->tokAt(-2), "%varid% = %varid% .", varid)) {
764783
break;
@@ -772,7 +791,7 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
772791
break;
773792
} else if (Token::Match(tok1->tokAt(-2), "&&|%oror% !")) {
774793
break;
775-
} else if (CheckNullPointer::isPointerDeRef(tok1, unknown)) {
794+
} else if (CheckNullPointer::isPointerDeRef(tok1, unknown, symbolDatabase)) {
776795
nullPointerError(tok1, varname, tok->linenr(), inconclusive);
777796
break;
778797
} else if (Token::simpleMatch(tok1->previous(), "&")) {
@@ -891,16 +910,22 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
891910
}
892911
}
893912

894-
if (Token::Match(tok2, "goto|return|continue|break|throw|if|switch|for")) {
895-
bool dummy = false;
896-
if (Token::Match(tok2, "return * %varid%", varid))
897-
nullPointerError(tok2, pointerName, linenr, inconclusive);
898-
else if (Token::Match(tok2, "return %varid%", varid) &&
899-
CheckNullPointer::isPointerDeRef(tok2->next(), dummy))
900-
nullPointerError(tok2, pointerName, linenr, inconclusive);
913+
if (tok2->str() == "return" || tok2->str() == "throw") {
914+
bool unknown = _settings->inconclusive;
915+
for (; tok2 && tok2->str() != ";"; tok2 = tok2->next()) {
916+
if (tok2->varId() == varid) {
917+
if (CheckNullPointer::isPointerDeRef(tok2, unknown, symbolDatabase))
918+
nullPointerError(tok2, pointerName, linenr, inconclusive);
919+
else if (unknown)
920+
nullPointerError(tok2, pointerName, linenr, true);
921+
}
922+
}
901923
break;
902924
}
903925

926+
if (Token::Match(tok2, "goto|continue|break|if|switch|for"))
927+
break;
928+
904929
// parameters to sizeof are not dereferenced
905930
if (Token::Match(tok2, "decltype|sizeof")) {
906931
if (tok2->strAt(1) != "(")
@@ -949,7 +974,7 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
949974
if (Token::Match(tok2->previous(), "[;{}=] %var% = 0 ;"))
950975
;
951976

952-
else if (CheckNullPointer::isPointerDeRef(tok2, unknown))
977+
else if (CheckNullPointer::isPointerDeRef(tok2, unknown, symbolDatabase))
953978
nullPointerError(tok2, pointerName, linenr, inconclusive);
954979

955980
else if (unknown && _settings->inconclusive)
@@ -982,15 +1007,18 @@ void CheckNullPointer::nullConstantDereference()
9821007
continue;
9831008

9841009
for (const Token *tok = i->classStart; tok != i->classEnd; tok = tok->next()) {
985-
if (tok->str() == "(" && Token::Match(tok->previous(), "sizeof|decltype|typeid"))
986-
tok = tok->link();
1010+
if (Token::Match(tok, "sizeof|decltype|typeid ("))
1011+
tok = tok->next()->link();
9871012

9881013
else if (Token::simpleMatch(tok, "* 0")) {
989-
if (Token::Match(tok->previous(), "return|;|{|}|=|(|,|%op%")) {
1014+
if (Token::Match(tok->previous(), "return|throw|;|{|}|:|[|(|,") || tok->previous()->isOp() || tok->previous()->isAssignmentOp()) {
9901015
nullPointerError(tok);
9911016
}
9921017
}
9931018

1019+
else if (Token::Match(tok, "0 [") && (tok->previous()->str() != "&" || !Token::Match(tok->next()->link()->next(), "[.(]")))
1020+
nullPointerError(tok);
1021+
9941022
else if (Token::Match(tok->previous(), "[={};] %var% (")) {
9951023
std::list<const Token *> var;
9961024
parseFunctionCall(*tok, var, 0);
@@ -1001,6 +1029,20 @@ void CheckNullPointer::nullConstantDereference()
10011029
nullPointerError(*it);
10021030
}
10031031
}
1032+
} else if (Token::Match(tok, "std :: string ( 0 )"))
1033+
nullPointerError(tok);
1034+
if (Token::Match(tok, "std :: string %var% ( 0 )"))
1035+
nullPointerError(tok);
1036+
1037+
unsigned int ovarid = 0;
1038+
if (Token::Match(tok, "0 ==|!= %var%"))
1039+
ovarid = tok->tokAt(2)->varId();
1040+
else if (Token::Match(tok, "%var% ==|!=|=|+= 0"))
1041+
ovarid = tok->varId();
1042+
if (ovarid) {
1043+
const Variable* var = symbolDatabase->getVariableFromVarId(ovarid);
1044+
if (var && !var->isPointer() && !var->isArray() && Token::Match(var->typeStartToken(), "const| std :: string !!::"))
1045+
nullPointerError(tok);
10041046
}
10051047
}
10061048
}
@@ -1018,13 +1060,16 @@ void CheckNullPointer::nullConstantDereference()
10181060
class Nullpointer : public ExecutionPath {
10191061
public:
10201062
/** Startup constructor */
1021-
explicit Nullpointer(Check *c) : ExecutionPath(c, 0), null(false) {
1063+
Nullpointer(Check *c, const SymbolDatabase* symbolDatabase_) : ExecutionPath(c, 0), symbolDatabase(symbolDatabase_), null(false) {
10221064
}
10231065

10241066
private:
1067+
const SymbolDatabase* symbolDatabase;
1068+
10251069
/** Create checking of specific variable: */
1026-
Nullpointer(Check *c, const unsigned int id, const std::string &name)
1070+
Nullpointer(Check *c, const unsigned int id, const std::string &name, const SymbolDatabase* symbolDatabase_)
10271071
: ExecutionPath(c, id),
1072+
symbolDatabase(symbolDatabase_),
10281073
varname(name),
10291074
null(false) {
10301075
}
@@ -1082,56 +1127,22 @@ class Nullpointer : public ExecutionPath {
10821127

10831128
/** parse tokens */
10841129
const Token *parse(const Token &tok, std::list<ExecutionPath *> &checks) const {
1085-
if (Token::Match(tok.previous(), "[;{}] const| struct| %type% * %var% ;")) {
1086-
const Token * vartok = tok.tokAt(2);
1087-
1088-
if (tok.str() == "const")
1089-
vartok = vartok->next();
1090-
1091-
if (tok.str() == "struct")
1092-
vartok = vartok->next();
1093-
1094-
if (vartok->varId() != 0)
1095-
checks.push_back(new Nullpointer(owner, vartok->varId(), vartok->str()));
1096-
return vartok->next();
1097-
}
1098-
1099-
// Template pointer variable..
1100-
if (Token::Match(tok.previous(), "[;{}] %type% ::|<")) {
1101-
const Token * vartok = &tok;
1102-
while (Token::Match(vartok, "%type% ::"))
1103-
vartok = vartok->tokAt(2);
1104-
if (Token::Match(vartok, "%type% < %type%")) {
1105-
vartok = vartok->tokAt(3);
1106-
while (vartok && (vartok->str() == "*" || vartok->isName()))
1107-
vartok = vartok->next();
1108-
}
1109-
if (vartok
1110-
&& (vartok->str() == ">" || vartok->isName())
1111-
&& Token::Match(vartok->next(), "* %var% ;|=")) {
1112-
vartok = vartok->tokAt(2);
1113-
checks.push_back(new Nullpointer(owner, vartok->varId(), vartok->str()));
1114-
if (Token::simpleMatch(vartok->next(), "= 0 ;"))
1115-
setnull(checks, vartok->varId());
1116-
return vartok->next();
1117-
}
1130+
if (tok.varId() != 0) {
1131+
// Pointer declaration declaration?
1132+
const Variable* var = symbolDatabase->getVariableFromVarId(tok.varId());
1133+
if (var && var->isPointer() && var->nameToken() == &tok)
1134+
checks.push_back(new Nullpointer(owner, var->varId(), var->name(), symbolDatabase));
11181135
}
11191136

11201137
if (Token::simpleMatch(&tok, "try {")) {
11211138
// Bail out all used variables
1122-
unsigned int indentlevel = 0;
1123-
for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) {
1124-
if (tok2->str() == "{")
1125-
++indentlevel;
1126-
else if (tok2->str() == "}") {
1127-
if (indentlevel == 0)
1128-
break;
1129-
if (indentlevel == 1 && !Token::simpleMatch(tok2,"} catch ("))
1130-
return tok2;
1131-
--indentlevel;
1132-
} else if (tok2->varId())
1139+
const Token* tok2 = &tok;
1140+
const Token* end = tok.linkAt(1);
1141+
for (; tok2 && tok2 != end; tok2 = tok2->next()) {
1142+
if (tok2->varId())
11331143
bailOutVar(checks,tok2->varId());
11341144
}
1145+
return tok2;
11351146
}
11361147

11371148
if (Token::Match(&tok, "%var% (")) {
@@ -1149,21 +1160,21 @@ class Nullpointer : public ExecutionPath {
11491160
return tok.link();
11501161

11511162
if (tok.varId() != 0) {
1152-
// unknown : not really used. it is passed to isPointerDeRef.
1153-
// if isPointerDeRef fails to determine if there
1154-
// is a dereference the this will be set to true.
1163+
// unknown: if isPointerDeRef fails to determine if there
1164+
// is a dereference this will be set to true.
11551165
bool unknown = owner->inconclusiveFlag();
1166+
bool deref = CheckNullPointer::isPointerDeRef(&tok, unknown, symbolDatabase);
11561167

1157-
if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;"))
1158-
setnull(checks, tok.varId());
1159-
else if (CheckNullPointer::isPointerDeRef(&tok, unknown))
1168+
if (deref)
11601169
dereference(checks, &tok);
11611170
else if (unknown && owner->inconclusiveFlag())
11621171
dereference(checks, &tok);
1163-
else
1164-
// TODO: Report debug warning that it's unknown if a
1165-
// pointer is dereferenced
1166-
bailOutVar(checks, tok.varId());
1172+
if (Token::Match(tok.previous(), "[;{}=] %var% = 0 ;"))
1173+
setnull(checks, tok.varId());
1174+
else if (!deref &&
1175+
!tok.previous()->isOp() && !tok.previous()->isAssignmentOp() &&
1176+
(!tok.next()->isOp() || tok.next()->str() == ">>"))
1177+
bailOutVar(checks, tok.varId()); // If its possible that the pointers value changes, bail out.
11671178
}
11681179

11691180
else if (tok.str() == "delete") {
@@ -1175,13 +1186,15 @@ class Nullpointer : public ExecutionPath {
11751186
}
11761187

11771188
else if (tok.str() == "return") {
1178-
bool unknown = false;
1179-
const Token *vartok = tok.next();
1180-
if (vartok->str() == "*")
1181-
vartok = vartok->next();
1182-
if (vartok->varId() && CheckNullPointer::isPointerDeRef(vartok, unknown)) {
1183-
dereference(checks, vartok);
1189+
bool unknown = owner->inconclusiveFlag();
1190+
const Token* tok2 = &tok;
1191+
for (; tok2 && tok2->str() != ";"; tok2 = tok2->next()) {
1192+
if (tok2->varId()) {
1193+
if (CheckNullPointer::isPointerDeRef(tok2, unknown, symbolDatabase) || unknown)
1194+
dereference(checks, tok2);
1195+
}
11841196
}
1197+
return tok2;
11851198
}
11861199

11871200
return &tok;
@@ -1192,8 +1205,9 @@ class Nullpointer : public ExecutionPath {
11921205
for (const Token *tok2 = &tok; tok2; tok2 = tok2->next()) {
11931206
if (tok2->str() == "(" || tok2->str() == ")")
11941207
break;
1195-
if (Token::Match(tok2, "[<>=] * %var%"))
1196-
dereference(checks, tok2->tokAt(2));
1208+
bool unknown = owner->inconclusiveFlag();
1209+
if (tok2->varId() && (CheckNullPointer::isPointerDeRef(tok2, unknown, symbolDatabase) || unknown))
1210+
dereference(checks, tok2);
11971211
}
11981212

11991213
if (Token::Match(&tok, "!| %var% (")) {
@@ -1224,7 +1238,7 @@ class Nullpointer : public ExecutionPath {
12241238
void CheckNullPointer::executionPaths()
12251239
{
12261240
// Check for null pointer errors..
1227-
Nullpointer c(this);
1241+
Nullpointer c(this, _tokenizer->getSymbolDatabase());
12281242
checkExecutionPaths(_tokenizer->tokens(), &c);
12291243
}
12301244

@@ -1246,6 +1260,3 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
12461260
else
12471261
reportError(tok, Severity::error, "nullPointer", errmsg);
12481262
}
1249-
1250-
1251-

‎lib/checknullpointer.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "settings.h"
2727

2828
class Token;
29+
class SymbolDatabase;
2930

3031
/// @addtogroup Checks
3132
/// @{
@@ -80,7 +81,7 @@ class CheckNullPointer : public Check {
8081
* @param unknown it is not known if there is a pointer dereference (could be reported as a debug message)
8182
* @return true => there is a dereference
8283
*/
83-
static bool isPointerDeRef(const Token *tok, bool &unknown);
84+
static bool isPointerDeRef(const Token *tok, bool &unknown, const SymbolDatabase* symbolDatabase);
8485

8586
/** @brief possible null pointer dereference */
8687
void nullPointer();

0 commit comments

Comments
 (0)