Skip to content

Commit 18eb218

Browse files
chrchr-githubchrchr-github
authored
Fix #14175 duplicated nullPointer (#8393)
1 parent b9e6ace commit 18eb218

3 files changed

Lines changed: 29 additions & 38 deletions

File tree

‎lib/checknullpointer.cpp‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,13 @@ static bool checkNullpointerFunctionCallPlausibility(const Function* func, unsig
5959
return !func || (func->argCount() >= arg && func->getArgumentVar(arg - 1) && func->getArgumentVar(arg - 1)->isPointer());
6060
}
6161

62-
/**
63-
* @brief parse a function call and extract information about variable usage
64-
* @param tok first token
65-
* @param var variables that the function read / write.
66-
* @param library --library files data
67-
* @param checkNullArg perform isnullargbad check for each argument?
68-
*/
69-
void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token *> &var, const Library &library, bool checkNullArg)
62+
std::list<const Token*> CheckNullPointer::parseFunctionCall(const Token &tok, const Library &library, bool checkNullArg)
7063
{
7164
if (Token::Match(&tok, "%name% ( )") || !tok.tokAt(2))
72-
return;
65+
return {};
7366

7467
const std::vector<const Token *> args = getArguments(&tok);
75-
68+
std::list<const Token*> var;
7669
for (int argnr = 1; argnr <= args.size(); ++argnr) {
7770
const Token *param = args[argnr - 1];
7871
if ((!checkNullArg || library.isnullargbad(&tok, argnr)) && checkNullpointerFunctionCallPlausibility(tok.function(), argnr))
@@ -87,14 +80,14 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
8780
if (library.formatstr_function(&tok)) {
8881
const int formatStringArgNr = library.formatstr_argno(&tok);
8982
if (formatStringArgNr < 0 || formatStringArgNr >= args.size())
90-
return;
83+
return var;
9184

9285
// 1st parameter..
9386
if (Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && args.size() > 1 && !(args[1] && args[1]->hasKnownIntValue() && args[1]->getKnownIntValue() == 0)) // Only if length (second parameter) is not zero
9487
var.push_back(args[0]);
9588

9689
if (args[formatStringArgNr]->tokType() != Token::eString)
97-
return;
90+
return var;
9891
const std::string &formatString = args[formatStringArgNr]->strValue();
9992
int argnr = formatStringArgNr + 1;
10093
const bool scan = library.formatstr_scan(&tok);
@@ -116,7 +109,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
116109
}
117110
++i;
118111
if (i == formatString.end())
119-
return;
112+
return var;
120113
}
121114
if (_continue)
122115
continue;
@@ -129,6 +122,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
129122
}
130123
}
131124
}
125+
return var;
132126
}
133127

