From 49982485010a48953b60e0252ec39de48962b34d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 4 Feb 2018 15:29:57 +0100 Subject: [PATCH] Null pointers: Fixed false positives when running whole program analysis. Copied the fix from the CheckUninitVar::isUnsafeFunction. --- lib/checknullpointer.cpp | 10 ++++++- lib/checknullpointer.h | 2 +- lib/checkuninitvar.cpp | 3 +- test/testnullpointer.cpp | 63 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 0233e2214..df4aaf14a 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -20,6 +20,7 @@ //--------------------------------------------------------------------------- #include "checknullpointer.h" +#include "astutils.h" #include "errorlogger.h" #include "library.h" #include "settings.h" @@ -548,12 +549,19 @@ void CheckNullPointer::arithmeticError(const Token *tok, const ValueFlow::Value value && value->isInconclusive()); } -bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) +bool CheckNullPointer::isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const { const Variable * const argvar = scope->function->getArgumentVar(argnr); if (!argvar->isPointer()) return false; for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) { + if (Token::simpleMatch(tok2, ") {")) { + tok2 = tok2->linkAt(1); + if (Token::findmatch(tok2->link(), "return|throw", tok2)) + return false; + if (isVariableChanged(tok2->link(), tok2, argvar->declarationId(), false, _settings)) + return false; + } if (tok2->variable() != argvar) continue; if (!Token::Match(tok2->astParent(), "*|[")) diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index a927549c0..ff89c8aac 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -100,7 +100,7 @@ public: } void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive); - static bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok); + bool isUnsafeFunction(const Scope *scope, int argnr, const Token **tok) const; private: /** Get error messages. Used by --errorlist */ diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 2cc9125b2..f3e2c98d5 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1425,11 +1425,12 @@ Check::FileInfo *CheckUninitVar::getFileInfo() const } // "Unsafe" functions unconditionally reads data before it is written.. + CheckNullPointer checkNullPointer(_tokenizer, _settings, _errorLogger); for (int argnr = 0; argnr < function->argCount(); ++argnr) { const Token *tok; if (isUnsafeFunction(&*scope, argnr, &tok)) fileInfo->readData.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok)); - if (CheckNullPointer::isUnsafeFunction(&*scope, argnr, &tok)) + if (checkNullPointer.isUnsafeFunction(&*scope, argnr, &tok)) fileInfo->dereferenced.push_back(MyFileInfo::FunctionArg(_tokenizer, &*scope, argnr+1, tok)); } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 4eb269f83..388b0e36a 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -17,6 +17,7 @@ */ #include "checknullpointer.h" +#include "checkuninitvar.h" #include "library.h" #include "settings.h" #include "testsuite.h" @@ -105,6 +106,7 @@ private: TEST_CASE(nullpointer_internal_error); // #5080 TEST_CASE(ticket6505); TEST_CASE(subtract); + TEST_CASE(ctu); } void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") { @@ -2572,6 +2574,67 @@ private: "}\n"); 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()); } + + void ctu(const char code[]) { + // Clear the error buffer.. + errout.str(""); + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList2(); + + // Check code.. + std::list fileInfo; + CheckUninitVar check(&tokenizer, &settings, this); + fileInfo.push_back(check.getFileInfo(&tokenizer, &settings)); + check.analyseWholeProgram(fileInfo, settings, *this); + while (!fileInfo.empty()) { + delete fileInfo.back(); + fileInfo.pop_back(); + } + } + + void ctu() { + ctu("void f(int *fp) {\n" + " a = *fp;\n" + "}\n" + "int main() {\n" + " int *p = 0;\n" + " f(p);\n" + "}"); + ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:2]: (error) Null pointer dereference: fp\n", errout.str()); + + ctu("void use(int *p) { a = *p + 3; }\n" + "void call(int x, int *p) { x++; use(p); }\n" + "int main() {\n" + " call(4,0);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (error) Null pointer dereference: p\n", errout.str()); + + ctu("void dostuff(int *x, int *y) {\n" + " if (!var)\n" + " return -1;\n" // <- early return + " *x = *y;\n" + "}\n" + "\n" + "void f() {\n" + " dostuff(a, 0);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + ctu("void dostuff(int *x, int *y) {\n" + " if (cond)\n" + " *y = -1;\n" // <- conditionally written + " *x = *y;\n" + "}\n" + "\n" + "void f() {\n" + " dostuff(a, 0);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestNullPointer)