Skip to content

Commit 4998248

Browse files
danmardanmar
committed
Null pointers: Fixed false positives when running whole program analysis. Copied the fix from the CheckUninitVar::isUnsafeFunction.
1 parent a61f21d commit 4998248

4 files changed

Lines changed: 75 additions & 3 deletions

File tree

‎lib/checknullpointer.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
//---------------------------------------------------------------------------
2121
#include "checknullpointer.h"
2222

23+
#include "astutils.h"
2324
#include "errorlogger.h"
2425
#include "library.h"
2526
#include "settings.h"
@@ -548,12 +549,19 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value
548549
value && value->isInconclusive());
549550
}
550551

551-
bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok)
552+
bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const
552553
{
553554
const Variable * const argvar = scope->function->getArgumentVar(argnr);
554555
if (!argvar->isPointer())
555556
return false;
556557
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
558+
if (Token::simpleMatch(tok2, ") {")) {
559+
tok2 = tok2->linkAt(1);
560+
if (Token::findmatch(tok2->link(), "return|throw", tok2))
561+
return false;
562+
if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, _settings))
563+
return false;
564+
}
557565
if (tok2->variable() != argvar)
558566
continue;
559567
if (!Token::Match(tok2->astParent(), "*|["))

‎lib/checknullpointer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class CPPCHECKLIB CheckNullPointer : public Check {
100100
}
101101
void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive);
102102

103-
static bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok);
103+
bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const;
104104
private:
105105

106106
/** Get error messages. Used by --errorlist */

‎lib/checkuninitvar.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1425,11 +1425,12 @@ Check::FileInfo *CheckUninitVar::getFileInfo() const
14251425
}
14261426

14271427
// "Unsafe" functions unconditionally reads data before it is written..
1428+
CheckNullPointer checkNullPointer(_tokenizer, _settings, _errorLogger);
14281429
for (int argnr = 0; argnr < function->argCount(); ++argnr) {
14291430
const Token *tok;
14301431
if (isUnsafeFunction(&*scope, argnr, &tok))
14311432
fileInfo->readData.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
1432-
if (CheckNullPointer::isUnsafeFunction(&*scope, argnr, &tok))
1433+
if (checkNullPointer.isUnsafeFunction(&*scope, argnr, &tok))
14331434
fileInfo->dereferenced.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok));
14341435
}
14351436

‎test/testnullpointer.cpp‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818

1919
#include "checknullpointer.h"
20+
#include "checkuninitvar.h"
2021
#include "library.h"
2122
#include "settings.h"
2223
#include "testsuite.h"
@@ -105,6 +106,7 @@ class TestNullPointer : public TestFixture {
105106
TEST_CASE(nullpointer_internal_error); // #5080
106107
TEST_CASE(ticket6505);
107108
TEST_CASE(subtract);
109+
TEST_CASE(ctu);
108110
}
109111

110112
void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") {
@@ -2572,6 +2574,67 @@ class TestNullPointer : public TestFixture {
25722574
"}\n");
25732575
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!s' is redundant or there is overflow in pointer subtraction.\n", errout.str());
25742576
}
2577+
2578+
void ctu(const char code[]) {
2579+
// Clear the error buffer..
2580+
errout.str("");
2581+
2582+
// Tokenize..
2583+
Tokenizer tokenizer(&settings, this);
2584+
std::istringstream istr(code);
2585+
tokenizer.tokenize(istr, "test.cpp");
2586+
tokenizer.simplifyTokenList2();
2587+
2588+
// Check code..
2589+
std::list<Check::FileInfo*> fileInfo;
2590+
CheckUninitVar check(&tokenizer, &settings, this);
2591+
fileInfo.push_back(check.getFileInfo(&tokenizer, &settings));
2592+
check.analyseWholeProgram(fileInfo, settings, *this);
2593+
while (!fileInfo.empty()) {
2594+
delete fileInfo.back();
2595+
fileInfo.pop_back();
2596+
}
2597+
}
2598+
2599+
void ctu() {
2600+
ctu("void f(int *fp) {\n"
2601+
" a = *fp;\n"
2602+
"}\n"
2603+
"int main() {\n"
2604+
" int *p = 0;\n"
2605+
" f(p);\n"
2606+
"}");
2607+
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Null pointer dereference: fp\n", errout.str());
2608+
2609+
ctu("void use(int *p) { a = *p + 3; }\n"
2610+
"void call(int x, int *p) { x++; use(p); }\n"
2611+
"int main() {\n"
2612+
" call(4,0);\n"
2613+
"}");
2614+
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (error) Null pointer dereference: p\n", errout.str());
2615+
2616+
ctu("void dostuff(int *x, int *y) {\n"
2617+
" if (!var)\n"
2618+
" return -1;\n" // <- early return
2619+
" *x = *y;\n"
2620+
"}\n"
2621+
"\n"
2622+
"void f() {\n"
2623+
" dostuff(a, 0);\n"
2624+
"}");
2625+
ASSERT_EQUALS("", errout.str());
2626+
2627+
ctu("void dostuff(int *x, int *y) {\n"
2628+
" if (cond)\n"
2629+
" *y = -1;\n" // <- conditionally written
2630+
" *x = *y;\n"
2631+
"}\n"
2632+
"\n"
2633+
"void f() {\n"
2634+
" dostuff(a, 0);\n"
2635+
"}");
2636+
ASSERT_EQUALS("", errout.str());
2637+
}
25752638
};
25762639

25772640
REGISTER_TEST(TestNullPointer)

0 commit comments

Comments
 (0)