134128
namespace {
@@ -166,8 +160,7 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
166160
ftok = ftok->previous();
167161
}
168162
if (ftok && ftok->previous()) {
169-
std::list<const Token *> varlist;
170-
parseFunctionCall(*ftok->previous(), varlist, settings.library, checkNullArg);
163+
const std::list<const Token *> varlist = parseFunctionCall(*ftok->previous(), settings.library, checkNullArg);
171164
if (std::find(varlist.cbegin(), varlist.cend(), tok) != varlist.cend()) {
172165
return true;
173166
}
@@ -376,8 +369,7 @@ void CheckNullPointer::nullConstantDereference()
376369
if (var && !var->isPointer() && !var->isArray() && var->isStlStringType())
377370
nullPointerError(tok);
378371
} else { // function call
379-
std::list<const Token *> var;
380-
parseFunctionCall(*tok, var, mSettings->library);
372+
const std::list<const Token *> var = parseFunctionCall(*tok, mSettings->library);
381373

382374
// is one of the var items a NULL pointer?
383375
for (const Token *vartok : var) {
@@ -456,6 +448,8 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
456448
reportError(tok, Severity::warning, "nullPointerOutOfResources", "Null pointer dereference", CWE_NULL_POINTER_DEREFERENCE, Certainty::normal);
457449
return;
458450
}
451+
if (diag(tok))
452+
return;
459453

460454
if (!value) {
461455
reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE_NULL_POINTER_DEREFERENCE, inconclusive ? Certainty::inconclusive : Certainty::normal);

‎lib/checknullpointer.h‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "config.h"
2727

2828
#include <list>
29+
#include <set>
2930
#include <string>
3031

3132
class ErrorLogger;
@@ -69,12 +70,11 @@ class CPPCHECKLIB CheckNullPointer : public Check {
6970
/**
7071
* @brief parse a function call and extract information about variable usage
7172
* @param tok first token
72-
* @param var variables that the function read / write.
7373
* @param library --library files data
74+
* @param checkNullArg perform isnullargbad check for each argument?
75+
* @return list of variables that the function reads / writes.
7476
*/
75-
static void parseFunctionCall(const Token &tok,
76-
std::list<const Token *> &var,
77-
const Library &library, bool checkNullArg = true);
77+
static std::list<const Token*> parseFunctionCall(const Token &tok, const Library &library, bool checkNullArg = true);
7878

7979
/** @brief This constructor is used when running checks. */
8080
CheckNullPointer(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
@@ -125,6 +125,12 @@ class CPPCHECKLIB CheckNullPointer : public Check {
125125
void arithmetic();
126126
void pointerArithmeticError(const Token* tok, const ValueFlow::Value *value, bool inconclusive);
127127
void redundantConditionWarning(const Token* tok, const ValueFlow::Value *value, const Token *condition, bool inconclusive);
128+
129+
bool diag(const Token* tok) {
130+
return !mDiag.emplace(tok).second;
131+
}
132+
133+
std::set<const Token*> mDiag;
128134
};
129135
/// @}
130136
//---------------------------------------------------------------------------

‎test/testnullpointer.cpp‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3489,8 +3489,7 @@ class TestNullPointer : public TestFixture {
34893489
" printf(\"%s\", s);\n"
34903490
"}");
34913491
ASSERT_EQUALS(
3492-
"[test.cpp:3:18]: (error) Null pointer dereference: s [nullPointer]\n"
3493-
"[test.cpp:3:18]: (error) Null pointer dereference [nullPointer]\n",
3492+
"[test.cpp:3:18]: (error) Null pointer dereference: s [nullPointer]\n",
34943493
errout_str());
34953494

34963495
check("void f() {\n"
@@ -3514,8 +3513,7 @@ class TestNullPointer : public TestFixture {
35143513
" printf(\"%u%s\", 123, s);\n"
35153514
"}");
35163515
ASSERT_EQUALS(
3517-
"[test.cpp:3:25]: (error) Null pointer dereference: s [nullPointer]\n"
3518-
"[test.cpp:3:25]: (error) Null pointer dereference [nullPointer]\n",
3516+
"[test.cpp:3:25]: (error) Null pointer dereference: s [nullPointer]\n",
35193517
errout_str());
35203518

35213519

@@ -3560,16 +3558,14 @@ class TestNullPointer : public TestFixture {
35603558
" sscanf(s, \"%s\", 0);\n"
35613559
"}");
35623560
ASSERT_EQUALS(
3563-
"[test.cpp:2:21]: (error) Null pointer dereference [nullPointer]\n"
3564-
"[test.cpp:2:21]: (error) Null pointer dereference [nullPointer]\n", // duplicate
3561+
"[test.cpp:2:21]: (error) Null pointer dereference [nullPointer]\n",
35653562
errout_str());
35663563

35673564
check("void f() {\n"
35683565
" scanf(\"%d\", 0);\n"
35693566
"}");
35703567
ASSERT_EQUALS(
3571-
"[test.cpp:2:17]: (error) Null pointer dereference [nullPointer]\n"
3572-
"[test.cpp:2:17]: (error) Null pointer dereference [nullPointer]\n", // duplicate
3568+
"[test.cpp:2:17]: (error) Null pointer dereference [nullPointer]\n",
35733569
errout_str());
35743570

35753571
check("void f(char* foo) {\n"
@@ -3590,9 +3586,7 @@ class TestNullPointer : public TestFixture {
35903586
" sscanf(dummy, \"%d\", iVal);\n"
35913587
"}");
35923588
ASSERT_EQUALS(
3593-
"[test.cpp:3:25]: (error) Null pointer dereference: iVal [nullPointer]\n"
3594-
"[test.cpp:3:25]: (error) Null pointer dereference [nullPointer]\n"
3595-
"[test.cpp:3:25]: (error) Null pointer dereference [nullPointer]\n", // duplicate
3589+
"[test.cpp:3:25]: (error) Null pointer dereference: iVal [nullPointer]\n",
35963590
errout_str());
35973591

35983592
check("void f(char *dummy) {\n"
@@ -3611,8 +3605,7 @@ class TestNullPointer : public TestFixture {
36113605
" sscanf(dummy, \"%*d%u\", 0);\n"
36123606
"}");
36133607
ASSERT_EQUALS(
3614-
"[test.cpp:2:28]: (error) Null pointer dereference [nullPointer]\n"
3615-
"[test.cpp:2:28]: (error) Null pointer dereference [nullPointer]\n", // duplicate
3608+
"[test.cpp:2:28]: (error) Null pointer dereference [nullPointer]\n",
36163609
errout_str());
36173610
}
36183611

@@ -4333,8 +4326,7 @@ class TestNullPointer : public TestFixture {
43334326
Library library;
43344327
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
43354328

4336-
std::list<const Token *> null;
4337-
CheckNullPointer::parseFunctionCall(*xtok, null, library);
4329+
const std::list<const Token *> null = CheckNullPointer::parseFunctionCall(*xtok, library);
43384330
ASSERT_EQUALS(0U, null.size());
43394331
}
43404332

@@ -4352,8 +4344,7 @@ class TestNullPointer : public TestFixture {
43524344
Library library;
43534345
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
43544346

4355-
std::list<const Token *> null;
4356-
CheckNullPointer::parseFunctionCall(*xtok, null, library);
4347+
const std::list<const Token *> null = CheckNullPointer::parseFunctionCall(*xtok, library);
43574348
ASSERT_EQUALS(1U, null.size());
43584349
ASSERT_EQUALS("a", null.front()->str());
43594350
}

0 commit comments

Comments
 (0)